From f12a465e0b4b14c9eb5316d0a21b85869f24523e Mon Sep 17 00:00:00 2001 From: Dave Walter Date: Fri, 10 Jan 2025 12:20:59 -0800 Subject: [PATCH 1/4] Reapply "update test to cover scenario where release download can be skipped" This reverts commit b42a5ced42fc3475441a549660d041371d40b9fa. --- internal/commands/update_stemcell_test.go | 74 ++++++++++++++++++++--- 1 file changed, 66 insertions(+), 8 deletions(-) diff --git a/internal/commands/update_stemcell_test.go b/internal/commands/update_stemcell_test.go index 477e0e8f..3c4edf6f 100644 --- a/internal/commands/update_stemcell_test.go +++ b/internal/commands/update_stemcell_test.go @@ -34,11 +34,15 @@ var _ = Describe("UpdateStemcell", func() { release1Version = "1" release2Name = "release2" release2Version = "2" + release3Name = "release3" + release3Version = "3" newRelease1SHA = "new-sha1-1" newRelease1RemotePath = "new-remote-path-1" newRelease2SHA = "new-sha1-2" newRelease2RemotePath = "new-remote-path-2" + newRelease3SHA = "new-sha1-3" + newRelease3RemotePath = "new-remote-path-3" publishableReleaseSourceID = "publishable" unpublishableReleaseSourceID = "test-only" @@ -66,8 +70,9 @@ var _ = Describe("UpdateStemcell", func() { Version: "^1", }, Releases: []cargo.BOSHReleaseTarballSpecification{ - {Name: release1Name, GitHubRepository: "https://example.com/lemon", Version: "*"}, + {Name: release1Name, GitHubRepository: "https://example.com/lemon", Version: "*"}, {Name: release2Name, GitHubRepository: "https://example.com/orange", Version: "*"}, + {Name: release3Name, GitHubRepository: "https://example.com/pomelo", Version: "*"}, }, } kilnfileLock = cargo.KilnfileLock{ @@ -86,6 +91,13 @@ var _ = Describe("UpdateStemcell", func() { RemoteSource: "old-remote-source-2", RemotePath: "old-remote-path-2", }, + { + Name: release3Name, + Version: release3Version, + SHA1: "old-sha-3", + RemoteSource: "old-remote-source-3", + RemotePath: "old-remote-path-3", + }, }, Stemcell: cargo.Stemcell{ OS: "old-os", @@ -101,6 +113,7 @@ var _ = Describe("UpdateStemcell", func() { Name: release1Name, Version: release1Version, RemotePath: newRelease1RemotePath, RemoteSource: publishableReleaseSourceID, + SHA1: "", } return remote, nil case release2Name: @@ -108,6 +121,15 @@ var _ = Describe("UpdateStemcell", func() { Name: release2Name, Version: release2Version, RemotePath: newRelease2RemotePath, RemoteSource: unpublishableReleaseSourceID, + SHA1: "not-calculated", + } + return remote, nil + case release3Name: + remote := cargo.BOSHReleaseTarballLock{ + Name: release3Name, Version: release3Version, + RemotePath: newRelease3RemotePath, + RemoteSource: publishableReleaseSourceID, + SHA1: newRelease3SHA, } return remote, nil default: @@ -122,6 +144,7 @@ var _ = Describe("UpdateStemcell", func() { Name: release1Name, Version: release1Version, RemotePath: newRelease1RemotePath, RemoteSource: publishableReleaseSourceID, + SHA1: "", } return remote, nil case release2Name: @@ -129,6 +152,15 @@ var _ = Describe("UpdateStemcell", func() { Name: release2Name, Version: release2Version, RemotePath: newRelease2RemotePath, RemoteSource: unpublishableReleaseSourceID, + SHA1: "not-calculated", + } + return remote, nil + case release3Name: + remote := cargo.BOSHReleaseTarballLock{ + Name: release3Name, Version: release3Version, + RemotePath: newRelease3RemotePath, + RemoteSource: publishableReleaseSourceID, + SHA1: newRelease3SHA, } return remote, nil default: @@ -151,7 +183,7 @@ var _ = Describe("UpdateStemcell", func() { } return local, nil default: - panic("unexpected release name '"+remote.Name +"'") + panic("unexpected release name '" + remote.Name + "'") } }) @@ -192,7 +224,7 @@ var _ = Describe("UpdateStemcell", func() { OS: newStemcellOS, Version: newStemcellVersion, })) - Expect(updatedLockfile.Releases).To(HaveLen(2)) + Expect(updatedLockfile.Releases).To(HaveLen(3)) Expect(updatedLockfile.Releases).To(ContainElement( cargo.BOSHReleaseTarballLock{ Name: release1Name, @@ -211,6 +243,15 @@ var _ = Describe("UpdateStemcell", func() { RemotePath: newRelease2RemotePath, }, )) + Expect(updatedLockfile.Releases).To(ContainElement( + cargo.BOSHReleaseTarballLock{ + Name: release3Name, + Version: release3Version, + SHA1: newRelease3SHA, + RemoteSource: publishableReleaseSourceID, + RemotePath: newRelease3RemotePath, + }, + )) }) It("looks up the correct releases", func() { @@ -219,7 +260,7 @@ var _ = Describe("UpdateStemcell", func() { }) Expect(err).NotTo(HaveOccurred()) - Expect(releaseSource.GetMatchedReleaseCallCount()).To(Equal(2)) + Expect(releaseSource.GetMatchedReleaseCallCount()).To(Equal(3)) req1 := releaseSource.GetMatchedReleaseArgsForCall(0) Expect(req1).To(Equal(cargo.BOSHReleaseTarballSpecification{ @@ -234,6 +275,13 @@ var _ = Describe("UpdateStemcell", func() { StemcellOS: newStemcellOS, StemcellVersion: newStemcellVersion, GitHubRepository: "https://example.com/orange", })) + + req3 := releaseSource.GetMatchedReleaseArgsForCall(2) + Expect(req3).To(Equal(cargo.BOSHReleaseTarballSpecification{ + Name: release3Name, Version: release3Version, + StemcellOS: newStemcellOS, StemcellVersion: newStemcellVersion, + GitHubRepository: "https://example.com/pomelo", + })) }) It("looks up the correct releases with --update-releases", func() { err := update.Execute([]string{ @@ -241,7 +289,7 @@ var _ = Describe("UpdateStemcell", func() { }) Expect(err).NotTo(HaveOccurred()) - Expect(releaseSource.FindReleaseVersionCallCount()).To(Equal(2)) + Expect(releaseSource.FindReleaseVersionCallCount()).To(Equal(3)) req1, noDownload1 := releaseSource.FindReleaseVersionArgsForCall(0) Expect(req1).To(Equal(cargo.BOSHReleaseTarballSpecification{ @@ -260,8 +308,7 @@ var _ = Describe("UpdateStemcell", func() { Expect(noDownload2).To(BeTrue()) }) - - It("downloads the correct releases", func() { + It("downloads 2 of the 3 correct releases, ", func() { err := update.Execute([]string{ "--kilnfile", kilnfilePath, "--version", newStemcellVersion, "--releases-directory", releasesDirPath, }) @@ -276,6 +323,7 @@ var _ = Describe("UpdateStemcell", func() { Name: release1Name, Version: release1Version, RemotePath: newRelease1RemotePath, RemoteSource: publishableReleaseSourceID, + SHA1: "", }, )) @@ -286,6 +334,7 @@ var _ = Describe("UpdateStemcell", func() { Name: release2Name, Version: release2Version, RemotePath: newRelease2RemotePath, RemoteSource: unpublishableReleaseSourceID, + SHA1: "not-calculated", }, )) }) @@ -390,7 +439,7 @@ var _ = Describe("UpdateStemcell", func() { Version: newStemcellVersion, })) - Expect(updatedLockfile.Releases).To(HaveLen(2)) + Expect(updatedLockfile.Releases).To(HaveLen(3)) Expect(updatedLockfile.Releases).To(ContainElement( cargo.BOSHReleaseTarballLock{ Name: release1Name, @@ -409,6 +458,15 @@ var _ = Describe("UpdateStemcell", func() { RemotePath: newRelease2RemotePath, }, )) + Expect(updatedLockfile.Releases).To(ContainElement( + cargo.BOSHReleaseTarballLock{ + Name: release3Name, + Version: release3Version, + SHA1: newRelease3SHA, + RemoteSource: publishableReleaseSourceID, + RemotePath: newRelease3RemotePath, + }, + )) }) }) From 100ead31d69b1f9821780fa5d795862946663a64 Mon Sep 17 00:00:00 2001 From: Dave Walter Date: Fri, 10 Jan 2025 12:21:09 -0800 Subject: [PATCH 2/4] Reapply "do not download release for update stemcell unless required" This reverts commit b5ddcab4a451000b54116c705c66ccc2b1f06ac4. --- internal/commands/update_stemcell.go | 17 ++++++++++------- .../component/artifactory_release_source.go | 13 ++++++++++--- .../artifactory_release_source_test.go | 1 + internal/component/bosh_io_release_source.go | 15 ++++++++------- .../component/bosh_io_release_source_test.go | 5 +++-- 5 files changed, 32 insertions(+), 19 deletions(-) diff --git a/internal/commands/update_stemcell.go b/internal/commands/update_stemcell.go index b70b68db..88e4505e 100644 --- a/internal/commands/update_stemcell.go +++ b/internal/commands/update_stemcell.go @@ -97,16 +97,19 @@ func (update UpdateStemcell) Execute(args []string) error { update.Logger.Printf("No change for release %q\n", rel.Name) continue } - - local, err := releaseSource.DownloadRelease(update.Options.ReleasesDir, remote) - if err != nil { - return fmt.Errorf("while downloading release %q, encountered error: %w", rel.Name, err) - } - lock := &kilnfileLock.Releases[i] - lock.SHA1 = local.Lock.SHA1 + lock.RemotePath = remote.RemotePath lock.RemoteSource = remote.RemoteSource + lock.SHA1 = remote.SHA1 + if remote.SHA1 == "" || remote.SHA1 == "not-calculated" { + // release source needs to download. + local, err := releaseSource.DownloadRelease(update.Options.ReleasesDir, remote) + if err != nil { + return fmt.Errorf("while downloading release %q, encountered error: %w", rel.Name, err) + } + lock.SHA1 = local.Lock.SHA1 + } } kilnfileLock.Stemcell.Version = trimmedInputVersion diff --git a/internal/component/artifactory_release_source.go b/internal/component/artifactory_release_source.go index 4f192c4e..57ac2c01 100644 --- a/internal/component/artifactory_release_source.go +++ b/internal/component/artifactory_release_source.go @@ -177,13 +177,20 @@ func (ars *ArtifactoryReleaseSource) GetMatchedRelease(spec cargo.BOSHReleaseTar default: return cargo.BOSHReleaseTarballLock{}, fmt.Errorf("unexpected http status: %s", http.StatusText(response.StatusCode)) } - - return cargo.BOSHReleaseTarballLock{ + + matchedRelease := cargo.BOSHReleaseTarballLock{ Name: spec.Name, Version: spec.Version, RemotePath: remotePath, RemoteSource: ars.ID, - }, nil + } + + matchedRelease.SHA1, err = ars.getFileSHA1(matchedRelease) + if err != nil { + return cargo.BOSHReleaseTarballLock{}, err + } + + return matchedRelease, nil } // FindReleaseVersion may use any of the fields on Requirement to return the best matching diff --git a/internal/component/artifactory_release_source_test.go b/internal/component/artifactory_release_source_test.go index 098c771e..2337e5e1 100644 --- a/internal/component/artifactory_release_source_test.go +++ b/internal/component/artifactory_release_source_test.go @@ -106,6 +106,7 @@ var _ = Describe("interacting with BOSH releases on Artifactory", func() { // StemcellVersion: "9.9", RemotePath: "bosh-releases/smoothie/9.9/mango/mango-2.3.4-smoothie-9.9.tgz", RemoteSource: "some-mango-tree", + SHA1: "some-sha", })) }) diff --git a/internal/component/bosh_io_release_source.go b/internal/component/bosh_io_release_source.go index c5094e9c..e65631b2 100644 --- a/internal/component/bosh_io_release_source.go +++ b/internal/component/bosh_io_release_source.go @@ -92,13 +92,14 @@ func (src BOSHIOReleaseSource) GetMatchedRelease(requirement cargo.BOSHReleaseTa for _, repo := range repos { for _, suf := range suffixes { fullName := repo + "/" + requirement.Name + suf - exists, err := src.releaseExistOnBoshio(fullName, requirement.Version) + exists, remoteSha, err := src.releaseExistOnBoshio(fullName, requirement.Version) if err != nil { return cargo.BOSHReleaseTarballLock{}, err } if exists { builtRelease := src.createReleaseRemote(requirement, fullName) + builtRelease.SHA1 = remoteSha return builtRelease, nil } } @@ -135,7 +136,7 @@ func (src BOSHIOReleaseSource) FindReleaseVersion(spec cargo.BOSHReleaseTarballS } spec.Version = validReleases[0].Version lock := src.createReleaseRemote(spec, fullName) - lock.SHA1 = validReleases[0].SHA + lock.SHA1 = validReleases[0].SHA1 return lock, nil } } @@ -232,18 +233,18 @@ func (src BOSHIOReleaseSource) getReleases(name string) ([]releaseResponse, erro type releaseResponse struct { Version string `json:"version"` - SHA string `json:"sha1"` + SHA1 string `json:"sha1"` } -func (src BOSHIOReleaseSource) releaseExistOnBoshio(name, version string) (bool, error) { +func (src BOSHIOReleaseSource) releaseExistOnBoshio(name, version string) (bool, string, error) { releaseResponses, err := src.getReleases(name) if err != nil { - return false, err + return false, "", err } for _, rel := range releaseResponses { if rel.Version == version { - return true, nil + return true, rel.SHA1, nil } } - return false, nil + return false, "", nil } diff --git a/internal/component/bosh_io_release_source_test.go b/internal/component/bosh_io_release_source_test.go index a9c7bcc2..447faf46 100644 --- a/internal/component/bosh_io_release_source_test.go +++ b/internal/component/bosh_io_release_source_test.go @@ -47,7 +47,7 @@ var _ = Describe("BOSHIOReleaseSource", func() { testServer.RouteToHandler("GET", path, ghttp.RespondWith(http.StatusOK, `null`)) path, _ = regexp.Compile(`/api/v1/releases/github.com/\S+/uaa.*`) - testServer.RouteToHandler("GET", path, ghttp.RespondWith(http.StatusOK, `[{"version": "73.3.0"}]`)) + testServer.RouteToHandler("GET", path, ghttp.RespondWith(http.StatusOK, `[{"version": "73.3.0", "sha1":"b6e8a9cbc8724edcecb8658fa9459ee6c8fc259e"}]`)) path, _ = regexp.Compile(`/api/v1/releases/github.com/\S+/metrics.*`) testServer.RouteToHandler("GET", path, ghttp.RespondWith(http.StatusOK, `[{"version": "2.3.0"}]`)) @@ -74,6 +74,7 @@ var _ = Describe("BOSHIOReleaseSource", func() { Version: "73.3.0", RemotePath: uaaURL, RemoteSource: component.ReleaseSourceTypeBOSHIO, + SHA1: "b6e8a9cbc8724edcecb8658fa9459ee6c8fc259e", })) foundRelease, err = releaseSource.GetMatchedRelease(rabbitmqRequirement) @@ -299,7 +300,7 @@ var _ = Describe("BOSHIOReleaseSource", func() { testServer = ghttp.NewServer() path, _ := regexp.Compile(`/api/v1/releases/github.com/\S+/cf-rabbitmq.*`) - testServer.RouteToHandler("GET", path, ghttp.RespondWith(http.StatusOK, `[{"name":"github.com/cloudfoundry/cf-rabbitmq-release","version":"309.0.5","url":"https://bosh.io/d/github.com/cloudfoundry/cf-rabbitmq-release?v=309.0.0","sha1":"5df538657c2cc830bda679420a9b162682018ded"},{"name":"github.com/cloudfoundry/cf-rabbitmq-release","version":"308.0.0","url":"https://bosh.io/d/github.com/cloudfoundry/cf-rabbitmq-release?v=308.0.0","sha1":"56202c9a466a8394683ae432ee2dea21ef6ef865"}]`)) + testServer.RouteToHandler("GET", path, ghttp.RespondWith(http.StatusOK, `[{"name":"github.com/cloudfoundry/cf-rabbitmq-release","version":"309.0.5","url":"https://bosh.io/d/github.com/cloudfoundry/cf-rabbitmq-release?v=309.0.0","sha1":"5df538657c2cc830bda679420a9b162682018ded","sha256": "49cad4a9758026cbae7a26f77921300595459dfa6bc0ca5332fc6ba52bd336b8"},{"name":"github.com/cloudfoundry/cf-rabbitmq-release","version":"308.0.0","url":"https://bosh.io/d/github.com/cloudfoundry/cf-rabbitmq-release?v=308.0.0","sha1":"56202c9a466a8394683ae432ee2dea21ef6ef865","sha256":"5970d4211d236896d9366b150a66e38cbbd757fed31895962e499bb5458e0937"}]`)) releaseSource = component.NewBOSHIOReleaseSource(cargo.ReleaseSourceConfig{ID: ID, Publishable: false}, testServer.URL(), logger) }) From 0ffc228a29ef35ce80093dfb2b7f8b42b79c91b3 Mon Sep 17 00:00:00 2001 From: Dave Walter Date: Fri, 10 Jan 2025 18:51:31 -0800 Subject: [PATCH 3/4] Add a without-download flag to update-stemcell Allows Kiln to skip downloading a release if the remote source can provide the SHA1 for the release tarball. --- internal/commands/update_release.go | 10 +- internal/commands/update_stemcell.go | 27 ++- internal/commands/update_stemcell_test.go | 207 ++++++++++++++---- .../component/artifactory_release_source.go | 2 +- internal/component/github_release_source.go | 2 +- pkg/cargo/bosh_release.go | 2 +- 6 files changed, 193 insertions(+), 57 deletions(-) diff --git a/internal/commands/update_release.go b/internal/commands/update_release.go index ef7f1593..191cbc72 100644 --- a/internal/commands/update_release.go +++ b/internal/commands/update_release.go @@ -16,11 +16,11 @@ type UpdateRelease struct { Options struct { flags.Standard - Name string `short:"n" long:"name" required:"true" description:"name of release to update"` - Version string `short:"v" long:"version" required:"true" description:"desired version of release"` - ReleasesDir string `short:"rd" long:"releases-directory" default:"releases" description:"path to a directory to download releases into"` - AllowOnlyPublishableReleases bool `long:"allow-only-publishable-releases" description:"include releases that would not be shipped with the tile (development builds)"` - WithoutDownload bool `long:"without-download" description:"updates releases without downloading them"` + Name string `short:"n" long:"name" required:"true" description:"name of release to update"` + Version string `short:"v" long:"version" required:"true" description:"desired version of release"` + ReleasesDir string `short:"rd" long:"releases-directory" default:"releases" description:"path to a directory to download releases into"` + AllowOnlyPublishableReleases bool ` long:"allow-only-publishable-releases" default:"false" description:"include releases that would not be shipped with the tile (development builds)"` + WithoutDownload bool `short:"wd" long:"without-download" default:"false" description:"updates releases without downloading them"` } multiReleaseSourceProvider MultiReleaseSourceProvider filesystem billy.Filesystem diff --git a/internal/commands/update_stemcell.go b/internal/commands/update_stemcell.go index 88e4505e..e0352eb0 100644 --- a/internal/commands/update_stemcell.go +++ b/internal/commands/update_stemcell.go @@ -19,9 +19,10 @@ type UpdateStemcell struct { Options struct { flags.Standard - Version string `short:"v" long:"version" required:"true" description:"desired version of stemcell"` - ReleasesDir string `short:"rd" long:"releases-directory" default:"releases" description:"path to a directory to download releases into"` - UpdateReleases bool `short:"ur" long:"update-releases" description:"finds latest matching releases for new stemcell version"` + Version string `short:"v" long:"version" required:"true" description:"desired version of stemcell"` + ReleasesDir string `short:"rd" long:"releases-directory" default:"releases" description:"path to a directory to download releases into"` + UpdateReleases bool `short:"ur" long:"update-releases" default:"false" description:"finds latest matching releases for new stemcell version"` + WithoutDownload bool `short:"wd" long:"without-download" default:"false" description:"updates stemcell releases without downloading releases"` } FS billy.Filesystem MultiReleaseSourceProvider MultiReleaseSourceProvider @@ -75,10 +76,12 @@ func (update UpdateStemcell) Execute(args []string) error { if err != nil { return err } + spec.StemcellOS = kilnfileLock.Stemcell.OS spec.StemcellVersion = trimmedInputVersion var remote cargo.BOSHReleaseTarballLock + if update.Options.UpdateReleases { remote, err = releaseSource.FindReleaseVersion(spec, true) } else { @@ -89,25 +92,35 @@ func (update UpdateStemcell) Execute(args []string) error { if err != nil { return fmt.Errorf("while finding release %q, encountered error: %w", rel.Name, err) } + if component.IsErrNotFound(err) { return fmt.Errorf("couldn't find release %q", rel.Name) } if remote.RemotePath == rel.RemotePath && remote.RemoteSource == rel.RemoteSource { update.Logger.Printf("No change for release %q\n", rel.Name) - continue + + if update.Options.WithoutDownload { + continue + } } - lock := &kilnfileLock.Releases[i] + lock := &kilnfileLock.Releases[i] lock.RemotePath = remote.RemotePath lock.RemoteSource = remote.RemoteSource lock.SHA1 = remote.SHA1 - if remote.SHA1 == "" || remote.SHA1 == "not-calculated" { + + if update.Options.UpdateReleases { + lock.Version = remote.Version + } + + if !update.Options.WithoutDownload || lock.SHA1 == "" || lock.SHA1 == "not-calculated" { // release source needs to download. local, err := releaseSource.DownloadRelease(update.Options.ReleasesDir, remote) if err != nil { - return fmt.Errorf("while downloading release %q, encountered error: %w", rel.Name, err) + return fmt.Errorf("while downloading release %s %s, encountered error: %w", lock.Name, lock.Version, err) } + lock.SHA1 = local.Lock.SHA1 } } diff --git a/internal/commands/update_stemcell_test.go b/internal/commands/update_stemcell_test.go index 3c4edf6f..a95a3e81 100644 --- a/internal/commands/update_stemcell_test.go +++ b/internal/commands/update_stemcell_test.go @@ -2,6 +2,7 @@ package commands_test import ( "errors" + "fmt" "log" "os" "path/filepath" @@ -110,7 +111,8 @@ var _ = Describe("UpdateStemcell", func() { switch requirement.Name { case release1Name: remote := cargo.BOSHReleaseTarballLock{ - Name: release1Name, Version: release1Version, + Name: release1Name, + Version: release1Version, RemotePath: newRelease1RemotePath, RemoteSource: publishableReleaseSourceID, SHA1: "", @@ -118,7 +120,8 @@ var _ = Describe("UpdateStemcell", func() { return remote, nil case release2Name: remote := cargo.BOSHReleaseTarballLock{ - Name: release2Name, Version: release2Version, + Name: release2Name, + Version: release2Version, RemotePath: newRelease2RemotePath, RemoteSource: unpublishableReleaseSourceID, SHA1: "not-calculated", @@ -126,7 +129,8 @@ var _ = Describe("UpdateStemcell", func() { return remote, nil case release3Name: remote := cargo.BOSHReleaseTarballLock{ - Name: release3Name, Version: release3Version, + Name: release3Name, + Version: release3Version, RemotePath: newRelease3RemotePath, RemoteSource: publishableReleaseSourceID, SHA1: newRelease3SHA, @@ -141,7 +145,8 @@ var _ = Describe("UpdateStemcell", func() { switch requirement.Name { case release1Name: remote := cargo.BOSHReleaseTarballLock{ - Name: release1Name, Version: release1Version, + Name: release1Name, + Version: release1Version, RemotePath: newRelease1RemotePath, RemoteSource: publishableReleaseSourceID, SHA1: "", @@ -149,7 +154,8 @@ var _ = Describe("UpdateStemcell", func() { return remote, nil case release2Name: remote := cargo.BOSHReleaseTarballLock{ - Name: release2Name, Version: release2Version, + Name: release2Name, + Version: release2Version, RemotePath: newRelease2RemotePath, RemoteSource: unpublishableReleaseSourceID, SHA1: "not-calculated", @@ -157,7 +163,8 @@ var _ = Describe("UpdateStemcell", func() { return remote, nil case release3Name: remote := cargo.BOSHReleaseTarballLock{ - Name: release3Name, Version: release3Version, + Name: release3Name, + Version: release3Version, RemotePath: newRelease3RemotePath, RemoteSource: publishableReleaseSourceID, SHA1: newRelease3SHA, @@ -172,13 +179,31 @@ var _ = Describe("UpdateStemcell", func() { switch remote.Name { case release1Name: local := component.Local{ - Lock: cargo.BOSHReleaseTarballLock{Name: release1Name, Version: release1Version, SHA1: newRelease1SHA}, + Lock: cargo.BOSHReleaseTarballLock{ + Name: release1Name, + Version: release1Version, + SHA1: newRelease1SHA, + }, LocalPath: "not-used", } return local, nil case release2Name: local := component.Local{ - Lock: cargo.BOSHReleaseTarballLock{Name: release2Name, Version: release2Version, SHA1: newRelease2SHA}, + Lock: cargo.BOSHReleaseTarballLock{ + Name: release2Name, + Version: release2Version, + SHA1: newRelease2SHA, + }, + LocalPath: "not-used", + } + return local, nil + case release3Name: + local := component.Local{ + Lock: cargo.BOSHReleaseTarballLock{ + Name: release3Name, + Version: release3Version, + SHA1: newRelease3SHA, + }, LocalPath: "not-used", } return local, nil @@ -260,26 +285,33 @@ var _ = Describe("UpdateStemcell", func() { }) Expect(err).NotTo(HaveOccurred()) + Expect(releaseSource.FindReleaseVersionCallCount()).To(Equal(0)) Expect(releaseSource.GetMatchedReleaseCallCount()).To(Equal(3)) req1 := releaseSource.GetMatchedReleaseArgsForCall(0) Expect(req1).To(Equal(cargo.BOSHReleaseTarballSpecification{ - Name: release1Name, Version: release1Version, - StemcellOS: newStemcellOS, StemcellVersion: newStemcellVersion, + Name: release1Name, + Version: release1Version, + StemcellOS: newStemcellOS, + StemcellVersion: newStemcellVersion, GitHubRepository: "https://example.com/lemon", })) req2 := releaseSource.GetMatchedReleaseArgsForCall(1) Expect(req2).To(Equal(cargo.BOSHReleaseTarballSpecification{ - Name: release2Name, Version: release2Version, - StemcellOS: newStemcellOS, StemcellVersion: newStemcellVersion, + Name: release2Name, + Version: release2Version, + StemcellOS: newStemcellOS, + StemcellVersion: newStemcellVersion, GitHubRepository: "https://example.com/orange", })) req3 := releaseSource.GetMatchedReleaseArgsForCall(2) Expect(req3).To(Equal(cargo.BOSHReleaseTarballSpecification{ - Name: release3Name, Version: release3Version, - StemcellOS: newStemcellOS, StemcellVersion: newStemcellVersion, + Name: release3Name, + Version: release3Version, + StemcellOS: newStemcellOS, + StemcellVersion: newStemcellVersion, GitHubRepository: "https://example.com/pomelo", })) }) @@ -290,37 +322,45 @@ var _ = Describe("UpdateStemcell", func() { Expect(err).NotTo(HaveOccurred()) Expect(releaseSource.FindReleaseVersionCallCount()).To(Equal(3)) + Expect(releaseSource.GetMatchedReleaseCallCount()).To(Equal(0)) req1, noDownload1 := releaseSource.FindReleaseVersionArgsForCall(0) Expect(req1).To(Equal(cargo.BOSHReleaseTarballSpecification{ - Name: release1Name, Version: "*", - StemcellOS: newStemcellOS, StemcellVersion: newStemcellVersion, + Name: release1Name, + Version: "*", + StemcellOS: newStemcellOS, + StemcellVersion: newStemcellVersion, GitHubRepository: "https://example.com/lemon", })) Expect(noDownload1).To(BeTrue()) req2, noDownload2 := releaseSource.FindReleaseVersionArgsForCall(1) Expect(req2).To(Equal(cargo.BOSHReleaseTarballSpecification{ - Name: release2Name, Version: "*", - StemcellOS: newStemcellOS, StemcellVersion: newStemcellVersion, + Name: release2Name, + Version: "*", + StemcellOS: newStemcellOS, + StemcellVersion: newStemcellVersion, GitHubRepository: "https://example.com/orange", })) Expect(noDownload2).To(BeTrue()) }) - It("downloads 2 of the 3 correct releases, ", func() { + It("downloads all of the releases", func() { err := update.Execute([]string{ - "--kilnfile", kilnfilePath, "--version", newStemcellVersion, "--releases-directory", releasesDirPath, + "--kilnfile", kilnfilePath, + "--version", newStemcellVersion, + "--releases-directory", releasesDirPath, }) Expect(err).NotTo(HaveOccurred()) - Expect(releaseSource.DownloadReleaseCallCount()).To(Equal(2)) + Expect(releaseSource.DownloadReleaseCallCount()).To(Equal(3)) actualDir, remote1 := releaseSource.DownloadReleaseArgsForCall(0) Expect(actualDir).To(Equal(releasesDirPath)) Expect(remote1).To(Equal( cargo.BOSHReleaseTarballLock{ - Name: release1Name, Version: release1Version, + Name: release1Name, + Version: release1Version, RemotePath: newRelease1RemotePath, RemoteSource: publishableReleaseSourceID, SHA1: "", @@ -331,12 +371,114 @@ var _ = Describe("UpdateStemcell", func() { Expect(actualDir).To(Equal(releasesDirPath)) Expect(remote2).To(Equal( cargo.BOSHReleaseTarballLock{ - Name: release2Name, Version: release2Version, + Name: release2Name, + Version: release2Version, RemotePath: newRelease2RemotePath, RemoteSource: unpublishableReleaseSourceID, SHA1: "not-calculated", }, )) + + actualDir, remote3 := releaseSource.DownloadReleaseArgsForCall(2) + Expect(actualDir).To(Equal(releasesDirPath)) + Expect(remote3).To(Equal( + cargo.BOSHReleaseTarballLock{ + Name: release3Name, + Version: release3Version, + RemotePath: newRelease3RemotePath, + RemoteSource: publishableReleaseSourceID, + SHA1: "new-sha1-3", + }, + )) + }) + + When("the remote information for a release doesn't change", func() { + BeforeEach(func() { + kilnfileLock.Releases[1].RemoteSource = unpublishableReleaseSourceID + kilnfileLock.Releases[1].RemotePath = newRelease2RemotePath + }) + + It("still downloads the release", func() { + err := update.Execute([]string{ + "--kilnfile", kilnfilePath, + "--version", newStemcellVersion, + }) + Expect(err).NotTo(HaveOccurred()) + + Expect(releaseSource.DownloadReleaseCallCount()).To(Equal(3)) + + _, remote := releaseSource.DownloadReleaseArgsForCall(0) + Expect(remote.Name).To(Equal(release1Name)) + + _, remote = releaseSource.DownloadReleaseArgsForCall(1) + Expect(remote.Name).To(Equal(release2Name)) + + _, remote = releaseSource.DownloadReleaseArgsForCall(2) + Expect(remote.Name).To(Equal(release3Name)) + + Expect(string(outputBuffer.Contents())).To(ContainSubstring(fmt.Sprintf("No change for release %q", release2Name))) + }) + }) + + When("when the --without-download flag is specified", func() { + It("downloads 2 of the 3 correct releases", func() { + err := update.Execute([]string{ + "--kilnfile", kilnfilePath, + "--version", newStemcellVersion, + "--releases-directory", releasesDirPath, + "--without-download", + }) + Expect(err).NotTo(HaveOccurred()) + + Expect(releaseSource.DownloadReleaseCallCount()).To(Equal(2)) + + actualDir, remote1 := releaseSource.DownloadReleaseArgsForCall(0) + Expect(actualDir).To(Equal(releasesDirPath)) + Expect(remote1).To(Equal( + cargo.BOSHReleaseTarballLock{ + Name: release1Name, + Version: release1Version, + RemotePath: newRelease1RemotePath, + RemoteSource: publishableReleaseSourceID, + SHA1: "", + }, + )) + + actualDir, remote2 := releaseSource.DownloadReleaseArgsForCall(1) + Expect(actualDir).To(Equal(releasesDirPath)) + Expect(remote2).To(Equal( + cargo.BOSHReleaseTarballLock{ + Name: release2Name, + Version: release2Version, + RemotePath: newRelease2RemotePath, + RemoteSource: unpublishableReleaseSourceID, + SHA1: "not-calculated", + }, + )) + }) + + When("the remote information for a release doesn't change", func() { + BeforeEach(func() { + kilnfileLock.Releases[1].RemoteSource = unpublishableReleaseSourceID + kilnfileLock.Releases[1].RemotePath = newRelease2RemotePath + }) + + It("doesn't download the release", func() { + err := update.Execute([]string{ + "--kilnfile", kilnfilePath, + "--version", newStemcellVersion, + "--without-download", + }) + Expect(err).NotTo(HaveOccurred()) + + Expect(releaseSource.DownloadReleaseCallCount()).To(Equal(1)) + + _, remote := releaseSource.DownloadReleaseArgsForCall(0) + Expect(remote.Name).To(Equal(release1Name)) + + Expect(string(outputBuffer.Contents())).To(ContainSubstring(fmt.Sprintf("No change for release %q", release2Name))) + }) + }) }) When("the version input is invalid", func() { @@ -470,25 +612,6 @@ var _ = Describe("UpdateStemcell", func() { }) }) - When("the remote information for a release doesn't change", func() { - BeforeEach(func() { - kilnfileLock.Releases[1].RemoteSource = unpublishableReleaseSourceID - kilnfileLock.Releases[1].RemotePath = newRelease2RemotePath - }) - - It("doesn't download the release", func() { - err := update.Execute([]string{"--kilnfile", kilnfilePath, "--version", newStemcellVersion}) - Expect(err).NotTo(HaveOccurred()) - - Expect(releaseSource.DownloadReleaseCallCount()).To(Equal(1)) - _, remote := releaseSource.DownloadReleaseArgsForCall(0) - Expect(remote.Name).To(Equal(release1Name)) - - Expect(string(outputBuffer.Contents())).To(ContainSubstring("No change")) - Expect(string(outputBuffer.Contents())).To(ContainSubstring(release2Name)) - }) - }) - When("the release can't be found", func() { BeforeEach(func() { releaseSource.GetMatchedReleaseReturns(cargo.BOSHReleaseTarballLock{}, component.ErrNotFound) diff --git a/internal/component/artifactory_release_source.go b/internal/component/artifactory_release_source.go index 57ac2c01..7b2cea8b 100644 --- a/internal/component/artifactory_release_source.go +++ b/internal/component/artifactory_release_source.go @@ -177,7 +177,7 @@ func (ars *ArtifactoryReleaseSource) GetMatchedRelease(spec cargo.BOSHReleaseTar default: return cargo.BOSHReleaseTarballLock{}, fmt.Errorf("unexpected http status: %s", http.StatusText(response.StatusCode)) } - + matchedRelease := cargo.BOSHReleaseTarballLock{ Name: spec.Name, Version: spec.Version, diff --git a/internal/component/github_release_source.go b/internal/component/github_release_source.go index bd2943c9..14ac439c 100644 --- a/internal/component/github_release_source.go +++ b/internal/component/github_release_source.go @@ -286,7 +286,7 @@ func downloadRelease(ctx context.Context, releaseDir string, remoteRelease cargo rTag, _, err := client.GetReleaseByTag(ctx, org, repo, "v"+remoteRelease.Version) if err != nil { - logger.Println("warning: failed to find release tag of ", "v"+remoteRelease.Version) + logger.Println("warning: failed to find release tag of", "v"+remoteRelease.Version) rTag, _, err = client.GetReleaseByTag(ctx, org, repo, remoteRelease.Version) if err != nil { return Local{}, fmt.Errorf("cant find release tag: %+v", err.Error()) diff --git a/pkg/cargo/bosh_release.go b/pkg/cargo/bosh_release.go index 9befde7f..8b6ca8fd 100644 --- a/pkg/cargo/bosh_release.go +++ b/pkg/cargo/bosh_release.go @@ -155,7 +155,7 @@ func ReadBOSHReleaseTarball(tarballPath string, r io.Reader) (BOSHReleaseTarball if err != nil { return BOSHReleaseTarball{}, err } - _, err = io.CopyBuffer(io.Discard, r, make([]byte, 1024 * 64)) + _, err = io.CopyBuffer(io.Discard, r, make([]byte, 1024*64)) return BOSHReleaseTarball{ Manifest: m, SHA1: hex.EncodeToString(sum.Sum(nil)), From 3140b09c840aff052a3009370287ea292dc48b3c Mon Sep 17 00:00:00 2001 From: Dave Walter Date: Thu, 16 Jan 2025 18:18:53 -0800 Subject: [PATCH 4/4] Add acceptance test coverage for new features * update-release --without-download * update-stemcell --without-download * updating_stemcell --update-releases --- .../workflows/scenario/initialize.go | 2 + .../workflows/scenario/step_funcs_kiln.go | 31 ++++++++++++++++ .../workflows/updating_releases.feature | 24 ++++++++++-- .../workflows/updating_stemcell.feature | 37 ++++++++++++++++++- 4 files changed, 88 insertions(+), 6 deletions(-) diff --git a/internal/acceptance/workflows/scenario/initialize.go b/internal/acceptance/workflows/scenario/initialize.go index b1df2391..fa9f5eac 100644 --- a/internal/acceptance/workflows/scenario/initialize.go +++ b/internal/acceptance/workflows/scenario/initialize.go @@ -55,6 +55,8 @@ func InitializeKiln(ctx *godog.ScenarioContext) { initializeKiln(ctx) } func initializeKiln(ctx scenarioContext) { ctx.Step(regexp.MustCompile(`^I invoke kiln$`), iInvokeKiln) ctx.Step(regexp.MustCompile(`^I try to invoke kiln$`), iTryToInvokeKiln) + ctx.Step(regexp.MustCompile(`^the "([^"]*)" release tarball exists$`), theReleaseTarballExists) + ctx.Step(regexp.MustCompile(`^the "([^"]*)" release tarball does not exist$`), theReleaseTarballDoesNotExist) } func InitializeRegex(ctx *godog.ScenarioContext) { initializeRegex(ctx) } diff --git a/internal/acceptance/workflows/scenario/step_funcs_kiln.go b/internal/acceptance/workflows/scenario/step_funcs_kiln.go index 5e9cad2b..73931d1a 100644 --- a/internal/acceptance/workflows/scenario/step_funcs_kiln.go +++ b/internal/acceptance/workflows/scenario/step_funcs_kiln.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "os" + "path/filepath" "regexp" "strings" @@ -18,6 +19,36 @@ func iTryToInvokeKiln(ctx context.Context, table *godog.Table) (context.Context, return invokeKiln(ctx, false, argsFromTable(table)...) } +func theReleaseTarballExists(ctx context.Context, tarballName string) error { + tileDir, err := tileRepoPath(ctx) + if err != nil { + return fmt.Errorf("failed to get tile repo path: %w", err) + } + + releasePath := filepath.Join(tileDir, "releases", tarballName) + _, err = os.Stat(releasePath) + return err +} + +func theReleaseTarballDoesNotExist(ctx context.Context, tarballName string) error { + tileDir, err := tileRepoPath(ctx) + if err != nil { + return fmt.Errorf("failed to get tile repo path: %w", err) + } + + releasePath := filepath.Join(tileDir, "releases", tarballName) + _, err = os.Stat(releasePath) + if err == nil { + return fmt.Errorf("release tarball %q exists", tarballName) + } + + if !os.IsNotExist(err) { + return err + } + + return nil +} + func kilnValidateSucceeds(ctx context.Context) (context.Context, error) { return invokeKiln(ctx, true, "validate", "--variable=github_access_token=banana") } diff --git a/internal/acceptance/workflows/updating_releases.feature b/internal/acceptance/workflows/updating_releases.feature index ac2bef48..6d08fc16 100644 --- a/internal/acceptance/workflows/updating_releases.feature +++ b/internal/acceptance/workflows/updating_releases.feature @@ -11,6 +11,7 @@ Feature: As a dependabot, I want to update a BOSH Release Scenario: Find a version on bosh.io Given I have a tile source directory "testdata/tiles/v2" + And the repository has no fetched releases And I set the version constraint to "1.1.18" for release "bpm" When I invoke kiln | find-release-version | @@ -18,15 +19,30 @@ Feature: As a dependabot, I want to update a BOSH Release | --variable=github_access_token="${GITHUB_ACCESS_TOKEN}" | Then stdout contains substring: "1.1.18" - Scenario: Update a component to a new release + Scenario: Update a component to a new release with download Given I have a tile source directory "testdata/tiles/v2" + And the repository has no fetched releases And the Kilnfile.lock specifies version "0.2.3" for release "hello-release" - And GitHub repository "crhntr/hello-release" has release with tag "v0.2.3" + And GitHub repository "crhntr/hello-release" has release with tag "0.3.0" When I invoke kiln | update-release | | --name=hello-release | - | --version=v0.2.3 | + | --version=0.3.0 | + | --variable=github_access_token="${GITHUB_ACCESS_TOKEN}" | + Then the Kilnfile.lock specifies version "0.3.0" for release "hello-release" + And kiln validate succeeds + And the "hello-release-0.3.0.tgz" release tarball exists + + Scenario: Update a component to a new release without download + Given I have a tile source directory "testdata/tiles/v2" + And the repository has no fetched releases + And the Kilnfile.lock specifies version "1.2.12" for release "bpm" + When I invoke kiln + | update-release | + | --name=bpm | + | --version=1.2.13 | | --without-download | | --variable=github_access_token="${GITHUB_ACCESS_TOKEN}" | - Then the Kilnfile.lock specifies version "0.2.3" for release "hello-release" + Then the Kilnfile.lock specifies version "1.2.13" for release "bpm" And kiln validate succeeds + And the "bpm-1.2.13.tgz" release tarball does not exist diff --git a/internal/acceptance/workflows/updating_stemcell.feature b/internal/acceptance/workflows/updating_stemcell.feature index c206e208..29000e79 100644 --- a/internal/acceptance/workflows/updating_stemcell.feature +++ b/internal/acceptance/workflows/updating_stemcell.feature @@ -13,12 +13,45 @@ Feature: As a dependabot, I want to update a stemcell | --variable=github_access_token="${GITHUB_ACCESS_TOKEN}" | Then stdout contains substring: "1.340" - Scenario: Update the stemcell + Scenario: Update the stemcell with download Given I have a tile source directory "testdata/tiles/v2" + And the repository has no fetched releases And TanzuNetwork has product "stemcells-ubuntu-jammy" with version "1.340" And "Kilnfile.lock" contains substring: version: "1.329" When I invoke kiln | update-stemcell | - | --version=1.340 | + | --version=1.340 | | --variable=github_access_token="${GITHUB_ACCESS_TOKEN}" | Then "Kilnfile.lock" contains substring: version: "1.340" + And the Kilnfile.lock specifies version "0.2.3" for release "hello-release" + And the "bpm-1.2.12.tgz" release tarball exists + And the "hello-release-0.2.3.tgz" release tarball exists + + Scenario: Update the stemcell without download + Given I have a tile source directory "testdata/tiles/v2" + And the repository has no fetched releases + And TanzuNetwork has product "stemcells-ubuntu-jammy" with version "1.340" + And "Kilnfile.lock" contains substring: version: "1.329" + When I invoke kiln + | update-stemcell | + | --version=1.340 | + | --without-download | + | --variable=github_access_token="${GITHUB_ACCESS_TOKEN}" | + Then "Kilnfile.lock" contains substring: version: "1.340" + And the Kilnfile.lock specifies version "0.2.3" for release "hello-release" + And the "bpm-1.2.12.tgz" release tarball does not exist + And the "hello-release-0.2.3.tgz" release tarball does not exist + + Scenario: Update the stemcell with release updates + Given I have a tile source directory "testdata/tiles/v2" + And the repository has no fetched releases + And TanzuNetwork has product "stemcells-ubuntu-jammy" with version "1.340" + And "Kilnfile.lock" contains substring: version: "1.329" + When I invoke kiln + | update-stemcell | + | --version=1.340 | + | --update-releases | + | --variable=github_access_token="${GITHUB_ACCESS_TOKEN}" | + Then "Kilnfile.lock" contains substring: version: "1.340" + And the Kilnfile.lock specifies version "0.3.0" for release "hello-release" + And the "hello-release-0.3.0.tgz" release tarball exists