Refactor CreateDownloadsTable to use fetchMetadata

Re-use the new `fetchMetadata` function on markdown downloads table
creation to reduce code duplication.

This also makes `CreateDownloadsTable` public, which will be needed for
creating the downloads table on remotely fetched release notes (new
final minor release).

Signed-off-by: Sascha Grunert <sgrunert@suse.com>

cmd/release-notes/BUILD.bazel

@@ -11,6 +11,7 @@ go_library(
         "//pkg/notes:go_default_library",
         "//pkg/notes/document:go_default_library",
         "//pkg/notes/options:go_default_library",
+        "//pkg/release:go_default_library",
         "//pkg/util:go_default_library",
         "@com_github_pkg_errors//:go_default_library",
         "@com_github_sirupsen_logrus//:go_default_library",

cmd/release-notes/main.go

@@ -34,6 +34,7 @@ import (
 	"k8s.io/release/pkg/notes"
 	"k8s.io/release/pkg/notes/document"
 	"k8s.io/release/pkg/notes/options"
+	"k8s.io/release/pkg/release"
 	"k8s.io/release/pkg/util"
 )
 
@@ -178,7 +179,7 @@ func init() {
 	cmd.PersistentFlags().StringVar(
 		&opts.ReleaseBucket,
 		"release-bucket",
-		util.EnvDefault("RELEASE_BUCKET", "kubernetes-release"),
+		util.EnvDefault("RELEASE_BUCKET", release.ProductionBucket),
 		"Specify gs bucket to point to in generated notes",
 	)
 

pkg/notes/document/BUILD.bazel

@@ -7,6 +7,7 @@ go_library(
     visibility = ["//visibility:public"],
     deps = [
         "//pkg/notes:go_default_library",
+        "//pkg/release:go_default_library",
         "@com_github_pkg_errors//:go_default_library",
     ],
 )
@@ -18,6 +19,7 @@ go_test(
     embed = [":go_default_library"],
     deps = [
         "//pkg/notes/internal:go_default_library",
+        "//pkg/release:go_default_library",
         "@com_github_kr_pretty//:go_default_library",
         "@com_github_stretchr_testify//require:go_default_library",
         "@io_bazel_rules_go//go/tools/bazel:go_default_library",

pkg/notes/document/document.go

@@ -28,6 +28,7 @@ import (
 
 	"github.com/pkg/errors"
 	"k8s.io/release/pkg/notes"
+	"k8s.io/release/pkg/release"
 )
 
 // Document represents the underlying structure of a release notes document.
@@ -45,18 +46,18 @@ type FileMetadata struct {
 	// Files containing source code.
 	Source []File
 
-	// Client binaries.
+	// Client binaries
 	Client []File
 
-	// Server binaries.
+	// Server binaries
 	Server []File
 
-	// TODO: What is this?
+	// Node binaries
 	Node []File
 }
 
-// FetchMetadata generates file metadata from files in `dir`
-func (f *FileMetadata) FetchMetadata(dir, urlPrefix, tag string) (*FileMetadata, error) {
+// fetchMetadata generates file metadata from files in `dir`
+func fetchMetadata(dir, urlPrefix, tag string) (*FileMetadata, error) {
 	if dir == "" {
 		return nil, nil
 	}
@@ -74,6 +75,7 @@ func (f *FileMetadata) FetchMetadata(dir, urlPrefix, tag string) (*FileMetadata,
 		&fm.Server: {"kubernetes-server*.tar.gz"},
 		&fm.Node:   {"kubernetes-node*.tar.gz"},
 	}
+	f := &FileMetadata{}
 	for fileType, patterns := range m {
 		fileMetadata, err := f.newFile(dir, patterns, urlPrefix, tag)
 		if err != nil {
@@ -204,13 +206,8 @@ func CreateDocument(releaseNotes notes.ReleaseNotes, history notes.ReleaseNotesH
 
 // RenderMarkdownTemplate renders a document using the Go template in `goTemplate`.
 func (d *Document) RenderMarkdownTemplate(bucket, fileDir, goTemplate string) (string, error) {
-	urlPrefix := fmt.Sprintf("https://storage.googleapis.com/%s/release", bucket)
-	if bucket == "kubernetes-release" {
-		urlPrefix = "https://dl.k8s.io"
-	}
-
-	fm := new(FileMetadata)
-	fileMetadata, err := fm.FetchMetadata(fileDir, urlPrefix, d.CurrentRevision)
+	urlPrefix := release.URLPrefixForBucket(bucket)
+	fileMetadata, err := fetchMetadata(fileDir, urlPrefix, d.CurrentRevision)
 	if err != nil {
 		return "", errors.Wrap(err, "fetching downloads metadata")
 	}
@@ -234,7 +231,7 @@ func (d *Document) RenderMarkdownTemplate(bucket, fileDir, goTemplate string) (s
 // supplied io.Writer in markdown format.
 func (d *Document) RenderMarkdown(bucket, tars, prevTag, newTag string) (string, error) {
 	o := &strings.Builder{}
-	if err := createDownloadsTable(o, bucket, tars, prevTag, newTag); err != nil {
+	if err := CreateDownloadsTable(o, bucket, tars, prevTag, newTag); err != nil {
 		return "", err
 	}
 
@@ -326,70 +323,46 @@ func sortKinds(notesByKind NotesByKind) []Kind {
 	return res
 }
 
-// createDownloadsTable creates the markdown table with the links to the tarballs.
+// CreateDownloadsTable creates the markdown table with the links to the tarballs.
 // The function does nothing if the `tars` variable is empty.
-func createDownloadsTable(w io.Writer, bucket, tars, prevTag, newTag string) error {
-	// Do not add the table if not explicitly requested
-	if tars == "" {
-		return nil
-	}
+func CreateDownloadsTable(w io.Writer, bucket, tars, prevTag, newTag string) error {
 	if prevTag == "" || newTag == "" {
 		return errors.New("release tags not specified")
 	}
 
+	urlPrefix := release.URLPrefixForBucket(bucket)
+	fileMetadata, err := fetchMetadata(tars, urlPrefix, newTag)
+
+	if err != nil {
+		return errors.Wrap(err, "fetching downloads metadata")
+	}
+
 	fmt.Fprintf(w, "# %s\n\n", newTag)
 	fmt.Fprintf(w, "[Documentation](https://docs.k8s.io)\n\n")
 
 	fmt.Fprintf(w, "## Downloads for %s\n\n", newTag)
 
-	urlPrefix := fmt.Sprintf("https://storage.googleapis.com/%s/release", bucket)
-	if bucket == "kubernetes-release" {
-		urlPrefix = "https://dl.k8s.io"
+	// Sort the files by their headers
+	headers := [4]string{
+		"", "Client Binaries", "Server Binaries", "Node Binaries",
+	}
+	files := map[string][]File{
+		headers[0]: fileMetadata.Source,
+		headers[1]: fileMetadata.Client,
+		headers[2]: fileMetadata.Server,
+		headers[3]: fileMetadata.Node,
 	}
 
-	for _, item := range []struct {
-		heading  string
-		patterns []string
-	}{
-		{"", []string{"kubernetes.tar.gz", "kubernetes-src.tar.gz"}},
-		{"Client Binaries", []string{"kubernetes-client*.tar.gz"}},
-		{"Server Binaries", []string{"kubernetes-server*.tar.gz"}},
-		{"Node Binaries", []string{"kubernetes-node*.tar.gz"}},
-	} {
-		if item.heading != "" {
-			fmt.Fprintf(w, "### %s\n\n", item.heading)
+	for _, header := range headers {
+		if header != "" {
+			fmt.Fprintf(w, "### %s\n\n", header)
 		}
 		fmt.Fprintln(w, "filename | sha512 hash")
 		fmt.Fprintln(w, "-------- | -----------")
 
-		for _, pattern := range item.patterns {
-			pattern := filepath.Join(tars, pattern)
-
-			matches, err := filepath.Glob(pattern)
-			if err != nil {
-				return err
-			}
-
-			for _, file := range matches {
-				f, err := os.Open(file)
-				if err != nil {
-					return err
-				}
-				defer f.Close()
-
-				h := sha512.New()
-				if _, err := io.Copy(h, f); err != nil {
-					return err
-				}
-
-				fileName := filepath.Base(file)
-				fmt.Fprintf(w,
-					"[%s](%s/%s/%s) | `%x`\n",
-					fileName, urlPrefix, newTag, fileName, h.Sum(nil),
-				)
-			}
+		for _, f := range files[header] {
+			fmt.Fprintf(w, "[%s](%s) | `%s`\n", f.Name, f.URL, f.Checksum)
 		}
-
 		fmt.Fprintln(w, "")
 	}
 

pkg/notes/document/document_test.go

@@ -26,7 +26,9 @@ import (
 	"github.com/bazelbuild/rules_go/go/tools/bazel"
 	"github.com/kr/pretty"
 	"github.com/stretchr/testify/require"
+
 	"k8s.io/release/pkg/notes/internal"
+	"k8s.io/release/pkg/release"
 )
 
 func TestFileMetadata(t *testing.T) {
@@ -65,8 +67,7 @@ func TestFileMetadata(t *testing.T) {
 		))
 	}
 
-	fm := FileMetadata{}
-	metadata, err := fm.FetchMetadata(dir, "http://test.com", "test-release")
+	metadata, err := fetchMetadata(dir, "http://test.com", "test-release")
 	require.Nil(t, err)
 
 	expected := &FileMetadata{
@@ -168,7 +169,7 @@ func TestRenderMarkdownTemplate(t *testing.T) {
 	require.Nil(t, err, "Reading golden file %q", goldenFile)
 	expected := string(b)
 
-	got, err := doc.RenderMarkdownTemplate("kubernetes-release", dir, internal.DefaultReleaseNotesTemplate)
+	got, err := doc.RenderMarkdownTemplate(release.ProductionBucket, dir, internal.DefaultReleaseNotesTemplate)
 	require.Nil(t, err, "Rendering document")
 
 	// Then
@@ -213,12 +214,12 @@ func TestCreateDownloadsTable(t *testing.T) {
 
 	// When
 	output := &strings.Builder{}
-	require.Nil(t, createDownloadsTable(
-		output, "kubernetes-release", dir, "v1.16.0", "v1.16.1",
+	require.Nil(t, CreateDownloadsTable(
+		output, release.ProductionBucket, dir, "v1.16.0", "v1.16.1",
 	))
 
 	// Then
-	require.Equal(t, output.String(), `# v1.16.1
+	require.Equal(t, `# v1.16.1
 
 [Documentation](https://docs.k8s.io)
 
@@ -267,7 +268,7 @@ filename | sha512 hash
 
 ## Changelog since v1.16.0
 
-`)
+`, output.String())
 }
 
 func TestSortKinds(t *testing.T) {

pkg/release/release.go

@@ -73,6 +73,12 @@ const (
 	// WindowsGCSPath is the directory where Windoes GCE scripts are staged
 	// before push to GCS.
 	WindowsGCSPath = "gcs-stage/extra/gce/windows"
+
+	// ProductionBucket is the default bucket for Kubernetes releases
+	ProductionBucket = "kubernetes-release"
+
+	// ProductionBucketURL is the url for the ProductionBucket
+	ProductionBucketURL = "https://dl.k8s.io"
 )
 
 var (
@@ -257,3 +263,12 @@ func GetKubecrossVersion(branches ...string) (string, error) {
 
 	return "", errors.New("kube-cross version should not be empty; cannot continue")
 }
+
+// URLPrefixForBucket returns the URL prefix for the provided bucket string
+func URLPrefixForBucket(bucket string) string {
+	urlPrefix := fmt.Sprintf("https://storage.googleapis.com/%s/release", bucket)
+	if bucket == ProductionBucket {
+		urlPrefix = ProductionBucketURL
+	}
+	return urlPrefix
+}