[Prototyping] Ensure git branch exists in write operations for github repositories (#96998)

* Create branch when ref is passed

* Change approach to ensure branch already exists

* Fix error handling branch not found

pkg/registry/apis/provisioning/repository/github.go

@@ -120,6 +120,14 @@ func (r *githubRepository) Read(ctx context.Context, logger *slog.Logger, filePa
 }
 
 func (r *githubRepository) Create(ctx context.Context, logger *slog.Logger, path, ref string, data []byte, comment string) error {
+	if ref == "" {
+		ref = r.config.Spec.GitHub.Branch
+	}
+
+	if err := r.ensureBranchExists(ctx, ref); err != nil {
+		return fmt.Errorf("create branch on create: %w", err)
+	}
+
 	owner := r.config.Spec.GitHub.Owner
 	repo := r.config.Spec.GitHub.Repository
 
@@ -136,6 +144,14 @@ func (r *githubRepository) Create(ctx context.Context, logger *slog.Logger, path
 }
 
 func (r *githubRepository) Update(ctx context.Context, logger *slog.Logger, path, ref string, data []byte, comment string) error {
+	if ref == "" {
+		ref = r.config.Spec.GitHub.Branch
+	}
+
+	if err := r.ensureBranchExists(ctx, ref); err != nil {
+		return fmt.Errorf("create branch on update: %w", err)
+	}
+
 	owner := r.config.Spec.GitHub.Owner
 	repo := r.config.Spec.GitHub.Repository
 
@@ -160,6 +176,14 @@ func (r *githubRepository) Update(ctx context.Context, logger *slog.Logger, path
 }
 
 func (r *githubRepository) Delete(ctx context.Context, logger *slog.Logger, path, ref, comment string) error {
+	if ref == "" {
+		ref = r.config.Spec.GitHub.Branch
+	}
+
+	if err := r.ensureBranchExists(ctx, ref); err != nil {
+		return fmt.Errorf("create branch on delete: %w", err)
+	}
+
 	owner := r.config.Spec.GitHub.Owner
 	repo := r.config.Spec.GitHub.Repository
 
@@ -179,6 +203,53 @@ func (r *githubRepository) Delete(ctx context.Context, logger *slog.Logger, path
 	return r.gh.DeleteFile(ctx, owner, repo, path, ref, comment, file.GetSHA())
 }
 
+// isValidGitBranchName checks if a branch name is valid.
+func isValidGitBranchName(branch string) bool {
+	// TODO: add branch validation here as well as in the frontend using the same restrictions
+	return branch != ""
+}
+
+func (r *githubRepository) ensureBranchExists(ctx context.Context, branchName string) error {
+	if !isValidGitBranchName(branchName) {
+		return &apierrors.StatusError{
+			ErrStatus: metav1.Status{
+				Code:    http.StatusBadRequest,
+				Message: "invalid branch name",
+			},
+		}
+	}
+
+	ok, err := r.gh.BranchExists(ctx, r.config.Spec.GitHub.Owner, r.config.Spec.GitHub.Repository, branchName)
+	if err != nil {
+		return fmt.Errorf("check branch exists: %w", err)
+	}
+
+	if ok {
+		r.logger.InfoContext(ctx, "branch already exists", "branch", branchName)
+
+		return nil
+	}
+
+	owner := r.config.Spec.GitHub.Owner
+	repo := r.config.Spec.GitHub.Repository
+
+	srcBranch := r.config.Spec.GitHub.Branch
+	if err := r.gh.CreateBranch(ctx, owner, repo, srcBranch, branchName); err != nil {
+		if errors.Is(err, pgh.ErrResourceAlreadyExists) {
+			return &apierrors.StatusError{
+				ErrStatus: metav1.Status{
+					Code:    http.StatusConflict,
+					Message: "branch already exists",
+				},
+			}
+		}
+
+		return fmt.Errorf("create branch: %w", err)
+	}
+
+	return nil
+}
+
 // Webhook implements provisioning.Repository.
 func (r *githubRepository) Webhook(ctx context.Context, logger *slog.Logger, responder rest.Responder) http.HandlerFunc {
 	return func(w http.ResponseWriter, req *http.Request) {

pkg/registry/apis/provisioning/repository/github/client.go

@@ -59,6 +59,11 @@ type Client interface {
 	// If ".." appears in the "path", this method will return an error.
 	DeleteFile(ctx context.Context, owner, repository, path, branch, message, hash string) error
 
+	// CreateBranch creates a new branch in the repository.
+	CreateBranch(ctx context.Context, owner, repository, sourceBranch, branchName string) error
+	// BranchExists checks if a branch exists in the repository.
+	BranchExists(ctx context.Context, owner, repository, branchName string) (bool, error)
+
 	ListWebhooks(ctx context.Context, owner, repository string) ([]WebhookConfig, error)
 	CreateWebhook(ctx context.Context, owner, repository string, cfg WebhookConfig) error
 	DeleteWebhook(ctx context.Context, owner, repository string, webhookID int64) error

pkg/registry/apis/provisioning/repository/github/client_mock.go

@@ -13,6 +13,44 @@ type MockClient struct {
 	mock.Mock
 }
 
+// CreateBranch provides a mock function with given fields: ctx, owner, repository, sourceBranch, branchName
+func (_m *MockClient) CreateBranch(ctx context.Context, owner string, repository string, sourceBranch string, branchName string) error {
+	ret := _m.Called(ctx, owner, repository, sourceBranch, branchName)
+
+	if len(ret) == 0 {
+		panic("no return value specified for CreateBranch")
+	}
+
+	var r0 error
+	if rf, ok := ret.Get(0).(func(context.Context, string, string, string, string) error); ok {
+		r0 = rf(ctx, owner, repository, sourceBranch, branchName)
+	} else {
+		r0 = ret.Error(0)
+	}
+
+	return r0
+}
+
+func (_m *MockClient) BranchExists(ctx context.Context, owner string, repository string, branchName string) (bool, error) {
+	ret := _m.Called(ctx, owner, repository, branchName)
+	if len(ret) == 0 {
+		panic("no return value specified for BranchExists")
+	}
+	var r0 bool
+	if rf, ok := ret.Get(0).(func(context.Context, string, string, string) bool); ok {
+		r0 = rf(ctx, owner, repository, branchName)
+	} else {
+		r0 = ret.Bool(0)
+	}
+	var r1 error
+	if rf, ok := ret.Get(1).(func(context.Context, string, string, string) error); ok {
+		r1 = rf(ctx, owner, repository, branchName)
+	} else {
+		r1 = ret.Error(1)
+	}
+	return r0, r1
+}
+
 // CreateFile provides a mock function with given fields: ctx, owner, repository, path, branch, message, content
 func (_m *MockClient) CreateFile(ctx context.Context, owner string, repository string, path string, branch string, message string, content []byte) error {
 	ret := _m.Called(ctx, owner, repository, path, branch, message, content)
@@ -195,7 +233,8 @@ func (_m *MockClient) UpdateFile(ctx context.Context, owner string, repository s
 func NewMockClient(t interface {
 	mock.TestingT
 	Cleanup(func())
-}) *MockClient {
+},
+) *MockClient {
 	mock := &MockClient{}
 	mock.Mock.Test(t)
 

pkg/registry/apis/provisioning/repository/github/real.go

@@ -148,6 +148,43 @@ func (r *realImpl) DeleteFile(ctx context.Context, owner, repository, path, bran
 	return err
 }
 
+func (r *realImpl) CreateBranch(ctx context.Context, owner, repository, sourceBranch, branchName string) error {
+	// Fail if the branch already exists
+	if _, _, err := r.gh.Repositories.GetBranch(ctx, owner, repository, branchName, 0); err == nil {
+		return ErrResourceAlreadyExists
+	}
+
+	// Branch out based on the repository branch
+	baseRef, _, err := r.gh.Repositories.GetBranch(ctx, owner, repository, sourceBranch, 0)
+	if err != nil {
+		return fmt.Errorf("get base branch: %w", err)
+	}
+
+	if _, _, err := r.gh.Git.CreateRef(ctx, owner, repository, &github.Reference{
+		Ref: github.String(fmt.Sprintf("refs/heads/%s", branchName)),
+		Object: &github.GitObject{
+			SHA: baseRef.Commit.SHA,
+		},
+	}); err != nil {
+		return fmt.Errorf("create branch ref: %w", err)
+	}
+
+	return nil
+}
+
+func (r *realImpl) BranchExists(ctx context.Context, owner, repository, branchName string) (bool, error) {
+	_, resp, err := r.gh.Repositories.GetBranch(ctx, owner, repository, branchName, 0)
+	if err == nil {
+		return true, nil
+	}
+
+	if resp.StatusCode == http.StatusNotFound {
+		return false, nil
+	}
+
+	return false, err
+}
+
 func (r *realImpl) ListWebhooks(ctx context.Context, owner, repository string) ([]WebhookConfig, error) {
 	hooks, _, err := r.gh.Repositories.ListHooks(ctx, owner, repository, nil)
 	if err != nil {

pkg/registry/apis/provisioning/repository/local.go

@@ -152,6 +152,10 @@ func (r *localRepository) Read(ctx context.Context, logger *slog.Logger, path st
 }
 
 func (r *localRepository) Create(ctx context.Context, logger *slog.Logger, path string, ref string, data []byte, comment string) error {
+	if ref != "" {
+		return apierrors.NewBadRequest("local repository does not support ref")
+	}
+
 	if r.path == "" {
 		return &apierrors.StatusError{
 			ErrStatus: metav1.Status{
@@ -172,6 +176,10 @@ func (r *localRepository) Create(ctx context.Context, logger *slog.Logger, path
 }
 
 func (r *localRepository) Update(ctx context.Context, logger *slog.Logger, path string, ref string, data []byte, comment string) error {
+	if ref != "" {
+		return apierrors.NewBadRequest("local repository does not support ref")
+	}
+
 	if r.path == "" {
 		return &apierrors.StatusError{
 			ErrStatus: metav1.Status{
@@ -189,6 +197,10 @@ func (r *localRepository) Update(ctx context.Context, logger *slog.Logger, path
 }
 
 func (r *localRepository) Delete(ctx context.Context, logger *slog.Logger, path string, ref string, comment string) error {
+	if ref != "" {
+		return apierrors.NewBadRequest("local repository does not support ref")
+	}
+
 	if r.path == "" {
 		return &apierrors.StatusError{
 			ErrStatus: metav1.Status{