From d81572c3cbf6f0378c0852a149702b5e04dbeb37 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nikola=20Prokopi=C4=87?= <5638639+nprokopic@users.noreply.github.com> Date: Fri, 26 Jul 2024 02:39:25 +0200 Subject: [PATCH] Fix GetLatestRelease (#1330) --- sdk/CHANGELOG.md | 5 ++++ sdk/builder.go | 14 +++++----- sdk/builder_test.go | 36 +++++++++++++------------- sdk/client.go | 62 +++++++++++++++++++++++++-------------------- sdk/client_test.go | 4 ++- 5 files changed, 70 insertions(+), 51 deletions(-) diff --git a/sdk/CHANGELOG.md b/sdk/CHANGELOG.md index 92b96fa44..8d21cebcb 100644 --- a/sdk/CHANGELOG.md +++ b/sdk/CHANGELOG.md @@ -9,6 +9,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Changed + +- Fix `GetLatestRelease` to correctly return the latest release according to semver, and not the latest published GitHub + release. The function now searches for the latest release in the releases GitHub repo and not in GitHub releases. + ## [0.5.1] - 2024-07-23 ### Fixed diff --git a/sdk/builder.go b/sdk/builder.go index a1b25aea0..e9a15092f 100644 --- a/sdk/builder.go +++ b/sdk/builder.go @@ -245,12 +245,14 @@ func (b *Builder) buildPreReleaseString() (string, error) { } // check if cluster app has a pre-release - clusterAppPreReleaseString, err := versionStringHasPreReleaseString(b.clusterApp.Version) - if err != nil { - return "", microerror.Mask(err) - } - if clusterAppPreReleaseString { - return preReleaseBuildFunc(), nil + if b.clusterApp != nil { + clusterAppPreReleaseString, err := versionStringHasPreReleaseString(b.clusterApp.Version) + if err != nil { + return "", microerror.Mask(err) + } + if clusterAppPreReleaseString { + return preReleaseBuildFunc(), nil + } } // check if apps have a pre-release diff --git a/sdk/builder_test.go b/sdk/builder_test.go index 6b9d17627..f3e3b2597 100644 --- a/sdk/builder_test.go +++ b/sdk/builder_test.go @@ -36,7 +36,7 @@ var _ = Describe("Builder", func() { // set a cluster app override where the pre-release is different const preReleasePrefix = "test" releasesBuilder = releasesBuilder. - WithClusterApp("0.76.1-efdaba18e6e9866d62f3214f3d898b21c21e8b48", ""). + WithClusterApp("1.0.0-efdaba18e6e9866d62f3214f3d898b21c21e8b48", ""). WithPreReleasePrefix(preReleasePrefix) // build the new release @@ -50,9 +50,9 @@ var _ = Describe("Builder", func() { newReleaseVersion, err := semver.NewVersion(newReleaseVersionString) Expect(err).NotTo(HaveOccurred()) - // check the new release version, expected is 25.1.1- + // check the new release version, expected is 25.0.1- Expect(newReleaseVersion.Major()).To(Equal(uint64(25))) - Expect(newReleaseVersion.Minor()).To(Equal(uint64(1))) + Expect(newReleaseVersion.Minor()).To(Equal(uint64(0))) Expect(newReleaseVersion.Patch()).To(Equal(uint64(1))) Expect(newReleaseVersion.Prerelease()).To(HavePrefix("%s.", preReleasePrefix)) Expect(newReleaseVersion.Prerelease()).To(HaveLen(expectedPreReleaseLength)) @@ -61,7 +61,7 @@ var _ = Describe("Builder", func() { It("Overrides cluster app and creates a new minor release with pre-release", func() { // set a cluster app override where the pre-release is different releasesBuilder = releasesBuilder. - WithClusterApp("0.77.0", "") + WithClusterApp("1.1.0", "") // build the new release newRelease, err := releasesBuilder.Build(context.Background()) @@ -75,22 +75,22 @@ var _ = Describe("Builder", func() { // Check the new release version. // - // Since the builder will use the latest release, which is v25.1.0-demo.0, it is expected that the custom + // Since the builder will use the latest release, which is v25.0.0, it is expected that the custom // release version has a minor bump (because we are building a newer release with a new cluster-aws minor - // version), so the new release version is 25.2.0-. + // version), so the new release version is 25.1.0. // - // New release also has a pre-release because the base release is a pre-release. + // New release also doesn't have a pre-release because the base release doesn't have it. Expect(newReleaseVersion.Major()).To(Equal(uint64(25))) - Expect(newReleaseVersion.Minor()).To(Equal(uint64(2))) + Expect(newReleaseVersion.Minor()).To(Equal(uint64(1))) Expect(newReleaseVersion.Patch()).To(Equal(uint64(0))) - Expect(newReleaseVersion.Prerelease()).To(HaveLen(10)) + Expect(newReleaseVersion.Prerelease()).To(BeEmpty()) }) It("Overrides cluster app and creates a new minor release with random pre-release", func() { // set a cluster app override where the pre-release is different const preReleaseLength = 10 releasesBuilder = releasesBuilder. - WithClusterApp("0.77.0", ""). + WithClusterApp("1.1.0", ""). WithRandomPreRelease(preReleaseLength) // build the new release @@ -105,13 +105,13 @@ var _ = Describe("Builder", func() { // Check the new release version. // - // Since the builder will use the latest release, which is v25.1.0-demo.0, it is expected that the custom + // Since the builder will use the latest release, which is v25.0.0, it is expected that the custom // release version has a minor bump (because we are building a newer release with a new cluster-aws minor - // version), so the new release version is 25.2.0-. + // version), so the new release version is 25.1.0-. // - // New release also has a pre-release because the base release is a pre-release. + // New release also has a random pre-release with the specified length. Expect(newReleaseVersion.Major()).To(Equal(uint64(25))) - Expect(newReleaseVersion.Minor()).To(Equal(uint64(2))) + Expect(newReleaseVersion.Minor()).To(Equal(uint64(1))) Expect(newReleaseVersion.Patch()).To(Equal(uint64(0))) Expect(newReleaseVersion.Prerelease()).To(HaveLen(preReleaseLength)) }) @@ -131,11 +131,13 @@ var _ = Describe("Builder", func() { newReleaseVersion, err := semver.NewVersion(newReleaseVersionString) Expect(err).NotTo(HaveOccurred()) - // Check the new release version. Since the builder will use the latest release, which is v25.1.0-demo.0, + // Check the new release version. Since the builder will use the latest release, which is v25.0.0, // expected is custom release version has a patch bump (because we are building a newer release), - // so the new release version is 25.1.1-. + // so the new release version is 25.0.1-. + // + // New release also has a random pre-release with the specified length. Expect(newReleaseVersion.Major()).To(Equal(uint64(25))) - Expect(newReleaseVersion.Minor()).To(Equal(uint64(1))) + Expect(newReleaseVersion.Minor()).To(Equal(uint64(0))) Expect(newReleaseVersion.Patch()).To(Equal(uint64(1))) Expect(newReleaseVersion.Prerelease()).To(HaveLen(10)) }) diff --git a/sdk/client.go b/sdk/client.go index e96b826a4..ccbd0e6c0 100644 --- a/sdk/client.go +++ b/sdk/client.go @@ -6,6 +6,7 @@ import ( "io" "net/http" "path" + "slices" "strings" "github.com/Masterminds/semver/v3" @@ -80,7 +81,7 @@ func (c *Client) GetRelease(ctx context.Context, provider Provider, releaseVersi // GetNewReleasesForGitReference returns newly added releases for the specified provider and from the specified git // reference. // -// Currently, the only supported provider is "aws". Git reference can be any commit, branch or tag. +// Currently, the supported providers are "aws" and "azure". Git reference can be any commit, branch or tag. func (c *Client) GetNewReleasesForGitReference(ctx context.Context, provider Provider, gitReference string) ([]Release, error) { providerDirectory, err := getProviderDirectory(provider) if err != nil { @@ -137,7 +138,7 @@ func (c *Client) GetNewReleasesForGitReference(ctx context.Context, provider Pro // GetReleasesForGitReference returns all releases for the specified provider and from the specified git reference. // -// Currently, the only supported provider is "aws". Git reference can be any commit, branch or tag. +// Currently, the supported providers are "aws" and "azure". Git reference can be any commit, branch or tag. func (c *Client) GetReleasesForGitReference(ctx context.Context, provider Provider, gitReference string) ([]Release, error) { providerDirectory, err := getProviderDirectory(provider) if err != nil { @@ -178,7 +179,7 @@ func (c *Client) GetReleasesForGitReference(ctx context.Context, provider Provid // GetReleaseForGitReference returns a release with the specified release version for the specified provider and from // the specified git reference. // -// Currently, the only supported provider is "aws". Release version can contain the 'v' prefix, but it doesn't have to. +// Currently, the supported providers are "aws" and "azure". Release version can contain the 'v' prefix, but it doesn't have to. // Git reference can be any commit, branch or tag. func (c *Client) GetReleaseForGitReference(ctx context.Context, provider Provider, releaseVersion, gitReference string) (*Release, error) { // First we get GitHub release for the specified provider and release version. @@ -214,37 +215,44 @@ func (c *Client) GetReleaseForGitReference(ctx context.Context, provider Provide // GetLatestRelease returns the latest release for the specified provider. func (c *Client) GetLatestRelease(ctx context.Context, provider Provider) (*Release, error) { - const releasesPerRequest = 30 - providerTagPrefix := fmt.Sprintf("%s/", provider) - - var releaseList []*github.RepositoryRelease - var response *github.Response - var err error - hasMorePages := func(page int) bool { - return page != 0 + allActiveReleases, err := c.GetReleasesForGitReference(ctx, provider, ReleasesRepoDefaultBranch) + if err != nil { + return nil, microerror.Mask(err) + } + if len(allActiveReleases) == 0 { + return nil, microerror.Maskf(ReleaseNotFoundError, "Did not found any release for provider '%s'", provider) } - for page := 1; hasMorePages(page); page = response.NextPage { - releaseList, response, err = c.gitHubClient.Repositories.ListReleases(ctx, GiantSwarmGitHubOrg, ReleasesRepo, &github.ListOptions{ - Page: page, - PerPage: releasesPerRequest, - }) + var sortErr error + slices.SortFunc[[]Release](allActiveReleases, func(r1, r2 Release) int { + r1VersionString, err := r1.GetVersion() if err != nil { - return nil, microerror.Mask(err) + sortErr = err + return 0 } - for _, gitHubRelease := range releaseList { - releaseTag := gitHubRelease.GetTagName() - if strings.HasPrefix(releaseTag, providerTagPrefix) { - release, err := c.getReleaseResourceFromGitHubRelease(gitHubRelease) - if err != nil { - return nil, microerror.Mask(err) - } - return release, nil - } + r1Version, err := semver.NewVersion(r1VersionString) + if err != nil { + sortErr = err + return 0 + } + r2VersionString, err := r2.GetVersion() + if err != nil { + sortErr = err + return 0 + } + r2Version, err := semver.NewVersion(r2VersionString) + if err != nil { + sortErr = err + return 0 } + return r1Version.Compare(r2Version) + }) + if sortErr != nil { + return nil, microerror.Mask(sortErr) } - return nil, microerror.Maskf(ReleaseNotFoundError, "Did not found any release for provider '%s'", provider) + latestRelease := allActiveReleases[len(allActiveReleases)-1] + return &latestRelease, nil } func (c *Client) getReleaseResourceFromGitHubRelease(gitHubRelease *github.RepositoryRelease) (*Release, error) { diff --git a/sdk/client_test.go b/sdk/client_test.go index c7ec53889..5952be21e 100644 --- a/sdk/client_test.go +++ b/sdk/client_test.go @@ -123,7 +123,7 @@ var _ = Describe("Client", func() { Expect(err).NotTo(HaveOccurred()) // Check release version - const expectedLatestReleaseVersion = "25.1.0-demo.0" + const expectedLatestReleaseVersion = "25.0.0" resultReleaseVersion, err := release.GetVersion() Expect(err).NotTo(HaveOccurred()) Expect(resultReleaseVersion).To(Equal(expectedLatestReleaseVersion)) @@ -199,6 +199,8 @@ func createAndStartTestServer() *httptest.Server { switch ref { case "add-capa-v25.0.0": + fallthrough + case ReleasesRepoDefaultBranch: _, err = rw.Write([]byte(capaDirectoryGitReferenceResponse)) default: _, err = rw.Write([]byte(capaDirectoryGitDefaultResponse))