Deployment openers enable the change index; opener classification test (#336)
Every site that opens the deployment bucket now uses NewDeploymentS3Store[In]. TestS3StoreOpenersClassified parses the module and requires every other NewS3Store[In] call and every S3Store literal to be listed with the bucket it opens. The console authority census follows the renamed constructor.
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
11 files changed,
+315
−15
cli.go+3 −3cmd/console/main.go+1 −1internal/console/claim_authority_census_test.go+1 −1internal/console/testdata/claim-authority-sites.txt+1 −1internal/edge/edge.go+1 −1internal/host/activation_command_linux.go+4 −4internal/host/agent.go+4 −2internal/host/boxpublish_test.go+38 −0internal/ops/ops.go+1 −1internal/release/openers_test.go+260 −0internal/vet/vet.go+1 −1
diff --git a/cli.go b/cli.go| index c806199a2..e97f3848a 100644 |
| --- a/cli.go |
| +++ b/cli.go |
| @@ -1954,7 +1954,7 @@ func deploymentReadSeam() (release.Store, ed25519.PublicKey) { |
| return nil, nil |
| } |
| if b := os.Getenv("CARLOS_DEPLOYMENT_BUCKET"); b != "" { |
| - s, err := release.NewS3Store(context.Background(), b) |
| + s, err := release.NewDeploymentS3Store(context.Background(), b) |
| if err != nil { |
| return nil, pub |
| } |
| @@ -2064,7 +2064,7 @@ func accountFlagsWithRegistry(fs *flag.FlagSet) (raw, rawID, regPath *string, re |
| func openStore() release.Store { |
| if b := os.Getenv("CARLOS_DEPLOYMENT_BUCKET"); b != "" { |
| - s, err := release.NewS3Store(context.Background(), b) |
| + s, err := release.NewDeploymentS3Store(context.Background(), b) |
| if err != nil { |
| log.Fatal(err) |
| } |
| @@ -3588,7 +3588,7 @@ func statusLive(w io.Writer) error { |
| if bucket == "" { |
| return fmt.Errorf("set CARLOS_STATUS_BUCKET or CARLOS_DEPLOYMENT_BUCKET") |
| } |
| - s, err := release.NewS3Store(ctx, bucket) |
| + s, err := release.NewDeploymentS3Store(ctx, bucket) |
| if err != nil { |
| return err |
| } |
diff --git a/cmd/console/main.go b/cmd/console/main.go| index bdee22837..20850877d 100644 |
| --- a/cmd/console/main.go |
| +++ b/cmd/console/main.go |
| @@ -640,7 +640,7 @@ func optionalStore() release.Store { |
| // duplicated here because cmd/console cannot import package main. |
| func openStore() release.Store { |
| if b := os.Getenv("CARLOS_DEPLOYMENT_BUCKET"); b != "" { |
| - s, err := release.NewS3Store(context.Background(), b) |
| + s, err := release.NewDeploymentS3Store(context.Background(), b) |
| if err != nil { |
| log.Fatal(err) |
| } |
diff --git a/internal/console/claim_authority_census_test.go b/internal/console/claim_authority_census_test.go| index e8646b917..eb84a7ab3 100644 |
| --- a/internal/console/claim_authority_census_test.go |
| +++ b/internal/console/claim_authority_census_test.go |
| @@ -21,7 +21,7 @@ func claimAuthoritySites(t *testing.T) []string { |
| "NewDirStore": true, "Open": true, "OpenDB": true, |
| "NewConditionalRecordStore": true, "NewVersionedBoundary": true, "NewFromConfig": true, "S3ClientFor": true, "NewControlWriter": true, "NewRestoreControl": true, "NewMutationBoundary": true, |
| "NewObservedMutationBoundary": true, "NewProductionMutationCheck": true, |
| - "NewS3Store": true, "NewS3StoreIn": true, "NewVersionedS3": true, "NewAWSProvisioner": true, |
| + "NewS3Store": true, "NewS3StoreIn": true, "NewDeploymentS3Store": true, "NewDeploymentS3StoreIn": true, "NewVersionedS3": true, "NewAWSProvisioner": true, |
| "NewClamAVScanner": true, "NewRoute53Writer": true, "BeginExclusive": true, |
| "ResumeExclusive": true, "RegisterStart": true, "FinishWakeRetryReset": true, "ReservePurge": true, |
| "ConsoleMove": true, "InspectMoveLoss": true, "DeclareMoveLoss": true, |
diff --git a/internal/console/testdata/claim-authority-sites.txt b/internal/console/testdata/claim-authority-sites.txt| index d88304f0b..e4c8047d5 100644 |
| --- a/internal/console/testdata/claim-authority-sites.txt |
| +++ b/internal/console/testdata/claim-authority-sites.txt |
| @@ -1,6 +1,6 @@ |
| ../../cmd/console/main.go|openServeDB|console.OpenGated(envOr("CONSOLE_DB", "/data/console/console.db")) |
| ../../cmd/console/main.go|openStore|release.NewDirStore(d) |
| -../../cmd/console/main.go|openStore|release.NewS3Store(context.Background(), b) |
| +../../cmd/console/main.go|openStore|release.NewDeploymentS3Store(context.Background(), b) |
| ../../cmd/console/main.go|serve|lease.NewVersionedS3(context.Background(), buckets) |
| ../../cmd/console/main.go|serve|release.NewS3StoreIn(context.Background(), dbBucket, dbRegion) |
| ../../cmd/console/main.go|storeBackend|objstore.NewAWSProvisioner(context.Background(), region, endpoint, boundary) |
diff --git a/internal/edge/edge.go b/internal/edge/edge.go| index 06c60b869..e38ab6594 100644 |
| --- a/internal/edge/edge.go |
| +++ b/internal/edge/edge.go |
| @@ -3404,7 +3404,7 @@ func enableStaticFromEnv(e *Edge, dataDir string) error { |
| } |
| var store release.Store |
| if bucket != "" { |
| - s, err := release.NewS3Store(context.Background(), bucket) |
| + s, err := release.NewDeploymentS3Store(context.Background(), bucket) |
| if err != nil { |
| return err |
| } |
diff --git a/internal/host/activation_command_linux.go b/internal/host/activation_command_linux.go| index 8f2525c1d..60795fce6 100644 |
| --- a/internal/host/activation_command_linux.go |
| +++ b/internal/host/activation_command_linux.go |
| @@ -64,7 +64,7 @@ func runSystemClaims(args []string, out io.Writer) error { |
| if err != nil { |
| return errActivation |
| } |
| - store, err := release.NewS3StoreIn(ctx, *bucket, *region) |
| + store, err := release.NewDeploymentS3StoreIn(ctx, *bucket, *region) |
| if err != nil { |
| return err |
| } |
| @@ -89,7 +89,7 @@ func runSystemClaims(args []string, out io.Writer) error { |
| if err != nil { |
| return errActivation |
| } |
| - store, err := release.NewS3StoreIn(ctx, *bucket, *region) |
| + store, err := release.NewDeploymentS3StoreIn(ctx, *bucket, *region) |
| if err != nil { |
| return err |
| } |
| @@ -203,7 +203,7 @@ func runSystemClaims(args []string, out io.Writer) error { |
| if produces != (*outPath != "") { |
| return errActivation |
| } |
| - store, err := release.NewS3StoreIn(ctx, plan.Bucket, *region) |
| + store, err := release.NewDeploymentS3StoreIn(ctx, plan.Bucket, *region) |
| if err != nil { |
| return err |
| } |
| @@ -280,7 +280,7 @@ func runSystemClaims(args []string, out io.Writer) error { |
| if err != nil { |
| return errActivation |
| } |
| - store, err := release.NewS3StoreIn(ctx, *bucket, *region) |
| + store, err := release.NewDeploymentS3StoreIn(ctx, *bucket, *region) |
| if err != nil { |
| return err |
| } |
diff --git a/internal/host/agent.go b/internal/host/agent.go| index 559c0a27c..ca78f08ca 100644 |
| --- a/internal/host/agent.go |
| +++ b/internal/host/agent.go |
| @@ -4365,7 +4365,7 @@ func OpenDeployStore(ctx context.Context) (release.Store, error) { return openDe |
| // API; put it back and it is off. |
| func openDeployStoreEnv(ctx context.Context, getenv func(string) string) (release.Store, error) { |
| if b := getenv("CARLOS_DEPLOYMENT_BUCKET"); b != "" { |
| - return release.NewS3Store(ctx, b) |
| + return release.NewDeploymentS3Store(ctx, b) |
| } |
| if d := getenv("CARLOS_DEPLOYMENT_DIR"); d != "" { |
| return release.NewDirStore(d), nil |
| @@ -4459,7 +4459,9 @@ var errNoStatusStore = errors.New("no status store configured") |
| // and wraps whichever store this returns. |
| func openStatusStore(ctx context.Context, getenv func(string) string) (release.Store, string, error) { |
| if bucket := resolveStatusBucket(getenv); bucket != "" { |
| - s, err := release.NewS3Store(ctx, bucket) |
| + // The fallback is the deployment bucket, so open it as one; status/ and |
| + // metrics/ are not change scopes and bump nothing. |
| + s, err := release.NewDeploymentS3Store(ctx, bucket) |
| if err != nil { |
| return nil, "", err |
| } |
diff --git a/internal/host/boxpublish_test.go b/internal/host/boxpublish_test.go| index ec6c80402..0e797f5d0 100644 |
| --- a/internal/host/boxpublish_test.go |
| +++ b/internal/host/boxpublish_test.go |
| @@ -735,3 +735,41 @@ func TestBoxRecordLoopGateIsUnchanged(t *testing.T) { |
| }) |
| } |
| } |
| + |
| +// Box publication reaches the deployment bucket through openDeployStoreEnv |
| +// and requires the *release.S3Store it returns (boxpublish.go's |
| +// opened.(*release.S3Store)). The change index must not cost it that type: |
| +// the opener hands back the same concrete store with the index enabled, |
| +// against the real S3 wiring, and publication still writes only its own |
| +// control/boxes/ key (a key outside every change scope, so no index traffic). |
| +// Not parallel: publishFixture sets the process environment. |
| +func TestBoxPublishThroughDeploymentOpener(t *testing.T) { |
| + b := publishFixture(t) |
| + ctx := context.Background() |
| + opened, err := openDeployStoreEnv(ctx, os.Getenv) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if _, ok := opened.(*release.S3Store); !ok { |
| + t.Fatalf("openDeployStoreEnv returned %T; box publication needs the *release.S3Store", opened) |
| + } |
| + // The opener enabled the index: a scoped write bumps its generation. |
| + if _, err := opened.Put(ctx, "accounts/shop/manifest.json", []byte("{}"), ""); err != nil { |
| + t.Fatal(err) |
| + } |
| + idx, _, err := release.ReadChangeIndex(ctx, opened) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if got := idx.Entries["accounts/shop"]; got != 1 { |
| + t.Fatalf("accounts/shop generation = %d after one scoped Put, want 1 (entries %v)", got, idx.Entries) |
| + } |
| + // Publication itself is unchanged: one create-only PUT on its own key. |
| + before := len(b.requests()) |
| + if _, err := b.run(t); err != nil { |
| + t.Fatal(err) |
| + } |
| + if puts := assertCreateOnly(t, b.requests()[before:]); puts != 1 { |
| + t.Fatalf("%d PUTs, want one", puts) |
| + } |
| +} |
diff --git a/internal/ops/ops.go b/internal/ops/ops.go| index dc24bb278..bd5f2fb9b 100644 |
| --- a/internal/ops/ops.go |
| +++ b/internal/ops/ops.go |
| @@ -336,7 +336,7 @@ func boxLabel() string { |
| // quietly. |
| func openDeployStore(ctx context.Context) (release.Store, error) { |
| if b := os.Getenv("CARLOS_DEPLOYMENT_BUCKET"); b != "" { |
| - return release.NewS3Store(ctx, b) |
| + return release.NewDeploymentS3Store(ctx, b) |
| } |
| if d := os.Getenv("CARLOS_DEPLOYMENT_DIR"); d != "" { |
| return release.NewDirStore(d), nil |
diff --git a/internal/release/openers_test.go b/internal/release/openers_test.go| new file mode 100644 |
| index 000000000..e98e1aa8e |
| --- /dev/null |
| +++ b/internal/release/openers_test.go |
| @@ -0,0 +1,260 @@ |
| +package release |
| + |
| +import ( |
| + "go/ast" |
| + "go/parser" |
| + "go/token" |
| + "io/fs" |
| + "os" |
| + "path/filepath" |
| + "sort" |
| + "strconv" |
| + "strings" |
| + "testing" |
| +) |
| + |
| +// The change index (control/changes.json) only moves if every writer of the |
| +// deployment bucket bumps it. A bump lives in the concrete store and is off |
| +// until EnableChangeIndex, so the way a deployment-bucket writer goes missing |
| +// is a new call site that opens the bucket with the plain constructor. This |
| +// test finds every place the module builds an S3Store and makes each one say |
| +// which kind of bucket it is for: |
| +// |
| +// - the deployment bucket (apps/, accounts/, control/): the site calls |
| +// NewDeploymentS3Store[In], so it never appears here at all; |
| +// - any other bucket: the site is listed in otherBucketOpeners with the |
| +// reason, and the plain constructor is right; |
| +// - a hand-built &release.S3Store{...} over the deployment bucket: listed |
| +// in deploymentLiteralSites, and the test checks the same function calls |
| +// EnableChangeIndex. |
| +// |
| +// Keys are "path/from/module/root.go:enclosingFunc". Two sites in one |
| +// function share a key, so a function that opens both kinds of bucket needs |
| +// splitting before it can be classified. |
| + |
| +// otherBucketOpeners are the sites that open a bucket which is not the |
| +// deployment bucket, through NewS3Store, NewS3StoreIn or a composite literal. |
| +var otherBucketOpeners = map[string]string{ |
| + "cmd/console/main.go:serve": "console seed data store: the database/data bucket (seeds/), not the deployment bucket", |
| + "internal/host/seedwiring.go:seedDataOpener": "this region's data bucket, where seeds/ lives", |
| + "internal/host/relocatepass.go:productionRelocationRuntimeInputs": "relocation files store: the data bucket, manifests under moves/ prefixes", |
| + "internal/host/relocatepass.go:registerRelocationSource": "relocation files store: the data bucket, manifests under moves/ prefixes", |
| + "internal/host/claimfiles.go:newProductionFilesStore": "claim files manifests: the data bucket, moves/ prefixes", |
| + "internal/hostfence/writer.go:OpenRootFenceStore": "fence writer: the fence bucket, written under its own scoped role", |
| + "internal/lease/execution_store.go:ExecutionStore": "execution/build state: the hibernate and claim data bucket, keyed by host prefix", |
| +} |
| + |
| +// deploymentLiteralSites are hand-built S3Store literals over the deployment |
| +// bucket. Each must call EnableChangeIndex in the same function. |
| +var deploymentLiteralSites = map[string]string{} |
| + |
| +type openerSite struct { |
| + key string // path:func |
| + line int |
| + what string // what was found, for the failure message |
| + literal bool |
| + enabled bool // the enclosing function calls EnableChangeIndex |
| +} |
| + |
| +const releaseImportPath = "github.com/carlosframework/platform/internal/release" |
| + |
| +// scanOpenerSites lists the S3Store constructions in one parsed file. |
| +// inReleasePkg is true for files of package release itself, where the |
| +// constructors and the type are referred to without a qualifier. |
| +func scanOpenerSites(fset *token.FileSet, f *ast.File, rel string, inReleasePkg bool) []openerSite { |
| + qual := "" |
| + for _, im := range f.Imports { |
| + p, _ := strconv.Unquote(im.Path.Value) |
| + if p != releaseImportPath { |
| + continue |
| + } |
| + qual = "release" |
| + if im.Name != nil { |
| + qual = im.Name.Name |
| + } |
| + } |
| + if qual == "" && !inReleasePkg { |
| + return nil |
| + } |
| + // isRelease reports whether e names the identifier in package release. |
| + isRelease := func(e ast.Expr, name string) bool { |
| + switch x := e.(type) { |
| + case *ast.SelectorExpr: |
| + id, ok := x.X.(*ast.Ident) |
| + return ok && qual != "" && id.Name == qual && x.Sel.Name == name |
| + case *ast.Ident: |
| + return inReleasePkg && x.Name == name |
| + } |
| + return false |
| + } |
| + var sites []openerSite |
| + for _, decl := range f.Decls { |
| + fn := "<package>" |
| + var root ast.Node = decl |
| + if fd, ok := decl.(*ast.FuncDecl); ok { |
| + fn = fd.Name.Name |
| + if fd.Body == nil { |
| + continue |
| + } |
| + root = fd.Body |
| + } |
| + enabled := false |
| + var found []openerSite |
| + ast.Inspect(root, func(n ast.Node) bool { |
| + switch x := n.(type) { |
| + case *ast.CallExpr: |
| + if sel, ok := x.Fun.(*ast.SelectorExpr); ok && sel.Sel.Name == "EnableChangeIndex" { |
| + enabled = true |
| + } |
| + for _, name := range []string{"NewS3Store", "NewS3StoreIn"} { |
| + if isRelease(x.Fun, name) { |
| + found = append(found, openerSite{key: rel + ":" + fn, line: fset.Position(x.Pos()).Line, what: "call to " + name}) |
| + } |
| + } |
| + case *ast.CompositeLit: |
| + if isRelease(x.Type, "S3Store") { |
| + found = append(found, openerSite{key: rel + ":" + fn, line: fset.Position(x.Pos()).Line, what: "S3Store literal", literal: true}) |
| + } |
| + } |
| + return true |
| + }) |
| + for _, s := range found { |
| + s.enabled = enabled |
| + sites = append(sites, s) |
| + } |
| + } |
| + return sites |
| +} |
| + |
| +func moduleRoot(t *testing.T) string { |
| + t.Helper() |
| + dir, err := os.Getwd() |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + for { |
| + if _, err := os.Stat(filepath.Join(dir, "go.mod")); err == nil { |
| + return dir |
| + } |
| + parent := filepath.Dir(dir) |
| + if parent == dir { |
| + t.Fatal("no go.mod above the test directory") |
| + } |
| + dir = parent |
| + } |
| +} |
| + |
| +func TestS3StoreOpenersClassified(t *testing.T) { |
| + root := moduleRoot(t) |
| + fset := token.NewFileSet() |
| + var sites []openerSite |
| + err := filepath.WalkDir(root, func(path string, d fs.DirEntry, err error) error { |
| + if err != nil { |
| + return err |
| + } |
| + name := d.Name() |
| + if d.IsDir() { |
| + if path != root && (strings.HasPrefix(name, ".") || name == "vendor" || name == "testdata" || name == "node_modules") { |
| + return filepath.SkipDir |
| + } |
| + if path != root { |
| + if _, err := os.Stat(filepath.Join(path, "go.mod")); err == nil { |
| + return filepath.SkipDir // a nested module |
| + } |
| + } |
| + return nil |
| + } |
| + if !strings.HasSuffix(name, ".go") || strings.HasSuffix(name, "_test.go") { |
| + return nil |
| + } |
| + rel, err := filepath.Rel(root, path) |
| + if err != nil { |
| + return err |
| + } |
| + rel = filepath.ToSlash(rel) |
| + if rel == "internal/release/store.go" { |
| + return nil // the constructors themselves |
| + } |
| + f, err := parser.ParseFile(fset, path, nil, parser.SkipObjectResolution) |
| + if err != nil { |
| + return err |
| + } |
| + sites = append(sites, scanOpenerSites(fset, f, rel, f.Name.Name == "release" && filepath.ToSlash(filepath.Dir(rel)) == "internal/release")...) |
| + return nil |
| + }) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + |
| + seen := map[string]bool{} |
| + var bad []string |
| + for _, s := range sites { |
| + seen[s.key] = true |
| + if _, ok := deploymentLiteralSites[s.key]; ok && s.literal { |
| + if !s.enabled { |
| + bad = append(bad, s.key+": deployment-bucket S3Store literal never calls EnableChangeIndex in this function") |
| + } |
| + continue |
| + } |
| + if _, ok := otherBucketOpeners[s.key]; ok { |
| + continue |
| + } |
| + bad = append(bad, s.key+" (line "+strconv.Itoa(s.line)+"): "+s.what+" is not classified. Deployment bucket: use NewDeploymentS3Store[In] (or EnableChangeIndex on the literal and list it in deploymentLiteralSites). Any other bucket: add it to otherBucketOpeners with the reason") |
| + } |
| + for k := range otherBucketOpeners { |
| + if !seen[k] { |
| + bad = append(bad, k+": listed in otherBucketOpeners but no longer opens an S3Store (remove the entry)") |
| + } |
| + } |
| + for k := range deploymentLiteralSites { |
| + if !seen[k] { |
| + bad = append(bad, k+": listed in deploymentLiteralSites but no longer builds an S3Store literal (remove the entry)") |
| + } |
| + } |
| + sort.Strings(bad) |
| + for _, b := range bad { |
| + t.Error(b) |
| + } |
| + if len(sites) == 0 { |
| + t.Fatal("the scan found no S3Store sites at all; the walk is not reaching the module") |
| + } |
| +} |
| + |
| +// TestOpenerScanRecognisesEveryForm keeps the scanner honest: qualified and |
| +// aliased constructors, literals with and without &, and the enable check. |
| +func TestOpenerScanRecognisesEveryForm(t *testing.T) { |
| + const src = `package x |
| + |
| +import rel "github.com/carlosframework/platform/internal/release" |
| + |
| +func a() { rel.NewS3Store(nil, "b") } |
| +func b() { rel.NewS3StoreIn(nil, "b", "r") } |
| +func c() { _ = &rel.S3Store{Bucket: "b"} } |
| +func d() { s := rel.S3Store{Bucket: "b"}; _ = s } |
| +func e() { s := &rel.S3Store{Bucket: "b"}; s.EnableChangeIndex() } |
| +func f() { rel.NewDeploymentS3Store(nil, "b") } |
| +` |
| + fset := token.NewFileSet() |
| + f, err := parser.ParseFile(fset, "x.go", src, 0) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + got := map[string]openerSite{} |
| + for _, s := range scanOpenerSites(fset, f, "x.go", false) { |
| + got[s.key] = s |
| + } |
| + for _, want := range []string{"x.go:a", "x.go:b", "x.go:c", "x.go:d", "x.go:e"} { |
| + if _, ok := got[want]; !ok { |
| + t.Errorf("%s not found", want) |
| + } |
| + } |
| + if _, ok := got["x.go:f"]; ok { |
| + t.Error("NewDeploymentS3Store is not a plain opener") |
| + } |
| + if !got["x.go:c"].literal || got["x.go:c"].enabled { |
| + t.Errorf("c: literal without enable, got %+v", got["x.go:c"]) |
| + } |
| + if !got["x.go:e"].enabled { |
| + t.Errorf("e: enable call not seen, got %+v", got["x.go:e"]) |
| + } |
| +} |
diff --git a/internal/vet/vet.go b/internal/vet/vet.go| index a52736f29..2409ec8ca 100644 |
| --- a/internal/vet/vet.go |
| +++ b/internal/vet/vet.go |
| @@ -223,7 +223,7 @@ func Run(args []string) { |
| // set, else a directory store at CARLOS_DEPLOYMENT_DIR. |
| func openStore() release.Store { |
| if b := os.Getenv("CARLOS_DEPLOYMENT_BUCKET"); b != "" { |
| - s, err := release.NewS3Store(context.Background(), b) |
| + s, err := release.NewDeploymentS3Store(context.Background(), b) |
| if err != nil { |
| log.Fatal(err) |
| } |