Skip to content
This repository has been archived by the owner on Dec 10, 2024. It is now read-only.

Add project setting ci_pipeline_variables_minimum_override_role #2057

Merged
merged 5 commits into from
Nov 12, 2024
Merged

Add project setting ci_pipeline_variables_minimum_override_role #2057

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
c4c9b8a
Add project setting ci_pipeline_variables_minimum_override_role
vindvaki Nov 6, 2024
fc7656e
Use a dedicated type for pipeline variable override role
vindvaki Nov 11, 2024
b1a9eac
Add documentation comments for new type
vindvaki Nov 12, 2024
0a513db
Update projects_test.go
timofurrer Nov 12, 2024
b1400c5
Update types.go
timofurrer Nov 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions projects.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,7 @@ type Project struct {
MergePipelinesEnabled bool `json:"merge_pipelines_enabled"`
MergeTrainsEnabled bool `json:"merge_trains_enabled"`
RestrictUserDefinedVariables bool `json:"restrict_user_defined_variables"`
CIPipelineVariablesMinimumOverrideRole AccessControlValue `json:"ci_pipeline_variables_minimum_override_role"`
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I chose AccessControlValue to match existing code, but for what it's worth, the AccessControlValue type seems to have been originally intended for something completely different. Over time, it seems to have morphed into more of a catch-all string type for different access control values 🤷

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vindvaki - I agree, I'm not sure AccessControlValue is the right type here. Since the API seems like it's accepting a string value for the Access Level instead of the integer value (which is usually defined using the AccessLevelValue type), I think we should probably create a new type for it in types.go, especially since no_one_allowed seems to deviate from the normal no_one that's used elsewhere.

would you mind doing that? Otherwise this LGTM!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@RicePatrick sure, I've created a new type in fc7656e. Let me know what you think 🏓

Copy link
Contributor

@RicePatrick RicePatrick Nov 11, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vindvaki - I apologize for nitpicking, but would you mind adding a comment on the new type to match the values of the others within the file? I tried to push a commit for you, but I don't have permissions to do so :)

I'd suggest something like this:

// CIPipelineVariablesMinimumOverrideRoleValue represents an access control
// value used for managing access to the CI Pipeline Variable Override feature.
//
// GitLab API docs: https://docs.gitlab.com/ee/api/projects.html
type CIPipelineVariablesMinimumOverrideRoleValue = string

// List of available CIPipelineVariablesMinimumOverrideRoleValue levels.
//
// GitLab API docs: https://docs.gitlab.com/ee/api/projects.html
const (
	CIPipelineVariables_NoOneAllowedRole CIPipelineVariablesMinimumOverrideRoleValue = "no_one_allowed"
	CiPipelineVariables_OwnerRole        CIPipelineVariablesMinimumOverrideRoleValue = "owner"
	CiPipelineVariables_MaintainerRole   CIPipelineVariablesMinimumOverrideRoleValue = "maintainer"
	CIPipelineVariables_DeveloperRole    CIPipelineVariablesMinimumOverrideRoleValue = "developer"
)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@RicePatrick thanks, done 🏓

MergeCommitTemplate string `json:"merge_commit_template"`
SquashCommitTemplate string `json:"squash_commit_template"`
AutoDevopsDeployStrategy string `json:"auto_devops_deploy_strategy"`
Expand Down Expand Up @@ -854,6 +855,7 @@ type EditProjectOptions struct {
CIForwardDeploymentRollbackAllowed *bool `url:"ci_forward_deployment_rollback_allowed,omitempty" json:"ci_forward_deployment_rollback_allowed,omitempty"`
CISeperateCache *bool `url:"ci_separated_caches,omitempty" json:"ci_separated_caches,omitempty"`
CIRestrictPipelineCancellationRole *AccessControlValue `url:"ci_restrict_pipeline_cancellation_role,omitempty" json:"ci_restrict_pipeline_cancellation_role,omitempty"`
CIPipelineVariablesMinimumOverrideRole *AccessControlValue `url:"ci_pipeline_variables_minimum_override_role,omitempty" json:"ci_pipeline_variables_minimum_override_role,omitempty"`
ContainerExpirationPolicyAttributes *ContainerExpirationPolicyAttributes `url:"container_expiration_policy_attributes,omitempty" json:"container_expiration_policy_attributes,omitempty"`
ContainerRegistryAccessLevel *AccessControlValue `url:"container_registry_access_level,omitempty" json:"container_registry_access_level,omitempty"`
DefaultBranch *string `url:"default_branch,omitempty" json:"default_branch,omitempty"`
Expand Down
32 changes: 19 additions & 13 deletions projects_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -280,13 +280,14 @@ func TestListOwnedProjects(t *testing.T) {
func TestEditProject(t *testing.T) {
mux, client := setup(t)

var developerAccessLevel AccessControlValue = "developer"
var developerRole AccessControlValue = "developer"
opt := &EditProjectOptions{
CIRestrictPipelineCancellationRole: Ptr(developerAccessLevel),
CIRestrictPipelineCancellationRole: Ptr(developerRole),
CIPipelineVariablesMinimumOverrideRole: Ptr(developerRole),
}

// Store whether we've set the restrict value in our edit properly
restrictValueSet := false
// Store whether we've seen all the attributes we set
attributesFound := false

mux.HandleFunc("/api/v4/projects/1", func(w http.ResponseWriter, r *http.Request) {
testMethod(t, r, http.MethodPut)
Expand All @@ -298,7 +299,8 @@ func TestEditProject(t *testing.T) {
}

// Set the value to check if our value is included
restrictValueSet = strings.Contains(string(body), "ci_restrict_pipeline_cancellation_role")
attributesFound = strings.Contains(string(body), "ci_restrict_pipeline_cancellation_role") &&
strings.Contains(string(body), "ci_pipeline_variables_minimum_override_role")

// Print the start of the mock example from https://docs.gitlab.com/ee/api/projects.html#edit-project
// including the attribute we edited
Expand All @@ -313,15 +315,17 @@ func TestEditProject(t *testing.T) {
"http_url_to_repo": "http://example.com/diaspora/diaspora-project-site.git",
"web_url": "http://example.com/diaspora/diaspora-project-site",
"readme_url": "http://example.com/diaspora/diaspora-project-site/blob/main/README.md",
"ci_restrict_pipeline_cancellation_role": "developer"
"ci_restrict_pipeline_cancellation_role": "developer",
"ci_pipeline_variables_minimum_override_role": "developer"
}`)
})

project, resp, err := client.Projects.EditProject(1, opt)
assert.NoError(t, err)
assert.Equal(t, http.StatusOK, resp.StatusCode)
assert.Equal(t, true, restrictValueSet)
assert.Equal(t, developerAccessLevel, project.CIRestrictPipelineCancellationRole)
assert.Equal(t, true, attributesFound)
assert.Equal(t, developerRole, project.CIRestrictPipelineCancellationRole)
assert.Equal(t, developerRole, project.CIPipelineVariablesMinimumOverrideRole)
}

func TestListStarredProjects(t *testing.T) {
Expand Down Expand Up @@ -374,6 +378,7 @@ func TestGetProjectByID(t *testing.T) {
"ci_forward_deployment_enabled": true,
"ci_forward_deployment_rollback_allowed": true,
"ci_restrict_pipeline_cancellation_role": "developer",
"ci_pipeline_variables_minimum_override_role": "no_one_allowed",
"packages_enabled": false,
"build_coverage_regex": "Total.*([0-9]{1,3})%"
}`)
Expand All @@ -387,11 +392,12 @@ func TestGetProjectByID(t *testing.T) {
Cadence: "7d",
NextRunAt: &wantTimestamp,
},
PackagesEnabled: false,
BuildCoverageRegex: `Total.*([0-9]{1,3})%`,
CIForwardDeploymentEnabled: true,
CIForwardDeploymentRollbackAllowed: true,
CIRestrictPipelineCancellationRole: "developer",
PackagesEnabled: false,
BuildCoverageRegex: `Total.*([0-9]{1,3})%`,
CIForwardDeploymentEnabled: true,
CIForwardDeploymentRollbackAllowed: true,
CIRestrictPipelineCancellationRole: "developer",
CIPipelineVariablesMinimumOverrideRole: "no_one_allowed",
}

project, _, err := client.Projects.GetProject(1, nil)
Expand Down