-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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 submodule and tests for private clusters #69
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because we're autogenerating a bunch of code this changeset is pretty hefty. Are there specific areas you want reviewed more closely?
In addition there are some failing tests that look like they're still under development - do you want additional review at this time or do you want me to check back in a few days?
5eabbfb
to
70a324b
Compare
da87606
to
a466549
Compare
@adrienthebo Thanks for checking in, and apologies for the delay. This PR is now in a workable state, and is ready for review. This is indeed a hefty change, due to the consolidation of logic in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While there's a large diff the main changes in this PR just deal with templating out a few resource variables. Changes look good, test coverage looks solid, let's merge this.
Thanks @adrienthebo! @aaron-lane could you kindly dismiss your review (or re-review if you're so inclined) so I can merge this? |
Verified that this change does not cause any Terraform state changes when upgraded from master. |
…modules/feature/private-cluster-module Add submodule and tests for private clusters
No description provided.