✨ Adding second group for aws auth flow #735
Conversation
Signed-off-by: Gaurav Jaswal <[email protected]>
|
/assign @qiujian16 |
Yes, we will add them later when we implement hub side changes. Hopefully, this addition should not break anything existing. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #735 +/- ##
==========================================
- Coverage 63.51% 63.32% -0.19%
==========================================
Files 185 186 +1
Lines 17838 17912 +74
==========================================
+ Hits 11329 11343 +14
- Misses 5576 5634 +58
- Partials 933 935 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
Looks like the e2e test failed because of intermittent error. Could you please re-run this build? |
Re-running as requested. |
| @@ -12,3 +12,6 @@ subjects: | |||
| - kind: Group | |||
| apiGroup: rbac.authorization.k8s.io | |||
| name: system:open-cluster-management:{{ .ManagedClusterName }} | |||
| - kind: Group | |||
There was a problem hiding this comment.
please add some comments on the reasoning and plan for migration.
There was a problem hiding this comment.
We are adding a second group, as for aws flow we are trying to use eks access entries which doesn't allow us to allow access to group with system prefix "system".
Also based on kubernetes docs, "system" prefix is a reserved prefix for kubernetes internal usage.
The reason to add second group is to not update existing group abruptly and cause failure while upgrade existing installation to newer version of OCM. So, the plan is adding second group now to support aws flow and later in the next version, delete old group.
There was a problem hiding this comment.
Also request you to drive the migration of existing group for csr flow on your end.
|
cc @elgnay |
|
something to followup:
|
|
LGTM |
|
/approve |
|
[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 a138a54
into
open-cluster-management-io:main
Summary
Adding a second group, that will be used when the spoke starts registration with aws auth type. The aws solution uses EKS access entries and an access entry cannot contain a group name with "system:" prefix. More details can be found on this slack thread.
Related issue(s)
Fixes # #514