fix: use Kubernetes configuration for name and version when available #805
Conversation
metacosm
commented
Jan 23, 2024
metacosm
commented
- fix: use Kubernetes configuration for name and version when available
- refactor: RBACRule should be used instead of PermissionRule
|
/cc @jcechace |
Fixes #801 Signed-off-by: Chris Laprun <claprun@redhat.com>
RBACRules should be used when the rules should apply to the service account of the Reconciler since they would also impact the generated Kubernetes manifests and be automatically propagated to the CSV, whereas the reverse is not true (i.e. PermissionRule will not appear in the generated Kubernetes manifests). Signed-off-by: Chris Laprun <claprun@redhat.com>
| /** | ||
| * Additional RBAC rules that need to be provided because they cannot be inferred automatically. Note that RBAC rules added | ||
| * to your reconciler via {@link RBACRule} should already be handled automatically, under the service account name | ||
| * associated with your Reconciler so this annotation should only be used to add additional rules to other service accounts | ||
| * or for rules that you don't want to appear in the generated Kubernetes manifests. | ||
| */ |
There was a problem hiding this comment.
Kudos for this. Javadocs should be used way more :)
@metacosm one thing I am still curious about is this being part of the CSVMetadata. Shouldn't this also have an impact when CSV bundle is not used? AFAIK the CSVMetadata class really is used just for CSV metadata generation, or am I wrong?
There was a problem hiding this comment.
RBACRule is not part of CSVMetadata and we still need permission rules on CSVMetadata in case you don't want something to be generated in the kubernetes manifests. Or am I misunderstanding your question?
There was a problem hiding this comment.
RBACRuleis not part ofCSVMetadataand we still need permission rules onCSVMetadatain case you don't want something to be generated in the kubernetes manifests. Or am I misunderstanding your question?
I see.. That makes sense. So you would use this when you don't want the permission in yourkubernetes.ymlbut still want to use it for CSV. Thanks!
/**
* @return the service account name for the permission rule. If not provided, it will use the default service account
* name.
*/
String serviceAccountName() default ""; Although this is not affected by this PR I have a question/note to add here. What is considered a "default service account"? When I incorrectly used |
Yes, this is confusing and something that I'm pondering. I will add some documentation here. |
Regarding your comment (#801 (comment)) I'm confused how these two relate together. We are still using QOSDK 6.3.5 which obviously doesn't include this fix. However the permission is added to the CSV just fine when only |
Signed-off-by: Chris Laprun <claprun@redhat.com>
Yes, that's how it should work. My previous comment was indeed wrong: I initially thought that a fix was needed for the RBAC rule to be propagated but that isn't the case. I will update the comment to reflect that. |
