pipeline: add renderRBAC func #494
Conversation
✅ Deploy Preview for kurator-dev canceled.
|
Signed-off-by: Xieql <[email protected]>
|
|
||
| // ServiceAccountName generates the service account name using the pipeline name and namespace. | ||
| func (rbac RBACConfig) ServiceAccountName() string { | ||
| return ServiceAccountNamePrefix + rbac.PipelineName + "-" + rbac.PipelineNamespace |
There was a problem hiding this comment.
seems we donot need the ServiceAccountNamePrefix
There was a problem hiding this comment.
And neither pipelinenamespace
There was a problem hiding this comment.
the ServiceAccountNamePrefix could be unnecessary if a dedicated namespace is configured specifically for Kurator pipelines.
NVM, in situations where Kurator Pipeline are deployed in more general namespaces, such as default, this prefix could be advantageous. It can assist users in quickly identifying which ServiceAccounts are linked to Kurator pipelines amongst many others."
Agreed on the redundancy of PipelineNamespace.
| const ( | ||
| ServiceAccountNamePrefix = "kurator-pipeline-robot-" | ||
| RoleBindingNameSuffix = "-binding" | ||
| ClusterRoleBindingNameSuffix = "-clusterbinding" |
There was a problem hiding this comment.
not such naming convention, clusterrolebinding or rolebinding is commonly used
Signed-off-by: Xieql <[email protected]>
|
/label tide/merge-method-squash |
|
|
||
| // RoleBindingName generates the role binding name using the service account name. | ||
| func (rbac RBACConfig) RoleBindingName() string { | ||
| return rbac.ServiceAccountName() + RoleBindingNameSuffix |
There was a problem hiding this comment.
nit: no need to add this suffix
Signed-off-by: Xieql <[email protected]>
|
|
||
| // ClusterRoleBindingName generates the cluster role binding name using the service account name. | ||
| func (rbac RBACConfig) ClusterRoleBindingName() string { | ||
| return rbac.ServiceAccountName() |
There was a problem hiding this comment.
Sorry just note this is cluster scope resource, if we create multiple clusterrolebinding, they conflict with same name.
Option one is to make it with different name, alternatively add different sa within the same clusterrolebinding.
Signed-off-by: Xieql <[email protected]>
| "kurator.dev/kurator/pkg/fleet-manager/manifests" | ||
| ) | ||
|
|
||
| var manifestFS = manifests.BuiltinOrDir("manifests") |
There was a problem hiding this comment.
I am not sure why we need to read the while manifest, it is costy
Signed-off-by: Xieql <[email protected]>
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: hzxuzhonghu The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
What type of PR is this?
/kind feature
What this PR does / why we need it:
part of #493