From 2f9589d3f662de3d56da84cd32b877bffca0fc92 Mon Sep 17 00:00:00 2001 From: Christian Schlotter Date: Wed, 15 May 2024 12:58:48 +0200 Subject: [PATCH] test: check for metadata.yaml when resolving releases to not try to use unreleased versions --- .../client/repository/repository_github.go | 6 + test/framework/clusterctl/e2e_config.go | 134 ++++++++++++++++-- test/framework/clusterctl/e2e_config_test.go | 69 ++++++++- 3 files changed, 193 insertions(+), 16 deletions(-) diff --git a/cmd/clusterctl/client/repository/repository_github.go b/cmd/clusterctl/client/repository/repository_github.go index 45ea67f54161..786a06128549 100644 --- a/cmd/clusterctl/client/repository/repository_github.go +++ b/cmd/clusterctl/client/repository/repository_github.go @@ -414,6 +414,12 @@ func (g *gitHubRepository) httpGetFilesFromRelease(ctx context.Context, version, } defer resp.Body.Close() + // if we get 404 there is no reason to retry + if resp.StatusCode == http.StatusNotFound { + retryError = errNotFound + return true, nil + } + if resp.StatusCode != http.StatusOK { retryError = errors.Errorf("error getting file, status code: %d", resp.StatusCode) return false, nil diff --git a/test/framework/clusterctl/e2e_config.go b/test/framework/clusterctl/e2e_config.go index adfe68e13e35..b50440a902be 100644 --- a/test/framework/clusterctl/e2e_config.go +++ b/test/framework/clusterctl/e2e_config.go @@ -19,6 +19,8 @@ package clusterctl import ( "context" "fmt" + "io" + "net/http" "net/url" "os" "path/filepath" @@ -33,6 +35,7 @@ import ( . "github.com/onsi/gomega" "github.com/pkg/errors" "k8s.io/apimachinery/pkg/util/version" + "k8s.io/apimachinery/pkg/util/wait" "k8s.io/utils/ptr" "sigs.k8s.io/yaml" @@ -304,13 +307,16 @@ func ResolveRelease(ctx context.Context, releaseMarker string) (string, error) { return "", errors.Errorf("releasemarker does not support disabling the go proxy: GOPROXY=%q", os.Getenv("GOPROXY")) } goproxyClient := goproxy.NewClient(scheme, host) - return resolveReleaseMarker(ctx, releaseMarker, goproxyClient) + return resolveReleaseMarker(ctx, releaseMarker, goproxyClient, githubReleaseMetadataURL) } // resolveReleaseMarker resolves releaseMarker string to verion string e.g. // - Resolves "go://sigs.k8s.io/cluster-api@v1.0" to the latest stable patch release of v1.0. // - Resolves "go://sigs.k8s.io/cluster-api@latest-v1.0" to the latest patch release of v1.0 including rc and pre releases. -func resolveReleaseMarker(ctx context.Context, releaseMarker string, goproxyClient *goproxy.Client) (string, error) { +// It also checks if a release actually exists by trying to query for the `metadata.yaml` file. +// To do that the url gets calculated by the toMetadataURL function. The toMetadataURL func +// is passed as variable to be able to write a proper unit test. +func resolveReleaseMarker(ctx context.Context, releaseMarker string, goproxyClient *goproxy.Client, toMetadataURL func(gomodule, version string) string) (string, error) { if !strings.HasPrefix(releaseMarker, "go://") { return "", errors.Errorf("unknown release marker scheme") } @@ -330,31 +336,131 @@ func resolveReleaseMarker(ctx context.Context, releaseMarker string, goproxyClie if strings.HasPrefix(gomoduleParts[1], "latest-") { includePrereleases = true } - version := strings.TrimPrefix(gomoduleParts[1], "latest-") + ".0" - version = strings.TrimPrefix(version, "v") - semVersion, err := semver.Parse(version) + rawVersion := strings.TrimPrefix(gomoduleParts[1], "latest-") + ".0" + rawVersion = strings.TrimPrefix(rawVersion, "v") + semVersion, err := semver.Parse(rawVersion) if err != nil { - return "", errors.Wrapf(err, "parsing semver for %s", version) + return "", errors.Wrapf(err, "parsing semver for %s", rawVersion) } - parsedTags, err := goproxyClient.GetVersions(ctx, gomodule) + versions, err := goproxyClient.GetVersions(ctx, gomodule) if err != nil { return "", err } - var picked semver.Version - for i, tag := range parsedTags { - if !includePrereleases && len(tag.Pre) > 0 { + // Search for the latest release according to semantic version ordering. + // Releases with tag name that are not in semver format are ignored. + versionCandidates := []semver.Version{} + for _, version := range versions { + if !includePrereleases && len(version.Pre) > 0 { continue } - if tag.Major == semVersion.Major && tag.Minor == semVersion.Minor { - picked = parsedTags[i] + if version.Major != semVersion.Major || version.Minor != semVersion.Minor { + continue } + + versionCandidates = append(versionCandidates, version) } - if picked.Major == 0 && picked.Minor == 0 && picked.Patch == 0 { + + if len(versionCandidates) == 0 { return "", errors.Errorf("no suitable release available for release marker %s", releaseMarker) } - return picked.String(), nil + + // Sort parsed versions by semantic version order. + sort.SliceStable(versionCandidates, func(i, j int) bool { + // Prioritize pre-release versions over releases. For example v2.0.0-alpha > v1.0.0 + // If both are pre-releases, sort by semantic version order as usual. + if len(versionCandidates[i].Pre) == 0 && len(versionCandidates[j].Pre) > 0 { + return false + } + if len(versionCandidates[j].Pre) == 0 && len(versionCandidates[i].Pre) > 0 { + return true + } + + return versionCandidates[j].LT(versionCandidates[i]) + }) + + // Limit the number of searchable versions by 5. + versionCandidates = versionCandidates[:min(5, len(versionCandidates))] + + for _, v := range versionCandidates { + // Iterate through sorted versions and try to fetch a file from that release. + // If it's completed successfully, we get the latest release. + // Note: the fetched file will be cached and next time we will get it from the cache. + versionString := "v" + v.String() + _, err := httpGetURL(ctx, toMetadataURL(gomodule, versionString)) + if err != nil { + if errors.Is(err, errNotFound) { + // Ignore this version + continue + } + + return "", err + } + + return v.String(), nil + } + + // If we reached this point, it means we didn't find any release. + return "", errors.New("failed to find releases tagged with a valid semantic version number") +} + +var ( + retryableOperationInterval = 10 * time.Second + retryableOperationTimeout = 1 * time.Minute + errNotFound = errors.New("404 Not Found") +) + +func githubReleaseMetadataURL(gomodule, version string) string { + // Rewrite gomodule to the github repository + if strings.HasPrefix(gomodule, "k8s.io") { + gomodule = strings.Replace(gomodule, "k8s.io", "github.com/kubernetes", 1) + } + if strings.HasPrefix(gomodule, "sigs.k8s.io") { + gomodule = strings.Replace(gomodule, "sigs.k8s.io", "github.com/kubernetes-sigs", 1) + } + + return fmt.Sprintf("https://%s/releases/download/%s/metadata.yaml", gomodule, version) +} + +// httpGetURL does a GET request to the given url and returns its content. +// If the responses StatusCode is 404 (StatusNotFound) it does not do a retry because +// the result is not expected to change. +func httpGetURL(ctx context.Context, url string) ([]byte, error) { + var retryError error + var content []byte + _ = wait.PollUntilContextTimeout(ctx, retryableOperationInterval, retryableOperationTimeout, true, func(context.Context) (bool, error) { + resp, err := http.Get(url) //nolint:gosec,noctx + if err != nil { + retryError = errors.Wrap(err, "error sending request") + return false, nil + } + defer resp.Body.Close() + + // if we get 404 there is no reason to retry + if resp.StatusCode == http.StatusNotFound { + retryError = errNotFound + return true, nil + } + + if resp.StatusCode != http.StatusOK { + retryError = errors.Errorf("error getting file, status code: %d", resp.StatusCode) + return false, nil + } + + content, err = io.ReadAll(resp.Body) + if err != nil { + retryError = errors.Wrap(err, "error reading response body") + return false, nil + } + + retryError = nil + return true, nil + }) + if retryError != nil { + return nil, retryError + } + return content, nil } // Defaults assigns default values to the object. More specifically: diff --git a/test/framework/clusterctl/e2e_config_test.go b/test/framework/clusterctl/e2e_config_test.go index 60e81a8087a3..ba93d51cacbf 100644 --- a/test/framework/clusterctl/e2e_config_test.go +++ b/test/framework/clusterctl/e2e_config_test.go @@ -20,6 +20,8 @@ import ( "context" "fmt" "net/http" + "net/http/httptest" + "strings" "testing" . "github.com/onsi/gomega" @@ -33,7 +35,24 @@ func Test_resolveReleaseMarker(t *testing.T) { clientGoproxy := goproxy.NewClient(scheme, host) defer teardownGoproxy() - // setup an handler with fake releases + toMetadataURL, mux, teardown := newFakeGithubReleases() + defer teardown() + + validMetadataURLs := []string{ + "github.com/o/r1/releases/download/v1.2.0/metadata.yaml", + "github.com/o/r1/releases/download/v1.2.1-rc.0/metadata.yaml", + "github.com/o/r2/releases/download/v1.2.0/metadata.yaml", + "github.com/kubernetes-sigs/foo/releases/download/v1.0.0/metadata.yaml", + } + // Setup an handler for returning metadata.yaml. + for _, url := range validMetadataURLs { + mux.HandleFunc("/"+url, func(w http.ResponseWriter, r *http.Request) { + goproxytest.HTTPTestMethod(t, r, "GET") + fmt.Fprint(w, `somedata`) + }) + } + + // setup an handlers with fake releases muxGoproxy.HandleFunc("/github.com/o/r1/@v/list", func(w http.ResponseWriter, r *http.Request) { goproxytest.HTTPTestMethod(t, r, "GET") fmt.Fprint(w, "v1.2.0\n") @@ -41,6 +60,19 @@ func Test_resolveReleaseMarker(t *testing.T) { fmt.Fprint(w, "v1.3.0-rc.0\n") fmt.Fprint(w, "v1.3.0-rc.1\n") }) + muxGoproxy.HandleFunc("/github.com/o/r2/@v/list", func(w http.ResponseWriter, r *http.Request) { + goproxytest.HTTPTestMethod(t, r, "GET") + fmt.Fprint(w, "v1.2.0\n") + fmt.Fprint(w, "v1.2.1\n") + fmt.Fprint(w, "v1.2.2\n") + }) + muxGoproxy.HandleFunc("/sigs.k8s.io/foo/@v/list", func(w http.ResponseWriter, r *http.Request) { + goproxytest.HTTPTestMethod(t, r, "GET") + fmt.Fprint(w, "v1.0.0\n") + fmt.Fprint(w, "v1.0.1\n") + fmt.Fprint(w, "v1.0.2\n") + }) + tests := []struct { name string releaseMarker string @@ -71,13 +103,25 @@ func Test_resolveReleaseMarker(t *testing.T) { want: "", wantErr: true, }, + { + name: "Get latest release with metadata", + releaseMarker: "go://github.com/o/r2@latest-v1.2", + want: "1.2.0", + wantErr: false, + }, + { + name: "Get latest release for kubernetes-sigs project", + releaseMarker: "go://sigs.k8s.io/foo@latest-v1.0", + want: "1.0.0", + wantErr: false, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { g := NewWithT(t) - got, err := resolveReleaseMarker(context.Background(), tt.releaseMarker, clientGoproxy) + got, err := resolveReleaseMarker(context.Background(), tt.releaseMarker, clientGoproxy, toMetadataURL) if tt.wantErr { g.Expect(err).To(HaveOccurred()) return @@ -159,3 +203,24 @@ func Test_E2EConfig_DeepCopy(t *testing.T) { "config1-interval": {"2m", "10s"}, })) } + +// newFakeGithubReleases sets up a test HTTP server along with a github.Client that is +// configured to talk to that test server. Tests should register handlers on +// mux which provide mock responses for the API method being tested. +func newFakeGithubReleases() (toMetadataURL func(gomodule, version string) string, mux *http.ServeMux, teardown func()) { + // mux is the HTTP request multiplexer used with the test server. + mux = http.NewServeMux() + + apiHandler := http.NewServeMux() + apiHandler.Handle("/", mux) + + // server is a test HTTP server used to provide mock API responses. + server := httptest.NewServer(apiHandler) + + toMetadataURL = func(gomodule, version string) string { + url := githubReleaseMetadataURL(gomodule, version) + return fmt.Sprintf("%s/%s", server.URL, strings.TrimPrefix(url, "https://")) + } + + return toMetadataURL, mux, server.Close +}