Skip to content

Commit

Permalink
Merge pull request #8330 from Nordix/cherry-pick-fetch-all-gh-release…
Browse files Browse the repository at this point in the history 
…s/mohammed

[release-1.3] 🐛 Ensure all GitHub releases are fetched when searching provider versions
  • Loading branch information
@k8s-ci-robot
k8s-ci-robot committed Mar 22, 2023
2 parents 7144f14 + aa9cf5a commit 2ed53ed
Show file tree
Hide file tree
Showing 2 changed files with 80 additions and 41 deletions.
51 changes: 35 additions & 16 deletions cmd/clusterctl/client/repository/repository_github.go
View file
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,7 +400,7 @@ func (g *gitHubRepository) downloadFilesFromRelease(release *github.RepositoryRe
reader, redirect, downloadReleaseError = client.Repositories.DownloadReleaseAsset(context.TODO(), 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
}
Expand Down
70 changes: 45 additions & 25 deletions cmd/clusterctl/client/repository/repository_github_test.go
View file
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 @@ -721,7 +741,7 @@ 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

// 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

0 comments on commit 2ed53ed

Please sign in to comment.