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

Add support for endpoint access control and custom cluster securitygroup #154

Merged
merged 4 commits into from
Jun 20, 2019

Conversation

lukehoban
Copy link
Member

@lukehoban lukehoban commented Jun 7, 2019

Expose these options through to the aws.eks.Cluster.

Fixes #86.

@metral
Copy link
Contributor

metral commented Jun 8, 2019

Per https://docs.aws.amazon.com/eks/latest/userguide/cluster-endpoint.html#modify-endpoint-access, when endpointPrivateAccess = true and endpointPublicAccess = false we have no internet access to the cluster. This means that to test this in tests requires a VM in the same VPC issuing the commands to the APIserver - either via kubectl or client-go. We should consider adding VM + test to our test suite.

Also per the defaults here of undefined for both if not provided - this does not match the state of what EKS defaults to (public: true, private: false) - do you foresee any issues with this mismatch?

@lukehoban
Copy link
Member Author

This means that to test this in tests requires a VM in the same VPC issuing the commands to the APIserver - either via kubectl or client-go

It's actually even worse than that - we won't even be able to successfully deploy resources into the Kubernetes cluster from the machine running pulumi up as needed to complete initialization - so the deployment will fail. I was just about to reply with some options on this on #86 (comment).

@lukehoban
Copy link
Member Author

Also per the defaults here of undefined for both if not provided - this does not match the state of what EKS defaults to (public: true, private: false) - do you foresee any issues with this mismatch?

It passes through the underlying defaults - we'll pass undefined (not present) which will trigger the underlying default value. Right?

@metral
Copy link
Contributor

metral commented Jun 8, 2019

It passes through the underlying defaults - we'll pass undefined (not present) which will trigger the underlying default value. Right?

Ya you're correct - I initially skipped over the undefined more than I should've and that's what caused me confusion. Ignore this comment 👍

@metral
Copy link
Contributor

metral commented Jun 8, 2019

From #86 (comment):

Ensure that users can provide a custom Cluster security group so that they can configure ingress with the CIDR blocks, IPs or other SGs they want to have access (and document that deployments will only be possible if done from an IP within one of these).

It'd be really great to see new examples added to the examples directory of privateAccess clusters with an accompanying VM in the same the VPC, for each of the config scenarios of the custom secgroup you list out (CIDR, IP, and other SGs).

@metral metral added this to the 0.24 milestone Jun 8, 2019
@lukehoban
Copy link
Member Author

It'd be really great to see new examples added to the examples directory of privateAccess clusters with an accompanying VM in the same the VPC

Yeah - I'll add a test before merging this. It's more involved that I initially expected unfortunately - it'll require setting up VPN endpoints inside the AWS VPC and creating a VPN connection from the test machine (or Travis). That's going to add non-trivial complexity to the test environment.

@lukehoban lukehoban changed the title Add support for endpointPrivateAccess and endpointPublicAccess Add support for endpoint access control and custom cluster securitygroup Jun 12, 2019
Espose these options through to the aws.eks.Cluster.

Fixes #86.
Users can now provide their own security group to apply to the cluster endpoint.
Copy link
Contributor

@metral metral left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. We'll want to rebase this PR off master once #162 get's merged.

@lukehoban
Copy link
Member Author

@metral After spending some time thinking about this - it's actually going to be exceedingly hard to test private-only endpoint access - as it will require VPN'ing the test machine into the VPC - or running the pulumi deployment from a bastion host in the VPC - neither of which is going to be easy to do in our current test/CI setup.

Given that this is mostly just passing flags through to EKS, I'm inclined to merge this to unblock all the usage scenarios here (including private+public) - and to open a follow-up issue to add test coverage (and examples) of private-only endpoint access.

Thoughts?

@metral
Copy link
Contributor

metral commented Jun 17, 2019

Given that this is mostly just passing flags through to EKS, I'm inclined to merge this to unblock all the usage scenarios here (including private+public) - and to open a follow-up issue to add test coverage (and examples) of private-only endpoint access.

That SGTM @lukehoban. I feel like the follow-up issue for test coverage & examples applies to not only pulumi/eks, but generally speaking, @pulumi/pulumi also for testing updates & flow from a bastion + VPC + private subnets client or CI setup.

@lukehoban lukehoban merged commit d729833 into master Jun 20, 2019
@pulumi-bot pulumi-bot deleted the lukehoban/endpointaccess branch June 20, 2019 18:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for cluster endpoint access control
2 participants