Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(roles): System ProviderRevision named clusterRoles should have labels #4716

Merged
merged 1 commit into from
Oct 3, 2023

Conversation

Mitsuwa
Copy link
Contributor

@Mitsuwa Mitsuwa commented Oct 2, 2023

Description of your changes

Fixes #4160

This adds the provider revision name as a label with the key that was initially discussed in the issue

rbac.crossplane.io/system

This pr.GetName() is used throughout this roles.go and prior to its use here so it shouldnt be an issue to use it to label these cluster roles. The tests also show that this change

I have:

  • Read and followed Crossplane's contribution process.
  • Added or updated unit and E2E tests for my change.
  • Run make reviewable to ensure this PR is ready for review.
...
ok  	github.com/crossplane/crossplane/apis/apiextensions/v1	0.236s	coverage: 17.3% of statements
testing: warning: no tests to run
PASS
coverage: 1.4% of statements
ok  	github.com/crossplane/crossplane/apis/pkg/meta/v1beta1	0.093s	coverage: 1.4% of statements [no tests to run]
?   	github.com/crossplane/crossplane/apis/pkg/v1alpha1	[no test files]
?   	github.com/crossplane/crossplane/apis/secrets	[no test files]
?   	github.com/crossplane/crossplane/apis/secrets/v1alpha1	[no test files]
testing: warning: no tests to run
PASS
coverage: 1.0% of statements
ok  	github.com/crossplane/crossplane/apis/pkg/v1	0.216s	coverage: 1.0% of statements [no tests to run]
testing: warning: no tests to run
PASS
coverage: 1.4% of statements
ok  	github.com/crossplane/crossplane/apis/pkg/v1beta1	0.205s	coverage: 1.4% of statements [no tests to run]
19:24:19 [ OK ] go test unit-test
  • Added backport release-x.y labels to auto-backport this PR, if necessary.
    • this does not need to be backported
  • Opened a PR updating the docs, if necessary.

@Mitsuwa Mitsuwa requested review from negz and a team as code owners October 2, 2023 22:42
@Mitsuwa Mitsuwa requested a review from pedjak October 2, 2023 22:42
…bels associated to them

adjust key providername to be called 'system' instead of 'provider' as it is the name discussed in the issue

remove the newline stuff

Signed-off-by: Bryan Nobuhara <bnobuhara@gmail.com>
@negz
Copy link
Member

negz commented Oct 2, 2023

This does not seem to exist or am i missing something?

@Mitsuwa Try just running make. If it's your first time running it, it should detect that it needs to pull in some dependencies (specifically, pull in http://github.com/upbound/build as a git submodule). Once that's done make reviewable should exist.

@Mitsuwa
Copy link
Contributor Author

Mitsuwa commented Oct 2, 2023

ty @negz, i was able to run make and make reviewable now, i am uncertain why the composition-webhook-schema-validation test is failing, from what I can tell it seems that it is not related to this pr but also broken in master

@phisco phisco merged commit b546d91 into crossplane:master Oct 3, 2023
15 of 18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

System Providerrevision named ClusterRoles should have Labels associated to them
3 participants