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

Added a check for AMI encryption #889

Merged
merged 5 commits into from Jun 17, 2019

Conversation

@adamjohnson01
Copy link
Contributor

commented Jun 17, 2019

Fix for #858

I have added a check to see if the ami being used is encrypted and updated the cloudformation template to include the value of that check. This is required to enable the use of encrypted amis

  • Code compiles correctly (i.e make build)
  • Added tests that cover your change (if possible)
  • All unit tests passing (i.e. make test)
  • Manually tested
@martina-if
Copy link
Member

left a comment

Thank you for the PR! Looks great! Can you add a little test that shows that when we enable it the right field appear in the cloud formation resource? Perhaps just switching the existing test to true and checking that the launchtemplate is correct?

pkg/apis/eksctl.io/v1alpha5/types.go Outdated Show resolved Hide resolved
pkg/cfn/builder/api_test.go Outdated Show resolved Hide resolved
@errordeveloper
Copy link
Member

left a comment

LGTM overall!

Added test for Encrypted field, moved Ebs specific tests out of the t…
…est for PrivateNetworking and SSH into their own test, removed redundant config
@adamjohnson01

This comment has been minimized.

Copy link
Contributor Author

commented Jun 17, 2019

Thank you for the PR! Looks great! Can you add a little test that shows that when we enable it the right field appear in the cloud formation resource? Perhaps just switching the existing test to true and checking that the launchtemplate is correct?

@martina-if , I have added a new test. I have also split out the Ebs tests that were already there into their own test. I hope that is OK.

@errordeveloper

This comment has been minimized.

Copy link
Member

commented Jun 17, 2019

@adamjohnson01 could you please rebase, run make generate-kubernetes-types and commit updated files? After that we can merge.

@martina-if

This comment has been minimized.

Copy link
Member

commented Jun 17, 2019

@adamjohnson01 even better. This looks great, thanks!

@errordeveloper

This comment has been minimized.

Copy link
Member

commented Jun 17, 2019

I'll merge and run make generate-kubernetes-types later.

@errordeveloper errordeveloper merged commit c02a069 into weaveworks:master Jun 17, 2019

3 checks passed

WIP Ready for review
Details
ci/circleci: make-eksctl-image Your tests passed on CircleCI!
Details
netlify/brave-ritchie-0c2cd8/deploy-preview Deploy preview ready!
Details

@adamjohnson01 adamjohnson01 deleted the adamjohnson01:encrypted-ami branch Jun 17, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.