From 8b06e8d832db00eb614dc82d5c3a46c37fb3a3c8 Mon Sep 17 00:00:00 2001 From: tuti Date: Mon, 23 Mar 2026 16:54:06 -0700 Subject: [PATCH 1/4] feat(release): add create-release-branch target --- Makefile | 4 +- RELEASING.md | 68 ++++-------- hack/release/README.md | 59 +++++++++- hack/release/branch.go | 245 +++++++++++++++++++++++++++++++++++++++++ hack/release/checks.go | 22 ++++ hack/release/flags.go | 35 +++++- hack/release/main.go | 1 + hack/release/prep.go | 109 +++++------------- 8 files changed, 409 insertions(+), 134 deletions(-) create mode 100644 hack/release/branch.go diff --git a/Makefile b/Makefile index bbfe05c8d2..e1c52d2dac 100644 --- a/Makefile +++ b/Makefile @@ -615,7 +615,6 @@ hack/release/ut: sh -c '$(GIT_CONFIG_SSH) \ gotestsum --format=testname --junitfile report/release/ut.xml $(PACKAGE_NAME)/hack/release' - release-from: hack/bin/release var-require-all-VERSION-OPERATOR_BASE_VERSION var-require-one-of-EE_IMAGES_VERSIONS-OS_IMAGES_VERSIONS hack/bin/release from @@ -631,6 +630,9 @@ endif release-prep: hack/bin/release hack/bin/gh var-require-all-VERSION var-require-one-of-CALICO_VERSION-ENTERPRISE_VERSION @REPO=$(REPO) hack/bin/release prep +create-release-branch: hack/bin/release var-require-all-STREAM-CALICO_REF-ENTERPRISE_REF + RELEASE_STREAM=$(STREAM) hack/bin/release branch + ############################################################################### # Utilities ############################################################################### diff --git a/RELEASING.md b/RELEASING.md index fd4176640e..c8242e2806 100644 --- a/RELEASING.md +++ b/RELEASING.md @@ -2,57 +2,35 @@ ## Preparing a new release branch -For a major or minor release, you will need to create: +For a major or minor release, you will need to create a new `release-vX.Y` branch, a dev tag on master, +and a GitHub milestone for the next release. The `create-release-branch` Makefile target automates creating +the branch and dev tag; you will create the milestone manually in a later step: -- A new `release-vX.Y` branch based on the target minor version. We always do releases from a release - branch, not from master. -- An empty commit on the master branch, tagged with the "dev" version for the next minor release. - This ensures that `git describe --tags` (which is used to generate versions for CI builds) will - produce a version that "makes sense" for master commits after the release branch is created. -- A new GitHub milestone for the next minor release. This ensures that new PRs get auto-added to - the correct milestone. - -To create a new release branch: - -1. If needed, fetch the latest changes from the repository remote ``: - - ```sh - git fetch - ``` - -1. Create a new branch based on the target minor version: - - ```sh - git checkout /master -b release-vX.Y - ``` - -1. Push the new branch to the repository: +```sh +make create-release-branch STREAM=vX.Y CALICO_REF= ENTERPRISE_REF= +``` - ```sh - git push release-vX.Y - ``` +This command: -To create an empty commit and tag on master; run the following commands. This will push directly to master, -bypassing the normal PR process. This is important to make sure that the tag is directly on the master branch. -We create an empty commit because, when the release branch is created, it shares its commit history with master. -So, if we tagged the tip of master, we'd also be tagging the tip of the release branch, which would give -incorrect results for `git describe --tags` on the release branch. +- Creates a `release-vX.Y` branch from master +- Updates `config/calico_versions.yml` and `config/enterprise_versions.yml` to point at the given refs +- Runs `make fix gen-versions-calico gen-versions-enterprise` to regenerate files +- Commits the changes to the release branch +- Switches back to master, creates an empty commit, and tags it `vX.(Y+1).0-0.dev` +- Pushes the release branch, master, and tag to the remote - ```sh - git checkout /master - git commit --allow-empty -m "Start development on vX.Y" # Where vX.Y is the next minor version - git tag vX.Y.0-0.dev - git push HEAD:master # Note: if someone updates master before you push, delete the tag and start over from the new tip of master. - git push vX.Y.0-0.dev - ``` +**Flags / environment variables:** -*Note* that the tag should have the exact format `vX.Y.0-0.dev` where `X.Y` is the next minor version. -The `-0.dev` suffix was chosen to produce a semver-compliant version that is less than the -first release version for the new minor version. +| Env var | Flag | Description | +| -------------------------------------- | ------------------------- | ----------------------------------------------------------------- | +| `STREAM` / `RELEASE_STREAM` (required) | `--stream` | Release stream, e.g., `v1.43` | +| `CALICO_REF` (required) | `--calico-ref` | Calico git ref (branch or tag), e.g., `release-v3.32` | +| `ENTERPRISE_REF` (required) | `--enterprise-ref` | Enterprise git ref (branch or tag), e.g., `release-calient-v3.22` | +| `RELEASE_BRANCH_PREFIX` | `--release-branch-prefix` | Branch name prefix (default: `release`) | -Finally, create the next minor release's first milestone at https://github.com/tigera/operator/milestones. -This would mean if the branch release-v1.30 is being created, then the milestone v1.31.0 should be created too. -This ensures that new PRs against master will be automatically given the correct tag. +After the branch is created, create the next minor release's first milestone at +https://github.com/tigera/operator/milestones (e.g., if `release-v1.43` was created, +create milestone `v1.44.0`). ## Preparing for the release diff --git a/hack/release/README.md b/hack/release/README.md index a96d2a803f..861fc449d9 100644 --- a/hack/release/README.md +++ b/hack/release/README.md @@ -8,18 +8,20 @@ - [Build and publish a release](#build-and-publish-a-release) - [Build and publish a hashrelease](#build-and-publish-a-hashrelease) - [Commands](#commands) - - [release build](#release-build) + - [release branch](#release-branch) - [Examples](#examples) - - [release publish](#release-publish) + - [release build](#release-build) - [Examples](#examples-1) - - [release prep](#release-prep) + - [release publish](#release-publish) - [Examples](#examples-2) - - [release notes](#release-notes) + - [release prep](#release-prep) - [Examples](#examples-3) - - [release github](#release-github) + - [release notes](#release-notes) - [Examples](#examples-4) - - [release from](#release-from) + - [release github](#release-github) - [Examples](#examples-5) + - [release from](#release-from) + - [Examples](#examples-6) ## Installation @@ -131,6 +133,51 @@ Unlike a release, a hashrelease is typically for either a Calico or Calico Enter ## Commands +### release branch + +This command creates a new release branch for the operator. +It updates the calico and enterprise version configs on the release branch, regenerates files, +and commits the changes. It then switches back to master, creates an empty commit tagged +with `vX.Y.0-0.dev` (so that `git describe --tags` produces sensible versions for subsequent +master commits), and pushes the release branch, master, and tag to the remote. + +Both `--calico-ref` and `--enterprise-ref` are required. +They specify the git ref (branch or tag) to use for each product's version config. + +```sh +release branch --stream --calico-ref --enterprise-ref +``` + +If the `--local` flag is specified, the branch and tag are created locally without pushing to the remote. + +| Flag | Env var | Description | +|------|---------|-------------| +| `--stream` | `RELEASE_STREAM` | Release stream (e.g., `v1.43`). Required. | +| `--calico-ref` | `CALICO_REF` | Calico git ref (branch or tag). Required. | +| `--enterprise-ref` | `ENTERPRISE_REF` | Enterprise git ref (branch or tag). Required. | +| `--release-branch-prefix` | `RELEASE_BRANCH_PREFIX` | Branch name prefix (default: `release`). | +| `--local` | `LOCAL` | Skip pushing to remote. | + +There is also a Makefile target: + +```sh +make create-release-branch STREAM=vX.Y CALICO_REF= ENTERPRISE_REF= +``` + +#### Examples + +1. To create a release branch for operator v1.43 with Calico v3.32 and Enterprise v3.22 + + ```sh + release branch --stream v1.43 --calico-ref release-v3.32 --enterprise-ref release-calient-v3.22 + ``` + +1. To create a release branch locally without pushing + + ```sh + release branch --stream v1.43 --calico-ref release-v3.32 --enterprise-ref release-calient-v3.22 --local + ``` + ### release build This command builds the operator image for a specific operator version. diff --git a/hack/release/branch.go b/hack/release/branch.go new file mode 100644 index 0000000000..952c690606 --- /dev/null +++ b/hack/release/branch.go @@ -0,0 +1,245 @@ +// Copyright (c) 2026 Tigera, Inc. All rights reserved. + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package main + +import ( + "context" + "fmt" + "os" + "strings" + + "github.com/sirupsen/logrus" + "github.com/urfave/cli/v3" +) + +// Context keys for branch/prep commands +const ( + branchNameCtxKey contextKey = "branch-name" + calicoConfigVersionCtxKey contextKey = "calico-config-version" + enterpriseConfigVersionCtxKey contextKey = "enterprise-config-version" +) + +var branchCommand = &cli.Command{ + Name: "branch", + Usage: "Create a new branch for the release", + Description: "This command creates a new branch for the release.", + Flags: []cli.Flag{ + streamFlag, + calicoRefFlag, + enterpriseRefFlag, + releaseBranchPrefixFlag, + devTagSuffixFlag, + calicoDirFlag, + enterpriseDirFlag, + enterpriseRegistryFlag, + skipValidationFlag, + localFlag, + gitRemoteFlag, + }, + Before: branchBefore, + Action: branchAction, + After: branchAfter, +} + +// branchBeforeCommon handles shared Before logic for both branch and prep +func branchBeforeCommon(ctx context.Context, c *cli.Command, scopeContextFn func(context.Context, *cli.Command) (context.Context, error), validateFn func(context.Context, *cli.Command) (context.Context, error)) (context.Context, error) { + configureLogging(c) + + // Start with a clean slate for branch cleanup functions. + branchCleanupFns = nil + + var err error + ctx, err = scopeContextFn(ctx, c) + if err != nil { + return ctx, err + } + + if c.Bool(skipValidationFlag.Name) { + logrus.Warnf("Skipping %s validations as requested.", c.Name) + return ctx, nil + } + + ctx, err = checkGitClean(ctx) + if err != nil { + return ctx, err + } + return validateFn(ctx, c) +} + +// branchContextValuesFunc sets branch cutting context values based on CLI flags +var branchContextValuesFunc = func(ctx context.Context, c *cli.Command) (context.Context, error) { + ctx = context.WithValue(ctx, branchNameCtxKey, fmt.Sprintf("%s-%s", c.String(releaseBranchPrefixFlag.Name), c.String(streamFlag.Name))) + if calicoRef := c.String(calicoRefFlag.Name); calicoRef != "" { + ctx = context.WithValue(ctx, calicoConfigVersionCtxKey, calicoRef) + } + if enterpriseRef := c.String(enterpriseRefFlag.Name); enterpriseRef != "" { + ctx = context.WithValue(ctx, enterpriseConfigVersionCtxKey, enterpriseRef) + } + return ctx, nil +} + +// Pre-action for branch command. +var branchBefore = cli.BeforeFunc(func(ctx context.Context, c *cli.Command) (context.Context, error) { + return branchBeforeCommon(ctx, c, branchContextValuesFunc, validateBranchRefs) +}) + +// Action for branch command. +var branchAction = cli.ActionFunc(func(ctx context.Context, c *cli.Command) error { + stream := c.String(streamFlag.Name) + branchName := ctx.Value(branchNameCtxKey).(string) + remote := c.String(gitRemoteFlag.Name) + + if _, err := branchActionCommon(ctx, c, fmt.Sprintf("build: update config for %s", stream)); err != nil { + return err + } + // We are now on the release branch with config changes committed. + + // Switch back to master to create the dev tag. + // The dev tag goes on an empty commit on master so that git describe --tags + // produces sensible versions for subsequent master commits. + if _, err := git("switch", "master"); err != nil { + return fmt.Errorf("error switching back to master: %w", err) + } + version := fmt.Sprintf("%s.0", stream) + devTag := fmt.Sprintf("%s-%s", version, c.String(devTagSuffixFlag.Name)) + if _, err := git("commit", "--allow-empty", "-m", fmt.Sprintf("Start development on %s", stream)); err != nil { + return fmt.Errorf("error creating empty commit on master: %w", err) + } + if _, err := git("tag", devTag); err != nil { + return fmt.Errorf("error creating git tag %s: %w", devTag, err) + } + logrus.Infof("Created branch %s and tagged master with %s", branchName, devTag) + + if c.Bool(localFlag.Name) { + logrus.Warnf("Local flag is set, skipping pushing to remote") + return nil + } + + // Push release branch, master (with empty commit), and dev tag to remote + if _, err := git("push", remote, branchName); err != nil { + return fmt.Errorf("error pushing %s to remote: %w", branchName, err) + } + logrus.Infof("Pushed %s to remote", branchName) + if _, err := git("push", remote, "HEAD:master"); err != nil { + return fmt.Errorf("error pushing master to remote: %w", err) + } + logrus.Info("Pushed master with dev commit to remote") + if _, err := git("push", remote, devTag); err != nil { + return fmt.Errorf("error pushing tag %s to remote: %w", devTag, err) + } + logrus.Infof("Pushed tag %s to remote", devTag) + return nil +}) + +var branchCleanupFns []func() + +var branchAfter = cli.AfterFunc(func(_ context.Context, _ *cli.Command) error { + for i := len(branchCleanupFns) - 1; i >= 0; i-- { + branchCleanupFns[i]() + } + return nil +}) + +func switchBranch(branchName string) error { + // get current branch to switch back to later + baseBranch, err := git("branch", "--show-current") + if err != nil { + return fmt.Errorf("error getting current branch: %w", err) + } + branchCleanupFns = append(branchCleanupFns, func() { + if _, err := git("switch", "-f", baseBranch); err != nil { + logrus.WithError(err).Errorf("Failed to reset to %q branch", baseBranch) + } + }) + if _, err := git("switch", "-C", branchName); err != nil { + return fmt.Errorf("error creating and switching to branch %s: %w", branchName, err) + } + return nil +} + +// branchActionCommon switches to a new branch, modifies config versions, and commits the changes. +// It reads the branch name and calico/enterprise versions from context (set by Before functions). +// It returns the repo root directory for subsequent operations. +func branchActionCommon(ctx context.Context, c *cli.Command, commitMsg string) (string, error) { + branchName := ctx.Value(branchNameCtxKey).(string) + if err := switchBranch(branchName); err != nil { + return "", err + } + repoRootDir, err := gitDir() + if err != nil { + return "", fmt.Errorf("error getting git directory: %w", err) + } + if err := modifyConfigVersions(ctx, c, repoRootDir); err != nil { + return "", fmt.Errorf("error modifying config versions: %w", err) + } + if err := commitConfigChanges(repoRootDir, commitMsg); err != nil { + return "", fmt.Errorf("error committing config changes: %w", err) + } + return repoRootDir, nil +} + +// modifyConfigVersions updates config versions and runs make targets to regenerate files. +// It reads calico/enterprise versions from context (set by Before functions). +func modifyConfigVersions(ctx context.Context, c *cli.Command, repoRootDir string) error { + calicoVersion, _ := ctx.Value(calicoConfigVersionCtxKey).(string) + enterpriseVersion, _ := ctx.Value(enterpriseConfigVersionCtxKey).(string) + makeTargets := []string{"fix"} + env := os.Environ() + if calicoVersion != "" { + makeTargets = append(makeTargets, "gen-versions-calico") + if err := updateConfigVersions(repoRootDir, calicoConfig, calicoVersion); err != nil { + return fmt.Errorf("error modifying Calico config: %w", err) + } + // Set CALICO_CRDS_DIR if specified + if crdsDir := c.String(calicoDirFlag.Name); crdsDir != "" { + logrus.Warnf("Using local Calico CRDs from %s", crdsDir) + env = append(env, fmt.Sprintf("CALICO_CRDS_DIR=%s", crdsDir)) + } + } + if enterpriseVersion != "" { + makeTargets = append(makeTargets, "gen-versions-enterprise") + if err := updateConfigVersions(repoRootDir, enterpriseConfig, enterpriseVersion); err != nil { + return fmt.Errorf("error modifying Enterprise config: %w", err) + } + // Update registry for Enterprise + if eRegistry := c.String(enterpriseRegistryFlag.Name); eRegistry != "" { + logrus.Debugf("Updating Enterprise registry to %s", eRegistry) + if err := modifyComponentImageConfig(repoRootDir, componentImageConfigRelPath, enterpriseRegistryConfigKey, eRegistry); err != nil { + return fmt.Errorf("error modifying Enterprise registry config: %w", err) + } + } + // Set ENTERPRISE_CRDS_DIR if specified + if crdsDir := c.String(enterpriseDirFlag.Name); crdsDir != "" { + logrus.Warnf("Using local Enterprise CRDs from %s", crdsDir) + env = append(env, fmt.Sprintf("ENTERPRISE_CRDS_DIR=%s", crdsDir)) + } + } + + // Run make target to ensure files are formatted correctly and generated files are up to date. + if _, err := makeInDir(repoRootDir, strings.Join(makeTargets, " "), env...); err != nil { + return fmt.Errorf("error running \"make %s\": %w", strings.Join(makeTargets, " "), err) + } + return nil +} + +func commitConfigChanges(repoRootDir, msg string) error { + if _, err := gitInDir(repoRootDir, append([]string{"add"}, changedFiles...)...); err != nil { + return fmt.Errorf("error staging git changes: %w", err) + } + if _, err := git("commit", "-m", msg); err != nil { + return fmt.Errorf("error committing git changes: %w", err) + } + return nil +} diff --git a/hack/release/checks.go b/hack/release/checks.go index fbb5bd8dd1..09d7143ea6 100644 --- a/hack/release/checks.go +++ b/hack/release/checks.go @@ -81,3 +81,25 @@ var checkVersion = func(ctx context.Context, c *cli.Command) (context.Context, e } return checkVersionMatchesGitVersion(ctx, c) } + +// check that at least one of the given flags is set to a non-empty value. +var checkAtLeastOneOfFlags = func(ctx context.Context, c *cli.Command, flagNames ...string) (context.Context, error) { + for _, flag := range flagNames { + if c.String(flag) != "" { + return ctx, nil + } + } + return ctx, fmt.Errorf("at least one of the following flags must be set: %s", strings.Join(flagNames, ", ")) +} + +// validateBranchRefs validates that the required ref flags are set for branch creation. +// By default, both calico-ref and enterprise-ref are required. +// Override via init() for different products (e.g., operator-cloud). +var validateBranchRefs = func(ctx context.Context, c *cli.Command) (context.Context, error) { + calicoRef := c.String(calicoRefFlag.Name) + enterpriseRef := c.String(enterpriseRefFlag.Name) + if calicoRef == "" || enterpriseRef == "" { + return ctx, fmt.Errorf("both --%s and --%s are required for branch creation", calicoRefFlag.Name, enterpriseRefFlag.Name) + } + return ctx, nil +} diff --git a/hack/release/flags.go b/hack/release/flags.go index 56d8725bf7..e94b54e7a5 100644 --- a/hack/release/flags.go +++ b/hack/release/flags.go @@ -116,7 +116,7 @@ var ( Category: operatorFlagCategory, Usage: "The suffix used to denote development tags", Sources: cli.EnvVars("DEV_TAG_SUFFIX"), - Value: "0-dev", + Value: "0.dev", } versionFlag = &cli.StringFlag{ Name: "version", @@ -137,6 +137,27 @@ var ( return nil }, } + streamFlag = &cli.StringFlag{ + Name: "stream", + Category: operatorFlagCategory, + Usage: "The release stream for the release branch name (e.g. vX.Y). The full branch name will be -", + Sources: cli.EnvVars("RELEASE_STREAM"), + Required: true, + Action: func(ctx context.Context, c *cli.Command, s string) error { + // Validate stream is in the format vX.Y + if !regexp.MustCompile(`^v\d+\.\d+$`).MatchString(s) { + return fmt.Errorf("stream must be in the format vX.Y, got %q", s) + } + return nil + }, + } + releaseBranchPrefixFlag = &cli.StringFlag{ + Name: "release-branch-prefix", + Category: operatorFlagCategory, + Usage: "The prefix to use for the release branch name. The full branch name will be -", + Sources: cli.EnvVars("RELEASE_BRANCH_PREFIX"), + Value: "release", + } baseOperatorFlag = &cli.StringFlag{ Name: "base-version", Category: operatorFlagCategory, @@ -269,6 +290,12 @@ var ( return nil }, } + calicoRefFlag = &cli.StringFlag{ + Name: "calico-ref", + Category: calicoFlagCategory, + Usage: "The Calico git ref (branch or tag) to use for version config (e.g. release-vX.Y)", + Sources: cli.EnvVars("CALICO_REF"), + } exceptCalicoFlag = &cli.StringSliceFlag{ Name: "except-calico", Category: calicoFlagCategory, @@ -358,6 +385,12 @@ var ( return nil }, } + enterpriseRefFlag = &cli.StringFlag{ + Name: "enterprise-ref", + Category: enterpriseFlagCategory, + Usage: "The Enterprise git ref (branch or tag) to use for version config (e.g. release-calient-vX.Y-1)", + Sources: cli.EnvVars("ENTERPRISE_REF"), + } enterpriseRegistryFlag = &cli.StringFlag{ Name: "enterprise-registry", Category: enterpriseFlagCategory, diff --git a/hack/release/main.go b/hack/release/main.go index 2b8f71b2fa..22bc6d6b1b 100644 --- a/hack/release/main.go +++ b/hack/release/main.go @@ -45,6 +45,7 @@ func app(version string) *cli.Command { Commands: []*cli.Command{ buildCommand, publishCommand, + branchCommand, prepCommand, publicCommand, releaseNotesCommand, diff --git a/hack/release/prep.go b/hack/release/prep.go index aff3f5920a..16444f96fd 100644 --- a/hack/release/prep.go +++ b/hack/release/prep.go @@ -66,27 +66,40 @@ to point to local repositories for Calico and Enterprise respectively.`, }, Before: prepBefore, Action: prepAction, + After: branchAfter, } -// Pre-action for release prep command. -var prepBefore = cli.BeforeFunc(func(ctx context.Context, c *cli.Command) (context.Context, error) { - configureLogging(c) +// validatePrepRefs checks that at least one of calico/enterprise version is set for prep. +var validatePrepRefs = func(ctx context.Context, c *cli.Command) (context.Context, error) { + return checkAtLeastOneOfFlags(ctx, c, calicoVersionFlag.Name, enterpriseVersionFlag.Name) +} +// prepContextValuesFunc sets context values for the prep command based on CLI flags. +var prepContextValuesFunc = func(ctx context.Context, c *cli.Command) (context.Context, error) { // Extract repo information from CLI repo flag into context - var err error - ctx, err = addRepoInfoToCtx(ctx, c.String(gitRepoFlag.Name)) + ctx, err := addRepoInfoToCtx(ctx, c.String(gitRepoFlag.Name)) if err != nil { return ctx, err } - // Skip validations if requested - if c.Bool(skipValidationFlag.Name) { - logrus.Warnf("Skipping %s validation as requested.", c.Name) - return ctx, nil + // Set branch cutting context values based on CLI flags + version := c.String(versionFlag.Name) + ctx = context.WithValue(ctx, versionCtxKey, version) + ctx = context.WithValue(ctx, branchNameCtxKey, fmt.Sprintf("build-%s", version)) + if calicoVer := c.String(calicoVersionFlag.Name); calicoVer != "" { + ctx = context.WithValue(ctx, calicoConfigVersionCtxKey, calicoVer) + } + if epVer := c.String(enterpriseVersionFlag.Name); epVer != "" { + ctx = context.WithValue(ctx, enterpriseConfigVersionCtxKey, epVer) } + return ctx, nil +} - // Ensure that git working tree is clean - ctx, err = checkGitClean(ctx) +// Pre-action for release prep command. +var prepBefore = cli.BeforeFunc(func(ctx context.Context, c *cli.Command) (context.Context, error) { + var err error + + ctx, err = branchBeforeCommon(ctx, c, prepContextValuesFunc, validatePrepRefs) if err != nil { return ctx, err } @@ -95,11 +108,6 @@ var prepBefore = cli.BeforeFunc(func(ctx context.Context, c *cli.Command) (conte return ctx, fmt.Errorf("GitHub token must be provided via --%s flag or GITHUB_TOKEN environment variable", githubTokenFlag.Name) } - // One of Calico or Enterprise version must be specified. - if c.String(calicoVersionFlag.Name) == "" && c.String(enterpriseVersionFlag.Name) == "" { - return ctx, fmt.Errorf("at least one of %s or %s must be specified", calicoVersionFlag.Name, enterpriseVersionFlag.Name) - } - // If Calico is not passed in, check the version in calico_versions.yml is a released version. // An operator release must always include a released Calico version. calicoVersion := c.String(calicoVersionFlag.Name) @@ -125,76 +133,15 @@ var prepBefore = cli.BeforeFunc(func(ctx context.Context, c *cli.Command) (conte // Action executed for release prep command. var prepAction = cli.ActionFunc(func(ctx context.Context, c *cli.Command) error { - // get current branch to switch back to later baseBranch, err := git("branch", "--show-current") if err != nil { return fmt.Errorf("error getting current branch: %w", err) } - defer func() { - if _, err := git("switch", "-f", baseBranch); err != nil { - logrus.WithError(err).Errorf("Failed to reset to %q branch", baseBranch) - } - }() - - makeTargets := []string{"fix"} - prepEnv := os.Environ() - - repoRootDir, err := gitDir() + version := ctx.Value(versionCtxKey).(string) + prepBranch := ctx.Value(branchNameCtxKey).(string) + repoRootDir, err := branchActionCommon(ctx, c, fmt.Sprintf("build: %s release", version)) if err != nil { - return fmt.Errorf("error getting git directory: %w", err) - } - version := c.String(versionFlag.Name) - ctx = context.WithValue(ctx, versionCtxKey, version) - - // Create and switch to new branch using "switch -C" to avoid issues if the branch already exists - prepBranch := fmt.Sprintf("build-%s", version) - if _, err := git("switch", "-C", prepBranch); err != nil { - return fmt.Errorf("error creating and switching to branch %s: %w", prepBranch, err) - } - - // Modify config versions files - if calico := c.String(calicoVersionFlag.Name); calico != "" { - makeTargets = append(makeTargets, "gen-versions-calico") - if err := updateConfigVersions(repoRootDir, calicoConfig, calico); err != nil { - return fmt.Errorf("error modifying Calico config: %w", err) - } - // Set CALICO_CRDS_DIR if specified - if crdsDir := c.String(calicoDirFlag.Name); crdsDir != "" { - logrus.Warnf("Using local Calico CRDs from %s", crdsDir) - prepEnv = append(prepEnv, fmt.Sprintf("CALICO_CRDS_DIR=%s", crdsDir)) - } - } - enterprise := c.String(enterpriseVersionFlag.Name) - if enterprise != "" { - makeTargets = append(makeTargets, "gen-versions-enterprise") - if err := updateConfigVersions(repoRootDir, enterpriseConfig, enterprise); err != nil { - return fmt.Errorf("error modifying Enterprise config: %w", err) - } - // Update registry for Enterprise - if eRegistry := c.String(enterpriseRegistryFlag.Name); eRegistry != "" { - logrus.Debugf("Updating Enterprise registry to %s", eRegistry) - if err := modifyComponentImageConfig(repoRootDir, componentImageConfigRelPath, enterpriseRegistryConfigKey, eRegistry); err != nil { - return err - } - } - // Set ENTERPRISE_CRDS_DIR if specified - if crdsDir := c.String(enterpriseDirFlag.Name); crdsDir != "" { - logrus.Warnf("Using local Enterprise CRDs from %s", crdsDir) - prepEnv = append(prepEnv, fmt.Sprintf("ENTERPRISE_CRDS_DIR=%s", crdsDir)) - } - } - - // Run make target to ensure files are formatted correctly and generated files are up to date. - if _, err := makeInDir(repoRootDir, strings.Join(makeTargets, " "), prepEnv...); err != nil { - return fmt.Errorf("error running \"make fix gen-versions\": %w", err) - } - - // Commit changes - if _, err := gitInDir(repoRootDir, append([]string{"add"}, changedFiles...)...); err != nil { - return fmt.Errorf("error staging git changes: %w", err) - } - if _, err := git("commit", "-m", fmt.Sprintf("build: %s release", version)); err != nil { - return fmt.Errorf("error committing git changes: %w", err) + return err } // If local flag is set, skip pushing prep branch and creating PR From 03b7817f6acf862afc8af146a34de2f0dbbfd855 Mon Sep 17 00:00:00 2001 From: tuti Date: Wed, 25 Mar 2026 08:45:52 -0700 Subject: [PATCH 2/4] feat(release): add remote ref validation to branch and prep commands Enhance validateBranchRefs to check that the operator branch doesn't already exist in the remote and that calico/enterprise refs exist as branches or tags. Similarly enhance validatePrepRefs to verify version tags exist in their respective remote repositories. Move validation logic from prepBefore into validatePrepRefs for consistency. --- hack/release/branch.go | 45 +++++++++++++++++++++++ hack/release/checks.go | 12 ------- hack/release/prep.go | 82 ++++++++++++++++++++++++++++++------------ 3 files changed, 105 insertions(+), 34 deletions(-) diff --git a/hack/release/branch.go b/hack/release/branch.go index 952c690606..ebfe26dd47 100644 --- a/hack/release/branch.go +++ b/hack/release/branch.go @@ -38,7 +38,9 @@ var branchCommand = &cli.Command{ Flags: []cli.Flag{ streamFlag, calicoRefFlag, + calicoGitRepoFlag, enterpriseRefFlag, + enterpriseGitRepoFlag, releaseBranchPrefixFlag, devTagSuffixFlag, calicoDirFlag, @@ -90,6 +92,49 @@ var branchContextValuesFunc = func(ctx context.Context, c *cli.Command) (context return ctx, nil } +// validateBranchRefs validates that the required ref flags are set for branch creation. +// - check that the operator branch does not already exist +// - check that both calico and enterprise refs are provided +// - check that the provided calico and enterprise refs exist as a branch or tag in the remote repository +var validateBranchRefs = func(ctx context.Context, c *cli.Command) (context.Context, error) { + // check that the operator branch does not already exist + remote := c.String(gitRemoteFlag.Name) + branchName := ctx.Value(branchNameCtxKey).(string) + out, err := git("ls-remote", "--heads", remote, branchName) + if err != nil { + return ctx, fmt.Errorf("checking if branch %s exists in remote %s: %w", branchName, remote, err) + } + if out != "" { + return ctx, fmt.Errorf("branch %s already exists in remote %s, please choose a different name or delete the existing branch", branchName, remote) + } + + // check that both calico and enterprise refs are provided + calicoRef := c.String(calicoRefFlag.Name) + enterpriseRef := c.String(enterpriseRefFlag.Name) + if calicoRef == "" || enterpriseRef == "" { + return ctx, fmt.Errorf("both --%s and --%s are required for branch creation", calicoRefFlag.Name, enterpriseRefFlag.Name) + } + + // check that the provided calico and enterprise refs exist as a branch or tag in the remote repository + for _, check := range []struct { + ref string + repo string + flag string + }{ + {calicoRef, calicoGitRepoFlag.Value, calicoRefFlag.Name}, + {enterpriseRef, enterpriseGitRepoFlag.Value, enterpriseRefFlag.Name}, + } { + out, err := git("ls-remote", "--branches", "--tags", fmt.Sprintf("git@github.com:%s", check.repo), check.ref) + if err != nil { + return ctx, fmt.Errorf("checking if ref %q exists in %s: %w", check.ref, check.repo, err) + } + if !strings.Contains(out, check.ref) { + return ctx, fmt.Errorf("ref %q not found as a branch or tag in %s", check.ref, check.repo) + } + } + return ctx, nil +} + // Pre-action for branch command. var branchBefore = cli.BeforeFunc(func(ctx context.Context, c *cli.Command) (context.Context, error) { return branchBeforeCommon(ctx, c, branchContextValuesFunc, validateBranchRefs) diff --git a/hack/release/checks.go b/hack/release/checks.go index 09d7143ea6..88fba2329e 100644 --- a/hack/release/checks.go +++ b/hack/release/checks.go @@ -91,15 +91,3 @@ var checkAtLeastOneOfFlags = func(ctx context.Context, c *cli.Command, flagNames } return ctx, fmt.Errorf("at least one of the following flags must be set: %s", strings.Join(flagNames, ", ")) } - -// validateBranchRefs validates that the required ref flags are set for branch creation. -// By default, both calico-ref and enterprise-ref are required. -// Override via init() for different products (e.g., operator-cloud). -var validateBranchRefs = func(ctx context.Context, c *cli.Command) (context.Context, error) { - calicoRef := c.String(calicoRefFlag.Name) - enterpriseRef := c.String(enterpriseRefFlag.Name) - if calicoRef == "" || enterpriseRef == "" { - return ctx, fmt.Errorf("both --%s and --%s are required for branch creation", calicoRefFlag.Name, enterpriseRefFlag.Name) - } - return ctx, nil -} diff --git a/hack/release/prep.go b/hack/release/prep.go index 16444f96fd..38d4f6c793 100644 --- a/hack/release/prep.go +++ b/hack/release/prep.go @@ -55,8 +55,10 @@ to point to local repositories for Calico and Enterprise respectively.`, versionFlag, calicoVersionFlag, calicoDirFlag, + calicoGitRepoFlag, enterpriseVersionFlag, enterpriseDirFlag, + enterpriseGitRepoFlag, enterpriseRegistryFlag, skipValidationFlag, skipMilestoneFlag, @@ -69,9 +71,65 @@ to point to local repositories for Calico and Enterprise respectively.`, After: branchAfter, } -// validatePrepRefs checks that at least one of calico/enterprise version is set for prep. +// validatePrepRefs checks the required refs for release prep: +// - check that at least one of calico or enterprise version is provided +// - if calico version is not provided, check that the version in calico_versions.yml is a released version +// - check that the provided calico and enterprise refs exist as a tag in the remote repository (if local directory not provided) var validatePrepRefs = func(ctx context.Context, c *cli.Command) (context.Context, error) { - return checkAtLeastOneOfFlags(ctx, c, calicoVersionFlag.Name, enterpriseVersionFlag.Name) + // check that at least one of calico/enterprise version is set for prep + ctx, err := checkAtLeastOneOfFlags(ctx, c, calicoVersionFlag.Name, enterpriseVersionFlag.Name) + if err != nil { + return ctx, err + } + + // If Calico is not passed in, check the version in calico_versions.yml is a released version. + // An operator release must always include a released Calico version. + calicoVersion := c.String(calicoVersionFlag.Name) + if calicoVersion != "" { + return ctx, nil + } + dir, err := gitDir() + if err != nil { + return ctx, fmt.Errorf("error getting git directory: %w", err) + } + versions, err := calicoConfigVersions(dir, calicoConfig) + if err != nil { + return ctx, fmt.Errorf("error retrieving Calico version: %w", err) + } + calicoVersion = versions.Title + if valid, err := isReleaseVersionFormat(calicoVersion); err != nil { + return ctx, fmt.Errorf("error validating Calico version format: %w", err) + } else if !valid { + return ctx, fmt.Errorf("every release must contain a released Calico version, but found %s in %s", calicoVersion, calicoConfig) + } + + // check that the ref for calico and/or enterprise provided exists as a tag in the specified remote repository + // unless a local directory is provided for the respective component, in which case we assume the version exists since it is being pulled from the local repo + for _, check := range []struct { + repo string + tag string + flag string + localDir string + }{ + {tag: calicoVersion, repo: c.String(calicoGitRepoFlag.Name), localDir: c.String(calicoDirFlag.Name), flag: calicoVersionFlag.Name}, + {tag: c.String(enterpriseVersionFlag.Name), repo: c.String(enterpriseGitRepoFlag.Name), localDir: c.String(enterpriseDirFlag.Name), flag: enterpriseVersionFlag.Name}, + } { + if check.tag == "" { + continue + } + if check.localDir != "" { + logrus.Warnf("Local directory provided for %s, skipping remote ref validation", check.flag) + continue + } + out, err := git("ls-remote", "--tags", fmt.Sprintf("git@github.com:%s", check.repo), check.tag) + if err != nil { + return ctx, fmt.Errorf("checking if ref %q exists in %s: %w", check.tag, check.repo, err) + } + if !strings.Contains(out, check.tag) { + return ctx, fmt.Errorf("ref %q not found as a tag in %s", check.tag, check.repo) + } + } + return ctx, nil } // prepContextValuesFunc sets context values for the prep command based on CLI flags. @@ -108,26 +166,6 @@ var prepBefore = cli.BeforeFunc(func(ctx context.Context, c *cli.Command) (conte return ctx, fmt.Errorf("GitHub token must be provided via --%s flag or GITHUB_TOKEN environment variable", githubTokenFlag.Name) } - // If Calico is not passed in, check the version in calico_versions.yml is a released version. - // An operator release must always include a released Calico version. - calicoVersion := c.String(calicoVersionFlag.Name) - if calicoVersion != "" { - return ctx, nil - } - dir, err := gitDir() - if err != nil { - return ctx, fmt.Errorf("error getting git directory: %w", err) - } - versions, err := calicoConfigVersions(dir, calicoConfig) - if err != nil { - return ctx, fmt.Errorf("error retrieving Calico version: %w", err) - } - calicoVersion = versions.Title - if valid, err := isReleaseVersionFormat(calicoVersion); err != nil { - return ctx, fmt.Errorf("error validating Calico version format: %w", err) - } else if !valid { - return ctx, fmt.Errorf("every release must contain a released Calico version, but found %s in %s", calicoVersion, calicoConfig) - } return ctx, nil }) From 54a25340ebc6fb12f14caa2863acb6246d4cbba2 Mon Sep 17 00:00:00 2001 From: tuti Date: Fri, 27 Mar 2026 16:30:00 -0700 Subject: [PATCH 3/4] feat(release): add base branch validation and flexible branching support Support branching from both master and release branches. Add isReleaseBranch validation, skip-branch-check flag, and dev tag creation only when branching from non-release branches. Consolidate development flags under a shared category and fix error message conventions. --- hack/release/branch.go | 186 +++++++++++++++++++++++++++++------------ hack/release/flags.go | 40 +++++---- hack/release/prep.go | 40 +++++---- 3 files changed, 186 insertions(+), 80 deletions(-) diff --git a/hack/release/branch.go b/hack/release/branch.go index ebfe26dd47..bba6c21852 100644 --- a/hack/release/branch.go +++ b/hack/release/branch.go @@ -18,8 +18,10 @@ import ( "context" "fmt" "os" + "regexp" "strings" + "github.com/blang/semver/v4" "github.com/sirupsen/logrus" "github.com/urfave/cli/v3" ) @@ -27,10 +29,26 @@ import ( // Context keys for branch/prep commands const ( branchNameCtxKey contextKey = "branch-name" + baseBranchCtxKey contextKey = "base-branch" calicoConfigVersionCtxKey contextKey = "calico-config-version" enterpriseConfigVersionCtxKey contextKey = "enterprise-config-version" ) +var ( + // releaseBranchFormat matches release branches with a version suffix (e.g. release-v1.2). + releaseBranchFormat = `^(%s-v\d+\.\d+)$` + + defaultBaseBranch = "master" + + changedFiles = []string{ + calicoConfig, + enterpriseConfig, + "pkg/components", + "pkg/imports/crds", + "pkg/imports/admission", + } +) + var branchCommand = &cli.Command{ Name: "branch", Usage: "Create a new branch for the release", @@ -46,6 +64,7 @@ var branchCommand = &cli.Command{ calicoDirFlag, enterpriseDirFlag, enterpriseRegistryFlag, + skipBranchCheckFlag, skipValidationFlag, localFlag, gitRemoteFlag, @@ -68,6 +87,13 @@ func branchBeforeCommon(ctx context.Context, c *cli.Command, scopeContextFn func return ctx, err } + if baseBranch := ctx.Value(baseBranchCtxKey).(string); baseBranch != defaultBaseBranch { + logrus.WithFields(logrus.Fields{ + "base": baseBranch, + "expected": defaultBaseBranch, + }).Warn("Current branch is not the default base branch") + } + if c.Bool(skipValidationFlag.Name) { logrus.Warnf("Skipping %s validations as requested.", c.Name) return ctx, nil @@ -82,6 +108,11 @@ func branchBeforeCommon(ctx context.Context, c *cli.Command, scopeContextFn func // branchContextValuesFunc sets branch cutting context values based on CLI flags var branchContextValuesFunc = func(ctx context.Context, c *cli.Command) (context.Context, error) { + currentBranch, err := git("branch", "--show-current") + if err != nil { + return ctx, fmt.Errorf("getting current branch: %w", err) + } + ctx = context.WithValue(ctx, baseBranchCtxKey, currentBranch) ctx = context.WithValue(ctx, branchNameCtxKey, fmt.Sprintf("%s-%s", c.String(releaseBranchPrefixFlag.Name), c.String(streamFlag.Name))) if calicoRef := c.String(calicoRefFlag.Name); calicoRef != "" { ctx = context.WithValue(ctx, calicoConfigVersionCtxKey, calicoRef) @@ -92,10 +123,19 @@ var branchContextValuesFunc = func(ctx context.Context, c *cli.Command) (context return ctx, nil } +var isReleaseBranch = func(releaseBranchPrefix, branch string) (bool, error) { + branchRegex, err := regexp.Compile(fmt.Sprintf(releaseBranchFormat, releaseBranchPrefix)) + if err != nil { + return false, fmt.Errorf("compiling release branch format regex: %w", err) + } + return branchRegex.MatchString(branch), nil +} + // validateBranchRefs validates that the required ref flags are set for branch creation. // - check that the operator branch does not already exist // - check that both calico and enterprise refs are provided // - check that the provided calico and enterprise refs exist as a branch or tag in the remote repository +// - check that the base operator branch is either a release branch (or master) (if not skipping branch check) var validateBranchRefs = func(ctx context.Context, c *cli.Command) (context.Context, error) { // check that the operator branch does not already exist remote := c.String(gitRemoteFlag.Name) @@ -121,10 +161,10 @@ var validateBranchRefs = func(ctx context.Context, c *cli.Command) (context.Cont repo string flag string }{ - {calicoRef, calicoGitRepoFlag.Value, calicoRefFlag.Name}, - {enterpriseRef, enterpriseGitRepoFlag.Value, enterpriseRefFlag.Name}, + {calicoRef, c.String(calicoGitRepoFlag.Name), calicoRefFlag.Name}, + {enterpriseRef, c.String(enterpriseGitRepoFlag.Name), enterpriseRefFlag.Name}, } { - out, err := git("ls-remote", "--branches", "--tags", fmt.Sprintf("git@github.com:%s", check.repo), check.ref) + out, err := git("ls-remote", "--heads", "--tags", fmt.Sprintf("git@github.com:%s", check.repo), check.ref) if err != nil { return ctx, fmt.Errorf("checking if ref %q exists in %s: %w", check.ref, check.repo, err) } @@ -132,6 +172,21 @@ var validateBranchRefs = func(ctx context.Context, c *cli.Command) (context.Cont return ctx, fmt.Errorf("ref %q not found as a branch or tag in %s", check.ref, check.repo) } } + + // check operator base branch is either the default base branch or a release branch (if not skipping branch check) + currBranch := ctx.Value(baseBranchCtxKey).(string) + if c.Bool(skipBranchCheckFlag.Name) { + logrus.Warnf("Skipping branch validation as requested.") + return ctx, nil + } + releaseBranch, err := isReleaseBranch(c.String(releaseBranchPrefixFlag.Name), currBranch) + if err != nil { + return ctx, fmt.Errorf("validating current branch: %w", err) + } + if currBranch != defaultBaseBranch && !releaseBranch { + return ctx, fmt.Errorf("current branch is %s, please switch to %s or a release branch before running this command or use --%s to skip this check", currBranch, defaultBaseBranch, skipBranchCheckFlag.Name) + } + return ctx, nil } @@ -143,48 +198,73 @@ var branchBefore = cli.BeforeFunc(func(ctx context.Context, c *cli.Command) (con // Action for branch command. var branchAction = cli.ActionFunc(func(ctx context.Context, c *cli.Command) error { stream := c.String(streamFlag.Name) - branchName := ctx.Value(branchNameCtxKey).(string) remote := c.String(gitRemoteFlag.Name) + baseBranch := ctx.Value(baseBranchCtxKey).(string) + branchName := ctx.Value(branchNameCtxKey).(string) + refs := []string{branchName} if _, err := branchActionCommon(ctx, c, fmt.Sprintf("build: update config for %s", stream)); err != nil { return err } - // We are now on the release branch with config changes committed. + logrus.WithField("newBranch", branchName).Info("Created new branch") - // Switch back to master to create the dev tag. - // The dev tag goes on an empty commit on master so that git describe --tags - // produces sensible versions for subsequent master commits. - if _, err := git("switch", "master"); err != nil { - return fmt.Errorf("error switching back to master: %w", err) - } - version := fmt.Sprintf("%s.0", stream) - devTag := fmt.Sprintf("%s-%s", version, c.String(devTagSuffixFlag.Name)) - if _, err := git("commit", "--allow-empty", "-m", fmt.Sprintf("Start development on %s", stream)); err != nil { - return fmt.Errorf("error creating empty commit on master: %w", err) + // If this was not branched off a release branch, switch back to baseBranch branch to create the dev tag. + // The dev tag goes on an empty commit on the baseBranch branch so that git describe --tags + // produces sensible versions for subsequent baseBranch branch commits. + var nextDevTag string + releaseBranch, err := isReleaseBranch(c.String(releaseBranchPrefixFlag.Name), baseBranch) + if err != nil { + return fmt.Errorf("checking if base branch is a release branch: %w", err) } - if _, err := git("tag", devTag); err != nil { - return fmt.Errorf("error creating git tag %s: %w", devTag, err) + version, err := semver.Parse(fmt.Sprintf("%s.0", strings.TrimPrefix(stream, "v"))) + if err != nil { + logrus.WithField("stream", stream).Warn("Cannot create a valid semver off stream") + } else if !releaseBranch { + refs = append(refs, baseBranch) + if err := version.IncrementMinor(); err != nil { + return fmt.Errorf("incrementing minor version: %w", err) + } + nextDevTag = fmt.Sprintf("v%s-%s", version.String(), c.String(devTagSuffixFlag.Name)) + refs = append(refs, nextDevTag) + if out, err := git("switch", baseBranch); err != nil { + logrus.Error(out) + return fmt.Errorf("switching back to %s: %w", baseBranch, err) + } + if out, err := git("commit", "--allow-empty", "-m", fmt.Sprintf("Start development on v%d.%d", version.Major, version.Minor)); err != nil { + logrus.Error(out) + return fmt.Errorf("creating empty commit on %s: %w", baseBranch, err) + } + if out, err := git("tag", nextDevTag, "-m", fmt.Sprintf("%s development", version)); err != nil { + logrus.Error(out) + return fmt.Errorf("creating git tag %s: %w", nextDevTag, err) + } + logrus.WithField("devTag", nextDevTag).Infof("Created dev tag on %s", baseBranch) } - logrus.Infof("Created branch %s and tagged master with %s", branchName, devTag) if c.Bool(localFlag.Name) { - logrus.Warnf("Local flag is set, skipping pushing to remote") + logrus.WithFields(logrus.Fields{ + "remote": remote, + "baseBranch": baseBranch, + "newBranch": branchName, + "newDevTag": nextDevTag, + }).Warn("Local flag is set, skipping pushing to remote") return nil } - // Push release branch, master (with empty commit), and dev tag to remote - if _, err := git("push", remote, branchName); err != nil { - return fmt.Errorf("error pushing %s to remote: %w", branchName, err) - } - logrus.Infof("Pushed %s to remote", branchName) - if _, err := git("push", remote, "HEAD:master"); err != nil { - return fmt.Errorf("error pushing master to remote: %w", err) - } - logrus.Info("Pushed master with dev commit to remote") - if _, err := git("push", remote, devTag); err != nil { - return fmt.Errorf("error pushing tag %s to remote: %w", devTag, err) + // Push refs to remote - release branch, base branch (with empty commit), and dev tag + for _, ref := range refs { + if ref == "" { + continue + } + if out, err := git("push", remote, ref); err != nil { + logrus.Error(out) + return fmt.Errorf("pushing %s to remote: %w", ref, err) + } + logrus.WithFields(logrus.Fields{ + "ref": ref, + "remote": remote, + }).Infof("Pushed to %s", remote) } - logrus.Infof("Pushed tag %s to remote", devTag) return nil }) @@ -197,19 +277,18 @@ var branchAfter = cli.AfterFunc(func(_ context.Context, _ *cli.Command) error { return nil }) -func switchBranch(branchName string) error { +func switchBranch(ctx context.Context, branchName string) error { // get current branch to switch back to later - baseBranch, err := git("branch", "--show-current") - if err != nil { - return fmt.Errorf("error getting current branch: %w", err) - } + baseBranch := ctx.Value(baseBranchCtxKey).(string) branchCleanupFns = append(branchCleanupFns, func() { - if _, err := git("switch", "-f", baseBranch); err != nil { + if out, err := git("switch", "-f", baseBranch); err != nil { + logrus.Error(out) logrus.WithError(err).Errorf("Failed to reset to %q branch", baseBranch) } }) - if _, err := git("switch", "-C", branchName); err != nil { - return fmt.Errorf("error creating and switching to branch %s: %w", branchName, err) + if out, err := git("switch", "-C", branchName); err != nil { + logrus.Error(out) + return fmt.Errorf("creating and switching to branch %s: %w", branchName, err) } return nil } @@ -219,18 +298,18 @@ func switchBranch(branchName string) error { // It returns the repo root directory for subsequent operations. func branchActionCommon(ctx context.Context, c *cli.Command, commitMsg string) (string, error) { branchName := ctx.Value(branchNameCtxKey).(string) - if err := switchBranch(branchName); err != nil { + if err := switchBranch(ctx, branchName); err != nil { return "", err } repoRootDir, err := gitDir() if err != nil { - return "", fmt.Errorf("error getting git directory: %w", err) + return "", fmt.Errorf("getting git directory: %w", err) } if err := modifyConfigVersions(ctx, c, repoRootDir); err != nil { - return "", fmt.Errorf("error modifying config versions: %w", err) + return "", fmt.Errorf("modifying config versions: %w", err) } if err := commitConfigChanges(repoRootDir, commitMsg); err != nil { - return "", fmt.Errorf("error committing config changes: %w", err) + return "", fmt.Errorf("committing config changes: %w", err) } return repoRootDir, nil } @@ -245,7 +324,7 @@ func modifyConfigVersions(ctx context.Context, c *cli.Command, repoRootDir strin if calicoVersion != "" { makeTargets = append(makeTargets, "gen-versions-calico") if err := updateConfigVersions(repoRootDir, calicoConfig, calicoVersion); err != nil { - return fmt.Errorf("error modifying Calico config: %w", err) + return fmt.Errorf("modifying Calico config: %w", err) } // Set CALICO_CRDS_DIR if specified if crdsDir := c.String(calicoDirFlag.Name); crdsDir != "" { @@ -256,13 +335,13 @@ func modifyConfigVersions(ctx context.Context, c *cli.Command, repoRootDir strin if enterpriseVersion != "" { makeTargets = append(makeTargets, "gen-versions-enterprise") if err := updateConfigVersions(repoRootDir, enterpriseConfig, enterpriseVersion); err != nil { - return fmt.Errorf("error modifying Enterprise config: %w", err) + return fmt.Errorf("modifying Enterprise config: %w", err) } // Update registry for Enterprise if eRegistry := c.String(enterpriseRegistryFlag.Name); eRegistry != "" { logrus.Debugf("Updating Enterprise registry to %s", eRegistry) if err := modifyComponentImageConfig(repoRootDir, componentImageConfigRelPath, enterpriseRegistryConfigKey, eRegistry); err != nil { - return fmt.Errorf("error modifying Enterprise registry config: %w", err) + return fmt.Errorf("modifying Enterprise registry config: %w", err) } } // Set ENTERPRISE_CRDS_DIR if specified @@ -273,18 +352,21 @@ func modifyConfigVersions(ctx context.Context, c *cli.Command, repoRootDir strin } // Run make target to ensure files are formatted correctly and generated files are up to date. - if _, err := makeInDir(repoRootDir, strings.Join(makeTargets, " "), env...); err != nil { - return fmt.Errorf("error running \"make %s\": %w", strings.Join(makeTargets, " "), err) + if out, err := makeInDir(repoRootDir, strings.Join(makeTargets, " "), env...); err != nil { + logrus.Error(out) + return fmt.Errorf("running \"make %s\": %w", strings.Join(makeTargets, " "), err) } return nil } func commitConfigChanges(repoRootDir, msg string) error { - if _, err := gitInDir(repoRootDir, append([]string{"add"}, changedFiles...)...); err != nil { - return fmt.Errorf("error staging git changes: %w", err) + if out, err := gitInDir(repoRootDir, append([]string{"add"}, changedFiles...)...); err != nil { + logrus.Error(out) + return fmt.Errorf("staging git changes: %w", err) } - if _, err := git("commit", "-m", msg); err != nil { - return fmt.Errorf("error committing git changes: %w", err) + if out, err := git("commit", "-m", msg); err != nil { + logrus.Error(out) + return fmt.Errorf("committing git changes: %w", err) } return nil } diff --git a/hack/release/flags.go b/hack/release/flags.go index e94b54e7a5..710f2c4286 100644 --- a/hack/release/flags.go +++ b/hack/release/flags.go @@ -64,7 +64,7 @@ var ( } skipMilestoneFlag = &cli.BoolFlag{ Name: "skip-milestone", - Category: githubFlagCategory, + Category: developmentFlagCategory, Usage: "Skip updating GitHub milestones (development and testing purposes only)", Sources: cli.EnvVars("SKIP_MILESTONE"), Value: false, @@ -230,13 +230,6 @@ var localFlag = &cli.BoolFlag{ Value: false, } -var skipValidationFlag = &cli.BoolFlag{ - Name: "skip-validation", - Usage: "Skip various validation steps (development and testing purposes only)", - Sources: cli.EnvVars("SKIP_VALIDATION"), - Value: false, -} - // Flag Action to check value is a valid directory. func dirFlagCheck(_ context.Context, _ *cli.Command, path string) error { if path == "" { @@ -468,9 +461,28 @@ var ( } ) -var skipRepoCheckFlag = &cli.BoolFlag{ - Name: "skip-repo-check", - Usage: fmt.Sprintf("Skip checking that the git repository is %s (development and testing purposes only)", mainRepo), - Sources: cli.EnvVars("SKIP_REPO_CHECK"), - Value: false, -} +// General development and testing flags +var ( + developmentFlagCategory = "Development Options" + skipValidationFlag = &cli.BoolFlag{ + Name: "skip-validation", + Category: developmentFlagCategory, + Usage: "Skip various validation steps (development and testing purposes only)", + Sources: cli.EnvVars("SKIP_VALIDATION"), + Value: false, + } + skipRepoCheckFlag = &cli.BoolFlag{ + Name: "skip-repo-check", + Category: developmentFlagCategory, + Usage: fmt.Sprintf("Skip checking that the git repository is %s (development and testing purposes only)", mainRepo), + Sources: cli.EnvVars("SKIP_REPO_CHECK"), + Value: false, + } + skipBranchCheckFlag = &cli.BoolFlag{ + Name: "skip-branch-check", + Category: developmentFlagCategory, + Usage: "Skip checking that the current git branch is main (development and testing purposes only)", + Sources: cli.EnvVars("SKIP_BRANCH_CHECK"), + Value: false, + } +) diff --git a/hack/release/prep.go b/hack/release/prep.go index 38d4f6c793..85ca252ee0 100644 --- a/hack/release/prep.go +++ b/hack/release/prep.go @@ -33,14 +33,6 @@ var excludedComponentsPatterns = []string{ `^eck-.*`, } -var changedFiles = []string{ - calicoConfig, - enterpriseConfig, - "pkg/components", - "pkg/imports/crds", - "pkg/imports/admission", -} - // Command to prepare repo for a new release. var prepCommand = &cli.Command{ Name: "prep", @@ -62,6 +54,7 @@ to point to local repositories for Calico and Enterprise respectively.`, enterpriseRegistryFlag, skipValidationFlag, skipMilestoneFlag, + skipBranchCheckFlag, skipRepoCheckFlag, githubTokenFlag, localFlag, @@ -75,6 +68,7 @@ to point to local repositories for Calico and Enterprise respectively.`, // - check that at least one of calico or enterprise version is provided // - if calico version is not provided, check that the version in calico_versions.yml is a released version // - check that the provided calico and enterprise refs exist as a tag in the remote repository (if local directory not provided) +// - check that the base branch is a release branch (if not skipped) var validatePrepRefs = func(ctx context.Context, c *cli.Command) (context.Context, error) { // check that at least one of calico/enterprise version is set for prep ctx, err := checkAtLeastOneOfFlags(ctx, c, calicoVersionFlag.Name, enterpriseVersionFlag.Name) @@ -129,13 +123,33 @@ var validatePrepRefs = func(ctx context.Context, c *cli.Command) (context.Contex return ctx, fmt.Errorf("ref %q not found as a tag in %s", check.tag, check.repo) } } + + // check operator base branch is a release branch unless skipped + if c.Bool(skipBranchCheckFlag.Name) { + logrus.Warnf("Skipping branch validation as requested.") + return ctx, nil + } + baseBranch := ctx.Value(baseBranchCtxKey).(string) + releaseBranch, err := isReleaseBranch(c.String(releaseBranchPrefixFlag.Name), baseBranch) + if err != nil { + return ctx, fmt.Errorf("validating current branch: %w", err) + } + if !releaseBranch { + return ctx, fmt.Errorf("current branch is %s, please switch to %s or a release branch before running this command or use --%s to skip this check", baseBranch, defaultBaseBranch, skipBranchCheckFlag.Name) + } return ctx, nil } // prepContextValuesFunc sets context values for the prep command based on CLI flags. var prepContextValuesFunc = func(ctx context.Context, c *cli.Command) (context.Context, error) { + baseBranch, err := git("branch", "--show-current") + if err != nil { + return ctx, fmt.Errorf("getting current branch: %w", err) + } + ctx = context.WithValue(ctx, baseBranchCtxKey, baseBranch) + // Extract repo information from CLI repo flag into context - ctx, err := addRepoInfoToCtx(ctx, c.String(gitRepoFlag.Name)) + ctx, err = addRepoInfoToCtx(ctx, c.String(gitRepoFlag.Name)) if err != nil { return ctx, err } @@ -171,10 +185,7 @@ var prepBefore = cli.BeforeFunc(func(ctx context.Context, c *cli.Command) (conte // Action executed for release prep command. var prepAction = cli.ActionFunc(func(ctx context.Context, c *cli.Command) error { - baseBranch, err := git("branch", "--show-current") - if err != nil { - return fmt.Errorf("error getting current branch: %w", err) - } + baseBranch := ctx.Value(baseBranchCtxKey).(string) version := ctx.Value(versionCtxKey).(string) prepBranch := ctx.Value(branchNameCtxKey).(string) repoRootDir, err := branchActionCommon(ctx, c, fmt.Sprintf("build: %s release", version)) @@ -192,7 +203,8 @@ var prepAction = cli.ActionFunc(func(ctx context.Context, c *cli.Command) error // Push branch to remote gitRemote := c.String(gitRemoteFlag.Name) logrus.Debugf("Pushing branch %s to %s", prepBranch, gitRemote) - if _, err := git("push", "--force", "--set-upstream", gitRemote, prepBranch); err != nil { + if out, err := git("push", "--force", "--set-upstream", gitRemote, prepBranch); err != nil { + logrus.Error(out) return fmt.Errorf("error pushing branch %s to remote %s: %w", prepBranch, gitRemote, err) } From e2cc724b22a73ac593a887ef9b1a7f9780ba5505 Mon Sep 17 00:00:00 2001 From: tuti Date: Fri, 27 Mar 2026 17:49:59 -0700 Subject: [PATCH 4/4] fix(release): harden validation and add branch command tests - Add unit tests for isReleaseBranch, dev tag version logic, stream format validation, and refExistsInRemote - Replace bare context type assertions with contextString helper to produce clear errors instead of panics - Use regexp.QuoteMeta on release branch prefix to prevent regex injection from custom prefixes - Add stream format validation (vX.Y) in validateBranchRefs - Fix ls-remote substring matching: parse output lines and match exact ref names instead of using strings.Contains --- Makefile | 4 +- RELEASING.md | 52 ++++++----- hack/release/README.md | 24 +++--- hack/release/branch.go | 112 +++++++++++++++++++----- hack/release/branch_test.go | 168 ++++++++++++++++++++++++++++++++++++ hack/release/checks.go | 6 +- hack/release/flags.go | 12 +-- hack/release/prep.go | 35 ++++++-- hack/release/utils.go | 9 ++ 9 files changed, 346 insertions(+), 76 deletions(-) create mode 100644 hack/release/branch_test.go diff --git a/Makefile b/Makefile index e1c52d2dac..4e614636d8 100644 --- a/Makefile +++ b/Makefile @@ -630,8 +630,8 @@ endif release-prep: hack/bin/release hack/bin/gh var-require-all-VERSION var-require-one-of-CALICO_VERSION-ENTERPRISE_VERSION @REPO=$(REPO) hack/bin/release prep -create-release-branch: hack/bin/release var-require-all-STREAM-CALICO_REF-ENTERPRISE_REF - RELEASE_STREAM=$(STREAM) hack/bin/release branch +create-release-branch: hack/bin/release var-require-all-CALICO_REF-ENTERPRISE_REF var-require-one-of-STREAM-RELEASE_STREAM + hack/bin/release branch ############################################################################### # Utilities diff --git a/RELEASING.md b/RELEASING.md index c8242e2806..9f93fe2282 100644 --- a/RELEASING.md +++ b/RELEASING.md @@ -7,7 +7,7 @@ and a GitHub milestone for the next release. The `create-release-branch` Makefil the branch and dev tag; you will create the milestone manually in a later step: ```sh -make create-release-branch STREAM=vX.Y CALICO_REF= ENTERPRISE_REF= +make create-release-branch RELEASE_STREAM=vX.Y CALICO_REF= ENTERPRISE_REF= ``` This command: @@ -21,12 +21,11 @@ This command: **Flags / environment variables:** -| Env var | Flag | Description | -| -------------------------------------- | ------------------------- | ----------------------------------------------------------------- | -| `STREAM` / `RELEASE_STREAM` (required) | `--stream` | Release stream, e.g., `v1.43` | -| `CALICO_REF` (required) | `--calico-ref` | Calico git ref (branch or tag), e.g., `release-v3.32` | -| `ENTERPRISE_REF` (required) | `--enterprise-ref` | Enterprise git ref (branch or tag), e.g., `release-calient-v3.22` | -| `RELEASE_BRANCH_PREFIX` | `--release-branch-prefix` | Branch name prefix (default: `release`) | +| Env var | Flag | Description | +| -------------------------------------- | ------------------ | ----------------------------------------------------------------- | +| `STREAM` / `RELEASE_STREAM` (required) | `--stream` | Release stream, e.g., `v1.43` | +| `CALICO_REF` (required) | `--calico-ref` | Calico git ref (branch or tag), e.g., `release-v3.32` | +| `ENTERPRISE_REF` (required) | `--enterprise-ref` | Enterprise git ref (branch or tag), e.g., `release-calient-v3.22` | After the branch is created, create the next minor release's first milestone at https://github.com/tigera/operator/milestones (e.g., if `release-v1.43` was created, @@ -34,7 +33,7 @@ create milestone `v1.44.0`). ## Preparing for the release -Checkout the branch from which you want to release. Ensure that you are using the correct +Checkout the release branch from which you want to release. Ensure that you are using the correct operator version for the version of Calico or Enterprise that you are releasing. If in doubt, check [the releases page](https://github.com/tigera/operator/releases) to find the most recent Operator release for your Calico or Enterprise minor version. @@ -42,28 +41,35 @@ recent Operator release for your Calico or Enterprise minor version. Run the following command: ```sh -make release-prep VERSION= CALICO_VERSION= ENTERPRISE_VERSION= +make release-prep VERSION= [CALICO_VERSION=] [ENTERPRISE_VERSION=] ``` -The command does the following: +At least one of `CALICO_VERSION` or `ENTERPRISE_VERSION` must be provided. The versions must +exist as tags in their respective GitHub repositories. -- It updates the image version and the title field with the appropriate versions in the -format `vX.Y.Z` for each of the following files: - 1. `config/calico_versions.yml` (Calico OSS version) - 2. `config/enterprise_versions.yml` (Calico Enterprise version) +This command: -- It updates the registry reference to `quay.io` from `gcr.io` in the following files: +- Validates that the current branch is a release branch (e.g. `release-v1.43`) +- Validates that the provided Calico/Enterprise versions exist as tags in their remote repositories +- Updates `config/calico_versions.yml` and/or `config/enterprise_versions.yml` with the specified versions +- Updates the Enterprise registry if needed +- Runs `make fix gen-versions` to regenerate component files +- Commits the changes to a new `build-` branch +- Pushes the branch and creates a PR against the release branch +- Manages GitHub milestones for the release stream (creates next patch milestone, closes current) - 1. `TigeraRegistry` in `pkg/components/images.go` +**Flags / environment variables:** -- It ensures `make gen-versions` is run and the resulting updates committed. -- It creates a PR with all the changes -- It manages the milestones on GitHub for the release stream associated with the new release, - which involves creating a new milestone for the next patch version and closing the current milestone - for the release version. All open issues and pull requests associated with the current milestone - are moved to the new milestone. +| Env var | Flag | Description | +| --------------------- | ----------------------- | ------------------------------------------------------------- | +| `VERSION` (required) | `--version` | Operator version to release, e.g., `v1.43.2` | +| `CALICO_VERSION` | `--calico-version` | Calico version tag, e.g., `v3.30.2` | +| `ENTERPRISE_VERSION` | `--enterprise-version` | Enterprise version tag, e.g., `v3.22.0-1.0` | +| `ENTERPRISE_REGISTRY` | `--enterprise-registry` | Override Enterprise image registry | +| `CALICO_DIR` | `--calico-dir` | Local Calico CRDs directory (skips remote ref validation) | +| `ENTERPRISE_DIR` | `--enterprise-dir` | Local Enterprise CRDs directory (skips remote ref validation) | -Go to the PR created and it is reviewed and merged. +Once the PR is created, get it reviewed and merged. ## Releasing diff --git a/hack/release/README.md b/hack/release/README.md index 861fc449d9..6f17bedb84 100644 --- a/hack/release/README.md +++ b/hack/release/README.md @@ -150,32 +150,32 @@ release branch --stream --calico-ref --enterprise-ref If the `--local` flag is specified, the branch and tag are created locally without pushing to the remote. -| Flag | Env var | Description | -|------|---------|-------------| -| `--stream` | `RELEASE_STREAM` | Release stream (e.g., `v1.43`). Required. | -| `--calico-ref` | `CALICO_REF` | Calico git ref (branch or tag). Required. | -| `--enterprise-ref` | `ENTERPRISE_REF` | Enterprise git ref (branch or tag). Required. | -| `--release-branch-prefix` | `RELEASE_BRANCH_PREFIX` | Branch name prefix (default: `release`). | -| `--local` | `LOCAL` | Skip pushing to remote. | +| Flag | Env var | Description | +| ------------------------- | -------------------------- | --------------------------------------------- | +| `--stream` | `RELEASE_STREAM`/ `STREAM` | Release stream (e.g., `v1.43`). Required. | +| `--calico-ref` | `CALICO_REF` | Calico git ref (branch or tag). Required. | +| `--enterprise-ref` | `ENTERPRISE_REF` | Enterprise git ref (branch or tag). Required. | +| `--release-branch-prefix` | `RELEASE_BRANCH_PREFIX` | Branch name prefix (default: `release`). | +| `--local` | `LOCAL` | Skip pushing to remote. | There is also a Makefile target: ```sh -make create-release-branch STREAM=vX.Y CALICO_REF= ENTERPRISE_REF= +make create-release-branch RELEASE_STREAM=vX.Y CALICO_REF= ENTERPRISE_REF= ``` #### Examples -1. To create a release branch for operator v1.43 with Calico v3.32 and Enterprise v3.22 +1. To create a release branch for operator v1.42 with Calico v3.32 and Enterprise v3.22 ```sh - release branch --stream v1.43 --calico-ref release-v3.32 --enterprise-ref release-calient-v3.22 + release branch --stream v1.42 --calico-ref release-v3.32 --enterprise-ref release-calient-v3.22 ``` -1. To create a release branch locally without pushing +1. To perform the same action as above but only locally without pushing to the remote ```sh - release branch --stream v1.43 --calico-ref release-v3.32 --enterprise-ref release-calient-v3.22 --local + release branch --stream v1.42 --calico-ref release-v3.32 --enterprise-ref release-calient-v3.22 --local ``` ### release build diff --git a/hack/release/branch.go b/hack/release/branch.go index bba6c21852..11810f727b 100644 --- a/hack/release/branch.go +++ b/hack/release/branch.go @@ -38,6 +38,9 @@ var ( // releaseBranchFormat matches release branches with a version suffix (e.g. release-v1.2). releaseBranchFormat = `^(%s-v\d+\.\d+)$` + // streamFormat validates the stream flag value (e.g. v1.43). + streamFormat = `^v\d+\.\d+$` + defaultBaseBranch = "master" changedFiles = []string{ @@ -50,9 +53,13 @@ var ( ) var branchCommand = &cli.Command{ - Name: "branch", - Usage: "Create a new branch for the release", - Description: "This command creates a new branch for the release.", + Name: "branch", + Usage: "Create a new branch for the release", + Description: `The branch command creates a new branch for the release off of the current branch (which should be master or a release branch). + The new branch name is in the format - (e.g. release-v1.43). + + The config versions are updated based on the provided Calico and Enterprise refs, which should point to branches or tags in the respective repositories. + If the base branch is not a release branch, an empty commit and dev tag are also created on the base branch to allow for proper versioning of future commits on the base branch.`, Flags: []cli.Flag{ streamFlag, calicoRefFlag, @@ -87,11 +94,15 @@ func branchBeforeCommon(ctx context.Context, c *cli.Command, scopeContextFn func return ctx, err } - if baseBranch := ctx.Value(baseBranchCtxKey).(string); baseBranch != defaultBaseBranch { - logrus.WithFields(logrus.Fields{ - "base": baseBranch, - "expected": defaultBaseBranch, - }).Warn("Current branch is not the default base branch") + // Only warn about non-default base branch for the branch command; + // prep is expected to run from a release branch. + if c.Name == "branch" { + if baseBranch, ok := ctx.Value(baseBranchCtxKey).(string); ok && baseBranch != defaultBaseBranch { + logrus.WithFields(logrus.Fields{ + "base": baseBranch, + "expected": defaultBaseBranch, + }).Warn("Current branch is not the default base branch") + } } if c.Bool(skipValidationFlag.Name) { @@ -123,23 +134,67 @@ var branchContextValuesFunc = func(ctx context.Context, c *cli.Command) (context return ctx, nil } +var isValidStream = func(stream string) (bool, error) { + matched, err := regexp.MatchString(streamFormat, stream) + if err != nil { + return false, fmt.Errorf("validating stream format: %w", err) + } + return matched, nil +} + var isReleaseBranch = func(releaseBranchPrefix, branch string) (bool, error) { - branchRegex, err := regexp.Compile(fmt.Sprintf(releaseBranchFormat, releaseBranchPrefix)) + matched, err := regexp.MatchString(fmt.Sprintf(releaseBranchFormat, regexp.QuoteMeta(releaseBranchPrefix)), branch) if err != nil { - return false, fmt.Errorf("compiling release branch format regex: %w", err) + return false, fmt.Errorf("validating release branch format: %w", err) + } + return matched, nil +} + +// refExistsInRemote checks if a ref exists in the ls-remote output by matching the full ref name. +func refExistsInRemote(lsRemoteOutput, ref string) bool { + for _, line := range strings.Split(lsRemoteOutput, "\n") { + parts := strings.Fields(line) + if len(parts) < 2 { + continue + } + // ls-remote output format: \trefs/heads/ or refs/tags/ + remoteRef := parts[1] + // Strip known prefixes to get the full ref name (preserving slashes in ref names) + name := remoteRef + for _, prefix := range []string{"refs/heads/", "refs/tags/"} { + if trimmed, ok := strings.CutPrefix(remoteRef, prefix); ok { + name = trimmed + break + } + } + if name == ref { + return true + } } - return branchRegex.MatchString(branch), nil + return false } // validateBranchRefs validates that the required ref flags are set for branch creation. +// - check that the stream flag is in the correct format // - check that the operator branch does not already exist // - check that both calico and enterprise refs are provided // - check that the provided calico and enterprise refs exist as a branch or tag in the remote repository // - check that the base operator branch is either a release branch (or master) (if not skipping branch check) var validateBranchRefs = func(ctx context.Context, c *cli.Command) (context.Context, error) { + // check that the stream format is valid + stream := c.String(streamFlag.Name) + if valid, err := isValidStream(stream); err != nil { + return ctx, err + } else if !valid { + return ctx, fmt.Errorf("stream %q is not valid, expected format: vX.Y (e.g., v1.43)", stream) + } + // check that the operator branch does not already exist remote := c.String(gitRemoteFlag.Name) - branchName := ctx.Value(branchNameCtxKey).(string) + branchName, err := contextString(ctx, branchNameCtxKey) + if err != nil { + return ctx, err + } out, err := git("ls-remote", "--heads", remote, branchName) if err != nil { return ctx, fmt.Errorf("checking if branch %s exists in remote %s: %w", branchName, remote, err) @@ -168,23 +223,26 @@ var validateBranchRefs = func(ctx context.Context, c *cli.Command) (context.Cont if err != nil { return ctx, fmt.Errorf("checking if ref %q exists in %s: %w", check.ref, check.repo, err) } - if !strings.Contains(out, check.ref) { + if !refExistsInRemote(out, check.ref) { return ctx, fmt.Errorf("ref %q not found as a branch or tag in %s", check.ref, check.repo) } } // check operator base branch is either the default base branch or a release branch (if not skipping branch check) - currBranch := ctx.Value(baseBranchCtxKey).(string) + baseBranch, err := contextString(ctx, baseBranchCtxKey) + if err != nil { + return ctx, err + } if c.Bool(skipBranchCheckFlag.Name) { logrus.Warnf("Skipping branch validation as requested.") return ctx, nil } - releaseBranch, err := isReleaseBranch(c.String(releaseBranchPrefixFlag.Name), currBranch) + releaseBranch, err := isReleaseBranch(c.String(releaseBranchPrefixFlag.Name), baseBranch) if err != nil { return ctx, fmt.Errorf("validating current branch: %w", err) } - if currBranch != defaultBaseBranch && !releaseBranch { - return ctx, fmt.Errorf("current branch is %s, please switch to %s or a release branch before running this command or use --%s to skip this check", currBranch, defaultBaseBranch, skipBranchCheckFlag.Name) + if baseBranch != defaultBaseBranch && !releaseBranch { + return ctx, fmt.Errorf("current branch is %s, please switch to %s or a release branch before running this command or use --%s to skip this check", baseBranch, defaultBaseBranch, skipBranchCheckFlag.Name) } return ctx, nil @@ -199,8 +257,14 @@ var branchBefore = cli.BeforeFunc(func(ctx context.Context, c *cli.Command) (con var branchAction = cli.ActionFunc(func(ctx context.Context, c *cli.Command) error { stream := c.String(streamFlag.Name) remote := c.String(gitRemoteFlag.Name) - baseBranch := ctx.Value(baseBranchCtxKey).(string) - branchName := ctx.Value(branchNameCtxKey).(string) + baseBranch, err := contextString(ctx, baseBranchCtxKey) + if err != nil { + return err + } + branchName, err := contextString(ctx, branchNameCtxKey) + if err != nil { + return err + } refs := []string{branchName} if _, err := branchActionCommon(ctx, c, fmt.Sprintf("build: update config for %s", stream)); err != nil { @@ -279,7 +343,10 @@ var branchAfter = cli.AfterFunc(func(_ context.Context, _ *cli.Command) error { func switchBranch(ctx context.Context, branchName string) error { // get current branch to switch back to later - baseBranch := ctx.Value(baseBranchCtxKey).(string) + baseBranch, err := contextString(ctx, baseBranchCtxKey) + if err != nil { + return err + } branchCleanupFns = append(branchCleanupFns, func() { if out, err := git("switch", "-f", baseBranch); err != nil { logrus.Error(out) @@ -297,7 +364,10 @@ func switchBranch(ctx context.Context, branchName string) error { // It reads the branch name and calico/enterprise versions from context (set by Before functions). // It returns the repo root directory for subsequent operations. func branchActionCommon(ctx context.Context, c *cli.Command, commitMsg string) (string, error) { - branchName := ctx.Value(branchNameCtxKey).(string) + branchName, err := contextString(ctx, branchNameCtxKey) + if err != nil { + return "", err + } if err := switchBranch(ctx, branchName); err != nil { return "", err } diff --git a/hack/release/branch_test.go b/hack/release/branch_test.go new file mode 100644 index 0000000000..8a39b9c0e5 --- /dev/null +++ b/hack/release/branch_test.go @@ -0,0 +1,168 @@ +// Copyright (c) 2026 Tigera, Inc. All rights reserved. + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package main + +import ( + "fmt" + "strings" + "testing" + + "github.com/blang/semver/v4" +) + +func TestIsReleaseBranch(t *testing.T) { + t.Parallel() + tests := []struct { + prefix string + branch string + want bool + }{ + // Default prefix "release" + {"release", "release-v1.43", true}, + {"release", "release-v1.0", true}, + {"release", "release-v10.20", true}, + {"release", "master", false}, + {"release", "main", false}, + {"release", "release-v1.43.1", false}, + {"release", "release-v1", false}, + {"release", "release-1.43", false}, + {"release", "feature/release-v1.43", false}, + {"release", "release-v1.43-rc1", false}, + {"release", "", false}, + + // Custom prefix + {"rel", "rel-v1.43", true}, + {"rel", "release-v1.43", false}, + + // Prefix with regex metacharacters should be escaped + {"release.test", "release.test-v1.43", true}, + {"release.test", "releasextest-v1.43", false}, + } + + for _, tt := range tests { + t.Run(fmt.Sprintf("%s/%s", tt.prefix, tt.branch), func(t *testing.T) { + t.Parallel() + got, err := isReleaseBranch(tt.prefix, tt.branch) + if err != nil { + t.Fatalf("isReleaseBranch(%q, %q) unexpected error: %v", tt.prefix, tt.branch, err) + } + if got != tt.want { + t.Fatalf("isReleaseBranch(%q, %q) = %v, want %v", tt.prefix, tt.branch, got, tt.want) + } + }) + } +} + +func TestDevTagVersion(t *testing.T) { + t.Parallel() + tests := []struct { + stream string + devSuffix string + wantTag string + wantParseErr bool + }{ + {"v1.43", "0.dev", "v1.44.0-0.dev", false}, + {"v1.0", "0.dev", "v1.1.0-0.dev", false}, + {"v10.20", "0.dev", "v10.21.0-0.dev", false}, + {"v0.1", "0.dev", "v0.2.0-0.dev", false}, + {"invalid", "0.dev", "", true}, + {"v1", "0.dev", "", true}, + } + + for _, tt := range tests { + t.Run(tt.stream, func(t *testing.T) { + t.Parallel() + version, err := semver.Parse(fmt.Sprintf("%s.0", strings.TrimPrefix(tt.stream, "v"))) + if tt.wantParseErr { + if err == nil { + t.Fatalf("expected parse error for stream %q, got nil", tt.stream) + } + return + } + if err != nil { + t.Fatalf("unexpected parse error for stream %q: %v", tt.stream, err) + } + if err := version.IncrementMinor(); err != nil { + t.Fatalf("unexpected IncrementMinor error: %v", err) + } + got := fmt.Sprintf("v%s-%s", version.String(), tt.devSuffix) + if got != tt.wantTag { + t.Fatalf("dev tag for stream %q = %q, want %q", tt.stream, got, tt.wantTag) + } + }) + } +} + +func TestRefExistsInRemote(t *testing.T) { + t.Parallel() + // Simulated ls-remote output + lsRemoteOutput := "abc123\trefs/heads/release-v1.43\ndef456\trefs/tags/v3.30\nghi789\trefs/tags/v3.30^{}\njkl012\trefs/heads/release/v1.2" + + tests := []struct { + ref string + want bool + }{ + {"release-v1.43", true}, + {"v3.30", true}, + {"v3.3", false}, // should not match partial + {"v3.30^{}", true}, + {"release-v1.4", false}, + {"nonexistent", false}, + {"release/v1.2", true}, // ref with slash + {"v1.2", false}, // should not match partial of slashed ref + } + + for _, tt := range tests { + t.Run(tt.ref, func(t *testing.T) { + t.Parallel() + got := refExistsInRemote(lsRemoteOutput, tt.ref) + if got != tt.want { + t.Fatalf("refExistsInRemote(output, %q) = %v, want %v", tt.ref, got, tt.want) + } + }) + } +} + +func TestValidateStreamFormat(t *testing.T) { + t.Parallel() + tests := []struct { + stream string + want bool + }{ + {"v1.43", true}, + {"v1.0", true}, + {"v10.20", true}, + {"v0.1", true}, + {"1.43", false}, + {"v1.43.1", false}, + {"v1", false}, + {"vX.Y", false}, + {"master", false}, + {"", false}, + } + + for _, tt := range tests { + t.Run(fmt.Sprintf("%q", tt.stream), func(t *testing.T) { + t.Parallel() + got, err := isValidStream(tt.stream) + if err != nil { + t.Fatalf("isValidStreamFormat(%q) unexpected error: %v", tt.stream, err) + } + if got != tt.want { + t.Fatalf("isValidStreamFormat(%q) = %v, want %v", tt.stream, got, tt.want) + } + }) + } +} diff --git a/hack/release/checks.go b/hack/release/checks.go index 88fba2329e..1dd117832d 100644 --- a/hack/release/checks.go +++ b/hack/release/checks.go @@ -89,5 +89,9 @@ var checkAtLeastOneOfFlags = func(ctx context.Context, c *cli.Command, flagNames return ctx, nil } } - return ctx, fmt.Errorf("at least one of the following flags must be set: %s", strings.Join(flagNames, ", ")) + formatted := make([]string, len(flagNames)) + for i, name := range flagNames { + formatted[i] = "--" + name + } + return ctx, fmt.Errorf("at least one of the following flags must be set: %s", strings.Join(formatted, ", ")) } diff --git a/hack/release/flags.go b/hack/release/flags.go index 710f2c4286..a85c2c0cd8 100644 --- a/hack/release/flags.go +++ b/hack/release/flags.go @@ -139,17 +139,11 @@ var ( } streamFlag = &cli.StringFlag{ Name: "stream", + Aliases: []string{"release-stream"}, Category: operatorFlagCategory, Usage: "The release stream for the release branch name (e.g. vX.Y). The full branch name will be -", - Sources: cli.EnvVars("RELEASE_STREAM"), + Sources: cli.EnvVars("RELEASE_STREAM", "STREAM"), Required: true, - Action: func(ctx context.Context, c *cli.Command, s string) error { - // Validate stream is in the format vX.Y - if !regexp.MustCompile(`^v\d+\.\d+$`).MatchString(s) { - return fmt.Errorf("stream must be in the format vX.Y, got %q", s) - } - return nil - }, } releaseBranchPrefixFlag = &cli.StringFlag{ Name: "release-branch-prefix", @@ -481,7 +475,7 @@ var ( skipBranchCheckFlag = &cli.BoolFlag{ Name: "skip-branch-check", Category: developmentFlagCategory, - Usage: "Skip checking that the current git branch is main (development and testing purposes only)", + Usage: "Skip checking that the current git branch is master or a release branch (development and testing purposes only)", Sources: cli.EnvVars("SKIP_BRANCH_CHECK"), Value: false, } diff --git a/hack/release/prep.go b/hack/release/prep.go index 85ca252ee0..87b12c4ad7 100644 --- a/hack/release/prep.go +++ b/hack/release/prep.go @@ -45,6 +45,7 @@ Otherwise, use the environment variables "CALICO_CRDS_DIR" and "ENTERPRISE_CRDS_ to point to local repositories for Calico and Enterprise respectively.`, Flags: []cli.Flag{ versionFlag, + releaseBranchPrefixFlag, calicoVersionFlag, calicoDirFlag, calicoGitRepoFlag, @@ -119,7 +120,7 @@ var validatePrepRefs = func(ctx context.Context, c *cli.Command) (context.Contex if err != nil { return ctx, fmt.Errorf("checking if ref %q exists in %s: %w", check.tag, check.repo, err) } - if !strings.Contains(out, check.tag) { + if !refExistsInRemote(out, check.tag) { return ctx, fmt.Errorf("ref %q not found as a tag in %s", check.tag, check.repo) } } @@ -129,13 +130,16 @@ var validatePrepRefs = func(ctx context.Context, c *cli.Command) (context.Contex logrus.Warnf("Skipping branch validation as requested.") return ctx, nil } - baseBranch := ctx.Value(baseBranchCtxKey).(string) + baseBranch, err := contextString(ctx, baseBranchCtxKey) + if err != nil { + return ctx, err + } releaseBranch, err := isReleaseBranch(c.String(releaseBranchPrefixFlag.Name), baseBranch) if err != nil { return ctx, fmt.Errorf("validating current branch: %w", err) } if !releaseBranch { - return ctx, fmt.Errorf("current branch is %s, please switch to %s or a release branch before running this command or use --%s to skip this check", baseBranch, defaultBaseBranch, skipBranchCheckFlag.Name) + return ctx, fmt.Errorf("current branch %s is not a release branch", baseBranch) } return ctx, nil } @@ -185,9 +189,18 @@ var prepBefore = cli.BeforeFunc(func(ctx context.Context, c *cli.Command) (conte // Action executed for release prep command. var prepAction = cli.ActionFunc(func(ctx context.Context, c *cli.Command) error { - baseBranch := ctx.Value(baseBranchCtxKey).(string) - version := ctx.Value(versionCtxKey).(string) - prepBranch := ctx.Value(branchNameCtxKey).(string) + baseBranch, err := contextString(ctx, baseBranchCtxKey) + if err != nil { + return err + } + version, err := contextString(ctx, versionCtxKey) + if err != nil { + return err + } + prepBranch, err := contextString(ctx, branchNameCtxKey) + if err != nil { + return err + } repoRootDir, err := branchActionCommon(ctx, c, fmt.Sprintf("build: %s release", version)) if err != nil { return err @@ -215,8 +228,14 @@ var prepAction = cli.ActionFunc(func(ctx context.Context, c *cli.Command) error } githubUser := strings.Split(remoteURL[strings.Index(remoteURL, "git@github.com:")+len("git@github.com:"):strings.LastIndex(remoteURL, ".git")], "/")[0] - githubOrg := ctx.Value(githubOrgCtxKey).(string) - githubRepo := ctx.Value(githubRepoCtxKey).(string) + githubOrg, err := contextString(ctx, githubOrgCtxKey) + if err != nil { + return err + } + githubRepo, err := contextString(ctx, githubRepoCtxKey) + if err != nil { + return err + } headBranch := prepBranch if githubUser != githubOrg { // Forked repo, need to specify head as user:branch diff --git a/hack/release/utils.go b/hack/release/utils.go index 6651ac80a0..79263b0c79 100644 --- a/hack/release/utils.go +++ b/hack/release/utils.go @@ -68,6 +68,15 @@ const ( type contextKey string +// contextString extracts a string value from context, returning an error if the key is not set. +func contextString(ctx context.Context, key contextKey) (string, error) { + v, ok := ctx.Value(key).(string) + if !ok { + return "", fmt.Errorf("required context value %q not set", string(key)) + } + return v, nil +} + type Component struct { Version string `yaml:"version"` Image string `yaml:"image,omitempty"`