✨ Assume prerequisite role on hub if initailize with aws-irsa #807
Conversation
jaswalkiranavtar
force-pushed
the
managed-cluster-identity-creator-role-changes
branch
2 times, most recently
from
1fd0cdf to
e63a013
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #807 +/- ##
==========================================
+ Coverage 63.78% 63.82% +0.04%
==========================================
Files 192 193 +1
Lines 18638 18673 +35
==========================================
+ Hits 11888 11918 +30
- Misses 5771 5774 +3
- Partials 979 981 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
jaswalkiranavtar
force-pushed
the
managed-cluster-identity-creator-role-changes
branch
4 times, most recently
from
e394c07 to
a21f7b4
Compare
Signed-off-by: Gaurav Jaswal <[email protected]>
jaswalkiranavtar
force-pushed
the
managed-cluster-identity-creator-role-changes
branch
from
a21f7b4 to
942f19b
Compare
pkg/common/helpers/parser.go
Outdated
|
|
||
| import "strings" | ||
|
|
||
| func GetAwsAccountIdAndClusterName(clusterArn string) (string, string) { |
There was a problem hiding this comment.
some comments on both of this func will be helpful.
There was a problem hiding this comment.
Checked that comments has been added.
| {{ if .ManagedClusterIdentityCreatorRole }} | ||
| annotations: | ||
| eks.amazonaws.com/role-arn: {{ .ManagedClusterIdentityCreatorRole }} | ||
| {{end}} |
pkg/common/helpers/parser.go
Outdated
| @@ -0,0 +1,15 @@ | |||
| package helpers | |||
|
|
|||
| import "strings" | |||
There was a problem hiding this comment.
you might want to change the file name to aws.go or awsparser.go
There was a problem hiding this comment.
Verified that it has been renamed to aws.go
| @@ -3,3 +3,7 @@ kind: ServiceAccount | |||
| metadata: | |||
| name: registration-controller-sa | |||
| namespace: {{ .ClusterManagerNamespace }} | |||
| {{ if .ManagedClusterIdentityCreatorRole }} | |||
| annotations: | |||
| eks.amazonaws.com/role-arn: {{ .ManagedClusterIdentityCreatorRole }} | |||
There was a problem hiding this comment.
this is not quite extensible, e.g. the key is specifically for eks. How do you think the keys/values could be configured from the template?
There was a problem hiding this comment.
I think same needs to be applied on klusterlet SA as well. Will address it in a seperate PR.
|
/approve some nit comment, I think we need to think about #807 (comment) for extensibility, but does not necessarily need to be resolved in this PR. |
jaswalkiranavtar
force-pushed
the
managed-cluster-identity-creator-role-changes
branch
2 times, most recently
from
78a5a95 to
f4828a3
Compare
Signed-off-by: Gaurav Jaswal <[email protected]>
jaswalkiranavtar
force-pushed
the
managed-cluster-identity-creator-role-changes
branch
from
f4828a3 to
6132fc7
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jaswalkiranavtar, mikeshng, qiujian16 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 |
openshift-merge-bot
bot
merged commit f62242d
into
open-cluster-management-io:main
Summary
If the hub is initialized with aws-irsa, it should assume a precreated IAM role with predefined named which will allow it permission to create IAM roles and policies for spokes that are trying to join.
Related issue(s)
Ref: #514