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

[aws-eks] Can we remove the use of {cluster resource,kubectl} provider completely if the situation is changed? #11403

Closed
foriequal0 opened this issue Nov 11, 2020 · 5 comments
Assignees
Labels
@aws-cdk/aws-eks Related to Amazon Elastic Kubernetes Service guidance Question that needs advice or information.

Comments

@foriequal0
Copy link

foriequal0 commented Nov 11, 2020

❓ General Issue

This is a summary of my understandings and questions about why we don't use CfnCluster directly, and whether it'll be forever like this.
Correct me if I'm wrong. Thanks.

If I understand correctly, the following issues are major reasons why we have {cluster resource,kubectl} providers.

  1. EKS RBAC is tied to who created the cluster: [EKS] [request]: Add AdminRole option at cluster creation containers-roadmap#554
  2. CF doesn't support some features that REST supports(?)

So we use a cluster resource provider to delegate the creation/management of a EKS cluster.
We have 2 roles so far:

  • adminRole( == kubectlRole): Creates and manage EKS cluster, and issues kubectl command. For CF to automation. Cannot be changed.
  • mastersRole: For users. Users assume this. Can be chagned.

Quickstart Amazon EKS takes similar approach for this, but with custom AWSQS::EKS::Cluster type: https://github.com/aws-quickstart/quickstart-amazon-eks/
(I feel like re:inventing wheels around the limitations, not fixing the core problem. It looks like it would support import since it implements update handler, but more complex. Why it isn't public/upstream?)

The Question

  1. Would it be possible if situations changed?
    What if we have AdminRole option? What if we have EKS API to manage IAM permissions to a cluster?
    Even if without the fix, what if new eks.Cluster() spawns a nested stack that has a cloudFormationExecutionRoleArn: adminRole?
    Or can we put AWS::EKS::Cluster directly on our stack, and delegate only unsupported options such as endpointPrivateAccess?

  2. If all the restrictions are gone, so all the providers become obsolete, how do we migrate after that?
    Would it be simply setting the deletion policy to Retain and importing them to the main stack?

Environment

Other information

@foriequal0 foriequal0 added guidance Question that needs advice or information. needs-triage This issue or PR still needs to be triaged. labels Nov 11, 2020
@github-actions github-actions bot added the @aws-cdk/aws-eks Related to Amazon Elastic Kubernetes Service label Nov 11, 2020
@SomayaB SomayaB added feature-request A feature should be added or improved. and removed guidance Question that needs advice or information. labels Dec 8, 2020
@iliapolo iliapolo added guidance Question that needs advice or information. and removed feature-request A feature should be added or improved. labels Dec 13, 2020
@iliapolo
Copy link
Contributor

@foriequal0 Your analysis is correct.

If all the restrictions are gone, so all the providers become obsolete, how do we migrate after that?
Would it be simply setting the deletion policy to Retain and importing them to the main stack?

It might be, we would need to dive deeper into this. Right now it doesn't look like CloudFormation supports importing EKS clusters.

Regardless, the KubectlProvider, will still remain because it serves not just for aws auth, but also for applying general manifests.

We would also need to consider whether or not its worth the effort, do you have specific considerations or are you asking for the sake of maintainability of the EKS construct?

@iliapolo iliapolo added response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. and removed needs-triage This issue or PR still needs to be triaged. labels Dec 13, 2020
@foriequal0
Copy link
Author

Oh I thought CF had supported importing ELS clusters. I didn't know that. Thanks for pointing it out.
I was interested in general maintanability/refactorability of CDK and Cfn stacks. I was concerned about that I cannot freely rename/move/refactor resources in general. EKS was just an example of it since I had tried to use it.

@github-actions github-actions bot removed the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Dec 14, 2020
@iliapolo
Copy link
Contributor

I understand. As far as maintainability, while I think that using the CFN resource would provide some relief, at this point, i'm not sure the migration is worth it.

As for refactoring, this is known discussion point (which I see you also participated in :)), but I don't see how using CFN resources helps in that regard, apart from just reducing the number of resources/moving-parts, but it doesn't change anything inherently.

@iliapolo iliapolo added the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Dec 14, 2020
@foriequal0
Copy link
Author

I understand that. Thank you for replying the question :)

@iliapolo iliapolo removed the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Dec 14, 2020
@github-actions
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-eks Related to Amazon Elastic Kubernetes Service guidance Question that needs advice or information.
Projects
None yet
Development

No branches or pull requests

3 participants