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

(eks): Use L1 construct (CfnCluster) for L2 eks.Cluster #18620

Closed
2 tasks done
literalice opened this issue Jan 24, 2022 · 6 comments
Closed
2 tasks done

(eks): Use L1 construct (CfnCluster) for L2 eks.Cluster #18620

literalice opened this issue Jan 24, 2022 · 6 comments
Labels
@aws-cdk/aws-eks Related to Amazon Elastic Kubernetes Service closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. effort/large Large work item – several weeks of effort feature/new-construct A request for a new L2 construct feature-request A feature should be added or improved. p2

Comments

@literalice
Copy link

literalice commented Jan 24, 2022

Description

Currently L2 Cluster construct of eks module doesn't use L1 CfnCluster.
It's confusing, for example if users need to "escape hatches" adding overrides to L1 CfnCluster doen't work.

Use Case

  • Users can escape hatches like as standard L2 constructs.
  • It should make it easy to catch up with the new features of EKS.
    • For example IPv6 need to fix the corresponding custom resource but Lambda's build-in version boto library cannot create a IPv6 Cluster.

Proposed Solution

Sample implementation is here.

Simply use the CfnCluster in the L2 construct:

new eks.CfnCluster(this, "Resource", {
      resourcesVpcConfig: {
        securityGroupIds: [securityGroup.securityGroupId],
        subnetIds: this.vpc.privateSubnets.map(s => s.subnetId),
        endpointPrivateAccess: endpointAccess._config.privateAccess,
        endpointPublicAccess: endpointAccess._config.publicAccess,
      },
      roleArn: clusterRole.roleArn,
  
      // the properties below are optional
      kubernetesNetworkConfig: {
        ipFamily: props.ipFamily ? props.ipFamily : 'ipv4',
      },
// ...
    });

Currently EKS cluster uses the principal that created the cluster as the administrator to the cluster so Kubectl provider should also use the principal. Specifying the CloudFormation execution role and using the role as kubectlRole would be the one of the solutions to the issue.

const adminRole = new iam.Role(this, 'AdminRole', {
  assumedBy: new iam.AccountRootPrincipal(),
});
if (adminRole.assumeRolePolicy) {
  new iam.ServicePrincipal('cloudformation.amazonaws.com').addToAssumeRolePolicy(adminRole.assumeRolePolicy)
}
adminRole.addManagedPolicy(
  iam.ManagedPolicy.fromAwsManagedPolicyName('AdministratorAccess'));

new ClusterStack(app, 'ClusterStack', adminRole, {
  synthesizer: new cdk.DefaultStackSynthesizer({
    cloudFormationExecutionRole: process.env.CLUSTER_ADMIN_ROLE_ARN,
  }),
});
import { aws_eks as eks } from "aws-cdk-lib";
import { Cluster } from "@literalice/aws-cdk-eks-cfn";

export class ClusterStack extends Stack {
  constructor(scope: Construct, id: string, adminRole: iam.IRole, props?: StackProps) {
    super(scope, id, props);

    // ...

    const cluster = new Cluster(this, 'Cluster', {
      vpc,
      clusterName,
      endpointAccess: eks.EndpointAccess.PUBLIC_AND_PRIVATE,
      adminRole,
      mastersRole,
      ipFamily: 'ipv6',
      version: eks.KubernetesVersion.V1_21,
    });
  }
}

Other information

No response

Acknowledge

  • I may be able to implement this feature request
  • This feature might incur a breaking change
@literalice literalice added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Jan 24, 2022
@github-actions github-actions bot added the @aws-cdk/aws-eks Related to Amazon Elastic Kubernetes Service label Jan 24, 2022
@otaviomacedo
Copy link
Contributor

Hi, @literalice. If you look at the custom resource handler that is responsible for managing the lifecycle of EKS clusters, you'll see that there is a lot of custom logic involved. If we used the L1 directly, we would lose that.

@peterwoodworth
Copy link
Contributor

@otaviomacedo have we considered offering an alternate L2 construct, or an alternate EKS library for constructs which utilize the L1 EKS constructs? I agree with both of the use cases OP listed as valid reasons to want this.

@peterwoodworth peterwoodworth added effort/large Large work item – several weeks of effort p2 feature/new-construct A request for a new L2 construct and removed needs-triage This issue or PR still needs to be triaged. labels Jan 24, 2022
@literalice
Copy link
Author

Thank you for your comments, I understand the EKS custom resource requires logics for managing the cluster's lifecycle.
I think it could be an option to delegate the lifecycle management to CloudFormation native EKS resource.
Obviously it'll break the behavior of the eks.Cluster construct. I agree with the idea of creating new L2 construct for it.
Could you give me opinions about validity of introducing the feature?

If it seems to be reasonable, the way to implementation would be following.

  • Creating the L2 construct 'ClusterNative' or something in sample construct library.
    • Or creating aws-eks-alpha for experimental features.
  • After reviewing the stability, move it to aws-eks module.

or

  • aws-eks-native module for experimental features.
  • Creating the L2 construct in the module.
  • After reviewing the stability, make it a stable version.

@otaviomacedo
Copy link
Contributor

@peterwoodworth Yes, that's fair. We're looking into possible solutions for this. One of them is to write Amazon public extensions, which would remove the need for custom resources and provide all the features. It's still early stages, so I don't have any details at this point. But I'll keep the community updated.

@otaviomacedo otaviomacedo removed their assignment Jan 31, 2022
@github-actions
Copy link

This issue has not received any attention in 1 year. If you want to keep this issue open, please leave a comment below and auto-close will be canceled.

@github-actions github-actions bot added closing-soon This issue will automatically close in 4 days unless further comments are made. closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. and removed closing-soon This issue will automatically close in 4 days unless further comments are made. labels Jan 31, 2023
@github-actions github-actions bot closed this as completed Feb 5, 2023
@diranged
Copy link

diranged commented Feb 5, 2023

I'd rally like to see this re-opened

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 closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. effort/large Large work item – several weeks of effort feature/new-construct A request for a new L2 construct feature-request A feature should be added or improved. p2
Projects
None yet
Development

No branches or pull requests

4 participants