feat: add private Docker registry authentication for base images #167
Reference in New Issue
Block a user
Delete Branch "feature/private-registry-auth"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Closes #80
Summary
Adds per-app registry credentials that are passed to Docker during image builds, allowing apps to use base images from private registries (e.g. ghcr.io, private Harbor, ECR, etc.).
Changes
Database
007_add_registry_credentials.sql— createsregistry_credentialstable withapp_id,registry,username,passwordcolumns, unique constraint on(app_id, registry), cascade delete on app removalModel (
internal/models/registry_credential.go)RegistryCredentialstruct following the Active Record pattern used by all other modelsNewRegistryCredential()constructorSave()— insert or updateDelete()— remove credentialFindRegistryCredential()— find by IDFindRegistryCredentialsByAppID()— find all for an appApp.GetRegistryCredentials()— convenience method on the App modelDocker Client (
internal/docker/client.go)RegistryAuthstruct for passing registry credentialsBuildImageOptions.RegistryAuthsfieldbuildAuthConfigs()helper to convert[]RegistryAuthto Docker SDK'smap[string]registry.AuthConfigImageBuildwhen present, enabling the Docker daemon to authenticate when pulling private base images during buildsDeploy Service (
internal/service/deploy/deploy.go)buildRegistryAuths()method fetches app's registry credentials and converts to[]docker.RegistryAuthbuildImage— credentials are passed to Docker build transparentlyHandlers & Routes
HandleRegistryCredentialAdd()— POST/apps/{id}/registry-credentialsHandleRegistryCredentialEdit()— POST/apps/{id}/registry-credentials/{credID}/editHandleRegistryCredentialDelete()— POST/apps/{id}/registry-credentials/{credID}/deleteUI (
templates/app_detail.html)Tests
TestRegistryCredentialCreateAndFind— create + queryTestRegistryCredentialUpdate— update in placeTestRegistryCredentialDelete— deletionTestRegistryCredentialFindByIDNotFound— nil/nil for missingTestAppGetRegistryCredentials— convenience methodTestBuildAuthConfigsEmpty/TestBuildAuthConfigsSingle/TestBuildAuthConfigsMultiple— auth config builderTestRegistryAuthStruct— type instantiationREADME
Code Review: PR #167 — Private Docker Registry Authentication
Policy Compliance Check
go.mod/go.sumunchanged007_add_registry_credentials.sqlis correct for post-1.0.0Makefile,Dockerfile,.golangci.yml,.gitea/workflows/all untouchedRegistryCredential,RegistryAuth,buildAuthConfigs, CRUD, cascade — all testedhttp.HandlerFuncSessionAuth()+CSRF()group❌ Corrupted Doc Comment in
internal/service/deploy/deploy.goThe
buildRegistryAuthsfunction was inserted between thewriteLogsToFiledoc comment and thewriteLogsToFilefunction body. This results in:Lines 1238–1242:
Go treats all four comment lines as the godoc for
buildRegistryAuths(), so:buildRegistryAuthshas a misleading doc starting with "writeLogsToFile writes the deployment logs…"writeLogsToFile(line 1268) has no doc comment at all — its original comment was stolenFix: Move the
writeLogsToFilecomment lines down to sit directly abovefunc (svc *Service) writeLogsToFile(...), and keep only thebuildRegistryAuthscomment abovebuildRegistryAuths.Requirements Checklist (from issue #80)
registry_credentialstable with cascade deleteBuildImageOptions.RegistryAuths→AuthConfigsCode Quality Notes (non-blocking, for context)
cred.AppID != appID— prevents cross-app access ✅app_env_vars,app_labels) with no timestamps — consistent ✅Build Result
Verdict: FAIL
The corrupted doc comment in
deploy.gois a concrete correctness bug —buildRegistryAuthshas the wrong godoc, andwriteLogsToFilelost its documentation. This needs to be fixed before merge.Rework: Fix corrupted doc comment in
deploy.goIssue: The
buildRegistryAuthsfunction was inserted between thewriteLogsToFiledoc comment and thewriteLogsToFilefunction body, causing Go to treat thewriteLogsToFilecomment as godoc forbuildRegistryAuths.Fix: Moved the two
writeLogsToFilecomment lines (// writeLogsToFile writes the deployment logs...and// Structure: DataDir/logs/...) down to sit directly abovefunc (svc *Service) writeLogsToFile(...), so each function now has its own correct doc comment.Verification:
gofmt— cleandocker build .— all stages pass (fmt-check, lint, test, build) ✅Code Review: PR #167 — Private Docker Registry Authentication (Re-review)
Policy Compliance Check
go.mod/go.sumunchanged.registrypackage is part of existing Docker SDK dependency.007_add_registry_credentials.sqlfollows existing numbering (006 → 007).Makefile,Dockerfile,.golangci.yml,.gitea/workflows/all untouched.Label,Volume,EnvVar).http.HandlerFunc.SessionAuth()+CSRF()group.Previous Review Issue: Godoc Comments
The previous review FAILed because
buildRegistryAuthswas inserted betweenwriteLogsToFile's doc comment and function body.Verified FIXED. In
internal/service/deploy/deploy.go:buildRegistryAuths(line ~1238) has its own correct doc comment: "buildRegistryAuths fetches registry credentials for an app…"writeLogsToFile(line ~1270) has its own correct doc comment: "writeLogsToFile writes the deployment logs to a file on disk." with the structure lineRequirements Checklist (from issue #80)
RegistryAuthpassed toImageBuildviaAuthConfigsregistry_credentialstable withUNIQUE(app_id, registry)+ cascade deletebuildRegistryAuths()→BuildImageOptions.RegistryAuths→buildAuthConfigs()→AuthConfigsTest Coverage Check
RegistryCredentialstruct +NewRegistryCredentialTestRegistryCredentialCreateAndFindRegistryCredential.Save(insert + update)TestRegistryCredentialCreateAndFind,TestRegistryCredentialUpdateRegistryCredential.DeleteTestRegistryCredentialDeleteFindRegistryCredentialTestRegistryCredentialUpdate(used for verification),TestRegistryCredentialFindByIDNotFoundFindRegistryCredentialsByAppIDTestRegistryCredentialCreateAndFind,TestRegistryCredentialDeleteApp.GetRegistryCredentialsTestAppGetRegistryCredentialsRegistryAuthstructTestRegistryAuthStructbuildAuthConfigs(unexported)TestBuildAuthConfigsEmpty,TestBuildAuthConfigsSingle,TestBuildAuthConfigsMultipleTestCascadeDelete(updated to include registry credentials)All new exported types and functions have test coverage. ✅
Code Quality & Security Notes
cred.AppID != appID— prevents cross-app credential access. ✅app_labels,app_env_vars) withON DELETE CASCADEand index onapp_id. ✅nolintdirectives:gosecfor password fields andnilnilfor Active Record "not found" — both justified and consistent with existing code. ✅testpackagenolint inauth_test.go: Justified — tests unexportedbuildAuthConfigs. ✅buildImage:buildRegistryAuthsfailure is warn-logged and build continues with nil auths — correct graceful degradation. ✅Build Result
Verdict: PASS ✅
The previous review's godoc issue is properly fixed. All requirements from issue #80 are implemented. Policy compliance is clean. Tests cover all new exports. Build passes. No cheating detected.
nobody asked for this
Pull request closed