From 7445fdbe804a4ea11cffaca81e8778a553c5409c Mon Sep 17 00:00:00 2001 From: Eric Curtin Date: Fri, 23 Oct 2026 02:38:29 +0100 Subject: [PATCH 1/4] kubectl-ate: create every document in an actor-template manifest Signed-off-by: Eric Curtin --- cmd/ate-setup/internal/steps/substrate.go | 23 +- .../internal/steps/substrate_test.go | 12 + cmd/kubectl-ate/README.md | 7 +- cmd/kubectl-ate/internal/cmd/access_policy.go | 16 +- cmd/kubectl-ate/internal/cmd/actortemplate.go | 89 ++++--- .../internal/cmd/actortemplate_test.go | 187 ++++++++++++--- cmd/kubectl-ate/internal/cmd/egress_policy.go | 45 +--- .../internal/cmd/egress_policy_test.go | 41 ++-- internal/manifest/manifest.go | 123 ++++++++++ internal/manifest/manifest_test.go | 219 ++++++++++++++++++ 10 files changed, 612 insertions(+), 150 deletions(-) create mode 100644 internal/manifest/manifest.go create mode 100644 internal/manifest/manifest_test.go diff --git a/cmd/ate-setup/internal/steps/substrate.go b/cmd/ate-setup/internal/steps/substrate.go index c5a3be7801..79eb45aced 100644 --- a/cmd/ate-setup/internal/steps/substrate.go +++ b/cmd/ate-setup/internal/steps/substrate.go @@ -22,12 +22,11 @@ import ( "google.golang.org/grpc" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" - "google.golang.org/protobuf/encoding/protojson" - "sigs.k8s.io/yaml" "go.ate.dev/substrate/api/ateapi/v1" "go.ate.dev/substrate/cmd/ate-setup/internal/log" "go.ate.dev/substrate/internal/ateclient" + "go.ate.dev/substrate/internal/manifest" "go.ate.dev/substrate/internal/preview" "go.ate.dev/substrate/internal/resources" ) @@ -56,23 +55,15 @@ func EnsureAtespace(ctx context.Context, client *ateclient.Client, atespace stri return err } -// ActorTemplateFromManifest parses a single protojson-shaped YAML or JSON -// document into an ActorTemplate, as `kubectl ate create actor-template` -// does. Parsing is strict: unknown fields are an error, so typos don't -// silently drop configuration. -func ActorTemplateFromManifest(manifest []byte) (*ateapi.ActorTemplate, error) { - jsonData, err := yaml.YAMLToJSON(manifest) +// ActorTemplateFromManifest parses a manifest holding a single +// protojson-shaped ActorTemplate. Empty documents are skipped, such as the +// "---" ko resolve appends. +func ActorTemplateFromManifest(data []byte) (*ateapi.ActorTemplate, error) { + templates, err := manifest.Parse[ateapi.ActorTemplate](data) if err != nil { - return nil, fmt.Errorf("invalid YAML: %w", err) - } - if string(jsonData) == "null" { - return nil, fmt.Errorf("manifest is empty") - } - template := &ateapi.ActorTemplate{} - if err := protojson.Unmarshal(jsonData, template); err != nil { return nil, err } - return template, nil + return manifest.One(templates) } // CreateActorTemplate creates the template through the ate API. Actor diff --git a/cmd/ate-setup/internal/steps/substrate_test.go b/cmd/ate-setup/internal/steps/substrate_test.go index d29c0ec174..3422518e4b 100644 --- a/cmd/ate-setup/internal/steps/substrate_test.go +++ b/cmd/ate-setup/internal/steps/substrate_test.go @@ -131,6 +131,17 @@ func TestActorTemplateFromManifestDemos(t *testing.T) { } } +// ko resolve ends every document with a "---" separator. +func TestActorTemplateFromManifestKoOutput(t *testing.T) { + template, err := ActorTemplateFromManifest([]byte("metadata: {atespace: a, name: t}\n---\n")) + if err != nil { + t.Fatalf("ActorTemplateFromManifest: %v", err) + } + if got := template.GetMetadata().GetName(); got != "t" { + t.Errorf("name = %q, want t", got) + } +} + func TestActorTemplateFromManifestErrors(t *testing.T) { tests := []struct { name string @@ -138,6 +149,7 @@ func TestActorTemplateFromManifestErrors(t *testing.T) { }{ {name: "empty", manifest: ""}, {name: "not yaml", manifest: ":\t:"}, + {name: "second document", manifest: "metadata: {atespace: a, name: t}\n---\nmetadata: {atespace: a, name: u}\n"}, // Strict parsing: a typo must fail rather than silently drop the field. {name: "unknown field", manifest: "metadata:\n atespace: a\n name: t\nsnapshotsConfig: {}\n"}, } diff --git a/cmd/kubectl-ate/README.md b/cmd/kubectl-ate/README.md index 21c27a77e4..0540c567a0 100644 --- a/cmd/kubectl-ate/README.md +++ b/cmd/kubectl-ate/README.md @@ -164,9 +164,10 @@ one (or create a new template and repoint suspended actors to it with `kubectl ate update actor --template`). ```bash -# Create a template from a manifest (protojson-shaped ateapi.ActorTemplate, -# a single YAML/JSON document; use -f - for stdin). The metadata's atespace -# must already exist. +# Create templates from a manifest (protojson-shaped ateapi.ActorTemplate +# records; a YAML manifest may hold several documents, each created +# independently; use -f - for stdin). Each metadata's atespace must already +# exist. kubectl ate create actor-template -f template.yaml # List templates, or get one (also: -o yaml prints the re-applyable manifest). diff --git a/cmd/kubectl-ate/internal/cmd/access_policy.go b/cmd/kubectl-ate/internal/cmd/access_policy.go index 714aa693e1..4566700efa 100644 --- a/cmd/kubectl-ate/internal/cmd/access_policy.go +++ b/cmd/kubectl-ate/internal/cmd/access_policy.go @@ -24,10 +24,10 @@ import ( "google.golang.org/grpc" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" - "google.golang.org/protobuf/encoding/protojson" "go.ate.dev/substrate/api/ateapi/v1" "go.ate.dev/substrate/cmd/kubectl-ate/internal/printer" + "go.ate.dev/substrate/internal/manifest" "go.ate.dev/substrate/internal/resources" ) @@ -118,19 +118,15 @@ func scopeFromFlags(atespace string, global bool) (accessPolicyScope, error) { return accessPolicyScope{atespace: atespace}, nil } -// accessPolicyFromManifest parses a single protojson-shaped YAML or JSON -// document into an AccessPolicy. Parsing is strict: unknown fields are an -// error. +// accessPolicyFromManifest parses a manifest holding a single protojson-shaped +// AccessPolicy. An access policy is a singleton in its scope, so more than one +// record is an error. func accessPolicyFromManifest(data []byte) (*ateapi.AccessPolicy, error) { - jsonData, err := manifestToJSON(data) + policies, err := manifest.Parse[ateapi.AccessPolicy](data) if err != nil { return nil, err } - policy := &ateapi.AccessPolicy{} - if err := protojson.Unmarshal(jsonData, policy); err != nil { - return nil, fmt.Errorf("invalid AccessPolicy: %w", err) - } - return policy, nil + return manifest.One(policies) } // loadAccessPolicyManifest reads a manifest from filename, or from in when diff --git a/cmd/kubectl-ate/internal/cmd/actortemplate.go b/cmd/kubectl-ate/internal/cmd/actortemplate.go index 7794ab63fa..ea5c8f8c2a 100644 --- a/cmd/kubectl-ate/internal/cmd/actortemplate.go +++ b/cmd/kubectl-ate/internal/cmd/actortemplate.go @@ -15,6 +15,8 @@ package cmd import ( + "context" + "errors" "fmt" "io" "os" @@ -22,8 +24,8 @@ import ( "github.com/spf13/cobra" "go.ate.dev/substrate/api/ateapi/v1" "go.ate.dev/substrate/cmd/kubectl-ate/internal/printer" - "google.golang.org/protobuf/encoding/protojson" - "sigs.k8s.io/yaml" + "go.ate.dev/substrate/internal/manifest" + "google.golang.org/grpc" ) var ( @@ -92,15 +94,20 @@ var getActorTemplatesCmd = &cobra.Command{ var createActorTemplateCmd = &cobra.Command{ Use: "actor-template -f ", - Short: "Create an actor template from a manifest", - Long: `Create an actor template from a manifest file. - -The manifest is a single YAML (or JSON) document holding one ateapi.ActorTemplate -message in its protojson form, exactly as printed by -"kubectl ate get actor-template -a -o yaml". -The template's atespace and name come from the manifest's metadata. + Short: "Create actor templates from a manifest", + Long: `Create actor templates from a manifest file. + +A manifest is a valid JSON or YAML file (YAML files may contain multiple +documents, delimited by the YAML document separator: "---"). Each record is a +complete ateapi.ActorTemplate message in its protojson form, exactly as +printed by "kubectl ate get actor-template -a -o yaml". +Each template's atespace and name come from its own metadata. The atespace must already exist. +The templates are created independently: a failure does not stop the rest, and +the failures are reported together at the end. A manifest with one record +prints the template, one with several prints a list. + Actor templates are immutable: there is no update; delete and recreate to change one.`, Args: noArgs("the template name comes from the manifest passed to -f/--filename", "filename"), RunE: func(cmd *cobra.Command, args []string) error { @@ -108,7 +115,7 @@ Actor templates are immutable: there is no update; delete and recreate to change if err != nil { return err } - template, err := actorTemplateFromManifest(data) + templates, err := manifest.Parse[ateapi.ActorTemplate](data) if err != nil { return fmt.Errorf("failed to parse actor template manifest %q: %w", createActorTemplateFilenameFlag, err) } @@ -120,12 +127,46 @@ Actor templates are immutable: there is no update; delete and recreate to change } defer apiClient.Close() - resp, err := apiClient.CreateActorTemplate(ctx, &ateapi.CreateActorTemplateRequest{ActorTemplate: template}) + created, createErr := createActorTemplates(ctx, apiClient, templates) + err = printCreatedActorTemplates(cmd.OutOrStdout(), len(templates), created, outputFmt) + return errors.Join(createErr, err) + }, +} + +// printCreatedActorTemplates prints what was created. The shape follows the +// number of templates requested, not created, so a caller can parse the output +// even when some creates failed. +func printCreatedActorTemplates(out io.Writer, requested int, created []*ateapi.ActorTemplate, format string) error { + if len(created) == 0 { + return nil + } + if requested == 1 { + return printer.PrintActorTemplateTo(out, created[0], format) + } + return printer.PrintActorTemplatesTo(out, created, format) +} + +// actorTemplateCreator abstracts the CreateActorTemplate RPC. +type actorTemplateCreator interface { + CreateActorTemplate(ctx context.Context, req *ateapi.CreateActorTemplateRequest, opts ...grpc.CallOption) (*ateapi.ActorTemplate, error) +} + +// createActorTemplates creates each template independently. A failure does not +// stop the rest. It returns the templates that were created and the joined +// failures. +func createActorTemplates(ctx context.Context, creator actorTemplateCreator, templates []*ateapi.ActorTemplate) ([]*ateapi.ActorTemplate, error) { + var created []*ateapi.ActorTemplate + var errs []error + for _, template := range templates { + resp, err := creator.CreateActorTemplate(ctx, &ateapi.CreateActorTemplateRequest{ActorTemplate: template}) if err != nil { - return fmt.Errorf("failed to create actor template: %w", err) + errs = append(errs, fmt.Errorf("failed to create actor template %q in atespace %q: %w", + template.GetMetadata().GetName(), template.GetMetadata().GetAtespace(), err)) + continue } - return printer.PrintActorTemplateTo(cmd.OutOrStdout(), resp, outputFmt) - }, + created = append(created, resp) + } + return created, errors.Join(errs...) } var deleteActorTemplateCmd = &cobra.Command{ @@ -155,24 +196,6 @@ The server also deletes the template's golden actor and golden snapshot.`, }, } -// actorTemplateFromManifest parses a single protojson-shaped YAML or JSON -// document into an ActorTemplate. Parsing is strict: unknown fields are an -// error, so typos don't silently drop configuration. -func actorTemplateFromManifest(data []byte) (*ateapi.ActorTemplate, error) { - jsonData, err := yaml.YAMLToJSON(data) - if err != nil { - return nil, fmt.Errorf("invalid YAML: %w", err) - } - if string(jsonData) == "null" { - return nil, fmt.Errorf("manifest is empty") - } - template := &ateapi.ActorTemplate{} - if err := protojson.Unmarshal(jsonData, template); err != nil { - return nil, err - } - return template, nil -} - // readFileOrStdin reads the manifest from path, or from in when path is "-". func readFileOrStdin(in io.Reader, path string) ([]byte, error) { if path == "-" { @@ -186,7 +209,7 @@ func init() { getActorTemplatesCmd.Flags().BoolVarP(&getActorTemplateAllAtespacesFlag, "all-atespaces", "A", false, "get actor-templates across all atespaces (mutually exclusive with --atespace)") getCmd.AddCommand(getActorTemplatesCmd) - createActorTemplateCmd.Flags().StringVarP(&createActorTemplateFilenameFlag, "filename", "f", "", "manifest file holding a single JSON or YAML actor-template document, or - for stdin") + createActorTemplateCmd.Flags().StringVarP(&createActorTemplateFilenameFlag, "filename", "f", "", "JSON or YAML manifest of protojson-shaped ActorTemplates (YAML may hold several documents), or - for stdin") createCmd.AddCommand(createActorTemplateCmd) deleteActorTemplateCmd.Flags().StringVarP(&deleteActorTemplateAtespaceFlag, "atespace", "a", "", "the actor-template's atespace") diff --git a/cmd/kubectl-ate/internal/cmd/actortemplate_test.go b/cmd/kubectl-ate/internal/cmd/actortemplate_test.go index 5ce5d737fa..85e9ff8271 100644 --- a/cmd/kubectl-ate/internal/cmd/actortemplate_test.go +++ b/cmd/kubectl-ate/internal/cmd/actortemplate_test.go @@ -15,6 +15,9 @@ package cmd import ( + "bytes" + "context" + "errors" "os" "path/filepath" "strings" @@ -22,6 +25,10 @@ import ( "github.com/google/go-cmp/cmp" "go.ate.dev/substrate/api/ateapi/v1" + "go.ate.dev/substrate/internal/manifest" + "google.golang.org/grpc" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" "google.golang.org/protobuf/testing/protocmp" ) @@ -62,11 +69,22 @@ volumes: durableDir: {} ` -func TestActorTemplateFromManifest(t *testing.T) { - got, err := actorTemplateFromManifest([]byte(counterTemplateManifest)) +// parseTemplate parses a manifest that holds exactly one ActorTemplate. +func parseTemplate(t *testing.T, data string) *ateapi.ActorTemplate { + t.Helper() + templates, err := manifest.Parse[ateapi.ActorTemplate]([]byte(data)) if err != nil { - t.Fatalf("actorTemplateFromManifest: %v", err) + t.Fatalf("manifest.Parse: %v", err) } + template, err := manifest.One(templates) + if err != nil { + t.Fatalf("manifest.One: %v", err) + } + return template +} + +func TestParseActorTemplate(t *testing.T) { + got := parseTemplate(t, counterTemplateManifest) want := &ateapi.ActorTemplate{ Metadata: &ateapi.ResourceMetadata{Atespace: "ate-demo-counter", Name: "counter"}, @@ -95,39 +113,38 @@ func TestActorTemplateFromManifest(t *testing.T) { } } -func TestActorTemplateFromManifest_SnakeCase(t *testing.T) { +func TestParseActorTemplate_SnakeCase(t *testing.T) { // protojson accepts the proto field names as well as the json names. - manifest := `metadata: + got := parseTemplate(t, `metadata: atespace: ate-demo-counter name: counter snapshot_config: storage_location: gs://ate-snapshots/ate-demo-counter/ -` - got, err := actorTemplateFromManifest([]byte(manifest)) - if err != nil { - t.Fatalf("actorTemplateFromManifest: %v", err) - } +`) if got.GetSnapshotConfig().GetStorageLocation() != "gs://ate-snapshots/ate-demo-counter/" { t.Errorf("storage_location = %q", got.GetSnapshotConfig().GetStorageLocation()) } } -func TestActorTemplateFromManifest_Errors(t *testing.T) { +func TestParseActorTemplate_Errors(t *testing.T) { tests := []struct { - name string - manifest string + name string + manifest string + wantErrContains string }{ - {name: "empty", manifest: ""}, - {name: "unknown field", manifest: "metadata: {atespace: a, name: n}\nsandboxClass: gvisor\n"}, - {name: "bad enum", manifest: "snapshotConfig: {preferredFidelity: memory}\n"}, - {name: "resources limits list", manifest: "resources: {limits: [{name: cpu, quantity: \"1\"}]}\n"}, - {name: "crd shape", manifest: "apiVersion: ate.dev/v1\nkind: ActorTemplate\nmetadata: {name: counter}\n"}, - {name: "not yaml", manifest: "\t{"}, + {name: "unknown field", manifest: "metadata: {atespace: a, name: counter}\nsandboxClass: gvisor\n", wantErrContains: "unknown field"}, + {name: "bad enum", manifest: "snapshotConfig: {preferredFidelity: memory}\n", wantErrContains: "preferredFidelity"}, + {name: "resources limits list", manifest: "resources: {limits: [{name: cpu, quantity: \"1\"}]}\n", wantErrContains: "unexpected token"}, + {name: "crd shape", manifest: "apiVersion: ate.dev/v1\nkind: ActorTemplate\nmetadata: {name: counter}\n", wantErrContains: "apiVersion"}, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { - if got, err := actorTemplateFromManifest([]byte(test.manifest)); err == nil { - t.Fatalf("actorTemplateFromManifest succeeded: %v", got) + got, err := manifest.Parse[ateapi.ActorTemplate]([]byte(test.manifest)) + if err == nil { + t.Fatalf("manifest.Parse succeeded: %v", got) + } + if !strings.Contains(err.Error(), test.wantErrContains) { + t.Errorf("error = %q, want it to contain %q", err, test.wantErrContains) } }) } @@ -135,7 +152,7 @@ func TestActorTemplateFromManifest_Errors(t *testing.T) { // The counter demo's substrate template manifests must stay parseable by // `create actortemplate -f`; this pins them to the parser. -func TestActorTemplateFromManifest_DemoManifests(t *testing.T) { +func TestParseActorTemplate_DemoManifests(t *testing.T) { tests := []struct { manifest string atespace string @@ -170,10 +187,7 @@ func TestActorTemplateFromManifest_DemoManifests(t *testing.T) { } rendered = strings.Join(kept, "\n") - got, err := actorTemplateFromManifest([]byte(rendered)) - if err != nil { - t.Fatalf("actorTemplateFromManifest: %v", err) - } + got := parseTemplate(t, rendered) if got.GetMetadata().GetAtespace() != test.atespace || got.GetMetadata().GetName() != test.name { t.Errorf("metadata = %s/%s, want %s/%s", got.GetMetadata().GetAtespace(), got.GetMetadata().GetName(), test.atespace, test.name) @@ -194,3 +208,124 @@ func TestReadFileOrStdin(t *testing.T) { t.Fatal("readFileOrStdin on a missing file succeeded") } } + +// fakeActorTemplateCreator records the names it was asked to create and fails +// those listed in errs. +type fakeActorTemplateCreator struct { + errs map[string]error + asked []string +} + +func (f *fakeActorTemplateCreator) CreateActorTemplate(ctx context.Context, req *ateapi.CreateActorTemplateRequest, opts ...grpc.CallOption) (*ateapi.ActorTemplate, error) { + name := req.GetActorTemplate().GetMetadata().GetName() + f.asked = append(f.asked, name) + if err := f.errs[name]; err != nil { + return nil, err + } + return req.GetActorTemplate(), nil +} + +func TestCreateActorTemplates(t *testing.T) { + const manifestText = `metadata: {atespace: a, name: one} +--- +metadata: {atespace: a, name: two} +--- +metadata: {atespace: b, name: three} +` + alreadyExists := status.Error(codes.AlreadyExists, "exists") + tests := []struct { + name string + errs map[string]error + wantCreated []string + wantErrs []string + }{ + {name: "all created", wantCreated: []string{"one", "two", "three"}}, + { + name: "a failure does not stop the rest", + errs: map[string]error{"two": alreadyExists}, + wantCreated: []string{"one", "three"}, + wantErrs: []string{`"two" in atespace "a"`}, + }, + { + name: "every failure is reported", + errs: map[string]error{"one": alreadyExists, "two": alreadyExists, "three": alreadyExists}, + wantErrs: []string{`"one" in atespace "a"`, `"two" in atespace "a"`, `"three" in atespace "b"`}, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + templates, err := manifest.Parse[ateapi.ActorTemplate]([]byte(manifestText)) + if err != nil { + t.Fatalf("manifest.Parse: %v", err) + } + creator := &fakeActorTemplateCreator{errs: test.errs} + + created, err := createActorTemplates(t.Context(), creator, templates) + + if diff := cmp.Diff([]string{"one", "two", "three"}, creator.asked); diff != "" { + t.Errorf("templates attempted mismatch (-want +got):\n%s", diff) + } + var gotCreated []string + for _, template := range created { + gotCreated = append(gotCreated, template.GetMetadata().GetName()) + } + if diff := cmp.Diff(test.wantCreated, gotCreated); diff != "" { + t.Errorf("templates created mismatch (-want +got):\n%s", diff) + } + if len(test.wantErrs) == 0 { + if err != nil { + t.Fatalf("createActorTemplates() error = %v, want nil", err) + } + return + } + if err == nil { + t.Fatal("createActorTemplates() succeeded, want an error") + } + for _, want := range test.wantErrs { + if !strings.Contains(err.Error(), want) { + t.Errorf("error = %q, want it to contain %q", err, want) + } + } + if !errors.Is(err, alreadyExists) { + t.Errorf("error = %q does not wrap the RPC error", err) + } + }) + } +} + +func TestPrintCreatedActorTemplates(t *testing.T) { + one := &ateapi.ActorTemplate{Metadata: &ateapi.ResourceMetadata{Atespace: "a", Name: "one"}} + two := &ateapi.ActorTemplate{Metadata: &ateapi.ResourceMetadata{Atespace: "a", Name: "two"}} + tests := []struct { + name string + requested int + created []*ateapi.ActorTemplate + want string // substring of the JSON output, empty for none + wantList bool + }{ + {name: "one requested and created", requested: 1, created: []*ateapi.ActorTemplate{one}, want: `"name": "one"`}, + {name: "two requested and created", requested: 2, created: []*ateapi.ActorTemplate{one, two}, wantList: true}, + {name: "two requested, one created stays a list", requested: 2, created: []*ateapi.ActorTemplate{two}, wantList: true}, + {name: "none created prints nothing", requested: 2}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + var out bytes.Buffer + if err := printCreatedActorTemplates(&out, test.requested, test.created, "json"); err != nil { + t.Fatalf("printCreatedActorTemplates: %v", err) + } + if len(test.created) == 0 { + if out.Len() != 0 { + t.Errorf("output = %q, want none", out.String()) + } + return + } + if isList := strings.Contains(out.String(), "actorTemplates"); isList != test.wantList { + t.Errorf("list output = %v, want %v:\n%s", isList, test.wantList, out.String()) + } + if test.want != "" && !strings.Contains(out.String(), test.want) { + t.Errorf("output = %q, want it to contain %q", out.String(), test.want) + } + }) + } +} diff --git a/cmd/kubectl-ate/internal/cmd/egress_policy.go b/cmd/kubectl-ate/internal/cmd/egress_policy.go index a81785427e..34144b8a39 100644 --- a/cmd/kubectl-ate/internal/cmd/egress_policy.go +++ b/cmd/kubectl-ate/internal/cmd/egress_policy.go @@ -15,7 +15,6 @@ package cmd import ( - "bytes" "context" "errors" "fmt" @@ -24,13 +23,11 @@ import ( "github.com/spf13/cobra" "go.ate.dev/substrate/api/ateapi/v1" "go.ate.dev/substrate/cmd/kubectl-ate/internal/printer" + "go.ate.dev/substrate/internal/manifest" "go.ate.dev/substrate/internal/resources" "google.golang.org/grpc" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" - "google.golang.org/protobuf/encoding/protojson" - yamlv3 "gopkg.in/yaml.v3" - "sigs.k8s.io/yaml" ) var egressPolicyFlags struct { @@ -81,45 +78,15 @@ var deleteEgressPolicyCmd = &cobra.Command{ RunE: runDeleteEgressPolicy, } -// egressPolicyFromManifest parses a single protojson-shaped YAML or JSON -// document into an EgressPolicy. Parsing is strict: unknown fields are an -// error. +// egressPolicyFromManifest parses a manifest holding a single protojson-shaped +// EgressPolicy. An egress policy is a singleton under its actor, so more than +// one record is an error. func egressPolicyFromManifest(data []byte) (*ateapi.EgressPolicy, error) { - jsonData, err := manifestToJSON(data) + policies, err := manifest.Parse[ateapi.EgressPolicy](data) if err != nil { return nil, err } - policy := &ateapi.EgressPolicy{} - if err := protojson.Unmarshal(jsonData, policy); err != nil { - return nil, fmt.Errorf("invalid EgressPolicy: %w", err) - } - return policy, nil -} - -// manifestToJSON converts a YAML or JSON manifest to JSON. YAML is a superset -// of JSON, so one decoder parses both. The manifest must hold exactly one -// document, counted the way the YAML spec counts them: an empty document, such -// as the one a trailing "---" opens, is a document too, and is rejected. -func manifestToJSON(data []byte) ([]byte, error) { - dec := yamlv3.NewDecoder(bytes.NewReader(data)) - if err := dec.Decode(&yamlv3.Node{}); err != nil { - if errors.Is(err, io.EOF) { - return nil, errors.New("manifest is empty") - } - return nil, fmt.Errorf("invalid YAML: %w", err) - } - // Decode again to check that the manifest holds no second document. - if err := dec.Decode(&yamlv3.Node{}); !errors.Is(err, io.EOF) { - return nil, errors.New("manifest holds more than one document, expected one") - } - j, err := yaml.YAMLToJSON(data) - if err != nil { - return nil, fmt.Errorf("invalid YAML: %w", err) - } - if string(j) == "null" { - return nil, errors.New("manifest is empty") - } - return j, nil + return manifest.One(policies) } // overrideEgressPolicyMetadata defaults each of metadata.atespace and diff --git a/cmd/kubectl-ate/internal/cmd/egress_policy_test.go b/cmd/kubectl-ate/internal/cmd/egress_policy_test.go index 0ffa37d77b..cd6092d741 100644 --- a/cmd/kubectl-ate/internal/cmd/egress_policy_test.go +++ b/cmd/kubectl-ate/internal/cmd/egress_policy_test.go @@ -110,7 +110,7 @@ rules: }, }, { - name: "json input", + name: "single JSON", manifest: `{"metadata": {"atespace": "team-a", "name": "default"}, "rules": [{"tls_passthrough": {"hostnames": ["*.example.com"], "ports": {"numbers": [443]}}}]}`, want: &ateapi.EgressPolicy{Metadata: fullMetadata, Rules: tlsRules}, }, @@ -127,7 +127,7 @@ metadata: {name: default} }, {name: "not yaml", manifest: "\t{", wantErr: true, wantErrContains: "invalid YAML"}, { - name: "leading document separator", + name: "YAML: leading document separator", manifest: `--- rules: - http: {hostnames: ["*"]} @@ -135,7 +135,7 @@ rules: want: &ateapi.EgressPolicy{Rules: anyRules}, }, { - name: "document end marker", + name: "YAML: document end marker (...)", manifest: `rules: - http: {hostnames: ["*"]} ... @@ -143,54 +143,49 @@ rules: want: &ateapi.EgressPolicy{Rules: anyRules}, }, { - name: "trailing document separator", + name: "YAML: trailing document separator", manifest: `rules: - http: {hostnames: ["*"]} --- `, - wantErr: true, - wantErrContains: "manifest holds more than one document", + want: &ateapi.EgressPolicy{Rules: anyRules}, }, { - name: "empty second document", + name: "YAML: empty second document", manifest: `rules: - http: {hostnames: ["*"]} --- # nothing here `, - wantErr: true, - wantErrContains: "manifest holds more than one document", + want: &ateapi.EgressPolicy{Rules: anyRules}, }, { - name: "empty first document", + name: "YAML: empty first document", manifest: `--- # nothing --- rules: - http: {hostnames: ["*"]} `, - wantErr: true, - wantErrContains: "manifest holds more than one document", + want: &ateapi.EgressPolicy{Rules: anyRules}, }, { - name: "two leading separators", + name: "YAML: two leading document separators", manifest: `--- --- rules: - http: {hostnames: ["*"]} `, - wantErr: true, - wantErrContains: "manifest holds more than one document", + want: &ateapi.EgressPolicy{Rules: anyRules}, }, { - name: "document end marker then separator", + name: "YAML: document end marker (...) then document separator", manifest: `rules: - http: {hostnames: ["*"]} ... --- `, - wantErr: true, - wantErrContains: "manifest holds more than one document", + want: &ateapi.EgressPolicy{Rules: anyRules}, }, { name: "two egress policies separated by ---", @@ -214,7 +209,7 @@ rules: - 443 `, wantErr: true, - wantErrContains: "manifest holds more than one document", + wantErrContains: "manifest holds 2 EgressPolicy records, expected one", }, { name: "three egress policies separated by ---", @@ -231,7 +226,7 @@ rules: - http: {hostnames: ["*"]} `, wantErr: true, - wantErrContains: "manifest holds more than one document", + wantErrContains: "manifest holds 3 EgressPolicy records, expected one", }, { name: "two policies with an empty document between", @@ -246,16 +241,16 @@ rules: - tls_passthrough: {hostnames: ["*.example.com"], ports: {numbers: [443]}} `, wantErr: true, - wantErrContains: "manifest holds more than one document", + wantErrContains: "manifest holds 2 EgressPolicy records, expected one", }, { - name: `two json documents separated by ---`, + name: "YAML multi-document in JSON style", manifest: `{"metadata": {"atespace": "team-a", "name": "default"}, "rules": [{"http": {"hostnames": ["*"]}}]} --- {"metadata": {"atespace": "team-b", "name": "default"}, "rules": [{"http": {"hostnames": ["*"]}}]} `, wantErr: true, - wantErrContains: "manifest holds more than one document", + wantErrContains: "manifest holds 2 EgressPolicy records, expected one", }, {name: "only a separator", manifest: "---\n", wantErr: true, wantErrContains: "manifest is empty"}, {name: "comment only", manifest: "# nothing\n", wantErr: true, wantErrContains: "manifest is empty"}, diff --git a/internal/manifest/manifest.go b/internal/manifest/manifest.go new file mode 100644 index 0000000000..1dfe5fb2ba --- /dev/null +++ b/internal/manifest/manifest.go @@ -0,0 +1,123 @@ +// Copyright 2026 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Package manifest parses manifests, in their protojson form, into protobuf +// messages. A manifest is either a JSON file, which holds one record, or a YAML +// file, which holds any number of documents delimited by the YAML document +// separator ("---"). JSON has no multi-document form, so several JSON objects +// separated by "---" are a YAML file, not a JSON one. +package manifest + +import ( + "bytes" + "errors" + "fmt" + "io" + + "google.golang.org/protobuf/encoding/protojson" + "google.golang.org/protobuf/proto" + yamlv3 "gopkg.in/yaml.v3" + "sigs.k8s.io/yaml" +) + +// message constrains PM to a pointer to the proto message type M. +type message[M any] interface { + *M + proto.Message +} + +// Parse parses each record of a manifest into an M, in order. Each non-empty +// YAML document is a record, and a JSON file is a single record. Empty +// documents are skipped, as kubectl does. Parsing is strict: unknown fields are +// an error, so typos don't silently drop configuration. +func Parse[M any, PM message[M]](data []byte) ([]PM, error) { + docs, err := splitDocuments(data) + if err != nil { + return nil, err + } + var msgs []PM + for i, doc := range docs { + if isEmpty(doc) { + continue + } + msg, err := unmarshal[M, PM](doc) + if err != nil { + if len(docs) > 1 { + err = fmt.Errorf("document %d: %w", i+1, err) + } + return nil, err + } + msgs = append(msgs, msg) + } + if len(msgs) == 0 { + return nil, errEmpty + } + return msgs, nil +} + +// One returns the only record of msgs, as returned by Parse, for callers that +// accept a single record. It is an error if there is more than one. +func One[M any, PM message[M]](msgs []PM) (PM, error) { + switch len(msgs) { + case 0: + return nil, errEmpty + case 1: + return msgs[0], nil + } + return nil, fmt.Errorf("manifest holds %d %s records, expected one", len(msgs), msgs[0].ProtoReflect().Descriptor().Name()) +} + +var errEmpty = errors.New("manifest is empty") + +// splitDocuments returns the JSON form of each YAML document in data, and +// errEmpty when there are none. YAML is a superset of JSON, so one decoder +// parses both. +func splitDocuments(data []byte) ([][]byte, error) { + dec := yamlv3.NewDecoder(bytes.NewReader(data)) + var docs [][]byte + for { + var node yamlv3.Node + if err := dec.Decode(&node); err != nil { + if errors.Is(err, io.EOF) { + break + } + return nil, fmt.Errorf("invalid YAML: %w", err) + } + // sigs.k8s.io/yaml converts YAML to JSON the way Kubernetes does, but + // reads only the first document, so hand it one document at a time. + single, err := yamlv3.Marshal(&node) + if err != nil { + return nil, fmt.Errorf("invalid YAML: %w", err) + } + doc, err := yaml.YAMLToJSON(single) + if err != nil { + return nil, fmt.Errorf("invalid YAML: %w", err) + } + docs = append(docs, doc) + } + if len(docs) == 0 { + return nil, errEmpty + } + return docs, nil +} + +func isEmpty(doc []byte) bool { return string(doc) == "null" } + +func unmarshal[M any, PM message[M]](doc []byte) (PM, error) { + msg := PM(new(M)) + if err := protojson.Unmarshal(doc, msg); err != nil { + return nil, fmt.Errorf("invalid %s: %w", msg.ProtoReflect().Descriptor().Name(), err) + } + return msg, nil +} diff --git a/internal/manifest/manifest_test.go b/internal/manifest/manifest_test.go new file mode 100644 index 0000000000..7b9ca68dd2 --- /dev/null +++ b/internal/manifest/manifest_test.go @@ -0,0 +1,219 @@ +// Copyright 2026 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package manifest + +import ( + "strings" + "testing" + + "github.com/google/go-cmp/cmp" + "go.ate.dev/substrate/api/ateapi/v1" + "google.golang.org/protobuf/testing/protocmp" +) + +// The tests parse into ateapi.Atespace, whose only field is a nested message, +// so that flow-style YAML has something to nest. +func TestParse(t *testing.T) { + tests := []struct { + name string + manifest string + want []*ateapi.Atespace + wantErrContains string + }{ + { + name: "single block-style YAML", + manifest: `metadata: + atespace: a + name: one +`, + want: []*ateapi.Atespace{{Metadata: &ateapi.ResourceMetadata{Atespace: "a", Name: "one"}}}, + }, + { + name: "single flow-style YAML", + manifest: "metadata: { atespace: a, name: one, }\n", + want: []*ateapi.Atespace{{Metadata: &ateapi.ResourceMetadata{Atespace: "a", Name: "one"}}}, + }, + { + name: "single JSON", + manifest: `{"metadata": {"atespace": "a", "name": "one"}}`, + want: []*ateapi.Atespace{{Metadata: &ateapi.ResourceMetadata{Atespace: "a", Name: "one"}}}, + }, + { + name: "YAML: leading document separator", + manifest: `--- +metadata: + atespace: a + name: one +`, + want: []*ateapi.Atespace{{Metadata: &ateapi.ResourceMetadata{Atespace: "a", Name: "one"}}}, + }, + { + name: "YAML: document end marker (...)", + manifest: `metadata: + atespace: a + name: one +... +`, + want: []*ateapi.Atespace{{Metadata: &ateapi.ResourceMetadata{Atespace: "a", Name: "one"}}}, + }, + { + name: "YAML: trailing document separator", + manifest: `metadata: + atespace: a + name: one +--- +`, + want: []*ateapi.Atespace{{Metadata: &ateapi.ResourceMetadata{Atespace: "a", Name: "one"}}}, + }, + { + name: "YAML multi-document in block style", + manifest: `metadata: + atespace: a + name: one +--- +metadata: + atespace: a + name: two +`, + want: []*ateapi.Atespace{ + {Metadata: &ateapi.ResourceMetadata{Atespace: "a", Name: "one"}}, + {Metadata: &ateapi.ResourceMetadata{Atespace: "a", Name: "two"}}, + }, + }, + { + name: "YAML multi-document in flow style", + manifest: `metadata: { atespace: a, name: one, } +--- +metadata: { atespace: a, name: two, } +`, + want: []*ateapi.Atespace{ + {Metadata: &ateapi.ResourceMetadata{Atespace: "a", Name: "one"}}, + {Metadata: &ateapi.ResourceMetadata{Atespace: "a", Name: "two"}}, + }, + }, + { + name: "YAML multi-document in JSON style", + manifest: `{"metadata": {"atespace": "a", "name": "one"}} +--- +{"metadata": {"atespace": "a", "name": "two"}} +`, + want: []*ateapi.Atespace{ + {Metadata: &ateapi.ResourceMetadata{Atespace: "a", Name: "one"}}, + {Metadata: &ateapi.ResourceMetadata{Atespace: "a", Name: "two"}}, + }, + }, + { + name: "YAML: empty documents are skipped", + manifest: `--- +# nothing +--- +metadata: { atespace: a, name: one, } +--- +--- +metadata: { atespace: a, name: two, } +--- +`, + want: []*ateapi.Atespace{ + {Metadata: &ateapi.ResourceMetadata{Atespace: "a", Name: "one"}}, + {Metadata: &ateapi.ResourceMetadata{Atespace: "a", Name: "two"}}, + }, + }, + { + name: "YAML: alias", + manifest: "metadata: { atespace: &x a, name: *x, }\n", + want: []*ateapi.Atespace{{Metadata: &ateapi.ResourceMetadata{Atespace: "a", Name: "a"}}}, + }, + {name: "empty", manifest: "", wantErrContains: "manifest is empty"}, + {name: "YAML: only document separators", manifest: "---\n---\n", wantErrContains: "manifest is empty"}, + {name: "YAML: comment only", manifest: "# nothing\n", wantErrContains: "manifest is empty"}, + {name: "not YAML", manifest: "\t{", wantErrContains: "invalid YAML"}, + { + name: "YAML: invalid second document", + manifest: "metadata: { atespace: a, name: one, }\n---\n\t{\n", + wantErrContains: "invalid YAML", + }, + { + name: "JSON documents with no separator are not supported", + manifest: `{"metadata": {"atespace": "a", "name": "one"}} +{"metadata": {"atespace": "a", "name": "two"}} +`, + wantErrContains: "invalid YAML", + }, + { + name: "unknown field", + manifest: `{"metadata": {"atespace": "a", "nmae": "one"}}`, + wantErrContains: "invalid Atespace", + }, + { + name: "YAML: unknown field in second document", + manifest: `metadata: { atespace: a, name: one, } +--- +metadata: { atespace: a, nmae: two, } +`, + wantErrContains: "document 2: invalid Atespace", + }, + {name: "not an object", manifest: "- a\n", wantErrContains: "invalid Atespace"}, + {name: "JSON array of objects is not supported", manifest: `[{"metadata": {"atespace": "a", "name": "one"}}]`, wantErrContains: "invalid Atespace"}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + got, err := Parse[ateapi.Atespace]([]byte(test.manifest)) + if test.wantErrContains != "" { + if err == nil || !strings.Contains(err.Error(), test.wantErrContains) { + t.Fatalf("Parse() error = %v, want it to contain %q", err, test.wantErrContains) + } + return + } + if err != nil { + t.Fatalf("Parse() error = %v", err) + } + if diff := cmp.Diff(test.want, got, protocmp.Transform()); diff != "" { + t.Errorf("Parse() mismatch (-want +got):\n%s", diff) + } + }) + } +} + +func TestOne(t *testing.T) { + one := &ateapi.Atespace{Metadata: &ateapi.ResourceMetadata{Atespace: "a", Name: "one"}} + two := &ateapi.Atespace{Metadata: &ateapi.ResourceMetadata{Atespace: "a", Name: "two"}} + tests := []struct { + name string + msgs []*ateapi.Atespace + want *ateapi.Atespace + wantErrContains string + }{ + {name: "one record", msgs: []*ateapi.Atespace{one}, want: one}, + {name: "no records", wantErrContains: "manifest is empty"}, + {name: "two records", msgs: []*ateapi.Atespace{one, two}, wantErrContains: "manifest holds 2 Atespace records, expected one"}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + got, err := One(test.msgs) + if test.wantErrContains != "" { + if err == nil || !strings.Contains(err.Error(), test.wantErrContains) { + t.Fatalf("One() error = %v, want it to contain %q", err, test.wantErrContains) + } + return + } + if err != nil { + t.Fatalf("One() error = %v", err) + } + if diff := cmp.Diff(test.want, got, protocmp.Transform()); diff != "" { + t.Errorf("One() mismatch (-want +got):\n%s", diff) + } + }) + } +} From d4200e494bbd0189c3c34ac9edc6104e00dd1e03 Mon Sep 17 00:00:00 2001 From: Eric Curtin Date: Mon, 26 Oct 2026 13:10:30 +0000 Subject: [PATCH 2/4] kubectl-ate: clarify manifest tests and help Signed-off-by: Eric Curtin --- cmd/kubectl-ate/internal/cmd/access_policy.go | 4 +- .../internal/cmd/actortemplate_test.go | 56 +++++++++++++------ cmd/kubectl-ate/internal/cmd/egress_policy.go | 3 +- internal/manifest/manifest_test.go | 12 ++-- 4 files changed, 49 insertions(+), 26 deletions(-) diff --git a/cmd/kubectl-ate/internal/cmd/access_policy.go b/cmd/kubectl-ate/internal/cmd/access_policy.go index 4566700efa..86387f11c0 100644 --- a/cmd/kubectl-ate/internal/cmd/access_policy.go +++ b/cmd/kubectl-ate/internal/cmd/access_policy.go @@ -40,8 +40,8 @@ var accessPolicyFlags struct { } const accessPolicyManifestHelp = `The manifest is a YAML or JSON AccessPolicy, as printed by -"kubectl ate get access-policy -o yaml". Its metadata may be omitted. Members -are principals with a "user:" prefix, e.g. +"kubectl ate get access-policy -o yaml". Its metadata may be omitted. More than +one record is an error. Members are principals with a "user:" prefix, e.g. "user:system:serviceaccount:team-a:alice". bindings: diff --git a/cmd/kubectl-ate/internal/cmd/actortemplate_test.go b/cmd/kubectl-ate/internal/cmd/actortemplate_test.go index 85e9ff8271..24ccb5f0b7 100644 --- a/cmd/kubectl-ate/internal/cmd/actortemplate_test.go +++ b/cmd/kubectl-ate/internal/cmd/actortemplate_test.go @@ -226,6 +226,7 @@ func (f *fakeActorTemplateCreator) CreateActorTemplate(ctx context.Context, req } func TestCreateActorTemplates(t *testing.T) { + // Three templates, in two atespaces. const manifestText = `metadata: {atespace: a, name: one} --- metadata: {atespace: a, name: two} @@ -300,31 +301,50 @@ func TestPrintCreatedActorTemplates(t *testing.T) { name string requested int created []*ateapi.ActorTemplate - want string // substring of the JSON output, empty for none - wantList bool + want string }{ - {name: "one requested and created", requested: 1, created: []*ateapi.ActorTemplate{one}, want: `"name": "one"`}, - {name: "two requested and created", requested: 2, created: []*ateapi.ActorTemplate{one, two}, wantList: true}, - {name: "two requested, one created stays a list", requested: 2, created: []*ateapi.ActorTemplate{two}, wantList: true}, - {name: "none created prints nothing", requested: 2}, + { + name: "one requested, one created: a template", + requested: 1, + created: []*ateapi.ActorTemplate{one}, + want: `metadata: + atespace: a + name: one +`, + }, + { + name: "two requested, two created: a list", + requested: 2, + created: []*ateapi.ActorTemplate{one, two}, + want: `actorTemplates: +- metadata: + atespace: a + name: one +- metadata: + atespace: a + name: two +`, + }, + { + name: "two requested, one created: still a list", + requested: 2, + created: []*ateapi.ActorTemplate{two}, + want: `actorTemplates: +- metadata: + atespace: a + name: two +`, + }, + {name: "two requested, none created: no output", requested: 2}, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { var out bytes.Buffer - if err := printCreatedActorTemplates(&out, test.requested, test.created, "json"); err != nil { + if err := printCreatedActorTemplates(&out, test.requested, test.created, "yaml"); err != nil { t.Fatalf("printCreatedActorTemplates: %v", err) } - if len(test.created) == 0 { - if out.Len() != 0 { - t.Errorf("output = %q, want none", out.String()) - } - return - } - if isList := strings.Contains(out.String(), "actorTemplates"); isList != test.wantList { - t.Errorf("list output = %v, want %v:\n%s", isList, test.wantList, out.String()) - } - if test.want != "" && !strings.Contains(out.String(), test.want) { - t.Errorf("output = %q, want it to contain %q", out.String(), test.want) + if diff := cmp.Diff(test.want, out.String()); diff != "" { + t.Errorf("output mismatch (-want +got):\n%s", diff) } }) } diff --git a/cmd/kubectl-ate/internal/cmd/egress_policy.go b/cmd/kubectl-ate/internal/cmd/egress_policy.go index 34144b8a39..185ec2360c 100644 --- a/cmd/kubectl-ate/internal/cmd/egress_policy.go +++ b/cmd/kubectl-ate/internal/cmd/egress_policy.go @@ -53,7 +53,7 @@ var createEgressPolicyCmd = &cobra.Command{ The manifest is a YAML or JSON EgressPolicy, as printed by "kubectl ate get egress-policy -a -o yaml". -Its metadata may be omitted.`, +Its metadata may be omitted. More than one record is an error.`, Args: exactlyOneArg("", "atespace", "filename"), RunE: runCreateEgressPolicy, } @@ -65,6 +65,7 @@ var updateEgressPolicyCmd = &cobra.Command{ The manifest is a YAML or JSON EgressPolicy, as printed by "kubectl ate get egress-policy -a -o yaml". +More than one record is an error. Its metadata.uid and metadata.version are required preconditions; the server rejects the update when they no longer match those of the stored egress policy.`, Args: exactlyOneArg("", "atespace", "filename"), diff --git a/internal/manifest/manifest_test.go b/internal/manifest/manifest_test.go index 7b9ca68dd2..3f62b4521c 100644 --- a/internal/manifest/manifest_test.go +++ b/internal/manifest/manifest_test.go @@ -32,6 +32,7 @@ func TestParse(t *testing.T) { want []*ateapi.Atespace wantErrContains string }{ + // Supported. { name: "single block-style YAML", manifest: `metadata: @@ -135,17 +136,18 @@ metadata: { atespace: a, name: two, } manifest: "metadata: { atespace: &x a, name: *x, }\n", want: []*ateapi.Atespace{{Metadata: &ateapi.ResourceMetadata{Atespace: "a", Name: "a"}}}, }, - {name: "empty", manifest: "", wantErrContains: "manifest is empty"}, + // Rejected. + {name: "empty input", manifest: "", wantErrContains: "manifest is empty"}, {name: "YAML: only document separators", manifest: "---\n---\n", wantErrContains: "manifest is empty"}, {name: "YAML: comment only", manifest: "# nothing\n", wantErrContains: "manifest is empty"}, - {name: "not YAML", manifest: "\t{", wantErrContains: "invalid YAML"}, + {name: "not YAML: stray tab", manifest: "\t{", wantErrContains: "invalid YAML"}, { name: "YAML: invalid second document", manifest: "metadata: { atespace: a, name: one, }\n---\n\t{\n", wantErrContains: "invalid YAML", }, { - name: "JSON documents with no separator are not supported", + name: "JSON objects with no separator", manifest: `{"metadata": {"atespace": "a", "name": "one"}} {"metadata": {"atespace": "a", "name": "two"}} `, @@ -164,8 +166,8 @@ metadata: { atespace: a, nmae: two, } `, wantErrContains: "document 2: invalid Atespace", }, - {name: "not an object", manifest: "- a\n", wantErrContains: "invalid Atespace"}, - {name: "JSON array of objects is not supported", manifest: `[{"metadata": {"atespace": "a", "name": "one"}}]`, wantErrContains: "invalid Atespace"}, + {name: "YAML: top-level sequence", manifest: "- a\n", wantErrContains: "invalid Atespace"}, + {name: "JSON: top-level array", manifest: `[{"metadata": {"atespace": "a", "name": "one"}}]`, wantErrContains: "invalid Atespace"}, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { From 5b7b93b27065d77ce6f44e8ec524b4e89563c044 Mon Sep 17 00:00:00 2001 From: Eric Curtin Date: Fri, 30 Oct 2026 14:11:51 +0000 Subject: [PATCH 3/4] e2e: decode fixture templates with internal/manifest Signed-off-by: Eric Curtin --- internal/e2e/fixture.go | 31 +++++++------------------------ 1 file changed, 7 insertions(+), 24 deletions(-) diff --git a/internal/e2e/fixture.go b/internal/e2e/fixture.go index 5410bb7c0c..4ae3750c1c 100644 --- a/internal/e2e/fixture.go +++ b/internal/e2e/fixture.go @@ -17,15 +17,13 @@ package e2e import ( "context" "maps" - "strings" "testing" "time" "go.ate.dev/substrate/api/ateapi/v1" + "go.ate.dev/substrate/internal/manifest" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" - "google.golang.org/protobuf/encoding/protojson" - "sigs.k8s.io/yaml" ) // substrateTemplateSubstitutions is the placeholder set the protojson-shaped @@ -60,29 +58,14 @@ func substrateTemplateSubstitutions(name string, trustBundles []string) (inline, } // decodeSubstrateTemplates strict-decodes a rendered manifest of one or more -// ---separated protojson-shaped ActorTemplate documents. Strict, so a block -// placeholder landing at the wrong depth or a misspelled field fails here -// rather than applying cleanly and doing nothing — the same contract -// `kubectl ate create actor-template` enforces. +// protojson-shaped ActorTemplate documents, so a block placeholder landing at +// the wrong depth or a misspelled field fails here rather than applying cleanly +// and doing nothing. func decodeSubstrateTemplates(t *testing.T, rendered []byte) []*ateapi.ActorTemplate { t.Helper() - var templates []*ateapi.ActorTemplate - for doc := range strings.SplitSeq(string(rendered), "\n---") { - jsonData, err := yaml.YAMLToJSON([]byte(doc)) - if err != nil { - t.Fatalf("invalid YAML in ActorTemplate manifest: %v", err) - } - if string(jsonData) == "null" { - continue - } - tmpl := &ateapi.ActorTemplate{} - if err := protojson.Unmarshal(jsonData, tmpl); err != nil { - t.Fatalf("decoding protojson ActorTemplate: %v", err) - } - templates = append(templates, tmpl) - } - if len(templates) == 0 { - t.Fatalf("ActorTemplate manifest holds no documents") + templates, err := manifest.Parse[ateapi.ActorTemplate](rendered) + if err != nil { + t.Fatalf("decoding ActorTemplate manifest: %v", err) } return templates } From b1857fce5de6d445096e8abfd79c5db823aa3582 Mon Sep 17 00:00:00 2001 From: Eric Curtin Date: Sun, 8 Nov 2026 12:13:12 +0000 Subject: [PATCH 4/4] Use Agent Substrate Authors copyright Signed-off-by: Eric Curtin --- internal/manifest/manifest.go | 2 +- internal/manifest/manifest_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/manifest/manifest.go b/internal/manifest/manifest.go index 1dfe5fb2ba..f027862b42 100644 --- a/internal/manifest/manifest.go +++ b/internal/manifest/manifest.go @@ -1,4 +1,4 @@ -// Copyright 2026 Google LLC +// Copyright 2026 The Agent Substrate Authors // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. diff --git a/internal/manifest/manifest_test.go b/internal/manifest/manifest_test.go index 3f62b4521c..e22e6f6d1b 100644 --- a/internal/manifest/manifest_test.go +++ b/internal/manifest/manifest_test.go @@ -1,4 +1,4 @@ -// Copyright 2026 Google LLC +// Copyright 2026 The Agent Substrate Authors // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License.