Merge pull request #3580 from embik/fix-3565

Refactor `pkg/announce` package into sub-packages and interface pattern

Author: k8s-ci-robot

cmd/publish-release/cmd/github.go

@@ -31,7 +31,8 @@ import (
 	"github.com/sirupsen/logrus"
 	"github.com/spf13/cobra"
 	"google.golang.org/api/option"
-	"k8s.io/release/pkg/announce"
+	"k8s.io/release/pkg/announce/github"
+	"k8s.io/release/pkg/announce/sbom"
 )
 
 // releaseNotesCmd represents the subcommand for `krel release-notes`
@@ -154,7 +155,7 @@ func init() {
 	githubPageCmd.PersistentFlags().StringVar(
 		&ghPageOpts.sbomFormat,
 		"sbom-format",
-		string(announce.FormatJSON),
+		string(sbom.FormatJSON),
 		"format to use for the SBOM [json|tag-value]",
 	)
 	githubPageCmd.PersistentFlags().StringVar(
@@ -183,8 +184,8 @@ func init() {
 	rootCmd.AddCommand(githubPageCmd)
 }
 
-func getAssetsFromStrings(assetStrings []string) ([]announce.Asset, error) {
-	r := []announce.Asset{}
+func getAssetsFromStrings(assetStrings []string) ([]sbom.Asset, error) {
+	r := []sbom.Asset{}
 	var isBucket bool
 	for _, s := range assetStrings {
 		isBucket = false
@@ -205,7 +206,7 @@ func getAssetsFromStrings(assetStrings []string) ([]announce.Asset, error) {
 			}
 			parts[0] = path
 		}
-		r = append(r, announce.Asset{
+		r = append(r, sbom.Asset{
 			Path:     filepath.Base(parts[0]),
 			ReadFrom: parts[0],
 			Label:    l,
@@ -274,24 +275,24 @@ func runGithubPage(opts *githubPageCmdLineOptions) (err error) {
 	if err != nil {
 		return fmt.Errorf("getting assets: %w", err)
 	}
-	sbom := ""
+	sbomStr := ""
 	if opts.sbom {
 		// Generate the assets file
-		sbom, err = announce.GenerateReleaseSBOM(&announce.SBOMOptions{
+		sbomStr, err = sbom.NewSBOM(&sbom.Options{
 			ReleaseName:   opts.name,
 			Repo:          opts.repo,
 			RepoDirectory: opts.repoPath,
 			Assets:        assets,
 			Tag:           commandLineOpts.tag,
-			Format:        announce.SBOMFormat(opts.sbomFormat),
-		})
+			Format:        sbom.SBOMFormat(opts.sbomFormat),
+		}).Generate()
 		if err != nil {
 			return fmt.Errorf("generating sbom: %w", err)
 		}
-		opts.assets = append(opts.assets, sbom+":SPDX Software Bill of Materials (SBOM)")
+		opts.assets = append(opts.assets, sbomStr+":SPDX Software Bill of Materials (SBOM)")
 		// Delete the temporary sbom  when we're done
 		if commandLineOpts.nomock {
-			defer os.Remove(sbom)
+			defer os.Remove(sbomStr)
 		}
 	}
 
@@ -301,10 +302,10 @@ func runGithubPage(opts *githubPageCmdLineOptions) (err error) {
 	}
 
 	// add sbom to the path to upload
-	newAssets[len(assets)] = sbom
+	newAssets[len(assets)] = sbomStr
 
 	// Build the release page options
-	announceOpts := announce.GitHubPageOptions{
+	ghOpts := github.Options{
 		AssetFiles:            newAssets,
 		Tag:                   commandLineOpts.tag,
 		NoMock:                commandLineOpts.nomock,
@@ -315,25 +316,25 @@ func runGithubPage(opts *githubPageCmdLineOptions) (err error) {
 	}
 
 	// Assign the repository data
-	if err := announceOpts.SetRepository(opts.repo); err != nil {
+	if err := ghOpts.SetRepository(opts.repo); err != nil {
 		return fmt.Errorf("assigning the repository slug: %w", err)
 	}
 
 	// Assign the substitutions
-	if err := announceOpts.ParseSubstitutions(opts.substitutions); err != nil {
+	if err := ghOpts.ParseSubstitutions(opts.substitutions); err != nil {
 		return fmt.Errorf("parsing template substitutions: %w", err)
 	}
 
 	// Read the csutom template data
-	if err := announceOpts.ReadTemplate(opts.template); err != nil {
+	if err := ghOpts.ReadTemplate(opts.template); err != nil {
 		return fmt.Errorf("reading the template file: %w", err)
 	}
 
 	// Validate the options
-	if err := announceOpts.Validate(); err != nil {
+	if err := ghOpts.Validate(); err != nil {
 		return fmt.Errorf("validating options: %w", err)
 	}
 
 	// Run the update process
-	return announce.UpdateGitHubPage(&announceOpts)
+	return github.NewGitHub(&ghOpts).UpdateGitHubPage()
 }

pkg/anago/anagofakes/fake_release_impl.go

@@ -25,6 +25,7 @@ import (
 	"github.com/sirupsen/logrus"
 
 	"k8s.io/release/pkg/announce"
+	"k8s.io/release/pkg/announce/github"
 	"k8s.io/release/pkg/build"
 	"k8s.io/release/pkg/gcp/gcb"
 	"k8s.io/release/pkg/release"
@@ -147,7 +148,7 @@ type releaseImpl interface {
 	CreateAnnouncement(
 		options *announce.Options,
 	) error
-	UpdateGitHubPage(options *announce.GitHubPageOptions) error
+	UpdateGitHubPage(options *github.Options) error
 	PushTags(pusher *release.GitObjectPusher, tagList []string) error
 	PushBranches(pusher *release.GitObjectPusher, branchList []string) error
 	PushMainBranch(pusher *release.GitObjectPusher) error
@@ -260,16 +261,16 @@ func (d *DefaultRelease) InitLogFile() error {
 
 func (d *defaultReleaseImpl) CreateAnnouncement(options *announce.Options) error {
 	// Create the announcement
-	return announce.CreateForRelease(options)
+	return announce.NewAnnounce(options).CreateForRelease()
 }
 
 func (d *defaultReleaseImpl) ArchiveRelease(options *release.ArchiverOptions) error {
 	// Create a new release archiver
 	return release.NewArchiver(options).ArchiveRelease()
 }
 
-func (d *defaultReleaseImpl) UpdateGitHubPage(options *announce.GitHubPageOptions) error {
-	return announce.UpdateGitHubPage(options)
+func (d *defaultReleaseImpl) UpdateGitHubPage(options *github.Options) error {
+	return github.NewGitHub(options).UpdateGitHubPage()
 }
 
 func (d *defaultReleaseImpl) PushTags(
@@ -576,7 +577,7 @@ func (d *DefaultRelease) UpdateGitHubPage() error {
 	)
 
 	// Build the options set for the GitHub page
-	ghPageOpts := &announce.GitHubPageOptions{
+	ghPageOpts := &github.Options{
 		Tag:                   d.state.versions.Prime(),
 		NoMock:                d.options.NoMock,
 		UpdateIfReleaseExists: true,

pkg/anago/release.go

@@ -20,14 +20,8 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
-	"regexp"
-	"strings"
 
 	"github.com/sirupsen/logrus"
-	"sigs.k8s.io/release-utils/command"
-	"sigs.k8s.io/release-utils/util"
-
-	"k8s.io/release/pkg/kubecross"
 )
 
 const (
@@ -69,16 +63,29 @@ Published by your
 Managers</a>.
 `
 
-func CreateForBranch(opts *Options) error {
+type Announce struct {
+	options *Options
+	impl
+}
+
+// NewAnnounce returns a new Announce instance.
+func NewAnnounce(opts *Options) *Announce {
+	return &Announce{
+		impl:    &defaultImpl{},
+		options: opts,
+	}
+}
+
+func (a *Announce) CreateForBranch() error {
 	logrus.Infof(
 		"Creating %s branch announcement in %s",
-		opts.branch, opts.workDir,
+		a.options.branch, a.options.workDir,
 	)
 
-	if err := create(
-		opts.workDir,
-		fmt.Sprintf("Kubernetes %s branch has been created", opts.branch),
-		fmt.Sprintf(branchAnnouncement, opts.branch),
+	if err := a.impl.create(
+		a.options.workDir,
+		fmt.Sprintf("Kubernetes %s branch has been created", a.options.branch),
+		fmt.Sprintf(branchAnnouncement, a.options.branch),
 	); err != nil {
 		return fmt.Errorf("creating branch announcement: %w", err)
 	}
@@ -90,39 +97,39 @@ func CreateForBranch(opts *Options) error {
 	return nil
 }
 
-func CreateForRelease(opts *Options) error {
-	logrus.Infof("Creating %s announcement in %s", opts.tag, opts.workDir)
+func (a *Announce) CreateForRelease() error {
+	logrus.Infof("Creating %s announcement in %s", a.options.tag, a.options.workDir)
 
 	changelog := ""
 
 	// Read the changelog from the specified file if we got one
-	if opts.changelogFile != "" {
-		changelogData, err := os.ReadFile(opts.changelogFile)
+	if a.options.changelogFile != "" {
+		changelogData, err := os.ReadFile(a.options.changelogFile)
 		if err != nil {
 			return fmt.Errorf("reading changelog html file: %w", err)
 		}
 		changelog = string(changelogData)
 	}
 
 	// ... unless it is overridden by passing the HTML directly
-	if opts.changelogHTML != "" {
-		changelog = opts.changelogHTML
+	if a.options.changelogHTML != "" {
+		changelog = a.options.changelogHTML
 	}
 
-	logrus.Infof("Trying to get the Go version used to build %s...", opts.tag)
-	goVersion, err := getGoVersion(opts.tag)
+	logrus.Infof("Trying to get the Go version used to build %s...", a.options.tag)
+	goVersion, err := a.impl.getGoVersion(a.options.tag)
 	if err != nil {
 		return err
 	}
 	logrus.Infof("Found the following Go version: %s", goVersion)
 
-	if err := create(
-		opts.workDir,
-		fmt.Sprintf("Kubernetes %s is live!", opts.tag),
+	if err := a.impl.create(
+		a.options.workDir,
+		fmt.Sprintf("Kubernetes %s is live!", a.options.tag),
 		fmt.Sprintf(releaseAnnouncement,
-			opts.tag, goVersion, opts.changelogPath,
-			filepath.Base(opts.changelogPath), opts.tag, changelog,
-			opts.changelogPath, filepath.Base(opts.changelogPath), opts.tag,
+			a.options.tag, goVersion, a.options.changelogPath,
+			filepath.Base(a.options.changelogPath), a.options.tag, changelog,
+			a.options.changelogPath, filepath.Base(a.options.changelogPath), a.options.tag,
 		),
 	); err != nil {
 		return fmt.Errorf("creating release announcement: %w", err)
@@ -131,68 +138,3 @@ func CreateForRelease(opts *Options) error {
 	logrus.Infof("Release announcement created")
 	return nil
 }
-
-func create(workDir, subject, message string) error {
-	subjectFile := filepath.Join(workDir, subjectFile)
-	//nolint:gosec // TODO(gosec): G306: Expect WriteFile permissions to be
-	// 0600 or less
-	if err := os.WriteFile(
-		subjectFile, []byte(subject), 0o755,
-	); err != nil {
-		return fmt.Errorf(
-			"writing subject to file %s: %w",
-			subjectFile,
-			err,
-		)
-	}
-	logrus.Debugf("Wrote file %s", subjectFile)
-
-	announcementFile := filepath.Join(workDir, announcementFile)
-	//nolint:gosec // TODO(gosec): G306: Expect WriteFile permissions to be
-	// 0600 or less
-	if err := os.WriteFile(
-		announcementFile, []byte(message), 0o755,
-	); err != nil {
-		return fmt.Errorf(
-			"writing announcement to file %s: %w",
-			announcementFile,
-			err,
-		)
-	}
-	logrus.Debugf("Wrote file %s", announcementFile)
-
-	return nil
-}
-
-// getGoVersion runs kube-cross container and go version inside it.
-// We're running kube-cross container because it's not guaranteed that
-// k8s-cloud-builder container will be running the same Go version as
-// the kube-cross container used to build the release.
-func getGoVersion(tag string) (string, error) {
-	semver, err := util.TagStringToSemver(tag)
-	if err != nil {
-		return "", fmt.Errorf("parse version tag: %w", err)
-	}
-
-	branch := fmt.Sprintf("release-%d.%d", semver.Major, semver.Minor)
-	kc := kubecross.New()
-	kubecrossVer, err := kc.ForBranch(branch)
-	if err != nil {
-		kubecrossVer, err = kc.Latest()
-		if err != nil {
-			return "", fmt.Errorf("get kubecross version: %w", err)
-		}
-	}
-
-	kubecrossImg := fmt.Sprintf("registry.k8s.io/build-image/kube-cross:%s", kubecrossVer)
-
-	res, err := command.New(
-		"docker", "run", "--rm", kubecrossImg, "go", "version",
-	).RunSilentSuccessOutput()
-	if err != nil {
-		return "", fmt.Errorf("get go version: %w", err)
-	}
-
-	versionRegex := regexp.MustCompile(`^?(\d+)(\.\d+)?(\.\d+)`)
-	return versionRegex.FindString(strings.TrimSpace(res.OutputTrimNL())), nil
-}