Merge pull request #1145 from saschagrunert/github-kubepkg

Refactor kubepkg.fetchReleases into github.Releases()

pkg/github/github.go

@@ -43,6 +43,10 @@ type Client interface {
 	ListTags(
 		context.Context, string, string, *github.ListOptions,
 	) ([]*github.RepositoryTag, *github.Response, error)
+
+	ListReleases(
+		context.Context, string, string, *github.ListOptions,
+	) ([]*github.RepositoryRelease, *github.Response, error)
 }
 
 // New creates a new default GitHub client
@@ -56,6 +60,12 @@ func (g *githubClient) ListTags(
 	return g.Client.Repositories.ListTags(ctx, owner, repo, opt)
 }
 
+func (g *githubClient) ListReleases(
+	ctx context.Context, owner, repo string, opt *github.ListOptions,
+) ([]*github.RepositoryRelease, *github.Response, error) {
+	return g.Client.Repositories.ListReleases(ctx, owner, repo, opt)
+}
+
 // SetClient can be used to manually set the internal GitHub client
 func (g *GitHub) SetClient(client Client) {
 	g.client = client
@@ -79,7 +89,7 @@ func (g *GitHub) LatestGitHubTagsPerBranch() (TagsPerBranch, error) {
 		context.Background(), git.DefaultGithubOrg, git.DefaultGithubRepo, nil,
 	)
 	if err != nil {
-		return nil, errors.Wrap(err, "unable to retrieve GitHub releases")
+		return nil, errors.Wrap(err, "unable to retrieve GitHub tags")
 	}
 
 	releases := make(TagsPerBranch)
@@ -117,3 +127,28 @@ func (t TagsPerBranch) addIfNotExisting(branch, tag string) {
 		t[branch] = tag
 	}
 }
+
+// Releases returns a list of GitHub releases for the provided `owner` and
+// `repo`. If `includePrereleases` is `true`, then the resulting slice will
+// also contain pre/drafted releases.
+func (g *GitHub) Releases(owner, repo string, includePrereleases bool) ([]*github.RepositoryRelease, error) {
+	allReleases, _, err := g.client.ListReleases(
+		context.Background(), owner, repo, nil,
+	)
+	if err != nil {
+		return nil, errors.Wrap(err, "unable to retrieve GitHub releases")
+	}
+
+	releases := []*github.RepositoryRelease{}
+	for _, release := range allReleases {
+		if release.GetPrerelease() {
+			if includePrereleases {
+				releases = append(releases, release)
+			}
+		} else {
+			releases = append(releases, release)
+		}
+	}
+
+	return releases, nil
+}

pkg/github/github_test.go

@@ -164,3 +164,86 @@ func TestLatestGitHubTagsPerBranchFailedNonSemverTag(t *testing.T) {
 	require.NotNil(t, err)
 	require.Nil(t, res)
 }
+
+func TestReleasesSuccessEmpty(t *testing.T) {
+	// Given
+	sut, client := newSUT()
+	client.ListReleasesReturns([]*gogithub.RepositoryRelease{}, nil, nil)
+
+	// When
+	res, err := sut.Releases("", "", false)
+
+	// Then
+	require.Nil(t, err)
+	require.Empty(t, res)
+}
+
+func TestReleasesSuccessNoPreReleases(t *testing.T) {
+	// Given
+	var (
+		tag1  = "v1.18.0"
+		tag2  = "v1.17.0"
+		tag3  = "v1.16.0"
+		tag4  = "v1.15.0"
+		aTrue = true
+	)
+	sut, client := newSUT()
+	client.ListReleasesReturns([]*gogithub.RepositoryRelease{
+		{TagName: &tag1},
+		{TagName: &tag2},
+		{TagName: &tag3, Prerelease: &aTrue},
+		{TagName: &tag4},
+	}, nil, nil)
+
+	// When
+	res, err := sut.Releases("", "", false)
+
+	// Then
+	require.Nil(t, err)
+	require.Len(t, res, 3)
+	require.Equal(t, tag1, res[0].GetTagName())
+	require.Equal(t, tag2, res[1].GetTagName())
+	require.Equal(t, tag4, res[2].GetTagName())
+}
+
+func TestReleasesSuccessWithPreReleases(t *testing.T) {
+	// Given
+	var (
+		tag1  = "v1.18.0"
+		tag2  = "v1.17.0"
+		tag3  = "v1.16.0"
+		tag4  = "v1.15.0"
+		aTrue = true
+	)
+	sut, client := newSUT()
+	client.ListReleasesReturns([]*gogithub.RepositoryRelease{
+		{TagName: &tag1},
+		{TagName: &tag2, Prerelease: &aTrue},
+		{TagName: &tag3, Prerelease: &aTrue},
+		{TagName: &tag4},
+	}, nil, nil)
+
+	// When
+	res, err := sut.Releases("", "", true)
+
+	// Then
+	require.Nil(t, err)
+	require.Len(t, res, 4)
+	require.Equal(t, tag1, res[0].GetTagName())
+	require.Equal(t, tag2, res[1].GetTagName())
+	require.Equal(t, tag3, res[2].GetTagName())
+	require.Equal(t, tag4, res[3].GetTagName())
+}
+
+func TestReleasesFailed(t *testing.T) {
+	// Given
+	sut, client := newSUT()
+	client.ListReleasesReturns(nil, nil, errors.New("error"))
+
+	// When
+	res, err := sut.Releases("", "", false)
+
+	// Then
+	require.NotNil(t, err)
+	require.Nil(t, res, nil)
+}

pkg/kubepkg/BUILD.bazel

@@ -10,10 +10,10 @@ go_library(
     visibility = ["//visibility:public"],
     deps = [
         "//pkg/command:go_default_library",
+        "//pkg/github:go_default_library",
         "//pkg/release:go_default_library",
         "//pkg/util:go_default_library",
         "@com_github_blang_semver//:go_default_library",
-        "@com_github_google_go_github_v29//github:go_default_library",
         "@com_github_pkg_errors//:go_default_library",
         "@com_github_sirupsen_logrus//:go_default_library",
     ],

pkg/kubepkg/kubepkg.go

@@ -17,7 +17,6 @@ limitations under the License.
 package kubepkg
 
 import (
-	"context"
 	"fmt"
 	"io/ioutil"
 	"os"
@@ -28,11 +27,11 @@ import (
 	"time"
 
 	"github.com/blang/semver"
-	"github.com/google/go-github/v29/github"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 
 	"k8s.io/release/pkg/command"
+	"k8s.io/release/pkg/github"
 	"k8s.io/release/pkg/release"
 	"k8s.io/release/pkg/util"
 )
@@ -474,7 +473,7 @@ func getCRIToolsVersion(packageDef *PackageDefinition) (string, error) {
 
 	criToolsVersion := fmt.Sprintf("%s.%s.0", criToolsMajor, criToolsMinor)
 
-	releases, err := fetchReleases("kubernetes-sigs", "cri-tools", false)
+	releases, err := github.New().Releases("kubernetes-sigs", "cri-tools", false)
 	if err != nil {
 		return "", err
 	}
@@ -510,28 +509,6 @@ func getCRIToolsVersion(packageDef *PackageDefinition) (string, error) {
 	return criToolsVersion, nil
 }
 
-func fetchReleases(owner, repo string, includePrereleases bool) ([]*github.RepositoryRelease, error) {
-	ghClient := github.NewClient(nil)
-
-	allReleases, _, err := ghClient.Repositories.ListReleases(context.Background(), owner, repo, nil)
-	if err != nil {
-		return nil, err
-	}
-
-	var releases []*github.RepositoryRelease
-	for _, release := range allReleases {
-		if *release.Prerelease {
-			if includePrereleases {
-				releases = append(releases, release)
-			}
-		} else {
-			releases = append(releases, release)
-		}
-	}
-
-	return releases, nil
-}
-
 func getDownloadLinkBase(packageDef *PackageDefinition) (string, error) {
 	if packageDef == nil {
 		return "", errors.New("package definition cannot be nil")