From 8195c521b6986945a162b08412c81d95e3a88fb4 Mon Sep 17 00:00:00 2001 From: George Gelashvili Date: Wed, 17 Jul 2024 10:54:48 -0700 Subject: [PATCH 01/15] Update CAPI v2 docs links to new subdomain --- pkg/notes/testdata/runtime-rn.html.md.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/notes/testdata/runtime-rn.html.md.erb b/pkg/notes/testdata/runtime-rn.html.md.erb index 55656385b..574269160 100644 --- a/pkg/notes/testdata/runtime-rn.html.md.erb +++ b/pkg/notes/testdata/runtime-rn.html.md.erb @@ -4187,7 +4187,7 @@ Overview tab in Apps Manager results in an `Unexpected error occurence` message. In <%= vars.app_runtime_abbr %> v2.7.17 and earlier, the `/v2/app_usage_events/destructively_purge_all_and_reseed_started_apps` endpoint may generate app events without valid GUIDs. These invalid GUIDs can cause errors with components that consume them when parsing and correlating events. This issue affects Cloud Controller and App Usage Service. -For more information about the API endpoint, see [Purge and reseed App Usage Events](https://apidocs.cloudfoundry.org/13.6.0/app_usage_events/purge_and_reseed_app_usage_events.html) in the _App Usage Events API_ documentation. For more information about the issue, see [App Usage Service startup errors and data inconsistency](https://community.pivotal.io/s/article/App-Usage-Service-startup-errors-and-data-inconsistency) in the knowledge base. +For more information about the API endpoint, see [Purge and reseed App Usage Events](https://v2-apidocs.cloudfoundry.org/app_usage_events/purge_and_reseed_app_usage_events.html) in the _App Usage Events API_ documentation. For more information about the issue, see [App Usage Service startup errors and data inconsistency](https://community.pivotal.io/s/article/App-Usage-Service-startup-errors-and-data-inconsistency) in the knowledge base. This issue is resolved in <%= vars.app_runtime_abbr %> v2.7.18. From 42f1ccb9d579339139c50c209550961198310f02 Mon Sep 17 00:00:00 2001 From: Janice Bailey Date: Mon, 12 Aug 2024 14:13:44 -0400 Subject: [PATCH 02/15] updated to go 1.22.6 --- go.mod | 2 +- go.sum | 6 ------ internal/acceptance/workflows/testdata/tiles/v2/go.mod | 2 +- internal/test/testdata/happy-tile/go.mod | 2 +- 4 files changed, 3 insertions(+), 9 deletions(-) diff --git a/go.mod b/go.mod index b50f5ef96..8b3a2155b 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/pivotal-cf/kiln -go 1.21 +go 1.22.6 require ( github.com/Masterminds/semver/v3 v3.2.1 diff --git a/go.sum b/go.sum index 8a3d47e84..9063f3b1a 100644 --- a/go.sum +++ b/go.sum @@ -629,8 +629,6 @@ golang.org/x/crypto v0.0.0-20210921155107-089bfa567519/go.mod h1:GvvjBRRGRdwPK5y golang.org/x/crypto v0.0.0-20220622213112-05595931fe9d/go.mod h1:IxCIyHEi3zRg3s0A5j5BB6A9Jmi73HwBIUl50j+osU4= golang.org/x/crypto v0.3.1-0.20221117191849-2c476679df9a/go.mod h1:hebNnKkNXi2UzZN1eVRvBB7co0a+JxK6XbPiWVs/3J4= golang.org/x/crypto v0.7.0/go.mod h1:pYwdfH91IfpZVANVyUOhSIPZaFoJGxTFbZhFTx+dXZU= -golang.org/x/crypto v0.19.0 h1:ENy+Az/9Y1vSrlrvBSyna3PITt4tiZLf7sgCjZBX7Wo= -golang.org/x/crypto v0.19.0/go.mod h1:Iy9bg/ha4yyC70EfRS8jz+B6ybOBKMaSxLj6P6oBDfU= golang.org/x/crypto v0.21.0 h1:X31++rzVUdKhX5sWmSOFZxx8UW/ldWx55cbf08iNAMA= golang.org/x/crypto v0.21.0/go.mod h1:0BP7YvVV9gBbVKyeTG0Gyn+gZm94bibOW5BjDEYAOMs= golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= @@ -802,8 +800,6 @@ golang.org/x/sys v0.2.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.3.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.5.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.17.0 h1:25cE3gD+tdBA7lp7QfhuV+rJiE9YXTcS3VG1SqssI/Y= -golang.org/x/sys v0.17.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/sys v0.18.0 h1:DBdB3niSjOA/O0blCZBqDefyWNYveAYMNF1Wum0DYQ4= golang.org/x/sys v0.18.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/term v0.0.0-20201117132131-f5c789dd3221/go.mod h1:Nr5EML6q2oocZ2LXRh80K7BxOlk5/8JxuGnuhpl+muw= @@ -813,8 +809,6 @@ golang.org/x/term v0.1.0/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8= golang.org/x/term v0.2.0/go.mod h1:TVmDHMZPmdnySmBfhjOoOdhjzdE1h4u1VwSiw2l1Nuc= golang.org/x/term v0.5.0/go.mod h1:jMB1sMXY+tzblOD4FWmEbocvup2/aLOaQEp7JmGp78k= golang.org/x/term v0.6.0/go.mod h1:m6U89DPEgQRMq3DNkDClhWw02AUbt2daBVO4cn4Hv9U= -golang.org/x/term v0.17.0 h1:mkTF7LCd6WGJNL3K1Ad7kwxNfYAW6a8a8QqtMblp/4U= -golang.org/x/term v0.17.0/go.mod h1:lLRBjIVuehSbZlaOtGMbcMncT+aqLLLmKrsjNrUguwk= golang.org/x/term v0.18.0 h1:FcHjZXDMxI8mM3nwhX9HlKop4C0YQvCVCdwYl2wOtE8= golang.org/x/term v0.18.0/go.mod h1:ILwASektA3OnRv7amZ1xhE/KTR+u50pbXfZ03+6Nx58= golang.org/x/text v0.0.0-20170915032832-14c0d48ead0c/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= diff --git a/internal/acceptance/workflows/testdata/tiles/v2/go.mod b/internal/acceptance/workflows/testdata/tiles/v2/go.mod index 946935e86..0c94a5798 100644 --- a/internal/acceptance/workflows/testdata/tiles/v2/go.mod +++ b/internal/acceptance/workflows/testdata/tiles/v2/go.mod @@ -1,6 +1,6 @@ module github.com/crhntr/hello-tile -go 1.20 +go 1.22.6 require ( github.com/cppforlife/go-patch v0.2.0 // indirect diff --git a/internal/test/testdata/happy-tile/go.mod b/internal/test/testdata/happy-tile/go.mod index 425264502..5ac61cb88 100644 --- a/internal/test/testdata/happy-tile/go.mod +++ b/internal/test/testdata/happy-tile/go.mod @@ -1,6 +1,6 @@ module github.com/pivotal-cf/kiln/internal/commands/testdata/tas_fake/tas -go 1.20 +go 1.22.6 require ( github.com/onsi/ginkgo v1.16.4 From fda5e4e677724a7dae85651c2a1f5cabdae35472 Mon Sep 17 00:00:00 2001 From: Christopher Hunter Date: Wed, 14 Aug 2024 17:10:14 -0700 Subject: [PATCH 03/15] bump(action): attempt to fix golangci-lint error by bumping the version --- .github/workflows/test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 4a99a1748..fe685da5a 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -20,7 +20,7 @@ jobs: go-version-file: go.mod - name: golangci-lint - uses: golangci/golangci-lint-action@v3 + uses: golangci/golangci-lint-action@v6 - name: Ensure Generate Succeeds and Does Not Make Changes run: | From bb0e12717f4f85de2813310f210431148d6c4b3a Mon Sep 17 00:00:00 2001 From: Christopher Hunter Date: Thu, 15 Aug 2024 09:51:13 -0700 Subject: [PATCH 04/15] fix(lint): non-constant format string in call to fmt.Errorf --- internal/component/artifactory.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/component/artifactory.go b/internal/component/artifactory.go index f34c5b5e7..6ed9fc119 100644 --- a/internal/component/artifactory.go +++ b/internal/component/artifactory.go @@ -311,7 +311,7 @@ func (ars *ArtifactoryReleaseSource) UploadRelease(spec cargo.BOSHReleaseTarball switch response.StatusCode { case http.StatusCreated: default: - return cargo.BOSHReleaseTarballLock{}, fmt.Errorf(response.Status) + return cargo.BOSHReleaseTarballLock{}, fmt.Errorf("response contained errror status code: %d %s", response.StatusCode, response.Status) } return cargo.BOSHReleaseTarballLock{ From 380077757638685d74f3af72f8ea83c6e10ddf66 Mon Sep 17 00:00:00 2001 From: Christopher Hunter Date: Thu, 15 Aug 2024 09:52:40 -0700 Subject: [PATCH 05/15] fix(lint): SA1006 warning mitigation printf-style function with dynamic format string and no further arguments should use print-style function instead --- internal/commands/find_stemcell_version.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/internal/commands/find_stemcell_version.go b/internal/commands/find_stemcell_version.go index c19606687..decc616fb 100644 --- a/internal/commands/find_stemcell_version.go +++ b/internal/commands/find_stemcell_version.go @@ -2,6 +2,7 @@ package commands import ( "encoding/json" + "errors" "fmt" "log" @@ -57,7 +58,7 @@ func (cmd FindStemcellVersion) Execute(args []string) error { } if kilnfile.Stemcell.Version == "" { - return fmt.Errorf(ErrStemcellMajorVersionMustBeValid) + return errors.New(ErrStemcellMajorVersionMustBeValid) } // Get stemcell version from pivnet From 7cd368b4509c98290429ce82d1755ffc9be7bb78 Mon Sep 17 00:00:00 2001 From: Joe Eltgroth Date: Thu, 15 Aug 2024 12:56:33 -0500 Subject: [PATCH 06/15] Fix test Co-authored-by: Christopher Hunter --- internal/acceptance/workflows/baking_a_tile.feature | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/acceptance/workflows/baking_a_tile.feature b/internal/acceptance/workflows/baking_a_tile.feature index b88b55b30..77cd16587 100644 --- a/internal/acceptance/workflows/baking_a_tile.feature +++ b/internal/acceptance/workflows/baking_a_tile.feature @@ -13,11 +13,11 @@ Feature: As a developer, I want to bake a tile | releases/bpm-1.2.12.tgz | | releases/hello-release-0.2.3.tgz | And "bake_records/0.2.0-dev.json" contains substring: "version": "0.2.0-dev" - And "bake_records/0.2.0-dev.json" contains substring: "source_revision": "bc3ac24e192ba06a2eca19381ad785ec7069e0d0" + And "bake_records/0.2.0-dev.json" contains substring: "source_revision": "6d5069f9dfb954ff77bb16c5aee670b9909f154a" And "bake_records/0.2.0-dev.json" contains substring: "tile_directory": "." And "bake_records/0.2.0-dev.json" contains substring: "kiln_version": "0.0.0+acceptance-tests" - And "bake_records/0.2.0-dev.json" contains substring: "file_checksum": "5f8abc7a3272a70fa716cdf120f6976f6b78e16a01a4b3e085ced7f51d6c7691" - And "tile-0.2.0-dev.pivotal" has sha256 sum "5f8abc7a3272a70fa716cdf120f6976f6b78e16a01a4b3e085ced7f51d6c7691" + And "bake_records/0.2.0-dev.json" contains substring: "file_checksum": "c94e5749bf676f03ff10539956e9445d309647c5299b16dfe71cb522e9258f0d" + And "tile-0.2.0-dev.pivotal" has sha256 sum "c94e5749bf676f03ff10539956e9445d309647c5299b16dfe71cb522e9258f0d" Scenario: it reads directory configuration from Kilnfile Given I have a tile source directory "testdata/tiles/non-standard-paths" From af455fb25584ab6e0796ba9029f930a9382ad00c Mon Sep 17 00:00:00 2001 From: Preethi Varambally Date: Tue, 10 Sep 2024 04:05:38 -0700 Subject: [PATCH 07/15] Update default Trainstat URL --- pkg/notes/notes_data.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/notes/notes_data.go b/pkg/notes/notes_data.go index e06175f00..d591ed204 100644 --- a/pkg/notes/notes_data.go +++ b/pkg/notes/notes_data.go @@ -102,7 +102,7 @@ type IssuesQuery struct { } type TrainstatQuery struct { - TrainstatURL string `long:"trainstat-url" short:"tu" description:"trainstat url to fetch the release notes for component bumps" default:"https://trainstat.sc2-04-pcf1-apps.oc.vmware.com"` + TrainstatURL string `long:"trainstat-url" short:"tu" description:"trainstat url to fetch the release notes for component bumps" default:"https://tas-trainstat.eng.tanzu.broadcom.com"` } func TrainstatURL() string { From fe7fd98832840b54cd073517afa3748492e3f246 Mon Sep 17 00:00:00 2001 From: Ajita Jain Date: Thu, 8 Aug 2024 13:43:05 -0700 Subject: [PATCH 08/15] Add flag for kiln validate to specify allow list for release source types Co-authored-by: Christopher Hunter --- internal/commands/validate.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/internal/commands/validate.go b/internal/commands/validate.go index f364fe226..b9b2fdab5 100644 --- a/internal/commands/validate.go +++ b/internal/commands/validate.go @@ -2,6 +2,7 @@ package commands import ( "fmt" + "slices" "strings" "github.com/go-git/go-billy/v5" @@ -14,6 +15,7 @@ import ( type Validate struct { Options struct { flags.Standard + ReleaseSourceTypeAllowList []string `long:"allow-release-source-type"` } FS billy.Filesystem @@ -38,6 +40,14 @@ func (v Validate) Execute(args []string) error { return fmt.Errorf("failed to load kilnfiles: %w", err) } + if len(v.Options.ReleaseSourceTypeAllowList) > 0 { + for _, s := range kf.ReleaseSources { + if !slices.Contains(v.Options.ReleaseSourceTypeAllowList, s.Type) { + return fmt.Errorf("release source type not allowed: %s", s.Type) + } + } + } + errs := cargo.Validate(kf, lock) if len(errs) > 0 { return errorList(errs) From 36abac561e557ab4c0f90e02073911d8465c3f19 Mon Sep 17 00:00:00 2001 From: Christopher Hunter Date: Fri, 9 Aug 2024 13:50:47 -0700 Subject: [PATCH 09/15] feat(cargo): allow configuration of Validate to check allow list for release_source types Co-authored-by: Ajita Jain --- internal/commands/validate.go | 11 +---- internal/commands/validate_test.go | 66 ++++++++++++++++++++++++++++++ pkg/cargo/validate.go | 34 ++++++++++++++- pkg/cargo/validate_test.go | 31 ++++++++++++++ 4 files changed, 131 insertions(+), 11 deletions(-) create mode 100644 internal/commands/validate_test.go diff --git a/internal/commands/validate.go b/internal/commands/validate.go index b9b2fdab5..448ac3141 100644 --- a/internal/commands/validate.go +++ b/internal/commands/validate.go @@ -2,7 +2,6 @@ package commands import ( "fmt" - "slices" "strings" "github.com/go-git/go-billy/v5" @@ -40,15 +39,7 @@ func (v Validate) Execute(args []string) error { return fmt.Errorf("failed to load kilnfiles: %w", err) } - if len(v.Options.ReleaseSourceTypeAllowList) > 0 { - for _, s := range kf.ReleaseSources { - if !slices.Contains(v.Options.ReleaseSourceTypeAllowList, s.Type) { - return fmt.Errorf("release source type not allowed: %s", s.Type) - } - } - } - - errs := cargo.Validate(kf, lock) + errs := cargo.Validate(kf, lock, cargo.ValidateResourceTypeAllowList(v.Options.ReleaseSourceTypeAllowList...)) if len(errs) > 0 { return errorList(errs) } diff --git a/internal/commands/validate_test.go b/internal/commands/validate_test.go new file mode 100644 index 000000000..d3b6dbd63 --- /dev/null +++ b/internal/commands/validate_test.go @@ -0,0 +1,66 @@ +package commands_test + +import ( + "io" + + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + + "github.com/go-git/go-billy/v5" + "github.com/go-git/go-billy/v5/memfs" + + "github.com/pivotal-cf/kiln/internal/commands" +) + +var _ = Describe("validate", func() { + var ( + validate commands.Validate + directory billy.Filesystem + ) + + BeforeEach(func() { + directory = memfs.New() + }) + + JustBeforeEach(func() { + validate = commands.NewValidate(directory) + }) + + When("the kilnfile has two release_sources", func() { + BeforeEach(func() { + f, err := directory.Create("Kilnfile") + Expect(err).NotTo(HaveOccurred()) + _, _ = io.WriteString(f, `--- +release_sources: + - type: "bosh.io" + - type: "github" +`) + _ = f.Close() + }) + + BeforeEach(func() { + f, err := directory.Create("Kilnfile.lock") + Expect(err).NotTo(HaveOccurred()) + _ = f.Close() + }) + + When("both types are in the allow list", func() { + It("it does fail", func() { + err := validate.Execute([]string{ + "--allow-release-source-type=bosh.io", + "--allow-release-source-type=github", + }) + Expect(err).NotTo(HaveOccurred()) + }) + }) + When("both one of the types is not in the allow list", func() { + It("it does fail", func() { + err := validate.Execute([]string{ + "--allow-release-source-type=bosh.io", + }) + Expect(err).To(MatchError(ContainSubstring("release source type not allowed: github"))) + }) + }) + }) + +}) diff --git a/pkg/cargo/validate.go b/pkg/cargo/validate.go index c3f036e03..94b2e7518 100644 --- a/pkg/cargo/validate.go +++ b/pkg/cargo/validate.go @@ -7,9 +7,41 @@ import ( "github.com/Masterminds/semver/v3" ) -func Validate(spec Kilnfile, lock KilnfileLock) []error { +type ValidationOptions struct { + resourceTypeAllowList []string +} + +func ValidateResourceTypeAllowList(allowList ...string) ValidationOptions { + return ValidationOptions{}.SetValidateResourceTypeAllowList(allowList) +} + +func (o ValidationOptions) SetValidateResourceTypeAllowList(allowList []string) ValidationOptions { + o.resourceTypeAllowList = allowList + return o +} + +func mergeOptions(options []ValidationOptions) ValidationOptions { + var opt ValidationOptions + for _, o := range options { + if o.resourceTypeAllowList != nil { + opt.resourceTypeAllowList = o.resourceTypeAllowList + } + } + return opt +} + +func Validate(spec Kilnfile, lock KilnfileLock, options ...ValidationOptions) []error { + opt := mergeOptions(options) var result []error + if len(opt.resourceTypeAllowList) > 0 { + for _, s := range spec.ReleaseSources { + if !slices.Contains(opt.resourceTypeAllowList, s.Type) { + result = append(result, fmt.Errorf("release source type not allowed: %s", s.Type)) + } + } + } + for index, componentSpec := range spec.Releases { if componentSpec.Name == "" { result = append(result, fmt.Errorf("release at index %d missing name in spec", index)) diff --git a/pkg/cargo/validate_test.go b/pkg/cargo/validate_test.go index bb3521367..aa23fe2a0 100644 --- a/pkg/cargo/validate_test.go +++ b/pkg/cargo/validate_test.go @@ -4,6 +4,8 @@ import ( "testing" . "github.com/onsi/gomega" + + "github.com/stretchr/testify/assert" ) const ( @@ -296,3 +298,32 @@ func TestValidate_checkComponentVersionsAndConstraint(t *testing.T) { )) }) } + +func TestValidateWithOptions(t *testing.T) { + t.Run("resource type allow list", func(t *testing.T) { + t.Run("when the types are permitted", func(t *testing.T) { + kf := Kilnfile{ + ReleaseSources: []ReleaseSourceConfig{ + {Type: "farm"}, + {Type: "orchard"}, + }, + } + kl := KilnfileLock{} + errs := Validate(kf, kl, ValidateResourceTypeAllowList("orchard", "farm")) + assert.Zero(t, errs) + }) + t.Run("when one of the types is not in the allow list", func(t *testing.T) { + kf := Kilnfile{ + ReleaseSources: []ReleaseSourceConfig{ + {Type: "farm"}, + {Type: "orchard"}, + }, + } + kl := KilnfileLock{} + errs := Validate(kf, kl, ValidateResourceTypeAllowList("orchard")) + if assert.Len(t, errs, 1) { + assert.ErrorContains(t, errs[0], "release source type not allowed: farm") + } + }) + }) +} From 9c05eb1f04e68d580b62bea520bb5c6c180d84fa Mon Sep 17 00:00:00 2001 From: Christopher Hunter Date: Fri, 9 Aug 2024 14:11:51 -0700 Subject: [PATCH 10/15] fix: add langauge annotation to test value this is just a nice to have --- internal/commands/validate_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/commands/validate_test.go b/internal/commands/validate_test.go index d3b6dbd63..b82ab3b72 100644 --- a/internal/commands/validate_test.go +++ b/internal/commands/validate_test.go @@ -30,6 +30,7 @@ var _ = Describe("validate", func() { BeforeEach(func() { f, err := directory.Create("Kilnfile") Expect(err).NotTo(HaveOccurred()) + // language=yaml _, _ = io.WriteString(f, `--- release_sources: - type: "bosh.io" From 6da7cf5212e861ba66ce385d1e512da6293cd729 Mon Sep 17 00:00:00 2001 From: Christopher Hunter Date: Fri, 9 Aug 2024 14:16:22 -0700 Subject: [PATCH 11/15] test(commands): add test for empty release_source type allow list --- internal/commands/validate_test.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/internal/commands/validate_test.go b/internal/commands/validate_test.go index b82ab3b72..bf477b37d 100644 --- a/internal/commands/validate_test.go +++ b/internal/commands/validate_test.go @@ -12,7 +12,7 @@ import ( "github.com/pivotal-cf/kiln/internal/commands" ) -var _ = Describe("validate", func() { +var _ = FDescribe("validate", func() { var ( validate commands.Validate directory billy.Filesystem @@ -62,6 +62,11 @@ release_sources: Expect(err).To(MatchError(ContainSubstring("release source type not allowed: github"))) }) }) + When("the allow list is empty", func() { + It("it does not fail", func() { + err := validate.Execute([]string{}) + Expect(err).NotTo(HaveOccurred()) + }) + }) }) - }) From eea919f0e7a0dc5a6714ab4f61a40597198872ad Mon Sep 17 00:00:00 2001 From: Christopher Hunter Date: Fri, 9 Aug 2024 14:27:34 -0700 Subject: [PATCH 12/15] fix: remove focus --- internal/commands/validate_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/commands/validate_test.go b/internal/commands/validate_test.go index bf477b37d..45561d7be 100644 --- a/internal/commands/validate_test.go +++ b/internal/commands/validate_test.go @@ -12,7 +12,7 @@ import ( "github.com/pivotal-cf/kiln/internal/commands" ) -var _ = FDescribe("validate", func() { +var _ = Describe("validate", func() { var ( validate commands.Validate directory billy.Filesystem From f29987aad91836fa73cc5394053ac2624598dacc Mon Sep 17 00:00:00 2001 From: Christopher Hunter Date: Fri, 9 Aug 2024 17:09:08 -0700 Subject: [PATCH 13/15] feat: add release_source field validation Co-authored-by: Ajita Jain --- pkg/cargo/validate.go | 63 +++++++- pkg/cargo/validate_test.go | 286 +++++++++++++++++++++++++++++++++++++ 2 files changed, 347 insertions(+), 2 deletions(-) diff --git a/pkg/cargo/validate.go b/pkg/cargo/validate.go index 94b2e7518..3878f72d9 100644 --- a/pkg/cargo/validate.go +++ b/pkg/cargo/validate.go @@ -2,9 +2,9 @@ package cargo import ( "fmt" - "slices" - "github.com/Masterminds/semver/v3" + "slices" + "text/template/parse" ) type ValidationOptions struct { @@ -75,6 +75,7 @@ func Validate(spec Kilnfile, lock KilnfileLock, options ...ValidationOptions) [] } result = append(result, ensureRemoteSourceExistsForEachReleaseLock(spec, lock)...) + result = append(result, ensureReleaseSourceConfiguration(spec.ReleaseSources)...) if len(result) > 0 { return result @@ -83,6 +84,64 @@ func Validate(spec Kilnfile, lock KilnfileLock, options ...ValidationOptions) [] return nil } +func ensureReleaseSourceConfiguration(sources []ReleaseSourceConfig) []error { + var errs []error + for _, source := range sources { + switch source.Type { + case BOSHReleaseTarballSourceTypeArtifactory: + if source.ArtifactoryHost == "" { + errs = append(errs, fmt.Errorf("missing required field artifactory_host")) + } + if source.Username == "" { + errs = append(errs, fmt.Errorf("missing required field username")) + } + if source.Password == "" { + errs = append(errs, fmt.Errorf("missing required field password")) + } + if source.Repo == "" { + errs = append(errs, fmt.Errorf("missing required field repo")) + } + if source.PathTemplate == "" { + errs = append(errs, fmt.Errorf("missing required field path_template")) + } else { + p := parse.New("path_template") + p.Mode |= parse.SkipFuncCheck + if _, err := p.Parse(source.PathTemplate, "", "", make(map[string]*parse.Tree)); err != nil { + errs = append(errs, fmt.Errorf("failed to parse path_template: %w", err)) + } + } + if source.Bucket != "" { + errs = append(errs, fmt.Errorf("artifactory has unexpected field bucket")) + } + if source.Region != "" { + errs = append(errs, fmt.Errorf("artifactory has unexpected field region")) + } + if source.AccessKeyId != "" { + errs = append(errs, fmt.Errorf("artifactory has unexpected field access_key_id")) + } + if source.SecretAccessKey != "" { + errs = append(errs, fmt.Errorf("artifactory has unexpected field secret_access_key")) + } + if source.RoleARN != "" { + errs = append(errs, fmt.Errorf("artifactory has unexpected field role_arn")) + } + if source.Endpoint != "" { + errs = append(errs, fmt.Errorf("artifactory has unexpected field endpoint")) + } + if source.Org != "" { + errs = append(errs, fmt.Errorf("artifactory has unexpected field org")) + } + if source.GithubToken != "" { + errs = append(errs, fmt.Errorf("artifactory has unexpected field github_token")) + } + case BOSHReleaseTarballSourceTypeBOSHIO: + case BOSHReleaseTarballSourceTypeS3: + case BOSHReleaseTarballSourceTypeGithub: + } + } + return errs +} + func ensureRemoteSourceExistsForEachReleaseLock(spec Kilnfile, lock KilnfileLock) []error { var result []error for _, release := range lock.Releases { diff --git a/pkg/cargo/validate_test.go b/pkg/cargo/validate_test.go index aa23fe2a0..d9b419278 100644 --- a/pkg/cargo/validate_test.go +++ b/pkg/cargo/validate_test.go @@ -1,6 +1,7 @@ package cargo import ( + "github.com/stretchr/testify/require" "testing" . "github.com/onsi/gomega" @@ -326,4 +327,289 @@ func TestValidateWithOptions(t *testing.T) { } }) }) + + t.Run("when a release_source is not configured properly", func(t *testing.T) { + for _, tt := range []struct { + Name string + Sources []ReleaseSourceConfig + Error func(t *testing.T, errs []error) + }{ + { + Name: "artifactory host is empty", + Sources: []ReleaseSourceConfig{ + { + Type: BOSHReleaseTarballSourceTypeArtifactory, + ID: "", + // ArtifactoryHost: "http://example.com", + Username: "bot", + Password: "beep boop", + Repo: "secret-stash", + PathTemplate: "some-path", + }, + }, + Error: func(t *testing.T, errs []error) { + require.Len(t, errs, 1) + assert.ErrorContains(t, errs[0], "missing required field artifactory_host") + }, + }, + { + Name: "artifactory password is empty", + Sources: []ReleaseSourceConfig{ + { + Type: BOSHReleaseTarballSourceTypeArtifactory, + ID: "", + ArtifactoryHost: "http://example.com", + Username: "bot", + // Password: "beep boop", + Repo: "secret-stash", + PathTemplate: "some-path", + }, + }, + Error: func(t *testing.T, errs []error) { + require.Len(t, errs, 1) + assert.ErrorContains(t, errs[0], "missing required field password") + }, + }, + { + Name: "artifactory username is empty", + Sources: []ReleaseSourceConfig{ + { + Type: BOSHReleaseTarballSourceTypeArtifactory, + ID: "", + ArtifactoryHost: "http://example.com", + // Username: "bot", + Password: "beep boop", + Repo: "secret-stash", + PathTemplate: "some-path", + }, + }, + Error: func(t *testing.T, errs []error) { + require.Len(t, errs, 1) + assert.ErrorContains(t, errs[0], "missing required field username") + }, + }, + { + Name: "artifactory repo is empty", + Sources: []ReleaseSourceConfig{ + { + Type: BOSHReleaseTarballSourceTypeArtifactory, + ID: "", + ArtifactoryHost: "http://example.com", + Username: "bot", + Password: "beep boop", + // Repo: "secret-stash", + PathTemplate: "some-path", + }, + }, + Error: func(t *testing.T, errs []error) { + require.Len(t, errs, 1) + assert.ErrorContains(t, errs[0], "missing required field repo") + }, + }, + { + Name: "artifactory path_template is empty", + Sources: []ReleaseSourceConfig{ + { + Type: BOSHReleaseTarballSourceTypeArtifactory, + ID: "", + ArtifactoryHost: "http://example.com", + Username: "bot", + Password: "beep boop", + Repo: "secret-stash", + // PathTemplate: "some-path", + }, + }, + Error: func(t *testing.T, errs []error) { + require.Len(t, errs, 1) + assert.ErrorContains(t, errs[0], "missing required field path_template") + }, + }, + { + Name: "artifactory path_template is malformed", + Sources: []ReleaseSourceConfig{ + { + Type: BOSHReleaseTarballSourceTypeArtifactory, + ID: "", + ArtifactoryHost: "http://example.com", + Username: "bot", + Password: "beep boop", + Repo: "secret-stash", + PathTemplate: "{{ loosing power", + }, + }, + Error: func(t *testing.T, errs []error) { + require.Len(t, errs, 1) + assert.ErrorContains(t, errs[0], "failed to parse path_template:") + }, + }, + { + Name: "artifactory has unexpected field bucket", + Sources: []ReleaseSourceConfig{ + { + Type: BOSHReleaseTarballSourceTypeArtifactory, + ID: "", + ArtifactoryHost: "http://example.com", + Username: "bot", + Password: "beep boop", + Repo: "secret-stash", + PathTemplate: "ok", + + Bucket: "UNEXPECTED", + }, + }, + Error: func(t *testing.T, errs []error) { + require.Len(t, errs, 1) + assert.ErrorContains(t, errs[0], "unexpected field bucket") + }, + }, + { + Name: "artifactory has unexpected field region", + Sources: []ReleaseSourceConfig{ + { + Type: BOSHReleaseTarballSourceTypeArtifactory, + ID: "", + ArtifactoryHost: "http://example.com", + Username: "bot", + Password: "beep boop", + Repo: "secret-stash", + PathTemplate: "ok", + + Region: "UNEXPECTED", + }, + }, + Error: func(t *testing.T, errs []error) { + require.Len(t, errs, 1) + assert.ErrorContains(t, errs[0], "unexpected field region") + }, + }, + { + Name: "artifactory has unexpected field access_key_id", + Sources: []ReleaseSourceConfig{ + { + Type: BOSHReleaseTarballSourceTypeArtifactory, + ID: "", + ArtifactoryHost: "http://example.com", + Username: "bot", + Password: "beep boop", + Repo: "secret-stash", + PathTemplate: "ok", + + AccessKeyId: "UNEXPECTED", + }, + }, + Error: func(t *testing.T, errs []error) { + require.Len(t, errs, 1) + assert.ErrorContains(t, errs[0], "unexpected field access_key_id") + }, + }, + { + Name: "artifactory has unexpected field secret_access_key", + Sources: []ReleaseSourceConfig{ + { + Type: BOSHReleaseTarballSourceTypeArtifactory, + ID: "", + ArtifactoryHost: "http://example.com", + Username: "bot", + Password: "beep boop", + Repo: "secret-stash", + PathTemplate: "ok", + + SecretAccessKey: "UNEXPECTED", + }, + }, + Error: func(t *testing.T, errs []error) { + require.Len(t, errs, 1) + assert.ErrorContains(t, errs[0], "unexpected field secret_access_key") + }, + }, + { + Name: "artifactory has unexpected field role_arn", + Sources: []ReleaseSourceConfig{ + { + Type: BOSHReleaseTarballSourceTypeArtifactory, + ID: "", + ArtifactoryHost: "http://example.com", + Username: "bot", + Password: "beep boop", + Repo: "secret-stash", + PathTemplate: "ok", + + RoleARN: "UNEXPECTED", + }, + }, + Error: func(t *testing.T, errs []error) { + require.Len(t, errs, 1) + assert.ErrorContains(t, errs[0], "unexpected field role_arn") + }, + }, + { + Name: "artifactory has unexpected field endpoint", + Sources: []ReleaseSourceConfig{ + { + Type: BOSHReleaseTarballSourceTypeArtifactory, + ID: "", + ArtifactoryHost: "http://example.com", + Username: "bot", + Password: "beep boop", + Repo: "secret-stash", + PathTemplate: "ok", + + Endpoint: "UNEXPECTED", + }, + }, + Error: func(t *testing.T, errs []error) { + require.Len(t, errs, 1) + assert.ErrorContains(t, errs[0], "unexpected field endpoint") + }, + }, + { + Name: "artifactory has unexpected field org", + Sources: []ReleaseSourceConfig{ + { + Type: BOSHReleaseTarballSourceTypeArtifactory, + ID: "", + ArtifactoryHost: "http://example.com", + Username: "bot", + Password: "beep boop", + Repo: "secret-stash", + PathTemplate: "ok", + + Org: "UNEXPECTED", + }, + }, + Error: func(t *testing.T, errs []error) { + require.Len(t, errs, 1) + assert.ErrorContains(t, errs[0], "unexpected field org") + }, + }, + { + Name: "artifactory has unexpected field github_token", + Sources: []ReleaseSourceConfig{ + { + Type: BOSHReleaseTarballSourceTypeArtifactory, + ID: "", + ArtifactoryHost: "http://example.com", + Username: "bot", + Password: "beep boop", + Repo: "secret-stash", + PathTemplate: "ok", + + GithubToken: "UNEXPECTED", + }, + }, + Error: func(t *testing.T, errs []error) { + require.Len(t, errs, 1) + assert.ErrorContains(t, errs[0], "unexpected field github_token") + }, + }, + } { + t.Run(tt.Name, func(t *testing.T) { + k := Kilnfile{ + ReleaseSources: tt.Sources, + } + errs := Validate(k, KilnfileLock{}) + tt.Error(t, errs) + }) + } + }) } From dba1aa19bd6e532cf481c71868a1a157a59e8fcb Mon Sep 17 00:00:00 2001 From: Christopher Hunter Date: Mon, 12 Aug 2024 11:17:55 -0700 Subject: [PATCH 14/15] fix(cargo): add Options constructor --- pkg/cargo/validate.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/pkg/cargo/validate.go b/pkg/cargo/validate.go index 3878f72d9..11f79f519 100644 --- a/pkg/cargo/validate.go +++ b/pkg/cargo/validate.go @@ -11,8 +11,13 @@ type ValidationOptions struct { resourceTypeAllowList []string } +func NewValidateOptions() ValidationOptions { + return ValidationOptions{} +} + +// ValidateResourceTypeAllowList calls ValidationOptions.SetValidateResourceTypeAllowList on the result of NewValidateOptions func ValidateResourceTypeAllowList(allowList ...string) ValidationOptions { - return ValidationOptions{}.SetValidateResourceTypeAllowList(allowList) + return NewValidateOptions().SetValidateResourceTypeAllowList(allowList) } func (o ValidationOptions) SetValidateResourceTypeAllowList(allowList []string) ValidationOptions { From e589cc1ef6b5e914e14eb0b47e79a5cabeeaf7c1 Mon Sep 17 00:00:00 2001 From: Ajita Jain Date: Thu, 3 Oct 2024 12:13:20 -0700 Subject: [PATCH 15/15] gofumpt Co-Authored-By: Christopher Hunter --- internal/commands/update_release.go | 2 -- internal/commands/update_stemcell.go | 1 - pkg/cargo/validate.go | 3 ++- pkg/cargo/validate_test.go | 3 ++- pkg/planitest/internal/config.go | 1 - pkg/planitest/internal/om_runner.go | 2 -- 6 files changed, 4 insertions(+), 8 deletions(-) diff --git a/internal/commands/update_release.go b/internal/commands/update_release.go index 003890f1a..d17a8445e 100644 --- a/internal/commands/update_release.go +++ b/internal/commands/update_release.go @@ -78,7 +78,6 @@ func (u UpdateRelease) Execute(args []string) error { StemcellOS: kilnfileLock.Stemcell.OS, GitHubRepository: releaseSpec.GitHubRepository, }, false) - if err != nil { if component.IsErrNotFound(err) { return fmt.Errorf("error finding the release: %w", err) @@ -99,7 +98,6 @@ func (u UpdateRelease) Execute(args []string) error { StemcellVersion: kilnfileLock.Stemcell.Version, GitHubRepository: releaseSpec.GitHubRepository, }) - if err != nil { if component.IsErrNotFound(err) { return fmt.Errorf("error finding the release: %w", err) diff --git a/internal/commands/update_stemcell.go b/internal/commands/update_stemcell.go index 128ec23f1..36992c79f 100644 --- a/internal/commands/update_stemcell.go +++ b/internal/commands/update_stemcell.go @@ -48,7 +48,6 @@ func (update UpdateStemcell) Execute(args []string) error { kilnStemcellVersion := kilnfile.Stemcell.Version releaseVersionConstraint, err = semver.NewConstraint(kilnStemcellVersion) - if err != nil { return fmt.Errorf("invalid stemcell constraint in kilnfile: %w", err) } diff --git a/pkg/cargo/validate.go b/pkg/cargo/validate.go index 11f79f519..e35067819 100644 --- a/pkg/cargo/validate.go +++ b/pkg/cargo/validate.go @@ -2,9 +2,10 @@ package cargo import ( "fmt" - "github.com/Masterminds/semver/v3" "slices" "text/template/parse" + + "github.com/Masterminds/semver/v3" ) type ValidationOptions struct { diff --git a/pkg/cargo/validate_test.go b/pkg/cargo/validate_test.go index d9b419278..53a2ee1d8 100644 --- a/pkg/cargo/validate_test.go +++ b/pkg/cargo/validate_test.go @@ -1,9 +1,10 @@ package cargo import ( - "github.com/stretchr/testify/require" "testing" + "github.com/stretchr/testify/require" + . "github.com/onsi/gomega" "github.com/stretchr/testify/assert" diff --git a/pkg/planitest/internal/config.go b/pkg/planitest/internal/config.go index 7e5b40463..4c3bb5cd6 100644 --- a/pkg/planitest/internal/config.go +++ b/pkg/planitest/internal/config.go @@ -43,7 +43,6 @@ func MergeAdditionalProductProperties(configFile io.Reader, additionalProperties var inputConfig ProductConfiguration err = yaml.Unmarshal(yamlInput, &inputConfig) - if err != nil { return nil, fmt.Errorf("could not parse config file: %s", err) } diff --git a/pkg/planitest/internal/om_runner.go b/pkg/planitest/internal/om_runner.go index 40fc80249..906bcb8be 100644 --- a/pkg/planitest/internal/om_runner.go +++ b/pkg/planitest/internal/om_runner.go @@ -89,7 +89,6 @@ func (o OMRunner) ResetAndConfigure(productName string, productVersion string, c "--product-name", productName, "--product-version", productVersion, ) - if err != nil { return fmt.Errorf("Unable to stage product %q, version %q: %s: %s", productName, productVersion, err, errOutput) @@ -114,7 +113,6 @@ func (o OMRunner) ResetAndConfigure(productName string, productVersion string, c "configure-product", "--config", configFile.Name(), ) - if err != nil { return fmt.Errorf("Unable to configure product %q: %s: %s", productName, err, errOutput) }