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

feat(aws): Flag to allow launching private thirdparty AMIs #1603

Merged
merged 1 commit into from
Apr 26, 2017

Conversation

robzienert
Copy link
Member

Some teams need the ability to launch private third-party AMIs. We don't want this capability allowed across the board, so I've added a flag that allows us to enable this on a per-account basis, which fits in well with the model that security has setup.

@spinnaker/netflix-reviewers PTAL

@anotherchrisberry
Copy link
Contributor

LGTM

Copy link
Contributor

@cfieber cfieber left a comment

Choose a reason for hiding this comment

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

LGTM after comments addressed and tests fixed

@@ -150,6 +149,7 @@ public void setDefaultResult(String defaultResult) {
private String assumeRole;
private String sessionName;
private List<LifecycleHook> lifecycleHooks;
private Boolean allowLaunchDescription;
Copy link
Contributor

Choose a reason for hiding this comment

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

You need to name this the same as the attribute on NetflixAmazonCredentials (it gets ObjectMapper.convertValue'd over there)

@@ -49,6 +49,7 @@
private final List<AWSRegion> regions;
private final List<String> defaultSecurityGroups;
private final List<LifecycleHook> lifecycleHooks;
private final Boolean allowPrivateThirdPartyImages;
Copy link
Contributor

Choose a reason for hiding this comment

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

can you make this little b boolean and assign a default value in the constructor? If we want to use this in non-groovy code it would be nice to not have to null check it

@robzienert robzienert merged commit ccea093 into spinnaker:master Apr 26, 2017
lwander pushed a commit to lwander/clouddriver that referenced this pull request Aug 8, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants