diff --git a/.govulncheck.yaml b/.govulncheck.yaml index 87404d72..a9f7c368 100644 --- a/.govulncheck.yaml +++ b/.govulncheck.yaml @@ -34,7 +34,9 @@ suppressions: Follow-up unchanged and still not landed: core migrates releaseupdate/go-selfupdate off x/crypto/openpgp. Drop this entry once that ships. - reviewed: 2026-09-14 + 2026-10-30 re-review: govulncheck 1.9.0 (db 2026-10-27) still reports + one module-level trace, no symbol reached, Fixed still N/A. + reviewed: 2026-10-30 - id: GO-2026-4887 module: github.com/docker/docker @@ -51,7 +53,11 @@ suppressions: Upstream fix: only in github.com/moby/moby/v2 >= 2.0.0-beta.8. That is a renamed module and still pre-release; github.com/docker/docker publishes no v29 tag, so no dependency bump makes this actionable. - reviewed: 2026-09-14 + 2026-10-30 re-review: docker/docker v28.5.2 is the newest v28 release; + the fix exists only in moby/moby/v2 (2.0.0-beta.8). 95 traces, all + client/* or api/types/* frames and package-init chains; none reaches + pkg/authorization or the daemon. + reviewed: 2026-10-30 - id: GO-2026-4883 module: github.com/docker/docker @@ -63,4 +69,6 @@ suppressions: pkg/authorization. Upstream fix: only in github.com/moby/moby/v2 >= 2.0.0-beta.8, the renamed pre-release module; unreachable from docker/docker v28. - reviewed: 2026-09-14 + 2026-10-30 re-review: same trace set and verdict as GO-2026-4887; + docker/docker v28.5.2 is the newest v28, fix only in moby/moby/v2. + reviewed: 2026-10-30 diff --git a/cmd/delivery.go b/cmd/delivery.go index cbbd3532..a859439e 100644 --- a/cmd/delivery.go +++ b/cmd/delivery.go @@ -5,6 +5,10 @@ import ( "github.com/spf13/cobra" ) -var DeliveryCmd = &cobra.Command{Use: "delivery", Short: "Manage delivery signing keys", Long: "Manage the local keys used to sign delivery documents for a local cluster.", Example: " codefly delivery key"} +var DeliveryCmd = &cobra.Command{Use: "delivery", Short: "Manage delivery signing keys and send prepared delivery documents", Long: "Manage the local keys used to sign delivery documents for a local cluster, and send the prepared paired documents a render wrote to the host's composition interface.", Example: " codefly delivery key\n codefly delivery apply --documents ./prepared --address localhost:8081 --identity-file ./token"} -func init() { DeliveryCmd.AddCommand(delivery.KeyCmd) } +func init() { + DeliveryCmd.AddCommand(delivery.KeyCmd) + DeliveryCmd.AddCommand(delivery.PrepareCmd) + DeliveryCmd.AddCommand(delivery.ApplyCmd) +} diff --git a/cmd/delivery/apply.go b/cmd/delivery/apply.go new file mode 100644 index 00000000..42899a2f --- /dev/null +++ b/cmd/delivery/apply.go @@ -0,0 +1,128 @@ +package delivery + +import ( + "errors" + "fmt" + "os" + "strings" + "time" + + "github.com/spf13/cobra" + + "github.com/codefly-dev/cli/cmd/common" + "github.com/codefly-dev/cli/pkg/gitops" + "github.com/codefly-dev/cli/pkg/orchestration" + + compositionv1 "github.com/codefly-dev/authority/libraries/authority-client/go/gen/host/composition/v1" +) + +// applyOptions are the flags, named so the Job that runs this reads as the +// thing it does. +var applyOptions struct { + documents, address, identityFile string + secure bool + budget, request, maxPause time.Duration +} + +// ApplyCmd sends the prepared paired delivery documents a render wrote to the +// host's composition interface. It is what a cell's delivery Job runs: the +// documents are mounted, the credential is projected, and neither is baked +// into anything. +// +// NOT exposed over MCP, deliberately. Every input it needs exists only inside +// the pod that runs it — a mounted document set, a projected credential and a +// cluster-internal address — so an assistant calling it from outside has +// nothing to pass, and the one thing it could pass is a path to a credential. +// The verbs that produce what this sends (render and publish) are the ones +// worth driving. +var ApplyCmd = &cobra.Command{ + Use: "apply", Short: "Send prepared paired delivery documents to the host's composition interface", + Long: `Send the prepared delivery documents under --documents to the host's composition +interface (host.composition.v1.CompositionService/ApplyManifest) at --address. + +Each document is one ApplyManifestRequest carrying both signed halves — core's +module manifest in its DSSE carrier, and the authority declaration signed +against that carrier's exact bytes: method policies, scope ceilings, roles, +owner-awareness, the installation selector and its person-context edges. Only +this interface takes the pair. The host's legacy HTTP manifest route takes the +Core half alone, so a module delivered that way reaches the host with no roles. + +A prepared document carries NO credential: a delivery credential is a +projected ServiceAccount token with a lifetime of minutes, and one written into +a rendered manifest would be a published secret and expired by the time the +manifest synced. The credential is read from --identity-file at execution, +once per request, so a token rotated between rounds is the one the next round +sends. It is sent RAW, in the request's credential field, because the host adds +the bearer scheme itself. It is never printed and never passed as an argument. + +Documents are sent in ROUNDS: every pending one once per round, then a pause +that doubles up to --max-pause, until each is answered or --budget is spent, so +a document the host keeps failing never keeps a later one — a withdrawal among +them — from its first attempt. A refusal the host means (an invalid document, a +refused carrier, a stale or rewritten generation, a withdrawal it keeps +terminal) refuses that document and is not retried; only "not serving yet" +(unavailable, out of time, out of capacity) is. The command fails when any +document was not accepted, so a delivery that did not land fails the Job that +ran it. + +The address is the host's delivery endpoint. A Connect endpoint is one: a +Connect server answers the gRPC protocol on the same listener (h2c in +plaintext), so --address is : of the endpoint the environment's +host.delivery names, and --secure only when that endpoint serves TLS itself. + +A GitOps publish toward a host whose delivery endpoint declares +codefly.dev/composition writes these documents into the delivery overlay, one +per manifest, and renders the manifest's delivery Job to run this command.`, + Example: ` codefly delivery apply --documents /etc/codefly/delivery --address accounts.platform.svc.cluster.local:8081 --identity-file /var/run/secrets/codefly/delivery-token + codefly delivery apply --documents ./prepared --address localhost:8081 --identity-file ./token --budget 2m`, + Args: cobra.NoArgs, + RunE: func(cmd *cobra.Command, _ []string) error { + ctx, cancel := common.NewContext() + defer cancel() + if applyOptions.documents == "" { + return errors.New("--documents is the directory holding the prepared delivery documents") + } + if applyOptions.address == "" { + return errors.New("--address is : of the endpoint the environment's host.delivery names") + } + if applyOptions.identityFile == "" { + return errors.New("--identity-file is the projected delivery credential, read at execution") + } + connection, err := orchestration.GRPCTarget{Address: applyOptions.address, Secure: applyOptions.secure}.Dial() + if err != nil { + return fmt.Errorf("dial the host's composition interface at %s: %w", applyOptions.address, err) + } + defer func() { _ = connection.Close() }() + sender := &gitops.PairedApply{ + Client: compositionv1.NewCompositionServiceClient(connection), + Credential: func() (string, error) { return readCredential(applyOptions.identityFile) }, + Budget: applyOptions.budget, Request: applyOptions.request, MaxPause: applyOptions.maxPause, + Report: func(line string) { fmt.Fprintln(cmd.OutOrStdout(), line) }, + } + _, err = sender.Apply(ctx, applyOptions.documents) + return err + }, +} + +// readCredential reads the projected token and returns it as the host takes +// it: raw, with no bearer scheme and no surrounding whitespace, which a +// projection's trailing newline would otherwise carry into the request. The +// value is returned and nothing more — it is not logged, and the error names +// the path rather than the content. +func readCredential(path string) (string, error) { + data, err := os.ReadFile(path) + if err != nil { + return "", fmt.Errorf("read the projected delivery credential at %s: %w", path, err) + } + return strings.TrimSpace(string(data)), nil +} + +func init() { + ApplyCmd.Flags().StringVar(&applyOptions.documents, "documents", "", "Directory holding the prepared delivery documents (*.json)") + ApplyCmd.Flags().StringVar(&applyOptions.address, "address", "", ": of the host's delivery endpoint") + ApplyCmd.Flags().StringVar(&applyOptions.identityFile, "identity-file", "", "Path of the projected delivery credential, read at execution") + ApplyCmd.Flags().BoolVar(&applyOptions.secure, "secure", false, "Dial with TLS, for a delivery endpoint that serves it itself") + ApplyCmd.Flags().DurationVar(&applyOptions.budget, "budget", 25*time.Minute, "Bound on the whole run") + ApplyCmd.Flags().DurationVar(&applyOptions.request, "request-timeout", 30*time.Second, "Bound on one call") + ApplyCmd.Flags().DurationVar(&applyOptions.maxPause, "max-pause", time.Minute, "Longest pause between rounds") +} diff --git a/cmd/delivery/apply_test.go b/cmd/delivery/apply_test.go new file mode 100644 index 00000000..51a1561b --- /dev/null +++ b/cmd/delivery/apply_test.go @@ -0,0 +1,146 @@ +package delivery + +import ( + "bytes" + "context" + "crypto/ed25519" + "net" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/codefly-dev/core/solutionhost" + "github.com/stretchr/testify/require" + "google.golang.org/grpc" + "google.golang.org/protobuf/proto" + + "github.com/codefly-dev/cli/pkg/gitops" + + compositionv1 "github.com/codefly-dev/authority/libraries/authority-client/go/gen/host/composition/v1" +) + +// qualifyingHost is a real composition host on a real TCP listener: the command +// dials it exactly as a delivery Job dials the cell's, so what this exercises +// is the whole path — flags, dial, the credential read from a file on disk, +// the generated client and the wire. +type qualifyingHost struct { + compositionv1.UnimplementedCompositionServiceServer + seen []*compositionv1.ApplyManifestRequest +} + +func (h *qualifyingHost) ApplyManifest(_ context.Context, request *compositionv1.ApplyManifestRequest) (*compositionv1.ApplyManifestResponse, error) { + h.seen = append(h.seen, proto.Clone(request).(*compositionv1.ApplyManifestRequest)) + manifest, err := solutionhost.ManifestFromVerified(request.GetCarrier().GetDocument()) + if err != nil { + return nil, err + } + return &compositionv1.ApplyManifestResponse{Principal: manifest.Alias, Generation: manifest.Generation}, nil +} + +// The command delivers both halves to a host it dials over TCP, with the +// credential read from the projected file at execution. This is the local +// qualification of the verb a cell's delivery Job runs. +func TestApplyDeliversBothHalvesToAHostItDials(t *testing.T) { + host := &qualifyingHost{} + server := grpc.NewServer() + compositionv1.RegisterCompositionServiceServer(server, host) + listener, err := net.Listen("tcp", "127.0.0.1:0") + require.NoError(t, err) + go func() { _ = server.Serve(listener) }() + t.Cleanup(server.Stop) + + signer, err := solutionhost.NewInMemorySigner("dev-0123456789abcdef", ed25519.NewKeyFromSeed(make([]byte, 32))) + require.NoError(t, err) + manifest := &solutionhost.ModuleManifest{ + Schema: solutionhost.SchemaModuleManifestV1, Binding: "wiki", Alias: "wiki", Kind: solutionhost.KindModule, Generation: 4, + Host: solutionhost.HostTarget{Coordinate: "local", Domain: "local"}, + Release: solutionhost.Release{Publisher: "test", Name: "wiki", Version: "0.1.0"}, + Workloads: []solutionhost.Workload{{Service: "api", Container: "app"}}, + Authority: &solutionhost.Authority{Principal: "wiki", InstallationRole: "installer"}, + } + payload, err := solutionhost.SignedPayload(manifest) + require.NoError(t, err) + envelope, err := solutionhost.SignEnvelope(payload, signer) + require.NoError(t, err) + signed, err := solutionhost.Carrier(payload, envelope) + require.NoError(t, err) + carrier, err := solutionhost.MarshalSigned(signed) + require.NoError(t, err) + declaration, err := proto.Marshal(&compositionv1.AuthorityManifestV1{ + InstallationRole: "installer", + Roles: []*compositionv1.RoleV1{{Name: "installer"}, {Name: "editor"}, {Name: "viewer"}}, + }) + require.NoError(t, err) + document, err := gitops.PreparedPairedDocument(carrier, declaration, signer) + require.NoError(t, err) + + documents := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(documents, "wiki.json"), document, 0o600)) + token := filepath.Join(t.TempDir(), "delivery-token") + require.NoError(t, os.WriteFile(token, []byte("projected-delivery-token\n"), 0o600)) + + for flag, value := range map[string]string{ + "documents": documents, "address": listener.Addr().String(), "identity-file": token, + } { + require.NoError(t, ApplyCmd.Flags().Set(flag, value)) + } + var output bytes.Buffer + ApplyCmd.SetOut(&output) + t.Cleanup(func() { ApplyCmd.SetOut(nil) }) + require.NoError(t, ApplyCmd.RunE(ApplyCmd, nil)) + + require.Len(t, host.seen, 1) + require.NotNil(t, host.seen[0].GetCarrier(), "the Core half") + require.NotNil(t, host.seen[0].GetAuthority(), "and the authority half the legacy route cannot carry") + require.Equal(t, "projected-delivery-token", host.seen[0].GetCredential(), + "raw and trimmed: the projection's trailing newline is not part of the token, and the host adds the bearer scheme") + var policy compositionv1.AuthorityManifestV1 + require.NoError(t, proto.Unmarshal(host.seen[0].GetAuthority().GetDocument(), &policy)) + require.Equal(t, []string{"installer", "editor", "viewer"}, + []string{policy.GetRoles()[0].GetName(), policy.GetRoles()[1].GetName(), policy.GetRoles()[2].GetName()}) + require.Equal(t, "installer", policy.GetInstallationRole()) + require.Contains(t, output.String(), "delivered wiki.json") + require.NotContains(t, output.String(), "projected-delivery-token", "a Job's log is a published record") +} + +// A credential file the projection has not written yet refuses, and nothing is +// sent with an empty credential. +func TestApplyRefusesWhenTheProjectionIsAbsent(t *testing.T) { + host := &qualifyingHost{} + server := grpc.NewServer() + compositionv1.RegisterCompositionServiceServer(server, host) + listener, err := net.Listen("tcp", "127.0.0.1:0") + require.NoError(t, err) + go func() { _ = server.Serve(listener) }() + t.Cleanup(server.Stop) + + documents := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(documents, "one.json"), []byte(`{"carrier":{"document":"e30=","envelope":"e30="}}`), 0o600)) + for flag, value := range map[string]string{ + "documents": documents, "address": listener.Addr().String(), + "identity-file": filepath.Join(t.TempDir(), "not-written-yet"), + } { + require.NoError(t, ApplyCmd.Flags().Set(flag, value)) + } + var output bytes.Buffer + ApplyCmd.SetOut(&output) + t.Cleanup(func() { ApplyCmd.SetOut(nil) }) + err = ApplyCmd.RunE(ApplyCmd, nil) + require.Error(t, err) + require.Empty(t, host.seen) +} + +// readCredential returns the token as the host takes it, with no scheme and no +// surrounding whitespace a projection's trailing newline would carry in. +func TestTheCredentialIsReadRawAndTrimmed(t *testing.T) { + path := filepath.Join(t.TempDir(), "token") + require.NoError(t, os.WriteFile(path, []byte(" eyJhbGciOiJSUzI1NiJ9.payload.signature\n\n"), 0o600)) + credential, err := readCredential(path) + require.NoError(t, err) + require.Equal(t, "eyJhbGciOiJSUzI1NiJ9.payload.signature", credential) + require.False(t, strings.HasPrefix(credential, "Bearer "), "the host adds the scheme itself") + + _, err = readCredential(filepath.Join(t.TempDir(), "absent")) + require.ErrorContains(t, err, "read the projected delivery credential at") +} diff --git a/cmd/delivery/prepare.go b/cmd/delivery/prepare.go new file mode 100644 index 00000000..5454e6ac --- /dev/null +++ b/cmd/delivery/prepare.go @@ -0,0 +1,115 @@ +package delivery + +import ( + "errors" + "fmt" + + "github.com/spf13/cobra" + + "github.com/codefly-dev/cli/cmd/common" + "github.com/codefly-dev/cli/pkg/delivery/signing" + "github.com/codefly-dev/cli/pkg/environments" + "github.com/codefly-dev/cli/pkg/gitops" +) + +var prepareOptions struct { + overlay, out, env string + developer bool +} + +// PrepareCmd writes the paired documents `codefly delivery apply` sends, from +// the carriers a render already settled and the declarations the modules +// themselves hold. +// +// NOT exposed over MCP: it signs with release custody and writes a document +// set a cell then delivers, which is a publishing step rather than something +// to drive conversationally. +var PrepareCmd = &cobra.Command{ + Use: "prepare", Short: "Prepare the paired delivery documents from a rendered delivery overlay", + Long: `Write one prepared paired document per module manifest in a rendered delivery +overlay, for 'codefly delivery apply' to send. + +Each document carries BOTH signed halves: the Core carrier the overlay already +holds — settled at its generation by the publish that wrote it — and the +authority declaration signed against that carrier's exact bytes, read from the +modules themselves: method policies, scope ceilings, roles, owner-awareness, +the installation selector and its person-context edges. Core's own manifest has +no roles field, so this is the only way a module's roles reach a host. + +Nothing is re-rendered and no generation is re-settled: a manifest the overlay +does not carry is not invented here, and a manifest whose carrier is unsigned +is refused by name rather than prepared Core-only — a document that quietly +dropped the authority half would reach the host looking like a success and +compose a module with no roles. + +A withdrawal declares nothing and keeps its Core-only identity. + +The documents carry no credential. 'codefly delivery apply' reads it from the +projection at execution. + +Signing uses release custody (VAULT_ADDR + VAULT_TOKEN + +CODEFLY_RELEASE_TRANSIT_KEY, or CODEFLY_RELEASE_KMS_KEY). Pass --developer to +sign with this machine's developer key instead, which only a local +qualification host admits.`, + Example: ` codefly delivery prepare --overlay deployments/environments/blue/modules/wiki/delivery --out ./prepared --env blue + codefly delivery prepare --overlay ./delivery --out ./prepared --env local --developer`, + Args: cobra.NoArgs, + RunE: func(cmd *cobra.Command, _ []string) error { + ctx, cancel := common.NewContext() + defer cancel() + if prepareOptions.overlay == "" { + return errors.New("--overlay is the rendered delivery overlay holding the module manifest carriers") + } + if prepareOptions.out == "" { + return errors.New("--out is the directory the prepared documents are written to") + } + if prepareOptions.env == "" { + return errors.New("--env names the environment whose host the declarations are read for") + } + workspace, err := common.LoadWorkspace(ctx) + if err != nil { + return err + } + env, err := environments.Select(workspace, prepareOptions.env) + if err != nil { + return err + } + signer, err := prepareSigner() + if err != nil { + return err + } + written, err := gitops.PreparePairedDocuments(ctx, prepareOptions.overlay, prepareOptions.out, + gitops.WorkspaceAuthoritySources(workspace, env), signer) + if err != nil { + return err + } + for _, name := range written { + if _, err = fmt.Fprintf(cmd.OutOrStdout(), "prepared %s\n", name); err != nil { + return err + } + } + return nil + }, +} + +// prepareSigner is release custody, or — asked for explicitly — this machine's +// developer key, which only a local qualification host admits. It is never +// chosen implicitly: a document signed by a developer key and delivered to a +// real host is refused there, far from the cause. +func prepareSigner() (signing.Signer, error) { + if prepareOptions.developer { + return loadKey() + } + signer := signing.FromEnvironment(nil) + if _, err := signer.Sign([]byte("probe")); err != nil { + return nil, fmt.Errorf("%w; pass --developer to sign with this machine's developer key for a local qualification host", err) + } + return signer, nil +} + +func init() { + PrepareCmd.Flags().StringVar(&prepareOptions.overlay, "overlay", "", "Rendered delivery overlay holding the module manifest carriers") + PrepareCmd.Flags().StringVar(&prepareOptions.out, "out", "", "Directory the prepared documents are written to") + PrepareCmd.Flags().StringVar(&prepareOptions.env, "env", "", "Environment whose host the declarations are read for") + PrepareCmd.Flags().BoolVar(&prepareOptions.developer, "developer", false, "Sign with this machine's developer key, for a local qualification host") +} diff --git a/cmd/delivery_test.go b/cmd/delivery_test.go index 0eb1cdf1..2b1eac21 100644 --- a/cmd/delivery_test.go +++ b/cmd/delivery_test.go @@ -23,3 +23,78 @@ func TestDeliveryKeyCommandIsRegisteredAndExplained(t *testing.T) { } } } + +func TestDeliveryApplyCommandIsRegisteredAndExplained(t *testing.T) { + command, args, err := RootCmd.Find([]string{"delivery", "apply"}) + if err != nil || len(args) != 0 || command == nil || command.CommandPath() != "codefly delivery apply" { + t.Fatalf("command missing: %v", err) + } + var output bytes.Buffer + command.SetOut(&output) + t.Cleanup(func() { command.SetOut(nil) }) + if err = command.Help(); err != nil { + t.Fatal(err) + } + // Help must stand alone: what the verb sends, why the credential is not in + // the document, and that only this interface takes the pair. + for _, want := range []string{ + "ApplyManifest", "both signed halves", "NO credential", "read from --identity-file at execution", + "--documents", "--address", "--identity-file", "--budget", + } { + if !strings.Contains(output.String(), want) { + t.Fatalf("help lacks %q", want) + } + } +} + +// Every flag the Job depends on is required, and the refusal says what it is +// for: a Job that ran with one missing would otherwise fail inside the dial or +// read an empty directory. +func TestDeliveryApplyRefusesEachMissingFlagByName(t *testing.T) { + command, _, err := RootCmd.Find([]string{"delivery", "apply"}) + if err != nil { + t.Fatal(err) + } + for _, want := range []string{"--documents", "--address", "--identity-file"} { + t.Run(want, func(t *testing.T) { + for _, flag := range []string{"documents", "address", "identity-file"} { + value := "set" + if "--"+flag == want { + value = "" + } + if err := command.Flags().Set(flag, value); err != nil { + t.Fatal(err) + } + } + var output bytes.Buffer + command.SetOut(&output) + command.SetErr(&output) + t.Cleanup(func() { command.SetOut(nil); command.SetErr(nil) }) + err := command.RunE(command, nil) + if err == nil || !strings.Contains(err.Error(), want) { + t.Fatalf("want a refusal naming %s, got %v", want, err) + } + }) + } +} + +func TestDeliveryPrepareCommandIsRegisteredAndExplained(t *testing.T) { + command, args, err := RootCmd.Find([]string{"delivery", "prepare"}) + if err != nil || len(args) != 0 || command == nil || command.CommandPath() != "codefly delivery prepare" { + t.Fatalf("command missing: %v", err) + } + var output bytes.Buffer + command.SetOut(&output) + t.Cleanup(func() { command.SetOut(nil) }) + if err = command.Help(); err != nil { + t.Fatal(err) + } + for _, want := range []string{ + "BOTH signed halves", "no roles field", "refused by name", "carry no credential", + "--overlay", "--out", "--env", "--developer", + } { + if !strings.Contains(output.String(), want) { + t.Fatalf("help lacks %q", want) + } + } +} diff --git a/cmd/environment/import.go b/cmd/environment/import.go index e14aa6fd..88f0d848 100644 --- a/cmd/environment/import.go +++ b/cmd/environment/import.go @@ -72,6 +72,11 @@ full delivery.intent.yaml is not interpreted.`, Args: cobra.ExactArgs(1), SilenceUsage: true, RunE: func(cmd *cobra.Command, args []string) error { + // First, so an early refusal resets too: cobra keeps parsed flag values + // and their Changed bits on this package-level command, and they would + // otherwise reach the next run in the same process. + defer resetInvocationFlags(cmd) + ctx, done := common.NewContext() defer done() @@ -91,6 +96,17 @@ full delivery.intent.yaml is not interpreted.`, if err != nil { return err } + // Read per invocation, and cleared afterwards. This command object is a + // package-level var, and cobra keeps a parsed flag's value and its + // Changed bit on that object's own FlagSet — so a second Execute in + // the same process inherited the first run's directives and then + // failed the one-shot check against a key the first run had already + // retired. Separate processes were never affected, which is why only + // the public two-Execute path showed it. + retireKeys, err := cmd.Flags().GetStringArray("retire-service-key") + if err != nil { + return err + } var productData []byte if cmd.Flags().Changed("product-intent") { if productPath == "" || productPath == "-" { @@ -115,6 +131,7 @@ full delivery.intent.yaml is not interpreted.`, namespace: importNamespace, namespaceSet: cmd.Flags().Changed("namespace"), dryRun: importDryRun, + retireServiceKeys: retireKeys, // UTC so the provenance timestamp does not vary by the operator's // machine timezone and churn the diff on re-import across a team. now: time.Now().UTC(), @@ -177,10 +194,37 @@ type importOptions struct { namespace string namespaceSet bool dryRun bool + retireServiceKeys []string now time.Time stdout io.Writer } +// retirementInputs parses the operator's --retire-service-key directives and, +// when there are any, loads the workspace graph they resolve against: the +// workspace as it stands on disk, because retirement names an existing +// service, not one this import adds. The graph is loaded under +// inspectionContext: LoadWorkspaceFromDir MIGRATES a legacy flat workspace, +// writing and deleting as it goes, and the host's resolver would clone an +// unmaterialized pin into the module cache. A --dry-run or a refused directive +// must do neither. A retirement needs an existing environment to retire from. +func retirementInputs(ctx context.Context, opts *importOptions, envNode *yaml.Node) ([]retirement, *resources.Workspace, error) { + retirements, err := parseRetirements(opts.retireServiceKeys) + if err != nil { + return nil, nil, err + } + if len(retirements) == 0 { + return nil, nil, nil + } + graphWorkspace, err := resources.LoadWorkspaceFromDir(inspectionContext(ctx), opts.dir) + if err != nil { + return nil, nil, err + } + if envNode == nil { + return nil, nil, fmt.Errorf("--retire-service-key needs an existing environment %q to retire a key from", opts.envName) + } + return retirements, graphWorkspace, nil +} + // runImport parses the contract, merges its owned fields into the named // environment of workspace.codefly.yaml, and either writes the file or, under // --dry-run, prints its unified diff and writes nothing. @@ -234,6 +278,10 @@ func runImport(ctx context.Context, opts *importOptions) error { return fmt.Errorf("environment %q already targets namespace %q, not %q; reconcile its existing identity, secrets and delivery declarations before importing a different target", opts.envName, existing.Value, env.Namespace) } + retirements, graphWorkspace, err := retirementInputs(ctx, opts, envNode) + if err != nil { + return err + } var updated []byte if envNode == nil { // New environment: serialize ToEnvironment's result whole, so a field @@ -256,6 +304,16 @@ func runImport(ctx context.Context, opts *importOptions) error { if intent != nil { withdrawOwnedAbsences(envNode, declaredEnvironment, productOwnedOptional, true) } + // Explicit operator-stated retirements, checked against the merged + // declaration and the workspace graph. Any refusal returns before the + // write below, so a rejected import changes nothing on disk. + notes, retireErr := applyRetirements(ctx, envNode, declaredEnvironment, graphWorkspace, retirements) + if retireErr != nil { + return retireErr + } + for _, note := range notes { + fmt.Fprintln(opts.stdout, note) + } stampProvenance(envNode, contract, opts) updated, err = spliceEnvironment(original, envNode, endLine) } @@ -601,6 +659,21 @@ func stampProvenance(envNode *yaml.Node, contract *environments.CoordinateContra line += " --product-intent …" } + // Retirements are recorded as historical metadata on their own lines, and + // deliberately NOT appended to the re-run hint above: the directive is + // one-shot — it requires the bare key to still be present — so a hint + // carrying it would fail if anyone re-ran it verbatim. + // + // These lines record that an operator asked for the change on a date. They + // are NOT evidence that the key was ever the contract's, and no import + // reads them: nothing here grants a later run permission to retire + // anything on its own. + retired := make([]string, 0, len(opts.retireServiceKeys)) + for _, directive := range opts.retireServiceKeys { + retired = append(retired, fmt.Sprintf("# %s: operator retired %s on %s", + provenanceMarker, strings.TrimSpace(directive), opts.now.Format(time.RFC3339))) + } + kept := make([]string, 0) for _, existing := range strings.Split(envNode.HeadComment, "\n") { if existing == "" || @@ -610,7 +683,7 @@ func stampProvenance(envNode *yaml.Node, contract *environments.CoordinateContra } kept = append(kept, existing) } - envNode.HeadComment = strings.Join(append([]string{line}, kept...), "\n") + envNode.HeadComment = strings.Join(append(append([]string{line}, retired...), kept...), "\n") } func init() { @@ -620,4 +693,7 @@ func init() { _ = importCmd.Flags().MarkDeprecated("cell-contract", "use --coordinate-contract") importCmd.Flags().StringVar(&importNamespace, "namespace", "", "Assert the namespace declared by the producer") importCmd.Flags().BoolVar(&importDryRun, "dry-run", false, "Print the unified diff of workspace.codefly.yaml and write nothing") + importCmd.Flags().StringArray("retire-service-key", nil, + "Retire a bare service key the producer re-keyed, as :=/ (repeatable). "+ + "Blocks: service-config, service-identity, service-mesh. The whole named entry is removed") } diff --git a/cmd/environment/retire_service_key.go b/cmd/environment/retire_service_key.go new file mode 100644 index 00000000..b6e4877c --- /dev/null +++ b/cmd/environment/retire_service_key.go @@ -0,0 +1,281 @@ +package environment + +import ( + "context" + "fmt" + "sort" + "strconv" + "strings" + + "github.com/codefly-dev/cli/pkg/cli/yamledit" + "github.com/codefly-dev/core/resources" + "github.com/spf13/cobra" + "github.com/spf13/pflag" + "gopkg.in/yaml.v3" +) + +// retirableBlocks are the environment blocks whose `services` map may be +// re-keyed by an explicit retirement. +// +// `service-secrets` is deliberately absent and is refused by name. A bare key +// there is not merely a different spelling: RemoteRef changes what is owned and +// what is a credential, so rewriting the key produces a workspace that +// validation then rejects as an unknown service, with the mapping falling back +// to a remote key the consumers never name. Retiring a secrets key needs its +// own answer, not this one. +var retirableBlocks = map[string]bool{"service-config": true, "service-identity": true, "service-mesh": true} + +// retirement is one operator-stated replacement: in this block, this bare key +// is the old spelling of this module-qualified service. +type retirement struct { + block string + bare string + destination string + raw string +} + +// parseRetirements reads the repeatable --retire-service-key directives. +// +// The CLI does not infer that a bare key was ever the contract's. Name suffixes +// and equal values are not evidence of that — a bare key can name a genuinely +// different service whose values happen to match — so the operator states both +// halves and the CLI does exactly what it was told, or refuses. +func parseRetirements(values []string) ([]retirement, error) { + parsed := make([]retirement, 0, len(values)) + seen := map[string]string{} + for _, raw := range values { + block, rest, ok := strings.Cut(strings.TrimSpace(raw), ":") + if !ok { + return nil, fmt.Errorf("--retire-service-key %q is malformed: expected :=/", raw) + } + bare, destination, ok := strings.Cut(rest, "=") + if !ok { + return nil, fmt.Errorf("--retire-service-key %q is malformed: expected :=/", raw) + } + block, bare, destination = strings.TrimSpace(block), strings.TrimSpace(bare), strings.TrimSpace(destination) + switch { + case bare == "" || destination == "": + return nil, fmt.Errorf("--retire-service-key %q is malformed: expected :=/", raw) + case strings.Contains(bare, "/"): + return nil, fmt.Errorf("--retire-service-key %q retires %q, which is already module-qualified; only a bare key is retired", raw, bare) + case !strings.Contains(destination, "/"): + return nil, fmt.Errorf("--retire-service-key %q names destination %q, which is not module-qualified; write it as /", raw, destination) + case block == "service-secrets": + return nil, fmt.Errorf("--retire-service-key %q: service-secrets keys are not retired this way, because a remote reference is not merely a spelling of the service name", raw) + case !retirableBlocks[block]: + return nil, fmt.Errorf("--retire-service-key %q names block %q; supported blocks are %s", raw, block, strings.Join(sortedRetirableBlocks(), ", ")) + } + key := block + ":" + bare + if previous, repeated := seen[key]; repeated { + if previous == destination { + return nil, fmt.Errorf("--retire-service-key names %s %q twice", block, bare) + } + return nil, fmt.Errorf("--retire-service-key names %s %q twice, for %q and %q; one key has one destination", block, bare, previous, destination) + } + seen[key] = destination + parsed = append(parsed, retirement{block: block, bare: bare, destination: destination, raw: raw}) + } + return parsed, nil +} + +func sortedRetirableBlocks() []string { + blocks := make([]string, 0, len(retirableBlocks)) + for block := range retirableBlocks { + blocks = append(blocks, block) + } + sort.Strings(blocks) + return blocks +} + +// applyRetirements removes each stated bare key from the selected environment, +// after checking every directive against the merged declaration and the +// workspace graph. Every refusal happens before the caller writes anything, so +// a rejected import leaves the file as it was. +// +// It removes the WHOLE named entry, not the part that overlaps its +// replacement. EnvironmentServiceConfig.ValuesFor copies the bare entry's +// values and then overlays the qualified entry's, so a bare entry can +// contribute keys the destination does not declare; those keys go with it. The +// returned notes say so, per retirement, naming the keys that will be lost, so +// the operator sees it before the write and in --dry-run. +// +// It changes only the environment being imported. Another environment that +// carries the same bare key keeps it: whether that one is also stale is its +// owner's call, not a consequence of this one. +// +// The directive is one-shot by design. It requires the bare key to still be +// present, so once a run has written the file the same command refuses — a +// retry after a post-import readiness error must omit the flag. +func applyRetirements(ctx context.Context, target, declaration *yaml.Node, workspace *resources.Workspace, retirements []retirement) ([]string, error) { + if len(retirements) == 0 { + return nil, nil + } + // Membership is proved against the real service graph: every module loaded + // and its service references read. A name index built from module + // references alone can report a qualified name the workspace does not + // actually compose, and a destination that does not exist is exactly what + // must be refused here. + composed, err := workspace.LoadServiceWithModules(inspectionContext(ctx)) + if err != nil { + return nil, err + } + notes := make([]string, 0, len(retirements)) + for _, r := range retirements { + if !composes(composed, r.destination) { + return nil, fmt.Errorf("--retire-service-key %q names destination %q, which the workspace composes no service for", r.raw, r.destination) + } + currentBlock := yamledit.MapValue(target, r.block) + currentServices := yamledit.MapValue(currentBlock, "services") + retiring := yamledit.MapValue(currentServices, r.bare) + if currentServices == nil || currentServices.Kind != yaml.MappingNode || retiring == nil { + return nil, fmt.Errorf("--retire-service-key %q: environment %s declares no %s key %q, so there is nothing to retire "+ + "(the directive is one-shot: omit it when re-running after a successful write)", + r.raw, environmentLabel(target), r.block, r.bare) + } + // The contract still naming the bare key means it is current, not + // retired: withdrawing it would delete a key the very next import + // writes back. + declaredServices := yamledit.MapValue(yamledit.MapValue(declaration, r.block), "services") + // The destination has to be what the INCOMING contract declares under + // this block. Retirement is authorized against a replacement that just + // arrived; pointing it at a key the contract does not name would make + // it a plain deletion wearing a migration's clothes. + if yamledit.MapValue(declaredServices, r.destination) == nil { + return nil, fmt.Errorf("--retire-service-key %q: the incoming contract declares no %s key %q, so there is no replacement to retire into", + r.raw, r.block, r.destination) + } + if yamledit.MapValue(declaredServices, r.bare) != nil { + return nil, fmt.Errorf("--retire-service-key %q: the incoming contract still declares %s %q, so it is not a retired spelling", + r.raw, r.block, r.bare) + } + note := fmt.Sprintf("Retiring %s %q into %q: the whole entry is removed.", r.block, r.bare, r.destination) + if lost := valueKeysLost(retiring, yamledit.MapValue(currentServices, r.destination)); len(lost) > 0 { + note += fmt.Sprintf(" %q does not declare %s, so %s no longer set for it.", + r.destination, strings.Join(quoteAll(lost), ", "), plural(len(lost), "that key is", "those keys are")) + } + notes = append(notes, note) + removeMapKey(currentServices, r.bare) + } + return notes, nil +} + +// valueKeysLost names the `values` keys the retiring entry carries that its +// destination does not, sorted. ValuesFor overlays bare then qualified, so +// these are the keys the service stops seeing once the bare entry is gone. +func valueKeysLost(retiring, destination *yaml.Node) []string { + kept := map[string]bool{} + for _, key := range yamledit.MapKeys(yamledit.MapValue(destination, "values")) { + kept[key] = true + } + var lost []string + for _, key := range yamledit.MapKeys(yamledit.MapValue(retiring, "values")) { + if !kept[key] { + lost = append(lost, key) + } + } + sort.Strings(lost) + return lost +} + +func quoteAll(values []string) []string { + quoted := make([]string, 0, len(values)) + for _, value := range values { + quoted = append(quoted, strconv.Quote(value)) + } + return quoted +} + +func plural(n int, one, many string) string { + if n == 1 { + return one + } + return many +} + +// resetInvocationFlags returns this command's flags to their unset state after +// a run. +// +// The command is a package-level var and cobra keeps each parsed flag's value +// and its Changed bit on that object's FlagSet, so a process that runs the +// command twice carries the first run's flags into the second. Separate +// processes are unaffected, which is why only the two-Execute path shows it. +// +// It is not specific to one flag. `--retire-service-key` inherited its +// directives and then failed the one-shot check; `--cell-contract` stayed +// Changed and made a later `--coordinate-contract` run fail "pass only one"; +// `--namespace` would leak an assertion the second run never made, since +// Changed is what distinguishes asserting a namespace from omitting it. Every +// flag here is invocation-scoped, so every one is cleared. +func resetInvocationFlags(cmd *cobra.Command) { + cmd.Flags().VisitAll(func(flag *pflag.Flag) { + if slice, ok := flag.Value.(pflag.SliceValue); ok { + _ = slice.Replace(nil) + } else { + _ = flag.Value.Set(flag.DefValue) + } + flag.Changed = false + }) +} + +// inspectionContext is the context a retirement directive is checked under.// inspectionContext is the context a retirement directive is checked under. It +// adds nothing to the host's own; it takes two things away. +// +// Migration is refused, so a legacy flat module is not decoded, merged, saved +// or removed on the way to answering whether a destination exists. Core's own +// doc is explicit that this bounds workspace files and no more: "hosts remain +// responsible for the effects of any resolver they attach to the context." +// +// So the resolver is replaced too. The host installs one that resolves a +// pinned `workspaces:` reference by cloning it into the CLI-managed module +// cache; a dry-run that acquired a dependency to decide it would refuse is not +// the read-only check this operation promises. Answering with a refusal costs +// the operator one explicit materialization they would have run anyway. +// +// Sources already prepared are unaffected, because they never reach a +// resolver: a module with a local path override, or one already materialized, +// resolves from disk. Only an unmaterialized pinned reference asks — and that +// is exactly the acquisition being declined. +func inspectionContext(ctx context.Context) context.Context { + return resources.WithWorkspaceResolver( + resources.WithWorkspaceMigrationRefusal(ctx), + func(_ context.Context, ref *resources.WorkspaceReference) (string, error) { + name := "a workspace reference" + if ref != nil { + name = strconv.Quote(ref.Name) + } + return "", fmt.Errorf("--retire-service-key checks the destination against the composition, and %s is not materialized here; "+ + "this check acquires nothing, so materialize it first and import again", name) + }, + ) +} + +// composes reports whether the workspace actually composes the qualified// composes reports whether the workspace actually composes the qualified +// / destination. +func composes(services []*resources.ServiceWithModule, destination string) bool { + module, service, qualified := strings.Cut(destination, "/") + if !qualified { + return false + } + for _, found := range services { + if found != nil && found.Module == module && found.Name == service { + return true + } + } + return false +} + +func removeMapKey(mapping *yaml.Node, key string) { + for i := 0; i+1 < len(mapping.Content); i += 2 { + if mapping.Content[i].Value == key { + mapping.Content = append(mapping.Content[:i], mapping.Content[i+2:]...) + return + } + } +} + +func environmentLabel(envNode *yaml.Node) string { + if name := yamledit.MapValue(envNode, "name"); name != nil { + return strconv.Quote(name.Value) + } + return "the selected environment" +} diff --git a/cmd/environment/retire_service_key_test.go b/cmd/environment/retire_service_key_test.go new file mode 100644 index 00000000..baafa058 --- /dev/null +++ b/cmd/environment/retire_service_key_test.go @@ -0,0 +1,453 @@ +package environment + +import ( + "context" + "io" + "os" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/codefly-dev/core/resources" +) + +// retireWorkspace writes a workspace that composes two services named +// object-storage, with `azure` carrying both the bare key and the qualified one +// the producer re-keyed to — the shape an import leaves behind, because it +// merges by key and removes nothing. +func retireWorkspace(t *testing.T, bareValues string) string { + t.Helper() + dir := t.TempDir() + write := func(rel, content string) { + full := filepath.Join(dir, rel) + if err := os.MkdirAll(filepath.Dir(full), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(full, []byte(content), 0o600); err != nil { + t.Fatal(err) + } + } + write(resources.WorkspaceConfigurationName, `name: acme +layout: modules +modules: + - name: documents + - name: depot +environments: + - name: azure + namespace: acme + service-config: + services: + object-storage: +`+bareValues+` + documents/object-storage: + values: + BUCKET: shared +`) + service := func(name string) string { + return "kind: service\nname: " + name + "\nversion: 0.0.0\n" + + "agent:\n kind: runtime::service\n name: go-grpc\n version: 0.0.1\n publisher: codefly.ai\n" + } + write(filepath.Join("modules", "documents", resources.ModuleConfigurationName), "kind: module\nname: documents\nservices:\n - name: object-storage\n") + write(filepath.Join("modules", "documents", "services", "object-storage", resources.ServiceConfigurationName), service("object-storage")) + write(filepath.Join("modules", "depot", resources.ModuleConfigurationName), "kind: module\nname: depot\nservices:\n - name: object-storage\n") + write(filepath.Join("modules", "depot", "services", "object-storage", resources.ServiceConfigurationName), service("object-storage")) + return dir +} + +const retireContract = `{"schema":"codefly/coordinate/v1","coordinate":"hosted-eastus2","environment":{ + "name":"azure","namespace":"acme", + "service-config":{"services":{"documents/object-storage":{"values":{"BUCKET":"shared"}}}} +}}` + +func retireImport(t *testing.T, dir string, directives []string, dryRun bool) (string, error) { + t.Helper() + var out strings.Builder + err := runImport(context.Background(), &importOptions{ + dir: dir, envName: "azure", contractData: []byte(retireContract), + retireServiceKeys: directives, dryRun: dryRun, stdout: &out, + now: time.Date(2026, 10, 29, 12, 0, 0, 0, time.UTC), + }) + return out.String(), err +} + +const bareShared = ` values: + BUCKET: shared` + +// The stated retirement removes the bare key and keeps the qualified one. +func TestRetireRemovesTheStatedBareKey(t *testing.T) { + dir := retireWorkspace(t, bareShared) + out, err := retireImport(t, dir, []string{"service-config:object-storage=documents/object-storage"}, false) + if err != nil { + t.Fatalf("runImport: %v", err) + } + if !strings.Contains(out, "the whole entry is removed") { + t.Errorf("stdout does not say the whole entry goes:\n%s", out) + } + env := loadWorkspace(t, dir).FindEnvironment("azure") + if _, stale := env.ServiceConfig.Services["object-storage"]; stale { + t.Error("the bare key survived an explicit retirement") + } + if _, ok := env.ServiceConfig.Services["documents/object-storage"]; !ok { + t.Error("the destination was lost") + } +} + +// The WHOLE entry goes, including values the destination does not declare — +// ValuesFor overlays bare then qualified, so those keys stop being set. The +// run says which ones before writing. +func TestRetireNamesTheValueKeysItRemoves(t *testing.T) { + dir := retireWorkspace(t, ` values: + BUCKET: shared + LEGACY_REGION: eastus2`) + out, err := retireImport(t, dir, []string{"service-config:object-storage=documents/object-storage"}, false) + if err != nil { + t.Fatalf("runImport: %v", err) + } + if !strings.Contains(out, "LEGACY_REGION") || !strings.Contains(out, "no longer set") { + t.Errorf("stdout does not name the value key that is lost:\n%s", out) + } + env := loadWorkspace(t, dir).FindEnvironment("azure") + if values := env.ServiceConfig.ValuesFor("documents", "object-storage"); values["LEGACY_REGION"] != "" { + t.Errorf("LEGACY_REGION still resolves: %v", values) + } +} + +// --dry-run writes nothing, directives included. +func TestRetireDryRunWritesNothing(t *testing.T) { + dir := retireWorkspace(t, bareShared) + file := filepath.Join(dir, resources.WorkspaceConfigurationName) + before := readFile(t, file) + if _, err := retireImport(t, dir, []string{"service-config:object-storage=documents/object-storage"}, true); err != nil { + t.Fatalf("runImport: %v", err) + } + if readFile(t, file) != before { + t.Error("--dry-run wrote the workspace") + } +} + +// Every refusal happens before the write: the file is byte-identical after. +func TestRetireRefusesBeforeWriting(t *testing.T) { + for name, directive := range map[string]string{ + "malformed, no block": "object-storage=documents/object-storage", + "malformed, no destination": "service-config:object-storage", + "empty destination": "service-config:object-storage=", + "bare is qualified": "service-config:saas/object-storage=documents/object-storage", + "destination is bare": "service-config:object-storage=object-storage", + "unsupported block": "service-secrets:object-storage=documents/object-storage", + "unknown block": "ingress:object-storage=documents/object-storage", + "unknown destination": "service-config:object-storage=nowhere/object-storage", + "destination not in contract": "service-config:object-storage=depot/object-storage", + "nothing to retire": "service-config:absent=documents/object-storage", + } { + t.Run(name, func(t *testing.T) { + dir := retireWorkspace(t, bareShared) + file := filepath.Join(dir, resources.WorkspaceConfigurationName) + before := readFile(t, file) + if _, err := retireImport(t, dir, []string{directive}, false); err == nil { + t.Fatal("expected a refusal") + } + if readFile(t, file) != before { + t.Error("the workspace was modified by a refused import") + } + }) + } +} + +// A legacy flat workspace must not be MIGRATED by the inspection a refused +// directive performs. Core's default loader saves workspace data and deletes +// the old module file on such a tree (postLoad), so the destination check runs +// under WithWorkspaceMigrationRefusal: a refusal leaves every file exactly as +// it was rather than half-migrating a workspace nobody asked to change. +func TestRetireRefusalDoesNotMigrateALegacyWorkspace(t *testing.T) { + dir := t.TempDir() + write := func(rel, content string) { + full := filepath.Join(dir, rel) + if err := os.MkdirAll(filepath.Dir(full), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(full, []byte(content), 0o600); err != nil { + t.Fatal(err) + } + } + // A FLAT workspace is what reaches the migration branch: Core rewrites the + // module list and deletes the legacy module file (postLoad). + write(resources.WorkspaceConfigurationName, `name: acme +layout: flat +environments: + - name: azure + namespace: acme + service-config: + services: + object-storage: + values: + BUCKET: shared + documents/object-storage: + values: + BUCKET: shared +`) + write(resources.ModuleConfigurationName, "kind: module\nname: acme\nservices:\n - name: object-storage\n") + write(filepath.Join("services", "object-storage", resources.ServiceConfigurationName), + "kind: service\nname: object-storage\nversion: 0.0.0\n"+ + "agent:\n kind: runtime::service\n name: go-grpc\n version: 0.0.1\n publisher: codefly.ai\n") + + snapshot := func() map[string]string { + t.Helper() + files := map[string]string{} + if err := filepath.WalkDir(dir, func(path string, entry os.DirEntry, err error) error { + if err != nil || entry.IsDir() { + return err + } + files[path] = readFile(t, path) + return nil + }); err != nil { + t.Fatal(err) + } + return files + } + before := snapshot() + + if _, err := retireImport(t, dir, []string{"service-config:object-storage=documents/object-storage"}, false); err == nil { + t.Fatal("expected a refusal: the workspace has a legacy module pending migration") + } + + after := snapshot() + for path, content := range before { + got, present := after[path] + switch { + case !present: + t.Errorf("%s was deleted by a refused import", path) + case got != content: + t.Errorf("%s was rewritten by a refused import", path) + } + } + for path := range after { + if _, present := before[path]; !present { + t.Errorf("%s was created by a refused import", path) + } + } +} + +// A bare key the INCOMING contract still declares is current, not retired: +// withdrawing it would delete what the very next import writes back. +func TestRetireRefusesAKeyTheContractStillDeclares(t *testing.T) { + dir := retireWorkspace(t, bareShared) + contract := `{"schema":"codefly/coordinate/v1","coordinate":"hosted-eastus2","environment":{ + "name":"azure","namespace":"acme", + "service-config":{"services":{ + "object-storage":{"values":{"BUCKET":"shared"}}, + "documents/object-storage":{"values":{"BUCKET":"shared"}}}} + }}` + before := readFile(t, filepath.Join(dir, resources.WorkspaceConfigurationName)) + var out strings.Builder + err := runImport(context.Background(), &importOptions{ + dir: dir, envName: "azure", contractData: []byte(contract), + retireServiceKeys: []string{"service-config:object-storage=documents/object-storage"}, + stdout: &out, now: time.Date(2026, 10, 29, 12, 0, 0, 0, time.UTC), + }) + if err == nil || !strings.Contains(err.Error(), "still declares") { + t.Fatalf("expected a refusal naming the contract, got %v", err) + } + if readFile(t, filepath.Join(dir, resources.WorkspaceConfigurationName)) != before { + t.Error("the workspace was modified by a refused import") + } +} + +// Duplicate and conflicting directives for one key are refused. +func TestRetireRefusesDuplicateDirectives(t *testing.T) { + for name, directives := range map[string][]string{ + "same destination twice": { + "service-config:object-storage=documents/object-storage", + "service-config:object-storage=documents/object-storage", + }, + "conflicting destinations": { + "service-config:object-storage=documents/object-storage", + "service-config:object-storage=depot/object-storage", + }, + } { + t.Run(name, func(t *testing.T) { + dir := retireWorkspace(t, bareShared) + before := readFile(t, filepath.Join(dir, resources.WorkspaceConfigurationName)) + if _, err := retireImport(t, dir, directives, false); err == nil || !strings.Contains(err.Error(), "twice") { + t.Fatalf("expected a duplicate refusal, got %v", err) + } + if readFile(t, filepath.Join(dir, resources.WorkspaceConfigurationName)) != before { + t.Error("the workspace was modified by a refused import") + } + }) + } +} + +// Unrelated keys survive, whatever their values: retirement removes only what +// was named, and never compares values to decide. +func TestRetireLeavesUnrelatedKeysAlone(t *testing.T) { + dir := retireWorkspace(t, bareShared) + file := filepath.Join(dir, resources.WorkspaceConfigurationName) + content := readFile(t, file) + // depot/object-storage holds the SAME values as the destination, and + // ledger holds different ones. Neither is named, so both must survive. + content = strings.Replace(content, " documents/object-storage:", + " depot/object-storage:\n values:\n BUCKET: shared\n"+ + " ledger:\n values:\n MODE: local\n"+ + " documents/object-storage:", 1) + if err := os.WriteFile(file, []byte(content), 0o600); err != nil { + t.Fatal(err) + } + if _, err := retireImport(t, dir, []string{"service-config:object-storage=documents/object-storage"}, false); err != nil { + t.Fatalf("runImport: %v", err) + } + services := loadWorkspace(t, dir).FindEnvironment("azure").ServiceConfig.Services + for _, key := range []string{"depot/object-storage", "ledger", "documents/object-storage"} { + if _, ok := services[key]; !ok { + t.Errorf("unrelated key %q was removed: %+v", key, services) + } + } + if _, stale := services["object-storage"]; stale { + t.Error("the named key survived") + } +} + +// Without directives the importer behaves exactly as before: the bare key is +// preserved, which is the behaviour on main. +func TestImportWithoutDirectivesStillPreservesTheBareKey(t *testing.T) { + dir := retireWorkspace(t, bareShared) + if _, err := retireImport(t, dir, nil, false); err != nil { + t.Fatalf("runImport: %v", err) + } + if _, ok := loadWorkspace(t, dir).FindEnvironment("azure").ServiceConfig.Services["object-storage"]; !ok { + t.Error("an import with no directives removed the bare key") + } +} + +// The directive is one-shot: after a successful write the same command refuses, +// because the key it names is gone. A retry must omit the flag. +func TestRetireIsOneShot(t *testing.T) { + dir := retireWorkspace(t, bareShared) + directive := []string{"service-config:object-storage=documents/object-storage"} + if _, err := retireImport(t, dir, directive, false); err != nil { + t.Fatalf("first import: %v", err) + } + _, err := retireImport(t, dir, directive, false) + if err == nil || !strings.Contains(err.Error(), "one-shot") { + t.Fatalf("expected a one-shot refusal naming the retry, got %v", err) + } + if _, err := retireImport(t, dir, nil, false); err != nil { + t.Fatalf("retry without the flag must succeed: %v", err) + } +} + +// TestRetireFlagDoesNotLeakBetweenExecutions is the reviewer's public-surface +// finding: the flag was bound to a package-level slice, so a second +// Cmd.Execute in the SAME process still carried the first run's directives and +// failed the one-shot check against a key the first run had already retired. +// Separate processes were never affected, which is why the command tests +// missed it. +func TestRetireFlagDoesNotLeakBetweenExecutions(t *testing.T) { + dir := retireWorkspace(t, bareShared) + contract := filepath.Join(t.TempDir(), "contract.json") + if err := os.WriteFile(contract, []byte(retireContract), 0o600); err != nil { + t.Fatal(err) + } + t.Chdir(dir) + // The same package-level command object both times, which is what a single + // process does. + run := func(args ...string) error { + Cmd.SetOut(io.Discard) + Cmd.SetErr(io.Discard) + Cmd.SetArgs(args) + return Cmd.Execute() + } + base := []string{"import", "azure", "--coordinate-contract", contract} + + if err := run(append(base, "--retire-service-key", "service-config:object-storage=documents/object-storage")...); err != nil { + t.Fatalf("first execute: %v", err) + } + // Second execute WITHOUT the flag. It must not inherit the directive. + if err := run(base...); err != nil { + t.Fatalf("second execute without the flag inherited the first run's directive: %v", err) + } +} + +// TestRetireRefusesToAcquireAnUnmaterializedReference: the destination check +// reads the composition, and the host's resolver would clone an unmaterialized +// pinned reference into the module cache to answer. A check that acquires a +// dependency in order to decide it would refuse is not read-only, so the +// resolver is replaced for the inspection and the directive refuses instead. +func TestRetireRefusesToAcquireAnUnmaterializedReference(t *testing.T) { + dir := t.TempDir() + write := func(rel, content string) { + full := filepath.Join(dir, rel) + if err := os.MkdirAll(filepath.Dir(full), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(full, []byte(content), 0o600); err != nil { + t.Fatal(err) + } + } + write(resources.WorkspaceConfigurationName, `name: acme +layout: modules +workspaces: + - name: upstream + source: obin-ai/not-materialized + version: 0.1.0 +modules: + - name: documents +environments: + - name: azure + namespace: acme + service-config: + services: + object-storage: + values: + BUCKET: shared + documents/object-storage: + values: + BUCKET: shared +`) + write(filepath.Join("modules", "documents", resources.ModuleConfigurationName), "kind: module\nname: documents\nservices:\n - name: object-storage\n") + write(filepath.Join("modules", "documents", "services", "object-storage", resources.ServiceConfigurationName), + "kind: service\nname: object-storage\nversion: 0.0.0\n"+ + "agent:\n kind: runtime::service\n name: go-grpc\n version: 0.0.1\n publisher: codefly.ai\n") + + before := readFile(t, filepath.Join(dir, resources.WorkspaceConfigurationName)) + _, err := retireImport(t, dir, []string{"service-config:object-storage=documents/object-storage"}, true) + if err == nil { + t.Fatal("expected a refusal rather than an acquisition") + } + if !strings.Contains(err.Error(), "acquires nothing") && !strings.Contains(err.Error(), "not materialized") { + t.Errorf("refusal does not explain the unmaterialized reference: %v", err) + } + if readFile(t, filepath.Join(dir, resources.WorkspaceConfigurationName)) != before { + t.Error("the workspace was modified by a refused import") + } +} + +// Provenance records the action as metadata, and does NOT put the one-shot +// directive into the re-run hint, which would fail if anyone re-ran it. +func TestRetireProvenanceKeepsTheRerunHintReusable(t *testing.T) { + dir := retireWorkspace(t, bareShared) + if _, err := retireImport(t, dir, []string{"service-config:object-storage=documents/object-storage"}, false); err != nil { + t.Fatalf("runImport: %v", err) + } + content := readFile(t, filepath.Join(dir, resources.WorkspaceConfigurationName)) + var hint, record string + for _, line := range strings.Split(content, "\n") { + switch { + case strings.Contains(line, "re-run:"): + hint = line + case strings.Contains(line, "operator retired"): + record = line + } + } + if record == "" { + t.Error("the retirement was not recorded in provenance") + } + if hint == "" { + t.Fatal("the re-run hint is missing") + } + if strings.Contains(hint, "retire-service-key") { + t.Errorf("the one-shot directive was baked into the re-run hint: %s", hint) + } +} + +var _ = io.Discard diff --git a/cmd/run/launcher.go b/cmd/run/launcher.go index 905347bc..4dc9985b 100644 --- a/cmd/run/launcher.go +++ b/cmd/run/launcher.go @@ -383,7 +383,7 @@ func (cluster *launcher) runtimeOverrides(service *resources.Service) map[string if ok { // A module-identity service reads its token from the path a cell's // render mounts unless told otherwise; this is the otherwise. - out[workloadTokenFileVariable] = cluster.vouch.ProjectionPath(identity.Workload(cluster.env.Host.TrustDomain)) + out[gitops.WorkloadTokenFileVariable] = cluster.vouch.ProjectionPath(identity.Workload(cluster.env.Host.TrustDomain)) } if unique == resources.ServiceUnique(cluster.hostModule, cluster.hostService) { out[nativeidentity.AnchorEnvironment] = cluster.hostIdentityAnchor @@ -643,6 +643,13 @@ func (cluster *launcher) newCompositionDelivery(conn grpc.ClientConnInterface) * return false, err } return gitops.OwnerAware(dir) + }, + func(ctx context.Context, module string) (string, []*compositionv1.EdgeDependencyV1, error) { + dir, err := load(ctx, module) + if err != nil { + return "", nil, err + } + return gitops.InstallationDeclaration(dir) }) } @@ -801,10 +808,6 @@ func (cluster *launcher) forget(unique string) { cluster.mu.Unlock() } -// workloadTokenFileVariable names the projected token's path to a process -// that cannot read it from the path a pod mounts it at. -const workloadTokenFileVariable = "CODEFLY__WORKLOAD_TOKEN_FILE" //nolint:gosec // G101: the name of a path variable, not a credential - // delivers reports whether this run delivers a manifest for an identity's // module: every module but the host's own. func (cluster *launcher) delivers(identity gitops.NativeIdentity) bool { diff --git a/cmd/run/launcher_admission_test.go b/cmd/run/launcher_admission_test.go index 536b476d..90170dba 100644 --- a/cmd/run/launcher_admission_test.go +++ b/cmd/run/launcher_admission_test.go @@ -328,7 +328,7 @@ func TestRuntimeOverridesDoNotIssueOrRestoreCredentials(t *testing.T) { cluster, _ := admissionLauncher(t) service, err := cluster.composed().LoadService(t.Context(), &resources.ServiceWithModule{Module: "alpha", Name: "api"}) require.NoError(t, err) - path := cluster.runtimeOverrides(service)[workloadTokenFileVariable] + path := cluster.runtimeOverrides(service)[gitops.WorkloadTokenFileVariable] require.NotEmpty(t, path) require.NoFileExists(t, path) _, projected := cluster.vouch.Projected("alpha/api") @@ -337,7 +337,7 @@ func TestRuntimeOverridesDoNotIssueOrRestoreCredentials(t *testing.T) { require.NoError(t, err) require.FileExists(t, path) attempt.Complete(false) - require.Equal(t, path, cluster.runtimeOverrides(service)[workloadTokenFileVariable]) + require.Equal(t, path, cluster.runtimeOverrides(service)[gitops.WorkloadTokenFileVariable]) _, projected = cluster.vouch.Projected("alpha/api") require.False(t, projected, "late environment preparation cannot restore refresh membership") } @@ -360,7 +360,7 @@ func (p *admittedRuntimePeer) Start(_ context.Context, req *runtimev0.StartReque if len(p.composition.snapshots()) < 2 { return nil, fmt.Errorf("Start preceded selected declarations") } - if _, err := os.Stat(req.GetOverrides()[workloadTokenFileVariable]); err != nil { + if _, err := os.Stat(req.GetOverrides()[gitops.WorkloadTokenFileVariable]); err != nil { return nil, err } p.starts.Add(1) diff --git a/cmd/run/native_delivery_rpc_cancellation_test.go b/cmd/run/native_delivery_rpc_cancellation_test.go index ec1ebc69..438dbe07 100644 --- a/cmd/run/native_delivery_rpc_cancellation_test.go +++ b/cmd/run/native_delivery_rpc_cancellation_test.go @@ -53,7 +53,7 @@ func TestNativeDeliveryCancelsTheGeneratedCompositionRPC(t *testing.T) { require.NoError(t, err) signer, err := solutionhost.NewInMemorySigner("delivery-test", private) require.NoError(t, err) - composition := gitops.NewCompositionDelivery(conn, signer, nil, nil, nil, nil) + composition := gitops.NewCompositionDelivery(conn, signer, nil, nil, nil, nil, nil) native := &gitops.NativeDeliverer{Signer: signer, StateDir: t.TempDir(), Prepare: composition.Prepare, Token: func(string) (string, error) { return "test-carrier", nil }} manifest := &solutionhost.ModuleManifest{ diff --git a/cmd/run/native_delivery_schedule_test.go b/cmd/run/native_delivery_schedule_test.go index d973b16f..fa83efa4 100644 --- a/cmd/run/native_delivery_schedule_test.go +++ b/cmd/run/native_delivery_schedule_test.go @@ -141,7 +141,7 @@ func deliverTo(t *testing.T, target orchestration.GRPCTarget) func(context.Conte conn, err := target.Dial() require.NoError(t, err) defer func() { _ = conn.Close() }() - composition := gitops.NewCompositionDelivery(conn, signer, nil, nil, nil, nil) + composition := gitops.NewCompositionDelivery(conn, signer, nil, nil, nil, nil, nil) deliverer := &gitops.NativeDeliverer{Signer: signer, StateDir: t.TempDir(), Prepare: composition.Prepare, Token: func(string) (string, error) { return "token", nil }} _, err = deliverer.Deliver(ctx, &solutionhost.ModuleManifest{ Schema: solutionhost.SchemaModuleManifestV1, Binding: "acme.local.model", Alias: "model", Kind: solutionhost.KindModule, diff --git a/docs/cli-reference.md b/docs/cli-reference.md index 5c29d48d..44e5283f 100644 --- a/docs/cli-reference.md +++ b/docs/cli-reference.md @@ -2043,10 +2043,10 @@ codefly delete service [flags] ## `codefly delivery` -Manage delivery signing keys +Manage delivery signing keys and send prepared delivery documents ``` -Manage the local keys used to sign delivery documents for a local cluster. +Manage the local keys used to sign delivery documents for a local cluster, and send the prepared paired documents a render wrote to the host's composition interface. ``` ``` @@ -2055,7 +2055,68 @@ codefly delivery Subcommands: +- [`codefly delivery apply`](#codefly-delivery-apply) - [`codefly delivery key`](#codefly-delivery-key) +- [`codefly delivery prepare`](#codefly-delivery-prepare) + +## `codefly delivery apply` + +Send prepared paired delivery documents to the host's composition interface + +``` +Send the prepared delivery documents under --documents to the host's composition +interface (host.composition.v1.CompositionService/ApplyManifest) at --address. + +Each document is one ApplyManifestRequest carrying both signed halves — core's +module manifest in its DSSE carrier, and the authority declaration signed +against that carrier's exact bytes: method policies, scope ceilings, roles, +owner-awareness, the installation selector and its person-context edges. Only +this interface takes the pair. The host's legacy HTTP manifest route takes the +Core half alone, so a module delivered that way reaches the host with no roles. + +A prepared document carries NO credential: a delivery credential is a +projected ServiceAccount token with a lifetime of minutes, and one written into +a rendered manifest would be a published secret and expired by the time the +manifest synced. The credential is read from --identity-file at execution, +once per request, so a token rotated between rounds is the one the next round +sends. It is sent RAW, in the request's credential field, because the host adds +the bearer scheme itself. It is never printed and never passed as an argument. + +Documents are sent in ROUNDS: every pending one once per round, then a pause +that doubles up to --max-pause, until each is answered or --budget is spent, so +a document the host keeps failing never keeps a later one — a withdrawal among +them — from its first attempt. A refusal the host means (an invalid document, a +refused carrier, a stale or rewritten generation, a withdrawal it keeps +terminal) refuses that document and is not retried; only "not serving yet" +(unavailable, out of time, out of capacity) is. The command fails when any +document was not accepted, so a delivery that did not land fails the Job that +ran it. + +The address is the host's delivery endpoint. A Connect endpoint is one: a +Connect server answers the gRPC protocol on the same listener (h2c in +plaintext), so --address is : of the endpoint the environment's +host.delivery names, and --secure only when that endpoint serves TLS itself. + +A GitOps publish toward a host whose delivery endpoint declares +codefly.dev/composition writes these documents into the delivery overlay, one +per manifest, and renders the manifest's delivery Job to run this command. +``` + +``` +codefly delivery apply [flags] +``` + +Flags: + +``` + --address string : of the host's delivery endpoint + --budget duration Bound on the whole run (default 25m0s) + --documents string Directory holding the prepared delivery documents (*.json) + --identity-file string Path of the projected delivery credential, read at execution + --max-pause duration Longest pause between rounds (default 1m0s) + --request-timeout duration Bound on one call (default 30s) + --secure Dial with TLS, for a delivery endpoint that serves it itself +``` ## `codefly delivery key` @@ -2069,6 +2130,51 @@ Create or load the machine's developer signing key and print its key ID and publ codefly delivery key [flags] ``` +## `codefly delivery prepare` + +Prepare the paired delivery documents from a rendered delivery overlay + +``` +Write one prepared paired document per module manifest in a rendered delivery +overlay, for 'codefly delivery apply' to send. + +Each document carries BOTH signed halves: the Core carrier the overlay already +holds — settled at its generation by the publish that wrote it — and the +authority declaration signed against that carrier's exact bytes, read from the +modules themselves: method policies, scope ceilings, roles, owner-awareness, +the installation selector and its person-context edges. Core's own manifest has +no roles field, so this is the only way a module's roles reach a host. + +Nothing is re-rendered and no generation is re-settled: a manifest the overlay +does not carry is not invented here, and a manifest whose carrier is unsigned +is refused by name rather than prepared Core-only — a document that quietly +dropped the authority half would reach the host looking like a success and +compose a module with no roles. + +A withdrawal declares nothing and keeps its Core-only identity. + +The documents carry no credential. 'codefly delivery apply' reads it from the +projection at execution. + +Signing uses release custody (VAULT_ADDR + VAULT_TOKEN + +CODEFLY_RELEASE_TRANSIT_KEY, or CODEFLY_RELEASE_KMS_KEY). Pass --developer to +sign with this machine's developer key instead, which only a local +qualification host admits. +``` + +``` +codefly delivery prepare [flags] +``` + +Flags: + +``` + --developer Sign with this machine's developer key, for a local qualification host + --env string Environment whose host the declarations are read for + --out string Directory the prepared documents are written to + --overlay string Rendered delivery overlay holding the module manifest carriers +``` + ## `codefly deploy` Deploy a service or module to a configured environment @@ -2734,10 +2840,11 @@ codefly environment import --coordinate-contract Flags: ``` - --coordinate-contract string Path to a codefly/coordinate/v1 descriptor, or - for stdin - --dry-run Print the unified diff of workspace.codefly.yaml and write nothing - --namespace string Assert the namespace declared by the producer - --product-intent string Authored product decisions: environments..exclude-services and service-secrets generate / may-be-empty + --coordinate-contract string Path to a codefly/coordinate/v1 descriptor, or - for stdin + --dry-run Print the unified diff of workspace.codefly.yaml and write nothing + --namespace string Assert the namespace declared by the producer + --product-intent string Authored product decisions: environments..exclude-services and service-secrets generate / may-be-empty + --retire-service-key stringArray Retire a bare service key the producer re-keyed, as :=/ (repeatable). Blocks: service-config, service-identity, service-mesh. The whole named entry is removed ``` ## `codefly environment show` diff --git a/docs/commands.md b/docs/commands.md index e5218cec..7011ccc7 100644 --- a/docs/commands.md +++ b/docs/commands.md @@ -121,6 +121,51 @@ never `--stand-alone` or `--exclude-root`; `--exclude-dependency` and out. The selected environment's `exclude-services` also removes services from the composition roots; a module withdrawn whole contributes no root, including its service-entry. CLI and run-profile exclusions still apply on top. + +**An operation is held to the environment it selected, never refused because a +DIFFERENT environment's `exclude-services` is inconsistent with the workspace +graph.** Run, build, deploy and `deploy gitops render` each check the one +environment they were given against the graph, and a refusal names that +environment in the message itself rather than only under `--debug`. + +This is scoped to the graph-consistency check, which is where the +cross-environment refusal came from. It is not a general rule that other +environments are ignored. The selection covers every check that judges an +environment's declarations against the workspace's current service graph: +`exclude-services`, the service-scoped names (`service-config`, +`service-identity`, `service-mesh`, `service-secrets`) and +`managed-services`. Those are narrowed together because they fail the same +way — a bare `store` key becomes ambiguous the moment a second module +composes a service of that name, and the environment that wrote the key may +not be the one being rendered. + +The graph itself is still the whole workspace's. A name is ambiguous because +of what the workspace composes; the selection decides whose declarations are +judged against it, never what it contains. + +Two things still hold every environment, deliberately. The check decodes and +validates EVERY environment's declarations before comparing the selected one +against the graph, so a **malformed** declaration anywhere — an unqualified +name, a duplicate, a service withdrawn from itself — still refuses, whatever +was selected; that is a workspace that does not say what it means, not a +disagreement with the graph. (Loading does not refuse: such a workspace still +loads and a clean environment still selects. It is the validation that +refuses.) Module namespaces and host composition are likewise whole-workspace, +being invariants of the workspace rather than of a selection. +This matters because the exclusion graph is the workspace's **current** +composition: an environment imported before a module overlay added a service +carries no `withdrawn-from` for it, so holding every environment to that graph +refused a correct render of a different environment — and the operator could +not satisfy it, since re-importing the offending environment requires the +product intent to declare it. + +Every CLI path that checks exclusions against the graph selects an +environment: run, build, test and deploy through the flow, `deploy gitops +render` and the solution and dev renders through the workspace validation, and +`codefly doctor workspace` through its own scope check — doctor selects and +calls the selected check directly, rather than relying on the fallback. The +whole-workspace form, which holds every environment's graph at once, remains +the behaviour for a caller that has selected none. The deterministic port hash folds every endpoint into a few thousand buckets, so a `--naming-scope` can hash two endpoints to one port and abort the run; `--temporary-ports` sidesteps the hash, under the ordering its @@ -742,6 +787,91 @@ machine key is returned on later calls. codefly delivery key ``` +### `codefly delivery prepare` + +Write the paired documents `codefly delivery apply` sends, from the carriers a +render already settled and the declarations the modules themselves hold. + +Each document carries both signed halves: the Core carrier the overlay holds — +settled at its generation by the publish that wrote it — and the authority +declaration signed against that carrier's exact bytes, read from the modules: +method policies, scope ceilings, roles, owner-awareness, the installation +selector and its person-context edges. Core's own manifest has **no roles +field**, so this is the only way a module's roles reach a host. + +A GitOps publish toward a host that declares `codefly.dev/composition` writes +these same documents into the overlay itself; `prepare` is for sending a pair +from outside the Job. + +Nothing is re-rendered and no generation is re-settled. A manifest the overlay +does not carry is not invented here, and a manifest whose carrier is unsigned +is **refused by name** rather than prepared Core-only — a document that quietly +dropped the authority half would reach the host looking like a success and +compose the module with no roles. A withdrawal declares nothing and keeps its +Core-only identity. + +Signing uses release custody (`VAULT_ADDR` + `VAULT_TOKEN` + +`CODEFLY_RELEASE_TRANSIT_KEY`, or `CODEFLY_RELEASE_KMS_KEY`). `--developer` +signs with this machine's developer key instead, which only a local +qualification host admits; it is never chosen implicitly, because a +developer-signed document delivered to a real host is refused there, far from +the cause. + +```bash +codefly delivery prepare --overlay deployments/environments/blue/modules/wiki/delivery \ + --out ./prepared --env blue +codefly delivery prepare --overlay ./delivery --out ./prepared --env local --developer +``` + +### `codefly delivery apply` + +Send the prepared paired delivery documents a render wrote to the host's +composition interface — `host.composition.v1.CompositionService/ApplyManifest`. +This is what a cell's delivery Job runs: a GitOps publish toward a host whose +delivery endpoint declares `codefly.dev/composition` writes one paired document +per manifest into the delivery overlay and renders the manifest Job to run +`/bin/codefly delivery apply` with the projected token, from the codefly +companion `0.0.7`, the first tag whose static binary carries this verb. + +Each document is one `ApplyManifestRequest` carrying **both** signed halves: +core's module manifest in its DSSE carrier, and the authority declaration +signed against that carrier's exact bytes — method policies, scope ceilings, +roles, owner-awareness, the installation selector and its person-context edges. +Only this interface takes the pair. The host's legacy HTTP manifest route takes +the Core half alone, so a module delivered that way reaches the host with no +roles at all. + +A prepared document carries **no credential**. A delivery credential is a +projected ServiceAccount token with a lifetime of minutes, and one written into +a rendered manifest would be a published secret and expired by the time the +manifest synced — so a document that holds one is refused rather than sent. The +credential is read from `--identity-file` at execution, once per request, so a +token rotated between rounds is the one the next round sends. It is sent raw in +the request's credential field, because the host adds the bearer scheme itself, +and it is never printed and never passed as an argument. + +Documents are sent in **rounds**: every pending one once per round, then a +pause that doubles up to `--max-pause`, until each is answered or `--budget` is +spent — so a document the host keeps failing never keeps a later one, a +withdrawal among them, from its first attempt. A refusal the host means (an +invalid document, a refused carrier, a stale or rewritten generation, a +withdrawal it keeps terminal) refuses that document and is not retried; only +"not serving yet" — unavailable, out of time, out of capacity — is. The command +fails when any document was not accepted, so a delivery that did not land fails +the Job that ran it. + +`--address` is `:` of the endpoint the environment's `host.delivery` +names. A Connect endpoint is one: a Connect server answers the gRPC protocol on +the same listener (h2c in plaintext). Pass `--secure` only for a delivery +endpoint that serves TLS itself. + +```bash +codefly delivery apply --documents /etc/codefly/delivery \ + --address accounts.platform.svc.cluster.local:8081 \ + --identity-file /var/run/secrets/codefly/delivery-token +codefly delivery apply --documents ./prepared --address localhost:8081 --identity-file ./token --budget 2m +``` + ### `codefly run cluster [module...]` Run the composition on the environment's local **k3d** cluster through the same @@ -2789,6 +2919,76 @@ environment no longer holds. There is no special `store` alias, single-database limit, default secret path or ignored producer extension. Unknown fields and capabilities fail validation. +#### Retiring a key the producer re-keyed + +Because maps merge by key, a producer that re-spells a service-scoped entry — +`object-storage` becoming `documents/object-storage` — leaves **both**. The bare +key then resolves against a workspace that may compose two services of that +name and is refused as ambiguous, and no ordinary re-import clears it, because +merging is what preserved it. + +`--retire-service-key :=/` (repeatable) is the +operator stating that one bare key is the old spelling of one qualified service: + +``` +codefly environment import blue --coordinate-contract contract.json \ + --retire-service-key service-config:object-storage=documents/object-storage \ + --retire-service-key service-identity:object-storage=documents/object-storage +``` + +The CLI never infers this. Equal values are not evidence and are never +compared: a bare key can name a genuinely different service whose values +happen to match, so only the directive authorizes a removal. + +**It deletes the whole named entry.** Not the part that overlaps the +replacement — the entry. `service-config` resolution copies a bare entry's +values and then overlays the qualified entry's, so a bare entry can supply keys +the destination does not declare; those stop being set. The run prints which +keys those are before writing, and `--dry-run` shows the same with the diff. +The replacement is whatever the contract imported, verbatim: nothing is copied, +renamed or merged across from the entry being retired. + +Supported blocks are `service-config`, `service-identity` and `service-mesh`. +`service-secrets` is **refused by name**: a bare key there is not merely a +different spelling, because a remote reference changes what is owned and what is +a credential, and rewriting it produces a workspace validation then rejects. +Retiring a secrets key needs its own answer. For `service-mesh`, shape +validation already requires qualified keys, so retirement there repairs an +invalid or stale declaration rather than migrating a once-valid bare form. + +A directive is refused, before anything is written, when it is malformed; when +it names the same key twice, or twice with different destinations; when the +block is unsupported; when the bare key is not declared by the selected +environment; when the incoming contract **still** declares that bare key, which +means it is current rather than retired; when the contract declares no such +destination under that block; or when the workspace composes no service for the +destination. Destination existence is proved against the loaded composition. + +That inspection acquires nothing and writes nothing. It refuses a pending +legacy-workspace migration rather than performing one, and it refuses a +`workspaces:` reference that is not materialized here rather than fetching it +to answer — a check that acquired a dependency in order to decide it would +refuse is not read-only. So a rejected directive and `--dry-run` leave +`workspace.codefly.yaml`, the module files, the service sources and the module +cache exactly as they were. + +Sources already prepared are unaffected: a module with a local path override, +or one already materialized, resolves from disk and never reaches a resolver. +Only an unmaterialized pinned reference is declined, and the refusal says to +materialize it and import again. + +It changes only the environment being imported. Another environment carrying the +same bare key keeps it; whether that one is also stale is its owner's call. + +**The directive is one-shot.** It requires the bare key to still be there, so +once a run has written the file the same command refuses. Note that readiness +validation runs *after* the write: a non-zero exit can still mean the retirement +committed, and the retry must omit the flag. Provenance records each retirement +as an operator action with its date, on its own line and deliberately not inside +the re-run hint, which stays replayable. That record is a note about one import, +not a durable audit log and not evidence that the key was ever the contract's — +nothing reads it, and a later ordinary import may replace it. + An import re-serializes only the selected environment item. Surrounding workspace bytes remain unchanged; a provenance comment records the coordinate and import time. `--dry-run` prints the diff without writing. After a write, workspace diff --git a/docs/configuration-contract.md b/docs/configuration-contract.md index b90b8194..3448c377 100644 --- a/docs/configuration-contract.md +++ b/docs/configuration-contract.md @@ -115,6 +115,39 @@ identity on a managed service uses service's own identity uses `["service-identity"]`. Proxy containers, image choices and loopback routing are not part of this contract. +## The projected identity token, and the path that names it + +A workload a host admits proves who it is with a ServiceAccount token the +render projects for the host's audience. Two things reach the container, and +both are the render's: + +- the `codefly-workload-identity` volume, mounted read-only at + `/var/run/secrets/codefly`, with the token at `token` inside it; and +- `CODEFLY__WORKLOAD_TOKEN_FILE`, carrying that file's full path. + +**The mount alone is not the contract.** The SDK's `ProjectedTokenFile` is a +path *value* its caller supplies and reads no environment itself, and the +consumer that presents the credential reads only this variable and refuses when +it is absent — it infers no default path. A workload that mounts the token and +is told nothing therefore acquires no publication lease, and its readiness +refuses, with a correctly rendered volume and ServiceAccount. A native run is +handed the same variable name, from the same constant, pointing at the path it +writes the projection to; deployed and native differ only in the path. + +The value is derived from the mount and the filename rather than written twice, +so the projection and the path it advertises cannot drift. Only the container +that mounts the token is told about it — never a sidecar beside it. + +The variable's name contains `TOKEN`, so it is classified as a credential key, +and a literal value under such a key is normally refused: that is how a secret +would otherwise become a plaintext manifest. A mount path is not secret +material, so the render admits **exactly one** pair — this variable name +carrying exactly the path it mounts — and refuses everything adjacent: a +different value under the same name, the same path under another +credential-named key, a real token literal, and the variable on any container +that does not mount the token at that path. There is no general exemption for +`*_FILE` names and the classifier itself is unchanged. + ## Public module artifacts A workspace configuration value may be **bound to a file in a composed @@ -246,7 +279,11 @@ solution consuming one of its endpoints, an environment declaration naming it and a configuration reference to one of its endpoints. **Those refusals are judged for the environment the operation selected, and the message names that environment**: an operation is not refused because a different environment's -`exclude-services` is inconsistent with the workspace graph. A **malformed** +`exclude-services`, service-scoped names or `managed-services` are +inconsistent with the workspace graph. A bare service-scoped key the producer +has re-keyed is retired explicitly, by an operator directive on +`codefly environment import` — never inferred from a matching name or equal +values; see [the command reference](commands.md#retiring-a-key-the-producer-re-keyed). A **malformed** exclusion is a separate matter — the check validates every environment's declarations for shape before comparing the selected one against the graph, so one of those refuses whatever was selected. It is the validation that refuses, diff --git a/docs/module-manifests.md b/docs/module-manifests.md index 752035ef..f086edd1 100644 --- a/docs/module-manifests.md +++ b/docs/module-manifests.md @@ -312,6 +312,57 @@ false declaration stays false, including for solutions with roles or solution by name. The declaration participates in delivery generation changes and is omitted from withdrawals together with other live policy. +**The installation declaration is authored, carried verbatim, and compared +across both signed documents.** A unit declares, at the top level of its +`module.contract.codefly.yaml`, which standing role an installation's agent +holds and which modules it reaches with the person's Work Context: + +```yaml +installation_role: installer # or "none", or omitted +edge_dependencies: + - module: robin # the signed principal, not a name or binding id + presence: required # "required" or "optional", no default + - module: annotations + presence: optional +``` + +`installation_role` has three states and core validates all of them: **omitted** +is undeclared, the literal **`none`** is no standing machine authority, and any +other value must name a role the unit itself declares. Matching is strictly +literal — no trimming or case-folding in any owner, so `" installer "`, +`" none "` and `" "` are refused rather than resolved, and an empty string is +not `none`. `edge_dependencies` declares presence only: it grants nothing, and +each `module` is the exact signed principal the host resolves through its own +catalogue. A dependency list is never inferred from a module's bindings, which +cover what it calls on its own behalf rather than what a person reaches at the +edge. + +The CLI carries both onto **two** signed documents and normalizes neither: the +core manifest's `authority` block (`installation_role`, `edge_dependencies`) and +the composition delivery's `AuthorityManifestV1` (`installation_role` field 8, +`edge_dependencies` field 9). Declaration order is preserved — the list is a +sequence, not a set, and sorting it would change what is signed. Nothing is +trimmed, defaulted, deduplicated or resolved, because authority compares the two +documents **byte for byte** and refuses a disagreement: `installation_role` must +match exactly (empty and `none` differ), and `edge_dependencies` must match in +length and in each ordered `module`/`presence` pair. An absent or empty list is +the same zero-edge declaration on either side, so the CLI delivers an empty list +as absent and the two spellings cannot disagree. A declaration on only one +document — including a module manifest whose authority document is missing +entirely — is refused before anything is stored, while a unit declaring neither +pairs as it always did. + +Because the comparison is authority's, the CLI reimplements none of it and adds +no second parser: core validates the declaration once, the CLI carries it, and +authority refuses any delivery whose two documents disagree. A declaration is +reason enough for an authority document to exist: a unit whose only policy is an +installation declaration still delivers one. Paired delivery of these fields +requires a host reader that reads them: a publish toward a host whose delivery +endpoint declares `codefly.dev/composition` renders the paired Job (see +[The paired delivery](#the-paired-delivery)); toward any other host the +rendered-manifest delivery Job carries the Core half alone and is not a paired +producer. + **A solution's roles are part of its contract.** A unit a person uses declares the roles a person may hold on an installation of it in the same `module.contract.codefly.yaml`, as `roles` beside its `scope_ceilings`, under @@ -622,6 +673,118 @@ units; the module that serves the delivery API itself (`hostsDelivery`) follows its units. What is not verified: no parent application has run this ordering against a real readiness model. +**The cell file names the delivery endpoint, and carries no port.** The cell's +top-level `delivery_target` is the `host.delivery` identity verbatim — +`//`, the same spelling the render resolves. A +consumer deriving what may reach the delivery API resolves that named endpoint +on the named service itself, as the render does; the document states no address +and no port, because a port here is the hardcoding the cell file exists to +remove. The per-workload `verifier` flag cannot stand in for it: one workload +serves every endpoint of its service, so the flag is identical whichever +endpoint the host names, and a consumer holding only the flag has to guess the +endpoint — by its `api`, which is how a `connect`-typed delivery API came to be +granted on a REST port. The field is always written, since a host block without +a well-formed delivery identity does not validate, so its absence means an +emitter older than the field rather than a cell without one. + +**Which Job a publish renders is the host's declaration.** When the endpoint +`host.delivery` names declares `codefly.dev/composition` (`HostImplements`, the +one reader a native run uses too), the manifest Job runs the paired runner — +see [The paired delivery](#the-paired-delivery). Otherwise it runs the shell +runner, which POSTs `/platform/_delivery/manifest` whatever the endpoint's api +is: a `connect` endpoint that declares no composition interface does not route +that POST, the 404 is not in the verdict set, and the Job retries it until the +budget is spent. The shell runner carries the Core signed carrier alone, and a +module's roles exist only on the proto sidecar the composition interface +carries, since core's own `Authority` has no roles field — so a module +delivered that way reaches the host with no roles at all. + +## The paired delivery + +A paired delivery carries BOTH signed documents to the host's composition +interface: the Core carrier, and the authority declaration signed against that +carrier's exact bytes. It is the only delivery that carries a module's roles. + +Two verbs, because the halves are signed where release custody is and sent from +inside the cell, after the module's workloads exist: + +- **`codefly delivery prepare`** reads the carriers a render settled in a + delivery overlay and the declarations the modules hold, and writes one + `ApplyManifestRequest` per manifest. The authority half is stamped with the + module, the generation and the digest of the exact Core bytes beside it + (`SignedAuthorityDocument`), so a signed authority cannot be replayed against + another manifest, generation or module. A carrier with no signature is + refused by name rather than prepared Core-only. A withdrawal declares nothing + and keeps its Core-only identity. +- **`codefly delivery apply`** sends them to + `host.composition.v1.CompositionService/ApplyManifest`. A Connect endpoint is + a gRPC endpoint: a Connect server answers the gRPC protocol on the same + listener (h2c in plaintext), so `--address` is the `:` of the + endpoint `host.delivery` names. + +**The credential is the one field that cannot travel between those moments.** A +delivery credential is a projected ServiceAccount token with a lifetime of +minutes; one written into a rendered manifest would be a published secret and +expired by the time the manifest synced. So a prepared document carries +`credential: ""`, the sender refuses a document that holds one, and the token is +read from the projection AT EXECUTION — once per request, so a rotation between +rounds is sent on the next one. It goes RAW, with no bearer scheme, because the +host adds the scheme itself before it authorizes the carrier. It is never +printed, never an argument, and never in an error. + +**One rule, two producers.** A native run and a prepared document are the two +producers of the authority half, and the host compares the pair under a single +rule, so the binding (`SignedAuthorityDocument`) and the five declarations read +(`AuthoritySources`) are each one implementation used by both. Two copies would +be two ways to disagree with a rule neither producer can see. + +**Retry and refusal.** Documents are sent in rounds — every pending one once +per round, a pause that doubles, until each is answered or the budget is spent — +so a document the host keeps failing never keeps a later one, a withdrawal +among them, from its first attempt. Only "not serving yet" is retried +(`HostStatusError.Transient`: unavailable, out of time, out of capacity). Every +other code is the host's answer: an invalid document, a refused carrier, a +stale or rewritten generation, a withdrawal it keeps terminal. `FailedPrecondition` +is among them, and it is the status a Connect transport renders as **412** — +which the shell runner's verdict set (`400|401|403|409|422`) did not hold, so it +retried a terminal refusal until the budget was gone. + +**What a GitOps publish renders.** Toward a host whose delivery endpoint +declares `codefly.dev/composition`, publish writes the pair itself, from the +same composed workspace a native run reads (`WorkspaceAuthoritySources`): + +- one paired document per manifest, in its own ConfigMap + `module-manifest--paired` (file `-paired.yaml`, label + `codefly.dev/document: paired-delivery`, key `paired-delivery.json`) beside + the manifest's own ConfigMap, which keeps its shape for every reader of a + delivered overlay. A withdrawal's document carries the Core half alone. +- the manifest Job, running `/bin/codefly delivery apply --documents … --address + ..svc.cluster.local: --identity-file …` with the + projected token, no shell and no `DELIVERY_*` environment, mounting the + paired ConfigMaps rather than the Core-only carriers. + +A publish toward such a host that has no way to read module declarations is +refused by name: it does not deliver the Core half alone. + +**The image.** The paired Job runs `pairedDeliveryImage`, the index digest of +the codefly companion tag `0.0.7`, whose `/bin/codefly` is a static binary that +carries this verb and executes on `linux/amd64` and `linux/arm64` (the shell runner's +`deliveryImage`, tag `0.0.4`, carries a glibc binary in an Alpine image and +cannot exec it). The image follows the ENVIRONMENT: in a paired environment +every delivery Job — the host namespace's envelope Job included — runs the +paired image, and the cell file declares that image for every namespace's +`delivery`, because a cell admits one carrier per namespace by its exact +declaration. What follows the document kind is only the command: the +composition interface has no envelope method, so the envelope Job keeps the +shell runner and POSTs `/platform/_delivery/envelope`, which the paired image +also carries every tool for. + +**Moving the pin.** Every companion bakes the CLI, so a CLI verb a Job must +run needs a new companion version for all of them (core +`docs/runbooks/publish-companions.md`); `0.0.6` predated `delivery apply` and +`0.0.7` is the first tag that carries it. A moved pin changes every cell's +`delivery` declaration, so the cell is re-rendered with it. + ## The module envelope The module envelope is what a host grants each module principal: one diff --git a/go.mod b/go.mod index a1ef2402..c18ccf91 100644 --- a/go.mod +++ b/go.mod @@ -11,11 +11,11 @@ require ( github.com/asottile/dockerfile v3.1.0+incompatible github.com/blang/semver v3.5.1+incompatible github.com/briandowns/spinner v1.23.2 - github.com/codefly-dev/authority/libraries/authority-client/go v0.0.0-20261026143932-67b85b14a942 - github.com/codefly-dev/authority/libraries/authority-core/go v0.0.0-20261026143932-67b85b14a942 + github.com/codefly-dev/authority/libraries/authority-client/go v0.1.1-0.20261029092314-21ce6d9284ed + github.com/codefly-dev/authority/libraries/authority-core/go v0.0.0-20261029092314-21ce6d9284ed github.com/codefly-dev/authority/libraries/delivery/go v0.0.0-20261026023425-000d44495d93 github.com/codefly-dev/authority/libraries/fixture-composition/go v0.0.0-20261027171336-5178f01696e3 - github.com/codefly-dev/authority/libraries/hop/go v0.0.0-20261026175541-3afad7224855 + github.com/codefly-dev/authority/libraries/hop/go v0.0.0-20261026193714-49101ae11a7f github.com/codefly-dev/core v0.19.18-0.20261029150419-9d2348c460ae github.com/codefly-dev/golor v0.1.3 github.com/codefly-dev/llm v0.1.7 diff --git a/go.sum b/go.sum index 0979f3b5..60e52973 100644 --- a/go.sum +++ b/go.sum @@ -105,16 +105,16 @@ github.com/clipperhouse/uax29/v2 v2.7.0 h1:+gs4oBZ2gPfVrKPthwbMzWZDaAFPGYK72F0NJ github.com/clipperhouse/uax29/v2 v2.7.0/go.mod h1:EFJ2TJMRUaplDxHKj1qAEhCtQPW2tJSwu5BF98AuoVM= github.com/cloudflare/circl v1.6.5 h1:O64F26HEqNhznd/hrC5KZXVKYuKM2rx4deZDTc4ihQA= github.com/cloudflare/circl v1.6.5/go.mod h1:h5LNyxAc5nTue9DS5jT+48en2PSDYt3zdGnz5OstK6c= -github.com/codefly-dev/authority/libraries/authority-client/go v0.0.0-20261026143932-67b85b14a942 h1:jLP7MJVHga8yOKMcS6pN2LF3b+usJcjtez8UFSXoc00= -github.com/codefly-dev/authority/libraries/authority-client/go v0.0.0-20261026143932-67b85b14a942/go.mod h1:4hRjiWuJOKzrEk8AS87pbWCbpiEmhEvQBGQaJo5Hmkg= -github.com/codefly-dev/authority/libraries/authority-core/go v0.0.0-20261026143932-67b85b14a942 h1:dZ62S0NAcbIamEuPzv0TeBEqDin0OSFbM1GbNHsnRao= -github.com/codefly-dev/authority/libraries/authority-core/go v0.0.0-20261026143932-67b85b14a942/go.mod h1:n+JVNRyv2dp7BIwUoKYn3TcjN7YpZmxMhBrWp14wsKQ= +github.com/codefly-dev/authority/libraries/authority-client/go v0.1.1-0.20261029092314-21ce6d9284ed h1:6dZKwVVpyiCIg25j32K6WGO72F8yHUKHvNbmYAt8Pq0= +github.com/codefly-dev/authority/libraries/authority-client/go v0.1.1-0.20261029092314-21ce6d9284ed/go.mod h1:4hRjiWuJOKzrEk8AS87pbWCbpiEmhEvQBGQaJo5Hmkg= +github.com/codefly-dev/authority/libraries/authority-core/go v0.0.0-20261029092314-21ce6d9284ed h1:37i4pTVoWoXzV6BTR5jNTWHn0h4AKrpDd09YWWwAI6o= +github.com/codefly-dev/authority/libraries/authority-core/go v0.0.0-20261029092314-21ce6d9284ed/go.mod h1:Cdk+10ebRh0tZA9ze2dDIrfQC34E+Y7LMi6UjSQ2dQU= github.com/codefly-dev/authority/libraries/delivery/go v0.0.0-20261026023425-000d44495d93 h1:PnhgA9wkib67bJnc5hkhkc80J2ch5uYiq+9/NiD/XFU= github.com/codefly-dev/authority/libraries/delivery/go v0.0.0-20261026023425-000d44495d93/go.mod h1:XsOFnO2MQdr37vM3ULa0Uwi8ZWptyVx7hRSUyIdaZ1Q= github.com/codefly-dev/authority/libraries/fixture-composition/go v0.0.0-20261027171336-5178f01696e3 h1:UGR5uQAPBO9YtTFCDWEfXeTgKBYU8vwSFaqza3gT9+U= github.com/codefly-dev/authority/libraries/fixture-composition/go v0.0.0-20261027171336-5178f01696e3/go.mod h1:RxoEMLDVDjAdTwHA7cDVQ9CwYGEuLjTMsf8HdPaHrho= -github.com/codefly-dev/authority/libraries/hop/go v0.0.0-20261026175541-3afad7224855 h1:XmPltOHirB97RopD8WIJe9bwt1wkHAzngKoj8k2XI8U= -github.com/codefly-dev/authority/libraries/hop/go v0.0.0-20261026175541-3afad7224855/go.mod h1:rfn41uiORQ36RrOeFEjXHPLC9NUi0j14AoYJtcovsn4= +github.com/codefly-dev/authority/libraries/hop/go v0.0.0-20261026193714-49101ae11a7f h1:oobjjGqbBaFjYLPYculFD74BOdN5+UZ6eyYl6mOsT7I= +github.com/codefly-dev/authority/libraries/hop/go v0.0.0-20261026193714-49101ae11a7f/go.mod h1:NqCbvIWbkbCzSpUUYQjP8Jv+LpFnDRbVhPdAj67DHhE= github.com/codefly-dev/authority/libraries/resources/go v0.0.0-20261026023425-000d44495d93 h1:4EygH7/J0E6aM7hUPdbQGfit1rsLUNCtWJ8kMh2b9PE= github.com/codefly-dev/authority/libraries/resources/go v0.0.0-20261026023425-000d44495d93/go.mod h1:1rmjrVyr0h7zOuwOHS9i+BoeS9dr3XNvtupLZFwY6As= github.com/codefly-dev/core v0.19.18-0.20261029150419-9d2348c460ae h1:nGuvGc4oRJamUQckRTR60hqOiNESR+qbUeaxwjH9aK0= @@ -388,8 +388,8 @@ github.com/rivo/uniseg v0.4.7 h1:WUdvkW8uEhrYfLC4ZzdpI2ztxP1I582+49Oc5Mq64VQ= github.com/rivo/uniseg v0.4.7/go.mod h1:FN3SvrM+Zdj16jyLfmOkMNblXMcoc8DfTHruCPUcx88= github.com/rodaine/protogofakeit v0.1.1 h1:ZKouljuRM3A+TArppfBqnH8tGZHOwM/pjvtXe9DaXH8= github.com/rodaine/protogofakeit v0.1.1/go.mod h1:pXn/AstBYMaSfc1/RqH3N82pBuxtWgejz1AlYpY1mI0= -github.com/rogpeppe/go-internal v1.14.1 h1:UQB4HGPB6osV0SQTLymcB4TgvyWu6ZyliaW0tI/otEQ= -github.com/rogpeppe/go-internal v1.14.1/go.mod h1:MaRKkUm5W0goXpeCfT7UZI6fk/L7L7so1lCWt35ZSgc= +github.com/rogpeppe/go-internal v1.16.0 h1:O9DK+vNMDVGLr2BeZqmpLeMjiMNkuXfcqntWbZV6S5g= +github.com/rogpeppe/go-internal v1.16.0/go.mod h1:DrUVZyrJU+txYW5/1kwtXQSMFio52ZOxX7yM1VHvnxs= github.com/rs/cors v1.11.1 h1:eU3gRzXLRK57F5rKMGMZURNdIG4EoAmX8k94r9wXWHA= github.com/rs/cors v1.11.1/go.mod h1:XyqrcTp5zjWr1wsJ8PIRZssZ8b/WMcMf71DJnit4EMU= github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM= diff --git a/pkg/conformance/installation_provider_test.go b/pkg/conformance/installation_provider_test.go new file mode 100644 index 00000000..f461759b --- /dev/null +++ b/pkg/conformance/installation_provider_test.go @@ -0,0 +1,62 @@ +package conformance + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +// TestTheCompositionDeliveryIsWiredToReadInstallationDeclarations is a TEXTUAL +// omission detector for one wiring site, and nothing more. +// +// A module's installation declaration reaches the signed authority document +// only if the composition delivery is constructed with a provider that reads +// it. That provider is supplied where the delivery is built, which is a +// different file from the one the delivery's own tests exercise — those +// construct the provider themselves, so dropping it at the construction site +// is invisible to them, and the symptom is a delivery that silently carries no +// declaration while the Core manifest carries one. Authority then refuses the +// pair, so the failure is loud at install and far from this line. +// +// Comments are stripped before matching, so a comment naming the reader does +// not satisfy this. It is a cheap backstop against one omission, not proof the +// wiring is correct. +func TestTheCompositionDeliveryIsWiredToReadInstallationDeclarations(t *testing.T) { + root := repositoryRoot(t) + var built, reads []string + err := filepath.WalkDir(filepath.Join(root, "cmd"), func(path string, entry os.DirEntry, err error) error { + if err != nil || entry.IsDir() || !strings.HasSuffix(path, ".go") || strings.HasSuffix(path, "_test.go") { + return err + } + body := codeOf(t, path, readFile(t, path)) + if !strings.Contains(body, "NewCompositionDelivery(") { + return nil + } + relative, relErr := filepath.Rel(root, path) + if relErr != nil { + relative = path + } + relative = filepath.ToSlash(relative) + built = append(built, relative) + if strings.Contains(body, "InstallationDeclaration(") { + reads = append(reads, relative) + } + return nil + }) + if err != nil { + t.Fatal(err) + } + if len(built) == 0 { + t.Fatal("no command file constructs a composition delivery; this guard has lost its subject") + } + for _, file := range built { + found := false + for _, reader := range reads { + found = found || reader == file + } + if !found { + t.Errorf("%s constructs a composition delivery without reading installation declarations: pass gitops.InstallationDeclaration as its installation provider, or the signed authority document carries no selector or edges while the Core manifest carries both, and the pair is refused at install", file) + } + } +} diff --git a/pkg/environments/excluded_services_test.go b/pkg/environments/excluded_services_test.go index 4ecd1fa2..a3282a6b 100644 --- a/pkg/environments/excluded_services_test.go +++ b/pkg/environments/excluded_services_test.go @@ -367,6 +367,127 @@ func TestValidateWorkspaceHoldsOnlyTheSelectedEnvironment(t *testing.T) { require.Error(t, err, "a nil selection must not skip the check") } +// TestValidateWorkspaceHoldsOnlyTheSelectedScopedNames is the direct half of +// the Blue m106 defect: a bare service-scoped name goes ambiguous when a +// second module composes a service of that name, and every environment was +// being held to the environment that wrote the bare key. +// +// Two modules each compose "store", so `stale`'s bare service-config key is +// ambiguous. Selecting `clean`, which declares nothing of the sort, must +// succeed; selecting `stale` must still refuse; and with no selection the +// whole workspace is still held. +func TestValidateWorkspaceHoldsOnlyTheSelectedScopedNames(t *testing.T) { + ctx := context.Background() + root := t.TempDir() + write := func(rel, content string) { + full := filepath.Join(root, rel) + require.NoError(t, os.MkdirAll(filepath.Dir(full), 0o755)) + require.NoError(t, os.WriteFile(full, []byte(content), 0o600)) + } + service := func(name string) string { + return "kind: service\nname: " + name + "\nversion: 0.0.0\n" + + "agent:\n kind: runtime::service\n name: go-grpc\n version: 0.0.1\n publisher: codefly.ai\n" + } + write(resources.WorkspaceConfigurationName, `name: platform +layout: modules +modules: + - name: shop + - name: depot +environments: + - name: clean + namespace: platform + - name: stale + namespace: platform + service-config: + services: + store: + values: + DATABASE_HOST: store.database.example +`) + write(filepath.Join("modules", "shop", resources.ModuleConfigurationName), "kind: module\nname: shop\nservices:\n - name: store\n") + write(filepath.Join("modules", "shop", "services", "store", resources.ServiceConfigurationName), service("store")) + write(filepath.Join("modules", "depot", resources.ModuleConfigurationName), "kind: module\nname: depot\nservices:\n - name: store\n") + write(filepath.Join("modules", "depot", "services", "store", resources.ServiceConfigurationName), service("store")) + + workspace, err := resources.LoadWorkspaceFromDir(ctx, root) + require.NoError(t, err) + byName := map[string]*environments.Environment{} + declared, err := environments.FromWorkspace(workspace) + require.NoError(t, err) + for _, env := range declared { + byName[env.Name] = env + } + + // The owning environment is genuinely ambiguous: it must still refuse when + // it is the selected one, or the next assertion proves nothing. + err = environments.ValidateWorkspace(ctx, workspace, byName["stale"]) + require.ErrorContains(t, err, `references ambiguous service "store"`) + + // This is the assertion that fails before the fix. + require.NoError(t, environments.ValidateWorkspace(ctx, workspace, byName["clean"]), + "a selected environment must not be refused over another environment's scoped name") + + // Nothing selected: the whole workspace is still held. + err = environments.ValidateWorkspace(ctx, workspace) + require.ErrorContains(t, err, `references ambiguous service "store"`, + "with no selection every environment is still held") +} + +// TestSelectedEnvironmentIsStillHeldToItsOwnMalformedKeys pins what the +// narrowing does NOT do, because Blue's real workspace is this shape and it +// would otherwise be easy to read the fix as clearing their render. +// +// Narrowing holds each environment to its OWN declarations. If the selected +// environment is itself malformed, it is still refused — the refusal moves to +// it rather than disappearing. Blue carries BOTH "object-storage" and +// "documents/object-storage" in the same block, because `environment import` +// adds a producer's new key spelling and keeps the old one, so a bare key +// outlives the rename. The qualified sibling does not redeem the bare key: +// the bare one still resolves against a workspace composing two services of +// that name, and is still ambiguous. +func TestSelectedEnvironmentIsStillHeldToItsOwnMalformedKeys(t *testing.T) { + ctx := context.Background() + root := t.TempDir() + write := func(rel, content string) { + full := filepath.Join(root, rel) + require.NoError(t, os.MkdirAll(filepath.Dir(full), 0o755)) + require.NoError(t, os.WriteFile(full, []byte(content), 0o600)) + } + svc := func(n string) string { + return "kind: service\nname: " + n + "\nversion: 0.0.0\n" + + "agent:\n kind: runtime::service\n name: go-grpc\n version: 0.0.1\n publisher: codefly.ai\n" + } + write(resources.WorkspaceConfigurationName, `name: platform +layout: modules +modules: + - name: documents + - name: saas +environments: + - name: blue + namespace: platform + service-config: + services: + object-storage: + values: {A: "1"} + documents/object-storage: + values: {B: "2"} +`) + write(filepath.Join("modules", "documents", resources.ModuleConfigurationName), "kind: module\nname: documents\nservices:\n - name: object-storage\n") + write(filepath.Join("modules", "documents", "services", "object-storage", resources.ServiceConfigurationName), svc("object-storage")) + write(filepath.Join("modules", "saas", resources.ModuleConfigurationName), "kind: module\nname: saas\nservices:\n - name: object-storage\n") + write(filepath.Join("modules", "saas", "services", "object-storage", resources.ServiceConfigurationName), svc("object-storage")) + + workspace, err := resources.LoadWorkspaceFromDir(ctx, root) + require.NoError(t, err) + declared, err := environments.FromWorkspace(workspace) + require.NoError(t, err) + require.Len(t, declared, 1) + + err = environments.ValidateWorkspace(ctx, workspace, declared[0]) + require.ErrorContains(t, err, `environment "blue" service-config references ambiguous service "object-storage"`, + "selecting an environment does not excuse its own stale bare key; the fix moves this refusal, it does not remove it") +} + // TestSelectionNarrowsTheGraphCheckAndNothingElse pins the limit of the // change, so the prose describing it cannot drift past the code. // diff --git a/pkg/environments/workspace.go b/pkg/environments/workspace.go index 4d687d0c..ac58719f 100644 --- a/pkg/environments/workspace.go +++ b/pkg/environments/workspace.go @@ -211,18 +211,26 @@ func validateResourcePathComponent(kind, name string) error { // caller: it selects, and calls ValidateEnvironmentExclusions directly // (cmd/doctor_workspace.go) rather than relying on this fallback. // -// What the selection narrows is EXACTLY the graph-consistency check, and -// nothing else in this function: +// What the selection narrows is EXACTLY the checks that judge an +// environment's declarations against the workspace's current service graph: +// exclude-services consistency, service-scoped names (service-config, +// service-identity, service-mesh) and managed-service keys. Those are the +// ones that can disagree with the graph for reasons outside the selected +// environment, and they are narrowed together because they fail the same way. +// +// Everything else still holds every environment, deliberately: // // - FromWorkspace above decodes and validates every environment, including -// the SHAPE of its exclude-services (qualified names, duplicates, -// self-withdrawal). A malformed exclusion anywhere still refuses here. -// - module namespaces, host composition, service-scoped names and -// managed-service keys are all still held across every environment. +// the SHAPE of its declarations (qualified names, duplicates, +// self-withdrawal). A malformed declaration anywhere still refuses here, +// whatever was selected: that is not a disagreement with the graph, it is +// a workspace that does not say what it means. +// - module namespaces and host composition are invariants of the workspace +// rather than of a selection. // -// Those remain whole-workspace deliberately — they are invariants of the -// workspace, not of a selection — but they are the same cross-environment -// shape, so narrowing them is a separate decision, not an oversight here. +// The graph itself is still built from the whole workspace. A bare name is +// ambiguous because of what the workspace composes; the selection decides +// whose declarations are judged against it, never what it contains. func ValidateWorkspace(ctx context.Context, workspace *resources.Workspace, selected ...*Environment) error { environments, err := FromWorkspace(workspace) if err != nil { @@ -243,9 +251,17 @@ func ValidateWorkspace(ctx context.Context, workspace *resources.Workspace, sele if excludedErr := validateSelectedExclusions(ctx, workspace, selected); excludedErr != nil { return w.Wrap(excludedErr) } + // The graph passes below hold the SELECTED environment, for the same + // reason the exclusion pass above does: they resolve a declared name + // against the workspace's CURRENT service graph, so an environment whose + // composition predates a module overlay disagrees with that graph through + // no fault of the operation being run. Holding every environment to it + // refused `render --env blue` over `staging`'s bare "object-storage", + // which two modules now declare — and blue could not be repaired from its + // own side, because the offending declaration is not blue's. + scoped := selectedOrAll(environments, selected) needsGraph := false - - for _, env := range environments { + for _, env := range scoped { if env != nil && (len(env.serviceScopedNames()) > 0 || len(env.ManagedServices) > 0) { needsGraph = true break @@ -254,11 +270,14 @@ func ValidateWorkspace(ctx context.Context, workspace *resources.Workspace, sele if !needsGraph { return nil } + // The graph itself stays whole-workspace: a name is ambiguous because of + // what the WORKSPACE composes, not because of what one environment says. + // Only the declarations judged against it are narrowed. byName, err := servicesByName(ctx, workspace) if err != nil { return w.Wrap(err) } - for _, env := range environments { + for _, env := range scoped { if env == nil { continue } @@ -299,14 +318,20 @@ func ValidateWorkspace(ctx context.Context, workspace *resources.Workspace, sele // // It loads the graph only when an environment declares a managed service, so a // workspace without any pays one in-memory projection and no disk read. -func ValidateManagedServices(ctx context.Context, workspace *resources.Workspace) error { +func ValidateManagedServices(ctx context.Context, workspace *resources.Workspace, selected ...*Environment) error { declared, err := FromWorkspace(workspace) if err != nil { return err } w := wool.Get(ctx).In("Workspace::ValidateManagedServices", wool.NameField(workspace.Name)) + // A caller that selected an environment is held to that environment's keys, + // the same way ValidateWorkspace is: a flow resolves managed services + // through the SELECTED env.ManagedService and never asks another + // environment's, so another's stale bare key must not refuse it. A caller + // with no selection still holds every environment. + scoped := selectedOrAll(declared, selected) needsGraph := false - for _, env := range declared { + for _, env := range scoped { if env != nil && len(env.ManagedServices) > 0 { needsGraph = true break @@ -319,7 +344,7 @@ func ValidateManagedServices(ctx context.Context, workspace *resources.Workspace if err != nil { return w.Wrap(err) } - for _, env := range declared { + for _, env := range scoped { if env == nil { continue } @@ -426,3 +451,18 @@ func validateSelectedExclusions(ctx context.Context, workspace *resources.Worksp } return ValidateExcludedServices(ctx, workspace) } + +// selectedOrAll answers which environments a selection-aware pass holds: the +// one that was selected, or every declared environment when nothing was. A nil +// selection is the same as none — a caller that has no environment must not +// silently skip the check — which is the rule validateSelectedExclusions +// already follows. +func selectedOrAll(environments []*Environment, selected []*Environment) []*Environment { + for _, env := range selected { + if env == nil { + continue + } + return []*Environment{env} + } + return environments +} diff --git a/pkg/gitops/authority.go b/pkg/gitops/authority.go index a7f68283..10153791 100644 --- a/pkg/gitops/authority.go +++ b/pkg/gitops/authority.go @@ -157,6 +157,16 @@ func moduleAuthority( return nil, err } } + // The installation declaration, carried VERBATIM. Core validated it + // (contracts/module rules.go: strict literal selector, no trim or + // case-fold) and the paired agreement rule compares it byte-for-byte on + // both signed carriers, where empty and "none" differ and nil and [] are + // equivalent zero edges. So the CLI normalizes nothing, defaults nothing, + // sorts nothing and re-parses nothing: an edge list keeps its declaration + // order, and EdgeDependency.Module stays the signed principal the host + // resolves through its catalogue — never a name, binding id or release. + authority.InstallationRole = resolved.InstallationRole + authority.EdgeDependencies = append([]modulecontract.EdgeDependency(nil), resolved.EdgeDependencies...) authority.Queues = append([]string(nil), resolved.Queues...) authority.Namespaces = append([]string(nil), resolved.Namespaces...) for _, ceiling := range resolved.ScopeCeilings { diff --git a/pkg/gitops/authority_test.go b/pkg/gitops/authority_test.go index 6ba7f2db..6a048b8f 100644 --- a/pkg/gitops/authority_test.go +++ b/pkg/gitops/authority_test.go @@ -94,6 +94,36 @@ func writeAuthorityWorkspace(t *testing.T) (*resources.Workspace, *resources.Mod // writeAuthorityWorkspaceRunning is writeAuthorityWorkspace with the service // declaration supplied, so a case about which agent runs "api" states that // rather than re-laying the whole composition beside it. +// declaringContract is the smallest valid contract for shop that carries an +// installation declaration: one ceiling, one role within it, and whatever the +// case declares. Core validates the selector against the declared roles, so a +// case naming a role must declare it. +func declaringContract(declaration string) string { + return `schema: codefly/module-contract/v1 +principal: shop +namespaces: [shop] +queues: [] +scope_ceilings: [{resource_kind: shop.orders, actions: [read]}] +bindings: [] +destinations: [] +roles: + - name: installer + scopes: [{resource_kind: shop.orders, actions: [read]}] +` + declaration +} + +// writeAuthorityWorkspaceDeclaring is writeAuthorityWorkspace with the module +// contract replaced by one carrying the stated installation declaration. +func writeAuthorityWorkspaceDeclaring(t *testing.T, declaration string) (*resources.Workspace, *resources.Module) { + t.Helper() + workspace, module := writeAuthorityWorkspace(t) + path := filepath.Join(module.Dir(), modulecontract.FileName) + require.NoError(t, os.WriteFile(path, []byte(declaringContract(declaration)), 0o644)) + reloaded, loadErr := workspace.LoadModuleFromName(context.Background(), "shop") + require.NoError(t, loadErr) + return workspace, reloaded +} + func writeAuthorityWorkspaceRunning(t *testing.T, service string) (*resources.Workspace, *resources.Module) { t.Helper() workspace := writeHostedWorkspace(t) @@ -189,6 +219,68 @@ func TestRenderDerivesAuthorityFromTheModuleContract(t *testing.T) { // TestAModuleWithoutAContractAsksForNothingButRunsAsItsPrincipal: a module // publishing no contract asks for no binding, and its manifest still names // its principal, so every workload of it runs as it. +// TestTheInstallationDeclarationIsCarriedVerbatim is the CLI's half of the +// paired agreement rule: the selector and the edge list reach the signed Core +// carrier exactly as declared. +// +// The rule compares both signed carriers BYTE-FOR-BYTE and refuses a +// mismatch, with empty and "none" distinct and nil/[] equivalent zero edges. +// So any normalization here — trimming, defaulting, sorting, deduplicating, +// or resolving EdgeDependency.Module to anything — would make the CLI the +// party that broke the comparison. Core validated the declaration already +// (contracts/module rules.go), and the host resolves the principal through +// its catalogue; neither is the CLI's to repeat. +func TestTheInstallationDeclarationIsCarriedVerbatim(t *testing.T) { + declared := `installation_role: installer +edge_dependencies: + - module: robin + presence: required + - module: annotations + presence: optional +` + workspace, module := writeAuthorityWorkspaceDeclaring(t, declared) + env := selectedEnvironment(t, workspace, "staging") + services := loadServices(t, workspace, "shop", "api") + + authority, err := shopAuthority(t, workspace, module, services, env) + require.NoError(t, err) + require.Equal(t, "installer", authority.InstallationRole) + // Declaration order, not sorted: "robin" before "annotations" is the + // authored order and sorting would reorder them. + require.Equal(t, []modulecontract.EdgeDependency{ + {Module: "robin", Presence: "required"}, + {Module: "annotations", Presence: "optional"}, + }, authority.EdgeDependencies) + + // And it survives into the delivered manifest, which is what is signed. + _, manifest := renderShopManifest(t, workspace, module, env) + require.Equal(t, authority.InstallationRole, manifest.Authority.InstallationRole) + require.Equal(t, authority.EdgeDependencies, manifest.Authority.EdgeDependencies) +} + +// "none" is a value, not an absence: the rule treats empty and "none" as +// different, so the CLI must deliver the literal rather than collapsing it. +func TestTheNoneSelectorIsCarriedAsAValue(t *testing.T) { + for name, tc := range map[string]struct { + declared string + want string + }{ + "none": {"\ninstallation_role: none\n", "none"}, + "undeclared": {"", ""}, + } { + t.Run(name, func(t *testing.T) { + workspace, module := writeAuthorityWorkspaceDeclaring(t, tc.declared) + env := selectedEnvironment(t, workspace, "staging") + services := loadServices(t, workspace, "shop", "api") + + authority, err := shopAuthority(t, workspace, module, services, env) + require.NoError(t, err) + require.Equal(t, tc.want, authority.InstallationRole) + require.Empty(t, authority.EdgeDependencies) + }) + } +} + func TestAModuleWithoutAContractAsksForNothingButRunsAsItsPrincipal(t *testing.T) { workspace, module := writeAuthorityWorkspace(t) require.NoError(t, os.Remove(filepath.Join(module.Dir(), modulecontract.FileName))) diff --git a/pkg/gitops/cell.go b/pkg/gitops/cell.go index 3e6b09a0..497a9e9c 100644 --- a/pkg/gitops/cell.go +++ b/pkg/gitops/cell.go @@ -121,10 +121,23 @@ func deriveCell(ctx context.Context, workspace *resources.Workspace, env *enviro if err != nil { return cellfile.File{}, CellResult{}, err } + // The delivery carrier the cell admits follows the HOST's own declaration, + // read the one way everything else reads it (HostImplements), so the cell + // and the Job publish writes cannot disagree about which image runs. + hostModule, hostService, hostEndpoint := env.Host.DeliveryEndpoint() + paired, err := HostImplements(ctx, workspace, hostModule, hostService, hostEndpoint, CompositionInterface) + if err != nil { + return cellfile.File{}, CellResult{}, err + } file := cellfile.File{ Schema: cellfile.Schema, Environment: env.Name, Coordinate: env.Host.Coordinate, Component: env.Host.Component, Domain: env.Host.Domain, TrustDomain: env.Host.TrustDomain, - Source: cellSource(ctx, workspace, env), + // Verbatim: the cell says WHICH endpoint is the delivery API, and its + // consumer resolves that named endpoint itself. isDeliveryVerifier + // below marks the workload that serves it but drops the endpoint name, + // which left a consumer nothing to resolve and no choice but to guess. + DeliveryTarget: env.Host.Delivery, + Source: cellSource(ctx, workspace, env), } result := CellResult{Source: file.Source != nil, ConfiguredEdges: configured} for _, entry := range entries { @@ -151,7 +164,7 @@ func deriveCell(ctx context.Context, workspace *resources.Workspace, env *enviro return cellfile.File{}, CellResult{}, fmt.Errorf("the render of module %s still delivers %s/%s, which environment %s excludes (exclude-services); render %s again for %s", inventory.Module, inventory.Module, unit.Name, env.Name, inventory.Module, env.Name) } } - namespace, namespaceErr := cellNamespace(ctx, workspace, env, tree, &inventory, consumers) + namespace, namespaceErr := cellNamespace(ctx, workspace, env, tree, &inventory, consumers, paired) if namespaceErr != nil { return cellfile.File{}, CellResult{}, namespaceErr } @@ -259,7 +272,7 @@ func cellSource(ctx context.Context, workspace *resources.Workspace, env *enviro // cellNamespace derives one module's namespace entry from its rendered tree // and the composition's declarations for it. -func cellNamespace(ctx context.Context, workspace *resources.Workspace, env *environments.Environment, tree string, inventory *Inventory, consumers map[string][]string) (cellfile.Namespace, error) { +func cellNamespace(ctx context.Context, workspace *resources.Workspace, env *environments.Environment, tree string, inventory *Inventory, consumers map[string][]string, paired bool) (cellfile.Namespace, error) { namespace := cellfile.Namespace{Name: inventory.Namespace, Module: inventory.Module} if namespace.Name == "" { return cellfile.Namespace{}, fmt.Errorf("the render of module %s names no namespace", inventory.Module) @@ -396,7 +409,7 @@ func cellNamespace(ctx context.Context, workspace *resources.Workspace, env *env return cellfile.Namespace{}, err } if carries { - delivery, err := cellDelivery(env, namespace.Name) + delivery, err := cellDelivery(env, namespace.Name, paired) if err != nil { return cellfile.Namespace{}, fmt.Errorf("module %s: %w", inventory.Module, err) } @@ -551,8 +564,13 @@ func carriesDelivery(workspace *resources.Workspace, env *environments.Environme // itself is always written at publish, into the delivery repository, so a tree // in the workspace never holds it, and the cell must declare the carrier // before publish runs for its admission to let the Job in. -func cellDelivery(env *environments.Environment, namespace string) (*cellfile.Delivery, error) { - carrier := deliveryCarrierOf() +// paired says whether the environment's delivery endpoint declares +// codefly.dev/composition, because a paired Job runs a different image — one +// that can execute codefly rather than a shell script — and a cell admits the +// carrier by its exact declaration, so the two must agree or the pod the Job +// creates matches no approved entry and the delivery sync fails naming neither. +func cellDelivery(env *environments.Environment, namespace string, paired bool) (*cellfile.Delivery, error) { + carrier := deliveryCarrierOf(paired) repository, digest, err := splitPinnedImage(carrier.Image) if err != nil { return nil, fmt.Errorf("the delivery Job's %w", err) diff --git a/pkg/gitops/cell_test.go b/pkg/gitops/cell_test.go index 1a171ad9..f6825e00 100644 --- a/pkg/gitops/cell_test.go +++ b/pkg/gitops/cell_test.go @@ -658,3 +658,39 @@ func TestThePreparedRunnableReachIsAConsumerEdge(t *testing.T) { require.EqualError(t, err, "module shop derived operation shop-workload-sync: service api publishes it on endpoint workload, which that service does not declare") }) } + +// The cell must say WHICH endpoint is the delivery API, not only which +// workload serves it. A consumer deriving what may reach the delivery API has +// to resolve a port, and the verifier flag alone leaves it nothing to resolve: +// one workload serves every endpoint of its service, so the flag is identical +// whichever endpoint the host names. A consumer with only that fact must guess +// — by the endpoint's api, which is how a connect-typed delivery API came to +// be granted on a REST port — so the name is carried verbatim, resolved by +// whoever reads it. +func TestTheCellNamesTheDeliveryEndpointAndNotOnlyItsWorkload(t *testing.T) { + host := " delivery: billing/ledger/http" + for _, declared := range []string{"billing/ledger/http", "billing/ledger/grpc"} { + t.Run(declared, func(t *testing.T) { + workspace := writeCellWorkspace(t, [3]string{ + resources.WorkspaceConfigurationName, host, " delivery: " + declared, + }) + env := selectedEnvironment(t, workspace, "staging") + renderCellTree(t, workspace, "shop", "api", "acme-shop", "staging") + renderCellTree(t, workspace, "billing", "ledger", "acme-billing", "staging") + result, err := RenderCell(context.Background(), workspace, env) + require.NoError(t, err) + data, err := os.ReadFile(result.Path) + require.NoError(t, err) + file, err := cellfile.Parse(data) + require.NoError(t, err) + require.Equal(t, declared, file.DeliveryTarget, + "the host block's delivery identity, verbatim: the same // spelling the render resolves, never an address or a port") + require.Contains(t, string(data), "delivery_target: "+declared) + ledger := file.Namespaces[0].Workloads[0] + require.True(t, ledger.Verifier, + "the same workload serves both endpoints, so the verifier flag is identical either way and cannot say which endpoint the host named") + require.Equal(t, []string{"grpc", "http"}, []string{ledger.Endpoints[0].Name, ledger.Endpoints[1].Name}, + "and both endpoints are inventoried either way, so neither is distinguishable by its presence") + }) + } +} diff --git a/pkg/gitops/cellfile/cellfile.go b/pkg/gitops/cellfile/cellfile.go index 5c969f7a..d929a581 100644 --- a/pkg/gitops/cellfile/cellfile.go +++ b/pkg/gitops/cellfile/cellfile.go @@ -30,14 +30,25 @@ const FileName = "cell.yaml" // File is the whole cell: one header naming the host, and one namespace entry // per module delivered to it, sorted by namespace name. type File struct { - Schema string `yaml:"schema"` - Coordinate string `yaml:"coordinate"` - Component string `yaml:"component"` - Environment string `yaml:"environment"` - Source *Source `yaml:"source,omitempty"` - Domain string `yaml:"domain,omitempty"` - TrustDomain string `yaml:"trust_domain"` - Namespaces []Namespace `yaml:"namespaces"` + Schema string `yaml:"schema"` + Coordinate string `yaml:"coordinate"` + Component string `yaml:"component"` + Environment string `yaml:"environment"` + Source *Source `yaml:"source,omitempty"` + Domain string `yaml:"domain,omitempty"` + TrustDomain string `yaml:"trust_domain"` + // DeliveryTarget is the composition identity of the host's delivery API — + // "//" — mirrored VERBATIM from the + // environment's host block, never resolved to an address or a port here. + // A consumer deriving what may reach the delivery API resolves the NAMED + // endpoint on the named service, the way the render does: a port in this + // document would be the hardcoding the cell file exists to remove, and + // guessing the endpoint by its api is how a connect-typed delivery API + // came to be granted on a REST port. It is always present: a host block + // without a well-formed delivery identity does not validate, so absence + // means an emitter older than this field rather than a cell without one. + DeliveryTarget string `yaml:"delivery_target"` + Namespaces []Namespace `yaml:"namespaces"` } // Source is the composition-owned repository, the branch the rendered trees diff --git a/pkg/gitops/composition_delivery.go b/pkg/gitops/composition_delivery.go index ef30d35f..6bb0efa9 100644 --- a/pkg/gitops/composition_delivery.go +++ b/pkg/gitops/composition_delivery.go @@ -2,6 +2,7 @@ package gitops import ( "context" + "errors" "fmt" "sync" @@ -25,17 +26,11 @@ type CompositionDelivery struct { client compositionv1.CompositionServiceClient // signer is the run's release key, which signs the core manifest too. signer solutionhost.Signer - // methods reads what each method of a module declares, from its package; - // ok false means the module declares no method policy. - methods func(ctx context.Context, module string) ([]*compositionv1.MethodPolicyV1, bool, error) - // ceilings reads the module's own resource kinds with their actions, from - // its module contract; none when it publishes no contract. - ceilings func(ctx context.Context, module string) ([]*authorityv1.WorkScopeV1, error) - // roles reads the roles a solution declares, from its module contract; - // none for a module, which declares no roles. - roles func(ctx context.Context, module string) ([]*compositionv1.RoleV1, error) - // ownerAware reads the module's explicit owner-selector enforcement declaration. - ownerAware func(ctx context.Context, module string) (bool, error) + // sources are the module's own declarations, read where they live. One + // value because a publish preparing a document for a cell to send reads the + // very same five (AuthoritySources), and two copies of that list is two + // ways for the pair the host compares to disagree. + sources AuthoritySources mu sync.Mutex applied map[string]AppliedManifest @@ -47,13 +42,22 @@ type CompositionDelivery struct { // actions on each of its own resource kinds, read by ceilings from its module // contract (core's manifest names the kinds alone), and the roles a solution // declares, read by roles from its module contract too (core's manifest -// carries none), and its explicit owner-aware declaration. +// carries none), and its explicit owner-aware declaration, and the +// installation declaration read by installation from that contract — the +// selector and person-context edges, carried verbatim because the paired +// agreement rule compares this document against the Core signed manifest +// byte-for-byte. func NewCompositionDelivery(conn grpc.ClientConnInterface, signer solutionhost.Signer, methods func(ctx context.Context, module string) ([]*compositionv1.MethodPolicyV1, bool, error), ceilings func(ctx context.Context, module string) ([]*authorityv1.WorkScopeV1, error), roles func(ctx context.Context, module string) ([]*compositionv1.RoleV1, error), - ownerAware func(ctx context.Context, module string) (bool, error)) *CompositionDelivery { - return &CompositionDelivery{client: compositionv1.NewCompositionServiceClient(conn), signer: signer, methods: methods, ceilings: ceilings, roles: roles, ownerAware: ownerAware, applied: map[string]AppliedManifest{}} + ownerAware func(ctx context.Context, module string) (bool, error), + installation func(ctx context.Context, module string) (string, []*compositionv1.EdgeDependencyV1, error)) *CompositionDelivery { + return &CompositionDelivery{ + client: compositionv1.NewCompositionServiceClient(conn), signer: signer, + sources: AuthoritySources{Methods: methods, Ceilings: ceilings, Roles: roles, OwnerAware: ownerAware, Installation: installation}, + applied: map[string]AppliedManifest{}, + } } func (c *CompositionDelivery) post(ctx context.Context, module string, signed *solutionhost.Signed, token string, declaration []byte) (Receipt, error) { @@ -94,7 +98,7 @@ func (c *CompositionDelivery) Prepare(ctx context.Context, module string, remove var declaration []byte if !removed && c.signer != nil { var err error - declaration, err = c.declaration(ctx, module) + declaration, err = c.sources.Declaration(ctx, module) if err != nil { return nil, err } @@ -109,49 +113,115 @@ func (c *CompositionDelivery) Prepare(ctx context.Context, module string, remove }}, nil } -func (c *CompositionDelivery) declaration(ctx context.Context, module string) ([]byte, error) { +// AuthoritySources are a module's own declarations, each read where it lives: +// the method policies its package declares, the actions on its own resource +// kinds, the roles a solution declares, its explicit owner-aware declaration, +// and its installation declaration — the standing role selector and the +// person-context edges it reaches. +// +// It is one type because there are two producers of the authority half: a +// native run sending it over the composition interface, and a publish +// preparing a document for a cell's delivery to send later. They must read the +// same five things and marshal them the same way, or the pair the host +// compares byte-for-byte disagrees for a reason neither producer can see. A +// nil reader declares nothing, which is how a caller that has only some of +// them (a local qualification) is not forced to invent the rest. +type AuthoritySources struct { + // Methods reads what each method of a module declares, from its package; + // ok false means the module declares no method policy. + Methods func(ctx context.Context, module string) ([]*compositionv1.MethodPolicyV1, bool, error) + // Ceilings reads the module's own resource kinds with their actions, from + // its module contract; none when it publishes no contract. + Ceilings func(ctx context.Context, module string) ([]*authorityv1.WorkScopeV1, error) + // Roles reads the roles a solution declares, from its module contract; + // none for a module, which declares no roles. Core's own manifest has no + // roles field at all, so this is the only way a role reaches a host. + Roles func(ctx context.Context, module string) ([]*compositionv1.RoleV1, error) + // OwnerAware reads the module's explicit owner-selector enforcement declaration. + OwnerAware func(ctx context.Context, module string) (bool, error) + // Installation reads the installation declaration. Carried verbatim — the + // paired agreement rule compares this document against the Core signed + // manifest byte-for-byte. + Installation func(ctx context.Context, module string) (string, []*compositionv1.EdgeDependencyV1, error) +} + +// Declaration captures one module's declaration as the deterministic bytes +// both identify its generation and are signed as the authority half. A module +// that declares nothing at all answers nil, not an empty document: absence and +// an empty declaration are different things to the host, and a nil here is +// what keeps a withdrawal Core-only. +func (s AuthoritySources) Declaration(ctx context.Context, module string) ([]byte, error) { var methods []*compositionv1.MethodPolicyV1 declared := false - if c.methods != nil { + if s.Methods != nil { var err error - if methods, declared, err = c.methods(ctx, module); err != nil { + if methods, declared, err = s.Methods(ctx, module); err != nil { return nil, err } } var ceilings []*authorityv1.WorkScopeV1 - if c.ceilings != nil { + if s.Ceilings != nil { var err error - if ceilings, err = c.ceilings(ctx, module); err != nil { + if ceilings, err = s.Ceilings(ctx, module); err != nil { return nil, err } } var roles []*compositionv1.RoleV1 - if c.roles != nil { + if s.Roles != nil { var err error - if roles, err = c.roles(ctx, module); err != nil { + if roles, err = s.Roles(ctx, module); err != nil { return nil, err } } var ownerAware bool - if c.ownerAware != nil { + if s.OwnerAware != nil { var err error - if ownerAware, err = c.ownerAware(ctx, module); err != nil { + if ownerAware, err = s.OwnerAware(ctx, module); err != nil { return nil, err } } - if !declared && len(ceilings) == 0 && len(roles) == 0 && !ownerAware { + var installationRole string + var edges []*compositionv1.EdgeDependencyV1 + if s.Installation != nil { + var err error + if installationRole, edges, err = s.Installation(ctx, module); err != nil { + return nil, err + } + } + if !declared && len(ceilings) == 0 && len(roles) == 0 && !ownerAware && + installationRole == "" && len(edges) == 0 { return nil, nil } return proto.MarshalOptions{Deterministic: true}.Marshal(&compositionv1.AuthorityManifestV1{ Methods: methods, ScopeCeilings: ceilings, Roles: roles, OwnerAware: ownerAware, + InstallationRole: installationRole, EdgeDependencies: edges, }) } // authority binds the captured declaration to the final Core bytes and signs it. func (c *CompositionDelivery) authority(manifest *solutionhost.ModuleManifest, core, declaration []byte) (*compositionv1.SignedManifestV1, error) { + return SignedAuthorityDocument(manifest, core, declaration, c.signer) +} + +// SignedAuthorityDocument binds a captured declaration to the final Core bytes +// and signs it: the authority half of a paired delivery. +// +// It is ONE function because there are two producers of that half — a native +// run sending it over the composition interface, and a publish preparing it +// for a cell's delivery to send later — and the host compares the pair under a +// single rule. Two implementations of this binding would be two ways to +// disagree with it. The module, the generation and the digest of the exact +// Core bytes are stamped here rather than taken from the declaration, so the +// signed authority cannot be replayed against another manifest, another +// generation or another module. A nil declaration carries nothing, which is +// how a withdrawal keeps its Core-only identity. +func SignedAuthorityDocument(manifest *solutionhost.ModuleManifest, core, declaration []byte, signer solutionhost.Signer) (*compositionv1.SignedManifestV1, error) { if declaration == nil { return nil, nil } + if signer == nil { + return nil, errors.New("the authority half of a paired delivery is signed by the release key, and none was given") + } var policy compositionv1.AuthorityManifestV1 if err := proto.Unmarshal(declaration, &policy); err != nil { return nil, err @@ -161,7 +231,7 @@ func (c *CompositionDelivery) authority(manifest *solutionhost.ModuleManifest, c if err != nil { return nil, err } - envelope, err := delivery.Sign(document, c.signer.KeyID(), c.signer.Sign) + envelope, err := delivery.Sign(document, signer.KeyID(), signer.Sign) if err != nil { return nil, err } diff --git a/pkg/gitops/composition_delivery_test.go b/pkg/gitops/composition_delivery_test.go index 41bff8d4..7c32b3d0 100644 --- a/pkg/gitops/composition_delivery_test.go +++ b/pkg/gitops/composition_delivery_test.go @@ -62,7 +62,7 @@ func TestCompositionDeliveryCarriesTheSignedManifestThroughApplyManifest(t *test if err != nil { t.Fatal(err) } - delivery := NewCompositionDelivery(conn, nil, nil, nil, nil, nil) + delivery := NewCompositionDelivery(conn, nil, nil, nil, nil, nil, nil) manifest := &solutionhost.ModuleManifest{ Schema: solutionhost.SchemaModuleManifestV1, Binding: "probe", Alias: "probe", Kind: solutionhost.KindModule, Generation: 2, Host: solutionhost.HostTarget{Coordinate: "local", Domain: "local"}, Release: solutionhost.Release{Publisher: "test", Name: "probe", Version: "0.1.0"}, @@ -189,7 +189,10 @@ func TestCompositionWithdrawalDoesNotReadOrDeliverLivePolicies(t *testing.T) { }, func(context.Context, string) ([]*authorityv1.WorkScopeV1, error) { return nil, missing() }, func(context.Context, string) ([]*compositionv1.RoleV1, error) { return nil, missing() }, - func(context.Context, string) (bool, error) { return false, missing() }) + func(context.Context, string) (bool, error) { return false, missing() }, + func(context.Context, string) (string, []*compositionv1.EdgeDependencyV1, error) { + return "", nil, missing() + }) manifest := &solutionhost.ModuleManifest{Schema: solutionhost.SchemaModuleManifestV1, Binding: "probe", Alias: "probe", Kind: kind, Generation: 2, Removed: true, Host: solutionhost.HostTarget{Coordinate: "local", Domain: "local"}, Release: solutionhost.Release{Publisher: "test", Name: "probe", Version: "0.1.0"}} payload, err := solutionhost.SignedPayload(manifest) @@ -233,7 +236,7 @@ func TestCompositionDeliveryRefusesAMismatchedAcknowledgement(t *testing.T) { t.Fatal(err) } t.Cleanup(func() { _ = conn.Close() }) - delivery := NewCompositionDelivery(conn, nil, nil, nil, nil, nil) + delivery := NewCompositionDelivery(conn, nil, nil, nil, nil, nil, nil) manifest := &solutionhost.ModuleManifest{Schema: solutionhost.SchemaModuleManifestV1, Binding: "probe", Alias: "probe", Kind: solutionhost.KindModule, Generation: 2, Removed: true, Host: solutionhost.HostTarget{Coordinate: "local", Domain: "local"}, Release: solutionhost.Release{Publisher: "test", Name: "probe", Version: "0.1.0"}} document, err := solutionhost.SignedPayload(manifest) if err != nil { diff --git a/pkg/gitops/delivery_carrier_contract_test.go b/pkg/gitops/delivery_carrier_contract_test.go index ea3f9803..8c9c9bde 100644 --- a/pkg/gitops/delivery_carrier_contract_test.go +++ b/pkg/gitops/delivery_carrier_contract_test.go @@ -42,12 +42,14 @@ func TestTheDeliveryCarrierLabelValueIsFrozen(t *testing.T) { // delivery Jobs. Within one render they must be the same label set. func TestTheCellSelectorIsTheLabelSetTheCarrierPodCarries(t *testing.T) { env := &environments.Environment{Name: "prod", Host: testHost()} - delivery, err := cellDelivery(env, "example-prod-shop") - require.NoError(t, err) - require.Equal(t, deliveryCarrierOf().Labels, delivery.Selector, - "the cell's carrier selector and the delivery pod's labels are one label set") - require.Equal(t, deliveryServiceAccount, delivery.ServiceAccount, - "the cell admits the account the delivery Job runs as") + for _, paired := range []bool{false, true} { + delivery, err := cellDelivery(env, "example-prod-shop", paired) + require.NoError(t, err) + require.Equal(t, deliveryCarrierOf(paired).Labels, delivery.Selector, + "the cell's carrier selector and the delivery pod's labels are one label set") + require.Equal(t, deliveryServiceAccount, delivery.ServiceAccount, + "the cell admits the account the delivery Job runs as") + } } // The cell file declares the carrier's image as codefly's own repository on @@ -55,10 +57,12 @@ func TestTheCellSelectorIsTheLabelSetTheCarrierPodCarries(t *testing.T) { // the render pins, and it is never pulled from Docker Hub. func TestTheCellDeclaresCodeflysOwnDeliveryImage(t *testing.T) { env := &environments.Environment{Name: "prod", Host: testHost()} - delivery, err := cellDelivery(env, "example-prod-shop") - require.NoError(t, err) - require.Equal(t, "ghcr.io/codefly-dev/codefly", delivery.Image.Repository) - require.Regexp(t, `^sha256:[0-9a-f]{64}$`, delivery.Image.Digest) - require.Equal(t, delivery.Image.Repository+"@"+delivery.Image.Digest, deliveryImage, - "the cell's carrier image is the image the delivery Job runs") + for paired, image := range map[bool]string{false: deliveryImage, true: pairedDeliveryImage} { + delivery, err := cellDelivery(env, "example-prod-shop", paired) + require.NoError(t, err) + require.Equal(t, "ghcr.io/codefly-dev/codefly", delivery.Image.Repository) + require.Regexp(t, `^sha256:[0-9a-f]{64}$`, delivery.Image.Digest) + require.Equal(t, delivery.Image.Repository+"@"+delivery.Image.Digest, image, + "the cell's carrier image is the image the delivery Job runs") + } } diff --git a/pkg/gitops/delivery_jobs.go b/pkg/gitops/delivery_jobs.go index f6ef19f4..765bb5da 100644 --- a/pkg/gitops/delivery_jobs.go +++ b/pkg/gitops/delivery_jobs.go @@ -68,6 +68,26 @@ const ( // first Job that runs the new one. deliveryImage = "ghcr.io/codefly-dev/codefly@sha256:8fcaaea34622eb1743a5b755c7c6540a0be245cb37c8b2188732f45c8207a833" + // pairedDeliveryImage is the image a PAIRED delivery Job runs, which must + // execute codefly itself rather than a shell script: the index digest of + // tag 0.0.7, the same companion built by the same recipe + // (core companions/codefly/Dockerfile, published by this repo's + // companions.yaml, which cross-compiles the CLI into + // bin/linux//codefly) from a CLI that carries `delivery apply` + // (core bumped every companion in codefly-dev/core#875, because every one + // of them bakes the CLI). Measured on linux/amd64 and linux/arm64: + // `delivery apply --help` answers. + // + // It is a SEPARATE pin because deliveryImage's /bin/codefly cannot execute: + // at 0.0.4 the cross-compiled binary was dynamically linked against glibc + // while the base is Alpine, so its interpreter — /lib/ld-linux-aarch64.so.1 + // on arm64, /lib64/ld-linux-x86-64.so.2 on amd64, where /lib64 does not + // exist at all — is absent and exec fails with ENOENT. Since 0.0.6 the + // publisher cross-compiles a STATIC binary, and 0.0.6's binary predated the + // verb. The shell runner's pin is left where it is so moving it is its own + // change, with its own re-render of every cell. + pairedDeliveryImage = "ghcr.io/codefly-dev/codefly@sha256:1cbd3826d33517926bdfeaec70c9666d7ccde8c7a8d7edf398540a892c75049c" + // deliveryContainerName is the one container of a delivery Job. deliveryContainerName = "deliver" @@ -153,6 +173,19 @@ type DeliveryTarget struct { // Audience is the audience the host verifies a carrier token against; the // Job's projected ServiceAccount token is minted for it. Audience string + // Address is "..svc.cluster.local:" — the same + // endpoint without a scheme, which is what a gRPC target is. + Address string + // Composition reports that the endpoint host.delivery names DECLARES + // codefly.dev/composition (HostImplements), so what delivers to it must + // carry BOTH signed halves through ApplyManifest. A host that declares no + // such interface takes the Core half alone over its HTTP manifest route, + // and a module delivered that way reaches it with no roles. + Composition bool + // API is the endpoint's declared api, carried so a render refuses an + // endpoint it cannot post to by name rather than rendering a request that + // endpoint does not route. + API string } // deliveryDocument is one carrier a Job delivers: the ConfigMap holding it and @@ -382,20 +415,86 @@ type deliveryCarrier struct { // the pod is labelled as the carrier and only the Job's own // deliveredDocumentLabel names the kind it delivers: an envelope Job in the // host's namespace runs the same admitted pod as that namespace's manifest -// Job. -func deliveryCarrierOf() deliveryCarrier { +// Job. paired is the ENVIRONMENT's — its delivery endpoint declares +// codefly.dev/composition — never the document's, so every Job in a paired +// environment, an envelope Job included, runs the paired image; what differs +// by kind is only the command (pairedRunner). +func deliveryCarrierOf(paired bool) deliveryCarrier { + image := deliveryImage + if paired { + image = pairedDeliveryImage + } return deliveryCarrier{ Kind: kindJob, Labels: map[string]string{managedByLabel: managedByCodefly, deliveryLabel: deliveryCarrierValue}, ServiceAccount: deliveryServiceAccount, Container: deliveryContainerName, - Image: deliveryImage, + Image: image, + } +} + +// pairedRunner reports whether a Job delivering kind to target runs codefly's +// paired runner rather than the shell runner. Only a module manifest has a +// method on the composition interface (ApplyManifest); the envelope has none, +// so an envelope Job keeps POSTing its carrier over HTTP even toward a host +// that declares codefly.dev/composition. The IMAGE is not decided here: it is +// the namespace's carrier (deliveryCarrierOf(target.Composition)), because a +// cell admits one carrier per namespace and the host's namespace runs both +// Jobs — the paired image carries every tool the shell runner calls. +func pairedRunner(target *DeliveryTarget, kind string) bool { + return target.Composition && kind == deliveryManifest +} + +// deliveryCommand is what the carrier's container runs: the shell runner that +// POSTs each document to the host's HTTP delivery route, or — for a manifest +// delivered to a host whose delivery endpoint declares codefly.dev/composition +// — codefly itself, sending both signed halves through ApplyManifest. +// +// The paired runner takes its inputs as FLAGS rather than environment, because +// there is no shell: the arguments are the manifest's own record of what the +// Job delivers and where. The credential is not among them; the runner reads +// it from the projected token at execution, which is the only moment a token +// with a lifetime of minutes exists. +func deliveryCommand(target *DeliveryTarget, kind string) []string { + if !pairedRunner(target, kind) { + return []string{"/bin/sh", "-ec", deliveryScript} + } + return []string{ + "/bin/codefly", "delivery", "apply", + "--documents", deliveryDocumentsMount, + "--address", target.Address, + "--identity-file", deliveryIdentityMount + "/" + deliveryTokenFile, + "--budget", strconv.Itoa(deliveryBudgetSeconds) + "s", + "--request-timeout", strconv.Itoa(deliveryRequestSeconds) + "s", + "--max-pause", strconv.Itoa(deliveryMaxPause) + "s", + } +} + +// deliveryEnvironment is the shell runner's environment. A paired Job carries +// none of it: its inputs are its arguments, and a variable a runner does not +// read is a statement nothing checks. +func deliveryEnvironment(target *DeliveryTarget, kind, path string) []deliveryEnv { + if pairedRunner(target, kind) { + return nil + } + return []deliveryEnv{ + {Name: "DELIVERY_URL", Value: target.URL}, + {Name: "DELIVERY_PATH", Value: path}, + {Name: "DELIVERY_DOCUMENTS", Value: deliveryDocumentsMount}, + {Name: "DELIVERY_BUDGET_SECONDS", Value: strconv.Itoa(deliveryBudgetSeconds)}, + {Name: "DELIVERY_REQUEST_SECONDS", Value: strconv.Itoa(deliveryRequestSeconds)}, + {Name: "DELIVERY_MAX_PAUSE_SECONDS", Value: strconv.Itoa(deliveryMaxPause)}, + // Named for what it is, the FILE the projected identity is read + // from: a variable named *_TOKEN carrying a value is classified + // as a credential by the promotable ruleset, which refuses the + // manifest — and the value is a path, never the token. + {Name: "DELIVERY_IDENTITY_FILE", Value: deliveryIdentityMount + "/" + deliveryTokenFile}, } } // renderDeliveryJob writes the Job delivering documents to the host's delivery // route into the overlay directory, and returns the manifest's file name. The -// pod it runs is deliveryCarrierOf(), and nothing else. +// pod it runs is deliveryCarrierOf(target.Composition), and nothing else. func renderDeliveryJob(directory, name, namespace, kind, path string, target *DeliveryTarget, documents []deliveryDocument) (string, error) { if len(documents) > deliveryDocumentBound { return "", fmt.Errorf("the %s set carries %d documents and one delivery Job attempts at most %d within its budget (%d seconds, %d per request); the set does not shrink until the host acknowledges its tombstones, which the delivery API does not offer yet", @@ -416,7 +515,7 @@ func renderDeliveryJob(directory, name, namespace, kind, path string, target *De Items: []deliveryKeyToPath{{Key: document.Key, Path: document.Name}}, }}) } - carrier := deliveryCarrierOf() + carrier := deliveryCarrierOf(target.Composition) jobLabels := maps.Clone(carrier.Labels) jobLabels[deliveredDocumentLabel] = kind job := deliveryJobManifest{ @@ -446,20 +545,8 @@ func renderDeliveryJob(directory, name, namespace, kind, path string, target *De Containers: []deliveryContainer{{ Name: carrier.Container, Image: carrier.Image, - Command: []string{"/bin/sh", "-ec", deliveryScript}, - Env: []deliveryEnv{ - {Name: "DELIVERY_URL", Value: target.URL}, - {Name: "DELIVERY_PATH", Value: path}, - {Name: "DELIVERY_DOCUMENTS", Value: deliveryDocumentsMount}, - {Name: "DELIVERY_BUDGET_SECONDS", Value: strconv.Itoa(deliveryBudgetSeconds)}, - {Name: "DELIVERY_REQUEST_SECONDS", Value: strconv.Itoa(deliveryRequestSeconds)}, - {Name: "DELIVERY_MAX_PAUSE_SECONDS", Value: strconv.Itoa(deliveryMaxPause)}, - // Named for what it is, the FILE the projected identity is read - // from: a variable named *_TOKEN carrying a value is classified - // as a credential by the promotable ruleset, which refuses the - // manifest — and the value is a path, never the token. - {Name: "DELIVERY_IDENTITY_FILE", Value: deliveryIdentityMount + "/" + deliveryTokenFile}, - }, + Command: deliveryCommand(target, kind), + Env: deliveryEnvironment(target, kind, path), VolumeMounts: []deliveryVolumeMount{ {Name: "documents", MountPath: deliveryDocumentsMount, ReadOnly: true}, {Name: "token", MountPath: deliveryIdentityMount, ReadOnly: true}, diff --git a/pkg/gitops/delivery_target.go b/pkg/gitops/delivery_target.go index aa84a8fb..09d2008d 100644 --- a/pkg/gitops/delivery_target.go +++ b/pkg/gitops/delivery_target.go @@ -66,8 +66,20 @@ func resolveDeliveryTarget(ctx context.Context, workspace *resources.Workspace, if endpoint.Secured { scheme = httpsScheme } + address := serviceName + "." + namespace + ".svc.cluster.local:" + strconv.FormatUint(uint64(port), 10) + // Which half (or halves) may be delivered here is the HOST's declaration, + // read the one way the run already reads it. A render that guessed from + // the endpoint's api instead is how a connect-typed delivery API came to + // be posted a REST path it does not route. + composition, err := HostImplements(ctx, workspace, module, serviceName, endpointName, CompositionInterface) + if err != nil { + return nil, err + } return &DeliveryTarget{ - URL: scheme + "://" + serviceName + "." + namespace + ".svc.cluster.local:" + strconv.FormatUint(uint64(port), 10), - Audience: env.Host.Audience, + URL: scheme + "://" + address, + Audience: env.Host.Audience, + Address: address, + Composition: composition, + API: endpoint.API, }, nil } diff --git a/pkg/gitops/delivery_target_test.go b/pkg/gitops/delivery_target_test.go index 20ef8bd5..aff0bceb 100644 --- a/pkg/gitops/delivery_target_test.go +++ b/pkg/gitops/delivery_target_test.go @@ -53,7 +53,13 @@ func TestDeliveryTargetIsResolvedFromTheCompositionNeverDeclared(t *testing.T) { // a rest endpoint (8080), never the container port the service declares // (8443): the Job dials the Service, and the Service publishes the // allocated port in front of whatever the pod listens on. - require.Equal(t, &DeliveryTarget{URL: "https://accounts.acme-platform.svc.cluster.local:8080", Audience: "accounts"}, target) + // The address is the same endpoint without a scheme, which is what the + // paired runner dials; a host whose endpoint declares no + // codefly.dev/composition is not a paired target. + require.Equal(t, &DeliveryTarget{ + URL: "https://accounts.acme-platform.svc.cluster.local:8080", Audience: "accounts", + Address: "accounts.acme-platform.svc.cluster.local:8080", API: "rest", + }, target) plain := writeHostWorkspace(t, false, true) target, err = resolveDeliveryTarget(context.Background(), plain, selectedEnvironment(t, plain, "staging")) diff --git a/pkg/gitops/excluded_services_test.go b/pkg/gitops/excluded_services_test.go index 5d66d006..bddd627b 100644 --- a/pkg/gitops/excluded_services_test.go +++ b/pkg/gitops/excluded_services_test.go @@ -4,6 +4,7 @@ import ( "context" "os" "path/filepath" + "strings" "testing" "github.com/codefly-dev/cli/pkg/environments" @@ -116,6 +117,77 @@ func TestRenderModuleIsNotRefusedOverAnotherEnvironmentsExclusion(t *testing.T) "and the service the OTHER environment excludes is rendered here") } +// TestRenderModuleIsNotRefusedOverAnotherEnvironmentsScopedName is the Blue +// m106 defect, reproduced on released source: `deploy gitops render --all +// --env blue` failed with +// +// environment "staging" service-config references ambiguous service "object-storage" +// +// A bare service-scoped name is resolved against the WORKSPACE's service +// graph, so it becomes ambiguous the moment a second module composes a +// service of that name. The declaration that goes stale belongs to whichever +// environment wrote the bare key first — here `staging` — but every +// environment was held to it, so a correct render of `blue` was refused over +// a declaration that is not blue's and that blue cannot repair. +// +// Here `depot` composes a second service named `store`, which makes staging's +// bare `store` key ambiguous. Rendering `blue`, whose own declarations are +// clean, must succeed. +func TestRenderModuleIsNotRefusedOverAnotherEnvironmentsScopedName(t *testing.T) { + installFakeAgents(t) + root := t.TempDir() + require.NoError(t, copyTree(filepath.Join("testdata", "container-ports"), root)) + + // A second module composing a service named "store": this is what makes + // the bare name ambiguous across the workspace. + depot := filepath.Join(root, "modules", "depot") + require.NoError(t, os.MkdirAll(filepath.Join(depot, "services", "store"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(depot, "module.codefly.yaml"), + []byte("kind: module\nname: depot\nservices:\n - name: store\n"), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(depot, "services", "store", "service.codefly.yaml"), + []byte("kind: service\nname: store\nversion: 0.0.1\n"+ + "agent:\n kind: codefly:service\n name: postgres\n version: 0.0.1\n publisher: codefly.dev\n"+ + "endpoints:\n - name: tcp\n api: tcp\n"), 0o644)) + + path := filepath.Join(root, resources.WorkspaceConfigurationName) + data, err := os.ReadFile(path) + require.NoError(t, err) + data = []byte(strings.Replace(string(data), "modules:\n - name: shop\n", + "modules:\n - name: shop\n - name: depot\n", 1)) + // staging carries the stale bare key; blue declares nothing of the sort. + data = append(data, []byte(` service-config: + services: + store: + values: + DATABASE_HOST: store.database.example + - name: blue + namespace: acme + registry: + url: registry.example.com/acme +`)...) + require.NoError(t, os.WriteFile(path, data, 0o644)) + + ctx := context.Background() + workspace, err := resources.LoadWorkspaceFromDir(ctx, root) + require.NoError(t, err) + module, err := workspace.LoadModuleFromName(ctx, "shop") + require.NoError(t, err) + + // staging really is ambiguous against the graph: selecting it refuses. + // Without this the next assertion could pass for the wrong reason. + staging, err := orchestration.SelectEnvironment(workspace, "staging") + require.NoError(t, err) + _, err = RenderModule(ctx, workspace, module, staging, "acme-staging", nil) + require.ErrorContains(t, err, `references ambiguous service "store"`, + "the environment that owns the stale declaration must still be refused") + + // blue must not inherit it. + blue, err := orchestration.SelectEnvironment(workspace, "blue") + require.NoError(t, err) + _, err = RenderModule(ctx, workspace, module, blue, "acme-blue", nil) + require.NoError(t, err, "a render is held to its own environment's declarations, never another's") +} + // A module the environment withdraws whole renders nothing there: the render // refuses it by name instead of producing an empty tree. func TestRenderModuleRefusesAModuleTheEnvironmentWithdrawsWhole(t *testing.T) { diff --git a/pkg/gitops/method_policies_test.go b/pkg/gitops/method_policies_test.go index a19dd176..02a47087 100644 --- a/pkg/gitops/method_policies_test.go +++ b/pkg/gitops/method_policies_test.go @@ -158,7 +158,7 @@ func TestTheAuthorityDocumentIsSignedAndBoundToItsManifest(t *testing.T) { return ceilings, nil }, func(_ context.Context, module string) ([]*compositionv1.RoleV1, error) { return roles, nil - }, nil) + }, nil, nil) manifest := &solutionhost.ModuleManifest{ Schema: solutionhost.SchemaModuleManifestV1, Binding: "console", Alias: "console", Kind: solutionhost.KindModule, Generation: 3, Host: solutionhost.HostTarget{Coordinate: "local", Domain: "local"}, Release: solutionhost.Release{Publisher: "test", Name: "console", Version: "0.1.0"}, diff --git a/pkg/gitops/module_manifest.go b/pkg/gitops/module_manifest.go index c6da1867..6c95eeed 100644 --- a/pkg/gitops/module_manifest.go +++ b/pkg/gitops/module_manifest.go @@ -47,6 +47,12 @@ const ( // binding the document declares. documentLabel = "codefly.dev/document" moduleManifestKind = "module-manifest" + // pairedDeliveryKind is its value for a PAIRED delivery document: the + // module manifest's signed carrier and the authority declaration signed + // against it, in the one request the composition interface takes. It is a + // separate kind because a reader of a delivered overlay must tell the two + // apart — a paired document is not a manifest and does not parse as one. + pairedDeliveryKind = "paired-delivery" bindingLabel = "codefly.dev/binding" managedByLabel = "app.kubernetes.io/managed-by" managedByCodefly = "codefly" diff --git a/pkg/gitops/owner_aware_test.go b/pkg/gitops/owner_aware_test.go index 095f963c..228ae8c2 100644 --- a/pkg/gitops/owner_aware_test.go +++ b/pkg/gitops/owner_aware_test.go @@ -101,7 +101,7 @@ func issuedScopes(in []*authorityv1.WorkScopeV1) []*basev0.WorkScopeV1 { func TestOwnerAwareReadFailureStopsDelivery(t *testing.T) { dir := writeContract(t, ownedContract+"owner_aware: yes\n") d := NewCompositionDelivery(nil, nativeSigner(t), nil, nil, nil, - func(context.Context, string) (bool, error) { return OwnerAware(dir) }) + func(context.Context, string) (bool, error) { return OwnerAware(dir) }, nil) _, err := d.Prepare(t.Context(), "shop", false) require.ErrorContains(t, err, "owner_aware") for _, dir := range []string{"", t.TempDir()} { diff --git a/pkg/gitops/paired_apply.go b/pkg/gitops/paired_apply.go new file mode 100644 index 00000000..c746464e --- /dev/null +++ b/pkg/gitops/paired_apply.go @@ -0,0 +1,333 @@ +package gitops + +import ( + "context" + "errors" + "fmt" + "os" + "path/filepath" + "sort" + "strings" + "time" + + "google.golang.org/protobuf/encoding/protojson" + + "github.com/codefly-dev/core/solutionhost" + + compositionv1 "github.com/codefly-dev/authority/libraries/authority-client/go/gen/host/composition/v1" +) + +// --- Applying prepared paired documents --- +// +// A rendered delivery carries BOTH signed documents: core's module manifest in +// its DSSE carrier, and the authority declaration — method policies, scope +// ceilings, roles, owner-awareness, the installation selector and its +// person-context edges — signed against that carrier's exact bytes. Only the +// composition interface takes the pair (CompositionDelivery); the host's +// legacy HTTP manifest route takes the Core half alone and answers for it, so +// a module delivered that way reaches the host with no roles at all. +// +// The pair is settled and signed where release custody is — at publish — and +// SENT from inside the cell, after the module's workloads exist. Those are two +// different moments and two different places, so the request travels as a +// prepared document and one field of it is deliberately left empty: the +// CREDENTIAL. A delivery credential is a projected ServiceAccount token with a +// lifetime of minutes; a token written into a rendered manifest would be a +// published secret and would be expired by the time the manifest synced. So +// the prepared document carries `credential: ""` and the sender fills it from +// the projection AT EXECUTION, once per request, so a token rotated between +// rounds is the one the next round sends. +// +// The token is read into the request and nothing else: it is never printed, +// never in an argument, never in an error. A refusal names the document, the +// code and the host's reason. + +// PreparedDeliveryDocument is the file name suffix of a prepared paired +// request. The sender reads every one of them in its directory, in name order, +// so what is sent is what the render wrote and the order is the render's. +const PreparedDeliveryDocument = ".json" + +// PairedApply sends prepared paired requests to one host's composition +// interface. Its retry shape is the delivery Job's: every pending document +// once per round, then a pause that doubles up to MaxPause, rounds until each +// is answered or Budget is spent — so a document the host keeps failing never +// keeps a later one, a withdrawal among them, from its first attempt. +type PairedApply struct { + // Client is the generated composition client, over any transport that + // carries it (the host's connect endpoint serves gRPC on the same + // listener). + Client compositionv1.CompositionServiceClient + // Credential reads the delivery credential. Called once per request, not + // once per run, so a rotated projection is re-read. + Credential func() (string, error) + // Budget bounds the whole run, Request one call, MaxPause the pause + // between rounds. + Budget, Request, MaxPause time.Duration + // Now and Sleep are the clock, for a test that does not wait. + Now func() time.Time + Sleep func(context.Context, time.Duration) error + // Report is called once per answered document: accepted, replayed or + // refused, with the host's reason. nil prints nothing. + Report func(string) +} + +// PairedReceipt is one document's outcome. +type PairedReceipt struct { + Document string + Binding string + Generation uint64 + Accepted bool + // Err is the host's refusal, or the reason the run gave up on it. + Err error +} + +// defaults for a sender a caller configured only partly. +const ( + pairedDefaultBudget = 1500 * time.Second + pairedDefaultRequest = 30 * time.Second + pairedDefaultMaxPause = 60 * time.Second +) + +// Apply sends every prepared document in directory and answers one receipt per +// document, in the order the documents were read. The error is non-nil when any +// document was refused or left unanswered: a delivery that did not land must +// fail the caller that ran it, since nothing else will notice. +func (a *PairedApply) Apply(ctx context.Context, directory string) ([]PairedReceipt, error) { + documents, err := readPreparedDocuments(directory) + if err != nil { + return nil, err + } + if a.Client == nil { + return nil, errors.New("a paired delivery needs the host's composition client") + } + if a.Credential == nil { + return nil, errors.New("a paired delivery needs the delivery credential, read at execution") + } + now, sleep := a.Now, a.Sleep + if now == nil { + now = time.Now + } + if sleep == nil { + sleep = func(ctx context.Context, d time.Duration) error { + timer := time.NewTimer(d) + defer timer.Stop() + select { + case <-ctx.Done(): + return ctx.Err() + case <-timer.C: + return nil + } + } + } + budget, request, maxPause := a.Budget, a.Request, a.MaxPause + if budget <= 0 { + budget = pairedDefaultBudget + } + if request <= 0 { + request = pairedDefaultRequest + } + if maxPause <= 0 { + maxPause = pairedDefaultMaxPause + } + started := now() + left := func() time.Duration { return budget - now().Sub(started) } + receipts := make([]PairedReceipt, len(documents)) + for i := range documents { + receipts[i] = PairedReceipt{Document: documents[i].name, Binding: documents[i].binding, Generation: documents[i].generation} + } + pending := make([]int, len(documents)) + for i := range pending { + pending[i] = i + } + pause := time.Second + for round := 1; len(pending) > 0; round++ { + var again []int + for _, i := range pending { + remaining := left() + if remaining <= 0 { + again = append(again, i) + continue + } + timeout := request + if timeout > remaining { + timeout = remaining + } + receipt := a.send(ctx, documents[i], timeout) + switch { + case receipt.Err == nil: + a.say(fmt.Sprintf("delivered %s: binding %s at generation %d", receipt.Document, receipt.Binding, receipt.Generation)) + case retryablePairedRefusal(receipt.Err): + a.say(fmt.Sprintf("the composition interface answered %v for %s in round %d; it is tried again next round", receipt.Err, receipt.Document, round)) + again = append(again, i) + continue + default: + a.say(fmt.Sprintf("refused %s: %v", receipt.Document, receipt.Err)) + } + receipts[i] = receipt + if err := ctx.Err(); err != nil { + return receipts, err + } + } + pending = again + if len(pending) == 0 { + break + } + remaining := left() + if remaining <= 0 { + for _, i := range pending { + receipts[i].Err = fmt.Errorf("giving up on %s after %d rounds within %s", documents[i].name, round, budget) + a.say(receipts[i].Err.Error()) + } + break + } + nap := pause + if nap > remaining { + nap = remaining + } + if err := sleep(ctx, nap); err != nil { + return receipts, err + } + if pause *= 2; pause > maxPause { + pause = maxPause + } + } + var refused []string + for _, receipt := range receipts { + if receipt.Err != nil { + refused = append(refused, receipt.Document) + } + } + if len(refused) > 0 { + return receipts, fmt.Errorf("the host did not accept %s", strings.Join(refused, ", ")) + } + return receipts, nil +} + +// send fills the credential and applies one document. The credential is read +// here, per request, so a projection rotated between rounds is re-read, and it +// is set on a COPY of the prepared request so no token is retained after the +// call. +func (a *PairedApply) send(ctx context.Context, document preparedDocument, timeout time.Duration) PairedReceipt { + receipt := PairedReceipt{Document: document.name, Binding: document.binding, Generation: document.generation} + credential, err := a.Credential() + if err != nil { + receipt.Err = fmt.Errorf("read the delivery credential: %w", err) + return receipt + } + if credential == "" { + receipt.Err = errors.New("the delivery credential is empty; the projected token is absent or not yet written") + return receipt + } + request := &compositionv1.ApplyManifestRequest{ + Carrier: document.request.GetCarrier(), Authority: document.request.GetAuthority(), + // RAW: the host adds the bearer scheme itself before it authorizes + // the carrier, so a scheme added here is a second one. + Credential: credential, + } + call, cancel := context.WithTimeout(ctx, timeout) + defer cancel() + out, err := a.Client.ApplyManifest(call, request) + if err != nil { + receipt.Err = refusal(document.binding, document.manifest, err) + return receipt + } + if out.GetGeneration() != document.generation { + receipt.Err = fmt.Errorf("the host acknowledged generation %d for binding %s, sent %d", out.GetGeneration(), document.binding, document.generation) + return receipt + } + receipt.Accepted = true + return receipt +} + +func (a *PairedApply) say(line string) { + if a.Report != nil { + a.Report(line) + } +} + +// retryablePairedRefusal reports whether asking again can answer differently. +// It reads the ONE classification the run already uses (HostStatusError +// .Transient): unavailable, out of time, out of capacity. Every other code is +// the host's answer — an invalid document, a refused carrier, a stale or +// rewritten generation, a withdrawal it keeps terminal — and asking again gets +// the same one, so it refuses the document rather than spending the budget on +// it. +func retryablePairedRefusal(err error) bool { + var refusal *HostStatusError + if errors.As(err, &refusal) { + return refusal.Transient() + } + return false +} + +// preparedDocument is one prepared request as read from disk, with the +// manifest its carrier holds: the binding and generation are read from the +// SIGNED document, never from the file's name or from an unsigned field, so +// what the sender reports is what the host verifies. +type preparedDocument struct { + name string + request *compositionv1.ApplyManifestRequest + manifest *solutionhost.ModuleManifest + binding string + generation uint64 +} + +// readPreparedDocuments reads every prepared request in directory, in name +// order. A directory with no document is an error: a delivery that carries +// nothing has gone wrong upstream, and silently succeeding would report a +// delivery that never happened. +func readPreparedDocuments(directory string) ([]preparedDocument, error) { + entries, err := os.ReadDir(directory) + if err != nil { + return nil, fmt.Errorf("read the prepared delivery documents: %w", err) + } + var names []string + for _, entry := range entries { + if !entry.IsDir() && strings.HasSuffix(entry.Name(), PreparedDeliveryDocument) { + names = append(names, entry.Name()) + } + } + sort.Strings(names) + if len(names) == 0 { + return nil, fmt.Errorf("no prepared delivery document under %s", directory) + } + documents := make([]preparedDocument, 0, len(names)) + for _, name := range names { + data, readErr := os.ReadFile(filepath.Join(directory, name)) + if readErr != nil { + return nil, readErr + } + document, parseErr := parsePreparedDocument(name, data) + if parseErr != nil { + return nil, parseErr + } + documents = append(documents, document) + } + return documents, nil +} + +// parsePreparedDocument decodes one prepared request and refuses what the host +// would refuse later and further from the cause: a request carrying no +// carrier, a carrier whose document is not a module manifest, or a credential +// already written into the rendered document — which would be a published +// secret, so it is refused here rather than sent. +func parsePreparedDocument(name string, data []byte) (preparedDocument, error) { + var request compositionv1.ApplyManifestRequest + if err := (protojson.UnmarshalOptions{DiscardUnknown: false}).Unmarshal(data, &request); err != nil { + return preparedDocument{}, fmt.Errorf("%s is not a prepared delivery request: %w", name, err) + } + if request.GetCredential() != "" { + return preparedDocument{}, fmt.Errorf("%s carries a credential; a prepared delivery document never holds one, and the sender reads it from the projection at execution", name) + } + if request.GetCarrier() == nil { + return preparedDocument{}, fmt.Errorf("%s carries no signed module manifest", name) + } + manifest, err := solutionhost.ManifestFromVerified(request.GetCarrier().GetDocument()) + if err != nil { + return preparedDocument{}, fmt.Errorf("%s: %w", name, err) + } + return preparedDocument{ + name: name, request: &request, manifest: manifest, + binding: manifest.Binding, generation: manifest.Generation, + }, nil +} diff --git a/pkg/gitops/paired_apply_test.go b/pkg/gitops/paired_apply_test.go new file mode 100644 index 00000000..5c70ad9e --- /dev/null +++ b/pkg/gitops/paired_apply_test.go @@ -0,0 +1,290 @@ +package gitops + +import ( + "context" + "net" + "os" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/codefly-dev/core/solutionhost" + "github.com/stretchr/testify/require" + "google.golang.org/grpc" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/credentials/insecure" + "google.golang.org/grpc/status" + "google.golang.org/grpc/test/bufconn" + "google.golang.org/protobuf/encoding/protojson" + "google.golang.org/protobuf/proto" + + compositionv1 "github.com/codefly-dev/authority/libraries/authority-client/go/gen/host/composition/v1" +) + +// applyHost is a host that records exactly what reached ApplyManifest and can +// be told to answer the way a real one does: not serving yet, or its own +// refusal. It is a real gRPC server over bufconn with the generated client in +// front of it, so what these tests exercise is the wire the Job uses. +type applyHost struct { + compositionv1.UnimplementedCompositionServiceServer + seen []*compositionv1.ApplyManifestRequest + // answers is consumed one per call; a nil entry accepts. + answers []error +} + +func (h *applyHost) ApplyManifest(_ context.Context, request *compositionv1.ApplyManifestRequest) (*compositionv1.ApplyManifestResponse, error) { + h.seen = append(h.seen, proto.Clone(request).(*compositionv1.ApplyManifestRequest)) + if len(h.answers) > 0 { + answer := h.answers[0] + h.answers = h.answers[1:] + if answer != nil { + return nil, answer + } + } + manifest, err := solutionhost.ManifestFromVerified(request.GetCarrier().GetDocument()) + if err != nil { + return nil, status.Error(codes.InvalidArgument, err.Error()) + } + return &compositionv1.ApplyManifestResponse{Principal: manifest.Alias, Generation: manifest.Generation}, nil +} + +func applyHostClient(t *testing.T, host compositionv1.CompositionServiceServer) compositionv1.CompositionServiceClient { + t.Helper() + server := grpc.NewServer() + compositionv1.RegisterCompositionServiceServer(server, host) + listener := bufconn.Listen(1 << 20) + go func() { _ = server.Serve(listener) }() + t.Cleanup(server.Stop) + connection, err := grpc.NewClient("passthrough:///bufconn", + grpc.WithContextDialer(func(context.Context, string) (net.Conn, error) { return listener.Dial() }), + grpc.WithTransportCredentials(insecure.NewCredentials())) + require.NoError(t, err) + t.Cleanup(func() { _ = connection.Close() }) + return compositionv1.NewCompositionServiceClient(connection) +} + +// preparedDocumentOf writes one prepared paired request: the Core carrier and +// the authority document beside it, and NO credential — the shape a render +// writes and the sender completes at execution. +func preparedDocumentOf(t *testing.T, directory, name, binding string, generation uint64, authority []byte) { + t.Helper() + manifest := &solutionhost.ModuleManifest{ + Schema: solutionhost.SchemaModuleManifestV1, Binding: binding, Alias: binding, Kind: solutionhost.KindModule, Generation: generation, + Host: solutionhost.HostTarget{Coordinate: "local", Domain: "local"}, + Release: solutionhost.Release{Publisher: "test", Name: binding, Version: "0.1.0"}, + Workloads: []solutionhost.Workload{{Service: "api", Container: "app"}}, + Authority: &solutionhost.Authority{Principal: binding}, + } + document, err := solutionhost.SignedPayload(manifest) + require.NoError(t, err) + request := &compositionv1.ApplyManifestRequest{ + Carrier: &compositionv1.SignedManifestV1{Document: document, Envelope: []byte(`{"envelope":"` + binding + `"}`)}, + } + if authority != nil { + request.Authority = &compositionv1.SignedManifestV1{Document: authority, Envelope: []byte(`{"envelope":"authority"}`)} + } + data, err := protojson.Marshal(request) + require.NoError(t, err) + require.NoError(t, os.WriteFile(filepath.Join(directory, name), data, 0o600)) +} + +func noPause(context.Context, time.Duration) error { return nil } + +// The whole point of the runner: both signed halves reach the host, and the +// credential is not in the document — it is read from the projection at +// execution, once per request, raw. +func TestTheRunnerSendsBothSignedHalvesWithACredentialReadAtExecution(t *testing.T) { + directory := t.TempDir() + preparedDocumentOf(t, directory, "a-billing.json", "billing", 3, []byte("authority-of-billing")) + preparedDocumentOf(t, directory, "b-shop.json", "shop", 1, []byte("authority-of-shop")) + host := &applyHost{} + reads := 0 + sender := &PairedApply{ + Client: applyHostClient(t, host), + Credential: func() (string, error) { + reads++ + return "projected-token", nil + }, + Sleep: noPause, + } + receipts, err := sender.Apply(context.Background(), directory) + require.NoError(t, err) + require.Len(t, receipts, 2) + require.True(t, receipts[0].Accepted) + require.True(t, receipts[1].Accepted) + require.Equal(t, []string{"billing", "shop"}, []string{receipts[0].Binding, receipts[1].Binding}, + "documents are sent in name order, which is the render's order") + require.Len(t, host.seen, 2) + for i, want := range []string{"authority-of-billing", "authority-of-shop"} { + require.NotNil(t, host.seen[i].GetCarrier(), "the Core signed carrier") + require.Equal(t, want, string(host.seen[i].GetAuthority().GetDocument()), + "and the authority document beside it — the half the legacy HTTP route cannot carry") + require.Equal(t, "projected-token", host.seen[i].GetCredential(), + "raw, with no bearer scheme: the host adds it before authorizing the carrier") + } + require.Equal(t, 2, reads, "read once per REQUEST, not once per run, so a rotated projection is re-read") +} + +// A credential in a rendered document would be a published secret with a +// lifetime of minutes. It is refused where it is read, not sent and refused at +// the host. +func TestAPreparedDocumentHoldingACredentialIsRefusedRatherThanSent(t *testing.T) { + directory := t.TempDir() + request := &compositionv1.ApplyManifestRequest{ + Carrier: &compositionv1.SignedManifestV1{Document: []byte("{}"), Envelope: []byte("{}")}, + Credential: "a-token-that-was-rendered", + } + data, err := protojson.Marshal(request) + require.NoError(t, err) + require.NoError(t, os.WriteFile(filepath.Join(directory, "leaky.json"), data, 0o600)) + host := &applyHost{} + sender := &PairedApply{Client: applyHostClient(t, host), Credential: func() (string, error) { return "t", nil }, Sleep: noPause} + _, err = sender.Apply(context.Background(), directory) + require.ErrorContains(t, err, "carries a credential") + require.Empty(t, host.seen, "nothing is sent once a document is refused") +} + +// A refusal the host MEANS is its answer, and asking again gets the same one. +// FailedPrecondition is how core's terminal withdrawal and a stale generation +// arrive, and it is the status a Connect transport renders as 412 — the one the +// shell runner retried until its budget was spent. +func TestTheHostsOwnRefusalIsAnsweredOnceAndNotRetried(t *testing.T) { + directory := t.TempDir() + preparedDocumentOf(t, directory, "one.json", "billing", 2, []byte("authority")) + host := &applyHost{answers: []error{status.Error(codes.FailedPrecondition, "generation 2 does not move billing forward")}} + sender := &PairedApply{ + Client: applyHostClient(t, host), Credential: func() (string, error) { return "t", nil }, + Sleep: noPause, Budget: time.Hour, + } + receipts, err := sender.Apply(context.Background(), directory) + require.ErrorContains(t, err, "one.json") + require.Len(t, host.seen, 1, "asked once; the host's answer is not a reason to ask again") + require.False(t, receipts[0].Accepted) + require.ErrorContains(t, receipts[0].Err, "does not move billing forward") +} + +// "Not serving yet" is the one answer worth asking again for. +func TestNotServingYetIsRetriedUntilTheHostAnswers(t *testing.T) { + directory := t.TempDir() + preparedDocumentOf(t, directory, "one.json", "billing", 2, []byte("authority")) + host := &applyHost{answers: []error{ + status.Error(codes.Unavailable, "not serving yet"), + status.Error(codes.DeadlineExceeded, "out of time"), + nil, + }} + sender := &PairedApply{ + Client: applyHostClient(t, host), Credential: func() (string, error) { return "t", nil }, + Sleep: noPause, Budget: time.Hour, + } + receipts, err := sender.Apply(context.Background(), directory) + require.NoError(t, err) + require.Len(t, host.seen, 3) + require.True(t, receipts[0].Accepted) +} + +// The credential is read per request, so a projection rotated between rounds +// is the one the next round sends. This is why it is not read once at startup. +func TestARotatedProjectionIsTheOneTheNextRoundSends(t *testing.T) { + directory := t.TempDir() + preparedDocumentOf(t, directory, "one.json", "billing", 2, []byte("authority")) + host := &applyHost{answers: []error{status.Error(codes.Unavailable, "not serving yet"), nil}} + tokens := []string{"first-token", "rotated-token"} + sender := &PairedApply{ + Client: applyHostClient(t, host), + Credential: func() (string, error) { + token := tokens[0] + if len(tokens) > 1 { + tokens = tokens[1:] + } + return token, nil + }, + Sleep: noPause, Budget: time.Hour, + } + _, err := sender.Apply(context.Background(), directory) + require.NoError(t, err) + require.Len(t, host.seen, 2) + require.Equal(t, "first-token", host.seen[0].GetCredential()) + require.Equal(t, "rotated-token", host.seen[1].GetCredential(), "the round after a rotation sends the new token") +} + +// A document still unanswered when the budget is spent is a refusal, not a +// silent pass: a delivery that did not land must fail the Job that ran it. +func TestADocumentNeverAnsweredFailsTheRun(t *testing.T) { + directory := t.TempDir() + preparedDocumentOf(t, directory, "one.json", "billing", 2, []byte("authority")) + host := &applyHost{answers: []error{status.Error(codes.Unavailable, "not serving yet")}} + now := time.Now() + clock := now + sender := &PairedApply{ + Client: applyHostClient(t, host), Credential: func() (string, error) { return "t", nil }, + Budget: 10 * time.Second, + Now: func() time.Time { return clock }, + Sleep: func(_ context.Context, d time.Duration) error { + clock = clock.Add(d + 10*time.Second) + return nil + }, + } + receipts, err := sender.Apply(context.Background(), directory) + require.ErrorContains(t, err, "one.json") + require.ErrorContains(t, receipts[0].Err, "giving up on one.json") + require.False(t, receipts[0].Accepted) +} + +// The generation is read from the SIGNED document, and a host acknowledging a +// different one is a disagreement about what was applied, not a success. +func TestAGenerationTheHostAcknowledgesDifferentlyIsRefused(t *testing.T) { + directory := t.TempDir() + preparedDocumentOf(t, directory, "one.json", "billing", 4, []byte("authority")) + sender := &PairedApply{Client: applyHostClient(t, &shiftedHost{applyHost: &applyHost{}}), Credential: func() (string, error) { return "t", nil }, Sleep: noPause} + _, err := sender.Apply(context.Background(), directory) + require.ErrorContains(t, err, "one.json") +} + +// shiftedHost acknowledges a generation one higher than it was sent. +type shiftedHost struct{ *applyHost } + +func (h *shiftedHost) ApplyManifest(ctx context.Context, request *compositionv1.ApplyManifestRequest) (*compositionv1.ApplyManifestResponse, error) { + out, err := h.applyHost.ApplyManifest(ctx, request) + if err != nil { + return nil, err + } + out.Generation++ + return out, nil +} + +// A directory with no document is not an empty success: a delivery that +// carries nothing has gone wrong upstream. +func TestADeliveryThatCarriesNothingIsRefused(t *testing.T) { + sender := &PairedApply{Client: applyHostClient(t, &applyHost{}), Credential: func() (string, error) { return "t", nil }} + _, err := sender.Apply(context.Background(), t.TempDir()) + require.ErrorContains(t, err, "no prepared delivery document") +} + +// A projection not yet written is a refusal that names the cause, and no +// request is made with an empty credential. +func TestAnAbsentProjectionRefusesBeforeTheCall(t *testing.T) { + directory := t.TempDir() + preparedDocumentOf(t, directory, "one.json", "billing", 2, nil) + host := &applyHost{} + sender := &PairedApply{Client: applyHostClient(t, host), Credential: func() (string, error) { return "", nil }, Sleep: noPause} + receipts, err := sender.Apply(context.Background(), directory) + require.Error(t, err) + require.ErrorContains(t, receipts[0].Err, "the delivery credential is empty") + require.Empty(t, host.seen) +} + +// The token never reaches a report line: the Job's log is a published record. +func TestTheCredentialIsNeverReported(t *testing.T) { + directory := t.TempDir() + preparedDocumentOf(t, directory, "one.json", "billing", 2, []byte("authority")) + var log strings.Builder + sender := &PairedApply{ + Client: applyHostClient(t, &applyHost{}), Credential: func() (string, error) { return "super-secret-token", nil }, + Sleep: noPause, Report: func(line string) { log.WriteString(line + "\n") }, + } + _, err := sender.Apply(context.Background(), directory) + require.NoError(t, err) + require.Contains(t, log.String(), "delivered one.json") + require.NotContains(t, log.String(), "super-secret-token") +} diff --git a/pkg/gitops/paired_delivery_job_test.go b/pkg/gitops/paired_delivery_job_test.go new file mode 100644 index 00000000..f61f9863 --- /dev/null +++ b/pkg/gitops/paired_delivery_job_test.go @@ -0,0 +1,278 @@ +package gitops + +import ( + "context" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/codefly-dev/cli/pkg/environments" + "github.com/codefly-dev/core/solutionhost" + "github.com/stretchr/testify/require" + "google.golang.org/protobuf/encoding/protojson" + "google.golang.org/protobuf/proto" + "gopkg.in/yaml.v3" + + compositionv1 "github.com/codefly-dev/authority/libraries/authority-client/go/gen/host/composition/v1" +) + +// compositionTarget is a delivery endpoint that declares +// codefly.dev/composition: the host takes both signed halves through +// ApplyManifest at its gRPC address. +func compositionTarget() *DeliveryTarget { + return &DeliveryTarget{ + URL: "http://accounts.platform.svc.cluster.local:8081", Audience: "accounts", + Address: "accounts.platform.svc.cluster.local:8081", Composition: true, API: "connect", + } +} + +// pairedOptions are publishOptions toward a composition host, reading every +// module's declaration as one that declares the three roles. +func pairedOptions(signer *fakeSigner) deliveryPublishOptions { + opts := publishOptions(signer) + opts.Target = compositionTarget() + opts.Declarations = &AuthoritySources{Roles: func(context.Context, string) ([]*compositionv1.RoleV1, error) { + return []*compositionv1.RoleV1{{Name: "installer"}, {Name: "editor"}, {Name: "viewer"}}, nil + }} + return opts +} + +// overlayFile reads one file of the settled delivery overlay. +func overlayFile(t *testing.T, repository *deliveryRepository, name string) []byte { + t.Helper() + data, err := os.ReadFile(filepath.Join(repository.target, filepath.FromSlash(moduleManifestOverlay("prod")), name)) + require.NoError(t, err) + return data +} + +// deliveredJob reads the delivery Job of one kind from the settled overlay. +func deliveredJob(t *testing.T, repository *deliveryRepository, kind string) deliveryContainer { + t.Helper() + var job deliveryJobManifest + require.NoError(t, yaml.Unmarshal(overlayFile(t, repository, "deliver-"+kind+".yaml"), &job)) + require.Len(t, job.Spec.Template.Spec.Containers, 1) + return job.Spec.Template.Spec.Containers[0] +} + +// pairedDocumentsIn reads every paired ConfigMap of the settled overlay, keyed +// by binding. +func pairedDocumentsIn(t *testing.T, repository *deliveryRepository) map[string]*compositionv1.ApplyManifestRequest { + t.Helper() + documents := map[string]*compositionv1.ApplyManifestRequest{} + for name, configMap := range settledDocuments(t, repository) { + if configMap.Metadata.Labels[documentLabel] != pairedDeliveryKind { + continue + } + require.Equal(t, pairedDocumentConfigMapName(configMap.Metadata.Labels[bindingLabel]), configMap.Metadata.Name) + require.Equal(t, configMap.Metadata.Labels[bindingLabel]+"-paired", name) + var request compositionv1.ApplyManifestRequest + require.NoError(t, protojson.Unmarshal([]byte(configMap.Data[PairedDocumentKey]), &request)) + require.Empty(t, request.GetCredential(), "a rendered document never holds a credential") + documents[configMap.Metadata.Labels[bindingLabel]] = &request + } + return documents +} + +// A publish toward a composition host writes one paired document per manifest +// — the manifest's own signed carrier with the authority half beside it, roles +// included — and a Job that runs codefly's paired runner from the paired image, +// with no shell and none of the shell runner's environment. A withdrawal is +// still delivered, Core-only, because a removed module declares nothing. +func TestAPairedPublishWritesAPairedDocumentPerManifestAndTheCodeflyJob(t *testing.T) { + repository := newDeliveryRepository(t) + signer := &fakeSigner{} + opts := pairedOptions(signer) + _, err := repository.settle(repository.stageRender(t, "billing"), &opts) + require.NoError(t, err) + repository.deliver(t) + + _, err = repository.settle(repository.stageRender(t, "crm"), &opts) + require.NoError(t, err) + documents := pairedDocumentsIn(t, repository) + require.Len(t, documents, 2, "one paired document per manifest: the new crm and the billing withdrawal") + manifests := settledDocuments(t, repository) + for binding, request := range documents { + require.Equal(t, []byte(manifests[binding].Data[solutionhost.SignedFileName]), carrierBytes(t, request), + "the Core half of %s is the manifest's own signed carrier", binding) + } + var policy compositionv1.AuthorityManifestV1 + require.NotNil(t, documents["example.prod.crm"].GetAuthority()) + require.NoError(t, proto.Unmarshal(documents["example.prod.crm"].GetAuthority().GetDocument(), &policy)) + require.Len(t, policy.GetRoles(), 3, "the roles reach the host on the authority half") + require.Nil(t, documents["example.prod.billing"].GetAuthority(), "a withdrawal carries no authority half") + kustomization := string(overlayFile(t, repository, kustomizationFile)) + for binding := range documents { + require.Contains(t, kustomization, pairedDocumentFile(binding), "Argo applies what the kustomization names") + } + + container := deliveredJob(t, repository, deliveryManifest) + require.Equal(t, pairedDeliveryImage, container.Image) + require.Equal(t, []string{ + "/bin/codefly", "delivery", "apply", + "--documents", deliveryDocumentsMount, + "--address", compositionTarget().Address, + "--identity-file", deliveryIdentityMount + "/" + deliveryTokenFile, + }, container.Command[:9]) + require.Empty(t, container.Env, "a paired Job's inputs are its arguments; no DELIVERY_* variable") + jobBytes := string(overlayFile(t, repository, "deliver-"+deliveryManifest+".yaml")) + for binding := range documents { + require.Contains(t, jobBytes, "name: "+pairedDocumentConfigMapName(binding), "the Job mounts the paired ConfigMap") + } + require.Contains(t, jobBytes, "key: "+PairedDocumentKey) + require.NotContains(t, jobBytes, "key: "+solutionhost.SignedFileName, "the Core-only carrier is not what a paired Job sends") +} + +// carrierBytes re-encodes a paired document's Core half as the carrier the +// manifest ConfigMap holds, so the two can be compared byte for byte. +func carrierBytes(t *testing.T, request *compositionv1.ApplyManifestRequest) []byte { + t.Helper() + signed, err := solutionhost.Carrier(request.GetCarrier().GetDocument(), request.GetCarrier().GetEnvelope()) + require.NoError(t, err) + carrier, err := solutionhost.MarshalSigned(signed) + require.NoError(t, err) + return carrier +} + +// A host that declares no composition interface keeps the shell runner: the +// Core-only carrier POSTed over HTTP, from the shell image, and no paired +// document is written. +func TestANonPairedPublishKeepsTheShellRunner(t *testing.T) { + repository := newDeliveryRepository(t) + opts := publishOptions(&fakeSigner{}) + _, err := repository.settle(repository.stageRender(t, "crm"), &opts) + require.NoError(t, err) + require.Empty(t, pairedDocumentsIn(t, repository)) + container := deliveredJob(t, repository, deliveryManifest) + require.Equal(t, deliveryImage, container.Image) + require.Equal(t, []string{"/bin/sh", "-ec", deliveryScript}, container.Command) + require.Contains(t, container.Env, deliveryEnv{Name: "DELIVERY_PATH", Value: manifestDeliveryPath}) + require.Contains(t, string(overlayFile(t, repository, "deliver-"+deliveryManifest+".yaml")), "key: "+solutionhost.SignedFileName) +} + +// A paired publish that cannot read declarations refuses by name rather than +// deliver the Core half alone, which would reach the host with no roles and +// look like a success. +func TestAPairedPublishWithNoDeclarationsIsRefused(t *testing.T) { + repository := newDeliveryRepository(t) + opts := pairedOptions(&fakeSigner{}) + opts.Declarations = nil + _, err := repository.settle(repository.stageRender(t, "crm"), &opts) + require.ErrorContains(t, err, CompositionInterface) + require.ErrorContains(t, err, compositionTarget().Address) + require.ErrorContains(t, err, "will not deliver the Core half alone") +} + +// The composition interface has no envelope method, so an envelope Job keeps +// the shell runner toward a composition host — but runs the paired IMAGE, +// because the cell admits one carrier per namespace and the host's namespace +// runs the manifest Job too. Both Jobs, in both modes, run exactly the image +// the cell declares for that namespace. +func TestEveryDeliveryJobRunsTheImageTheCellAdmits(t *testing.T) { + env := &environments.Environment{Name: "prod", Host: testHost()} + for _, paired := range []bool{false, true} { + target := testDeliveryTarget() + if paired { + target = compositionTarget() + } + declared, err := cellDelivery(env, "crm", paired) + require.NoError(t, err) + for kind, path := range map[string]string{deliveryManifest: manifestDeliveryPath, deliveredEnvelopeKind: envelopeDeliveryPath} { + directory := t.TempDir() + file, err := renderDeliveryJob(directory, deliveryJobName(kind, "crm", "prod", [][]byte{[]byte("carrier")}), "crm", + kind, path, target, []deliveryDocument{{ConfigMap: "documents", Key: "document.json", Name: "crm.json"}}) + require.NoError(t, err) + data, err := os.ReadFile(filepath.Join(directory, file)) + require.NoError(t, err) + var job deliveryJobManifest + require.NoError(t, yaml.Unmarshal(data, &job)) + container := job.Spec.Template.Spec.Containers[0] + require.Equal(t, declared.Image.Repository+"@"+declared.Image.Digest, container.Image, + "paired=%v %s: the Job runs the image the cell admits", paired, kind) + require.Equal(t, declared.Selector, job.Spec.Template.Metadata.Labels) + runsCodefly := strings.HasPrefix(strings.Join(container.Command, " "), "/bin/codefly delivery apply") + require.Equal(t, paired && kind == deliveryManifest, runsCodefly, + "paired=%v %s: only a manifest has an ApplyManifest; an envelope is POSTed by the shell runner", paired, kind) + if !runsCodefly { + require.Contains(t, container.Env, deliveryEnv{Name: "DELIVERY_PATH", Value: path}) + } + } + } +} + +// A paired document is signed once: the publish that executes an inspected +// plan delivers the exact pair the plan prepared, because a fresh signature is +// fresh bytes and would make the plan refuse itself as stale. Executing a plan +// that prepared no pair for a binding is refused rather than signed. +func TestAPairedDocumentIsSignedOnceAndCarriedToTheExecutingPublish(t *testing.T) { + repository := newDeliveryRepository(t) + signer := &fakeSigner{nonce: true} + planned := pairedOptions(signer) + _, err := repository.settle(repository.stageRender(t, "crm"), &planned) + require.NoError(t, err) + first := pairedDocumentsIn(t, repository) + key := carrierKey(pairedDeliveryKind, "example.prod.crm", 1) + require.NotEmpty(t, planned.Carriers[key], "the plan hands the pair to the publish that executes it") + signedByPlan := signer.signed + + executing := pairedOptions(signer) + executing.Reuse = planned.Carriers + executing.Executing = true + _, err = repository.settle(repository.stageRender(t, "crm"), &executing) + require.NoError(t, err) + second := pairedDocumentsIn(t, repository) + require.Equal(t, signedByPlan, signer.signed, "execution signs nothing the plan did not") + require.Equal(t, first["example.prod.crm"].GetAuthority().GetEnvelope(), second["example.prod.crm"].GetAuthority().GetEnvelope(), + "the executed pair is byte for byte the planned one") + + unplanned := pairedOptions(signer) + unplanned.Reuse = map[string][]byte{} + for planned, carrier := range planned.Carriers { + if planned != key { + unplanned.Reuse[planned] = carrier // the Core carrier the plan signed, and not the pair + } + } + unplanned.Executing = true + _, err = repository.settle(repository.stageRender(t, "crm"), &unplanned) + require.ErrorContains(t, err, "was not prepared by the inspected plan") +} + +// A manifest the base branch delivered Core-only enters paired settlement at +// the next generation even when its content is unchanged: the host holds that +// generation with no authority document and refuses the same generation +// offered with one as a history conflict, so the pair is signed at a new one. +// Once delivered paired, an unchanged manifest keeps its generation. +func TestAnUnchangedManifestEnteringPairedSettlementAdvancesOnce(t *testing.T) { + repository := newDeliveryRepository(t) + signer := &fakeSigner{} + coreOnly := publishOptions(signer) + _, err := repository.settle(repository.stageRender(t, "crm"), &coreOnly) + require.NoError(t, err) + require.Equal(t, uint64(1), settledGenerationOf(t, repository, "example.prod.crm")) + repository.deliver(t) + + paired := pairedOptions(signer) + _, err = repository.settle(repository.stageRender(t, "crm"), &paired) + require.NoError(t, err) + require.Equal(t, uint64(2), settledGenerationOf(t, repository, "example.prod.crm"), "entering paired settlement advances once") + require.NotNil(t, pairedDocumentsIn(t, repository)["example.prod.crm"].GetAuthority()) + repository.deliver(t) + + again := pairedOptions(signer) + _, err = repository.settle(repository.stageRender(t, "crm"), &again) + require.NoError(t, err) + require.Equal(t, uint64(2), settledGenerationOf(t, repository, "example.prod.crm"), "delivered paired, unchanged content keeps its generation") +} + +// settledGenerationOf reads the generation the settled overlay carries for a +// binding. +func settledGenerationOf(t *testing.T, repository *deliveryRepository, binding string) uint64 { + t.Helper() + document := settledDocuments(t, repository)[binding] + encoded, err := yaml.Marshal(document) + require.NoError(t, err) + manifest, _, held, err := manifestFromConfigMap(encoded) + require.NoError(t, err) + require.True(t, held) + return manifest.Generation +} diff --git a/pkg/gitops/paired_document.go b/pkg/gitops/paired_document.go new file mode 100644 index 00000000..b7c47642 --- /dev/null +++ b/pkg/gitops/paired_document.go @@ -0,0 +1,280 @@ +package gitops + +import ( + "bytes" + "context" + "encoding/json" + "errors" + "fmt" + "os" + "path/filepath" + "sort" + "strings" + + "google.golang.org/protobuf/encoding/protojson" + "gopkg.in/yaml.v3" + + "github.com/codefly-dev/core/resources" + "github.com/codefly-dev/core/solutionhost" + + "github.com/codefly-dev/cli/pkg/environments" + + authorityv1 "github.com/codefly-dev/authority/libraries/authority-client/go/gen/host/authority/v1" + compositionv1 "github.com/codefly-dev/authority/libraries/authority-client/go/gen/host/composition/v1" +) + +// PreparedPairedDocument is the document a render writes and `codefly delivery +// apply` sends: one ApplyManifestRequest carrying both signed halves and NO +// credential. +// +// The two halves are settled and signed where release custody is — at publish +// — and sent from inside the cell, after the module's workloads exist. The +// credential is the one field that cannot travel between those two moments: a +// delivery credential is a projected ServiceAccount token with a lifetime of +// minutes, so one written here would be a published secret and would be +// expired by the time the manifest synced. It is left empty deliberately, and +// the sender refuses a document that holds one rather than sending it. +// +// carrier is the Core signed carrier exactly as it is delivered +// (solutionhost.MarshalSigned of the settled manifest), and declaration is the +// captured AuthorityManifestV1 bytes — nil for a withdrawal, which keeps its +// Core-only identity. The authority half is bound to the carrier's own +// document bytes by SignedAuthorityDocument, so the pair the host receives is +// the pair that was signed. +func PreparedPairedDocument(carrier []byte, declaration []byte, signer solutionhost.Signer) ([]byte, error) { + if len(carrier) == 0 { + return nil, errors.New("a prepared delivery document needs the signed Core carrier") + } + signed, err := solutionhost.ParseSigned(carrier) + if err != nil { + return nil, fmt.Errorf("read the signed Core carrier: %w", err) + } + manifest, err := solutionhost.ManifestFromVerified(signed.Document) + if err != nil { + return nil, fmt.Errorf("read the module manifest from the signed carrier: %w", err) + } + request := &compositionv1.ApplyManifestRequest{ + Carrier: &compositionv1.SignedManifestV1{Document: signed.Document, Envelope: signed.Envelope}, + } + if request.Authority, err = SignedAuthorityDocument(manifest, signed.Document, declaration, signer); err != nil { + return nil, fmt.Errorf("sign the authority half of %s: %w", manifest.Binding, err) + } + // A prepared document is rendered into a manifest, so a re-render of + // unchanged content must write identical bytes or every cell sees a change + // that is only an encoding — and the Job's name carries the digest of what + // it delivers, so an unstable encoding is a new Job every sync. protojson + // deliberately perturbs its whitespace, and its seed is per-binary, so two + // CLI builds can encode one message differently. Compacting the result + // removes every insignificant byte, leaving protojson's field order, which + // is the message's own. + encoded, err := protojson.Marshal(request) + if err != nil { + return nil, fmt.Errorf("encode the prepared delivery document of %s: %w", manifest.Binding, err) + } + var stable bytes.Buffer + if err = json.Compact(&stable, encoded); err != nil { + return nil, fmt.Errorf("encode the prepared delivery document of %s: %w", manifest.Binding, err) + } + data := stable.Bytes() + // Read it back through the sender's own reader, so a document this writer + // produced that the sender would refuse is caught where it is written. + if _, err = parsePreparedDocument(manifest.Binding, data); err != nil { + return nil, fmt.Errorf("the prepared delivery document does not read back: %w", err) + } + return data, nil +} + +// PreparePairedDocuments writes one prepared paired document per module +// manifest a render left in overlay, into out, and answers the file names in +// the order they were written. +// +// The carriers come from the overlay the render wrote — the signed Core halves +// already settled at their generations — and the declarations from the modules +// themselves through sources. So this prepares exactly what is about to be +// delivered: nothing is re-rendered, no generation is re-settled, and a +// manifest the overlay does not carry is not invented here. +// +// A manifest whose carrier is absent is refused by name rather than prepared +// Core-only: an unsigned manifest cannot carry a signed authority half beside +// it, and a document that quietly dropped the half would reach the host +// looking like a success and compose a module with no roles. +func PreparePairedDocuments(ctx context.Context, overlay, out string, sources AuthoritySources, signer solutionhost.Signer) ([]string, error) { + rendered, err := renderedCarriers(overlay) + if err != nil { + return nil, err + } + if len(rendered) == 0 { + return nil, fmt.Errorf("no module manifest is rendered under %s, so there is nothing to deliver", overlay) + } + if err = os.MkdirAll(out, 0o755); err != nil { + return nil, fmt.Errorf("create the prepared delivery directory: %w", err) + } + bindings := make([]string, 0, len(rendered)) + for binding := range rendered { + bindings = append(bindings, binding) + } + sort.Strings(bindings) + written := make([]string, 0, len(bindings)) + for _, binding := range bindings { + entry := rendered[binding] + if len(entry.carrier) == 0 { + return nil, fmt.Errorf("the module manifest of %s carries no signature, so no authority half can be signed beside it; publish it signed before preparing a paired delivery", binding) + } + // A withdrawal declares nothing and keeps its Core-only identity. + var declaration []byte + if !entry.manifest.Removed { + if declaration, err = sources.Declaration(ctx, entry.manifest.Alias); err != nil { + return nil, fmt.Errorf("read the declaration of module %s: %w", entry.manifest.Alias, err) + } + } + document, prepareErr := PreparedPairedDocument(entry.carrier, declaration, signer) + if prepareErr != nil { + return nil, prepareErr + } + name := binding + PreparedDeliveryDocument + if err = os.WriteFile(filepath.Join(out, name), document, 0o600); err != nil { + return nil, err + } + written = append(written, name) + } + return written, nil +} + +// WorkspaceAuthoritySources are the five declarations read from a composed +// workspace, exactly as a native run reads them at the moment of a delivery +// (launcher.newCompositionDelivery). A publish preparing a cell's documents +// reads them the same way, so neither producer can read a different module +// than the other. +func WorkspaceAuthoritySources(workspace *resources.Workspace, env *environments.Environment) AuthoritySources { + dir := func(ctx context.Context, module string) (string, error) { + m, err := workspace.LoadModuleFromName(ctx, module) + if err != nil { + return "", err + } + return m.Dir(), nil + } + return AuthoritySources{ + Methods: func(ctx context.Context, module string) ([]*compositionv1.MethodPolicyV1, bool, error) { + m, err := workspace.LoadModuleFromName(ctx, module) + if err != nil { + return nil, false, err + } + return HostMethodPolicies(ctx, workspace, env, m) + }, + Ceilings: func(ctx context.Context, module string) ([]*authorityv1.WorkScopeV1, error) { + at, err := dir(ctx, module) + if err != nil { + return nil, err + } + return ScopeCeilings(at) + }, + Roles: func(ctx context.Context, module string) ([]*compositionv1.RoleV1, error) { + at, err := dir(ctx, module) + if err != nil { + return nil, err + } + return Roles(at) + }, + OwnerAware: func(ctx context.Context, module string) (bool, error) { + at, err := dir(ctx, module) + if err != nil { + return false, err + } + return OwnerAware(at) + }, + Installation: func(ctx context.Context, module string) (string, []*compositionv1.EdgeDependencyV1, error) { + at, err := dir(ctx, module) + if err != nil { + return "", nil, err + } + return InstallationDeclaration(at) + }, + } +} + +// renderedCarriers reads the module manifests a render wrote into overlay, +// keyed by binding, each WITH the signed carrier it holds. +// +// renderedManifests deliberately drops the carrier: at publish the carrier is +// settled fresh, and a stale one read back would be a signature nobody +// checked. Preparing a paired document is the opposite case — the carrier is +// the settled Core half and is exactly what must be sent — so it is read here +// rather than by widening that one. +func renderedCarriers(directory string) (map[string]deliveredManifest, error) { + entries, err := os.ReadDir(directory) + if err != nil { + return nil, fmt.Errorf("read the rendered module manifests: %w", err) + } + rendered := map[string]deliveredManifest{} + for _, entry := range entries { + if entry.IsDir() || !strings.HasSuffix(entry.Name(), ".yaml") || entry.Name() == kustomizationFile { + continue + } + data, readErr := readWithin(directory, entry.Name()) + if readErr != nil { + return nil, readErr + } + manifest, carrier, held, parseErr := manifestFromConfigMap(data) + if parseErr != nil { + return nil, fmt.Errorf("%s: %w", entry.Name(), parseErr) + } + if held { + rendered[manifest.Binding] = deliveredManifest{manifest: manifest, carrier: carrier} + } + } + return rendered, nil +} + +// PairedDocumentKey is the key a paired delivery document is carried under in +// its ConfigMap, beside the module manifest's own keys. +const PairedDocumentKey = "paired-delivery.json" + +// pairedDocumentConfigMapName is the ConfigMap holding one binding's paired +// document. It is its own ConfigMap rather than another key on the manifest's, +// because the manifest ConfigMap is read back by every reader of a delivered +// overlay (renderedManifests, priorDeliveredManifests, the cell's own +// validators) and an unknown key there is a change to a published shape. +func pairedDocumentConfigMapName(binding string) string { + return moduleManifestConfigMapName(binding) + "-paired" +} + +// pairedDocumentFile is the file a binding's paired document is written to in +// the overlay. +func pairedDocumentFile(binding string) string { return binding + "-paired.yaml" } + +// pairedDocumentConfigMap is the ConfigMap a paired document travels in, as a +// struct so the delivered YAML has a stable field order. +type pairedDocumentConfigMap struct { + APIVersion string `yaml:"apiVersion"` + Kind string `yaml:"kind"` + Metadata moduleManifestConfigMapMeta `yaml:"metadata"` + Data map[string]string `yaml:"data"` +} + +// writePairedDocumentInTree writes one binding's paired document into the +// overlay. It carries no credential, so it is an ordinary rendered manifest +// and nothing in it is a secret. +func writePairedDocumentInTree(tree *renderRoot, directory, namespace, binding string, document []byte) error { + configMap := pairedDocumentConfigMap{ + APIVersion: "v1", + Kind: kindConfigMap, + Metadata: moduleManifestConfigMapMeta{ + Name: pairedDocumentConfigMapName(binding), + Namespace: namespace, + Labels: map[string]string{ + managedByLabel: managedByCodefly, + documentLabel: pairedDeliveryKind, + bindingLabel: binding, + }, + }, + Data: map[string]string{PairedDocumentKey: string(document)}, + } + body, err := yaml.Marshal(configMap) + if err != nil { + return fmt.Errorf("encode the paired delivery document of %s: %w", binding, err) + } + if err = tree.write(directory, pairedDocumentFile(binding), body); err != nil { + return fmt.Errorf("write the paired delivery document of %s: %w", binding, err) + } + return nil +} diff --git a/pkg/gitops/paired_document_test.go b/pkg/gitops/paired_document_test.go new file mode 100644 index 00000000..e0065fca --- /dev/null +++ b/pkg/gitops/paired_document_test.go @@ -0,0 +1,277 @@ +package gitops + +import ( + "context" + "os" + "path/filepath" + "testing" + + "github.com/codefly-dev/core/solutionhost" + "github.com/stretchr/testify/require" + "google.golang.org/protobuf/encoding/protojson" + "google.golang.org/protobuf/proto" + + delivery "github.com/codefly-dev/authority/libraries/delivery/go" + + compositionv1 "github.com/codefly-dev/authority/libraries/authority-client/go/gen/host/composition/v1" +) + +// signedCarrierOf is the Core half exactly as it is delivered: the settled +// manifest, signed, in the carrier the host reads. +func signedCarrierOf(t *testing.T, manifest *solutionhost.ModuleManifest, signer solutionhost.Signer) []byte { + t.Helper() + payload, err := solutionhost.SignedPayload(manifest) + require.NoError(t, err) + envelope, err := solutionhost.SignEnvelope(payload, signer) + require.NoError(t, err) + signed, err := solutionhost.Carrier(payload, envelope) + require.NoError(t, err) + carrier, err := solutionhost.MarshalSigned(signed) + require.NoError(t, err) + return carrier +} + +func preparedPairManifest(binding string, generation uint64) *solutionhost.ModuleManifest { + return &solutionhost.ModuleManifest{ + Schema: solutionhost.SchemaModuleManifestV1, Binding: binding, Alias: binding, Kind: solutionhost.KindModule, Generation: generation, + Host: solutionhost.HostTarget{Coordinate: "local", Domain: "local"}, + Release: solutionhost.Release{Publisher: "test", Name: binding, Version: "0.1.0"}, + Workloads: []solutionhost.Workload{{Service: "api", Container: "app"}}, + Authority: &solutionhost.Authority{Principal: binding, InstallationRole: "installer"}, + } +} + +// The prepared document carries both halves, and the authority half is stamped +// with the module, the generation and the digest of the EXACT Core bytes +// beside it — so a signed authority cannot be replayed against another +// manifest, generation or module. +func TestAPreparedDocumentBindsTheAuthorityHalfToTheCoreBytes(t *testing.T) { + signer := nativeSigner(t) + manifest := preparedPairManifest("wiki", 7) + carrier := signedCarrierOf(t, manifest, signer) + declaration, err := proto.Marshal(&compositionv1.AuthorityManifestV1{ + InstallationRole: "installer", + Roles: []*compositionv1.RoleV1{{Name: "installer"}, {Name: "editor"}, {Name: "viewer"}}, + }) + require.NoError(t, err) + + data, err := PreparedPairedDocument(carrier, declaration, signer) + require.NoError(t, err) + + var request compositionv1.ApplyManifestRequest + require.NoError(t, protojson.Unmarshal(data, &request)) + require.Empty(t, request.GetCredential(), "a rendered document never holds a credential") + require.NotNil(t, request.GetCarrier()) + require.NotNil(t, request.GetAuthority(), "the half the legacy HTTP route cannot carry") + + var policy compositionv1.AuthorityManifestV1 + require.NoError(t, proto.Unmarshal(request.GetAuthority().GetDocument(), &policy)) + require.Equal(t, "wiki", policy.GetModule()) + require.Equal(t, uint64(7), policy.GetGeneration()) + require.Equal(t, delivery.Digest(request.GetCarrier().GetDocument()), policy.GetCoreManifestDigest(), + "bound to the exact Core bytes beside it") + require.Equal(t, []string{"installer", "editor", "viewer"}, + []string{policy.GetRoles()[0].GetName(), policy.GetRoles()[1].GetName(), policy.GetRoles()[2].GetName()}, + "the roles a module declares travel here and nowhere else: core's Authority has no roles field") + require.Equal(t, "installer", policy.GetInstallationRole()) +} + +// The two producers of the authority half — a native run and a publish +// preparing a document for a cell to send — must agree byte for byte, or the +// host compares one rule against two implementations of it. +func TestBothProducersOfTheAuthorityHalfAgreeByteForByte(t *testing.T) { + signer := nativeSigner(t) + manifest := preparedPairManifest("wiki", 3) + carrier := signedCarrierOf(t, manifest, signer) + declaration, err := proto.Marshal(&compositionv1.AuthorityManifestV1{ + InstallationRole: "installer", + EdgeDependencies: []*compositionv1.EdgeDependencyV1{{Module: "accounts"}, {Module: "documents"}}, + }) + require.NoError(t, err) + signed, err := solutionhost.ParseSigned(carrier) + require.NoError(t, err) + + // The native producer, through the one it has always used. + native := &CompositionDelivery{signer: signer} + fromRun, err := native.authority(manifest, signed.Document, declaration) + require.NoError(t, err) + + // The publish producer, through the prepared document. + data, err := PreparedPairedDocument(carrier, declaration, signer) + require.NoError(t, err) + var request compositionv1.ApplyManifestRequest + require.NoError(t, protojson.Unmarshal(data, &request)) + + require.Equal(t, fromRun.GetDocument(), request.GetAuthority().GetDocument(), + "the same declaration, the same stamp, the same deterministic bytes") + require.Equal(t, fromRun.GetEnvelope(), request.GetAuthority().GetEnvelope(), + "and therefore the same signature") +} + +// A withdrawal keeps its Core-only identity: it declares nothing, so it +// carries no authority half, and the sender still delivers it. +func TestAWithdrawalCarriesNoAuthorityHalfAndIsStillDelivered(t *testing.T) { + signer := nativeSigner(t) + manifest := preparedPairManifest("wiki", 9) + manifest.Removed = true + manifest.Workloads = nil + manifest.Authority = nil + carrier := signedCarrierOf(t, manifest, signer) + + data, err := PreparedPairedDocument(carrier, nil, signer) + require.NoError(t, err) + var request compositionv1.ApplyManifestRequest + require.NoError(t, protojson.Unmarshal(data, &request)) + require.Nil(t, request.GetAuthority(), "a withdrawal declares nothing") + require.NotNil(t, request.GetCarrier()) + + directory := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(directory, "withdrawal.json"), data, 0o600)) + host := &applyHost{} + sender := &PairedApply{Client: applyHostClient(t, host), Credential: func() (string, error) { return "t", nil }, Sleep: noPause} + receipts, err := sender.Apply(context.Background(), directory) + require.NoError(t, err) + require.True(t, receipts[0].Accepted) + require.Nil(t, host.seen[0].GetAuthority()) +} + +// The bytes are the Job's name, so an unchanged pair must encode identically — +// across processes, since protojson's whitespace seed is per-binary. +func TestAnUnchangedPairEncodesIdentically(t *testing.T) { + signer := nativeSigner(t) + carrier := signedCarrierOf(t, preparedPairManifest("wiki", 2), signer) + declaration, err := proto.Marshal(&compositionv1.AuthorityManifestV1{InstallationRole: "none"}) + require.NoError(t, err) + first, err := PreparedPairedDocument(carrier, declaration, signer) + require.NoError(t, err) + for range 5 { + again, err := PreparedPairedDocument(carrier, declaration, signer) + require.NoError(t, err) + require.Equal(t, string(first), string(again)) + } + require.NotContains(t, string(first), " ", "compacted: no insignificant whitespace to perturb") +} + +// Without the release key there is no authority half to sign, and the producer +// says so rather than preparing a Core-only document that would reach the host +// with no roles and look like a success. +func TestAPairedDocumentIsNotPreparedWithoutTheReleaseKey(t *testing.T) { + carrier := signedCarrierOf(t, preparedPairManifest("wiki", 2), nativeSigner(t)) + declaration, err := proto.Marshal(&compositionv1.AuthorityManifestV1{InstallationRole: "installer"}) + require.NoError(t, err) + _, err = PreparedPairedDocument(carrier, declaration, nil) + require.ErrorContains(t, err, "signed by the release key") +} + +// writeOverlayCarrier writes one module manifest ConfigMap into a delivery +// overlay the way a render does, so prepare reads what a render leaves. +func writeOverlayCarrier(t *testing.T, overlay string, manifest *solutionhost.ModuleManifest, carrier []byte) { + t.Helper() + tree, err := newRenderRoot(overlay) + require.NoError(t, err) + defer tree.Close() + _, err = writeModuleManifestInTree(tree, overlay, "acme", manifest, carrier) + require.NoError(t, err) +} + +// Prepare reads the carriers a render settled and the declarations the modules +// hold, and writes one paired document per manifest — in binding order, so the +// Job's digest of its document set is stable. +func TestPrepareWritesAPairedDocumentPerSettledCarrier(t *testing.T) { + signer := nativeSigner(t) + overlay := t.TempDir() + for _, binding := range []string{"wiki", "accounts"} { + manifest := preparedPairManifest(binding, 2) + writeOverlayCarrier(t, overlay, manifest, signedCarrierOf(t, manifest, signer)) + } + sources := AuthoritySources{ + Roles: func(_ context.Context, module string) ([]*compositionv1.RoleV1, error) { + if module != "wiki" { + return nil, nil + } + return []*compositionv1.RoleV1{{Name: "installer"}, {Name: "editor"}, {Name: "viewer"}}, nil + }, + Installation: func(_ context.Context, module string) (string, []*compositionv1.EdgeDependencyV1, error) { + if module != "wiki" { + return "", nil, nil + } + return "installer", nil, nil + }, + } + out := t.TempDir() + written, err := PreparePairedDocuments(context.Background(), overlay, out, sources, signer) + require.NoError(t, err) + require.Equal(t, []string{"accounts.json", "wiki.json"}, written, "binding order, so the document set's digest is stable") + + wiki := readPrepared(t, out, "wiki.json") + require.NotNil(t, wiki.GetAuthority()) + var policy compositionv1.AuthorityManifestV1 + require.NoError(t, proto.Unmarshal(wiki.GetAuthority().GetDocument(), &policy)) + require.Equal(t, []string{"installer", "editor", "viewer"}, + []string{policy.GetRoles()[0].GetName(), policy.GetRoles()[1].GetName(), policy.GetRoles()[2].GetName()}) + require.Equal(t, "installer", policy.GetInstallationRole()) + + accounts := readPrepared(t, out, "accounts.json") + require.Nil(t, accounts.GetAuthority(), "a module that declares nothing carries no authority half") + require.NotNil(t, accounts.GetCarrier()) +} + +// An unsigned carrier cannot carry a signed authority half beside it. Refusing +// by name is the whole point: a Core-only document would reach the host looking +// like a success and compose the module with no roles — the defect this path +// exists to end. +func TestPrepareRefusesAnUnsignedCarrierRatherThanDropTheAuthorityHalf(t *testing.T) { + overlay := t.TempDir() + manifest := preparedPairManifest("wiki", 2) + writeOverlayCarrier(t, overlay, manifest, nil) + _, err := PreparePairedDocuments(context.Background(), overlay, t.TempDir(), + AuthoritySources{Roles: func(context.Context, string) ([]*compositionv1.RoleV1, error) { + return []*compositionv1.RoleV1{{Name: "installer"}}, nil + }}, nativeSigner(t)) + require.ErrorContains(t, err, "carries no signature") + require.ErrorContains(t, err, "wiki") +} + +// An overlay with no manifest is not an empty success. +func TestPrepareRefusesAnOverlayWithNoManifest(t *testing.T) { + _, err := PreparePairedDocuments(context.Background(), t.TempDir(), t.TempDir(), AuthoritySources{}, nativeSigner(t)) + require.ErrorContains(t, err, "nothing to deliver") +} + +// What prepare writes is what apply sends: the two halves survive the file and +// reach a real host over the generated client. +func TestWhatPrepareWritesIsWhatApplySends(t *testing.T) { + signer := nativeSigner(t) + overlay := t.TempDir() + manifest := preparedPairManifest("wiki", 5) + writeOverlayCarrier(t, overlay, manifest, signedCarrierOf(t, manifest, signer)) + out := t.TempDir() + _, err := PreparePairedDocuments(context.Background(), overlay, out, AuthoritySources{ + Installation: func(context.Context, string) (string, []*compositionv1.EdgeDependencyV1, error) { + return "installer", []*compositionv1.EdgeDependencyV1{{Module: "accounts"}}, nil + }, + }, signer) + require.NoError(t, err) + + host := &applyHost{} + sender := &PairedApply{Client: applyHostClient(t, host), Credential: func() (string, error) { return "t", nil }, Sleep: noPause} + receipts, err := sender.Apply(context.Background(), out) + require.NoError(t, err) + require.True(t, receipts[0].Accepted) + require.Equal(t, uint64(5), receipts[0].Generation, "the generation the render settled, read from the signed document") + var policy compositionv1.AuthorityManifestV1 + require.NoError(t, proto.Unmarshal(host.seen[0].GetAuthority().GetDocument(), &policy)) + require.Equal(t, "installer", policy.GetInstallationRole()) + require.Equal(t, []string{"accounts"}, []string{policy.GetEdgeDependencies()[0].GetModule()}) + require.Equal(t, delivery.Digest(host.seen[0].GetCarrier().GetDocument()), policy.GetCoreManifestDigest()) +} + +func readPrepared(t *testing.T, directory, name string) *compositionv1.ApplyManifestRequest { + t.Helper() + data, err := os.ReadFile(filepath.Join(directory, name)) + require.NoError(t, err) + var request compositionv1.ApplyManifestRequest + require.NoError(t, protojson.Unmarshal(data, &request)) + require.Empty(t, request.GetCredential(), "a rendered document never holds a credential") + return &request +} diff --git a/pkg/gitops/paired_generation_test.go b/pkg/gitops/paired_generation_test.go index 676a03be..e760c3a1 100644 --- a/pkg/gitops/paired_generation_test.go +++ b/pkg/gitops/paired_generation_test.go @@ -124,7 +124,7 @@ func TestNativeGenerationIncludesEachAuthorityPolicyInput(t *testing.T) { return nil, nil } return []*compositionv1.RoleV1{{Name: "viewer", Scopes: scopes()}}, nil - }, func(context.Context, string) (bool, error) { return input == "owner-aware" && action == "write", nil }) + }, func(context.Context, string) (bool, error) { return input == "owner-aware" && action == "write", nil }, nil) d := &NativeDeliverer{Signer: signer, StateDir: t.TempDir(), Prepare: composition.Prepare, Token: func(string) (string, error) { return "credential", nil }} _, err := d.Deliver(t.Context(), pairedManifest()) require.NoError(t, err) @@ -152,7 +152,7 @@ func TestNativeLostAcknowledgementCannotReuseGenerationForDifferentPolicy(t *tes action := "read" composition := NewCompositionDelivery(conn, signer, func(context.Context, string) ([]*compositionv1.MethodPolicyV1, bool, error) { return []*compositionv1.MethodPolicyV1{{Method: "/probe.Items/Get", Requires: []*authorityv1.WorkScopeV1{{ResourceKind: "probe.items", Actions: []string{action}}}}}, true, nil - }, nil, nil, nil) + }, nil, nil, nil, nil) d := &NativeDeliverer{Signer: signer, StateDir: t.TempDir(), Prepare: composition.Prepare, Token: func(string) (string, error) { return "credential", nil }} peer.mu.Lock() peer.loseReply = true @@ -186,7 +186,7 @@ func TestNativePolicySnapshotCannotChangeBetweenSettlementAndSend(t *testing.T) composition := NewCompositionDelivery(conn, signer, func(context.Context, string) ([]*compositionv1.MethodPolicyV1, bool, error) { reads++ return []*compositionv1.MethodPolicyV1{policy}, true, nil - }, nil, nil, nil) + }, nil, nil, nil, nil) d := &NativeDeliverer{Signer: signer, StateDir: t.TempDir(), Prepare: composition.Prepare, Token: func(string) (string, error) { policy.Requires[0].Actions[0] = "write"; return "credential", nil }} _, err := d.Deliver(t.Context(), pairedManifest()) require.NoError(t, err) @@ -203,7 +203,7 @@ func TestPairedPolicyPresenceAndWithdrawalReplay(t *testing.T) { c := NewCompositionDelivery(conn, signer, func(context.Context, string) ([]*compositionv1.MethodPolicyV1, bool, error) { reads++ return nil, declared, nil - }, nil, nil, nil) + }, nil, nil, nil, nil) d := &NativeDeliverer{Signer: signer, StateDir: t.TempDir(), Prepare: c.Prepare, Token: func(string) (string, error) { return "credential", nil }} for _, present := range []bool{false, false, true, true, false} { declared = present @@ -234,7 +234,7 @@ func TestPairedMigrationPreservesTerminalGeneration(t *testing.T) { t.Run(fmt.Sprint(removed), func(t *testing.T) { conn, peer := pairedConnection(t) signer := nativeSigner(t) - c := NewCompositionDelivery(conn, signer, nil, nil, nil, nil) + c := NewCompositionDelivery(conn, signer, nil, nil, nil, nil, nil) d := &NativeDeliverer{Signer: signer, StateDir: t.TempDir(), Prepare: c.Prepare, Token: func(string) (string, error) { return "credential", nil }} manifest := pairedManifest() if removed { @@ -395,7 +395,7 @@ func TestNativeRevivalAfterAWithdrawalIsTheHostsToRefuse(t *testing.T) { t.Run(outcome, func(t *testing.T) { conn, peer := pairedConnection(t) signer := nativeSigner(t) - c := NewCompositionDelivery(conn, signer, nil, nil, nil, nil) + c := NewCompositionDelivery(conn, signer, nil, nil, nil, nil, nil) d := &NativeDeliverer{Signer: signer, StateDir: t.TempDir(), Prepare: c.Prepare, Token: func(string) (string, error) { return "credential", nil }} _, err := d.Deliver(t.Context(), pairedManifest()) require.NoError(t, err) @@ -435,7 +435,7 @@ func TestNativeRevivalAfterAWithdrawalIsTheHostsToRefuse(t *testing.T) { require.False(t, refusal.Transient()) restarted, _ := pairedConnection(t) - fresh := NewCompositionDelivery(restarted, signer, nil, nil, nil, nil) + fresh := NewCompositionDelivery(restarted, signer, nil, nil, nil, nil, nil) d = &NativeDeliverer{Signer: signer, StateDir: d.StateDir, Prepare: fresh.Prepare, Token: d.Token} _, err = d.Deliver(t.Context(), pairedManifest()) require.NoError(t, err, "a host started again accepts the module at the reserved generation") diff --git a/pkg/gitops/paired_installation_agreement_test.go b/pkg/gitops/paired_installation_agreement_test.go new file mode 100644 index 00000000..360b1faa --- /dev/null +++ b/pkg/gitops/paired_installation_agreement_test.go @@ -0,0 +1,340 @@ +package gitops + +import ( + "context" + "os" + "path/filepath" + "testing" + + authorityv1 "github.com/codefly-dev/authority/libraries/authority-client/go/gen/host/authority/v1" + compositionv1 "github.com/codefly-dev/authority/libraries/authority-client/go/gen/host/composition/v1" + "github.com/codefly-dev/authority/libraries/authority-core/go/delivered" + "github.com/codefly-dev/authority/libraries/authority-core/go/domain" + modulecontract "github.com/codefly-dev/core/contracts/module" + "github.com/codefly-dev/core/solutionhost" + "github.com/stretchr/testify/require" + "google.golang.org/protobuf/proto" +) + +// documentOf is the host's proto -> delivered.Document mapping, for the two +// declaration fields this proof is about. The mapping itself belongs to the +// Authority service, not to its shared library, so it is reproduced here and +// is NOT what this test proves: what it proves is that the two carriers the +// CLI produces AGREE under Authority's real sameInstallationDeclaration rule. +func documentOf(t *testing.T, sidecar []byte, manifest *solutionhost.ModuleManifest) delivered.Document { + t.Helper() + var policy compositionv1.AuthorityManifestV1 + require.NoError(t, proto.Unmarshal(sidecar, &policy)) + doc := delivered.Document{ + Module: manifest.Alias, + Generation: manifest.Generation, + InstallationRole: policy.GetInstallationRole(), + } + for _, edge := range policy.GetEdgeDependencies() { + doc.EdgeDependencies = append(doc.EdgeDependencies, + domain.EdgeDependency{Module: edge.GetModule(), Presence: edge.GetPresence()}) + } + return doc +} + +// producedCarriers renders a module declaring `declaration` and returns the two +// signed carriers' declaration content: the Core manifest's Authority block and +// the AuthorityManifestV1 bytes, each from the real producer. +func producedCarriers(t *testing.T, declaration string) (*solutionhost.ModuleManifest, []byte) { + t.Helper() + workspace, module := writeAuthorityWorkspaceDeclaring(t, declaration) + env := selectedEnvironment(t, workspace, "staging") + _, manifest := renderShopManifest(t, workspace, module, env) + + // The same reader the delivery uses, over the same contract. + role, edges, err := InstallationDeclaration(module.Dir()) + require.NoError(t, err) + sidecar, err := proto.MarshalOptions{Deterministic: true}.Marshal(&compositionv1.AuthorityManifestV1{ + InstallationRole: role, EdgeDependencies: edges, + }) + require.NoError(t, err) + return manifest, sidecar +} + +// TestTheSignedSidecarOnTheWireCarriesTheDeclaration is the actual-wire +// regression the independent review asked for: it drives the REAL +// CompositionDelivery with a REAL contract read, delivers through +// NativeDeliverer, and reads the declaration back off the signed authority +// document the peer received. +// +// The agreement proof below marshals the proto itself, so it cannot catch +// three regressions the review named: dropping the installation callback at +// the construction site, omitting either field in +// CompositionDelivery.declaration, or reverting its declaration-presence +// condition. This does, because the only thing it reads is what went over the +// wire. +func TestTheSignedSidecarOnTheWireCarriesTheDeclaration(t *testing.T) { + for name, tc := range map[string]struct { + declaration string + wantRole string + wantEdges [][2]string + wantSidecar bool + }{ + "a named selector with ordered edges": {`installation_role: installer +edge_dependencies: + - module: robin + presence: required + - module: annotations + presence: optional +`, "installer", [][2]string{{"robin", "required"}, {"annotations", "optional"}}, true}, + "the none selector alone is reason enough for a sidecar": { + "installation_role: none\n", "none", nil, true}, + "edges alone are reason enough for a sidecar": {`edge_dependencies: + - module: robin + presence: required +`, "", [][2]string{{"robin", "required"}}, true}, + "no declaration, and nothing else to declare, means no sidecar": {"", "", nil, false}, + } { + t.Run(name, func(t *testing.T) { + // A real contract on disk, read by the real reader. + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, modulecontract.FileName), + []byte(declaringContract(tc.declaration)), 0o600)) + + conn, peer := pairedConnection(t) + signer := nativeSigner(t) + composition := NewCompositionDelivery(conn, signer, + func(context.Context, string) ([]*compositionv1.MethodPolicyV1, bool, error) { return nil, false, nil }, + func(context.Context, string) ([]*authorityv1.WorkScopeV1, error) { return nil, nil }, + func(context.Context, string) ([]*compositionv1.RoleV1, error) { return nil, nil }, + func(context.Context, string) (bool, error) { return false, nil }, + func(_ context.Context, _ string) (string, []*compositionv1.EdgeDependencyV1, error) { + return InstallationDeclaration(dir) + }) + d := &NativeDeliverer{Signer: signer, StateDir: t.TempDir(), Prepare: composition.Prepare, + Token: func(string) (string, error) { return "credential", nil }} + _, err := d.Deliver(t.Context(), pairedManifest()) + require.NoError(t, err) + + requests := peer.requests() + require.Len(t, requests, 1) + if !tc.wantSidecar { + require.Nil(t, requests[0].Authority, + "a module declaring nothing must deliver no authority document") + return + } + require.NotNil(t, requests[0].Authority, "the declaration must produce a signed sidecar") + + var policy compositionv1.AuthorityManifestV1 + require.NoError(t, proto.Unmarshal(requests[0].Authority.GetDocument(), &policy)) + require.Equal(t, tc.wantRole, policy.GetInstallationRole()) + require.Len(t, policy.GetEdgeDependencies(), len(tc.wantEdges)) + for i, want := range tc.wantEdges { + require.Equal(t, want[0], policy.GetEdgeDependencies()[i].GetModule(), + "edge %d module, in authored order", i) + require.Equal(t, want[1], policy.GetEdgeDependencies()[i].GetPresence(), "edge %d presence", i) + } + }) + } +} + +// TestAnInstallationOnlyChangeAdvancesTheGeneration is the case the existing +// generation test does not cover: TestNativeGenerationIncludesEachAuthorityPolicyInput +// enumerates method, ceiling, role and owner-aware, and passes nil for +// installation. +// +// It matters beyond coverage. A host that already holds a manifest at +// generation 1 with a NULL authority document will not replace it unless a +// later generation arrives — so if editing only the installation declaration +// left the generation where it was, a repaired delivery would re-send +// generation 1 and the host would keep the policy-less record it already has. +func TestAnInstallationOnlyChangeAdvancesTheGeneration(t *testing.T) { + dir := t.TempDir() + write := func(declaration string) { + t.Helper() + require.NoError(t, os.WriteFile(filepath.Join(dir, modulecontract.FileName), + []byte(declaringContract(declaration)), 0o600)) + } + write("") + + conn, peer := pairedConnection(t) + signer := nativeSigner(t) + composition := NewCompositionDelivery(conn, signer, + func(context.Context, string) ([]*compositionv1.MethodPolicyV1, bool, error) { return nil, false, nil }, + func(context.Context, string) ([]*authorityv1.WorkScopeV1, error) { return nil, nil }, + func(context.Context, string) ([]*compositionv1.RoleV1, error) { return nil, nil }, + func(context.Context, string) (bool, error) { return false, nil }, + func(_ context.Context, _ string) (string, []*compositionv1.EdgeDependencyV1, error) { + return InstallationDeclaration(dir) + }) + d := &NativeDeliverer{Signer: signer, StateDir: t.TempDir(), Prepare: composition.Prepare, + Token: func(string) (string, error) { return "credential", nil }} + + // Declaring nothing: delivered, and no authority document at all — which + // is how a host ends up holding a record with a NULL document. + _, err := d.Deliver(t.Context(), pairedManifest()) + require.NoError(t, err) + require.Nil(t, peer.requests()[0].Authority) + + // Add ONLY the selector. The generation must move, or the host keeps the + // policy-less record. + write("installation_role: none\n") + _, err = d.Deliver(t.Context(), pairedManifest()) + require.NoError(t, err) + + // Add ONLY an edge. The generation must move again. + write(`installation_role: none +edge_dependencies: + - module: robin + presence: required +`) + _, err = d.Deliver(t.Context(), pairedManifest()) + require.NoError(t, err) + + var generations []uint64 + var documents []bool + for _, request := range peer.requests() { + manifest, err := solutionhost.ManifestFromVerified(request.Carrier.Document) + require.NoError(t, err) + generations = append(generations, manifest.Generation) + documents = append(documents, request.Authority != nil) + } + require.Equal(t, []uint64{1, 2, 3}, generations, + "an installation-only edit must advance the generation, or a host holding an older record never replaces it") + require.Equal(t, []bool{false, true, true}, documents, + "a declaration is reason enough for a signed authority document to exist") +} + +// TestTheProducedCarriersAgreeUnderAuthoritysRule runs the CLI's OWN two +// carriers through Authority's real paired-agreement rule +// (delivered.Pair/Module at authority-core 21ce6d92), rather than asserting the +// CLI's own expectations back at itself. +// +// Authority's own tests begin at a constructed document; this begins at what +// the producer actually emits, which is the gap their suite names and leaves +// open. +func TestTheProducedCarriersAgreeUnderAuthoritysRule(t *testing.T) { + for name, declaration := range map[string]string{ + "a named selector with ordered edges": `installation_role: installer +edge_dependencies: + - module: robin + presence: required + - module: annotations + presence: optional +`, + "the none selector": "installation_role: none\n", + "no declaration": "", + "edges without a selector": `edge_dependencies: + - module: robin + presence: required +`, + } { + t.Run(name, func(t *testing.T) { + manifest, sidecar := producedCarriers(t, declaration) + doc := documentOf(t, sidecar, manifest) + + // The agreement rule, reached through Module, which is what the + // reader composes a delivery with. + composed, err := delivered.Module(manifest, &doc) + require.NoError(t, err, "the CLI's two carriers disagree under Authority's rule") + + // And the declaration survives into what Authority composes. + require.Equal(t, manifest.Authority.InstallationRole, composed.InstallationRole) + require.Len(t, composed.EdgeDependencies, len(manifest.Authority.EdgeDependencies)) + for i, edge := range manifest.Authority.EdgeDependencies { + require.Equal(t, edge.Module, composed.EdgeDependencies[i].Module) + require.Equal(t, edge.Presence, composed.EdgeDependencies[i].Presence) + } + }) + } +} + +// TestAuthorityRefusesEveryDisagreementTheRuleNames mutates ONE carrier of a +// genuinely produced pair and asserts Authority refuses. Each case starts from +// a pair that agrees, so the refusal is the disagreement and nothing else. +func TestAuthorityRefusesEveryDisagreementTheRuleNames(t *testing.T) { + ordered := `installation_role: installer +edge_dependencies: + - module: robin + presence: required + - module: annotations + presence: optional +` + for name, tc := range map[string]struct { + declaration string + breakDoc func(*delivered.Document) + want string + }{ + "selector mismatch": {ordered, func(d *delivered.Document) { + d.InstallationRole = "viewer" + }, "installation_role differs"}, + "selector one-sided: document declares nothing": {ordered, func(d *delivered.Document) { + d.InstallationRole = "" + }, "installation_role differs"}, + "empty is not none": {"installation_role: none\n", func(d *delivered.Document) { + d.InstallationRole = "" + }, "installation_role differs"}, + "edges one-sided: document carries none": {ordered, func(d *delivered.Document) { + d.EdgeDependencies = nil + }, "edge_dependencies differs"}, + "edge identity mismatch": {ordered, func(d *delivered.Document) { + d.EdgeDependencies[0].Module = "documents" + }, "edge_dependencies differs"}, + "edge presence mismatch": {ordered, func(d *delivered.Document) { + d.EdgeDependencies[1].Presence = "required" + }, "edge_dependencies differs"}, + "edge order mismatch": {ordered, func(d *delivered.Document) { + d.EdgeDependencies[0], d.EdgeDependencies[1] = d.EdgeDependencies[1], d.EdgeDependencies[0] + }, "edge_dependencies differs"}, + "edge count mismatch": {ordered, func(d *delivered.Document) { + d.EdgeDependencies = append(d.EdgeDependencies, + domain.EdgeDependency{Module: "tracing", Presence: "optional"}) + }, "edge_dependencies differs"}, + } { + t.Run(name, func(t *testing.T) { + manifest, sidecar := producedCarriers(t, tc.declaration) + doc := documentOf(t, sidecar, manifest) + + // It agrees before the mutation, so the case is not vacuous. + _, err := delivered.Module(manifest, &doc) + require.NoError(t, err, "the produced pair must agree before it is broken") + + tc.breakDoc(&doc) + _, err = delivered.Module(manifest, &doc) + require.Error(t, err, "Authority admitted a disagreeing pair") + require.Contains(t, err.Error(), tc.want) + }) + } +} + +// A declared nonempty selector with NO sidecar at all refuses: an absent +// document is the one-sided case the rule names explicitly. +func TestAnAbsentDocumentRefusesADeclaredSelector(t *testing.T) { + manifest, _ := producedCarriers(t, "installation_role: none\n") + _, err := delivered.Module(manifest, nil) + require.Error(t, err) + require.Contains(t, err.Error(), "installation_role differs") + + // And a module declaring nothing pairs with no document, unchanged — the + // legacy both-absent case the rule preserves. + plain, _ := producedCarriers(t, "") + _, err = delivered.Module(plain, nil) + require.NoError(t, err) +} + +// nil and [] are equivalent zero edges on BOTH carriers: the producer delivers +// nil for an empty list precisely so the two spellings cannot disagree. +func TestNilAndEmptyEdgesAgree(t *testing.T) { + manifest, sidecar := producedCarriers(t, "installation_role: none\n") + require.Empty(t, manifest.Authority.EdgeDependencies) + + doc := documentOf(t, sidecar, manifest) + require.Empty(t, doc.EdgeDependencies) + + // The produced manifest carries nil; an explicitly empty slice on either + // side is the same zero-edge declaration. + doc.EdgeDependencies = []domain.EdgeDependency{} + _, err := delivered.Module(manifest, &doc) + require.NoError(t, err, "[] and nil must be equivalent zero edges") + + manifest.Authority.EdgeDependencies = []modulecontract.EdgeDependency{} + _, err = delivered.Module(manifest, &doc) + require.NoError(t, err) + + _ = context.Background() +} diff --git a/pkg/gitops/publish.go b/pkg/gitops/publish.go index e8c2d803..d6fd7bd9 100644 --- a/pkg/gitops/publish.go +++ b/pkg/gitops/publish.go @@ -427,6 +427,15 @@ func (p *deliveryPublication) addressHost(ctx context.Context, workspace *resour return err } p.options.Target = target + // A host that declares codefly.dev/composition takes BOTH signed halves, + // so the publish that settles the Core half must also be able to read each + // module's declaration. Read the same five things a native run reads, from + // the same composed workspace, so neither producer of the authority half + // can read a different module than the other. + if target != nil && target.Composition { + sources := WorkspaceAuthoritySources(workspace, env) + p.options.Declarations = &sources + } if env.Host != nil { p.options.Domain = env.Host.Domain p.options.Coordinate = env.Host.Coordinate diff --git a/pkg/gitops/publish_delivery.go b/pkg/gitops/publish_delivery.go index 7864be2b..ea869b6a 100644 --- a/pkg/gitops/publish_delivery.go +++ b/pkg/gitops/publish_delivery.go @@ -96,6 +96,14 @@ type deliveryPublishOptions struct { // publish that may not deliver unsigned gives when its signer had no // identity. Empty means the release workflow. Signs string + // Declarations reads a module's own declaration — the authority half of a + // paired delivery. It is read only where the host's delivery endpoint + // declares codefly.dev/composition (Target.Composition), because that is + // the only interface that takes the pair. nil leaves a paired publish + // unable to prepare, and it refuses rather than deliver the Core half + // alone: a module delivered Core-only reaches the host with no roles and + // looks like a success. + Declarations *AuthoritySources } // unsignedRefusal is the refusal of a publish that may not deliver unsigned @@ -204,6 +212,13 @@ func settleManifests( if err != nil { return nil, nil, err } + if opts.Target != nil && opts.Target.Composition { + delivered, filesErr := deliveredOverlayFiles(ctx, repo, baseBranch, filepath.ToSlash(filepath.Join(targetPath, overlay))) + if filesErr != nil { + return nil, nil, filesErr + } + advanceIntoPairedSettlement(settled, prior, delivered) + } names := sortedBindings(settled) // Each manifest at its settled generation is held against what the base // branch delivered for its binding with core's own admission — a @@ -227,7 +242,7 @@ func settleManifests( if err != nil { return nil, nil, err } - files, err := writeManifestSet(target, overlay, inventory, settled, names, opts) + files, err := writeManifestSet(ctx, target, overlay, inventory, settled, names, opts) if err != nil { return nil, nil, err } @@ -333,6 +348,34 @@ func settledManifestSet(rendered, prior map[string]deliveredManifest) (map[strin return settled, nil } +// advanceIntoPairedSettlement moves a binding into paired settlement: a +// manifest the base branch delivered Core-only — no paired document beside +// its carrier — that this publish delivers paired keeps nothing of that +// generation, because the host holds the generation with no authority +// document and refuses the same generation offered with one as a history +// conflict ("authority document digest stored=… (present=false)"), and the +// old record cannot establish policy equality either way. It advances once, +// as the native run's record does on the same transition, and is signed +// again. A withdrawal keeps its Core-only identity. +func advanceIntoPairedSettlement(settled, prior map[string]deliveredManifest, deliveredFiles []string) { + paired := map[string]bool{} + for _, name := range deliveredFiles { + paired[name] = true + } + for binding, entry := range settled { + previous, delivered := prior[binding] + if !delivered || entry.manifest.Removed || entry.manifest.Generation != previous.manifest.Generation { + continue + } + if paired[pairedDocumentFile(binding)] { + continue + } + entry.manifest.Generation = previous.manifest.Generation + 1 + entry.carrier = nil + settled[binding] = entry + } +} + // settledGeneration is the generation of a candidate against the manifest // delivered for the same binding: the same canonical content keeps it, other // content is the next one. Only the NUMBER is chosen here; whether the @@ -470,8 +513,10 @@ func signCarrier(ctx context.Context, what string, payload []byte, carrier *[]by } // writeManifestSet writes the settled manifests, the delivery account and the -// Job that POSTs every signed carrier, and returns the overlay's file list. -func writeManifestSet(target, overlay string, inventory *Inventory, settled map[string]deliveredManifest, names []string, opts *deliveryPublishOptions) ([]string, error) { +// Job that delivers every signed carrier — toward a host that declares +// codefly.dev/composition, as one paired document per carrier — and returns +// the overlay's file list. +func writeManifestSet(ctx context.Context, target, overlay string, inventory *Inventory, settled map[string]deliveredManifest, names []string, opts *deliveryPublishOptions) ([]string, error) { directory := filepath.Join(target, filepath.FromSlash(overlay)) if err := os.MkdirAll(directory, 0o755); err != nil { return nil, fmt.Errorf("create delivery overlay: %w", err) @@ -481,6 +526,11 @@ func writeManifestSet(target, overlay string, inventory *Inventory, settled map[ return nil, err } defer tree.Close() + paired := opts.Target != nil && opts.Target.Composition + if paired && opts.Declarations == nil { + return nil, fmt.Errorf("the delivery endpoint %s declares %s, which takes both signed halves, and this publish was given no way to read module declarations; it will not deliver the Core half alone, because a module delivered that way reaches the host with no roles", + opts.Target.Address, CompositionInterface) + } var files []string var documents []deliveryDocument var carriers [][]byte @@ -491,12 +541,54 @@ func writeManifestSet(target, overlay string, inventory *Inventory, settled map[ return nil, writeErr } files = append(files, file) - if len(entry.carrier) > 0 { + if len(entry.carrier) == 0 { + continue + } + if !paired { documents = append(documents, deliveryDocument{ ConfigMap: moduleManifestConfigMapName(binding), Key: solutionhost.SignedFileName, Name: binding + ".json", }) carriers = append(carriers, entry.carrier) + continue + } + // The paired document: both signed halves in one ApplyManifestRequest, + // in its own ConfigMap beside the manifest's (pairedDocumentConfigMapName + // says why), so the Job mounts one file per binding and the authority + // half travels with the Core bytes it was signed against. + var declaration []byte + if !entry.manifest.Removed { + if declaration, err = opts.Declarations.Declaration(ctx, entry.manifest.Alias); err != nil { + return nil, fmt.Errorf("read the declaration of module %s: %w", entry.manifest.Alias, err) + } + } + // The pair is signed ONCE per publish and carried from the inspected + // plan to the publish that executes it, exactly like the Core carrier: + // a signature is fresh bytes each time (ECDSA, KMS), so a pair signed + // again on execution would change the overlay the plan was identified + // by and the publish would refuse its own plan as stale. + key := carrierKey(pairedDeliveryKind, binding, entry.manifest.Generation) + pair := opts.Reuse[key] + if pair == nil { + if opts.Executing { + return nil, fmt.Errorf("the paired document of %s was not prepared by the inspected plan, and the publish that executes a plan signs nothing the plan did not compare", binding) + } + var pairErr error + if pair, pairErr = PreparedPairedDocument(entry.carrier, declaration, resolvedSigner(opts)); pairErr != nil { + return nil, pairErr + } + } + if opts.Carriers == nil { + opts.Carriers = map[string][]byte{} } + opts.Carriers[key] = pair + if err = writePairedDocumentInTree(tree, directory, inventory.Namespace, binding, pair); err != nil { + return nil, err + } + files = append(files, pairedDocumentFile(binding)) + documents = append(documents, deliveryDocument{ + ConfigMap: pairedDocumentConfigMapName(binding), Key: PairedDocumentKey, Name: binding + ".json", + }) + carriers = append(carriers, pair) } if opts.Target == nil { return files, nil diff --git a/pkg/gitops/publish_envelope_test.go b/pkg/gitops/publish_envelope_test.go index df5a2c7c..92572473 100644 --- a/pkg/gitops/publish_envelope_test.go +++ b/pkg/gitops/publish_envelope_test.go @@ -101,7 +101,7 @@ func TestPublishDeliversTheModuleEnvelopeLikeAManifest(t *testing.T) { require.NoError(t, yaml.Unmarshal(jobBytes, &job)) require.Equal(t, deliveredEnvelopeKind, job.Metadata.Labels[deliveredDocumentLabel]) require.Equal(t, deliveryCarrierValue, job.Metadata.Labels[deliveryLabel], "the Job is selected as the carrier too") - require.Equal(t, deliveryCarrierOf().Labels, job.Spec.Template.Metadata.Labels, "the pod is the carrier the cell admits") + require.Equal(t, deliveryCarrierOf(false).Labels, job.Spec.Template.Metadata.Labels, "the pod is the carrier the cell admits") require.Equal(t, "crm", job.Metadata.Namespace) for _, want := range []string{"value: " + envelopeDeliveryPath, "key: " + solutionhost.EnvelopeSignedFileName, "name: " + moduleEnvelopeConfigMap, "serviceAccountName: delivery"} { require.Contains(t, string(jobBytes), want) diff --git a/pkg/gitops/render.go b/pkg/gitops/render.go index 5c4ef365..92f7182f 100644 --- a/pkg/gitops/render.go +++ b/pkg/gitops/render.go @@ -1284,9 +1284,16 @@ func inspectValueAllowingExpressions( } switch typed := value.(type) { case map[string]any: - if name, ok := typed["name"].(string); ok && isCredentialKey(name, true) && scalarHasValue(typed["value"]) { + if name, ok := typed["name"].(string); ok && isCredentialKey(name, true) && scalarHasValue(typed["value"]) && + !projectedTokenPathEntry(typed) { return fmt.Errorf("%s.value contains credential value", strings.Join(path, ".")) } + // A container telling a process where its projected token is must be + // the container that mounts it, or the path names a file this + // workload does not have and the credential never resolves. + if err := holdTokenPathToItsMount(typed, path); err != nil { + return err + } for key, child := range typed { next := extendPath(path, key) if isCredentialKey(key, isConfigurationDataPath(path)) && scalarHasValue(child) && @@ -1426,6 +1433,61 @@ var manifestFieldNormalizer = strings.NewReplacer("-", "", "_", "", ".", "") // // One narrowing of core's classification is made here, and only here: a public // client identifier. See publicClientIdentifier. +// holdTokenPathToItsMount refuses a container that declares the projected +// token's path variable without mounting the token at that path. +// +// The allowance in projectedTokenPathEntry exempts one exact name and value +// from the credential-literal refusal; this is what keeps that exemption from +// becoming a way to put the variable anywhere. A sidecar beside the +// authenticating container, or a workload no host admits, gets neither the +// mount nor the variable — and if something hands it the variable alone, the +// render says so here rather than at runtime, where it reads as a refused +// credential rather than a missing file. +func holdTokenPathToItsMount(container map[string]any, path []string) error { + entries := sliceField(container, "env") + if len(entries) == 0 { + return nil + } + declares := false + for _, raw := range entries { + if entry, _ := raw.(map[string]any); entry != nil && entry["name"] == WorkloadTokenFileVariable { + declares = true + break + } + } + if !declares { + return nil + } + for _, raw := range sliceField(container, "volumeMounts") { + mount, _ := raw.(map[string]any) + if mount != nil && mount["name"] == identityTokenVolume && mount["mountPath"] == identityTokenMount { + return nil + } + } + return fmt.Errorf("%s declares %s and mounts no %s at %s, so the path names a file it does not have", + strings.Join(path, "."), WorkloadTokenFileVariable, identityTokenVolume, identityTokenMount) +} + +// projectedTokenPathEntry reports whether an env entry is the render's own// projectedTokenPathEntry reports whether an env entry is the render's own +// projected-token PATH: the one variable name Hop's RunCredential reads, +// carrying exactly the path this render mounts the token at. +// +// isCredentialKey classifies that name as credential material because it +// contains TOKEN, and a literal value under a credential-named key is +// otherwise refused — correctly, since that is how a secret becomes a +// plaintext manifest. A mount path is not secret material, so this corrects +// the classification for one exact pair and nothing else: a different value +// under the same name stays refused, as does every other credential-named key. +// +// It does not establish that the container holding it mounts the token — +// validateProjectedTokenPaths does, over the rendered overlay, so a workload +// told to read a file it does not have is refused there. +func projectedTokenPathEntry(entry map[string]any) bool { + name, named := entry["name"].(string) + value, valued := entry["value"].(string) + return named && valued && name == WorkloadTokenFileVariable && value == projectedTokenPath() +} + func isCredentialKey(key string, configurationName bool) bool { classify := resources.IsSensitiveKey scanned := key diff --git a/pkg/gitops/roles.go b/pkg/gitops/roles.go index eea6fae3..249c3949 100644 --- a/pkg/gitops/roles.go +++ b/pkg/gitops/roles.go @@ -59,7 +59,40 @@ func Roles(dir string) ([]*compositionv1.RoleV1, error) { return out, nil } -// OwnerAware reads the module's explicit declaration that it checks resource +// InstallationDeclaration reads a module's installation declaration from its +// own contract: the agent's standing role selector and the modules it reaches +// with the person's Work Context. +// +// Both are carried VERBATIM. Core validated them already (contracts/module +// rules.go: strict literal selector, no trim or case-fold, a named selector +// held to a declared role), and the paired agreement rule compares this +// document against the Core signed manifest byte-for-byte — empty and "none" +// differ, nil and [] are equivalent zero edges. So nothing here trims, +// defaults, sorts, deduplicates or resolves: an edge's Module is the signed +// principal the host resolves through its catalogue, never a name, a binding +// id or a release. +// +// No contract means no declaration, which is distinct from a contract +// declaring none. +func InstallationDeclaration(dir string) (string, []*compositionv1.EdgeDependencyV1, error) { + contract, err := ownContract(dir) + if err != nil || contract == nil { + return "", nil, err + } + edges := make([]*compositionv1.EdgeDependencyV1, 0, len(contract.EdgeDependencies)) + for _, edge := range contract.EdgeDependencies { + edges = append(edges, &compositionv1.EdgeDependencyV1{Module: edge.Module, Presence: edge.Presence}) + } + if len(edges) == 0 { + // nil and [] are equivalent zero edges, and a repeated proto field + // cannot carry the difference; deliver nil so the two spellings + // serialize identically. + edges = nil + } + return contract.InstallationRole, edges, nil +} + +// OwnerAware reads the module's explicit declaration that it checks resource// OwnerAware reads the module's explicit declaration that it checks resource // owners. No contract or an omitted declaration means false, never an inference // from the presence of roles or installation-owned scopes. func OwnerAware(dir string) (bool, error) { diff --git a/pkg/gitops/roles_test.go b/pkg/gitops/roles_test.go index 19f19cba..4155b67c 100644 --- a/pkg/gitops/roles_test.go +++ b/pkg/gitops/roles_test.go @@ -232,7 +232,7 @@ func deliveredAuthorityDocument(t *testing.T, dir string) *compositionv1.Authori func(context.Context, string) ([]*compositionv1.MethodPolicyV1, bool, error) { return nil, false, nil }, func(context.Context, string) ([]*authorityv1.WorkScopeV1, error) { return ScopeCeilings(dir) }, func(context.Context, string) ([]*compositionv1.RoleV1, error) { return Roles(dir) }, - func(context.Context, string) (bool, error) { return OwnerAware(dir) }) + func(context.Context, string) (bool, error) { return OwnerAware(dir) }, nil) manifest := &solutionhost.ModuleManifest{ Schema: solutionhost.SchemaModuleManifestV1, Binding: "shop", Alias: "shop", Kind: solutionhost.KindSolution, Generation: 1, Host: solutionhost.HostTarget{Coordinate: "local", Domain: "local"}, Release: solutionhost.Release{Publisher: "test", Name: "shop", Version: "0.1.0"}, diff --git a/pkg/gitops/runnable_grants.go b/pkg/gitops/runnable_grants.go index 6c3a3516..f1e0b111 100644 --- a/pkg/gitops/runnable_grants.go +++ b/pkg/gitops/runnable_grants.go @@ -145,9 +145,19 @@ func scopeStrings(scopes []preparedScope) []string { // appendPreparedBindings adds a binding per prepared key to a module's request. // A key the module's own contract already uses is refused by name: one id, two // declarations of what it asks. +// +// A prepared binding addressed to the module itself is not installed: the +// composition prepares every runnable for every installer, so a module whose +// own operations are runnables (robin's chat methods) would otherwise ask for +// a person-delegated capability to call itself — which the host refuses, as +// a binding naming its own module is only ever headless work — and reaching +// its own operations needs no capability at all. func appendPreparedBindings(authority *solutionhost.Authority, module string, grants []preparedGrant) error { for i := range grants { grant := &grants[i] + if grant.audience == module { + continue + } for _, existing := range authority.Bindings { if existing.ID == grant.key { return fmt.Errorf("module %s declares binding %s and installs the prepared binding %s/%s under the same id; a binding has one declaration", module, existing.ID, runnablespkg.RunnableBindingsGroup, grant.key) @@ -279,6 +289,9 @@ func withPreparedEntries(principal string, grant json.RawMessage, grants []prepa } for i := range grants { prepared := &grants[i] + if prepared.audience == principal { + continue + } if _, stated := audiences[prepared.key]; stated { continue } diff --git a/pkg/gitops/runnable_grants_test.go b/pkg/gitops/runnable_grants_test.go index 708c7e0e..77a25512 100644 --- a/pkg/gitops/runnable_grants_test.go +++ b/pkg/gitops/runnable_grants_test.go @@ -258,3 +258,23 @@ func TestAPublishListsPreparedBindingsInTheEnvelopeItDelivers(t *testing.T) { require.NoError(t, json.Unmarshal(publication.options.Envelope.Principals["shop"], &grant)) require.Equal(t, "module_actor", grant.OperationAudiences[preparedKey].DelegationMode) } + +// A prepared binding addressed to the installing module itself is installed +// nowhere: not in the module's request and not in its envelope entry. The host +// admits a self-addressed binding only as headless work, and a module needs no +// capability to reach its own operations. +func TestAPreparedBindingAddressedToTheInstallerItselfIsNotInstalled(t *testing.T) { + grants := []preparedGrant{ + {key: "SHOP__API_CONNECT_LIST", audience: "shop", invoke: []preparedScope{{kind: "shop.items", actions: []string{"read"}}}}, + {key: "ACME__ITEMS_CONNECT_READ", audience: "acme.items", invoke: []preparedScope{{kind: "acme.item", actions: []string{"read"}}}}, + } + authority := &solutionhost.Authority{Principal: "shop"} + require.NoError(t, appendPreparedBindings(authority, "shop", grants)) + require.Len(t, authority.Bindings, 1) + require.Equal(t, "ACME__ITEMS_CONNECT_READ", authority.Bindings[0].ID) + + entry, err := withPreparedEntries("shop", json.RawMessage(`{"resources":["shop.items"]}`), grants) + require.NoError(t, err) + require.Contains(t, string(entry), "ACME__ITEMS_CONNECT_READ") + require.NotContains(t, string(entry), "SHOP__API_CONNECT_LIST") +} diff --git a/pkg/gitops/workload_projection.go b/pkg/gitops/workload_projection.go index 9f6efb7b..8adc2077 100644 --- a/pkg/gitops/workload_projection.go +++ b/pkg/gitops/workload_projection.go @@ -157,7 +157,29 @@ func refuseReservedAnnotation(tree *renderRoot, owned string, opts *RenderOption return nil } -// projectIdentityToken projects the host-audience token into the +// WorkloadTokenFileVariable names the projected identity token's path to the +// process that must present it. The mount alone does not satisfy the consumer: +// the SDK's ProjectedTokenFile is a path VALUE its caller supplies and reads +// no environment itself (sdk-go workcontext mint.go), and Hop's RunCredential +// reads only this variable and refuses when it is absent — it infers no +// Kubernetes default. So a workload that mounts the token and is told nothing +// acquires no publication lease, and its Front.Ready refuses, which is how a +// correctly rendered volume and ServiceAccount still produced a gateway that +// could not register. +// +// A native run supplies it from the path it writes the projection to; a +// deployed workload is told the path this render mounts it at. One name, one +// meaning, both paths — and it is a path, never a credential. +const WorkloadTokenFileVariable = "CODEFLY__WORKLOAD_TOKEN_FILE" //nolint:gosec // G101: the name of a path variable, not a credential + +// projectedTokenPath is where this render mounts the identity token, which is +// the value WorkloadTokenFileVariable carries. Derived from the mount and file +// so the projection and the variable cannot drift apart. +func projectedTokenPath() string { + return path.Join(identityTokenMount, identityTokenFile) +} + +// projectIdentityToken projects the host-audience token into the// projectIdentityToken projects the host-audience token into the // authenticating container of each serving workload of an identity service's // unit, unless that container already mounts one for the audience. func projectIdentityToken(tree *renderRoot, owned string, opts *RenderOptions, identity IdentityWorkload) error { @@ -190,7 +212,14 @@ func projectIdentityToken(tree *renderRoot, owned string, opts *RenderOptions, i if workload.hasVolume(identityTokenVolume) { return fmt.Errorf("workload %s already declares a volume named %s, which the render projects; rename it", workload.Name, identityTokenVolume) } - patches = append(patches, &podProjection{kind: workload.Kind, name: workload.Name, container: container.Name, volume: volume, mount: mount}) + patches = append(patches, &podProjection{ + kind: workload.Kind, name: workload.Name, container: container.Name, + volume: volume, mount: mount, + // The path travels with the mount that creates it, into the same + // container, so the consumer is never told to read a file this + // workload does not have. + env: map[string]string{WorkloadTokenFileVariable: projectedTokenPath()}, + }) } return addOverlayPatches(tree, unitDir, opts.Environment, patches) } diff --git a/pkg/gitops/workload_projection_test.go b/pkg/gitops/workload_projection_test.go index 066fe8d5..d33e6ca4 100644 --- a/pkg/gitops/workload_projection_test.go +++ b/pkg/gitops/workload_projection_test.go @@ -134,6 +134,190 @@ func renderedVolumeOf(t *testing.T, destination, service, volume string) (map[st return nil, nil } +// renderedContainerEnv is every env entry of one container of the rendered +// unit, keyed by variable name. +func renderedContainerEnv(t *testing.T, destination, service, container string) map[string]map[string]any { + t.Helper() + manifests, err := overlayManifests(filepath.Join(destination, "services", service, "overlays", "prod")) + if err != nil { + t.Fatal(err) + } + for _, item := range manifests { + if item.kind != kindDeployment { + continue + } + spec, _ := podSpec(item) + for _, entry := range sliceField(spec, "containers") { + found, _ := entry.(map[string]any) + if found["name"] != container { + continue + } + env := map[string]map[string]any{} + for _, raw := range sliceField(found, "env") { + if variable, _ := raw.(map[string]any); variable != nil { + name, _ := variable["name"].(string) + env[name] = variable + } + } + return env + } + } + // An absent container is not a failure: a single-container fixture has no + // sidecar, and a caller asserting "told nothing" reads an empty set. + return nil +} + +// TestAnIdentityWorkloadIsToldWhereItsTokenIs is the Blue gateway failure: +// the render mounted the projection correctly and told the process nothing, +// so Hop's RunCredential — which reads CODEFLY__WORKLOAD_TOKEN_FILE and +// refuses when it is absent, inferring no Kubernetes default — acquired no +// publication lease and Front.Ready refused, on a Deployment whose +// ServiceAccount, volume and mount were all right. +// +// The variable travels with the mount, into the SAME container, and its value +// is derived from the mount and file rather than written twice, so the +// projection and the path it advertises cannot drift. A native run is handed +// the same variable name from the same constant. +func TestAnIdentityWorkloadIsToldWhereItsTokenIs(t *testing.T) { + for name, generate := range map[string]func(context.Context, string) error{ + "a kustomized overlay": kustomizedWorkload(sidecarDeployment), + "an overlay of manifests": renderWorkload(sidecarDeployment), + "a single-container agent": kustomizedWorkload(pinnedDeployment), + } { + t.Run(name, func(t *testing.T) { + destination := filepath.Join(t.TempDir(), "modules", "crm") + if _, err := RenderOwnedTree(context.Background(), solutionRenderOptions(destination), generate); err != nil { + t.Fatal(err) + } + + // The mount is the premise: without it the variable would name a + // file the container does not have. + volume, mounts := renderedVolume(t, destination, identityTokenVolume) + if volume == nil || len(mounts["api"]) != 1 { + t.Fatal("the workload projects no identity token into its api container") + } + + env := renderedContainerEnv(t, destination, "api", "api") + entry, told := env[WorkloadTokenFileVariable] + if !told { + t.Fatalf("the api container is not told where its token is; it has %v", env) + } + if got := entry["value"]; got != projectedTokenPath() { + t.Errorf("%s = %v, want the mounted path %s", WorkloadTokenFileVariable, got, projectedTokenPath()) + } + // The value must BE the mount's path, not a literal that happens + // to match it today. + if entry["value"] != mounts["api"][0]["mountPath"].(string)+"/"+identityTokenFile { + t.Errorf("%s = %v, which is not the file inside the mount at %v", + WorkloadTokenFileVariable, entry["value"], mounts["api"][0]["mountPath"]) + } + // A path is a literal: any authored source for this key is cleared, + // or the container would read a ConfigMap value instead. + if source, present := entry["valueFrom"]; present && source != nil { + t.Errorf("%s carries valueFrom %v, want no source for a literal path", WorkloadTokenFileVariable, source) + } + + // Only the container that holds the token is told about it. + if sidecar := renderedContainerEnv(t, destination, "api", "sidecar"); len(sidecar) > 0 { + if _, leaked := sidecar[WorkloadTokenFileVariable]; leaked { + t.Error("the sidecar is told where a token it does not mount is") + } + } + }) + } +} + +// A service no host admits is told nothing, because it is projected nothing: +// the variable must never name a file that is not there. The module keeps an +// identity service — a module with no workload at all is refused by its own +// manifest — and the assertion is on the service that has none. +func TestAServiceWithoutAnIdentityIsToldNoTokenPath(t *testing.T) { + destination := filepath.Join(t.TempDir(), "modules", "crm") + options := solutionRenderOptions(destination) + options.Units = promotableServiceGraph("crm", []string{"api", "worker"}) + if _, err := RenderOwnedTree(context.Background(), options, kustomizedWorkloads(map[string]string{ + "api": pinnedDeployment, + "worker": strings.ReplaceAll(pinnedDeployment, "name: api", "name: worker"), + })); err != nil { + t.Fatal(err) + } + // api is the identity service and keeps its projection. + if volume, _ := renderedVolumeOf(t, destination, "api", identityTokenVolume); volume == nil { + t.Fatal("the identity service projects no token") + } + // worker is not one. + if volume, _ := renderedVolumeOf(t, destination, "worker", identityTokenVolume); volume != nil { + t.Fatal("a service with no identity projects a token") + } + if _, told := renderedContainerEnv(t, destination, "worker", "api")[WorkloadTokenFileVariable]; told { + t.Error("a service with no identity is told where a token it does not have is") + } +} + +// TestTheTokenPathAllowanceIsExactlyOnePair holds the credential guard where +// the allowance stops. The render exempts ONE name carrying ONE value, on a +// container that mounts the token; everything adjacent stays refused, because +// the exemption exists for a path and not for the word TOKEN. +// +// The two predicates are exercised directly: driving these through a whole +// render refuses them earlier, on the hand-written mount, which would prove +// the projection validator rather than the classification. +func TestTheTokenPathAllowanceIsExactlyOnePair(t *testing.T) { + entry := func(name, value string) map[string]any { + return map[string]any{"name": name, "value": value} + } + for name, tc := range map[string]struct { + entry map[string]any + allowed bool + }{ + "the exact pair": {entry(WorkloadTokenFileVariable, projectedTokenPath()), true}, + "a different value, same name": {entry(WorkloadTokenFileVariable, "/etc/elsewhere/token"), false}, + "a real token literal, same shape": {entry("CODEFLY__WORKLOAD_TOKEN", "eyJhbGciOi.not.a.path"), false}, + "the path under another name": {entry("CODEFLY__SESSION_TOKEN", projectedTokenPath()), false}, + "the mount directory, not the file": {entry(WorkloadTokenFileVariable, identityTokenMount), false}, + } { + t.Run(name, func(t *testing.T) { + if allowed := projectedTokenPathEntry(tc.entry); allowed != tc.allowed { + t.Errorf("projectedTokenPathEntry(%v) = %v, want %v", tc.entry, allowed, tc.allowed) + } + // Whatever the allowance says, a credential-named key still has to + // be classified as one: the exemption narrows the refusal, it does + // not reclassify the key. + if !isCredentialKey(tc.entry["name"].(string), true) { + t.Errorf("%v is no longer classified as a credential key", tc.entry["name"]) + } + }) + } +} + +// The variable may only sit on the container that mounts the token: the +// allowance above must not become a way to put the path anywhere. +func TestTheTokenPathMustSitOnTheMountingContainer(t *testing.T) { + told := []any{map[string]any{"name": WorkloadTokenFileVariable, "value": projectedTokenPath()}} + mount := []any{map[string]any{"name": identityTokenVolume, "mountPath": identityTokenMount}} + for name, tc := range map[string]struct { + container map[string]any + refused bool + }{ + "told and mounting": {map[string]any{"name": "api", "env": told, "volumeMounts": mount}, false}, + "told, mounting nothing": {map[string]any{"name": "api", "env": told}, true}, + "told, mounting elsewhere": {map[string]any{"name": "api", "env": told, "volumeMounts": []any{ + map[string]any{"name": identityTokenVolume, "mountPath": "/somewhere/else"}, + }}, true}, + "mounting, told nothing": {map[string]any{"name": "api", "volumeMounts": mount}, false}, + } { + t.Run(name, func(t *testing.T) { + err := holdTokenPathToItsMount(tc.container, []string{"spec", "containers", "[0]"}) + if tc.refused != (err != nil) { + t.Fatalf("refused=%v, want %v (err=%v)", err != nil, tc.refused, err) + } + if err != nil && !strings.Contains(err.Error(), "mounts no "+identityTokenVolume) { + t.Errorf("refusal does not name the missing mount: %v", err) + } + }) + } +} + // TestEveryWorkloadAHostAdmitsProjectsItsIdentityToken: an identity workload // proves who it is with a ServiceAccount token projected for the host's // audience — mounted at the path the SDK reads, ONLY into the container its diff --git a/pkg/orchestration/excluded_services_test.go b/pkg/orchestration/excluded_services_test.go index 59bfc74e..9d8927df 100644 --- a/pkg/orchestration/excluded_services_test.go +++ b/pkg/orchestration/excluded_services_test.go @@ -4,6 +4,7 @@ import ( "context" "testing" + "github.com/codefly-dev/cli/pkg/environments" "github.com/codefly-dev/core/resources" "github.com/stretchr/testify/require" ) @@ -123,6 +124,84 @@ func TestPlanRefusesARootGroupReferenceToAnExcludedService(t *testing.T) { require.ErrorContains(t, err, "platform/authority") } +// TestNewFlowIsNotRefusedOverAnotherEnvironmentsManagedService closes the gap +// the reviewer found: ValidateWorkspace narrows managed-service keys, but +// NewFlow validates them on its OWN call (flow.go), which every render, +// build, run and deploy reaches through renderServiceFlow and friends. A +// regression that only drives ValidateWorkspace cannot see that call, so the +// narrowing has to be proved here, at the seam the real render uses. +// +// `stale` names a bare managed service "vault" while two modules compose a +// service of that name, so the key is ambiguous. A flow selecting `staging`, +// which declares no managed services at all, must not inherit that refusal — +// a flow resolves managed services through the SELECTED env.ManagedService +// and never asks another environment's. +func TestNewFlowIsNotRefusedOverAnotherEnvironmentsManagedService(t *testing.T) { + ctx := context.Background() + files := exclusionWorkspaceFiles("") + files["modules/depot/module.codefly.yaml"] = "kind: module\nname: depot\nservices:\n - name: vault\n" + files["modules/depot/services/vault/service.codefly.yaml"] = `kind: service +name: vault +version: 0.0.0 +module: depot +agent: + kind: runtime::service + name: go-grpc + version: 0.0.1 + publisher: codefly.ai +` + files["workspace.codefly.yaml"] = `name: acme +layout: modules +modules: + - name: shop + - name: depot +environments: + - name: staging + namespace: acme + - name: stale + namespace: acme + managed-services: + vault: + kind: external + external-name: vault.example.com +` + workspace := writeTempWorkspace(t, files) + module, err := workspace.LoadModuleFromName(ctx, "shop") + require.NoError(t, err) + api, err := module.LoadServiceFromName(ctx, "api") + require.NoError(t, err) + + // The owning environment really is ambiguous: selecting it still refuses, + // so the next assertion is not vacuous. + stale, err := SelectEnvironment(workspace, "stale") + require.NoError(t, err) + _, err = NewFlow(ctx, workspace, module, api, stale, RunMode) + require.ErrorContains(t, err, "ambiguous", + "the environment that owns the stale managed-service key must still be refused") + + staging, err := SelectEnvironment(workspace, "staging") + require.NoError(t, err) + _, err = NewFlow(ctx, workspace, module, api, staging, RunMode) + require.NoError(t, err, "a flow is held to its own environment's managed services, never another's") + + // With no selection the whole workspace is still held, so the + // workspace-wide callers keep the behaviour they had. + require.ErrorContains(t, environments.ValidateManagedServices(ctx, workspace), "ambiguous", + "with no selection every environment's managed services are still held") + require.ErrorContains(t, environments.ValidateManagedServices(ctx, workspace, nil), "ambiguous", + "a nil selection is the same as none, not a silent skip") + + // And the check is still against the whole COMPOSITION rather than the + // run's closure: shop/api does not depend on anything in depot, so a + // closure-scoped check would not see the second "vault" at all. Selecting + // the environment that owns the key must still refuse it. + staleFlowService, err := module.LoadServiceFromName(ctx, "vault") + require.NoError(t, err) + _, err = NewFlow(ctx, workspace, module, staleFlowService, stale, RunMode) + require.ErrorContains(t, err, "ambiguous", + "ambiguity outside the run's closure is still judged, for the selected environment's own keys") +} + // TestFlowIsNotRefusedOverAnotherEnvironmentsExclusion is the Blue render // defect: a flow selecting one environment was refused over a DIFFERENT // environment's exclude-services. diff --git a/pkg/orchestration/flow.go b/pkg/orchestration/flow.go index e11c38a6..ed0cb23d 100644 --- a/pkg/orchestration/flow.go +++ b/pkg/orchestration/flow.go @@ -473,12 +473,14 @@ func NewFlow(ctx context.Context, workspace *resources.Workspace, module *resour if err := refuseInvocationOverridesInARender(mode); err != nil { return nil, w.Wrap(err) } - // Against the whole composition, not the run's closure: whether a bare - // managed-services key names one service is a property of the declaration, and - // a flow resolves managed services through env.ManagedService — the remote - // network manager asks it whether a service is replaced — without ever - // reaching the workspace-wide gitops pass. - if err := environments.ValidateManagedServices(ctx, workspace); err != nil { + // Against the whole composition's service graph, but only for THIS + // environment's keys: whether a bare managed-services key names one service + // is a property of the declaration, and a flow resolves managed services + // through env.ManagedService — the remote network manager asks it whether a + // service is replaced — without ever reaching the workspace-wide gitops + // pass. It asks the SELECTED environment, so another environment's stale + // bare key is not this flow's to answer for. + if err := environments.ValidateManagedServices(ctx, workspace, env); err != nil { return nil, w.Wrap(err) } if err := environments.ValidateEnvironmentExclusions(ctx, workspace, env); err != nil {