Skip to content

Commit

Permalink
return error if gh client returns 404 to bubble up the workaround
Browse files Browse the repository at this point in the history
  • Loading branch information
chrischdi committed Mar 23, 2023
1 parent 8ecf669 commit 9a1b6a8
Show file tree
Hide file tree
Showing 3 changed files with 52 additions and 8 deletions.
18 changes: 17 additions & 1 deletion cmd/clusterctl/client/repository/repository_github.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ const (
)

var (
errNotFound = errors.New("404 Not Found")

// Caches used to limit the number of GitHub API calls.

cacheVersions = map[string][]string{}
Expand Down Expand Up @@ -156,6 +158,11 @@ func (g *gitHubRepository) ComponentsPath() string {
func (g *gitHubRepository) GetFile(version, path string) ([]byte, error) {
release, err := g.getReleaseByTag(version)
if err != nil {
if errors.Is(err, errNotFound) {
// If it was ErrNotFound, then there is no release yet for the resolved tag.
// Ref: https://github.com/kubernetes-sigs/cluster-api/issues/7889
return nil, errors.Wrap(err, "release does not exist yet, consider setting \"GOPROXY=off\" as workaround")
}
return nil, errors.Wrapf(err, "failed to get GitHub release %s", version)
}

Expand Down Expand Up @@ -227,7 +234,7 @@ func NewGitHubRepository(providerConfig config.Provider, configVariablesClient c
if defaultVersion == githubLatestReleaseLabel {
repo.defaultVersion, err = latestContractRelease(repo, clusterv1.GroupVersion.Version)
if err != nil {
return nil, errors.Wrap(err, "failed to get GitHub latest version")
return nil, errors.Wrap(err, "failed to get latest release")
}
}

Expand Down Expand Up @@ -351,6 +358,10 @@ 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 not found
if errors.Is(retryError, errNotFound) {
return false, retryError
}
// Return immediately if we are rate limited.
if _, ok := getReleasesErr.(*github.RateLimitError); ok {
return false, retryError
Expand Down Expand Up @@ -437,5 +448,10 @@ func (g *gitHubRepository) handleGithubErr(err error, message string, args ...in
if _, ok := err.(*github.RateLimitError); ok {
return errors.New("rate limit for github api has been reached. Please wait one hour or get a personal API token and assign it to the GITHUB_TOKEN environment variable")
}
if ghErr, ok := err.(*github.ErrorResponse); ok {
if ghErr.Response.StatusCode == http.StatusNotFound {
return errNotFound
}
}
return errors.Wrapf(err, message, args...)
}
33 changes: 28 additions & 5 deletions cmd/clusterctl/client/repository/repository_github_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -441,6 +441,15 @@ func Test_gitHubRepository_getLatestContractRelease(t *testing.T) {
fmt.Fprint(w, "v0.3.1\n")
})

// setup an handler for returning 4 fake releases but no actual tagged release
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")
fmt.Fprint(w, "v0.4.0\n")
fmt.Fprint(w, "v0.3.2\n")
fmt.Fprint(w, "v0.3.1\n")
})

configVariablesClient := test.NewFakeVariableClient()

type field struct {
Expand Down Expand Up @@ -480,6 +489,15 @@ func Test_gitHubRepository_getLatestContractRelease(t *testing.T) {
contract: "foo",
wantErr: false,
},
{
name: "Return 404 if there is no release for the tag",
field: field{
providerConfig: config.NewProvider("test", "https://github.com/o/r2/releases/v0.99.0/path", clusterctlv1.CoreProviderType),
},
want: "0.99.0",
contract: "v1alpha4",
wantErr: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
Expand Down Expand Up @@ -514,6 +532,11 @@ func Test_gitHubRepository_getLatestRelease(t *testing.T) {
fmt.Fprint(w, "v0.4.3-alpha\n") // prerelease
fmt.Fprint(w, "foo\n") // no semantic version tag
})
// And also expose a release for them
muxGoproxy.HandleFunc("/api.github.com/repos/o/r1/releases/tags/v0.4.2", func(w http.ResponseWriter, r *http.Request) {
testMethod(t, r, "GET")
fmt.Fprint(w, "{}\n")
})

// Setup a handler for returning no releases.
muxGoproxy.HandleFunc("/github.com/o/r2/@v/list", func(w http.ResponseWriter, r *http.Request) {
Expand Down Expand Up @@ -543,7 +566,7 @@ func Test_gitHubRepository_getLatestRelease(t *testing.T) {
{
name: "Get latest release, ignores pre-release version",
field: field{
providerConfig: config.NewProvider("test", "https://github.com/o/r1/releases/latest/path", clusterctlv1.CoreProviderType),
providerConfig: config.NewProvider("test", "https://github.com/o/r1/releases/v0.4.2/path", clusterctlv1.CoreProviderType),
},
want: "v0.4.2",
wantErr: false,
Expand All @@ -559,7 +582,7 @@ func Test_gitHubRepository_getLatestRelease(t *testing.T) {
{
name: "Falls back to latest prerelease when no official release present",
field: field{
providerConfig: config.NewProvider("test", "https://github.com/o/r3/releases/latest/path", clusterctlv1.CoreProviderType),
providerConfig: config.NewProvider("test", "https://github.com/o/r3/releases/v0.1.0-alpha.2/path", clusterctlv1.CoreProviderType),
},
want: "v0.1.0-alpha.2",
wantErr: false,
Expand Down Expand Up @@ -619,7 +642,7 @@ func Test_gitHubRepository_getLatestPatchRelease(t *testing.T) {
{
name: "Get latest patch release, no Major/Minor specified",
field: field{
providerConfig: config.NewProvider("test", "https://github.com/o/r1/releases/latest/path", clusterctlv1.CoreProviderType),
providerConfig: config.NewProvider("test", "https://github.com/o/r1/releases/v1.3.2/path", clusterctlv1.CoreProviderType),
},
minor: nil,
major: nil,
Expand All @@ -629,7 +652,7 @@ func Test_gitHubRepository_getLatestPatchRelease(t *testing.T) {
{
name: "Get latest patch release, for Major 0 and Minor 3",
field: field{
providerConfig: config.NewProvider("test", "https://github.com/o/r1/releases/latest/path", clusterctlv1.CoreProviderType),
providerConfig: config.NewProvider("test", "https://github.com/o/r1/releases/v0.3.2/path", clusterctlv1.CoreProviderType),
},
major: &major0,
minor: &minor3,
Expand All @@ -639,7 +662,7 @@ func Test_gitHubRepository_getLatestPatchRelease(t *testing.T) {
{
name: "Get latest patch release, for Major 0 and Minor 4",
field: field{
providerConfig: config.NewProvider("test", "https://github.com/o/r1/releases/latest/path", clusterctlv1.CoreProviderType),
providerConfig: config.NewProvider("test", "https://github.com/o/r1/releases/v0.4.0/path", clusterctlv1.CoreProviderType),
},
major: &major0,
minor: &minor4,
Expand Down
9 changes: 7 additions & 2 deletions cmd/clusterctl/client/repository/repository_versions.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,16 @@ func latestContractRelease(repo Repository, contract string) (string, error) {
}
// Attempt to check if the latest release satisfies the API Contract
// This is a best-effort attempt to find the latest release for an older API contract if it's not the latest release.
// If an error occurs, we just return the latest release.
file, err := repo.GetFile(latest, metadataFile)
// If an error occurs, we just return the latest release.
if err != nil {
if errors.Is(err, errNotFound) {
// If it was ErrNotFound, then there is no release yet for the resolved tag.
// Ref: https://github.com/kubernetes-sigs/cluster-api/issues/7889
return "", err
}
// if we can't get the metadata file from the release, we return latest.
return latest, nil //nolint:nilerr
return latest, nil
}
latestMetadata := &clusterctlv1.Metadata{}
codecFactory := serializer.NewCodecFactory(scheme.Scheme)
Expand Down

0 comments on commit 9a1b6a8

Please sign in to comment.