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.

Sign up for GitHub

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

Jump to bottom

Change service account default to create a new SA per cluster #88

Merged
merged 14 commits into from
Apr 5, 2019
Merged

Change service account default to create a new SA per cluster #88

merged 14 commits into from
Apr 5, 2019

Conversation

Jberlinsky
Copy link
Contributor

Per #80

adrienthebo
adrienthebo reviewed Mar 5, 2019
View reviewed changes
Copy link
Contributor

@adrienthebo adrienthebo left a comment

Choose a reason for hiding this comment

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

Soft +1, will switch to hard +1 and merge when CI is green.

@adrienthebo
Copy link
Contributor

Upon closer review I wasn't able to find integration tests verifying the presence of the created service accounts, is this a worthwhile addition?

@aaron-lane aaron-lane self-assigned this Apr 3, 2019
@aaron-lane aaron-lane force-pushed the feature/default-to-creating-new-service-account-for-cluster branch from de1a787 to 98e8640 Compare April 3, 2019 15:28
@aaron-lane aaron-lane changed the base branch from master to aaron-lane-v2.0.0 April 4, 2019 22:52
@aaron-lane aaron-lane force-pushed the feature/default-to-creating-new-service-account-for-cluster branch from 1326847 to 96bb056 Compare April 4, 2019 22:58
Jberlinsky and others added 13 commits April 5, 2019 11:21
@Jberlinsky @aaron-lane
Change service account default to create a new SA per cluster
c4e2b36
@aaron-lane
Clarify description of var.service_account
e5a00cf
@aaron-lane
Add Changelog entry
5517a3e 
Since this change is breaking, it can't be merged to master as
Unreleased; it must be scheduled for v2.0.0.
@aaron-lane
Add test for generated service account
619937d
@aaron-lane
Add InSpec GCP to simple_zonal profile
ae13dbd
@aaron-lane
Add service_account_resource_name to outputs
8efe9f4
@aaron-lane
Isolate service_account_resource_name test output
cbfc3c7
@aaron-lane
Update service account output description
b36c1b6
@aaron-lane
Fix example module output reference
febbacc
@aaron-lane
Update service_account description and regenerate
086f686
@aaron-lane
service_account_resource_name -> service_account_name
5a0a3b6
@aaron-lane
Add section to upgrading guide
cad247c
@aaron-lane
Handle service account name in control
177991a 
The ambiguity of what google_container_cluster expects as a value for
service_account is not worth addressing in this module.
@aaron-lane aaron-lane force-pushed the feature/default-to-creating-new-service-account-for-cluster branch from ab7a251 to 177991a Compare April 5, 2019 17:20
@morgante
Copy link
Contributor

morgante commented Apr 5, 2019

@Jberlinsky This looks fine to me, but make generate is showing a diff.

@aaron-lane aaron-lane merged commit 5fd2a31 into aaron-lane-v2.0.0 Apr 5, 2019
@aaron-lane aaron-lane deleted the feature/default-to-creating-new-service-account-for-cluster branch April 5, 2019 19:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@adrienthebo adrienthebo adrienthebo left review comments

@morgante morgante morgante approved these changes

@aaron-lane aaron-lane Awaiting requested review from aaron-lane

Assignees

@aaron-lane aaron-lane

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@Jberlinsky @adrienthebo @morgante @aaron-lane