Make the default value of Repository's spec.git.branch field explicit

nokia · Aug 6, 2024 · 4889bb6 · 4889bb6
1 parent 23d62c2
commit 4889bb6
Show file tree

Hide file tree

Showing 3 changed files with 13 additions and 4 deletions.
diff --git a/api/porchconfig/v1alpha1/config.porch.kpt.dev_repositories.yaml b/api/porchconfig/v1alpha1/config.porch.kpt.dev_repositories.yaml
@@ -77,9 +77,11 @@ spec:
                   Ignored if `type` is not `git`.
                 properties:
                   branch:
+                    default: main
                     description: Name of the branch containing the packages. Finalized
                       packages will be committed to this branch (if the repository
                       allows write access). If unspecified, defaults to "main".
+                    minLength: 1
                     type: string
                   createBranch:
                     description: CreateBranch specifies if Porch should create the
@@ -176,9 +178,11 @@ spec:
                       Must be unspecified if `type` is not `git`.
                     properties:
                       branch:
+                        default: main
                         description: Name of the branch containing the packages. Finalized
                           packages will be committed to this branch (if the repository
                           allows write access). If unspecified, defaults to "main".
+                        minLength: 1
                         type: string
                       createBranch:
                         description: CreateBranch specifies if Porch should create

diff --git a/api/porchconfig/v1alpha1/types.go b/api/porchconfig/v1alpha1/types.go
@@ -92,6 +92,8 @@ type GitRepository struct {
 	// Address of the Git repository, for example:
 	//   `https://github.com/GoogleCloudPlatform/blueprints.git`
 	Repo string `json:"repo"`
+	// +kubebuilder:default=main
+	// +kubebuilder:validation:MinLength=1
 	// Name of the branch containing the packages. Finalized packages will be committed to this branch (if the repository allows write access). If unspecified, defaults to "main".
 	Branch string `json:"branch,omitempty"`
 	// CreateBranch specifies if Porch should create the package branch if it doesn't exist.

diff --git a/pkg/git/git.go b/pkg/git/git.go
@@ -121,6 +121,9 @@ func OpenRepository(ctx context.Context, name, namespace string, spec *configapi
 		return nil, fmt.Errorf("error cloning git repository %q, cannot create remote: %v", spec.Repo, err)
 	}
 
+	// NOTE: the spec.git.branch field in the Repository CRD (OpenAPI schema) defined with
+	//		 MinLength=1 validation and its default value is set to "main". This means that
+	// 		 it should never be empty at this point. The following code is left here as a last resort failsafe.
 	branch := MainBranch
 	if spec.Branch != "" {
 		branch = BranchName(spec.Branch)
@@ -140,8 +143,8 @@ func OpenRepository(ctx context.Context, name, namespace string, spec *configapi
 	}
 
 	if opts.UseGitCaBundle {
-		if caBundle, err := opts.CredentialResolver.ResolveCredential(ctx, namespace, namespace + "-ca-bundle"); err != nil {
-			klog.Errorf("failed to obtain caBundle from secret %s/%s: %v", namespace, namespace + "-ca-bundle", err)
+		if caBundle, err := opts.CredentialResolver.ResolveCredential(ctx, namespace, namespace+"-ca-bundle"); err != nil {
+			klog.Errorf("failed to obtain caBundle from secret %s/%s: %v", namespace, namespace+"-ca-bundle", err)
 		} else {
 			repository.caBundle = []byte(caBundle.ToString())
 		}
@@ -1095,8 +1098,8 @@ func (r *gitRepository) pushAndCleanup(ctx context.Context, ph *pushRefSpecBuild
 			Auth:              auth,
 			RequireRemoteRefs: require,
 			// TODO(justinsb): Need to ensure this is a compare-and-swap
-			Force: true,
-			CABundle:          r.caBundle,
+			Force:    true,
+			CABundle: r.caBundle,
 		})
 	}); err != nil {
 		return err