Allow dots in app names (closes #260)
Check / check (pull_request) Skipped
Check / check (pull_request) Skipped
App names may now contain dots, such as sneak.berlin: runs of lowercase letters and numbers joined by single dots or by hyphens, 2 to 63 characters. Docker accepts every such name in the app's image name, upaas-<name>; a dot needs a letter or number on both sides because Docker requires it. The rule also keeps a dot off either end, so the name is safe as a directory and log file name. The new and edit app forms use the same pattern. Their old one was not a valid regular expression under the flag browsers compile it with, so browsers ignored it. A test deploys an app named sneak.berlin against the fake Docker API and checks the image and container names with Docker's own rules. Model: opus-5-5
This commit is contained in:
@@ -13,12 +13,15 @@ const (
|
||||
appNameMaxLength = 63
|
||||
)
|
||||
|
||||
// validAppNameRe matches names containing only lowercase alphanumeric characters and
|
||||
// hyphens, starting and ending with an alphanumeric character.
|
||||
var validAppNameRe = regexp.MustCompile(`^[a-z0-9][a-z0-9-]*[a-z0-9]$`)
|
||||
// validAppNameRe matches runs of lowercase letters and digits joined by
|
||||
// single dots or by hyphens, such as "my-app" or "sneak.berlin". Docker
|
||||
// accepts every name it allows as the app's image name, upaas-<name>; a
|
||||
// dot needs a letter or digit on both sides because Docker requires it.
|
||||
// It also keeps the name from being "." or ".." or starting or ending
|
||||
// with a dot, so it is safe as a directory and file name. The pattern
|
||||
// attribute of the name field on the new and edit app forms is the same.
|
||||
var validAppNameRe = regexp.MustCompile(`^[a-z0-9]+((\.|-+)[a-z0-9]+)*$`)
|
||||
|
||||
// validateAppName checks that the given app name is safe for use in Docker
|
||||
// container names, image tags, and file system paths.
|
||||
var (
|
||||
errAppNameLength = errors.New(
|
||||
"app name must be between " +
|
||||
@@ -26,11 +29,14 @@ var (
|
||||
strconv.Itoa(appNameMaxLength) + " characters",
|
||||
)
|
||||
errAppNamePattern = errors.New(
|
||||
"app name must contain only lowercase letters, numbers, " +
|
||||
"and hyphens, and must start and end with a letter or number",
|
||||
"app name must contain only lowercase letters, numbers, hyphens, " +
|
||||
"and dots, must start and end with a letter or number, " +
|
||||
"and must have a letter or number on both sides of each dot",
|
||||
)
|
||||
)
|
||||
|
||||
// validateAppName checks that the given app name is safe for use in Docker
|
||||
// container names, image tags, and file system paths.
|
||||
func validateAppName(name string) error {
|
||||
if len(name) < appNameMinLength || len(name) > appNameMaxLength {
|
||||
return errAppNameLength
|
||||
|
||||
@@ -18,6 +18,10 @@ func TestValidateAppName(t *testing.T) {
|
||||
{"valid two chars", "ab", false},
|
||||
{"valid complex", "my-cool-app-v2", false},
|
||||
{"valid all numbers", "123", false},
|
||||
{"valid double hyphen", "my--app", false},
|
||||
{"valid domain", "sneak.berlin", false},
|
||||
{"valid two dots", "www.sneak.berlin", false},
|
||||
{"valid dot and hyphen", "my-app.example.com", false},
|
||||
{"empty", "", true},
|
||||
{"single char", "a", true},
|
||||
{"too long", "a" + string(make([]byte, 63)), true},
|
||||
@@ -36,7 +40,12 @@ func TestValidateAppName(t *testing.T) {
|
||||
{"starts with hyphen", "-myapp", true},
|
||||
{"ends with hyphen", "myapp-", true},
|
||||
{"underscore", "my_app", true},
|
||||
{"dot", "my.app", true},
|
||||
{"two dots in a row", "a..b", true},
|
||||
{"starts with dot", ".a", true},
|
||||
{"ends with dot", "a.", true},
|
||||
{"only dots", "..", true},
|
||||
{"hyphen before dot", "a-.b", true},
|
||||
{"hyphen after dot", "a.-b", true},
|
||||
{"slash", "my/app", true},
|
||||
{"path traversal", "../etc/passwd", true},
|
||||
{"special chars", "app@name!", true},
|
||||
|
||||
@@ -0,0 +1,57 @@
|
||||
package deploy_test
|
||||
|
||||
import (
|
||||
"context"
|
||||
"slices"
|
||||
"testing"
|
||||
|
||||
"github.com/distribution/reference"
|
||||
"github.com/docker/docker/daemon/names"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
|
||||
"sneak.berlin/go/upaas/internal/models"
|
||||
)
|
||||
|
||||
// TestDeployAppWithDotInName deploys an app named sneak.berlin against a
|
||||
// fake Docker API and checks that Docker accepts the names of the image it
|
||||
// builds and of the container it runs, using Docker's own rules for each.
|
||||
func TestDeployAppWithDotInName(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
api := &fakeImageAPI{
|
||||
images: map[string][]string{},
|
||||
shortSHA: "abc1234",
|
||||
nextID: "sha256:built",
|
||||
}
|
||||
svc, db := newImageTestService(t, api)
|
||||
ctx := context.Background()
|
||||
|
||||
app := saveApp(t, db, "sneak.berlin", "", "")
|
||||
|
||||
deployment := models.NewDeployment(db)
|
||||
deployment.AppID = app.ID
|
||||
require.NoError(t, deployment.Save(ctx))
|
||||
|
||||
imageID, err := svc.BuildImage(ctx, app, deployment)
|
||||
require.NoError(t, err)
|
||||
require.NoError(t, svc.DeployContainer(ctx, app, deployment, imageID))
|
||||
|
||||
images, _ := api.state()
|
||||
|
||||
api.mu.Lock()
|
||||
containers := slices.Clone(api.containers)
|
||||
api.mu.Unlock()
|
||||
|
||||
require.Equal(t, map[string][]string{
|
||||
"sha256:built": {"upaas-sneak.berlin:abc1234"},
|
||||
}, images)
|
||||
|
||||
_, err = reference.ParseNormalizedNamed("upaas-sneak.berlin:abc1234")
|
||||
require.NoError(t, err)
|
||||
|
||||
require.Equal(t, []string{"upaas-sneak.berlin"}, containers)
|
||||
assert.Regexp(t, names.RestrictedNamePattern, containers[0])
|
||||
|
||||
assert.DirExists(t, svc.GetBuildDirExported(app.Name))
|
||||
}
|
||||
@@ -35,12 +35,13 @@ import (
|
||||
// an image's last tag, or an untagged image by its ID, deletes the image.
|
||||
// It also answers the steps of a git clone that reports shortSHA.
|
||||
type fakeImageAPI struct {
|
||||
mu sync.Mutex
|
||||
images map[string][]string // image ID -> tags
|
||||
shortSHA string // the commit's short hash the clone reports
|
||||
nextID string // ID of the image the next build creates
|
||||
removed []string // each tag or ID removed
|
||||
forced bool // whether a removal was forced
|
||||
mu sync.Mutex
|
||||
images map[string][]string // image ID -> tags
|
||||
shortSHA string // the commit's short hash the clone reports
|
||||
nextID string // ID of the image the next build creates
|
||||
removed []string // each tag or ID removed
|
||||
forced bool // whether a removal was forced
|
||||
containers []string // name of each named container created
|
||||
}
|
||||
|
||||
func (api *fakeImageAPI) ServeHTTP(w http.ResponseWriter, r *http.Request) {
|
||||
@@ -67,6 +68,11 @@ func (api *fakeImageAPI) ServeHTTP(w http.ResponseWriter, r *http.Request) {
|
||||
case strings.HasSuffix(r.URL.Path, "/version"):
|
||||
_, _ = w.Write([]byte(`{"Version":"27.3.1","ApiVersion":"1.47"}`))
|
||||
case strings.HasSuffix(r.URL.Path, "/containers/create"):
|
||||
// The git clone's container has no name; the app's has.
|
||||
if containerName := r.URL.Query().Get("name"); containerName != "" {
|
||||
api.containers = append(api.containers, containerName)
|
||||
}
|
||||
|
||||
_, _ = w.Write([]byte(`{"Id":"gitcontainer"}`))
|
||||
case strings.HasSuffix(r.URL.Path, "/logs"):
|
||||
writeCloneOutput(w, api.shortSHA)
|
||||
|
||||
@@ -113,6 +113,16 @@ func (svc *Service) BuildImage(
|
||||
return svc.buildImage(ctx, app, deployment)
|
||||
}
|
||||
|
||||
// DeployContainer exposes deployContainerWithTimeout for testing.
|
||||
func (svc *Service) DeployContainer(
|
||||
ctx context.Context,
|
||||
app *models.App,
|
||||
deployment *models.Deployment,
|
||||
imageID docker.ImageID,
|
||||
) error {
|
||||
return svc.deployContainerWithTimeout(ctx, app, deployment, imageID)
|
||||
}
|
||||
|
||||
// BuildContainerOptionsExported exposes buildContainerOptions for testing.
|
||||
func (svc *Service) BuildContainerOptionsExported(
|
||||
ctx context.Context,
|
||||
|
||||
Reference in New Issue
Block a user