From 37334028f638990753b71db45eb7649671ccf88d Mon Sep 17 00:00:00 2001 From: Justin Field Date: Tue, 11 Dec 2018 09:26:00 -0800 Subject: [PATCH] Remove ami tag check logic and don't run x-region config validation when the environment doesn't exist (#128) --- gradle.properties | 2 +- .../com/nike/cerberus/cli/CerberusRunner.java | 7 +- .../command/cms/CreateCmsClusterCommand.java | 13 +- .../cms/CreateCmsClusterOperation.java | 10 -- .../operation/core/UpdateStackOperation.java | 14 +- .../cerberus/service/AmiTagCheckService.java | 98 ------------ .../service/AmiTagCheckServiceTest.java | 149 ------------------ 7 files changed, 9 insertions(+), 284 deletions(-) delete mode 100644 src/main/java/com/nike/cerberus/service/AmiTagCheckService.java delete mode 100644 src/test/java/com/nike/cerberus/service/AmiTagCheckServiceTest.java diff --git a/gradle.properties b/gradle.properties index aa28fd10..e529fee9 100644 --- a/gradle.properties +++ b/gradle.properties @@ -16,4 +16,4 @@ group=com.nike artifactId=cerberus-lifecycle-cli -version=4.6.4 +version=4.7.0 diff --git a/src/main/java/com/nike/cerberus/cli/CerberusRunner.java b/src/main/java/com/nike/cerberus/cli/CerberusRunner.java index 376ffdd7..a3e4bb28 100644 --- a/src/main/java/com/nike/cerberus/cli/CerberusRunner.java +++ b/src/main/java/com/nike/cerberus/cli/CerberusRunner.java @@ -19,6 +19,7 @@ import ch.qos.logback.classic.Level; import com.beust.jcommander.JCommander; import com.github.tomaslanger.chalk.Chalk; +import com.google.common.collect.ImmutableList; import com.google.common.collect.Maps; import com.google.inject.Guice; import com.google.inject.Injector; @@ -131,7 +132,9 @@ public void run(String[] args) { validator.validate(); ConfigStore configStore = injector.getInstance(ConfigStore.class); - if (!cerberusCommand.isSkipDataCheck() && !configStore.isConfigSynchronized()){ + if (!ImmutableList.of(CreateEnvironmentCommand.COMMAND_NAME, InitializeEnvironmentCommand.COMMAND_NAME) + .contains(commandName) && !cerberusCommand.isSkipDataCheck() && !configStore.isConfigSynchronized()) { + ConsoleService consoleService = injector.getInstance(ConsoleService.class); String msg = "The config buckets are out of sync between regions. Do you wish to proceed?"; if (cerberusCommand.isTty()) { @@ -274,4 +277,4 @@ public static void main(String[] args) { CerberusRunner runner = new CerberusRunner(); runner.run(args); } -} \ No newline at end of file +} diff --git a/src/main/java/com/nike/cerberus/command/cms/CreateCmsClusterCommand.java b/src/main/java/com/nike/cerberus/command/cms/CreateCmsClusterCommand.java index 124253a0..7115a930 100644 --- a/src/main/java/com/nike/cerberus/command/cms/CreateCmsClusterCommand.java +++ b/src/main/java/com/nike/cerberus/command/cms/CreateCmsClusterCommand.java @@ -16,7 +16,6 @@ package com.nike.cerberus.command.cms; -import com.beust.jcommander.Parameter; import com.beust.jcommander.Parameters; import com.beust.jcommander.ParametersDelegate; import com.nike.cerberus.command.Command; @@ -24,8 +23,6 @@ import com.nike.cerberus.operation.Operation; import com.nike.cerberus.operation.cms.CreateCmsClusterOperation; -import static com.nike.cerberus.ConfigConstants.SKIP_AMI_TAG_CHECK_ARG; -import static com.nike.cerberus.ConfigConstants.SKIP_AMI_TAG_CHECK_DESCRIPTION; import static com.nike.cerberus.command.cms.CreateCmsClusterCommand.COMMAND_NAME; /** @@ -48,16 +45,8 @@ public String getCommandName() { return COMMAND_NAME; } - @Parameter(names = SKIP_AMI_TAG_CHECK_ARG, - description = SKIP_AMI_TAG_CHECK_DESCRIPTION) - private boolean skipAmiTagCheck; - - public boolean isSkipAmiTagCheck() { - return skipAmiTagCheck; - } - @Override public Class> getOperationClass() { return CreateCmsClusterOperation.class; } -} \ No newline at end of file +} diff --git a/src/main/java/com/nike/cerberus/operation/cms/CreateCmsClusterOperation.java b/src/main/java/com/nike/cerberus/operation/cms/CreateCmsClusterOperation.java index 71b5c0d1..2f03a780 100644 --- a/src/main/java/com/nike/cerberus/operation/cms/CreateCmsClusterOperation.java +++ b/src/main/java/com/nike/cerberus/operation/cms/CreateCmsClusterOperation.java @@ -22,7 +22,6 @@ import com.nike.cerberus.domain.environment.CertificateInformation; import com.nike.cerberus.domain.environment.Stack; import com.nike.cerberus.operation.Operation; -import com.nike.cerberus.service.AmiTagCheckService; import com.nike.cerberus.service.CloudFormationService; import com.nike.cerberus.service.Ec2UserDataService; import com.nike.cerberus.store.ConfigStore; @@ -49,8 +48,6 @@ public class CreateCmsClusterOperation implements Operation { private final Ec2UserDataService ec2UserDataService; private final String environmentName; private final ConfigStore configStore; - private final AmiTagCheckService amiTagCheckService; @Inject public UpdateStackOperation(CloudFormationService cloudFormationService, Ec2UserDataService ec2UserDataService, @Named(ENV_NAME) String environmentName, - AmiTagCheckService amiTagCheckService, ConfigStore configStore) { this.cloudFormationService = cloudFormationService; this.ec2UserDataService = ec2UserDataService; this.environmentName = environmentName; - this.amiTagCheckService = amiTagCheckService; this.configStore = configStore; } @@ -76,13 +72,7 @@ public void run(UpdateStackCommand command) { Optional.ofNullable(tags.getOrDefault("ownerGroup", null)))); } - if (Stack.CMS.equals(command.getStack()) && ! command.isSkipAmiTagCheck()) { - command.getDynamicParameters().forEach((key, value) -> { - if (key.equals("amiId")) { - amiTagCheckService.validateAmiTagForStack(value, Stack.CMS); - } - }); - } else if (Stack.DATABASE.equals(command.getStack())) { + if (Stack.DATABASE.equals(command.getStack())) { Optional dbPasswordOverwrite = command.getDynamicParameters().entrySet().stream() .filter(entry -> entry.getKey().equals("cmsDbMasterPassword")) .map(Map.Entry::getValue) @@ -141,4 +131,4 @@ public boolean isRunnable(UpdateStackCommand command) { return isRunnable; } -} \ No newline at end of file +} diff --git a/src/main/java/com/nike/cerberus/service/AmiTagCheckService.java b/src/main/java/com/nike/cerberus/service/AmiTagCheckService.java deleted file mode 100644 index 6239fb7a..00000000 --- a/src/main/java/com/nike/cerberus/service/AmiTagCheckService.java +++ /dev/null @@ -1,98 +0,0 @@ -/* - * Copyright (c) 2016 Nike, Inc. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package com.nike.cerberus.service; - -import com.amazonaws.AmazonServiceException; -import com.amazonaws.regions.Regions; -import com.amazonaws.services.ec2.AmazonEC2; -import com.amazonaws.services.ec2.AmazonEC2Client; -import com.amazonaws.services.ec2.model.DescribeImagesRequest; -import com.amazonaws.services.ec2.model.DescribeImagesResult; -import com.amazonaws.services.ec2.model.Filter; -import com.nike.cerberus.ConfigConstants; -import com.nike.cerberus.domain.environment.Stack; -import com.nike.cerberus.store.ConfigStore; - -import javax.inject.Inject; -import java.util.HashMap; -import java.util.Map; - -/** - * Service wrapper for AWS EC2. - */ -public class AmiTagCheckService { - - private final AwsClientFactory amazonEC2ClientFactory; - private final ConfigStore configStore; - - private final Map stackAmiTagValueMap; - - @Inject - public AmiTagCheckService(AwsClientFactory amazonEC2ClientFactory, - ConfigStore configStore) { - - this.amazonEC2ClientFactory = amazonEC2ClientFactory; - this.configStore = configStore; - - stackAmiTagValueMap = new HashMap<>(); - stackAmiTagValueMap.put(Stack.CMS, ConfigConstants.CMS_AMI_TAG_VALUE); - } - - /** - * Validates if the given AMI has given tag and value in the primary region. - * - * @return true if matches otherwise false - */ - public boolean isAmiWithTagExist(String amiId, String tagName, String tagValue) { - return isAmiWithTagExist(configStore.getPrimaryRegion(), amiId, tagName, tagValue); - } - - /** - * Validates if the given AMI has given tag and value in the provided region. - * - * @return true if matches otherwise false - */ - public boolean isAmiWithTagExist(Regions region, String amiId, String tagName, String tagValue) { - AmazonEC2 ec2Client = amazonEC2ClientFactory.getClient(region); - - DescribeImagesRequest request = new DescribeImagesRequest() - .withFilters(new Filter().withName(tagName).withValues(tagValue)) - .withFilters(new Filter().withName("image-id").withValues(amiId)); - - try { - DescribeImagesResult result = ec2Client.describeImages(request); - return result.getImages().size() > 0; - } catch (final AmazonServiceException ase) { - if (ase.getErrorCode() == "InvalidAMIID.NotFound") { - return false; - } - - throw ase; - } - } - - /** - * Validates if the given AMI has given cerberus tag and value. - * - * @return void - */ - public void validateAmiTagForStack(final String amiId, final Stack stack) { - if (!isAmiWithTagExist(amiId, ConfigConstants.CERBERUS_AMI_TAG_NAME, stackAmiTagValueMap.get(stack))) { - throw new IllegalStateException(ConfigConstants.AMI_TAG_CHECK_ERROR_MESSAGE); - } - } -} diff --git a/src/test/java/com/nike/cerberus/service/AmiTagCheckServiceTest.java b/src/test/java/com/nike/cerberus/service/AmiTagCheckServiceTest.java deleted file mode 100644 index 2ad8e631..00000000 --- a/src/test/java/com/nike/cerberus/service/AmiTagCheckServiceTest.java +++ /dev/null @@ -1,149 +0,0 @@ -/* - * Copyright (c) 2016 Nike, Inc. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package com.nike.cerberus.service; - -import com.amazonaws.AmazonServiceException; -import com.amazonaws.services.ec2.AmazonEC2; -import com.amazonaws.services.ec2.AmazonEC2Client; -import com.amazonaws.services.ec2.model.DescribeImagesRequest; -import com.amazonaws.services.ec2.model.DescribeImagesResult; -import com.amazonaws.services.ec2.model.Filter; -import com.amazonaws.services.ec2.model.Image; -import com.nike.cerberus.store.ConfigStore; -import org.junit.Before; -import org.junit.Test; -import org.mockito.Mock; - -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; -import static org.mockito.Matchers.any; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; -import static org.mockito.MockitoAnnotations.initMocks; - -public class AmiTagCheckServiceTest { - - @Mock - private ConfigStore configStore; - - @Mock - private AmazonEC2Client ec2Client; - - @Mock - private AwsClientFactory amazonEC2ClientFactory; - - @Before - public void before() { - initMocks(this); - when(amazonEC2ClientFactory.getClient(any())).thenReturn(ec2Client); - } - - @Test - public void isAmiWithTagExistTrue() { - AmiTagCheckService amiTagCheckService = new AmiTagCheckService(amazonEC2ClientFactory, configStore); - - String amiId = "ami-1234abcd"; - String tagName = "sometag"; - String tagValue = "someval"; - - when(ec2Client.describeImages( - new DescribeImagesRequest() - .withFilters(new Filter().withName(tagName).withValues(tagValue)) - .withFilters(new Filter().withName("image-id").withValues(amiId)) - ) - ).thenReturn( - new DescribeImagesResult().withImages(new Image()) - ); - - // invoke method under test - assertTrue(amiTagCheckService.isAmiWithTagExist(amiId, tagName, tagValue)); - } - - @Test - public void isAmiWithTagExistFalse() { - AmazonEC2 ec2Client = mock(AmazonEC2.class); - AmiTagCheckService amiTagCheckService = new AmiTagCheckService(amazonEC2ClientFactory, configStore); - - String amiId = "ami-1234abcd"; - String tagName = "sometag"; - String tagValue = "someval"; - - when(ec2Client.describeImages( - new DescribeImagesRequest() - .withFilters(new Filter().withName(tagName).withValues(tagValue)) - .withFilters(new Filter().withName("image-id").withValues(amiId)) - ) - ).thenReturn( - new DescribeImagesResult() - ); - - // invoke method under test - assertFalse(amiTagCheckService.isAmiWithTagExist(amiId, tagName, tagValue)); - } - - @Test - public void isAmiWithTagExistNotFound() { - AmazonEC2 ec2Client = mock(AmazonEC2.class); - AmiTagCheckService amiTagCheckService = new AmiTagCheckService(amazonEC2ClientFactory, configStore); - - String amiId = "ami-1234abcd"; - String tagName = "sometag"; - String tagValue = "someval"; - - AmazonServiceException ex = new AmazonServiceException("fake-exception"); - ex.setErrorCode("InvalidAMIID.NotFound"); - - when(ec2Client.describeImages( - new DescribeImagesRequest() - .withFilters(new Filter().withName(tagName).withValues(tagValue)) - .withFilters(new Filter().withName("image-id").withValues(amiId)) - ) - ).thenThrow(ex); - - // invoke method under test - assertFalse(amiTagCheckService.isAmiWithTagExist(amiId, tagName, tagValue)); - } - - @Test - public void isAmiWithTagExistThrowException() { - AmazonEC2 ec2Client = mock(AmazonEC2.class); - AmiTagCheckService amiTagCheckService = new AmiTagCheckService(amazonEC2ClientFactory, configStore); - - String amiId = "ami-1234abcd"; - String tagName = "sometag"; - String tagValue = "someval"; - String unknownAwsExMessage = "Unknown AWS exception message"; - - when(ec2Client.describeImages( - new DescribeImagesRequest() - .withFilters(new Filter().withName(tagName).withValues(tagValue)) - .withFilters(new Filter().withName("image-id").withValues(amiId)) - ) - ).thenThrow(new AmazonServiceException(unknownAwsExMessage)); - - try { - // invoke method under test - amiTagCheckService.isAmiWithTagExist(amiId, tagName, tagValue); - fail("Expected exception message '" + unknownAwsExMessage + "'not received"); - } catch (AmazonServiceException ex) { - // pass - assertEquals(unknownAwsExMessage, ex.getErrorMessage()); - } - } -} \ No newline at end of file