diff --git a/apis/cluster/projects/v1alpha1/zz_generated.deepcopy.go b/apis/cluster/projects/v1alpha1/zz_generated.deepcopy.go index 60e2ae7b..471e4768 100644 --- a/apis/cluster/projects/v1alpha1/zz_generated.deepcopy.go +++ b/apis/cluster/projects/v1alpha1/zz_generated.deepcopy.go @@ -403,6 +403,51 @@ func (in *ApprovalRuleStatus) DeepCopy() *ApprovalRuleStatus { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *Approvals) DeepCopyInto(out *Approvals) { + *out = *in + if in.ResetApprovalsOnPush != nil { + in, out := &in.ResetApprovalsOnPush, &out.ResetApprovalsOnPush + *out = new(bool) + **out = **in + } + if in.DisableOverridingApproversPerMergeRequest != nil { + in, out := &in.DisableOverridingApproversPerMergeRequest, &out.DisableOverridingApproversPerMergeRequest + *out = new(bool) + **out = **in + } + if in.MergeRequestsAuthorApproval != nil { + in, out := &in.MergeRequestsAuthorApproval, &out.MergeRequestsAuthorApproval + *out = new(bool) + **out = **in + } + if in.MergeRequestsDisableCommittersApproval != nil { + in, out := &in.MergeRequestsDisableCommittersApproval, &out.MergeRequestsDisableCommittersApproval + *out = new(bool) + **out = **in + } + if in.RequireReauthenticationToApprove != nil { + in, out := &in.RequireReauthenticationToApprove, &out.RequireReauthenticationToApprove + *out = new(bool) + **out = **in + } + if in.SelectiveCodeOwnerRemovals != nil { + in, out := &in.SelectiveCodeOwnerRemovals, &out.SelectiveCodeOwnerRemovals + *out = new(bool) + **out = **in + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Approvals. +func (in *Approvals) DeepCopy() *Approvals { + if in == nil { + return nil + } + out := new(Approvals) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Badge) DeepCopyInto(out *Badge) { *out = *in @@ -2512,6 +2557,11 @@ func (in *ProjectParameters) DeepCopyInto(out *ProjectParameters) { *out = new(PushRules) (*in).DeepCopyInto(*out) } + if in.Approvals != nil { + in, out := &in.Approvals, &out.Approvals + *out = new(Approvals) + (*in).DeepCopyInto(*out) + } if in.RemoveSourceBranchAfterMerge != nil { in, out := &in.RemoveSourceBranchAfterMerge, &out.RemoveSourceBranchAfterMerge *out = new(bool) diff --git a/apis/cluster/projects/v1alpha1/zz_project_types.go b/apis/cluster/projects/v1alpha1/zz_project_types.go index 87da8d60..f90397ac 100644 --- a/apis/cluster/projects/v1alpha1/zz_project_types.go +++ b/apis/cluster/projects/v1alpha1/zz_project_types.go @@ -388,6 +388,14 @@ type ProjectParameters struct { // +optional PushRules *PushRules `json:"pushRules,omitempty"` + // Approvals configures the project's merge request approval + // configuration. Fields left unset are not managed by this provider. + // + // GitLab API docs: + // https://docs.gitlab.com/api/merge_request_approvals/#update-approval-configuration-for-a-project + // +optional + Approvals *Approvals `json:"approvals,omitempty"` + // Enable Delete source branch option by default for all new merge requests. // +optional RemoveSourceBranchAfterMerge *bool `json:"removeSourceBranchAfterMerge,omitempty"` @@ -511,6 +519,41 @@ type PushRules struct { RejectNonDCOCommits *bool `json:"rejectNonDcoCommits,omitempty"` } +// Approvals configures a project's merge request approval configuration. +// +// GitLab API docs: +// https://docs.gitlab.com/api/merge_request_approvals/#update-approval-configuration-for-a-project +type Approvals struct { + // ResetApprovalsOnPush resets approvals on a new push to the merge request. + // +optional + ResetApprovalsOnPush *bool `json:"resetApprovalsOnPush,omitempty"` + + // DisableOverridingApproversPerMergeRequest prevents users from overriding + // approvers per merge request. + // +optional + DisableOverridingApproversPerMergeRequest *bool `json:"disableOverridingApproversPerMergeRequest,omitempty"` + + // MergeRequestsAuthorApproval allows merge request authors to self-approve + // their own merge requests. + // +optional + MergeRequestsAuthorApproval *bool `json:"mergeRequestsAuthorApproval,omitempty"` + + // MergeRequestsDisableCommittersApproval prevents committers from + // approving their own merge requests. + // +optional + MergeRequestsDisableCommittersApproval *bool `json:"mergeRequestsDisableCommittersApproval,omitempty"` + + // RequireReauthenticationToApprove requires the reauthentication password + // to approve merge requests. + // +optional + RequireReauthenticationToApprove *bool `json:"requireReauthenticationToApprove,omitempty"` + + // SelectiveCodeOwnerRemovals allows removing individual code owner + // approval requirements for a merge request. + // +optional + SelectiveCodeOwnerRemovals *bool `json:"selectiveCodeOwnerRemovals,omitempty"` +} + // ProjectNamespace represents a project namespace. type ProjectNamespace struct { ID int64 `json:"ID"` diff --git a/apis/namespaced/projects/v1alpha1/project_types.go b/apis/namespaced/projects/v1alpha1/project_types.go index 293f2366..70083e88 100644 --- a/apis/namespaced/projects/v1alpha1/project_types.go +++ b/apis/namespaced/projects/v1alpha1/project_types.go @@ -386,6 +386,14 @@ type ProjectParameters struct { // +optional PushRules *PushRules `json:"pushRules,omitempty"` + // Approvals configures the project's merge request approval + // configuration. Fields left unset are not managed by this provider. + // + // GitLab API docs: + // https://docs.gitlab.com/api/merge_request_approvals/#update-approval-configuration-for-a-project + // +optional + Approvals *Approvals `json:"approvals,omitempty"` + // Enable Delete source branch option by default for all new merge requests. // +optional RemoveSourceBranchAfterMerge *bool `json:"removeSourceBranchAfterMerge,omitempty"` @@ -509,6 +517,41 @@ type PushRules struct { RejectNonDCOCommits *bool `json:"rejectNonDcoCommits,omitempty"` } +// Approvals configures a project's merge request approval configuration. +// +// GitLab API docs: +// https://docs.gitlab.com/api/merge_request_approvals/#update-approval-configuration-for-a-project +type Approvals struct { + // ResetApprovalsOnPush resets approvals on a new push to the merge request. + // +optional + ResetApprovalsOnPush *bool `json:"resetApprovalsOnPush,omitempty"` + + // DisableOverridingApproversPerMergeRequest prevents users from overriding + // approvers per merge request. + // +optional + DisableOverridingApproversPerMergeRequest *bool `json:"disableOverridingApproversPerMergeRequest,omitempty"` + + // MergeRequestsAuthorApproval allows merge request authors to self-approve + // their own merge requests. + // +optional + MergeRequestsAuthorApproval *bool `json:"mergeRequestsAuthorApproval,omitempty"` + + // MergeRequestsDisableCommittersApproval prevents committers from + // approving their own merge requests. + // +optional + MergeRequestsDisableCommittersApproval *bool `json:"mergeRequestsDisableCommittersApproval,omitempty"` + + // RequireReauthenticationToApprove requires the reauthentication password + // to approve merge requests. + // +optional + RequireReauthenticationToApprove *bool `json:"requireReauthenticationToApprove,omitempty"` + + // SelectiveCodeOwnerRemovals allows removing individual code owner + // approval requirements for a merge request. + // +optional + SelectiveCodeOwnerRemovals *bool `json:"selectiveCodeOwnerRemovals,omitempty"` +} + // ProjectNamespace represents a project namespace. type ProjectNamespace struct { ID int64 `json:"ID"` diff --git a/apis/namespaced/projects/v1alpha1/zz_generated.deepcopy.go b/apis/namespaced/projects/v1alpha1/zz_generated.deepcopy.go index eb8c61cd..b128ee61 100644 --- a/apis/namespaced/projects/v1alpha1/zz_generated.deepcopy.go +++ b/apis/namespaced/projects/v1alpha1/zz_generated.deepcopy.go @@ -403,6 +403,51 @@ func (in *ApprovalRuleStatus) DeepCopy() *ApprovalRuleStatus { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *Approvals) DeepCopyInto(out *Approvals) { + *out = *in + if in.ResetApprovalsOnPush != nil { + in, out := &in.ResetApprovalsOnPush, &out.ResetApprovalsOnPush + *out = new(bool) + **out = **in + } + if in.DisableOverridingApproversPerMergeRequest != nil { + in, out := &in.DisableOverridingApproversPerMergeRequest, &out.DisableOverridingApproversPerMergeRequest + *out = new(bool) + **out = **in + } + if in.MergeRequestsAuthorApproval != nil { + in, out := &in.MergeRequestsAuthorApproval, &out.MergeRequestsAuthorApproval + *out = new(bool) + **out = **in + } + if in.MergeRequestsDisableCommittersApproval != nil { + in, out := &in.MergeRequestsDisableCommittersApproval, &out.MergeRequestsDisableCommittersApproval + *out = new(bool) + **out = **in + } + if in.RequireReauthenticationToApprove != nil { + in, out := &in.RequireReauthenticationToApprove, &out.RequireReauthenticationToApprove + *out = new(bool) + **out = **in + } + if in.SelectiveCodeOwnerRemovals != nil { + in, out := &in.SelectiveCodeOwnerRemovals, &out.SelectiveCodeOwnerRemovals + *out = new(bool) + **out = **in + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Approvals. +func (in *Approvals) DeepCopy() *Approvals { + if in == nil { + return nil + } + out := new(Approvals) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Badge) DeepCopyInto(out *Badge) { *out = *in @@ -2512,6 +2557,11 @@ func (in *ProjectParameters) DeepCopyInto(out *ProjectParameters) { *out = new(PushRules) (*in).DeepCopyInto(*out) } + if in.Approvals != nil { + in, out := &in.Approvals, &out.Approvals + *out = new(Approvals) + (*in).DeepCopyInto(*out) + } if in.RemoveSourceBranchAfterMerge != nil { in, out := &in.RemoveSourceBranchAfterMerge, &out.RemoveSourceBranchAfterMerge *out = new(bool) diff --git a/examples/projects/project.yaml b/examples/projects/project.yaml index 5068ec69..44cdc56d 100644 --- a/examples/projects/project.yaml +++ b/examples/projects/project.yaml @@ -11,6 +11,15 @@ spec: name: example-group description: "example project description" buildGitStrategy: "fetch" + # Merge request approval configuration for the project. Fields left + # unset here are not managed by this provider. + approvals: + resetApprovalsOnPush: true + disableOverridingApproversPerMergeRequest: false + mergeRequestsAuthorApproval: false + mergeRequestsDisableCommittersApproval: true + requireReauthenticationToApprove: true + selectiveCodeOwnerRemovals: false providerConfigRef: name: gitlab-provider kind: ProviderConfig diff --git a/package/crds/projects.gitlab.crossplane.io_projects.yaml b/package/crds/projects.gitlab.crossplane.io_projects.yaml index a49f94de..42a200fa 100644 --- a/package/crds/projects.gitlab.crossplane.io_projects.yaml +++ b/package/crds/projects.gitlab.crossplane.io_projects.yaml @@ -78,6 +78,44 @@ spec: description: Set whether or not merge requests can be merged with skipped jobs. type: boolean + approvals: + description: |- + Approvals configures the project's merge request approval + configuration. Fields left unset are not managed by this provider. + + GitLab API docs: + https://docs.gitlab.com/api/merge_request_approvals/#update-approval-configuration-for-a-project + properties: + disableOverridingApproversPerMergeRequest: + description: |- + DisableOverridingApproversPerMergeRequest prevents users from overriding + approvers per merge request. + type: boolean + mergeRequestsAuthorApproval: + description: |- + MergeRequestsAuthorApproval allows merge request authors to self-approve + their own merge requests. + type: boolean + mergeRequestsDisableCommittersApproval: + description: |- + MergeRequestsDisableCommittersApproval prevents committers from + approving their own merge requests. + type: boolean + requireReauthenticationToApprove: + description: |- + RequireReauthenticationToApprove requires the reauthentication password + to approve merge requests. + type: boolean + resetApprovalsOnPush: + description: ResetApprovalsOnPush resets approvals on a new + push to the merge request. + type: boolean + selectiveCodeOwnerRemovals: + description: |- + SelectiveCodeOwnerRemovals allows removing individual code owner + approval requirements for a merge request. + type: boolean + type: object approvalsBeforeMerge: description: |- How many approvers should approve merge request by default.More actions diff --git a/package/crds/projects.gitlab.m.crossplane.io_projects.yaml b/package/crds/projects.gitlab.m.crossplane.io_projects.yaml index 9ff6aba7..70d610d3 100644 --- a/package/crds/projects.gitlab.m.crossplane.io_projects.yaml +++ b/package/crds/projects.gitlab.m.crossplane.io_projects.yaml @@ -64,6 +64,44 @@ spec: description: Set whether or not merge requests can be merged with skipped jobs. type: boolean + approvals: + description: |- + Approvals configures the project's merge request approval + configuration. Fields left unset are not managed by this provider. + + GitLab API docs: + https://docs.gitlab.com/api/merge_request_approvals/#update-approval-configuration-for-a-project + properties: + disableOverridingApproversPerMergeRequest: + description: |- + DisableOverridingApproversPerMergeRequest prevents users from overriding + approvers per merge request. + type: boolean + mergeRequestsAuthorApproval: + description: |- + MergeRequestsAuthorApproval allows merge request authors to self-approve + their own merge requests. + type: boolean + mergeRequestsDisableCommittersApproval: + description: |- + MergeRequestsDisableCommittersApproval prevents committers from + approving their own merge requests. + type: boolean + requireReauthenticationToApprove: + description: |- + RequireReauthenticationToApprove requires the reauthentication password + to approve merge requests. + type: boolean + resetApprovalsOnPush: + description: ResetApprovalsOnPush resets approvals on a new + push to the merge request. + type: boolean + selectiveCodeOwnerRemovals: + description: |- + SelectiveCodeOwnerRemovals allows removing individual code owner + approval requirements for a merge request. + type: boolean + type: object approvalsBeforeMerge: description: |- How many approvers should approve merge request by default.More actions diff --git a/pkg/cluster/clients/projects/fake/zz_fake.go b/pkg/cluster/clients/projects/fake/zz_fake.go index 5624388b..28215226 100644 --- a/pkg/cluster/clients/projects/fake/zz_fake.go +++ b/pkg/cluster/clients/projects/fake/zz_fake.go @@ -80,6 +80,9 @@ type MockClient struct { MockAddProjectPushRule func(pid any, opt *gitlab.AddProjectPushRuleOptions, options ...gitlab.RequestOptionFunc) (*gitlab.ProjectPushRules, *gitlab.Response, error) MockEditProjectPushRule func(pid any, opt *gitlab.EditProjectPushRuleOptions, options ...gitlab.RequestOptionFunc) (*gitlab.ProjectPushRules, *gitlab.Response, error) + MockGetApprovalConfiguration func(pid any, options ...gitlab.RequestOptionFunc) (*gitlab.ProjectApprovals, *gitlab.Response, error) + MockChangeApprovalConfiguration func(pid any, opt *gitlab.ChangeApprovalConfigurationOptions, options ...gitlab.RequestOptionFunc) (*gitlab.ProjectApprovals, *gitlab.Response, error) + MockGetProjectApprovalRule func(pid any, ruleID int64, options ...gitlab.RequestOptionFunc) (*gitlab.ProjectApprovalRule, *gitlab.Response, error) MockCreateProjectApprovalRule func(pid any, opt *gitlab.CreateProjectLevelRuleOptions, options ...gitlab.RequestOptionFunc) (*gitlab.ProjectApprovalRule, *gitlab.Response, error) MockUpdateProjectApprovalRule func(pid any, approvalRule int64, opt *gitlab.UpdateProjectLevelRuleOptions, options ...gitlab.RequestOptionFunc) (*gitlab.ProjectApprovalRule, *gitlab.Response, error) @@ -302,6 +305,16 @@ func (c *MockClient) EditProjectPushRule(pid any, opt *gitlab.EditProjectPushRul return c.MockEditProjectPushRule(pid, opt, options...) } +// GetApprovalConfiguration calls the underlying MockGetApprovalConfiguration method. +func (c *MockClient) GetApprovalConfiguration(pid any, options ...gitlab.RequestOptionFunc) (*gitlab.ProjectApprovals, *gitlab.Response, error) { + return c.MockGetApprovalConfiguration(pid, options...) +} + +// ChangeApprovalConfiguration calls the underlying MockChangeApprovalConfiguration method. +func (c *MockClient) ChangeApprovalConfiguration(pid any, opt *gitlab.ChangeApprovalConfigurationOptions, options ...gitlab.RequestOptionFunc) (*gitlab.ProjectApprovals, *gitlab.Response, error) { + return c.MockChangeApprovalConfiguration(pid, opt, options...) +} + func (c *MockClient) GetProjectApprovalRule(pid any, ruleID int64, options ...gitlab.RequestOptionFunc) (*gitlab.ProjectApprovalRule, *gitlab.Response, error) { return c.MockGetProjectApprovalRule(pid, ruleID, options...) } diff --git a/pkg/cluster/clients/projects/zz_project.go b/pkg/cluster/clients/projects/zz_project.go index 9d4b7a84..01e6880e 100644 --- a/pkg/cluster/clients/projects/zz_project.go +++ b/pkg/cluster/clients/projects/zz_project.go @@ -45,6 +45,9 @@ type Client interface { AddProjectPushRule(pid interface{}, opt *gitlab.AddProjectPushRuleOptions, options ...gitlab.RequestOptionFunc) (*gitlab.ProjectPushRules, *gitlab.Response, error) EditProjectPushRule(pid interface{}, opt *gitlab.EditProjectPushRuleOptions, options ...gitlab.RequestOptionFunc) (*gitlab.ProjectPushRules, *gitlab.Response, error) + GetApprovalConfiguration(pid interface{}, options ...gitlab.RequestOptionFunc) (*gitlab.ProjectApprovals, *gitlab.Response, error) + ChangeApprovalConfiguration(pid interface{}, opt *gitlab.ChangeApprovalConfigurationOptions, options ...gitlab.RequestOptionFunc) (*gitlab.ProjectApprovals, *gitlab.Response, error) + ShareProjectWithGroup(pid any, opt *gitlab.ShareWithGroupOptions, options ...gitlab.RequestOptionFunc) (*gitlab.Response, error) DeleteSharedProjectFromGroup(pid any, groupID int64, options ...gitlab.RequestOptionFunc) (*gitlab.Response, error) } @@ -462,6 +465,22 @@ func GenerateAddPushRulesOptions(p *v1alpha1.ProjectParameters) *gitlab.AddProje return o } +// GenerateChangeApprovalConfigurationOptions generates options to update a +// project's merge request approval configuration. +func GenerateChangeApprovalConfigurationOptions(p *v1alpha1.Approvals) *gitlab.ChangeApprovalConfigurationOptions { + o := &gitlab.ChangeApprovalConfigurationOptions{} + if p == nil { + return o + } + o.ResetApprovalsOnPush = p.ResetApprovalsOnPush + o.DisableOverridingApproversPerMergeRequest = p.DisableOverridingApproversPerMergeRequest + o.MergeRequestsAuthorApproval = p.MergeRequestsAuthorApproval + o.MergeRequestsDisableCommittersApproval = p.MergeRequestsDisableCommittersApproval + o.RequireReauthenticationToApprove = p.RequireReauthenticationToApprove + o.SelectiveCodeOwnerRemovals = p.SelectiveCodeOwnerRemovals + return o +} + func GenerateEditPushRulesOptions(p *v1alpha1.ProjectParameters) *gitlab.EditProjectPushRuleOptions { o := &gitlab.EditProjectPushRuleOptions{} if p.PushRules != nil { diff --git a/pkg/cluster/clients/projects/zz_project_test.go b/pkg/cluster/clients/projects/zz_project_test.go index 67c1021f..9ee363aa 100644 --- a/pkg/cluster/clients/projects/zz_project_test.go +++ b/pkg/cluster/clients/projects/zz_project_test.go @@ -773,3 +773,50 @@ func TestGenerateEditProjectOptions(t *testing.T) { }) } } + +func TestGenerateChangeApprovalConfigurationOptions(t *testing.T) { + cases := map[string]struct { + approvals *v1alpha1.Approvals + want *gitlab.ChangeApprovalConfigurationOptions + }{ + "Nil": { + approvals: nil, + want: &gitlab.ChangeApprovalConfigurationOptions{}, + }, + "AllFieldsSet": { + approvals: &v1alpha1.Approvals{ + ResetApprovalsOnPush: ptr.To(true), + DisableOverridingApproversPerMergeRequest: ptr.To(true), + MergeRequestsAuthorApproval: ptr.To(false), + MergeRequestsDisableCommittersApproval: ptr.To(true), + RequireReauthenticationToApprove: ptr.To(true), + SelectiveCodeOwnerRemovals: ptr.To(false), + }, + want: &gitlab.ChangeApprovalConfigurationOptions{ + ResetApprovalsOnPush: ptr.To(true), + DisableOverridingApproversPerMergeRequest: ptr.To(true), + MergeRequestsAuthorApproval: ptr.To(false), + MergeRequestsDisableCommittersApproval: ptr.To(true), + RequireReauthenticationToApprove: ptr.To(true), + SelectiveCodeOwnerRemovals: ptr.To(false), + }, + }, + "PartiallySet": { + approvals: &v1alpha1.Approvals{ + ResetApprovalsOnPush: ptr.To(true), + }, + want: &gitlab.ChangeApprovalConfigurationOptions{ + ResetApprovalsOnPush: ptr.To(true), + }, + }, + } + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + got := GenerateChangeApprovalConfigurationOptions(tc.approvals) + if diff := cmp.Diff(tc.want, got); diff != "" { + t.Errorf("r: -want, +got:\n%s", diff) + } + }) + } +} diff --git a/pkg/cluster/controller/projects/projects/zz_project.go b/pkg/cluster/controller/projects/projects/zz_project.go index 3fd0ca12..ab66bba4 100644 --- a/pkg/cluster/controller/projects/projects/zz_project.go +++ b/pkg/cluster/controller/projects/projects/zz_project.go @@ -59,6 +59,11 @@ const ( errLateInitialize = "cannot late-initialize Gitlab project" errLateInitializePushRules = "cannot late-initialize Gitlab project push rules" errCheckPushRulesUpToDate = "cannot compare project push rules" + + errUpdateApprovalsFailed = "cannot update Gitlab project approvals" + errGetApprovalsFailed = "cannot retrieve Gitlab project approvals" + errLateInitializeApprovals = "cannot late-initialize Gitlab project approvals" + errCheckApprovalsUpToDate = "cannot compare project approvals" ) // SetupProject adds a controller that reconciles Projects. @@ -127,6 +132,9 @@ type external struct { cache struct { externalPushRules *v1alpha1.PushRules isPushRulesUpToDate bool + + externalApprovals *gitlab.ProjectApprovals + isApprovalsUpToDate bool } } @@ -195,10 +203,15 @@ func (e *external) Observe(ctx context.Context, mg resource.Managed) (managed.Ex return managed.ExternalObservation{}, errors.Wrap(err, errCheckPushRulesUpToDate) } + e.cache.isApprovalsUpToDate, err = e.isApprovalsUpToDate(ctx, cr) + if err != nil { + return managed.ExternalObservation{}, errors.Wrap(err, errCheckApprovalsUpToDate) + } + cr.Status.AtProvider = projects.GenerateObservation(prj) return managed.ExternalObservation{ ResourceExists: true, - ResourceUpToDate: isProjectUpToDate(current, prj) && e.cache.isPushRulesUpToDate, + ResourceUpToDate: isProjectUpToDate(current, prj) && e.cache.isPushRulesUpToDate && e.cache.isApprovalsUpToDate, // Compare against specSnapshot (pre-secret-substitution) ResourceLateInitialized: !cmp.Equal(specSnapshot, &cr.Spec.ForProvider), ConnectionDetails: managed.ConnectionDetails{"runnersToken": []byte(prj.RunnersToken)}, @@ -263,9 +276,27 @@ func (e *external) Update(ctx context.Context, mg resource.Managed) (managed.Ext return managed.ExternalUpdate{}, err } } + + if cr.Spec.ForProvider.Approvals != nil && !e.cache.isApprovalsUpToDate { + if err := e.updateApprovals(ctx, cr); err != nil { + return managed.ExternalUpdate{}, err + } + } return managed.ExternalUpdate{}, nil } +// updateApprovals reconciles the project's merge request approval +// configuration. Unlike push rules, the approvals endpoint always exists and +// is updated via a single POST, so there is no add-vs-edit distinction. +func (e *external) updateApprovals(ctx context.Context, cr *v1alpha1.Project) error { + _, _, err := e.client.ChangeApprovalConfiguration( + meta.GetExternalName(cr), + projects.GenerateChangeApprovalConfigurationOptions(cr.Spec.ForProvider.Approvals), + gitlab.WithContext(ctx), + ) + return errors.Wrap(err, errUpdateApprovalsFailed) +} + // updatePushRules reconciles push rules for a project. It decides whether to // add (POST) or edit (PUT) push rules based on whether they already exist in // GitLab (cached in e.cache.externalPushRules). If neither cached rules nor @@ -452,9 +483,98 @@ func (e *external) lateInitialize(ctx context.Context, cr *v1alpha1.Project, pro return errors.Wrap(err, errLateInitializePushRules) } + if err := e.lateInitializeApprovals(ctx, cr); err != nil { + return errors.Wrap(err, errLateInitializeApprovals) + } + return nil } +// getApprovals fetches and caches the project's merge request approval +// configuration. It only fetches from GitLab when the spec opts into managing +// approvals, since the endpoint is otherwise irrelevant to reconciliation. +func (e *external) getApprovals(ctx context.Context, cr *v1alpha1.Project) (*gitlab.ProjectApprovals, error) { + if cr.Spec.ForProvider.Approvals == nil { + return nil, nil + } + if e.cache.externalApprovals != nil { + return e.cache.externalApprovals, nil + } + res, _, err := e.client.GetApprovalConfiguration(meta.GetExternalName(cr), gitlab.WithContext(ctx)) + if err != nil { + return nil, errors.Wrap(err, errGetApprovalsFailed) + } + e.cache.externalApprovals = res + return res, nil +} + +// lateInitializeApprovals fills empty fields of the approvals spec block +// with the values seen in GitLab. It only runs if the user has already +// opted into managing approvals by setting a non-nil block in the spec. +func (e *external) lateInitializeApprovals(ctx context.Context, cr *v1alpha1.Project) error { + live, err := e.getApprovals(ctx, cr) + if err != nil || live == nil { + return err + } + + in := cr.Spec.ForProvider.Approvals + if in.ResetApprovalsOnPush == nil { + in.ResetApprovalsOnPush = &live.ResetApprovalsOnPush + } + if in.DisableOverridingApproversPerMergeRequest == nil { + in.DisableOverridingApproversPerMergeRequest = &live.DisableOverridingApproversPerMergeRequest + } + if in.MergeRequestsAuthorApproval == nil { + in.MergeRequestsAuthorApproval = &live.MergeRequestsAuthorApproval + } + if in.MergeRequestsDisableCommittersApproval == nil { + in.MergeRequestsDisableCommittersApproval = &live.MergeRequestsDisableCommittersApproval + } + if in.RequireReauthenticationToApprove == nil { + in.RequireReauthenticationToApprove = &live.RequireReauthenticationToApprove + } + if in.SelectiveCodeOwnerRemovals == nil { + in.SelectiveCodeOwnerRemovals = &live.SelectiveCodeOwnerRemovals + } + return nil +} + +// isApprovalsUpToDate checks whether the approvals spec block (if any) +// matches the live GitLab configuration. A nil spec block means approvals +// are not managed by this provider and is always considered up to date. +func (e *external) isApprovalsUpToDate(ctx context.Context, cr *v1alpha1.Project) (bool, error) { + if cr.Spec.ForProvider.Approvals == nil { + return true, nil + } + + live, err := e.getApprovals(ctx, cr) + if err != nil { + return false, err + } + + p := cr.Spec.ForProvider.Approvals + if !clients.IsBoolEqualToBoolPtr(p.ResetApprovalsOnPush, live.ResetApprovalsOnPush) { + return false, nil + } + if !clients.IsBoolEqualToBoolPtr(p.DisableOverridingApproversPerMergeRequest, live.DisableOverridingApproversPerMergeRequest) { + return false, nil + } + if !clients.IsBoolEqualToBoolPtr(p.MergeRequestsAuthorApproval, live.MergeRequestsAuthorApproval) { + return false, nil + } + if !clients.IsBoolEqualToBoolPtr(p.MergeRequestsDisableCommittersApproval, live.MergeRequestsDisableCommittersApproval) { + return false, nil + } + if !clients.IsBoolEqualToBoolPtr(p.RequireReauthenticationToApprove, live.RequireReauthenticationToApprove) { + return false, nil + } + if !clients.IsBoolEqualToBoolPtr(p.SelectiveCodeOwnerRemovals, live.SelectiveCodeOwnerRemovals) { + return false, nil + } + + return true, nil +} + func (e *external) lateInitializePushRules(ctx context.Context, cr *v1alpha1.Project) error { pr, err := e.getProjectPushRules(ctx, cr) if err != nil || pr == nil { diff --git a/pkg/cluster/controller/projects/projects/zz_project_test.go b/pkg/cluster/controller/projects/projects/zz_project_test.go index d5d068d0..15f65cc8 100644 --- a/pkg/cluster/controller/projects/projects/zz_project_test.go +++ b/pkg/cluster/controller/projects/projects/zz_project_test.go @@ -86,6 +86,10 @@ func withProjectPushRules(pr *v1alpha1.PushRules) projectModifier { return func(r *v1alpha1.Project) { r.Spec.ForProvider.PushRules = pr } } +func withApprovals(a *v1alpha1.Approvals) projectModifier { + return func(r *v1alpha1.Project) { r.Spec.ForProvider.Approvals = a } +} + func withMergeTrainSettings(enabled, skipAllowed, pipelines *bool) projectModifier { return func(r *v1alpha1.Project) { r.Spec.ForProvider.MergeTrainsEnabled = enabled @@ -461,6 +465,134 @@ func TestObserve(t *testing.T) { }, }, }, + // Approvals is not set in the spec, so GetApprovalConfiguration must + // not be called at all (no mock is provided; a call would panic). + "ApprovalsNotManaged": { + args: args{ + project: &fake.MockClient{ + MockGetProject: func(pid interface{}, opt *gitlab.GetProjectOptions, options ...gitlab.RequestOptionFunc) (*gitlab.Project, *gitlab.Response, error) { + return &gitlab.Project{Name: "example-project"}, &gitlab.Response{}, nil + }, + MockGetProjectPushRules: func(pid interface{}, options ...gitlab.RequestOptionFunc) (*gitlab.ProjectPushRules, *gitlab.Response, error) { + return nil, &gitlab.Response{Response: &http.Response{StatusCode: 404}}, errBoom + }, + }, + cr: project( + withClientDefaultValues(), + withExternalName(extName), + ), + }, + want: want{ + cr: project( + withClientDefaultValues(), + withExternalName(extName), + withConditions(v2.Available()), + withStatus(v1alpha1.ProjectObservation{}), + ), + result: managed.ExternalObservation{ + ResourceExists: true, + ResourceUpToDate: true, + ResourceLateInitialized: false, + ConnectionDetails: managed.ConnectionDetails{"runnersToken": []byte("")}, + }, + }, + }, + "ApprovalsUpToDate": { + args: args{ + project: &fake.MockClient{ + MockGetProject: func(pid interface{}, opt *gitlab.GetProjectOptions, options ...gitlab.RequestOptionFunc) (*gitlab.Project, *gitlab.Response, error) { + return &gitlab.Project{Name: "example-project"}, &gitlab.Response{}, nil + }, + MockGetProjectPushRules: func(pid interface{}, options ...gitlab.RequestOptionFunc) (*gitlab.ProjectPushRules, *gitlab.Response, error) { + return nil, &gitlab.Response{Response: &http.Response{StatusCode: 404}}, errBoom + }, + MockGetApprovalConfiguration: func(pid interface{}, options ...gitlab.RequestOptionFunc) (*gitlab.ProjectApprovals, *gitlab.Response, error) { + return &gitlab.ProjectApprovals{ + ResetApprovalsOnPush: true, + MergeRequestsAuthorApproval: false, + RequireReauthenticationToApprove: true, + }, &gitlab.Response{}, nil + }, + }, + cr: project( + withClientDefaultValues(), + withExternalName(extName), + withApprovals(&v1alpha1.Approvals{ + ResetApprovalsOnPush: ptr.To(true), + MergeRequestsAuthorApproval: ptr.To(false), + RequireReauthenticationToApprove: ptr.To(true), + }), + ), + }, + want: want{ + cr: project( + withClientDefaultValues(), + withExternalName(extName), + withConditions(v2.Available()), + withStatus(v1alpha1.ProjectObservation{}), + withApprovals(&v1alpha1.Approvals{ + ResetApprovalsOnPush: ptr.To(true), + MergeRequestsAuthorApproval: ptr.To(false), + RequireReauthenticationToApprove: ptr.To(true), + DisableOverridingApproversPerMergeRequest: ptr.To(false), + MergeRequestsDisableCommittersApproval: ptr.To(false), + SelectiveCodeOwnerRemovals: ptr.To(false), + }), + ), + result: managed.ExternalObservation{ + ResourceExists: true, + ResourceUpToDate: true, + ResourceLateInitialized: true, + ConnectionDetails: managed.ConnectionDetails{"runnersToken": []byte("")}, + }, + }, + }, + "ApprovalsNotUpToDate": { + args: args{ + project: &fake.MockClient{ + MockGetProject: func(pid interface{}, opt *gitlab.GetProjectOptions, options ...gitlab.RequestOptionFunc) (*gitlab.Project, *gitlab.Response, error) { + return &gitlab.Project{Name: "example-project"}, &gitlab.Response{}, nil + }, + MockGetProjectPushRules: func(pid interface{}, options ...gitlab.RequestOptionFunc) (*gitlab.ProjectPushRules, *gitlab.Response, error) { + return nil, &gitlab.Response{Response: &http.Response{StatusCode: 404}}, errBoom + }, + MockGetApprovalConfiguration: func(pid interface{}, options ...gitlab.RequestOptionFunc) (*gitlab.ProjectApprovals, *gitlab.Response, error) { + return &gitlab.ProjectApprovals{ + ResetApprovalsOnPush: false, + }, &gitlab.Response{}, nil + }, + }, + cr: project( + withClientDefaultValues(), + withExternalName(extName), + withApprovals(&v1alpha1.Approvals{ + ResetApprovalsOnPush: ptr.To(true), + }), + ), + }, + want: want{ + cr: project( + withClientDefaultValues(), + withExternalName(extName), + withConditions(v2.Available()), + withStatus(v1alpha1.ProjectObservation{}), + withApprovals(&v1alpha1.Approvals{ + ResetApprovalsOnPush: ptr.To(true), + DisableOverridingApproversPerMergeRequest: ptr.To(false), + MergeRequestsAuthorApproval: ptr.To(false), + MergeRequestsDisableCommittersApproval: ptr.To(false), + RequireReauthenticationToApprove: ptr.To(false), + SelectiveCodeOwnerRemovals: ptr.To(false), + }), + ), + result: managed.ExternalObservation{ + ResourceExists: true, + ResourceUpToDate: false, + ResourceLateInitialized: true, + ConnectionDetails: managed.ConnectionDetails{"runnersToken": []byte("")}, + }, + }, + }, // Regression test: ImportURLSecretRef with credentials embedded in the URL. // GitLab ALWAYS strips userinfo from ImportURL in API responses. // "https://TOKEN:@github.com/org/repo.git" becomes "https://github.com/org/repo.git". @@ -1195,8 +1327,9 @@ func TestUpdate(t *testing.T) { cases := map[string]struct { args - cacheExternalPushRules *v1alpha1.PushRules - cachePushRulesUpToDate bool + cacheExternalPushRules *v1alpha1.PushRules + cachePushRulesUpToDate bool + cacheIsApprovalsUpToDate bool want }{ "InValidInput": { @@ -1361,12 +1494,97 @@ func TestUpdate(t *testing.T) { ), }, }, + "ApprovalsNotManagedSkipped": { + args: args{ + project: &fake.MockClient{ + MockEditProject: func(pid interface{}, opt *gitlab.EditProjectOptions, options ...gitlab.RequestOptionFunc) (*gitlab.Project, *gitlab.Response, error) { + return &gitlab.Project{}, &gitlab.Response{}, nil + }, + // No approvals mocks needed - Approvals is not set in spec, so + // ChangeApprovalConfiguration must not be called. + }, + cr: project(withStatus(v1alpha1.ProjectObservation{ID: 1234})), + }, + cacheIsApprovalsUpToDate: false, + want: want{ + cr: project(withStatus(v1alpha1.ProjectObservation{ID: 1234})), + }, + }, + "ApprovalsUpToDateSkipped": { + args: args{ + project: &fake.MockClient{ + MockEditProject: func(pid interface{}, opt *gitlab.EditProjectOptions, options ...gitlab.RequestOptionFunc) (*gitlab.Project, *gitlab.Response, error) { + return &gitlab.Project{}, &gitlab.Response{}, nil + }, + // No approvals mocks needed - already up to date per cache. + }, + cr: project( + withStatus(v1alpha1.ProjectObservation{ID: 1234}), + withApprovals(&v1alpha1.Approvals{ResetApprovalsOnPush: ptr.To(true)}), + ), + }, + cacheIsApprovalsUpToDate: true, + want: want{ + cr: project( + withStatus(v1alpha1.ProjectObservation{ID: 1234}), + withApprovals(&v1alpha1.Approvals{ResetApprovalsOnPush: ptr.To(true)}), + ), + }, + }, + "SuccessfulUpdateApprovals": { + args: args{ + project: &fake.MockClient{ + MockEditProject: func(pid interface{}, opt *gitlab.EditProjectOptions, options ...gitlab.RequestOptionFunc) (*gitlab.Project, *gitlab.Response, error) { + return &gitlab.Project{}, &gitlab.Response{}, nil + }, + MockChangeApprovalConfiguration: func(pid interface{}, opt *gitlab.ChangeApprovalConfigurationOptions, options ...gitlab.RequestOptionFunc) (*gitlab.ProjectApprovals, *gitlab.Response, error) { + return &gitlab.ProjectApprovals{}, &gitlab.Response{}, nil + }, + }, + cr: project( + withStatus(v1alpha1.ProjectObservation{ID: 1234}), + withApprovals(&v1alpha1.Approvals{ResetApprovalsOnPush: ptr.To(true)}), + ), + }, + cacheIsApprovalsUpToDate: false, + want: want{ + cr: project( + withStatus(v1alpha1.ProjectObservation{ID: 1234}), + withApprovals(&v1alpha1.Approvals{ResetApprovalsOnPush: ptr.To(true)}), + ), + }, + }, + "FailedUpdateApprovals": { + args: args{ + project: &fake.MockClient{ + MockEditProject: func(pid interface{}, opt *gitlab.EditProjectOptions, options ...gitlab.RequestOptionFunc) (*gitlab.Project, *gitlab.Response, error) { + return &gitlab.Project{}, &gitlab.Response{}, nil + }, + MockChangeApprovalConfiguration: func(pid interface{}, opt *gitlab.ChangeApprovalConfigurationOptions, options ...gitlab.RequestOptionFunc) (*gitlab.ProjectApprovals, *gitlab.Response, error) { + return &gitlab.ProjectApprovals{}, &gitlab.Response{}, errBoom + }, + }, + cr: project( + withStatus(v1alpha1.ProjectObservation{ID: 1234}), + withApprovals(&v1alpha1.Approvals{ResetApprovalsOnPush: ptr.To(true)}), + ), + }, + cacheIsApprovalsUpToDate: false, + want: want{ + cr: project( + withStatus(v1alpha1.ProjectObservation{ID: 1234}), + withApprovals(&v1alpha1.Approvals{ResetApprovalsOnPush: ptr.To(true)}), + ), + err: errors.Wrap(errBoom, errUpdateApprovalsFailed), + }, + }, } for name, tc := range cases { t.Run(name, func(t *testing.T) { e := &external{kube: tc.kube, client: tc.project} e.cache.externalPushRules = tc.cacheExternalPushRules e.cache.isPushRulesUpToDate = tc.cachePushRulesUpToDate + e.cache.isApprovalsUpToDate = tc.cacheIsApprovalsUpToDate o, err := e.Update(context.Background(), tc.args.cr) if diff := cmp.Diff(tc.want.err, err, test.EquateErrors()); diff != "" { diff --git a/pkg/namespaced/clients/projects/fake/fake.go b/pkg/namespaced/clients/projects/fake/fake.go index 952767ad..7093a1e1 100644 --- a/pkg/namespaced/clients/projects/fake/fake.go +++ b/pkg/namespaced/clients/projects/fake/fake.go @@ -78,6 +78,9 @@ type MockClient struct { MockAddProjectPushRule func(pid any, opt *gitlab.AddProjectPushRuleOptions, options ...gitlab.RequestOptionFunc) (*gitlab.ProjectPushRules, *gitlab.Response, error) MockEditProjectPushRule func(pid any, opt *gitlab.EditProjectPushRuleOptions, options ...gitlab.RequestOptionFunc) (*gitlab.ProjectPushRules, *gitlab.Response, error) + MockGetApprovalConfiguration func(pid any, options ...gitlab.RequestOptionFunc) (*gitlab.ProjectApprovals, *gitlab.Response, error) + MockChangeApprovalConfiguration func(pid any, opt *gitlab.ChangeApprovalConfigurationOptions, options ...gitlab.RequestOptionFunc) (*gitlab.ProjectApprovals, *gitlab.Response, error) + MockGetProjectApprovalRule func(pid any, ruleID int64, options ...gitlab.RequestOptionFunc) (*gitlab.ProjectApprovalRule, *gitlab.Response, error) MockCreateProjectApprovalRule func(pid any, opt *gitlab.CreateProjectLevelRuleOptions, options ...gitlab.RequestOptionFunc) (*gitlab.ProjectApprovalRule, *gitlab.Response, error) MockUpdateProjectApprovalRule func(pid any, approvalRule int64, opt *gitlab.UpdateProjectLevelRuleOptions, options ...gitlab.RequestOptionFunc) (*gitlab.ProjectApprovalRule, *gitlab.Response, error) @@ -300,6 +303,16 @@ func (c *MockClient) EditProjectPushRule(pid any, opt *gitlab.EditProjectPushRul return c.MockEditProjectPushRule(pid, opt, options...) } +// GetApprovalConfiguration calls the underlying MockGetApprovalConfiguration method. +func (c *MockClient) GetApprovalConfiguration(pid any, options ...gitlab.RequestOptionFunc) (*gitlab.ProjectApprovals, *gitlab.Response, error) { + return c.MockGetApprovalConfiguration(pid, options...) +} + +// ChangeApprovalConfiguration calls the underlying MockChangeApprovalConfiguration method. +func (c *MockClient) ChangeApprovalConfiguration(pid any, opt *gitlab.ChangeApprovalConfigurationOptions, options ...gitlab.RequestOptionFunc) (*gitlab.ProjectApprovals, *gitlab.Response, error) { + return c.MockChangeApprovalConfiguration(pid, opt, options...) +} + func (c *MockClient) GetProjectApprovalRule(pid any, ruleID int64, options ...gitlab.RequestOptionFunc) (*gitlab.ProjectApprovalRule, *gitlab.Response, error) { return c.MockGetProjectApprovalRule(pid, ruleID, options...) } diff --git a/pkg/namespaced/clients/projects/project.go b/pkg/namespaced/clients/projects/project.go index 8c443c78..f6c4aa9d 100644 --- a/pkg/namespaced/clients/projects/project.go +++ b/pkg/namespaced/clients/projects/project.go @@ -43,6 +43,9 @@ type Client interface { AddProjectPushRule(pid interface{}, opt *gitlab.AddProjectPushRuleOptions, options ...gitlab.RequestOptionFunc) (*gitlab.ProjectPushRules, *gitlab.Response, error) EditProjectPushRule(pid interface{}, opt *gitlab.EditProjectPushRuleOptions, options ...gitlab.RequestOptionFunc) (*gitlab.ProjectPushRules, *gitlab.Response, error) + GetApprovalConfiguration(pid interface{}, options ...gitlab.RequestOptionFunc) (*gitlab.ProjectApprovals, *gitlab.Response, error) + ChangeApprovalConfiguration(pid interface{}, opt *gitlab.ChangeApprovalConfigurationOptions, options ...gitlab.RequestOptionFunc) (*gitlab.ProjectApprovals, *gitlab.Response, error) + ShareProjectWithGroup(pid any, opt *gitlab.ShareWithGroupOptions, options ...gitlab.RequestOptionFunc) (*gitlab.Response, error) DeleteSharedProjectFromGroup(pid any, groupID int64, options ...gitlab.RequestOptionFunc) (*gitlab.Response, error) } @@ -460,6 +463,22 @@ func GenerateAddPushRulesOptions(p *v1alpha1.ProjectParameters) *gitlab.AddProje return o } +// GenerateChangeApprovalConfigurationOptions generates options to update a +// project's merge request approval configuration. +func GenerateChangeApprovalConfigurationOptions(p *v1alpha1.Approvals) *gitlab.ChangeApprovalConfigurationOptions { + o := &gitlab.ChangeApprovalConfigurationOptions{} + if p == nil { + return o + } + o.ResetApprovalsOnPush = p.ResetApprovalsOnPush + o.DisableOverridingApproversPerMergeRequest = p.DisableOverridingApproversPerMergeRequest + o.MergeRequestsAuthorApproval = p.MergeRequestsAuthorApproval + o.MergeRequestsDisableCommittersApproval = p.MergeRequestsDisableCommittersApproval + o.RequireReauthenticationToApprove = p.RequireReauthenticationToApprove + o.SelectiveCodeOwnerRemovals = p.SelectiveCodeOwnerRemovals + return o +} + func GenerateEditPushRulesOptions(p *v1alpha1.ProjectParameters) *gitlab.EditProjectPushRuleOptions { o := &gitlab.EditProjectPushRuleOptions{} if p.PushRules != nil { diff --git a/pkg/namespaced/clients/projects/project_test.go b/pkg/namespaced/clients/projects/project_test.go index 09e0404d..10680894 100644 --- a/pkg/namespaced/clients/projects/project_test.go +++ b/pkg/namespaced/clients/projects/project_test.go @@ -771,3 +771,50 @@ func TestGenerateEditProjectOptions(t *testing.T) { }) } } + +func TestGenerateChangeApprovalConfigurationOptions(t *testing.T) { + cases := map[string]struct { + approvals *v1alpha1.Approvals + want *gitlab.ChangeApprovalConfigurationOptions + }{ + "Nil": { + approvals: nil, + want: &gitlab.ChangeApprovalConfigurationOptions{}, + }, + "AllFieldsSet": { + approvals: &v1alpha1.Approvals{ + ResetApprovalsOnPush: ptr.To(true), + DisableOverridingApproversPerMergeRequest: ptr.To(true), + MergeRequestsAuthorApproval: ptr.To(false), + MergeRequestsDisableCommittersApproval: ptr.To(true), + RequireReauthenticationToApprove: ptr.To(true), + SelectiveCodeOwnerRemovals: ptr.To(false), + }, + want: &gitlab.ChangeApprovalConfigurationOptions{ + ResetApprovalsOnPush: ptr.To(true), + DisableOverridingApproversPerMergeRequest: ptr.To(true), + MergeRequestsAuthorApproval: ptr.To(false), + MergeRequestsDisableCommittersApproval: ptr.To(true), + RequireReauthenticationToApprove: ptr.To(true), + SelectiveCodeOwnerRemovals: ptr.To(false), + }, + }, + "PartiallySet": { + approvals: &v1alpha1.Approvals{ + ResetApprovalsOnPush: ptr.To(true), + }, + want: &gitlab.ChangeApprovalConfigurationOptions{ + ResetApprovalsOnPush: ptr.To(true), + }, + }, + } + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + got := GenerateChangeApprovalConfigurationOptions(tc.approvals) + if diff := cmp.Diff(tc.want, got); diff != "" { + t.Errorf("r: -want, +got:\n%s", diff) + } + }) + } +} diff --git a/pkg/namespaced/controller/projects/projects/project.go b/pkg/namespaced/controller/projects/projects/project.go index 56e7767b..5742099c 100644 --- a/pkg/namespaced/controller/projects/projects/project.go +++ b/pkg/namespaced/controller/projects/projects/project.go @@ -57,6 +57,11 @@ const ( errLateInitialize = "cannot late-initialize Gitlab project" errLateInitializePushRules = "cannot late-initialize Gitlab project push rules" errCheckPushRulesUpToDate = "cannot compare project push rules" + + errUpdateApprovalsFailed = "cannot update Gitlab project approvals" + errGetApprovalsFailed = "cannot retrieve Gitlab project approvals" + errLateInitializeApprovals = "cannot late-initialize Gitlab project approvals" + errCheckApprovalsUpToDate = "cannot compare project approvals" ) // SetupProject adds a controller that reconciles Projects. @@ -125,6 +130,9 @@ type external struct { cache struct { externalPushRules *v1alpha1.PushRules isPushRulesUpToDate bool + + externalApprovals *gitlab.ProjectApprovals + isApprovalsUpToDate bool } } @@ -193,10 +201,15 @@ func (e *external) Observe(ctx context.Context, mg resource.Managed) (managed.Ex return managed.ExternalObservation{}, errors.Wrap(err, errCheckPushRulesUpToDate) } + e.cache.isApprovalsUpToDate, err = e.isApprovalsUpToDate(ctx, cr) + if err != nil { + return managed.ExternalObservation{}, errors.Wrap(err, errCheckApprovalsUpToDate) + } + cr.Status.AtProvider = projects.GenerateObservation(prj) return managed.ExternalObservation{ ResourceExists: true, - ResourceUpToDate: isProjectUpToDate(current, prj) && e.cache.isPushRulesUpToDate, + ResourceUpToDate: isProjectUpToDate(current, prj) && e.cache.isPushRulesUpToDate && e.cache.isApprovalsUpToDate, // Compare against specSnapshot (pre-secret-substitution) ResourceLateInitialized: !cmp.Equal(specSnapshot, &cr.Spec.ForProvider), ConnectionDetails: managed.ConnectionDetails{"runnersToken": []byte(prj.RunnersToken)}, @@ -261,9 +274,27 @@ func (e *external) Update(ctx context.Context, mg resource.Managed) (managed.Ext return managed.ExternalUpdate{}, err } } + + if cr.Spec.ForProvider.Approvals != nil && !e.cache.isApprovalsUpToDate { + if err := e.updateApprovals(ctx, cr); err != nil { + return managed.ExternalUpdate{}, err + } + } return managed.ExternalUpdate{}, nil } +// updateApprovals reconciles the project's merge request approval +// configuration. Unlike push rules, the approvals endpoint always exists and +// is updated via a single POST, so there is no add-vs-edit distinction. +func (e *external) updateApprovals(ctx context.Context, cr *v1alpha1.Project) error { + _, _, err := e.client.ChangeApprovalConfiguration( + meta.GetExternalName(cr), + projects.GenerateChangeApprovalConfigurationOptions(cr.Spec.ForProvider.Approvals), + gitlab.WithContext(ctx), + ) + return errors.Wrap(err, errUpdateApprovalsFailed) +} + // updatePushRules reconciles push rules for a project. It decides whether to // add (POST) or edit (PUT) push rules based on whether they already exist in // GitLab (cached in e.cache.externalPushRules). If neither cached rules nor @@ -450,9 +481,98 @@ func (e *external) lateInitialize(ctx context.Context, cr *v1alpha1.Project, pro return errors.Wrap(err, errLateInitializePushRules) } + if err := e.lateInitializeApprovals(ctx, cr); err != nil { + return errors.Wrap(err, errLateInitializeApprovals) + } + return nil } +// getApprovals fetches and caches the project's merge request approval +// configuration. It only fetches from GitLab when the spec opts into managing +// approvals, since the endpoint is otherwise irrelevant to reconciliation. +func (e *external) getApprovals(ctx context.Context, cr *v1alpha1.Project) (*gitlab.ProjectApprovals, error) { + if cr.Spec.ForProvider.Approvals == nil { + return nil, nil + } + if e.cache.externalApprovals != nil { + return e.cache.externalApprovals, nil + } + res, _, err := e.client.GetApprovalConfiguration(meta.GetExternalName(cr), gitlab.WithContext(ctx)) + if err != nil { + return nil, errors.Wrap(err, errGetApprovalsFailed) + } + e.cache.externalApprovals = res + return res, nil +} + +// lateInitializeApprovals fills empty fields of the approvals spec block +// with the values seen in GitLab. It only runs if the user has already +// opted into managing approvals by setting a non-nil block in the spec. +func (e *external) lateInitializeApprovals(ctx context.Context, cr *v1alpha1.Project) error { + live, err := e.getApprovals(ctx, cr) + if err != nil || live == nil { + return err + } + + in := cr.Spec.ForProvider.Approvals + if in.ResetApprovalsOnPush == nil { + in.ResetApprovalsOnPush = &live.ResetApprovalsOnPush + } + if in.DisableOverridingApproversPerMergeRequest == nil { + in.DisableOverridingApproversPerMergeRequest = &live.DisableOverridingApproversPerMergeRequest + } + if in.MergeRequestsAuthorApproval == nil { + in.MergeRequestsAuthorApproval = &live.MergeRequestsAuthorApproval + } + if in.MergeRequestsDisableCommittersApproval == nil { + in.MergeRequestsDisableCommittersApproval = &live.MergeRequestsDisableCommittersApproval + } + if in.RequireReauthenticationToApprove == nil { + in.RequireReauthenticationToApprove = &live.RequireReauthenticationToApprove + } + if in.SelectiveCodeOwnerRemovals == nil { + in.SelectiveCodeOwnerRemovals = &live.SelectiveCodeOwnerRemovals + } + return nil +} + +// isApprovalsUpToDate checks whether the approvals spec block (if any) +// matches the live GitLab configuration. A nil spec block means approvals +// are not managed by this provider and is always considered up to date. +func (e *external) isApprovalsUpToDate(ctx context.Context, cr *v1alpha1.Project) (bool, error) { + if cr.Spec.ForProvider.Approvals == nil { + return true, nil + } + + live, err := e.getApprovals(ctx, cr) + if err != nil { + return false, err + } + + p := cr.Spec.ForProvider.Approvals + if !clients.IsBoolEqualToBoolPtr(p.ResetApprovalsOnPush, live.ResetApprovalsOnPush) { + return false, nil + } + if !clients.IsBoolEqualToBoolPtr(p.DisableOverridingApproversPerMergeRequest, live.DisableOverridingApproversPerMergeRequest) { + return false, nil + } + if !clients.IsBoolEqualToBoolPtr(p.MergeRequestsAuthorApproval, live.MergeRequestsAuthorApproval) { + return false, nil + } + if !clients.IsBoolEqualToBoolPtr(p.MergeRequestsDisableCommittersApproval, live.MergeRequestsDisableCommittersApproval) { + return false, nil + } + if !clients.IsBoolEqualToBoolPtr(p.RequireReauthenticationToApprove, live.RequireReauthenticationToApprove) { + return false, nil + } + if !clients.IsBoolEqualToBoolPtr(p.SelectiveCodeOwnerRemovals, live.SelectiveCodeOwnerRemovals) { + return false, nil + } + + return true, nil +} + func (e *external) lateInitializePushRules(ctx context.Context, cr *v1alpha1.Project) error { pr, err := e.getProjectPushRules(ctx, cr) if err != nil || pr == nil { diff --git a/pkg/namespaced/controller/projects/projects/project_test.go b/pkg/namespaced/controller/projects/projects/project_test.go index 75cca308..db9ea403 100644 --- a/pkg/namespaced/controller/projects/projects/project_test.go +++ b/pkg/namespaced/controller/projects/projects/project_test.go @@ -84,6 +84,10 @@ func withProjectPushRules(pr *v1alpha1.PushRules) projectModifier { return func(r *v1alpha1.Project) { r.Spec.ForProvider.PushRules = pr } } +func withApprovals(a *v1alpha1.Approvals) projectModifier { + return func(r *v1alpha1.Project) { r.Spec.ForProvider.Approvals = a } +} + func withMergeTrainSettings(enabled, skipAllowed, pipelines *bool) projectModifier { return func(r *v1alpha1.Project) { r.Spec.ForProvider.MergeTrainsEnabled = enabled @@ -459,6 +463,134 @@ func TestObserve(t *testing.T) { }, }, }, + // Approvals is not set in the spec, so GetApprovalConfiguration must + // not be called at all (no mock is provided; a call would panic). + "ApprovalsNotManaged": { + args: args{ + project: &fake.MockClient{ + MockGetProject: func(pid interface{}, opt *gitlab.GetProjectOptions, options ...gitlab.RequestOptionFunc) (*gitlab.Project, *gitlab.Response, error) { + return &gitlab.Project{Name: "example-project"}, &gitlab.Response{}, nil + }, + MockGetProjectPushRules: func(pid interface{}, options ...gitlab.RequestOptionFunc) (*gitlab.ProjectPushRules, *gitlab.Response, error) { + return nil, &gitlab.Response{Response: &http.Response{StatusCode: 404}}, errBoom + }, + }, + cr: project( + withClientDefaultValues(), + withExternalName(extName), + ), + }, + want: want{ + cr: project( + withClientDefaultValues(), + withExternalName(extName), + withConditions(v2.Available()), + withStatus(v1alpha1.ProjectObservation{}), + ), + result: managed.ExternalObservation{ + ResourceExists: true, + ResourceUpToDate: true, + ResourceLateInitialized: false, + ConnectionDetails: managed.ConnectionDetails{"runnersToken": []byte("")}, + }, + }, + }, + "ApprovalsUpToDate": { + args: args{ + project: &fake.MockClient{ + MockGetProject: func(pid interface{}, opt *gitlab.GetProjectOptions, options ...gitlab.RequestOptionFunc) (*gitlab.Project, *gitlab.Response, error) { + return &gitlab.Project{Name: "example-project"}, &gitlab.Response{}, nil + }, + MockGetProjectPushRules: func(pid interface{}, options ...gitlab.RequestOptionFunc) (*gitlab.ProjectPushRules, *gitlab.Response, error) { + return nil, &gitlab.Response{Response: &http.Response{StatusCode: 404}}, errBoom + }, + MockGetApprovalConfiguration: func(pid interface{}, options ...gitlab.RequestOptionFunc) (*gitlab.ProjectApprovals, *gitlab.Response, error) { + return &gitlab.ProjectApprovals{ + ResetApprovalsOnPush: true, + MergeRequestsAuthorApproval: false, + RequireReauthenticationToApprove: true, + }, &gitlab.Response{}, nil + }, + }, + cr: project( + withClientDefaultValues(), + withExternalName(extName), + withApprovals(&v1alpha1.Approvals{ + ResetApprovalsOnPush: ptr.To(true), + MergeRequestsAuthorApproval: ptr.To(false), + RequireReauthenticationToApprove: ptr.To(true), + }), + ), + }, + want: want{ + cr: project( + withClientDefaultValues(), + withExternalName(extName), + withConditions(v2.Available()), + withStatus(v1alpha1.ProjectObservation{}), + withApprovals(&v1alpha1.Approvals{ + ResetApprovalsOnPush: ptr.To(true), + MergeRequestsAuthorApproval: ptr.To(false), + RequireReauthenticationToApprove: ptr.To(true), + DisableOverridingApproversPerMergeRequest: ptr.To(false), + MergeRequestsDisableCommittersApproval: ptr.To(false), + SelectiveCodeOwnerRemovals: ptr.To(false), + }), + ), + result: managed.ExternalObservation{ + ResourceExists: true, + ResourceUpToDate: true, + ResourceLateInitialized: true, + ConnectionDetails: managed.ConnectionDetails{"runnersToken": []byte("")}, + }, + }, + }, + "ApprovalsNotUpToDate": { + args: args{ + project: &fake.MockClient{ + MockGetProject: func(pid interface{}, opt *gitlab.GetProjectOptions, options ...gitlab.RequestOptionFunc) (*gitlab.Project, *gitlab.Response, error) { + return &gitlab.Project{Name: "example-project"}, &gitlab.Response{}, nil + }, + MockGetProjectPushRules: func(pid interface{}, options ...gitlab.RequestOptionFunc) (*gitlab.ProjectPushRules, *gitlab.Response, error) { + return nil, &gitlab.Response{Response: &http.Response{StatusCode: 404}}, errBoom + }, + MockGetApprovalConfiguration: func(pid interface{}, options ...gitlab.RequestOptionFunc) (*gitlab.ProjectApprovals, *gitlab.Response, error) { + return &gitlab.ProjectApprovals{ + ResetApprovalsOnPush: false, + }, &gitlab.Response{}, nil + }, + }, + cr: project( + withClientDefaultValues(), + withExternalName(extName), + withApprovals(&v1alpha1.Approvals{ + ResetApprovalsOnPush: ptr.To(true), + }), + ), + }, + want: want{ + cr: project( + withClientDefaultValues(), + withExternalName(extName), + withConditions(v2.Available()), + withStatus(v1alpha1.ProjectObservation{}), + withApprovals(&v1alpha1.Approvals{ + ResetApprovalsOnPush: ptr.To(true), + DisableOverridingApproversPerMergeRequest: ptr.To(false), + MergeRequestsAuthorApproval: ptr.To(false), + MergeRequestsDisableCommittersApproval: ptr.To(false), + RequireReauthenticationToApprove: ptr.To(false), + SelectiveCodeOwnerRemovals: ptr.To(false), + }), + ), + result: managed.ExternalObservation{ + ResourceExists: true, + ResourceUpToDate: false, + ResourceLateInitialized: true, + ConnectionDetails: managed.ConnectionDetails{"runnersToken": []byte("")}, + }, + }, + }, // Regression test: ImportURLSecretRef with credentials embedded in the URL. // GitLab ALWAYS strips userinfo from ImportURL in API responses. // "https://TOKEN:@github.com/org/repo.git" becomes "https://github.com/org/repo.git". @@ -1193,8 +1325,9 @@ func TestUpdate(t *testing.T) { cases := map[string]struct { args - cacheExternalPushRules *v1alpha1.PushRules - cachePushRulesUpToDate bool + cacheExternalPushRules *v1alpha1.PushRules + cachePushRulesUpToDate bool + cacheIsApprovalsUpToDate bool want }{ "InValidInput": { @@ -1359,12 +1492,97 @@ func TestUpdate(t *testing.T) { ), }, }, + "ApprovalsNotManagedSkipped": { + args: args{ + project: &fake.MockClient{ + MockEditProject: func(pid interface{}, opt *gitlab.EditProjectOptions, options ...gitlab.RequestOptionFunc) (*gitlab.Project, *gitlab.Response, error) { + return &gitlab.Project{}, &gitlab.Response{}, nil + }, + // No approvals mocks needed - Approvals is not set in spec, so + // ChangeApprovalConfiguration must not be called. + }, + cr: project(withStatus(v1alpha1.ProjectObservation{ID: 1234})), + }, + cacheIsApprovalsUpToDate: false, + want: want{ + cr: project(withStatus(v1alpha1.ProjectObservation{ID: 1234})), + }, + }, + "ApprovalsUpToDateSkipped": { + args: args{ + project: &fake.MockClient{ + MockEditProject: func(pid interface{}, opt *gitlab.EditProjectOptions, options ...gitlab.RequestOptionFunc) (*gitlab.Project, *gitlab.Response, error) { + return &gitlab.Project{}, &gitlab.Response{}, nil + }, + // No approvals mocks needed - already up to date per cache. + }, + cr: project( + withStatus(v1alpha1.ProjectObservation{ID: 1234}), + withApprovals(&v1alpha1.Approvals{ResetApprovalsOnPush: ptr.To(true)}), + ), + }, + cacheIsApprovalsUpToDate: true, + want: want{ + cr: project( + withStatus(v1alpha1.ProjectObservation{ID: 1234}), + withApprovals(&v1alpha1.Approvals{ResetApprovalsOnPush: ptr.To(true)}), + ), + }, + }, + "SuccessfulUpdateApprovals": { + args: args{ + project: &fake.MockClient{ + MockEditProject: func(pid interface{}, opt *gitlab.EditProjectOptions, options ...gitlab.RequestOptionFunc) (*gitlab.Project, *gitlab.Response, error) { + return &gitlab.Project{}, &gitlab.Response{}, nil + }, + MockChangeApprovalConfiguration: func(pid interface{}, opt *gitlab.ChangeApprovalConfigurationOptions, options ...gitlab.RequestOptionFunc) (*gitlab.ProjectApprovals, *gitlab.Response, error) { + return &gitlab.ProjectApprovals{}, &gitlab.Response{}, nil + }, + }, + cr: project( + withStatus(v1alpha1.ProjectObservation{ID: 1234}), + withApprovals(&v1alpha1.Approvals{ResetApprovalsOnPush: ptr.To(true)}), + ), + }, + cacheIsApprovalsUpToDate: false, + want: want{ + cr: project( + withStatus(v1alpha1.ProjectObservation{ID: 1234}), + withApprovals(&v1alpha1.Approvals{ResetApprovalsOnPush: ptr.To(true)}), + ), + }, + }, + "FailedUpdateApprovals": { + args: args{ + project: &fake.MockClient{ + MockEditProject: func(pid interface{}, opt *gitlab.EditProjectOptions, options ...gitlab.RequestOptionFunc) (*gitlab.Project, *gitlab.Response, error) { + return &gitlab.Project{}, &gitlab.Response{}, nil + }, + MockChangeApprovalConfiguration: func(pid interface{}, opt *gitlab.ChangeApprovalConfigurationOptions, options ...gitlab.RequestOptionFunc) (*gitlab.ProjectApprovals, *gitlab.Response, error) { + return &gitlab.ProjectApprovals{}, &gitlab.Response{}, errBoom + }, + }, + cr: project( + withStatus(v1alpha1.ProjectObservation{ID: 1234}), + withApprovals(&v1alpha1.Approvals{ResetApprovalsOnPush: ptr.To(true)}), + ), + }, + cacheIsApprovalsUpToDate: false, + want: want{ + cr: project( + withStatus(v1alpha1.ProjectObservation{ID: 1234}), + withApprovals(&v1alpha1.Approvals{ResetApprovalsOnPush: ptr.To(true)}), + ), + err: errors.Wrap(errBoom, errUpdateApprovalsFailed), + }, + }, } for name, tc := range cases { t.Run(name, func(t *testing.T) { e := &external{kube: tc.kube, client: tc.project} e.cache.externalPushRules = tc.cacheExternalPushRules e.cache.isPushRulesUpToDate = tc.cachePushRulesUpToDate + e.cache.isApprovalsUpToDate = tc.cacheIsApprovalsUpToDate o, err := e.Update(context.Background(), tc.args.cr) if diff := cmp.Diff(tc.want.err, err, test.EquateErrors()); diff != "" {