Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

🐛 Ensure all GitHub releases are fetched when searching provider versions #8240

Merged
merged 1 commit into from
Mar 20, 2023
Merged

🐛 Ensure all GitHub releases are fetched when searching provider versions #8240

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 36 additions & 17 deletions cmd/clusterctl/client/repository/repository_github.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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{
Expand All @@ -214,7 +215,7 @@ func NewGitHubRepository(providerConfig config.Provider, configVariablesClient c
componentsPath: componentsPath,
}

// process githubRepositoryOptions
// Process githubRepositoryOptions.
for _, o := range opts {
o(repo)
}
Expand Down Expand Up @@ -278,29 +279,47 @@ 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
})
if retryError != nil {
return nil, retryError
}
versions := []string{}
for _, r := range releases {
for _, r := range allReleases {
r := r // pin
if r.TagName == nil {
continue
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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 {
Expand All @@ -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
}
Expand Down
72 changes: 46 additions & 26 deletions cmd/clusterctl/client/repository/repository_github_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand All @@ -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")
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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", `<https://api.github.com/repositories/12345/releases?page=2>; 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", `<https://api.github.com/repositories/12345/releases?page=3>; 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()
Expand All @@ -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,
},
}
Expand Down Expand Up @@ -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")
Expand All @@ -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")
Expand Down Expand Up @@ -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")
Expand All @@ -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")
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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"}`)
Expand Down Expand Up @@ -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)
Expand Down