Always use a clean checkout for clones during release-notes

This avoids a large category of failure modes where an existing on-disk
repository is in an unexpected state. We also delete the repo afterwards
to avoid multiple runs filling up the disk.

This change therefore removes configuration flags for the fork
locations.

Also included:

* Set the upstream when pushing a new branch
* Use HTTP transport for git instead of SSH

  As we are now using the GitHub token for authentication, we need the
  HTTP URL and not the SSH one.

* Remove check for branch existing on remote

  The existing code does not work for a number of reasons:

  - It only checks the default remote, not the user's fork.
  - Any error returned would be treated as a success, not just a "not
    found".

  Even without this check there is no risk, as GitHub will simply reject
  the pushing of a new branch without `--force`, which we do not use.

* Use existing branch if it already exists

  The `-B` flag to `git checkout` will create the branch if it does not
  exist (existing beaviour) but will also use it if it does exist.

Author: JamesLaverack

cmd/krel/cmd/release_notes.go

@@ -87,17 +87,15 @@ permissions to your fork of k/sig-release and k-sigs/release-notes.`,
 }
 
 type releaseNotesOptions struct {
-	tag                    string
-	draftOrg               string
-	draftRepo              string
-	createDraftPR          bool
-	createWebsitePR        bool
-	outputDir              string
-	sigreleaseForkPath     string
-	kubernetessigsForkPath string
-	Format                 string
-	websiteOrg             string
-	websiteRepo            string
+	tag             string
+	draftOrg        string
+	draftRepo       string
+	createDraftPR   bool
+	createWebsitePR bool
+	outputDir       string
+	Format          string
+	websiteOrg      string
+	websiteRepo     string
 }
 
 type releaseNotesResult struct {
@@ -173,20 +171,6 @@ func init() {
 		"The format for notes output (options: markdown, json)",
 	)
 
-	releaseNotesCmd.PersistentFlags().StringVar(
-		&releaseNotesOpts.kubernetessigsForkPath,
-		"kubernetes-sigs-fork-path",
-		filepath.Join(os.TempDir(), "k8s-sigs"),
-		"fork kubernetes-sigs/release-notes and output a copy of the json release notes to this directory",
-	)
-
-	releaseNotesCmd.PersistentFlags().StringVar(
-		&releaseNotesOpts.sigreleaseForkPath,
-		"sigrelease-fork-path",
-		filepath.Join(os.TempDir(), "k8s-sigrelease"),
-		"fork k/sig-release and output a copy of the release notes draft to this directory",
-	)
-
 	rootCmd.AddCommand(releaseNotesCmd)
 }
 
@@ -328,7 +312,7 @@ func validateWebsitePROptions() error {
 }
 
 // createDraftPR pushes the release notes draft to the users fork
-func createDraftPR(tag string, result *releaseNotesResult) error {
+func createDraftPR(tag string, result *releaseNotesResult) (err error) {
 	s, err := util.TagStringToSemver(tag)
 	if err != nil {
 		return errors.Wrapf(err, "no valid tag: %v", tag)
@@ -338,14 +322,18 @@ func createDraftPR(tag string, result *releaseNotesResult) error {
 
 	// Prepare the fork of k/sig-release
 	sigReleaseRepo, err := prepareFork(
-		branchname, releaseNotesOpts.sigreleaseForkPath,
+		branchname,
 		git.DefaultGithubOrg, git.DefaultGithubReleaseRepo,
 		releaseNotesOpts.draftOrg, releaseNotesOpts.draftRepo,
 	)
 	if err != nil {
 		return errors.Wrap(err, "preparing local fork of kubernetes/sig-release")
 	}
 
+	defer func() {
+		err = sigReleaseRepo.Cleanup()
+	}()
+
 	// generate the notes
 	targetdir := filepath.Join(sigReleaseRepo.Dir(), "releases", fmt.Sprintf("release-%d.%d", s.Major, s.Minor))
 	logrus.Debugf("release notes markdown will be written to %v", targetdir)
@@ -375,19 +363,19 @@ func createDraftPR(tag string, result *releaseNotesResult) error {
 }
 
 // prepareFork Prepare a branch a repo
-func prepareFork(branchName, repoPath, upstreamOrg, upstreamRepo, myOrg, myRepo string) (repo *git.Repo, err error) {
+func prepareFork(branchName, upstreamOrg, upstreamRepo, myOrg, myRepo string) (repo *git.Repo, err error) {
 	// checkout the upstream repository
 	logrus.Infof("cloning/updating repository %s/%s", upstreamOrg, upstreamRepo)
 
-	repo, err = git.CloneOrOpenGitHubRepo(
-		repoPath, upstreamOrg, upstreamRepo, true,
+	repo, err = git.CleanCloneGitHubRepo(
+		upstreamOrg, upstreamRepo, false,
 	)
 	if err != nil {
 		return nil, errors.Wrapf(err, "cloning %s/%s", upstreamOrg, upstreamRepo)
 	}
 
 	// test if the fork remote is already existing
-	url := git.GetRepoURL(myOrg, myRepo, true)
+	url := git.GetRepoURL(myOrg, myRepo, false)
 	if repo.HasRemote(userForkName, url) {
 		logrus.Infof(
 			"Using already existing remote %v (%v) in repository",
@@ -401,14 +389,8 @@ func prepareFork(branchName, repoPath, upstreamOrg, upstreamRepo, myOrg, myRepo
 		}
 	}
 
-	// verify the branch doesn't already exist on the user's fork
-	err = repo.HasRemoteBranch(branchName)
-	if err == nil {
-		return nil, errors.Errorf("remote repo already has a branch named %s", branchName)
-	}
-
 	// checkout the new branch
-	err = repo.Checkout("-b", branchName)
+	err = repo.Checkout("-B", branchName)
 	if err != nil {
 		return nil, errors.Wrapf(err, "creating new branch %s", branchName)
 	}
@@ -483,8 +465,8 @@ func processJSONOutput(repoPath string) error {
 }
 
 // createWebsitePR creates the JSON version of the release notes and pushes them to a user fork
-func createWebsitePR(tag string, result *releaseNotesResult) error {
-	_, err := util.TagStringToSemver(tag)
+func createWebsitePR(tag string, result *releaseNotesResult) (err error) {
+	_, err = util.TagStringToSemver(tag)
 	if err != nil {
 		return errors.Wrapf(err, "no valid tag: %v", tag)
 	}
@@ -494,12 +476,15 @@ func createWebsitePR(tag string, result *releaseNotesResult) error {
 
 	// checkout kubernetes-sigs/release-notes
 	k8sSigsRepo, err := prepareFork(
-		branchname, releaseNotesOpts.kubernetessigsForkPath, defaultKubernetesSigsOrg,
+		branchname, defaultKubernetesSigsOrg,
 		defaultKubernetesSigsRepo, releaseNotesOpts.websiteOrg, releaseNotesOpts.websiteRepo,
 	)
 	if err != nil {
 		return errors.Wrap(err, "preparing local fork branch")
 	}
+	defer func() {
+		err = k8sSigsRepo.Cleanup()
+	}()
 
 	// add a reference to the new json file in assets.ts
 	if err := addReferenceToAssetsFile(k8sSigsRepo.Dir(), jsonNotesFilename); err != nil {

docs/krel/release-notes.md

@@ -40,9 +40,7 @@ Before running `krel release-notes` export your GitHub token to \$GITHUB_TOKEN:
       --draft-repo string                  the name of the fork of k/sig-release, the Release Notes Draft PR will be created from this repository (default "sig-release")
       --format string                      The format for notes output (options: markdown, json) (default "markdown")
   -h, --help                               help for release-notes
-      --kubernetes-sigs-fork-path string   fork kubernetes-sigs/release-notes and output a copy of the json release notes to this directory (default "/tmp/k8s-sigs")
   -o, --output-dir string                  output a copy of the release notes to this directory (default ".")
-      --sigrelease-fork-path string        fork k/sig-release and output a copy of the release notes draft to this directory (default "/tmp/k8s-sigrelease")
   -t, --tag string                         version tag for the notes
       --website-org string                 a Github organization owner of the fork of kuberntets-sigs/release-notes where the Website PR will be created
       --website-repo string                the name of the fork of kuberntets-sigs/release-notes, the Release Notes Draft PR will be created from this repository (default "release-notes")

pkg/git/git.go

@@ -198,9 +198,18 @@ func CloneOrOpenDefaultGitHubRepoSSH(repoPath string) (*Repo, error) {
 	)
 }
 
-// CloneOrOpenGitHubRepo creates a temp directory containing the provided
-// GitHub repository via the owner and repo. If useSSH is true, then it will
-// clone the repository using the defaultGithubAuthRoot.
+// CleanCloneGitHubRepo creates a guaranteed fresh checkout of a given repository. The returned *Repo has a Cleanup()
+// method that should be used to delete the repository on-disk afterwards.
+func CleanCloneGitHubRepo(owner, repo string, useSSH bool) (*Repo, error) {
+	repoURL := GetRepoURL(owner, repo, useSSH)
+	// The use of a blank string for the repo path triggers special behaviour in CloneOrOpenRepo that causes a true
+	// temporary directory with a random name to be created.
+	return CloneOrOpenRepo("", repoURL, useSSH)
+}
+
+// CloneOrOpenGitHubRepo works with a repository in the given directory, or creates one if the directory is empty. The
+// repo uses the provided GitHub repository via the owner and repo. If useSSH is true, then it will clone the
+// repository using the defaultGithubAuthRoot.
 func CloneOrOpenGitHubRepo(repoPath, owner, repo string, useSSH bool) (*Repo, error) {
 	repoURL := GetRepoURL(owner, repo, useSSH)
 	return CloneOrOpenRepo(repoPath, repoURL, useSSH)
@@ -843,7 +852,7 @@ func (r *Repo) AddRemote(name, owner, repo string) error {
 // PushToRemote push the current branch to a spcified remote, but only if the
 // repository is not in dry run mode
 func (r *Repo) PushToRemote(remote, remoteBranch string) error {
-	args := []string{"push"}
+	args := []string{"push", "--set-upstream"}
 	if r.dryRun {
 		logrus.Infof("Won't push due to dry run repository")
 		args = append(args, "--dry-run")