From 3de0c9366c70de290b7965b51c664173c988506e Mon Sep 17 00:00:00 2001 From: Henry Sachs Date: Thu, 15 Oct 2026 15:37:10 +0200 Subject: [PATCH] fix(project): skip defaultBranch late-init for empty repo GitLab reports default_branch for empty repos although the branch does not exist and rejects it on edit. Signed-off-by: Henry Sachs --- .../projects/projects/zz_project.go | 6 ++- .../projects/projects/zz_project_test.go | 48 +++++++++++++++++++ .../controller/projects/projects/project.go | 6 ++- .../projects/projects/project_test.go | 48 +++++++++++++++++++ 4 files changed, 106 insertions(+), 2 deletions(-) diff --git a/pkg/cluster/controller/projects/projects/zz_project.go b/pkg/cluster/controller/projects/projects/zz_project.go index 7824afe4..098ad2db 100644 --- a/pkg/cluster/controller/projects/projects/zz_project.go +++ b/pkg/cluster/controller/projects/projects/zz_project.go @@ -352,7 +352,11 @@ func (e *external) lateInitialize(ctx context.Context, cr *v1alpha1.Project, pro in.ContainerRegistryAccessLevel = clients.LateInitializeAccessControlValue(in.ContainerRegistryAccessLevel, project.ContainerRegistryAccessLevel) } - in.DefaultBranch = clients.LateInitializeStringPtr(in.DefaultBranch, project.DefaultBranch) + // GitLab reports a default branch for empty repositories although the + // branch does not exist yet, and rejects it on edit. + if !project.EmptyRepo { + in.DefaultBranch = clients.LateInitializeStringPtr(in.DefaultBranch, project.DefaultBranch) + } in.Description = clients.LateInitializeStringPtr(in.Description, project.Description) in.EnvironmentsAccessLevel = clients.LateInitializeAccessControlValue(in.EnvironmentsAccessLevel, project.EnvironmentsAccessLevel) in.FeatureFlagsAccessLevel = clients.LateInitializeAccessControlValue(in.FeatureFlagsAccessLevel, project.FeatureFlagsAccessLevel) diff --git a/pkg/cluster/controller/projects/projects/zz_project_test.go b/pkg/cluster/controller/projects/projects/zz_project_test.go index 3f397b3f..06406b0d 100644 --- a/pkg/cluster/controller/projects/projects/zz_project_test.go +++ b/pkg/cluster/controller/projects/projects/zz_project_test.go @@ -1652,6 +1652,54 @@ func TestLateInitializeSkipsMirrorFields(t *testing.T) { } } +// TestLateInitializeDefaultBranch guards against late-initializing the default +// branch GitLab reports for an empty repository, which does not exist yet and +// is rejected on edit. An explicitly set default branch is kept so GitLab's +// error still surfaces. +func TestLateInitializeDefaultBranch(t *testing.T) { + cases := map[string]struct { + spec *string + project *gitlab.Project + want *string + }{ + "EmptyRepoUnset": { + project: &gitlab.Project{DefaultBranch: "main", EmptyRepo: true}, + want: nil, + }, + "EmptyRepoExplicit": { + spec: ptr.To("develop"), + project: &gitlab.Project{DefaultBranch: "main", EmptyRepo: true}, + want: ptr.To("develop"), + }, + "NonEmptyRepoUnset": { + project: &gitlab.Project{DefaultBranch: "main"}, + want: ptr.To("main"), + }, + "NonEmptyRepoExplicit": { + spec: ptr.To("develop"), + project: &gitlab.Project{DefaultBranch: "main"}, + want: ptr.To("develop"), + }, + } + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + e := &external{client: &fake.MockClient{ + MockGetProjectPushRules: func(pid interface{}, options ...gitlab.RequestOptionFunc) (*gitlab.ProjectPushRules, *gitlab.Response, error) { + return nil, &gitlab.Response{Response: &http.Response{StatusCode: http.StatusNotFound}}, nil + }, + }} + cr := &v1alpha1.Project{Spec: v1alpha1.ProjectSpec{ForProvider: v1alpha1.ProjectParameters{DefaultBranch: tc.spec}}} + + if err := e.lateInitialize(context.Background(), cr, tc.project); err != nil { + t.Fatalf("lateInitialize returned error: %v", err) + } + if diff := cmp.Diff(tc.want, cr.Spec.ForProvider.DefaultBranch); diff != "" { + t.Errorf("DefaultBranch: -want, +got:\n%s", diff) + } + }) + } +} + // TestLateInitializeProjectFeatureAccessLevels verifies that each feature // access level is late-initialized from its own observed field, and that // packageRegistryAccessLevel is not, because it takes precedence over diff --git a/pkg/namespaced/controller/projects/projects/project.go b/pkg/namespaced/controller/projects/projects/project.go index 83e5d1ae..8950717b 100644 --- a/pkg/namespaced/controller/projects/projects/project.go +++ b/pkg/namespaced/controller/projects/projects/project.go @@ -350,7 +350,11 @@ func (e *external) lateInitialize(ctx context.Context, cr *v1alpha1.Project, pro in.ContainerRegistryAccessLevel = clients.LateInitializeAccessControlValue(in.ContainerRegistryAccessLevel, project.ContainerRegistryAccessLevel) } - in.DefaultBranch = clients.LateInitializeStringPtr(in.DefaultBranch, project.DefaultBranch) + // GitLab reports a default branch for empty repositories although the + // branch does not exist yet, and rejects it on edit. + if !project.EmptyRepo { + in.DefaultBranch = clients.LateInitializeStringPtr(in.DefaultBranch, project.DefaultBranch) + } in.Description = clients.LateInitializeStringPtr(in.Description, project.Description) in.EnvironmentsAccessLevel = clients.LateInitializeAccessControlValue(in.EnvironmentsAccessLevel, project.EnvironmentsAccessLevel) in.FeatureFlagsAccessLevel = clients.LateInitializeAccessControlValue(in.FeatureFlagsAccessLevel, project.FeatureFlagsAccessLevel) diff --git a/pkg/namespaced/controller/projects/projects/project_test.go b/pkg/namespaced/controller/projects/projects/project_test.go index 8e57fb6c..3c72112f 100644 --- a/pkg/namespaced/controller/projects/projects/project_test.go +++ b/pkg/namespaced/controller/projects/projects/project_test.go @@ -1650,6 +1650,54 @@ func TestLateInitializeSkipsMirrorFields(t *testing.T) { } } +// TestLateInitializeDefaultBranch guards against late-initializing the default +// branch GitLab reports for an empty repository, which does not exist yet and +// is rejected on edit. An explicitly set default branch is kept so GitLab's +// error still surfaces. +func TestLateInitializeDefaultBranch(t *testing.T) { + cases := map[string]struct { + spec *string + project *gitlab.Project + want *string + }{ + "EmptyRepoUnset": { + project: &gitlab.Project{DefaultBranch: "main", EmptyRepo: true}, + want: nil, + }, + "EmptyRepoExplicit": { + spec: ptr.To("develop"), + project: &gitlab.Project{DefaultBranch: "main", EmptyRepo: true}, + want: ptr.To("develop"), + }, + "NonEmptyRepoUnset": { + project: &gitlab.Project{DefaultBranch: "main"}, + want: ptr.To("main"), + }, + "NonEmptyRepoExplicit": { + spec: ptr.To("develop"), + project: &gitlab.Project{DefaultBranch: "main"}, + want: ptr.To("develop"), + }, + } + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + e := &external{client: &fake.MockClient{ + MockGetProjectPushRules: func(pid interface{}, options ...gitlab.RequestOptionFunc) (*gitlab.ProjectPushRules, *gitlab.Response, error) { + return nil, &gitlab.Response{Response: &http.Response{StatusCode: http.StatusNotFound}}, nil + }, + }} + cr := &v1alpha1.Project{Spec: v1alpha1.ProjectSpec{ForProvider: v1alpha1.ProjectParameters{DefaultBranch: tc.spec}}} + + if err := e.lateInitialize(context.Background(), cr, tc.project); err != nil { + t.Fatalf("lateInitialize returned error: %v", err) + } + if diff := cmp.Diff(tc.want, cr.Spec.ForProvider.DefaultBranch); diff != "" { + t.Errorf("DefaultBranch: -want, +got:\n%s", diff) + } + }) + } +} + // TestLateInitializeProjectFeatureAccessLevels verifies that each feature // access level is late-initialized from its own observed field, and that // packageRegistryAccessLevel is not, because it takes precedence over