diff --git a/cmd/clusterctl/client/repository/repository_github.go b/cmd/clusterctl/client/repository/repository_github.go index 38f4b12cdcb8..1632abd0cbfa 100644 --- a/cmd/clusterctl/client/repository/repository_github.go +++ b/cmd/clusterctl/client/repository/repository_github.go @@ -42,10 +42,11 @@ import ( ) const ( - httpsScheme = "https" - githubDomain = "github.com" - githubReleaseRepository = "releases" - githubLatestReleaseLabel = "latest" + httpsScheme = "https" + githubDomain = "github.com" + githubReleaseRepository = "releases" + githubLatestReleaseLabel = "latest" + githubListReleasesPerPageLimit = 100 ) var ( @@ -158,7 +159,7 @@ func (g *gitHubRepository) GetFile(version, path string) ([]byte, error) { return nil, errors.Wrapf(err, "failed to get GitHub release %s", version) } - // download files from the release + // Download files from the release. files, err := g.downloadFilesFromRelease(release, path) if err != nil { return nil, errors.Wrapf(err, "failed to download files from GitHub release %s", version) @@ -199,9 +200,9 @@ func NewGitHubRepository(providerConfig config.Provider, configVariablesClient c defaultVersion := urlSplit[3] path := strings.Join(urlSplit[4:], "/") - // use path's directory as a rootPath + // Use path's directory as a rootPath. rootPath := filepath.Dir(path) - // use the file name (if any) as componentsPath + // Use the file name (if any) as componentsPath. componentsPath := getComponentsPath(path, rootPath) repo := &gitHubRepository{ @@ -214,7 +215,7 @@ func NewGitHubRepository(providerConfig config.Provider, configVariablesClient c componentsPath: componentsPath, } - // process githubRepositoryOptions + // Process githubRepositoryOptions. for _, o := range opts { o(repo) } @@ -278,21 +279,39 @@ func (g *gitHubRepository) setClientToken(token string) { func (g *gitHubRepository) getVersions() ([]string, error) { client := g.getClient() - // get all the releases + // Get all the releases. // NB. currently Github API does not support result ordering, so it not possible to limit results - var releases []*github.RepositoryRelease + var allReleases []*github.RepositoryRelease var retryError error _ = wait.PollImmediate(retryableOperationInterval, retryableOperationTimeout, func() (bool, error) { var listReleasesErr error - releases, _, listReleasesErr = client.Repositories.ListReleases(context.TODO(), g.owner, g.repository, nil) + // Get the first page of GitHub releases. + releases, response, listReleasesErr := client.Repositories.ListReleases(context.TODO(), g.owner, g.repository, &github.ListOptions{PerPage: githubListReleasesPerPageLimit}) if listReleasesErr != nil { retryError = g.handleGithubErr(listReleasesErr, "failed to get the list of releases") - // return immediately if we are rate limited + // Return immediately if we are rate limited. if _, ok := listReleasesErr.(*github.RateLimitError); ok { return false, retryError } return false, nil } + allReleases = append(allReleases, releases...) + + // Paginated GitHub APIs provide pointers to the first, next, previous and last + // pages in the response, which can be used to iterate through the pages. + // https://github.com/google/go-github/blob/14bb610698fc2f9013cad5db79b2d5fe4d53e13c/github/github.go#L541-L551 + for response.NextPage != 0 { + releases, response, listReleasesErr = client.Repositories.ListReleases(context.TODO(), g.owner, g.repository, &github.ListOptions{Page: response.NextPage, PerPage: githubListReleasesPerPageLimit}) + if listReleasesErr != nil { + retryError = g.handleGithubErr(listReleasesErr, "failed to get the list of releases") + // Return immediately if we are rate limited. + if _, ok := listReleasesErr.(*github.RateLimitError); ok { + return false, retryError + } + return false, nil + } + allReleases = append(allReleases, releases...) + } retryError = nil return true, nil }) @@ -300,7 +319,7 @@ func (g *gitHubRepository) getVersions() ([]string, error) { return nil, retryError } versions := []string{} - for _, r := range releases { + for _, r := range allReleases { r := r // pin if r.TagName == nil { continue @@ -332,7 +351,7 @@ func (g *gitHubRepository) getReleaseByTag(tag string) (*github.RepositoryReleas release, _, getReleasesErr = client.Repositories.GetReleaseByTag(context.TODO(), g.owner, g.repository, tag) if getReleasesErr != nil { retryError = g.handleGithubErr(getReleasesErr, "failed to read release %q", tag) - // return immediately if we are rate limited + // Return immediately if we are rate limited. if _, ok := getReleasesErr.(*github.RateLimitError); ok { return false, retryError } @@ -361,7 +380,7 @@ func (g *gitHubRepository) downloadFilesFromRelease(release *github.RepositoryRe client := g.getClient() absoluteFileName := filepath.Join(g.rootPath, fileName) - // search for the file into the release assets, retrieving the asset id + // Search for the file into the release assets, retrieving the asset id. var assetID *int64 for _, a := range release.Assets { if a.Name != nil && *a.Name == absoluteFileName { @@ -381,14 +400,14 @@ func (g *gitHubRepository) downloadFilesFromRelease(release *github.RepositoryRe reader, redirect, downloadReleaseError = client.Repositories.DownloadReleaseAsset(ctx, g.owner, g.repository, *assetID, http.DefaultClient) if downloadReleaseError != nil { retryError = g.handleGithubErr(downloadReleaseError, "failed to download file %q from %q release", *release.TagName, fileName) - // return immediately if we are rate limited + // Return immediately if we are rate limited. if _, ok := downloadReleaseError.(*github.RateLimitError); ok { return false, retryError } return false, nil } if redirect != "" { - // NOTE: DownloadReleaseAsset should not return a redirect address when used with the DefaultClient + // NOTE: DownloadReleaseAsset should not return a redirect address when used with the DefaultClient. retryError = errors.New("unexpected redirect while downloading the release asset") return true, retryError } diff --git a/cmd/clusterctl/client/repository/repository_github_test.go b/cmd/clusterctl/client/repository/repository_github_test.go index a24631102704..ee55abcea316 100644 --- a/cmd/clusterctl/client/repository/repository_github_test.go +++ b/cmd/clusterctl/client/repository/repository_github_test.go @@ -41,21 +41,21 @@ func Test_gitHubRepository_GetVersions(t *testing.T) { client, mux, teardown := test.NewFakeGitHub() defer teardown() - // setup an handler for returning 5 fake releases + // Setup an handler for returning 5 fake releases. mux.HandleFunc("/repos/o/r1/releases", func(w http.ResponseWriter, r *http.Request) { testMethod(t, r, "GET") fmt.Fprint(w, `[`) fmt.Fprint(w, `{"id":1, "tag_name": "v0.4.0"},`) fmt.Fprint(w, `{"id":2, "tag_name": "v0.4.1"},`) fmt.Fprint(w, `{"id":3, "tag_name": "v0.4.2"},`) - fmt.Fprint(w, `{"id":4, "tag_name": "v0.4.3-alpha"}`) // prerelease + fmt.Fprint(w, `{"id":4, "tag_name": "v0.4.3-alpha"}`) // Pre-release fmt.Fprint(w, `]`) }) clientGoproxy, muxGoproxy, teardownGoproxy := newFakeGoproxy() defer teardownGoproxy() - // setup an handler for returning 4 fake releases + // Setup a handler for returning 4 fake releases. muxGoproxy.HandleFunc("/github.com/o/r2/@v/list", func(w http.ResponseWriter, r *http.Request) { testMethod(t, r, "GET") fmt.Fprint(w, "v0.5.0\n") @@ -64,7 +64,7 @@ func Test_gitHubRepository_GetVersions(t *testing.T) { fmt.Fprint(w, "v0.3.1\n") }) - // setup an handler for returning 3 different major fake releases + // Setup a handler for returning 3 different major fake releases. muxGoproxy.HandleFunc("/github.com/o/r3/@v/list", func(w http.ResponseWriter, r *http.Request) { testMethod(t, r, "GET") fmt.Fprint(w, "v1.0.0\n") @@ -265,13 +265,13 @@ func Test_githubRepository_getFile(t *testing.T) { providerConfig := config.NewProvider("test", "https://github.com/o/r/releases/v0.4.1/file.yaml", clusterctlv1.CoreProviderType) - // test.NewFakeGitHub and handler for returning a fake release + // Setup a handler for returning a fake release. mux.HandleFunc("/repos/o/r/releases/tags/v0.4.1", func(w http.ResponseWriter, r *http.Request) { testMethod(t, r, "GET") fmt.Fprint(w, `{"id":13, "tag_name": "v0.4.1", "assets": [{"id": 1, "name": "file.yaml"}] }`) }) - // test.NewFakeGitHub an handler for returning a fake release asset + // Setup a handler for returning a fake release asset. mux.HandleFunc("/repos/o/r/releases/assets/1", func(w http.ResponseWriter, r *http.Request) { testMethod(t, r, "GET") w.Header().Set("Content-Type", "application/octet-stream") @@ -337,16 +337,36 @@ func Test_gitHubRepository_getVersions(t *testing.T) { client, mux, teardown := test.NewFakeGitHub() defer teardown() - // setup an handler for returning 5 fake releases + // Setup a handler for returning fake releases in a paginated manner + // Each response contains a link to the next page (if available) which + // is parsed by the handler to navigate through all pages mux.HandleFunc("/repos/o/r1/releases", func(w http.ResponseWriter, r *http.Request) { testMethod(t, r, "GET") - fmt.Fprint(w, `[`) - fmt.Fprint(w, `{"id":1, "tag_name": "v0.4.0"},`) - fmt.Fprint(w, `{"id":2, "tag_name": "v0.4.1"},`) - fmt.Fprint(w, `{"id":3, "tag_name": "v0.4.2"},`) - fmt.Fprint(w, `{"id":4, "tag_name": "v0.4.3-alpha"},`) // prerelease - fmt.Fprint(w, `{"id":5, "tag_name": "foo"}`) // no semantic version tag - fmt.Fprint(w, `]`) + page := r.URL.Query().Get("page") + switch page { + case "", "1": + // Page 1 + w.Header().Set("Link", `; rel="next"`) // Link to page 2 + fmt.Fprint(w, `[`) + fmt.Fprint(w, `{"id":1, "tag_name": "v0.4.0"},`) + fmt.Fprint(w, `{"id":2, "tag_name": "v0.4.1"}`) + fmt.Fprint(w, `]`) + case "2": + // Page 2 + w.Header().Set("Link", `; rel="next"`) // Link to page 3 + fmt.Fprint(w, `[`) + fmt.Fprint(w, `{"id":3, "tag_name": "v0.4.2"},`) + fmt.Fprint(w, `{"id":4, "tag_name": "v0.4.3-alpha"}`) // Pre-release + fmt.Fprint(w, `]`) + case "3": + // Page 3 (last page) + fmt.Fprint(w, `[`) + fmt.Fprint(w, `{"id":4, "tag_name": "v0.4.4-beta"},`) // Pre-release + fmt.Fprint(w, `{"id":5, "tag_name": "foo"}`) // No semantic version tag + fmt.Fprint(w, `]`) + default: + t.Fatalf("unexpected page requested") + } }) configVariablesClient := test.NewFakeVariableClient() @@ -361,11 +381,11 @@ func Test_gitHubRepository_getVersions(t *testing.T) { wantErr bool }{ { - name: "Get versions", + name: "Get versions with all releases", field: field{ providerConfig: config.NewProvider("test", "https://github.com/o/r1/releases/v0.4.1/path", clusterctlv1.CoreProviderType), }, - want: []string{"v0.4.0", "v0.4.1", "v0.4.2", "v0.4.3-alpha"}, + want: []string{"v0.4.0", "v0.4.1", "v0.4.2", "v0.4.3-alpha", "v0.4.4-beta"}, wantErr: false, }, } @@ -395,13 +415,13 @@ func Test_gitHubRepository_getLatestContractRelease(t *testing.T) { client, mux, teardown := test.NewFakeGitHub() defer teardown() - // test.NewFakeGitHub and handler for returning a fake release + // Setup a handler for returning a fake release. mux.HandleFunc("/repos/o/r1/releases/tags/v0.5.0", func(w http.ResponseWriter, r *http.Request) { testMethod(t, r, "GET") fmt.Fprint(w, `{"id":13, "tag_name": "v0.5.0", "assets": [{"id": 1, "name": "metadata.yaml"}] }`) }) - // test.NewFakeGitHub an handler for returning a fake release metadata file + // Setup a handler for returning a fake release metadata file. mux.HandleFunc("/repos/o/r1/releases/assets/1", func(w http.ResponseWriter, r *http.Request) { testMethod(t, r, "GET") w.Header().Set("Content-Type", "application/octet-stream") @@ -412,7 +432,7 @@ func Test_gitHubRepository_getLatestContractRelease(t *testing.T) { clientGoproxy, muxGoproxy, teardownGoproxy := newFakeGoproxy() defer teardownGoproxy() - // setup an handler for returning 4 fake releases + // Setup a handler for returning 4 fake releases. muxGoproxy.HandleFunc("/github.com/o/r1/@v/list", func(w http.ResponseWriter, r *http.Request) { testMethod(t, r, "GET") fmt.Fprint(w, "v0.5.0\n") @@ -486,7 +506,7 @@ func Test_gitHubRepository_getLatestRelease(t *testing.T) { clientGoproxy, muxGoproxy, teardownGoproxy := newFakeGoproxy() defer teardownGoproxy() - // setup an handler for returning 4 fake releases + // Setup a handler for returning 4 fake releases. muxGoproxy.HandleFunc("/github.com/o/r1/@v/list", func(w http.ResponseWriter, r *http.Request) { testMethod(t, r, "GET") fmt.Fprint(w, "v0.4.1\n") @@ -495,13 +515,13 @@ func Test_gitHubRepository_getLatestRelease(t *testing.T) { fmt.Fprint(w, "foo\n") // no semantic version tag }) - // setup an handler for returning no releases + // Setup a handler for returning no releases. muxGoproxy.HandleFunc("/github.com/o/r2/@v/list", func(w http.ResponseWriter, r *http.Request) { testMethod(t, r, "GET") // no releases }) - // setup an handler for returning fake prereleases only + // Setup a handler for returning fake prereleases only. muxGoproxy.HandleFunc("/github.com/o/r3/@v/list", func(w http.ResponseWriter, r *http.Request) { testMethod(t, r, "GET") fmt.Fprint(w, "v0.1.0-alpha.0\n") @@ -571,7 +591,7 @@ func Test_gitHubRepository_getLatestPatchRelease(t *testing.T) { clientGoproxy, muxGoproxy, teardownGoproxy := newFakeGoproxy() defer teardownGoproxy() - // setup an handler for returning 4 fake releases + // Setup a handler for returning 4 fake releases. muxGoproxy.HandleFunc("/github.com/o/r1/@v/list", func(w http.ResponseWriter, r *http.Request) { testMethod(t, r, "GET") fmt.Fprint(w, "v0.4.0\n") @@ -654,7 +674,7 @@ func Test_gitHubRepository_getReleaseByTag(t *testing.T) { providerConfig := config.NewProvider("test", "https://github.com/o/r/releases/v0.4.1/path", clusterctlv1.CoreProviderType) - // setup and handler for returning a fake release + // Setup a handler for returning a fake release. mux.HandleFunc("/repos/o/r/releases/tags/foo", func(w http.ResponseWriter, r *http.Request) { testMethod(t, r, "GET") fmt.Fprint(w, `{"id":13, "tag_name": "v0.4.1"}`) @@ -722,14 +742,14 @@ func Test_gitHubRepository_downloadFilesFromRelease(t *testing.T) { providerConfig := config.NewProvider("test", "https://github.com/o/r/releases/v0.4.1/file.yaml", clusterctlv1.CoreProviderType) // tree/main/path not relevant for the test providerConfigWithRedirect := config.NewProvider("test", "https://github.com/o/r-with-redirect/releases/v0.4.1/file.yaml", clusterctlv1.CoreProviderType) // tree/main/path not relevant for the test - // test.NewFakeGitHub an handler for returning a fake release asset + // Setup a handler for returning a fake release asset. mux.HandleFunc("/repos/o/r/releases/assets/1", func(w http.ResponseWriter, r *http.Request) { testMethod(t, r, "GET") w.Header().Set("Content-Type", "application/octet-stream") w.Header().Set("Content-Disposition", "attachment; filename=file.yaml") fmt.Fprint(w, "content") }) - // handler which redirects to a different location + // Setup a handler which redirects to a different location. mux.HandleFunc("/repos/o/r-with-redirect/releases/assets/1", func(w http.ResponseWriter, r *http.Request) { testMethod(t, r, "GET") http.Redirect(w, r, "/api-v3/repos/o/r/releases/assets/1", http.StatusFound)