From 5da0da5beb9c44cd33ac5d1ce62dca61c0f4dcdc Mon Sep 17 00:00:00 2001 From: Derek Richard Date: Thu, 18 Jun 2026 17:00:04 -0400 Subject: [PATCH 1/3] feat(carvel): support structured BOSH variable declarations in base.yml Changes Variables []string to []proofing.Variable in both Metadata structs. Adds validateVariables() to baker.go to catch missing name/type at bake time. Covers with 4 Ginkgo specs. TNZ-112157 Co-authored-by: Cursor --- internal/carvel/baker.go | 20 +++++++++++++ internal/carvel/baker_test.go | 41 +++++++++++++++++++++++++- internal/carvel/models/metadata.go | 28 ++++++++++-------- internal/carvel/models/metadata_out.go | 36 +++++++++++----------- 4 files changed, 94 insertions(+), 31 deletions(-) diff --git a/internal/carvel/baker.go b/internal/carvel/baker.go index 35852a833..740d6ea36 100644 --- a/internal/carvel/baker.go +++ b/internal/carvel/baker.go @@ -17,6 +17,7 @@ import ( "github.com/pivotal-cf/kiln/internal/carvel/models" "github.com/pivotal-cf/kiln/pkg/cargo" + "github.com/pivotal-cf/kiln/pkg/proofing" "github.com/hashicorp/go-version" "gopkg.in/yaml.v3" @@ -89,6 +90,9 @@ func (b *baker) Bake(source string) error { if err != nil { return err } + if err := validateVariables(b.metadata.Variables); err != nil { + return err + } ver, err := b.GetVersion() if err != nil { @@ -137,6 +141,9 @@ func (b *baker) BakeFromLockfile(source string, releaseLock cargo.BOSHReleaseTar if err != nil { return err } + if err := validateVariables(b.metadata.Variables); err != nil { + return err + } ver, err := b.GetVersion() if err != nil { @@ -248,6 +255,19 @@ func (b *baker) progress(message string) { _, _ = fmt.Fprintln(b.progressWriter, message) } +// validateVariables checks that each variable declaration has the required name and type fields. +func validateVariables(vars []proofing.Variable) error { + for i, v := range vars { + if v.Name == "" { + return fmt.Errorf("variables[%d]: missing required field 'name'", i) + } + if v.Type == "" { + return fmt.Errorf("variables[%d] (%q): missing required field 'type'", i, v.Name) + } + } + return nil +} + func (b *baker) generateBoshReleaseDir() error { dirName := path.Join(b.source, ".boshrelease") err := os.RemoveAll(dirName) diff --git a/internal/carvel/baker_test.go b/internal/carvel/baker_test.go index 740743f14..ede3eb073 100644 --- a/internal/carvel/baker_test.go +++ b/internal/carvel/baker_test.go @@ -14,6 +14,7 @@ import ( . "github.com/onsi/gomega" "github.com/pivotal-cf/kiln/internal/carvel/models" "github.com/pivotal-cf/kiln/pkg/cargo" + "github.com/pivotal-cf/kiln/pkg/proofing" "gopkg.in/yaml.v3" ) @@ -297,7 +298,7 @@ var _ = Describe("Carvel Baker", func() { `$( property "admin_password" )`, }, FormTypes: []string{`$( form "db_props" )`}, - Variables: []string{}, + Variables: []proofing.Variable{}, PackageInstalls: []string{`$( package "test-install" )`}, } yamlData, err := yaml.Marshal(&m) @@ -591,4 +592,42 @@ var _ = Describe("Carvel Baker", func() { Expect(nonEmpty).To(Equal(5)) }) }) + + Context("validateVariables", func() { + It("passes for an empty list", func() { + err := validateVariables([]proofing.Variable{}) + Expect(err).NotTo(HaveOccurred()) + }) + + It("passes for a valid certificate variable", func() { + err := validateVariables([]proofing.Variable{ + { + Name: "/cf/diego-instance-identity-root-ca-2-6", + Type: "certificate", + Options: map[string]any{ + "common_name": "Diego Instance Identity Root CA", + "is_ca": true, + "duration": 1095, + }, + }, + }) + Expect(err).NotTo(HaveOccurred()) + }) + + It("errors when name is empty", func() { + err := validateVariables([]proofing.Variable{ + {Name: "", Type: "certificate"}, + }) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("missing required field 'name'")) + }) + + It("errors when type is empty", func() { + err := validateVariables([]proofing.Variable{ + {Name: "my-var", Type: ""}, + }) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("missing required field 'type'")) + }) + }) }) diff --git a/internal/carvel/models/metadata.go b/internal/carvel/models/metadata.go index a22b21d3c..7f7312826 100644 --- a/internal/carvel/models/metadata.go +++ b/internal/carvel/models/metadata.go @@ -1,17 +1,19 @@ package models +import "github.com/pivotal-cf/kiln/pkg/proofing" + type Metadata struct { - Name string `yaml:"name"` - ProductVersion string `yaml:"product_version"` - IconImage string `yaml:"icon_image"` - Label string `yaml:"label"` - MetadataVersion string `yaml:"metadata_version"` - MinimumVersionForUpgrade string `yaml:"minimum_version_for_upgrade"` - Rank int `yaml:"rank"` - Serial bool `yaml:"serial"` - PropertyBlueprints []string `yaml:"property_blueprints"` - FormTypes []string `yaml:"form_types"` - Variables []string `yaml:"variables"` - PackageInstalls []string `yaml:"package_installs"` - CompatibleKubernetesDistributions []ProductVersion `yaml:"compatible_kubernetes_distributions,omitempty"` + Name string `yaml:"name"` + ProductVersion string `yaml:"product_version"` + IconImage string `yaml:"icon_image"` + Label string `yaml:"label"` + MetadataVersion string `yaml:"metadata_version"` + MinimumVersionForUpgrade string `yaml:"minimum_version_for_upgrade"` + Rank int `yaml:"rank"` + Serial bool `yaml:"serial"` + PropertyBlueprints []string `yaml:"property_blueprints"` + FormTypes []string `yaml:"form_types"` + Variables []proofing.Variable `yaml:"variables"` + PackageInstalls []string `yaml:"package_installs"` + CompatibleKubernetesDistributions []ProductVersion `yaml:"compatible_kubernetes_distributions,omitempty"` } diff --git a/internal/carvel/models/metadata_out.go b/internal/carvel/models/metadata_out.go index 88b652e96..3a4bed4a9 100644 --- a/internal/carvel/models/metadata_out.go +++ b/internal/carvel/models/metadata_out.go @@ -1,23 +1,25 @@ package models +import "github.com/pivotal-cf/kiln/pkg/proofing" + type MetadataOut struct { - Name string `yaml:"name"` - ProductVersion string `yaml:"product_version"` - IconImage string `yaml:"icon_image"` - Label string `yaml:"label"` - MetadataVersion string `yaml:"metadata_version"` - MinimumVersionForUpgrade string `yaml:"minimum_version_for_upgrade"` - Rank int `yaml:"rank"` - Serial bool `yaml:"serial"` - PropertyBlueprints []string `yaml:"property_blueprints"` - FormTypes []string `yaml:"form_types"` - Variables []string `yaml:"variables"` - InstanceGroups []string `yaml:"job_types"` - StemcellCriteria StemcellCriteria `yaml:"stemcell_criteria"` - Releases []string `yaml:"releases"` - RuntimeConfigs []string `yaml:"runtime_configs"` - RequiresKubernetes bool `yaml:"requires_kubernetes"` - CompatibleKubernetesDistributions []ProductVersion `yaml:"compatible_kubernetes_distributions"` + Name string `yaml:"name"` + ProductVersion string `yaml:"product_version"` + IconImage string `yaml:"icon_image"` + Label string `yaml:"label"` + MetadataVersion string `yaml:"metadata_version"` + MinimumVersionForUpgrade string `yaml:"minimum_version_for_upgrade"` + Rank int `yaml:"rank"` + Serial bool `yaml:"serial"` + PropertyBlueprints []string `yaml:"property_blueprints"` + FormTypes []string `yaml:"form_types"` + Variables []proofing.Variable `yaml:"variables"` + InstanceGroups []string `yaml:"job_types"` + StemcellCriteria StemcellCriteria `yaml:"stemcell_criteria"` + Releases []string `yaml:"releases"` + RuntimeConfigs []string `yaml:"runtime_configs"` + RequiresKubernetes bool `yaml:"requires_kubernetes"` + CompatibleKubernetesDistributions []ProductVersion `yaml:"compatible_kubernetes_distributions"` } type StemcellCriteria struct { From 0cbe74e8027d02a76d45d035e20203fe97f50296 Mon Sep 17 00:00:00 2001 From: Derek Richard Date: Thu, 18 Jun 2026 17:00:53 -0400 Subject: [PATCH 2/3] test(carvel): add structured variable to sample-tile for round-trip coverage Updates testdata/sample-tile/base.yml from variables: [] to a real certificate variable declaration, and updates the "populates the output metadata" integration test to assert the variable is preserved through the bake process. TNZ-112157 Co-authored-by: Cursor --- internal/carvel/baker_test.go | 4 +++- internal/carvel/testdata/sample-tile/base.yml | 8 +++++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/internal/carvel/baker_test.go b/internal/carvel/baker_test.go index ede3eb073..f21cec8d4 100644 --- a/internal/carvel/baker_test.go +++ b/internal/carvel/baker_test.go @@ -170,7 +170,9 @@ var _ = Describe("Carvel Baker", func() { Expect(outMeta.Serial).To(BeFalse()) Expect(outMeta.PropertyBlueprints).To(HaveLen(2)) Expect(outMeta.FormTypes).To(HaveLen(1)) - Expect(outMeta.Variables).To(BeEmpty()) + Expect(outMeta.Variables).To(HaveLen(1)) + Expect(outMeta.Variables[0].Name).To(Equal("sample-tile-ca")) + Expect(outMeta.Variables[0].Type).To(Equal("certificate")) Expect(outMeta.Releases).To(HaveLen(1)) Expect(outMeta.Releases[0]).To(ContainSubstring("k8s-tile-test")) Expect(outMeta.InstanceGroups).To(HaveLen(0)) diff --git a/internal/carvel/testdata/sample-tile/base.yml b/internal/carvel/testdata/sample-tile/base.yml index d91f08bba..5776a7797 100644 --- a/internal/carvel/testdata/sample-tile/base.yml +++ b/internal/carvel/testdata/sample-tile/base.yml @@ -11,7 +11,13 @@ property_blueprints: - $( property "admin_password" ) form_types: - $( form "db_props" ) -variables: [] +variables: +- name: sample-tile-ca + type: certificate + options: + common_name: Sample Tile CA + is_ca: true + duration: 730 package_installs: - $( package "test-install" ) compatible_kubernetes_distributions: From be63033e1104440a1d35f8aeecd9d3e48c6d0bb2 Mon Sep 17 00:00:00 2001 From: Derek Richard Date: Fri, 19 Jun 2026 12:39:22 -0400 Subject: [PATCH 3/3] fix(carvel): address PR review comments on structured variable declarations - Fix indentation of Variables[0] assertions to match surrounding 5-tab style - Add Options assertions (common_name, is_ca) to verify round-trip fidelity of the full variable definition including nested options block - Refactor validateVariables to use errors.Join so all malformed entries are reported in a single error instead of failing on the first one - Fix pre-existing AfterEach indentation so gofmt is clean Co-Authored-By: Claude Sonnet 4.6 --- internal/carvel/baker.go | 11 +++++------ internal/carvel/baker_test.go | 18 ++++++++++-------- 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/internal/carvel/baker.go b/internal/carvel/baker.go index 740d6ea36..6792ab6d0 100644 --- a/internal/carvel/baker.go +++ b/internal/carvel/baker.go @@ -255,17 +255,16 @@ func (b *baker) progress(message string) { _, _ = fmt.Fprintln(b.progressWriter, message) } -// validateVariables checks that each variable declaration has the required name and type fields. func validateVariables(vars []proofing.Variable) error { + var errs []error for i, v := range vars { if v.Name == "" { - return fmt.Errorf("variables[%d]: missing required field 'name'", i) - } - if v.Type == "" { - return fmt.Errorf("variables[%d] (%q): missing required field 'type'", i, v.Name) + errs = append(errs, fmt.Errorf("variables[%d]: missing required field 'name'", i)) + } else if v.Type == "" { + errs = append(errs, fmt.Errorf("variables[%d] (%q): missing required field 'type'", i, v.Name)) } } - return nil + return errors.Join(errs...) } func (b *baker) generateBoshReleaseDir() error { diff --git a/internal/carvel/baker_test.go b/internal/carvel/baker_test.go index f21cec8d4..aadc0328e 100644 --- a/internal/carvel/baker_test.go +++ b/internal/carvel/baker_test.go @@ -141,12 +141,12 @@ var _ = Describe("Carvel Baker", func() { subject = NewBaker() subject.SetWriter(GinkgoWriter) }) - AfterEach(func() { - // Clean up the temp directory - if inputPath != "" { - _ = os.RemoveAll(filepath.Dir(inputPath)) - } - }) + AfterEach(func() { + // Clean up the temp directory + if inputPath != "" { + _ = os.RemoveAll(filepath.Dir(inputPath)) + } + }) JustBeforeEach(func() { err = subject.Bake(inputPath) }) @@ -171,8 +171,10 @@ var _ = Describe("Carvel Baker", func() { Expect(outMeta.PropertyBlueprints).To(HaveLen(2)) Expect(outMeta.FormTypes).To(HaveLen(1)) Expect(outMeta.Variables).To(HaveLen(1)) - Expect(outMeta.Variables[0].Name).To(Equal("sample-tile-ca")) - Expect(outMeta.Variables[0].Type).To(Equal("certificate")) + Expect(outMeta.Variables[0].Name).To(Equal("sample-tile-ca")) + Expect(outMeta.Variables[0].Type).To(Equal("certificate")) + Expect(outMeta.Variables[0].Options).To(HaveKeyWithValue("common_name", "Sample Tile CA")) + Expect(outMeta.Variables[0].Options).To(HaveKeyWithValue("is_ca", true)) Expect(outMeta.Releases).To(HaveLen(1)) Expect(outMeta.Releases[0]).To(ContainSubstring("k8s-tile-test")) Expect(outMeta.InstanceGroups).To(HaveLen(0))