diff --git a/.github/coverage-floors.txt b/.github/coverage-floors.txt index 8a7fe670b..aee5a0935 100644 --- a/.github/coverage-floors.txt +++ b/.github/coverage-floors.txt @@ -55,10 +55,12 @@ internal/controlplane/sim 0 # measured 0.0 2026-10-05 internal/cost 38 # measured 40.8 2026-10-05 internal/credmask 72 # measured 72.1 2026-10-26 internal/database 21 # measured 23.1 2026-10-05 +internal/database/metadatavalue 79 # measured 81.8 2026-10-29 internal/database/vectorsearch 94 # measured 96.9 2026-10-10 internal/deploy 53 # measured 55.2 2026-10-05 internal/deploy/archguard 76 # measured 78.6 2026-10-09 internal/deploy/bluegreen 44 # measured 46.3 2026-10-05 +internal/deploy/imagetags 98 # measured 100.0 2026-10-29 internal/deploy/pintest 57 # measured 59.5 2026-10-13 (P7-HYG-49) internal/deploy/progress 86 # measured 88.8 2026-10-12 internal/deprecation 75 # measured 77.3 2026-10-05 @@ -73,6 +75,7 @@ internal/errs 93 # measured 95.8 2026-10-05 internal/eval 77 # measured 79.1 2026-10-05 internal/exporter 58 # measured 60.2 2026-10-12 (P7-ADOPT-17; snapshot, pg_dump and auth reads need a live Postgres, covered by scripts/e2e/export.sh) internal/featureflags 69 # measured 71.7 2026-10-05 +internal/frontcheck 98 # measured 100.0 2026-10-29 internal/generate 65 # measured 67.5 2026-10-05 internal/hasura 87 # measured 89.1 2026-10-05 internal/health 26 # measured 28.1 2026-10-05 @@ -110,6 +113,7 @@ internal/migration 14 # measured 16.8 2026-10-05 internal/migrationai 69 # measured 71.3 2026-10-05 internal/model 65 # measured 67.3 2026-10-05 internal/nginx 75 # measured 77.4 2026-10-05 +internal/nginx/plugincheck 98 # measured 100.0 2026-10-29 internal/nginx/routemodel 66 # measured 68.2 2026-10-08 internal/nginxtopo 98 # measured 100.0 2026-10-05 internal/oauth 82 # measured 84.4 2026-10-05 diff --git a/.github/docs/maintenance-safety.md b/.github/docs/maintenance-safety.md new file mode 100644 index 000000000..727d3e3fe --- /dev/null +++ b/.github/docs/maintenance-safety.md @@ -0,0 +1,24 @@ +# Maintenance safety + +Deploy rollback treats an absent `.nself/state/deploy.json` as a fresh install. +An existing record must identify its schema version or legacy strategy. Empty +objects and JSON `null` are errors, so they cannot select legacy rollback. +Before remote image pulls, an inspect failure stops the apply unless Docker +explicitly reports that the image is absent. Invalid image IDs also stop the apply. + +Operation locks invalidate their re-entry token before releasing the kernel +lock. A token from a different host cannot authorize re-entry. Unsupported +platform notices appear only when stderr is a terminal. + +Plugin nginx validation ignores template examples in comments and permits +native uppercase nginx variables. Known unresolved template names still refuse +the snippet. Init container classification uses the effective Compose label +after applying overrides in file order. + +Hasura metadata exports preserve strings with multiple trailing newlines through +the include layout. Hasura CLI projects may omit `metadata_directory` to use +`metadata`; directory aliases are compared by filesystem identity. + +The affected-package helper scans owned data paths for readers outside their +owning package and includes importers when embedded data is deleted. The vendor +check preserves spaces in local replacement paths when scanning for symlinks. diff --git a/cmd/commands/deploy_promote_proof.go b/cmd/commands/deploy_promote_proof.go index 80b0005a5..0f005e48c 100644 --- a/cmd/commands/deploy_promote_proof.go +++ b/cmd/commands/deploy_promote_proof.go @@ -7,7 +7,6 @@ package commands import ( "context" - "fmt" "sort" cpromote "github.com/nself-org/cli/internal/controlplane/promote" @@ -52,28 +51,6 @@ func provePromoteApplied(ctx context.Context, w *promoteWiring, st *cpromote.Sta return nil } -// matchPromoteRunning requires one running identity for each compared service. func matchPromoteRunning(rows []cpromote.DigestRow, images []cpromote.SourceImage) error { - if len(rows) == 0 { - return fmt.Errorf("no compared image identities") - } - running := make(map[string]string, len(images)) - for _, image := range images { - running[image.Service] = image.Digest - } - for _, row := range rows { - if row.Status == "unverifiable" { - delete(running, row.Service) - continue - } - digest, found := running[row.Service] - if !found || row.Target == "" || digest != row.Target || row.Source != row.Target { - return fmt.Errorf("service %s running digest %s differs from compared digest %s", row.Service, digest, row.Target) - } - delete(running, row.Service) - } - if len(running) > 0 { - return fmt.Errorf("running release has uncompared services") - } - return nil + return cpromote.MatchRunning(rows, images) } diff --git a/cmd/commands/deploy_promote_signal_test.go b/cmd/commands/deploy_promote_signal_test.go index f16a50b1d..9482655a2 100644 --- a/cmd/commands/deploy_promote_signal_test.go +++ b/cmd/commands/deploy_promote_signal_test.go @@ -31,6 +31,9 @@ func (r *promoteSignalRemote) Run(ctx context.Context, command string) ([]byte, <-ctx.Done() return nil, ctx.Err() } + if strings.Contains(command, "{{.Id}}") { + return []byte("sha256:" + strings.Repeat("a", 64) + " web:1\n"), nil + } raw, err := r.v15Remote.Run(ctx, command) if err == nil && (strings.Contains(command, "images --format json") || strings.Contains(command, " ps --all --format json") || strings.Contains(command, "image inspect") || strings.Contains(command, "config --format json --no-env-resolution") || strings.Contains(command, "__NSELF_ABSENT__")) { return r.proof.Run(ctx, command) diff --git a/internal/build/plugin_configs_test.go b/internal/build/plugin_configs_test.go index ee8586e77..02787f87a 100644 --- a/internal/build/plugin_configs_test.go +++ b/internal/build/plugin_configs_test.go @@ -191,6 +191,9 @@ func TestPluginSnippetSkip(t *testing.T) { hasSSL bool want string // substring of the reason; "" = injected }{ + {"comment placeholder", "# ${NSELF_DOMAIN}\nserver { listen 80; }\n", false, ""}, + {"native uppercase variable", "map $host $UPSTREAM { default localhost; }\nserver { location / { proxy_pass http://${UPSTREAM}; } }\n", false, ""}, + {"native route variable", "map $host $CUSTOM_ROUTE { default localhost; }\nserver { location / { proxy_pass http://${CUSTOM_ROUTE}; } }\n", false, ""}, {"valid server block", "server {\n listen 80;\n server_name a.example.com;\n location / { return 204; }\n}\n", false, ""}, {"nginx variable in braces", "server {\n listen 80;\n location / { return 301 https://${host}$request_uri; }\n}\n", false, ""}, {"upstream and map at top level", "upstream u { server a:1; }\nmap $http_upgrade $c { default upgrade; }\nserver { listen 80; }\n", false, ""}, diff --git a/internal/build/plugin_nginx.go b/internal/build/plugin_nginx.go index 2f4ccf194..13457949e 100644 --- a/internal/build/plugin_nginx.go +++ b/internal/build/plugin_nginx.go @@ -5,13 +5,11 @@ import ( "log/slog" "os" "path/filepath" - "regexp" "sort" "strings" "github.com/nself-org/cli/internal/config" - "github.com/nself-org/cli/internal/nginx/routemodel" - "github.com/nself-org/cli/internal/nginxtopo" + "github.com/nself-org/cli/internal/nginx/plugincheck" ) // claimsOf returns the set of "|" claims a rendered @@ -109,45 +107,8 @@ func InjectPluginNginxRoutes(workdir, pluginDir string, cfg *config.Config) (int return injectPluginNginxRoutesVia(newDiskSink(workdir), workdir, pluginDir, cfg, true) } -// placeholderRE matches a ${NAME} template variable buildTemplateVars does -// not provide. nginx's own variables are lower case, so ${host} is left alone. -var placeholderRE = regexp.MustCompile(`\$\{[A-Z][A-Z0-9_]*\}`) - -// serverOnly lists directives nginx accepts only inside server (or deeper): -// at the http{} level nginx/sites is included in, each one stops nginx with -// "directive is not allowed here". -var serverOnly = map[string]bool{"location": true, "if": true, "listen": true, "server_name": true, - "proxy_pass": true, "fastcgi_pass": true, "uwsgi_pass": true, "scgi_pass": true, "grpc_pass": true, - "return": true, "rewrite": true, "try_files": true, "set": true, "alias": true, "internal": true, "limit_except": true} - -// pluginSnippetSkip returns why a rendered plugin snippet must stay out of -// nginx/sites, or "" to inject it. One bad file in nginx/sites stops nginx, -// taking every route down, and installed plugin copies can be old, so the -// build checks what it can before nginx does (D-0478 review): -// - a ${NAME} the CLI does not render (nginx reads it as an unknown variable); -// - text that does not parse, or a top-level directive that is valid only -// inside a server block (a bare location block); -// - a load of the base domain's certificate pair (the ${SSL_DIR} path) while -// the build serves no TLS (hasSSL false: SSL_MODE none, or letsencrypt/ -// custom before the pair exists), which would also block the ACME -// challenge that issues it. func pluginSnippetSkip(rendered string, vars map[string]string, hasSSL bool) string { - if m := placeholderRE.FindString(rendered); m != "" { - return "it uses " + m + ", which nself does not provide" - } - tree, err := routemodel.Parse(rendered) - if err != nil { - return "it does not parse: " + err.Error() - } - for _, b := range tree { - if serverOnly[b.Name] { - return "it has a top-level " + b.Name + " directive, which nginx accepts only inside a server block" - } - } - if !hasSSL && strings.Contains(rendered, nginxtopo.NginxSSLContainerPath+"/"+vars["SSL_DIR"]+"/") { - return "it needs the base domain's TLS certificate, which this build does not serve (issue it with nself trust ssl setup, or set SSL_MODE=local)" - } - return "" + return plugincheck.Skip(rendered, vars, hasSSL) } // resolvePluginNginxDir returns the plugin directory the nginx snippets are diff --git a/internal/controlplane/promote/running.go b/internal/controlplane/promote/running.go new file mode 100644 index 000000000..f830f1b2f --- /dev/null +++ b/internal/controlplane/promote/running.go @@ -0,0 +1,29 @@ +package promote + +import "fmt" + +// MatchRunning requires one running identity for each compared service. +func MatchRunning(rows []DigestRow, images []SourceImage) error { + if len(rows) == 0 { + return fmt.Errorf("no compared image identities") + } + running := make(map[string]string, len(images)) + for _, image := range images { + running[image.Service] = image.Digest + } + for _, row := range rows { + if row.Status == "unverifiable" { + delete(running, row.Service) + continue + } + digest, found := running[row.Service] + if !found || row.Target == "" || digest != row.Target || row.Source != row.Target { + return fmt.Errorf("service %s running digest %s differs from compared digest %s", row.Service, digest, row.Target) + } + delete(running, row.Service) + } + if len(running) > 0 { + return fmt.Errorf("running release has uncompared services") + } + return nil +} diff --git a/internal/controlplane/promote/running_test.go b/internal/controlplane/promote/running_test.go new file mode 100644 index 000000000..d215874b1 --- /dev/null +++ b/internal/controlplane/promote/running_test.go @@ -0,0 +1,27 @@ +package promote + +import "testing" + +func TestMatchRunningIdentities(t *testing.T) { + row := DigestRow{Service: "web", Source: "sha256:a", Target: "sha256:a"} + image := SourceImage{Service: "web", Digest: "sha256:a"} + for _, tc := range []struct { + name string + rows []DigestRow + images []SourceImage + wantErr bool + }{ + {"equal", []DigestRow{row}, []SourceImage{image}, false}, + {"no comparison", nil, []SourceImage{image}, true}, + {"missing running service", []DigestRow{row}, nil, true}, + {"changed running digest", []DigestRow{row}, []SourceImage{{Service: "web", Digest: "sha256:b"}}, true}, + {"uncompared service", []DigestRow{row}, []SourceImage{image, {Service: "other", Digest: "sha256:a"}}, true}, + {"unverifiable service", []DigestRow{{Service: "web", Status: "unverifiable"}}, []SourceImage{image}, false}, + } { + t.Run(tc.name, func(t *testing.T) { + if err := MatchRunning(tc.rows, tc.images); (err != nil) != tc.wantErr { + t.Fatalf("MatchRunning = %v, wantErr %t", err, tc.wantErr) + } + }) + } +} diff --git a/internal/controlplane/sim/promote_integration_test.go b/internal/controlplane/sim/promote_integration_test.go index b01c82562..92fddf0d1 100644 --- a/internal/controlplane/sim/promote_integration_test.go +++ b/internal/controlplane/sim/promote_integration_test.go @@ -124,6 +124,8 @@ func TestSimPromote(t *testing.T) { else printf '[]'; fi ;; *" ps --all --format json"*) printf '{"Name":"proj-web-1","Service":"web","State":"running"}\n' ;; + "image inspect --format {{.Id}} "*) + printf 'sha256:bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb\n' ;; *"image inspect --format"*) for last; do :; done digest=bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb diff --git a/internal/controlplane/sim/remote_apply_integration_test.go b/internal/controlplane/sim/remote_apply_integration_test.go index 3b0b28dbf..ec589d3e9 100644 --- a/internal/controlplane/sim/remote_apply_integration_test.go +++ b/internal/controlplane/sim/remote_apply_integration_test.go @@ -45,6 +45,8 @@ case "$*" in fi ;; *"up -d --no-deps"*) for last; do :; done; n=$(cat "/tmp/sim-up-$last" 2>/dev/null || echo 0); echo $((n+1)) > "/tmp/sim-up-$last" ;; "inspect -f"*) shift 3; for i in "$@"; do n=$(cat "/tmp/sim-up-${i#id-}" 2>/dev/null || echo 0); printf '2026-10-10T00:00:%02dZ\n' "$n"; done ;; + "image inspect --format {{.Id}} "*) + printf 'sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n' ;; "network inspect"*) exit 1 ;; esac exit 0 diff --git a/internal/database/hasura_apply_layout.go b/internal/database/hasura_apply_layout.go index 75c981471..a8ce75c40 100644 --- a/internal/database/hasura_apply_layout.go +++ b/internal/database/hasura_apply_layout.go @@ -4,7 +4,7 @@ package database // D-0564). nself writes its own layout (a config.yaml index of !include lines // at the metadata root, one .yaml per top-level key; hasura_export_layout.go), // which only the in-process include resolver reads. The Hasura CLI expects its -// own project (hasura/config.yaml naming metadata_directory, then version.yaml +// own project (hasura/config.yaml, optional metadata_directory, then version.yaml // and databases/), so it is used only for a real Hasura CLI project. // Inputs: the hasura/ project directory and its metadata/ directory. // Outputs: isHasuraCLIProject. @@ -27,7 +27,7 @@ type hasuraCLIProjectConfig struct { // isHasuraCLIProject reports whether hasuraDir is a Hasura CLI project whose // metadata the hasura binary should apply: hasuraDir/config.yaml names a -// metadata_directory that resolves to metadataDir, and metadataDir carries no +// metadata_directory (default metadata) identifying metadataDir, which carries no // nself config.yaml index (that index is nself's layout, which the Hasura CLI // cannot read). func isHasuraCLIProject(hasuraDir, metadataDir string) bool { @@ -44,10 +44,15 @@ func isHasuraCLIProject(hasuraDir, metadataDir string) bool { } dir := strings.TrimSpace(c.MetadataDirectory) if dir == "" { - return false + dir = "metadata" } if !filepath.IsAbs(dir) { dir = filepath.Join(hasuraDir, dir) } - return filepath.Clean(dir) == filepath.Clean(metadataDir) + configured, err := os.Stat(dir) + if err != nil || !configured.IsDir() { + return false + } + metadata, err := os.Stat(metadataDir) + return err == nil && metadata.IsDir() && os.SameFile(configured, metadata) } diff --git a/internal/database/hasura_apply_layout_test.go b/internal/database/hasura_apply_layout_test.go index 066b8b67a..6d7846159 100644 --- a/internal/database/hasura_apply_layout_test.go +++ b/internal/database/hasura_apply_layout_test.go @@ -128,7 +128,7 @@ func TestIsHasuraCLIProject(t *testing.T) { {"cli project", map[string]string{"hasura/config.yaml": "metadata_directory: metadata\n"}, true}, {"cli project ./metadata", map[string]string{"hasura/config.yaml": "metadata_directory: ./metadata\n"}, true}, {"no project config", map[string]string{"hasura/metadata/tables.yaml": "[]\n"}, false}, - {"no metadata_directory", map[string]string{"hasura/config.yaml": "version: 3\n"}, false}, + {"default metadata_directory", map[string]string{"hasura/config.yaml": "version: 3\n"}, true}, {"other metadata_directory", map[string]string{"hasura/config.yaml": "metadata_directory: md\n"}, false}, {"unparsable config", map[string]string{"hasura/config.yaml": "metadata_directory: [\n"}, false}, {"nself index wins", map[string]string{ @@ -150,3 +150,19 @@ func TestIsHasuraCLIProject(t *testing.T) { } } } + +func TestIsHasuraCLIProjectSymlinkDirectory(t *testing.T) { + project := t.TempDir() + metadata := filepath.Join(project, "metadata") + if err := os.Mkdir(metadata, 0o755); err != nil { + t.Fatal(err) + } + alias := filepath.Join(project, "linked-metadata") + if err := os.Symlink(metadata, alias); err != nil { + t.Skipf("symlink unavailable: %v", err) + } + writeFixtureFile(t, project, "config.yaml", "metadata_directory: linked-metadata\n") + if !isHasuraCLIProject(project, metadata) { + t.Fatal("metadata directory aliases must identify the same CLI project") + } +} diff --git a/internal/database/hasura_archive_layout_test.go b/internal/database/hasura_archive_layout_test.go index 24f88605f..fe89da922 100644 --- a/internal/database/hasura_archive_layout_test.go +++ b/internal/database/hasura_archive_layout_test.go @@ -153,6 +153,24 @@ func TestHasuraLayoutExportEqualsArchive(t *testing.T) { } } +// Keep-chomp YAML strings must preserve every trailing newline through includes. +func TestHasuraLayoutPreservesTrailingNewlines(t *testing.T) { + data := []byte(`{"actions":[{"definition":{"handler":"https://example.test\n\n\n"}}]}`) + dir := t.TempDir() + if err := writeMetadataTree(dir, data); err != nil { + t.Fatal(err) + } + resolved, err := resolveIncludes(dir, filepath.Join(dir, metadataIndexFile)) + if err != nil { + t.Fatal(err) + } + var doc interface{} + if err := yaml.Unmarshal(resolved, &doc); err != nil { + t.Fatal(err) + } + sameJSON(t, convertYAMLToJSON(doc), data) +} + // An older monolithic archive (one config.yaml, everything inline) still resolves. func TestHasuraLayoutLegacyMonolithicStillResolves(t *testing.T) { data := layoutInput(t) diff --git a/internal/database/hasura_export_layout.go b/internal/database/hasura_export_layout.go index 6e65877c5..bc7b83377 100644 --- a/internal/database/hasura_export_layout.go +++ b/internal/database/hasura_export_layout.go @@ -12,7 +12,8 @@ package database // style) instead. The same JSON form is used when the body contains the text // "!include", with each "!include" written as "\u0021include", so a string value // is never mistaken for an include directive; YAML decodes it back to "!". -// Multi-line bodies without "!include" stay block YAML. +// Keep-chomp scalars also use JSON because the resolver trims trailing newlines. +// Other multi-line bodies without "!include" stay block YAML. // Pure and deterministic (sorted keys, no clock, no disk) except // writeMetadataTree, which only writes the files it is given. A key that is not // a safe file name is rejected. The layout is what applyViaIncludeResolver reads. @@ -25,7 +26,7 @@ import ( "sort" "strings" - "gopkg.in/yaml.v3" + "github.com/nself-org/cli/internal/database/metadatavalue" ) const metadataIndexFile = "config.yaml" @@ -55,7 +56,7 @@ func metadataLayout(data []byte) ([]metadataFile, error) { files := make([]metadataFile, 0, len(keys)+1) var index strings.Builder for _, k := range keys { - body, err := marshalMetadataValue(raw[k]) + body, err := metadatavalue.Marshal(raw[k]) if err != nil { return nil, fmt.Errorf("marshal %s to YAML: %w", k, err) } @@ -67,26 +68,6 @@ func metadataLayout(data []byte) ([]metadataFile, error) { return files, nil } -// marshalMetadataValue encodes one top-level value for its .yaml file: -// block YAML, except single-line bodies and bodies containing "!include", which -// become one line of JSON with "!include" escaped (see the header for why). -func marshalMetadataValue(v interface{}) ([]byte, error) { - body, err := yaml.Marshal(v) - if err != nil { - return nil, err - } - oneLine := strings.Count(strings.TrimRight(string(body), "\n"), "\n") == 0 - if !oneLine && !strings.Contains(string(body), "!include") { - return body, nil - } - js, err := json.Marshal(v) - if err != nil { - return nil, err - } - esc := strings.ReplaceAll(string(js), "!include", `\u0021include`) - return []byte(esc + "\n"), nil -} - // checkMetadataKey rejects a top-level key that would escape the directory, // collide with the index file, or break an `!include` line. func checkMetadataKey(k string) error { diff --git a/internal/database/metadatavalue/marshal.go b/internal/database/metadatavalue/marshal.go new file mode 100644 index 000000000..29a317599 --- /dev/null +++ b/internal/database/metadatavalue/marshal.go @@ -0,0 +1,34 @@ +// Package metadatavalue encodes values for the textual Hasura include resolver. +package metadatavalue + +import ( + "encoding/json" + "regexp" + "strings" + + "gopkg.in/yaml.v3" +) + +var metadataKeepChomp = regexp.MustCompile(`(?m)[|>](?:[1-9]?\+|\+[1-9])\n`) + +// Marshal encodes one top-level value for its .yaml file: +// block YAML, except single-line bodies, keep-chomp scalars, and "!include", which +// become one line of JSON with "!include" escaped. +func Marshal(v interface{}) ([]byte, error) { + body, err := yaml.Marshal(v) + if err != nil { + return nil, err + } + oneLine := strings.Count(strings.TrimRight(string(body), "\n"), "\n") == 0 + // The textual include resolver trims terminal newlines, so keep-chomp + // scalars must use JSON escapes to preserve their exact string values. + if !oneLine && !strings.Contains(string(body), "!include") && !metadataKeepChomp.Match(body) { + return body, nil + } + js, err := json.Marshal(v) + if err != nil { + return nil, err + } + esc := strings.ReplaceAll(string(js), "!include", `\u0021include`) + return []byte(esc + "\n"), nil +} diff --git a/internal/database/metadatavalue/marshal_test.go b/internal/database/metadatavalue/marshal_test.go new file mode 100644 index 000000000..5cf0071e9 --- /dev/null +++ b/internal/database/metadatavalue/marshal_test.go @@ -0,0 +1,31 @@ +package metadatavalue + +import ( + "gopkg.in/yaml.v3" + "reflect" + "strings" + "testing" +) + +func TestMarshalPreservesIncludeResolverValues(t *testing.T) { + for _, value := range []any{ + "one line", "keep terminal newline\n", "two\nnewlines\n\n", + "literal !include file.yaml", map[string]any{"a": true}, + map[string]any{"a": "first", "b": "second"}, []any{"first", "second"}, + } { + body, err := Marshal(value) + if err != nil { + t.Fatal(err) + } + if strings.Contains(string(body), "!include") { + t.Fatalf("resolver would interpret literal as directive: %s", body) + } + var got any + if err := yaml.Unmarshal(body, &got); err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(got, value) { + t.Fatalf("roundtrip %q = %#v, want %#v", body, got, value) + } + } +} diff --git a/internal/deploy/apply_remote_images.go b/internal/deploy/apply_remote_images.go index e6b58d16c..f4dca6a09 100644 --- a/internal/deploy/apply_remote_images.go +++ b/internal/deploy/apply_remote_images.go @@ -15,18 +15,17 @@ package deploy import ( "context" - "fmt" - "regexp" "sort" "strings" - "time" + + "github.com/nself-org/cli/internal/deploy/imagetags" ) // imageTag is one image ref and the image id it pointed at before the pull. -type imageTag struct{ ID, Ref string } +type imageTag = imagetags.Tag // imageIDRe is the only id shape a saved pair may carry. -var imageIDRe = regexp.MustCompile(`^sha256:[0-9a-f]{64}$`) +var imageIDRe = imagetags.IDRe // imageRefs are the model's distinct image refs, sorted, without digest pins. func imageRefs(model ComposeModel) []string { @@ -43,46 +42,9 @@ func imageRefs(model ComposeModel) []string { return refs } -// saveImageTags records the image id each ref points at on the host now. A ref -// the host does not have is absent from the result. func saveImageTags(ctx context.Context, r Remote, refs []string) ([]imageTag, error) { - if len(refs) == 0 { - return nil, nil - } - script := fmt.Sprintf(`for img in %s; do id=$(docker image inspect --format '{{.Id}}' "$img" 2>/dev/null) && printf '%%s %%s\n' "$id" "$img"; done; exit 0`, strings.Join(quoteAll(refs), " ")) - out, err := r.Run(ctx, shScript(script)) - if err != nil { - return nil, fmt.Errorf("reading the image ids on %s before the pull: %w", r.Label(), err) - } - want := map[string]bool{} - for _, ref := range refs { - want[ref] = true - } - var saved []imageTag - for _, line := range strings.Split(string(out), "\n") { - f := strings.Fields(line) - if len(f) != 2 || !imageIDRe.MatchString(f[0]) || !want[f[1]] { - continue - } - saved = append(saved, imageTag{ID: f[0], Ref: f[1]}) - } - return saved, nil + return imagetags.Save(ctx, r, refs) } - -// restoreImageTags points every saved ref back at its old image id. It runs on -// a context that survives the caller's cancellation, bounded by a minute. func restoreImageTags(ctx context.Context, r Remote, saved []imageTag) error { - if len(saved) == 0 { - return nil - } - ctx, cancel := context.WithTimeout(context.WithoutCancel(ctx), time.Minute) - defer cancel() - parts := []string{"rc=0"} - for _, t := range saved { - parts = append(parts, fmt.Sprintf("docker tag %s %s || rc=1", q(t.ID), q(t.Ref))) - } - if _, err := r.Run(ctx, shScript(strings.Join(append(parts, "exit $rc"), "; "))); err != nil { - return fmt.Errorf("putting the image tags back on %s: %w", r.Label(), err) - } - return nil + return imagetags.Restore(ctx, r, saved) } diff --git a/internal/deploy/imagetags/tags.go b/internal/deploy/imagetags/tags.go new file mode 100644 index 000000000..9f90619ad --- /dev/null +++ b/internal/deploy/imagetags/tags.go @@ -0,0 +1,92 @@ +// Package imagetags snapshots and restores mutable Docker image references. +package imagetags + +import ( + "context" + "fmt" + "regexp" + "strings" + "time" + + "github.com/nself-org/cli/sdk/go/v2/remote" +) + +// Runner executes a shell command on one named host. +type Runner interface { + Run(context.Context, string) ([]byte, error) + Label() string +} + +// Tag records the image ID a mutable reference pointed to. +type Tag struct{ ID, Ref string } + +// IDRe accepts only full Docker content IDs. +var IDRe = regexp.MustCompile(`^sha256:[0-9a-f]{64}$`) + +func quoteAll(refs []string) []string { + out := make([]string, len(refs)) + for i, ref := range refs { + out[i] = remote.ShellQuote(ref) + } + return out +} + +// Save records the image id each ref points at on the host now. A ref +// the host does not have is absent from the result. +func Save(ctx context.Context, r Runner, refs []string) ([]Tag, error) { + if len(refs) == 0 { + return nil, nil + } + // Docker normalizes missing references (for example web -> web:latest). + // Accept only its single-line missing-image diagnostic, with a nonempty name. + script := fmt.Sprintf(`for img in %s; do +if id=$(docker image inspect --format '{{.Id}}' "$img" 2>&1); then + printf '%%s %%s\n' "$id" "$img" +else + case "$id" in + *' +'*) exit 1 ;; + "Error response from daemon: No such image: "?*|"Error: No such image: "?*) ;; + *) printf 'cannot inspect image %%s\n' "$img" >&2; exit 1 ;; + esac +fi +done`, strings.Join(quoteAll(refs), " ")) + out, err := r.Run(ctx, "sh -c "+remote.ShellQuote(script)) + if err != nil { + return nil, fmt.Errorf("reading the image ids on %s before the pull: %w", r.Label(), err) + } + want := map[string]bool{} + for _, ref := range refs { + want[ref] = true + } + var saved []Tag + for _, line := range strings.Split(string(out), "\n") { + if line == "" { + continue + } + f := strings.Fields(line) + if len(f) != 2 || !IDRe.MatchString(f[0]) || !want[f[1]] { + return nil, fmt.Errorf("reading the image ids on %s before the pull: invalid inspect response", r.Label()) + } + saved = append(saved, Tag{ID: f[0], Ref: f[1]}) + } + return saved, nil +} + +// Restore points every saved ref back at its old image id. It runs on +// a context that survives the caller's cancellation, bounded by a minute. +func Restore(ctx context.Context, r Runner, saved []Tag) error { + if len(saved) == 0 { + return nil + } + ctx, cancel := context.WithTimeout(context.WithoutCancel(ctx), time.Minute) + defer cancel() + parts := []string{"rc=0"} + for _, t := range saved { + parts = append(parts, fmt.Sprintf("docker tag %s %s || rc=1", remote.ShellQuote(t.ID), remote.ShellQuote(t.Ref))) + } + if _, err := r.Run(ctx, "sh -c "+remote.ShellQuote(strings.Join(append(parts, "exit $rc"), "; "))); err != nil { + return fmt.Errorf("putting the image tags back on %s: %w", r.Label(), err) + } + return nil +} diff --git a/internal/deploy/imagetags/tags_test.go b/internal/deploy/imagetags/tags_test.go new file mode 100644 index 000000000..c72aac5cc --- /dev/null +++ b/internal/deploy/imagetags/tags_test.go @@ -0,0 +1,83 @@ +package imagetags + +import ( + "context" + "os/exec" + "strings" + "testing" +) + +// imageInspectRemote executes only a shell function double, never Docker. +type imageInspectRemote struct { + answer string +} + +func (r imageInspectRemote) Run(ctx context.Context, command string) ([]byte, error) { + script := "docker() { " + r.answer + "; }; " + strings.ReplaceAll(strings.TrimSuffix(strings.TrimPrefix(command, "sh -c '"), "'"), `'\''`, `'`) + return exec.CommandContext(ctx, "sh", "-c", script).Output() +} + +func TestSaveImageTagsInspectOutcomes(t *testing.T) { + for _, tc := range []struct { + name, answer string + wantErr bool + count int + }{ + {"present", "printf '%s\\n' " + q(fakeImageID("a")), false, 1}, + {"absent", "printf '%s\\n' 'Error response from daemon: No such image: web:1' >&2; return 1", false, 0}, + {"daemon unavailable", "printf '%s\\n' 'Cannot connect to the Docker daemon' >&2; return 1", true, 0}, + {"permission denied", "printf '%s\\n' 'permission denied' >&2; return 1", true, 0}, + {"empty success", "return 0", true, 0}, + {"malformed success", "printf '%s\\n' invalid", true, 0}, + {"successful warning", "printf '%s\\n' warning >&2; printf '%s\\n' " + q(fakeImageID("a")), true, 0}, + {"empty missing suffix", "printf '%s\\n' 'Error: No such image: ' >&2; return 1", true, 0}, + {"missing plus diagnostic", "printf '%s\\n' 'Error: No such image: web:1' 'permission denied' >&2; return 1", true, 0}, + } { + t.Run(tc.name, func(t *testing.T) { + r := imageInspectRemote{tc.answer} + saved, err := Save(context.Background(), r, []string{"web:1"}) + if (err != nil) != tc.wantErr || len(saved) != tc.count { + t.Fatalf("saved=%v err=%v", saved, err) + } + }) + } +} + +func TestSaveImageTagsNormalizedMissingReference(t *testing.T) { + for _, tc := range []struct{ ref, missing string }{ + {"web", "web:latest"}, + {"docker.io/library/web:1", "web:1"}, + } { + t.Run(tc.ref, func(t *testing.T) { + r := imageInspectRemote{"printf '%s\\n' " + q("Error response from daemon: No such image: "+tc.missing) + " >&2; return 1"} + saved, err := Save(context.Background(), r, []string{tc.ref}) + if err != nil || len(saved) != 0 { + t.Fatalf("missing normalized reference: saved=%v err=%v", saved, err) + } + }) + } +} + +func (imageInspectRemote) Label() string { return "fixture" } +func fakeImageID(digit string) string { return "sha256:" + strings.Repeat(digit, 64) } +func q(s string) string { return "'" + strings.ReplaceAll(s, "'", `'\''`) + "'" } + +func TestRestoreSurvivesCancellationAndQuotesReferences(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + cancel() + ref := "registry.example/web'quoted:1" + saved := []Tag{{ID: fakeImageID("a"), Ref: ref}} + r := imageInspectRemote{`[ "$1" = tag ] && [ "$2" = ` + q(saved[0].ID) + ` ] && [ "$3" = ` + q(ref) + ` ]`} + if err := Restore(ctx, r, saved); err != nil { + t.Fatal(err) + } + if err := Restore(ctx, imageInspectRemote{"return 1"}, saved); err == nil { + t.Fatal("failed Docker tag must fail restoration") + } + if err := Restore(ctx, r, nil); err != nil { + t.Fatal(err) + } + if saved, err := Save(ctx, r, nil); err != nil || len(saved) != 0 { + t.Fatalf("empty refs: %v %v", saved, err) + } +} diff --git a/internal/deploy/remote_fixes_test.go b/internal/deploy/remote_fixes_test.go index 41ad84e7e..3ea93f5d7 100644 --- a/internal/deploy/remote_fixes_test.go +++ b/internal/deploy/remote_fixes_test.go @@ -227,6 +227,8 @@ func TestNginxReloadOnlyWhenRunning(t *testing.T) { inner := stackHandler(rp2, "") f2.handler = func(s string) (string, bool) { switch { + case strings.Contains(s, "docker image inspect"): + return fakeImageID("a") + " p:1\n" + fakeImageID("b") + " w:1\n", true case strings.Contains(s, "config --format json"): return `{"name":"proj","services":{"db":{"image":"p:1","healthcheck":{"test":["CMD","x"]}},"web":{"image":"w:1","restart":"always"}}}`, true case strings.Contains(s, "config --services"): diff --git a/internal/deploy/state.go b/internal/deploy/state.go index 7c630f67b..5d3338698 100644 --- a/internal/deploy/state.go +++ b/internal/deploy/state.go @@ -115,6 +115,11 @@ func LoadDeployState(projectRoot string) (StrategyState, error) { if err := json.Unmarshal(data, &s); err != nil { return StrategyState{}, fmt.Errorf("parsing deploy state: %w", err) } + // Existing legacy records have a strategy, while v1.5 records carry a + // schema version. A structurally empty record must not select legacy rollback. + if s.SchemaVersion == 0 && s.Strategy == "" { + return StrategyState{}, fmt.Errorf("parsing deploy state: record has no schema version or strategy") + } return s, nil } diff --git a/internal/deploy/state_empty_test.go b/internal/deploy/state_empty_test.go new file mode 100644 index 000000000..71e6c3cf0 --- /dev/null +++ b/internal/deploy/state_empty_test.go @@ -0,0 +1,25 @@ +package deploy + +import ( + "os" + "path/filepath" + "testing" +) + +func TestLoadDeployStateRejectsEmptyRecords(t *testing.T) { + for _, body := range []string{"null", "{}", `{"unknown":true}`} { + t.Run(body, func(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, DeployStateFile) + if err := os.MkdirAll(filepath.Dir(path), 0700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, []byte(body), 0600); err != nil { + t.Fatal(err) + } + if _, err := LoadDeployState(dir); err == nil { + t.Fatal("empty state must not permit legacy rollback fallback") + } + }) + } +} diff --git a/internal/frontcheck/conf_test.go b/internal/frontcheck/conf_test.go new file mode 100644 index 000000000..5eacf89c7 --- /dev/null +++ b/internal/frontcheck/conf_test.go @@ -0,0 +1,43 @@ +package frontcheck + +import ( + "reflect" + "strings" + "testing" +) + +func TestBuildMarkerMustBeWithinHeader(t *testing.T) { + const marker = "# Generated by nself" + for _, tc := range []struct { + conf string + want bool + }{ + {marker + " build\nserver {}", true}, + {strings.Repeat("# header\n", 4) + marker, true}, + {strings.Repeat("# header\n", 5) + marker, false}, + {" " + marker, false}, + {"server {}", false}, + } { + if got := HasBuildMarker(tc.conf, marker); got != tc.want { + t.Errorf("marker in %q = %t, want %t", tc.conf, got, tc.want) + } + } +} + +func TestServerNamesEstablishOwnership(t *testing.T) { + conf := "# server_name foreign.example;\nserver { server_name b.example a.example; }\nserver_name a.example; # server_name ignored.example;\n" + got := ServerNameSet(conf) + want := []string{"a.example", "b.example"} + if !reflect.DeepEqual(got, want) { + t.Fatalf("names = %v, want %v", got, want) + } + if !SameNames(got, want) { + t.Fatal("identical sorted names must establish ownership") + } + if SameNames(got, []string{"a.example"}) || SameNames(nil, nil) || SameNames(got, nil) { + t.Fatal("different or empty names must not establish ownership") + } + if names := ServerNameSet("# server_name comment.example;"); len(names) != 0 { + t.Fatalf("comment establishes ownership: %v", names) + } +} diff --git a/internal/nginx/plugincheck/snippet.go b/internal/nginx/plugincheck/snippet.go new file mode 100644 index 000000000..51da09d0a --- /dev/null +++ b/internal/nginx/plugincheck/snippet.go @@ -0,0 +1,57 @@ +// Package plugincheck validates rendered plugin nginx snippets before installation. +package plugincheck + +import ( + "regexp" + "strings" + + "github.com/nself-org/cli/internal/nginx/routemodel" + "github.com/nself-org/cli/internal/nginxtopo" +) + +// placeholderRE finds template-shaped variables; the parsed directive and +// known template vocabulary distinguish them from comments and nginx variables. +var placeholderRE = regexp.MustCompile(`\$\{[A-Z][A-Z0-9_]*\}`) + +// serverOnly lists directives nginx accepts only inside server (or deeper): +// at the http{} level nginx/sites is included in, each one stops nginx with +// "directive is not allowed here". +var serverOnly = map[string]bool{"location": true, "if": true, "listen": true, "server_name": true, + "proxy_pass": true, "fastcgi_pass": true, "uwsgi_pass": true, "scgi_pass": true, "grpc_pass": true, + "return": true, "rewrite": true, "try_files": true, "set": true, "alias": true, "internal": true, "limit_except": true} + +// Skip returns why a rendered plugin snippet must stay out of +// nginx/sites, or "" to inject it. One bad file in nginx/sites stops nginx, +// taking every route down, and installed plugin copies can be old, so the +// build checks what it can before nginx does (D-0478 review): +// - a known template name that remains unrendered in directive arguments; +// - text that does not parse, or a top-level directive that is valid only +// inside a server block (a bare location block); +// - a load of the base domain's certificate pair (the ${SSL_DIR} path) while +// the build serves no TLS (hasSSL false: SSL_MODE none, or letsencrypt/ +// custom before the pair exists), which would also block the ACME +// challenge that issues it. +func Skip(rendered string, vars map[string]string, hasSSL bool) string { + tree, err := routemodel.Parse(rendered) + if err != nil { + return "it does not parse: " + err.Error() + } + for _, b := range tree { + for _, m := range placeholderRE.FindAllString(b.Render(), -1) { + name := strings.TrimSuffix(strings.TrimPrefix(m, "${"), "}") + _, known := vars[name] + if known || name == "DOMAIN" || name == "NSELF_DOMAIN" || name == "GRAFANA_ROUTE" { + return "it uses " + m + ", which nself does not provide" + } + } + } + for _, b := range tree { + if serverOnly[b.Name] { + return "it has a top-level " + b.Name + " directive, which nginx accepts only inside a server block" + } + } + if !hasSSL && strings.Contains(rendered, nginxtopo.NginxSSLContainerPath+"/"+vars["SSL_DIR"]+"/") { + return "it needs the base domain's TLS certificate, which this build does not serve (issue it with nself trust ssl setup, or set SSL_MODE=local)" + } + return "" +} diff --git a/internal/nginx/plugincheck/snippet_test.go b/internal/nginx/plugincheck/snippet_test.go new file mode 100644 index 000000000..76c19f241 --- /dev/null +++ b/internal/nginx/plugincheck/snippet_test.go @@ -0,0 +1,37 @@ +package plugincheck + +import ( + "strings" + "testing" +) + +func TestPluginSnippetSkip(t *testing.T) { + vars := map[string]string{"SSL_DIR": "certificates/example-com", "BASE_DOMAIN": "example.com"} + cert := " ssl_certificate /etc/nginx/ssl/certificates/example-com/fullchain.pem;\n" + cases := []struct { + name, text string + hasSSL bool + want string // substring of the reason; "" = injected + }{ + {"comment placeholder", "# ${NSELF_DOMAIN}\nserver { listen 80; }\n", false, ""}, + {"native uppercase variable", "map $host $UPSTREAM { default localhost; }\nserver { location / { proxy_pass http://${UPSTREAM}; } }\n", false, ""}, + {"native route variable", "map $host $CUSTOM_ROUTE { default localhost; }\nserver { location / { proxy_pass http://${CUSTOM_ROUTE}; } }\n", false, ""}, + {"valid server block", "server {\n listen 80;\n server_name a.example.com;\n location / { return 204; }\n}\n", false, ""}, + {"nginx variable in braces", "server {\n listen 80;\n location / { return 301 https://${host}$request_uri; }\n}\n", false, ""}, + {"upstream and map at top level", "upstream u { server a:1; }\nmap $http_upgrade $c { default upgrade; }\nserver { listen 80; }\n", false, ""}, + {"unrendered placeholder", "server {\n listen 443 ssl;\n ssl_certificate /etc/nginx/ssl/${NSELF_DOMAIN}/fullchain.pem;\n}\n", true, "${NSELF_DOMAIN}"}, + {"unrendered route var", "server {\n server_name ${GRAFANA_ROUTE}.example.com;\n}\n", true, "${GRAFANA_ROUTE}"}, + {"bare location block", "location /plugins/documents/ {\n proxy_pass http://documents:3000;\n}\n", true, "top-level location"}, + {"does not parse", "server { listen 80;", true, "does not parse"}, + {"top-level set", "set $upstream http://a:1;\nserver { listen 80; }\n", true, "top-level set"}, + {"top-level cert pair is legal at http level", "ssl_certificate /etc/nginx/ssl/other/fullchain.pem;\nssl_certificate_key /etc/nginx/ssl/other/privkey.pem;\nserver { listen 80; }\n", true, ""}, + {"base cert, TLS served", "server {\n listen 443 ssl;\n" + cert + "}\n", true, ""}, + {"base cert, no TLS served", "server {\n listen 443 ssl;\n" + cert + "}\n", false, "TLS certificate"}, + } + for _, c := range cases { + got := Skip(c.text, vars, c.hasSSL) + if (c.want == "") != (got == "") || !strings.Contains(got, c.want) { + t.Errorf("%s: pluginSnippetSkip = %q, want reason containing %q", c.name, got, c.want) + } + } +} diff --git a/internal/oplock/debt_rollup_test.go b/internal/oplock/debt_rollup_test.go new file mode 100644 index 000000000..40f71a650 --- /dev/null +++ b/internal/oplock/debt_rollup_test.go @@ -0,0 +1,44 @@ +//go:build !windows + +package oplock + +import ( + "context" + "encoding/json" + "os" + "testing" +) + +func TestReleaseInvalidatesHolderToken(t *testing.T) { + dir := t.TempDir() + l, err := Acquire(context.Background(), dir, Opts{}) + if err != nil { + t.Fatal(err) + } + l.Release() + raw, err := os.ReadFile(LockPath(dir)) + if err != nil { + t.Fatal(err) + } + var h Holder + if err := json.Unmarshal(raw, &h); err != nil { + t.Fatal(err) + } + if h.Token != "" { + t.Fatal("released holder retains its re-entry token") + } +} + +func TestStderrNullIsNotTerminal(t *testing.T) { + f, err := os.OpenFile(os.DevNull, os.O_WRONLY, 0) + if err != nil { + t.Fatal(err) + } + defer f.Close() + old := Stderr + Stderr = f + t.Cleanup(func() { Stderr = old }) + if stderrIsTerminal() { + t.Fatal("null device is not a terminal") + } +} diff --git a/internal/oplock/guard.go b/internal/oplock/guard.go index 9f99d0b1d..3dcba6909 100644 --- a/internal/oplock/guard.go +++ b/internal/oplock/guard.go @@ -33,6 +33,8 @@ import ( "sync" "time" + "golang.org/x/term" + "github.com/nself-org/cli/internal/compat" "github.com/nself-org/cli/internal/errs" ) @@ -163,15 +165,14 @@ func noteUnsupported(quiet bool) { }) } -// stderrIsTerminal reports whether Stderr is a character device. A variable so +// stderrIsTerminal reports whether Stderr is a terminal. A variable so // tests can pretend to be a terminal. var stderrIsTerminal = func() bool { f, ok := Stderr.(*os.File) if !ok { return false } - fi, err := f.Stat() - return err == nil && fi.Mode()&os.ModeCharDevice != 0 + return term.IsTerminal(int(f.Fd())) } func isHeld(err error) bool { _, ok := err.(*HeldError); return ok } diff --git a/internal/oplock/oplock.go b/internal/oplock/oplock.go index 886dcb092..b65f7d0c2 100644 --- a/internal/oplock/oplock.go +++ b/internal/oplock/oplock.go @@ -146,6 +146,13 @@ func (l *Lock) Release() { return } l.once.Do(func() { + h := l.holder + h.Token = "" + if err := writeHolder(l.f, h); err != nil { + // An unreadable record also refuses re-entry; never leave a usable + // previous token behind while releasing the kernel lock. + _ = l.f.Truncate(0) + } unlock(l.f) _ = l.f.Close() }) @@ -185,7 +192,8 @@ func Acquire(ctx context.Context, projectDir string, opts Opts) (*Lock, error) { } _ = f.Close() h, known := readHolderSettled(ctx, projectDir, o) - if known && token != "" && h.Token == token { + host, hostErr := os.Hostname() + if known && token != "" && h.Token == token && hostErr == nil && host != "" && h.Host == host { return &Lock{holder: h, reentrant: true}, nil } waited := o.Clock.Now().Sub(start) diff --git a/internal/oplock/stale_test.go b/internal/oplock/stale_test.go index c9c7fc0d1..39fe79325 100644 --- a/internal/oplock/stale_test.go +++ b/internal/oplock/stale_test.go @@ -78,8 +78,8 @@ func TestOpLockStaleHolderIsUnknown(t *testing.T) { // Control: a live pid with the same token re-enters, so the refusal // above comes from the stale check alone. {name: "live pid same host", holder: Holder{PID: os.Getpid(), Command: "old", Token: stale, Host: host}, wantKnown: true, wantReent: true}, - // Another host cannot be checked: the record is taken as is. - {name: "dead pid other host", holder: Holder{PID: deadPID, Command: "old", Token: stale, Host: host + "-elsewhere"}, wantKnown: true, wantReent: true}, + // Another host cannot be checked, so its token cannot permit re-entry. + {name: "dead pid other host", holder: Holder{PID: deadPID, Command: "old", Token: stale, Host: host + "-elsewhere"}, wantKnown: true}, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { @@ -99,7 +99,7 @@ func TestOpLockStaleHolderIsUnknown(t *testing.T) { if he.Known != tc.wantKnown { t.Fatalf("Known = %v, want %v (holder %+v)", he.Known, tc.wantKnown, he.Holder) } - if strings.Contains(he.Error(), "999999") { + if !tc.wantKnown && strings.Contains(he.Error(), "999999") { t.Errorf("error names the dead pid: %q", he.Error()) } }) diff --git a/internal/reconcile/containers_init.go b/internal/reconcile/containers_init.go index 3a9f59fd7..76c6cc515 100644 --- a/internal/reconcile/containers_init.go +++ b/internal/reconcile/containers_init.go @@ -19,17 +19,14 @@ import ( "gopkg.in/yaml.v3" ) -// initContainerLabel is the label docker.cleanupInitContainers removes by. -const initContainerLabel = "nself.type=init-container" - type composeLabels struct { Services map[string]struct { Labels yaml.Node `yaml:"labels"` } `yaml:"services"` } -// initContainerServices returns the services carrying initContainerLabel in -// any of files. +// initContainerServices returns services whose effective nself.type label is +// init-container after applying label overrides in Compose file order. func initContainerServices(files []string) (map[string]bool, error) { out := map[string]bool{} for _, path := range files { @@ -45,31 +42,37 @@ func initContainerServices(files []string) (map[string]bool, error) { return nil, fmt.Errorf("parsing compose file %s: %w", path, err) } for name, svc := range doc.Services { - if hasLabel(&svc.Labels, initContainerLabel) { - out[name] = true + if value, present := labelValue(&svc.Labels, "nself.type"); present { + if value == "init-container" { + out[name] = true + } else { + delete(out, name) + } } } } return out, nil } -// hasLabel reports whether a compose labels node (map or list) holds kv. -func hasLabel(n *yaml.Node, kv string) bool { - key, val, _ := strings.Cut(kv, "=") +// labelValue reads one Compose label, preserving whether an override sets it. +func labelValue(n *yaml.Node, key string) (string, bool) { switch n.Kind { case yaml.MappingNode: - var m map[string]string - return n.Decode(&m) == nil && m[key] == val - case yaml.SequenceNode: - var l []string - if n.Decode(&l) != nil { - return false + var labels map[string]string + if n.Decode(&labels) == nil { + value, ok := labels[key] + return value, ok } - for _, e := range l { - if strings.TrimSpace(e) == kv { - return true + case yaml.SequenceNode: + var labels []string + if n.Decode(&labels) == nil { + for i := len(labels) - 1; i >= 0; i-- { + k, value, _ := strings.Cut(labels[i], "=") + if k == key { + return value, true + } } } } - return false + return "", false } diff --git a/internal/reconcile/containers_test.go b/internal/reconcile/containers_test.go index 29544d4bb..5f0397acd 100644 --- a/internal/reconcile/containers_test.go +++ b/internal/reconcile/containers_test.go @@ -135,6 +135,14 @@ func TestInitContainerServices(t *testing.T) { if want := map[string]bool{"meilisearch-init": true, "plugin-init": true}; !reflect.DeepEqual(got, want) { t.Errorf("initContainerServices = %v, want %v", got, want) } + override := write("override.yml", "services:\n meilisearch-init:\n labels: [\"nself.type=service\"]\n plugin-init:\n labels:\n extra: value\n") + got, err = initContainerServices([]string{base, frag, override}) + if err != nil { + t.Fatal(err) + } + if want := map[string]bool{"plugin-init": true}; !reflect.DeepEqual(got, want) { + t.Errorf("merged init services = %v, want %v", got, want) + } if _, err := initContainerServices([]string{write("bad.yml", "services: [")}); err == nil { t.Error("a broken compose file must be an error (container impact unknown), not an empty set") } diff --git a/scripts/ci/affected-pkgs.sh b/scripts/ci/affected-pkgs.sh index 637a64a9c..2d0383344 100644 --- a/scripts/ci/affected-pkgs.sh +++ b/scripts/ci/affected-pkgs.sh @@ -12,12 +12,12 @@ # - the package in its nearest enclosing package directory (its own # .go files, testdata/, data files read beside the code); a root # package only for root files and the root testdata/; -# - for a file outside every package directory (docs, scripts, -# .github/), every package whose .go files name its directory +# - for every non-Go file (including docs, scripts, .github/), +# every package whose .go files name its directory # path (`dir/` or `/dir"` for a top-level one), its top-level # directory as a quoted literal, or (for a root file) its file # name: tests that read the wiki or a CI script are found that way. -# Seeds from .go and embedded files are closed over reverse imports +# Seeds from .go, embedded files and deleted data owners are closed over reverse imports # (`go list -deps`, test imports included); the other seeds only # feed their own package's tests, so they are listed as they are. Prints `./...` alone when go.mod, go.sum, # vendor/ or anything under internal/errs/ changed (everything @@ -114,8 +114,9 @@ awk -F'|' -v modpath="$MODPATH" -v moddir="$MODDIR" ' sed -n 's/^P //p' "$TMP/mapped" > "$TMP/seeds" sed -n 's/^D //p' "$TMP/mapped" > "$TMP/direct" -# Files outside every package: find the module .go files that name them. -sed -n 's/^U //p' "$TMP/mapped" | while IFS= read -r f; do +# Data can be owned by one package and read by another. Scan every non-Go +# path, including owned data, for those readers. +sed '/\.go$/d' "$TMP/changed" | while IFS= read -r f; do case "$f" in */*/*) printf '%s\n' "${f%/*}" @@ -149,6 +150,23 @@ if [ -s "$TMP/patterns" ]; then fi fi +# A removed embedded file no longer appears in go list's EmbedFiles. Close +# deleted data owners over imports conservatively, so wildcard embeds still +# reach their consumers when another matching file keeps the pattern valid. +while IFS= read -r f; do + [ -e "$f" ] && continue + case "$f" in *.go) continue ;; esac + d="$f" + while [ "${d%/*}" != "$d" ]; do + d="${d%/*}" + awk -F'|' -v dir="$MODDIR/$d" '$2 == dir { print $1 }' "$TMP/graph" + done + case "$f" in + */*) ;; + *) awk -F'|' -v dir="$MODDIR" '$2 == dir { print $1 }' "$TMP/graph" ;; + esac +done < "$TMP/changed" >> "$TMP/seeds" + if [ ! -s "$TMP/seeds" ] && [ ! -s "$TMP/direct" ]; then echo "affected-pkgs: no Go package is affected by this change set" >&2 exit 0 diff --git a/scripts/ci/affected-pkgs_test.sh b/scripts/ci/affected-pkgs_test.sh index 4504521dd..c8bea94c3 100755 --- a/scripts/ci/affected-pkgs_test.sh +++ b/scripts/ci/affected-pkgs_test.sh @@ -38,19 +38,20 @@ printf 'module example.invalid/fx\n\ngo %s\n' "$GOV" > "$R/go.mod" cat > "$R/a/a.go" <<'EOF' package a -import _ "embed" +import "embed" -//go:embed assets/conf.yaml -var Conf string +//go:embed assets/*.yaml +var Conf embed.FS EOF printf 'key: one\n' > "$R/a/assets/conf.yaml" +printf 'key: two\n' > "$R/a/assets/other.yaml" printf 'package assets\n' > "$R/a/assets/assets.go" cat > "$R/b/b.go" <<'EOF' package b import "example.invalid/fx/a" -func Conf() string { return a.Conf } +func Conf() ([]byte, error) { return a.Conf.ReadFile("assets/other.yaml") } EOF printf 'package c\n' > "$R/c/c.go" printf 'input\n' > "$R/c/testdata/in.txt" @@ -64,6 +65,7 @@ import ( ) func TestGuide(t *testing.T) { + _, _ = os.ReadFile("../c/testdata/in.txt") if _, err := os.ReadFile("../docs/guide.md"); err != nil { t.Fatal(err) } @@ -77,6 +79,7 @@ import _ "example.invalid/fx/d" EOF printf '# guide\n' > "$R/docs/guide.md" printf 'todo\n' > "$R/notes/todo.txt" +(cd "$R" && go test ./...) git -C "$R" init -q git -C "$R" add -A git -C "$R" commit -qm base @@ -119,6 +122,7 @@ check() { case "$c" in edit:*) printf '// changed\n' >> "$R/${c#edit:}" ;; new:*) mkdir -p "$(dirname "$R/${c#new:}")"; printf 'new\n' > "$R/${c#new:}" ;; + delete:*) rm "$R/${c#delete:}" ;; stage) git -C "$R" add -A ;; commit) git -C "$R" add -A && git -C "$R" commit -qm change ;; esac @@ -144,7 +148,9 @@ cases() { # cases