generate: consider service accounts when generating a CSV #3610
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
openshift-ci-robot
added
the
do-not-merge/work-in-progress
openshift-ci-robot
requested review from
hasbro17,
rashmigottipati and
joelanford
estroz
commented
Jul 30, 2020
internal/generate/clusterserviceversion/clusterserviceversion_updaters.go
Outdated
Show resolved
Hide resolved
estroz
commented
Jul 30, 2020
internal/generate/clusterserviceversion/clusterserviceversion_updaters.go
Outdated
Show resolved
Hide resolved
estroz
force-pushed
the
bugfix/csv-role-bindings
branch
4 times, most recently
from
8ae4709 to
2389149
Compare
estroz
changed the title
openshift-ci-robot
removed
the
do-not-merge/work-in-progress
92 tasks
estroz
force-pushed
the
bugfix/csv-role-bindings
branch
3 times, most recently
from
f99bba0 to
452d972
Compare
|
The algorithm for writing permissions, as a result of discussion in #3610 (comment) and with @joelanford offline:
|
`generate <bundle|packagemanifests>` input. These objects
will be written to the resulting bundle. For now, only Roles,
RoleBindings, their Cluster equivalents, and ServiceAccounts
are written.
internal/cmd/operator-sdk/generate: write RBAC objects to stdout
or files named with object.Name + GVK
internal/generate/{collector/clusterserviceversion}: consider
(cluster) role bindings so CSV generator can assign the correct
service account names to roles
estroz
force-pushed
the
bugfix/csv-role-bindings
branch
from
452d972 to
1adee98
Compare
joelanford
reviewed
Aug 1, 2020
internal/generate/clusterserviceversion/clusterserviceversion_test.go
Outdated
Show resolved
Hide resolved
estroz
force-pushed
the
bugfix/csv-role-bindings
branch
from
1adee98 to
5c7a47d
Compare
estroz
force-pushed
the
bugfix/csv-role-bindings
branch
2 times, most recently
from
14ec7f9 to
b4ec8e4
Compare
estroz
commented
Aug 1, 2020
| @@ -8,10 +8,10 @@ metadata: | |||
| "apiVersion": "cache.example.com/v1alpha1", | |||
There was a problem hiding this comment.
These testdata changes can be refactored into another PR, they're not directly related to the bugfix.
estroz
force-pushed
the
bugfix/csv-role-bindings
branch
from
b4ec8e4 to
8aebd0c
Compare
estroz
changed the title
This was referenced Aug 3, 2020
estroz
added a commit
to estroz/operator-sdk
that referenced
this pull request
Aug 10, 2020
estroz
added a commit
to estroz/operator-sdk
that referenced
this pull request
Aug 10, 2020
estroz
added a commit
to estroz/operator-sdk
that referenced
this pull request
Aug 10, 2020
estroz
pushed a commit
that referenced
this pull request
Aug 17, 2020
darkowlzz
added a commit
to darkowlzz/cluster-operator
that referenced
this pull request
Nov 5, 2020
The bundle generated by operator-sdk v0.17.0 contains incorrect cluster permission service account name. This issue is fixed in v0.19.0. Patch using yq with the correct value. Refer: operator-framework/operator-sdk#3610 Processing the CSV file via yq changes the indentations, resulting in unrelated changes to the file.
darkowlzz
added a commit
to storageos/cluster-operator
that referenced
this pull request
Nov 5, 2020
The bundle generated by operator-sdk v0.17.0 contains incorrect cluster permission service account name. This issue is fixed in v0.19.0. Patch using yq with the correct value. Refer: operator-framework/operator-sdk#3610 Processing the CSV file via yq changes the indentations, resulting in unrelated changes to the file.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of the change:
--update-crdsto--update-objectsMotivation for the change: This PR adds handling for extra RBAC objects present in
generate <bundle|packagemanifests>input. These objects will be written to the resulting bundle. For now, only Roles, RoleBindings, their Cluster equivalents, and ServiceAccounts are written.This PR also correctly names service account for (cluster) role permissions. These are currently incorrect because the CSV generator is naively using (cluster) role names instead of actual service account names. Previously this was ok because the names match the service account, but this is no longer the case. See #3600.
Old test data has been removed, and a static
basic.operator.yamlcontaining the output ofkustomize build config/manifestsadded; the static file's contents match a current project manifest build.Checklist
If the pull request includes user-facing changes, extra documentation is required:
changelog/fragments(seechangelog/fragments/00-template.yaml)website/content/en/docs