Skip to content

CCO-430: Use per-project custom roles instead of per-cluster custom roles#611

Merged
openshift-ci[bot] merged 1 commit intoopenshift:masterfrom
dlom:CCO-430
Nov 2, 2023
Merged

CCO-430: Use per-project custom roles instead of per-cluster custom roles#611
openshift-ci[bot] merged 1 commit intoopenshift:masterfrom
dlom:CCO-430

Conversation

@dlom
Copy link
Contributor

@dlom dlom commented Oct 25, 2023

Because CCO now manages custom roles per-project instead of per-cluster, the name format is fixed instead of random so that it all clusters within the same project will use the same custom roles.

Additionally, by default, custom roles will not be deleted. To delete per-project custom roles, pass the --force-delete-custom-roles flag to ccoctl.

xref: https://issues.redhat.com/browse/CCO-430

/assign @jstuever
/cc @abutcher

@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Oct 25, 2023

@dlom: This pull request references CCO-430 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.15.0" version, but no target version was set.

Details

In response to this:

Because CCO now manages roles per-account instead of per-cluster, the name is static and shared by all clusters. If no name is supplied, the default name is used in all cases.

Additionally, by default, attempts to delete per-account credentials resources will not be performed. To delete per-account credentials resources, pass the --force-delete flag.

TODO:

  • Scrub and warn about removals in permission deltas when updating a role

xref: https://issues.redhat.com/browse/CCO-430

/assign @jstuever
/cc @abutcher

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Oct 25, 2023
@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 25, 2023
@openshift-ci openshift-ci bot requested a review from abutcher October 25, 2023 23:20
@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 25, 2023
@codecov
Copy link

codecov bot commented Oct 25, 2023

Codecov Report

Merging #611 (9eee360) into master (c653b3b) will decrease coverage by 0.21%.
Report is 4 commits behind head on master.
The diff coverage is 45.58%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #611      +/-   ##
==========================================
- Coverage   48.10%   47.90%   -0.21%     
==========================================
  Files          96       96              
  Lines       11656    11680      +24     
==========================================
- Hits         5607     5595      -12     
- Misses       5435     5465      +30     
- Partials      614      620       +6     
Files Coverage Δ
pkg/gcp/actuator/actuator.go 57.71% <100.00%> (+0.24%) ⬆️
pkg/gcp/actuator/utils.go 82.22% <100.00%> (-1.46%) ⬇️
pkg/cmd/provisioning/gcp/create_all.go 0.00% <0.00%> (ø)
...visioning/gcp/create_workload_identity_provider.go 50.92% <0.00%> (ø)
pkg/gcp/actuator/policy.go 69.36% <0.00%> (-0.41%) ⬇️
...kg/cmd/provisioning/gcp/create_service_accounts.go 53.81% <50.00%> (-1.56%) ⬇️
pkg/gcp/actuator/role.go 60.81% <42.85%> (-16.24%) ⬇️
pkg/cmd/provisioning/gcp/delete.go 0.00% <0.00%> (ø)

... and 2 files with indirect coverage changes

@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Oct 27, 2023

@dlom: This pull request references CCO-430 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.15.0" version, but no target version was set.

Details

In response to this:

Because CCO now manages roles per-project instead of per-cluster, the name is static and shared by all clusters in the same project. If no name is supplied, the default name is used in all cases.

Additionally, by default, attempts to delete per-project credentials resources will not be performed. To delete per-project credentials resources, pass the --force-delete flag.

TODO:

  • Scrub and warn about removals in permission deltas when updating a role

xref: https://issues.redhat.com/browse/CCO-430

/assign @jstuever
/cc @abutcher

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@dlom dlom force-pushed the CCO-430 branch 3 times, most recently from e5c2163 to 346b7de Compare October 31, 2023 23:38
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Oct 31, 2023

@dlom: This pull request references CCO-430 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.15.0" version, but no target version was set.

Details

In response to this:

Because CCO now manages roles per-project instead of per-cluster, the name is static and shared by all clusters in the same project. If no name is supplied, the default name is used in all cases.

Additionally, by default, attempts to delete per-project credentials resources will not be performed. To delete per-project credentials resources, pass the --force-delete flag.

TODO:

  • Scrub and warn about removals in permission deltas when updating a role

xref: https://issues.redhat.com/browse/CCO-430

/assign @jstuever
/cc @abutcher

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@dlom dlom changed the title [WIP] CCO-430: Remove --name as a required flag and deprecate it CCO-430: Remove --name as a required flag and deprecate it Oct 31, 2023
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 31, 2023
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Oct 31, 2023

@dlom: This pull request references CCO-430 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.15.0" version, but no target version was set.

Details

In response to this:

Because CCO now manages custom roles per-project instead of per-cluster, the name format is fixed instead of random so that it all clusters within the same project will use the same custom roles.

Additionally, by default, custom roles will not be deleted. To delete per-project custom roles, pass the --force-delete-custom-roles flag to ccoctl.

xref: https://issues.redhat.com/browse/CCO-430

/assign @jstuever
/cc @abutcher

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@dlom dlom changed the title CCO-430: Remove --name as a required flag and deprecate it CCO-430: Use per-project custom roles instead of per-cluster custom roles Oct 31, 2023
@dlom
Copy link
Contributor Author

dlom commented Oct 31, 2023

/cc @sdodson can you give this a quick sanity check?

@dlom
Copy link
Contributor Author

dlom commented Oct 31, 2023

/cc @sdodson

@openshift-ci openshift-ci bot requested a review from sdodson October 31, 2023 23:42
@dlom
Copy link
Contributor Author

dlom commented Oct 31, 2023

/override ci/prow/security

lets see if I did that right...

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Oct 31, 2023

@dlom: Overrode contexts on behalf of dlom: ci/prow/security

Details

In response to this:

/override ci/prow/security

lets see if I did that right...

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

// is ready to receive the generated files, and will create the directory if necessary.
func validationForCreateAllCmd(cmd *cobra.Command, args []string) {
if len(CreateWorkloadIdentityPoolOpts.Name) > 32 {
if len(CreateAllOpts.Name) > 32 {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is unrelated to the topic of the PR, but the wrong variable was used here (and in a few other places)

Prior to this change, all custom roles were unique per-cluster, ensured
by randomizing the name.  Now, all clusters in a project will reuse the
same custom role, which has a stable name based on the project name and
the credentials request containing the permissions required.
@dlom
Copy link
Contributor Author

dlom commented Nov 1, 2023

/jira refresh

@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Nov 1, 2023

@dlom: This pull request references CCO-430 which is a valid jira issue.

Details

In response to this:

/jira refresh

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@dlom
Copy link
Contributor Author

dlom commented Nov 1, 2023

/override ci/prow/security

Security check is new and requires a seperate PR to get master in a clean state

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Nov 1, 2023

@dlom: Overrode contexts on behalf of dlom: ci/prow/security

Details

In response to this:

/override ci/prow/security

Security check is new and requires a seperate PR to get master in a clean state

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@dlom
Copy link
Contributor Author

dlom commented Nov 1, 2023

/override ci/prow/security

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Nov 1, 2023

@dlom: Overrode contexts on behalf of dlom: ci/prow/security

Details

In response to this:

/override ci/prow/security

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@jstuever
Copy link
Contributor

jstuever commented Nov 1, 2023

/test e2e-gcp-manual-oidc

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Nov 2, 2023

@dlom: all tests passed!

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@jstuever
Copy link
Contributor

jstuever commented Nov 2, 2023

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Nov 2, 2023
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Nov 2, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dlom, jstuever

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@jstuever
Copy link
Contributor

/cherry-pick release-4.14

@openshift-cherrypick-robot

@jstuever: new pull request created: #631

Details

In response to this:

/cherry-pick release-4.14

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

ming1013 pushed a commit to ming1013/cloud-credential-operator that referenced this pull request Dec 15, 2025
CCO-430: Use per-project custom roles instead of per-cluster custom roles
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants