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] private endpoint doesn't work with looked up vpc #9542

Closed
iliapolo opened this issue Aug 8, 2020 · 2 comments · Fixed by #9544
Closed

[aws-eks] private endpoint doesn't work with looked up vpc #9542

iliapolo opened this issue Aug 8, 2020 · 2 comments · Fixed by #9544
Assignees
Labels
@aws-cdk/aws-eks Related to Amazon Elastic Kubernetes Service bug This issue is a bug. effort/small Small work item – less than a day of effort p1

Comments

@iliapolo
Copy link
Contributor

iliapolo commented Aug 8, 2020

It's impossible to configure a private access endpoint when using a looked up vpc (i.e Vpc.fromLookup).

Reproduction Steps

  1. Create a VPC using the following guide: https://docs.aws.amazon.com/batch/latest/userguide/create-public-private-vpc.html

  2. Deploy the following:

const vpc = ec2.Vpc.fromLookup(this, 'Vpc', {
  vpcId: '<VPC_ID>'
})

const cluster = new eks.Cluster(this, 'Cluster', {
  vpc: vpc,
  version: eks.KubernetesVersion.V1_16,
  defaultCapacity: 2,
  endpointAccess: eks.EndpointAccess.PRIVATE
});

cluster.addResource('config-map', {
  kind: 'ConfigMap',
  apiVersion: 'v1',
  data: {
    hello: 'world',
  },
  metadata: {
    name: 'config-map',
  },
});

What did you expect to happen?

Expected for the cluster to created and for the config map to be correctly applied

What actually happened?

Screen Shot 2020-08-08 at 9 57 46 PM

Environment

  • CLI Version :1.57.0
  • Framework Version:1.57.0
  • Node.js Version:
  • OS :
  • Language (Version):

Other

Following is an excerpt from CloudFormation resource for the kubectl handler, we can see that the subnet id's are indeed empty:

    "Handler886CB40B": {
      "Type": "AWS::Lambda::Function",
      "Properties": {
        ....
        "VpcConfig": {
          "SecurityGroupIds": [
            {
              "Ref": "referencetoekslookedupvpcPlaygroundClusterKubectlProviderSecurityGroupBFF8A75BGroupId"
            }
          ],
          "SubnetIds": []
        }
        ...
      },
    },

This is 🐛 Bug Report

@iliapolo iliapolo added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Aug 8, 2020
@github-actions github-actions bot added the @aws-cdk/aws-eks Related to Amazon Elastic Kubernetes Service label Aug 8, 2020
@iliapolo iliapolo self-assigned this Aug 8, 2020
@iliapolo iliapolo added p1 effort/small Small work item – less than a day of effort and removed needs-triage This issue or PR still needs to be triaged. labels Aug 8, 2020
@iliapolo
Copy link
Contributor Author

iliapolo commented Aug 8, 2020

After some investigation, it seems that the problem is:

privateSubnets.push(...this.vpc.selectSubnets(placement).subnets.filter(s => s instanceof ec2.PrivateSubnet));

For looked up VPC's the type of the class is not PrivateSubnet, which results in the filtering returning an empty list.

@JasperW01
Copy link

Hi @iliapolo , thx a lot for raising this. really appreciate.

Just my 2 cents on the subnet selections for the kubectl handler Lambda given the enterprise context:

i. In most cases, we will configure vpcSubnets when calling eks.Cluster or FargateCluster to create eks clusters, so we can make sure the ENIs of the EKS control plane will be put into the subnets as required. But regardless the vpcSubnets is set or not, I guess by default the kubectl handler Lambda can just be associated with the subnets associated with the EKS Control plane.

ii. However, that might not be enough. This kubectl handler Lambda fundamentally is applying kubectl/helm, so arguably it's mostly doing the tasks of the data plane. So it would be great if we have a separate subnet selection option in eks.Cluster & eks.FargateClusters specifically for the kubectl handler Lambda, sth like kubectlSubnets along with kubectlEnvironment.

Having said that, I fully appreciate that you need to address this issue by considering the whole thing holistically. So please feel free to ignore this suggestion if it's contradicting to other factors you might need to consider.

Thx a lot in advance. Jasper

@mergify mergify bot closed this as completed in #9544 Aug 10, 2020
mergify bot pushed a commit that referenced this issue Aug 10, 2020
…9544)

Fixes #9542 
Related #5383 

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@iliapolo iliapolo added this to the EKS Dev Preview milestone Aug 10, 2020
eladb pushed a commit that referenced this issue Aug 10, 2020
…9544)

Fixes #9542 
Related #5383 

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
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 bug This issue is a bug. effort/small Small work item – less than a day of effort p1
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants