Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Auto accept for AWS based registration #822

Open
jaswalkiranavtar opened this issue Feb 4, 2025 · 7 comments
Open

Auto accept for AWS based registration #822

jaswalkiranavtar opened this issue Feb 4, 2025 · 7 comments
Labels
enhancement New feature or request

Comments

@jaswalkiranavtar
Copy link
Contributor

jaswalkiranavtar commented Feb 4, 2025

Describe the enhancement
Currently the AutoApproval feature for CSR registration has a feature auto-approve based on the username in CSR.

However there is no CSR in the picture for AWS registration, so we want to propose following for AWS registration:

  1. Add a new field called autoAcceptEKSClusters to registrationConfiguration.registrationDrivers under clustermanager.
  2. This will be array type as we can pass list of EKS cluster ARN patterns e.g. arn:aws:eks:us-west-2:123456789012:cluster/*
  3. Then in managedcluster controller, we only approve the ManagedCluster whose ARN matches with one of the patterns in autoAcceptEKSClusters, if ManagedClusterAutoApproval feature gate is enabled. The ManagedCluster ARN can be retrieved from annotation on ManagedCluster CR.

This will allow hub admin to restrict which trusted clusters they want to autoapprove based on the clusterARN. Please advise if this idea sounds ok to you before we implement anything.
cc: @mikeshng @qiujian16 @zhiweiyin318

@jaswalkiranavtar jaswalkiranavtar added the enhancement New feature or request label Feb 4, 2025
@qiujian16
Copy link
Member

cc @skeeey

I wonder what this ARN represents. could we have a more general concept of it? Having a field in API stating EKS is a bit vendor specific, and we should try to avoid that.

@jaswalkiranavtar
Copy link
Contributor Author

It stands for Amazon Resources Name. I think it is used to unique identity an EKS cluster in a customer's aws account.

Sure we can remove word EKS, does autoAcceptClusterARNPatterns sound ok?

@skeeey
Copy link
Member

skeeey commented Feb 5, 2025

How about we enhance the autoApproveUsers to autoApproveIdentities , it may like

autoApproveIdentities:
- identityType: ARNPattern
   value: arn:aws:eks:us-west-2:123456789012:cluster/*
- identityType: CSRUser
   value: user1

@jaswalkiranavtar
Copy link
Contributor Author

Please note that ARN is unique id or name of the EKS cluster, it is not a principle of any subject. It might not fit as an identity which is like an id of a principle.

@qiujian16
Copy link
Member

qiujian16 commented Feb 5, 2025

I think it is could be represented as the identity of a managed cluster, or a pattern of the identity. e.g. arn:aws:eks:us-west-2:123456789012:cluster is a certain pattern for a certain registration driver. So I would think there should be something similar as @skeeey suggests

approvedIdentities
- driver: CSR
   identities:
   - user1
   - user2
- driver: AWS
   identities:
   - arn:aws:eks:us-west-2:123456789012:cluster/*

@jaswalkiranavtar
Copy link
Contributor Author

To be clear, we are talking about replacing existing autoApproveUsers field in clustermanager with new field called approvedIdentities correct?

@qiujian16
Copy link
Member

yes, I think so, we can not directly deprecate the existing field, the two have to exist for a time period. And we can deprecate the old one finally.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants