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 creates an app named sneak.berlin through the new app form, then builds and deploys it 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:
@@ -20,6 +20,13 @@ regress.
|
|||||||
|
|
||||||
# Completed Steps
|
# Completed Steps
|
||||||
|
|
||||||
|
- 2026-10-02: App names may contain dots, such as `sneak.berlin`: lowercase
|
||||||
|
letters and numbers joined by single dots or by hyphens, 2 to 63 characters.
|
||||||
|
Docker accepts every such name in the image name `upaas-<name>`; a dot needs a
|
||||||
|
letter or number on both sides because Docker requires it. The new and edit
|
||||||
|
app forms check the same rule; browsers ignored their old pattern, which was
|
||||||
|
not a valid regular expression there (#260).
|
||||||
|
|
||||||
- 2026-10-02: `.dockerignore` leaves out `.git/config`, so a remote URL there
|
- 2026-10-02: `.dockerignore` leaves out `.git/config`, so a remote URL there
|
||||||
that carries a credential no longer goes into the Docker build; the image
|
that carries a credential no longer goes into the Docker build; the image
|
||||||
still shows the commit it was built from (#269).
|
still shows the commit it was built from (#269).
|
||||||
|
|||||||
@@ -4,6 +4,7 @@ go 1.25
|
|||||||
|
|
||||||
require (
|
require (
|
||||||
github.com/99designs/basicauth-go v0.0.0-20230316000542-bf6f9cbbf0f8
|
github.com/99designs/basicauth-go v0.0.0-20230316000542-bf6f9cbbf0f8
|
||||||
|
github.com/distribution/reference v0.6.0
|
||||||
github.com/docker/docker v27.3.1+incompatible
|
github.com/docker/docker v27.3.1+incompatible
|
||||||
github.com/docker/go-connections v0.6.0
|
github.com/docker/go-connections v0.6.0
|
||||||
github.com/go-chi/chi/v5 v5.2.3
|
github.com/go-chi/chi/v5 v5.2.3
|
||||||
@@ -39,7 +40,6 @@ require (
|
|||||||
github.com/containerd/ttrpc v1.2.5 // indirect
|
github.com/containerd/ttrpc v1.2.5 // indirect
|
||||||
github.com/containerd/typeurl/v2 v2.2.0 // indirect
|
github.com/containerd/typeurl/v2 v2.2.0 // indirect
|
||||||
github.com/davecgh/go-spew v1.1.1 // indirect
|
github.com/davecgh/go-spew v1.1.1 // indirect
|
||||||
github.com/distribution/reference v0.6.0 // indirect
|
|
||||||
github.com/docker/go-units v0.5.0 // indirect
|
github.com/docker/go-units v0.5.0 // indirect
|
||||||
github.com/felixge/httpsnoop v1.0.4 // indirect
|
github.com/felixge/httpsnoop v1.0.4 // indirect
|
||||||
github.com/fsnotify/fsnotify v1.9.0 // indirect
|
github.com/fsnotify/fsnotify v1.9.0 // indirect
|
||||||
|
|||||||
@@ -13,12 +13,15 @@ const (
|
|||||||
appNameMaxLength = 63
|
appNameMaxLength = 63
|
||||||
)
|
)
|
||||||
|
|
||||||
// validAppNameRe matches names containing only lowercase alphanumeric characters and
|
// validAppNameRe matches runs of lowercase letters and digits joined by
|
||||||
// hyphens, starting and ending with an alphanumeric character.
|
// single dots or by hyphens, such as "my-app" or "sneak.berlin". Docker
|
||||||
var validAppNameRe = regexp.MustCompile(`^[a-z0-9][a-z0-9-]*[a-z0-9]$`)
|
// 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 (
|
var (
|
||||||
errAppNameLength = errors.New(
|
errAppNameLength = errors.New(
|
||||||
"app name must be between " +
|
"app name must be between " +
|
||||||
@@ -26,11 +29,14 @@ var (
|
|||||||
strconv.Itoa(appNameMaxLength) + " characters",
|
strconv.Itoa(appNameMaxLength) + " characters",
|
||||||
)
|
)
|
||||||
errAppNamePattern = errors.New(
|
errAppNamePattern = errors.New(
|
||||||
"app name must contain only lowercase letters, numbers, " +
|
"app name must contain only lowercase letters, numbers, hyphens, " +
|
||||||
"and hyphens, and must start and end with a letter or number",
|
"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 {
|
func validateAppName(name string) error {
|
||||||
if len(name) < appNameMinLength || len(name) > appNameMaxLength {
|
if len(name) < appNameMinLength || len(name) > appNameMaxLength {
|
||||||
return errAppNameLength
|
return errAppNameLength
|
||||||
|
|||||||
@@ -1,7 +1,16 @@
|
|||||||
package handlers //nolint:testpackage // testing unexported validateAppName
|
package handlers //nolint:testpackage // testing unexported validateAppName
|
||||||
|
|
||||||
import (
|
import (
|
||||||
|
"bytes"
|
||||||
|
"strconv"
|
||||||
|
"strings"
|
||||||
"testing"
|
"testing"
|
||||||
|
|
||||||
|
"github.com/stretchr/testify/assert"
|
||||||
|
"github.com/stretchr/testify/require"
|
||||||
|
|
||||||
|
"sneak.berlin/go/upaas/internal/models"
|
||||||
|
"sneak.berlin/go/upaas/templates"
|
||||||
)
|
)
|
||||||
|
|
||||||
func TestValidateAppName(t *testing.T) {
|
func TestValidateAppName(t *testing.T) {
|
||||||
@@ -18,6 +27,10 @@ func TestValidateAppName(t *testing.T) {
|
|||||||
{"valid two chars", "ab", false},
|
{"valid two chars", "ab", false},
|
||||||
{"valid complex", "my-cool-app-v2", false},
|
{"valid complex", "my-cool-app-v2", false},
|
||||||
{"valid all numbers", "123", 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},
|
{"empty", "", true},
|
||||||
{"single char", "a", true},
|
{"single char", "a", true},
|
||||||
{"too long", "a" + string(make([]byte, 63)), true},
|
{"too long", "a" + string(make([]byte, 63)), true},
|
||||||
@@ -36,7 +49,12 @@ func TestValidateAppName(t *testing.T) {
|
|||||||
{"starts with hyphen", "-myapp", true},
|
{"starts with hyphen", "-myapp", true},
|
||||||
{"ends with hyphen", "myapp-", true},
|
{"ends with hyphen", "myapp-", true},
|
||||||
{"underscore", "my_app", 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},
|
{"slash", "my/app", true},
|
||||||
{"path traversal", "../etc/passwd", true},
|
{"path traversal", "../etc/passwd", true},
|
||||||
{"special chars", "app@name!", true},
|
{"special chars", "app@name!", true},
|
||||||
@@ -54,3 +72,42 @@ func TestValidateAppName(t *testing.T) {
|
|||||||
})
|
})
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestValidateAppNameErrorMentionsDots(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
err := validateAppName("a..b")
|
||||||
|
|
||||||
|
require.ErrorIs(t, err, errAppNamePattern)
|
||||||
|
assert.Contains(t, err.Error(), "dots")
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestAppFormsCheckAppNameLikeServer checks that the name field on the new
|
||||||
|
// and edit app forms has the server's pattern and length limits, and that
|
||||||
|
// its hint mentions dots.
|
||||||
|
func TestAppFormsCheckAppNameLikeServer(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
pattern := strings.TrimSuffix(strings.TrimPrefix(validAppNameRe.String(), "^"), "$")
|
||||||
|
|
||||||
|
pages := map[string]map[string]any{
|
||||||
|
"app_new.html": {},
|
||||||
|
"app_edit.html": {dataKeyApp: &models.App{}},
|
||||||
|
}
|
||||||
|
|
||||||
|
for page, data := range pages {
|
||||||
|
var out bytes.Buffer
|
||||||
|
|
||||||
|
require.NoError(t, templates.GetParsed().ExecuteTemplate(&out, page, data))
|
||||||
|
|
||||||
|
// The name field and its hint, up to the end of their div.
|
||||||
|
_, field, found := strings.Cut(out.String(), `id="name"`)
|
||||||
|
require.True(t, found, page)
|
||||||
|
|
||||||
|
field, _, _ = strings.Cut(field, "</div>")
|
||||||
|
assert.Contains(t, field, `pattern="`+pattern+`"`, page)
|
||||||
|
assert.Contains(t, field, `minlength="`+strconv.Itoa(appNameMinLength)+`"`, page)
|
||||||
|
assert.Contains(t, field, `maxlength="`+strconv.Itoa(appNameMaxLength)+`"`, page)
|
||||||
|
assert.Contains(t, field, "hyphens, and dots", page)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
@@ -0,0 +1,120 @@
|
|||||||
|
package deploy_test
|
||||||
|
|
||||||
|
import (
|
||||||
|
"context"
|
||||||
|
"log/slog"
|
||||||
|
"net/http"
|
||||||
|
"net/http/httptest"
|
||||||
|
"net/url"
|
||||||
|
"os"
|
||||||
|
"slices"
|
||||||
|
"strings"
|
||||||
|
"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/database"
|
||||||
|
"sneak.berlin/go/upaas/internal/globals"
|
||||||
|
"sneak.berlin/go/upaas/internal/handlers"
|
||||||
|
"sneak.berlin/go/upaas/internal/logger"
|
||||||
|
"sneak.berlin/go/upaas/internal/models"
|
||||||
|
"sneak.berlin/go/upaas/internal/service/app"
|
||||||
|
)
|
||||||
|
|
||||||
|
// TestDeployAppWithDotInName creates an app named sneak.berlin through the
|
||||||
|
// new app form, as a user does, then builds and deploys it 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()
|
||||||
|
|
||||||
|
createdApp := createAppWithForm(t, db, "sneak.berlin")
|
||||||
|
|
||||||
|
deployment := models.NewDeployment(db)
|
||||||
|
deployment.AppID = createdApp.ID
|
||||||
|
require.NoError(t, deployment.Save(ctx))
|
||||||
|
|
||||||
|
imageID, err := svc.BuildImage(ctx, createdApp, deployment)
|
||||||
|
require.NoError(t, err)
|
||||||
|
require.NoError(t, svc.DeployContainer(ctx, createdApp, 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(createdApp.Name))
|
||||||
|
}
|
||||||
|
|
||||||
|
// createAppWithForm posts the new app form with the given name, which
|
||||||
|
// checks the name as it does for a user, and returns the app it created.
|
||||||
|
func createAppWithForm(
|
||||||
|
t *testing.T,
|
||||||
|
db *database.Database,
|
||||||
|
name string,
|
||||||
|
) *models.App {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
log := logger.NewForTest(slog.New(slog.NewTextHandler(os.Stderr, nil)))
|
||||||
|
|
||||||
|
appSvc, err := app.New(nil, app.ServiceParams{Logger: log, Database: db})
|
||||||
|
require.NoError(t, err)
|
||||||
|
|
||||||
|
globalInstance, err := globals.New(nil)
|
||||||
|
require.NoError(t, err)
|
||||||
|
|
||||||
|
handlersInstance, err := handlers.New(nil, handlers.Params{
|
||||||
|
Logger: log,
|
||||||
|
Globals: globalInstance,
|
||||||
|
Database: db,
|
||||||
|
App: appSvc,
|
||||||
|
})
|
||||||
|
require.NoError(t, err)
|
||||||
|
|
||||||
|
form := url.Values{
|
||||||
|
"name": {name},
|
||||||
|
"repo_url": {"git@example.com:sneak/" + name + ".git"},
|
||||||
|
}
|
||||||
|
request := httptest.NewRequestWithContext(
|
||||||
|
t.Context(), http.MethodPost, "/apps", strings.NewReader(form.Encode()),
|
||||||
|
)
|
||||||
|
request.Header.Set("Content-Type", "application/x-www-form-urlencoded")
|
||||||
|
recorder := httptest.NewRecorder()
|
||||||
|
|
||||||
|
handlersInstance.HandleAppCreate().ServeHTTP(recorder, request)
|
||||||
|
|
||||||
|
// The form redirects to the new app's page; it shows the form again,
|
||||||
|
// with the error, when it refuses the name.
|
||||||
|
require.Equal(t, http.StatusSeeOther, recorder.Code, recorder.Body.String())
|
||||||
|
|
||||||
|
appID, found := strings.CutPrefix(recorder.Header().Get("Location"), "/apps/")
|
||||||
|
require.True(t, found)
|
||||||
|
|
||||||
|
createdApp, err := models.FindApp(t.Context(), db, appID)
|
||||||
|
require.NoError(t, err)
|
||||||
|
require.NotNil(t, createdApp)
|
||||||
|
|
||||||
|
return createdApp
|
||||||
|
}
|
||||||
@@ -41,6 +41,7 @@ type fakeImageAPI struct {
|
|||||||
nextID string // ID of the image the next build creates
|
nextID string // ID of the image the next build creates
|
||||||
removed []string // each tag or ID removed
|
removed []string // each tag or ID removed
|
||||||
forced bool // whether a removal was forced
|
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) {
|
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"):
|
case strings.HasSuffix(r.URL.Path, "/version"):
|
||||||
_, _ = w.Write([]byte(`{"Version":"27.3.1","ApiVersion":"1.47"}`))
|
_, _ = w.Write([]byte(`{"Version":"27.3.1","ApiVersion":"1.47"}`))
|
||||||
case strings.HasSuffix(r.URL.Path, "/containers/create"):
|
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"}`))
|
_, _ = w.Write([]byte(`{"Id":"gitcontainer"}`))
|
||||||
case strings.HasSuffix(r.URL.Path, "/logs"):
|
case strings.HasSuffix(r.URL.Path, "/logs"):
|
||||||
writeCloneOutput(w, api.shortSHA)
|
writeCloneOutput(w, api.shortSHA)
|
||||||
|
|||||||
@@ -113,6 +113,16 @@ func (svc *Service) BuildImage(
|
|||||||
return svc.buildImage(ctx, app, deployment)
|
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.
|
// BuildContainerOptionsExported exposes buildContainerOptions for testing.
|
||||||
func (svc *Service) BuildContainerOptionsExported(
|
func (svc *Service) BuildContainerOptionsExported(
|
||||||
ctx context.Context,
|
ctx context.Context,
|
||||||
|
|||||||
@@ -30,10 +30,12 @@
|
|||||||
name="name"
|
name="name"
|
||||||
value="{{.App.Name}}"
|
value="{{.App.Name}}"
|
||||||
required
|
required
|
||||||
pattern="[a-z0-9-]+"
|
minlength="2"
|
||||||
|
maxlength="63"
|
||||||
|
pattern="[a-z0-9]+((\.|-+)[a-z0-9]+)*"
|
||||||
class="input"
|
class="input"
|
||||||
>
|
>
|
||||||
<p class="text-sm text-gray-500 mt-1">Lowercase letters, numbers, and hyphens only</p>
|
<p class="text-sm text-gray-500 mt-1">Lowercase letters, numbers, hyphens, and dots, such as my-app or example.com</p>
|
||||||
</div>
|
</div>
|
||||||
|
|
||||||
<div class="form-group">
|
<div class="form-group">
|
||||||
|
|||||||
@@ -30,11 +30,13 @@
|
|||||||
name="name"
|
name="name"
|
||||||
value="{{.Name}}"
|
value="{{.Name}}"
|
||||||
required
|
required
|
||||||
pattern="[a-z0-9-]+"
|
minlength="2"
|
||||||
|
maxlength="63"
|
||||||
|
pattern="[a-z0-9]+((\.|-+)[a-z0-9]+)*"
|
||||||
class="input"
|
class="input"
|
||||||
placeholder="my-app"
|
placeholder="my-app"
|
||||||
>
|
>
|
||||||
<p class="text-sm text-gray-500 mt-1">Lowercase letters, numbers, and hyphens only</p>
|
<p class="text-sm text-gray-500 mt-1">Lowercase letters, numbers, hyphens, and dots, such as my-app or example.com</p>
|
||||||
</div>
|
</div>
|
||||||
|
|
||||||
<div class="form-group">
|
<div class="form-group">
|
||||||
|
|||||||
Reference in New Issue
Block a user