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
|
||||
|
||||
- 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
|
||||
that carries a credential no longer goes into the Docker build; the image
|
||||
still shows the commit it was built from (#269).
|
||||
|
||||
@@ -4,6 +4,7 @@ go 1.25
|
||||
|
||||
require (
|
||||
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/go-connections v0.6.0
|
||||
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/typeurl/v2 v2.2.0 // 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/felixge/httpsnoop v1.0.4 // indirect
|
||||
github.com/fsnotify/fsnotify v1.9.0 // indirect
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -1,7 +1,16 @@
|
||||
package handlers //nolint:testpackage // testing unexported validateAppName
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"strconv"
|
||||
"strings"
|
||||
"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) {
|
||||
@@ -18,6 +27,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 +49,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},
|
||||
@@ -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,121 @@
|
||||
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
|
||||
}
|
||||
@@ -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,
|
||||
|
||||
@@ -30,10 +30,12 @@
|
||||
name="name"
|
||||
value="{{.App.Name}}"
|
||||
required
|
||||
pattern="[a-z0-9-]+"
|
||||
minlength="2"
|
||||
maxlength="63"
|
||||
pattern="[a-z0-9]+((\.|-+)[a-z0-9]+)*"
|
||||
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 class="form-group">
|
||||
|
||||
@@ -30,11 +30,13 @@
|
||||
name="name"
|
||||
value="{{.Name}}"
|
||||
required
|
||||
pattern="[a-z0-9-]+"
|
||||
minlength="2"
|
||||
maxlength="63"
|
||||
pattern="[a-z0-9]+((\.|-+)[a-z0-9]+)*"
|
||||
class="input"
|
||||
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 class="form-group">
|
||||
|
||||
Reference in New Issue
Block a user