Merge pull request #37 from GCMGrosvenor/features/Issue36

Resolve issue #36: repo or package names with special characters won't be processed
indix · Jul 3, 2016 · ba633eb · ba633eb
2 parents 36036ed + 99f79f7
commit ba633eb
Show file tree

Hide file tree

Showing 2 changed files with 55 additions and 5 deletions.
diff --git a/fetch/src/main/java/com/indix/gocd/s3fetch/FetchConfig.java b/fetch/src/main/java/com/indix/gocd/s3fetch/FetchConfig.java
@@ -1,12 +1,12 @@
 package com.indix.gocd.s3fetch;
 
-import com.amazonaws.util.StringUtils;
+import org.apache.commons.lang3.StringUtils;
 import com.indix.gocd.utils.GoEnvironment;
 import com.thoughtworks.go.plugin.api.response.validation.ValidationError;
 import com.thoughtworks.go.plugin.api.response.validation.ValidationResult;
 import com.thoughtworks.go.plugin.api.task.TaskConfig;
 import com.thoughtworks.go.plugin.api.task.TaskExecutionContext;
-
+import com.thoughtworks.go.plugin.api.logging.Logger;
 import static com.indix.gocd.utils.Constants.*;
 
 public class FetchConfig {
@@ -15,6 +15,7 @@ public class FetchConfig {
     private final String stage;
     private final String job;
     private GoEnvironment env;
+    private static Logger logger = Logger.getLoggerFor(FetchConfig.class);
 
     public FetchConfig(TaskConfig config, TaskExecutionContext context) {
         this(config, context, new GoEnvironment());
@@ -24,22 +25,30 @@ public FetchConfig(TaskConfig config, TaskExecutionContext context, GoEnvironmen
         this.env = goEnvironment;
         env.putAll(context.environment().asMap());
 
-        String repoName = config.getValue(FetchTask.REPO).toUpperCase().replaceAll("-", "_");
-        String packageName = config.getValue(FetchTask.PACKAGE).toUpperCase().replaceAll("-", "_");
+        String repoName = escapeEnvironmentVariable(config.getValue(FetchTask.REPO));
+        String packageName = escapeEnvironmentVariable(config.getValue(FetchTask.PACKAGE));
+        logger.debug(String.format("s3 fetch config uses repoName=%s and packageName=%s", repoName, packageName));
         this.materialLabel = env.get(String.format("GO_PACKAGE_%s_%s_LABEL", repoName, packageName));
         this.pipeline = env.get(String.format("GO_PACKAGE_%s_%s_PIPELINE_NAME", repoName, packageName));
         this.stage = env.get(String.format("GO_PACKAGE_%s_%s_STAGE_NAME", repoName, packageName));
         this.job = env.get(String.format("GO_PACKAGE_%s_%s_JOB_NAME", repoName, packageName));
     }
 
+    private static String escapeEnvironmentVariable(String value) {
+        if (value == null) {
+            return "";
+        }
+        return value.replaceAll("[^A-Za-z0-9_]", "_").toUpperCase();
+    }
+
     public ValidationResult validate() {
         ValidationResult validationResult = new ValidationResult();
         if (!env.hasAWSUseIamRole()) {
             if (env.isAbsent(AWS_ACCESS_KEY_ID)) validationResult.addError(envNotFound(AWS_ACCESS_KEY_ID));
             if (env.isAbsent(AWS_SECRET_ACCESS_KEY)) validationResult.addError(envNotFound(AWS_SECRET_ACCESS_KEY));
         }
         if (env.isAbsent(GO_ARTIFACTS_S3_BUCKET)) validationResult.addError(envNotFound(GO_ARTIFACTS_S3_BUCKET));
-        if (StringUtils.isNullOrEmpty(materialLabel))
+        if (StringUtils.isBlank(materialLabel))
             validationResult.addError(new ValidationError("Please check Repository name or Package name configuration. Also ensure that the appropriate S3 material is configured for the pipeline."));
 
         return validationResult;

diff --git a/fetch/src/test/java/com/indix/gocd/s3fetch/FetchConfigTest.java b/fetch/src/test/java/com/indix/gocd/s3fetch/FetchConfigTest.java
@@ -237,6 +237,47 @@ public void shouldAllowFetchTaskVariablesWithDashesInTheName() throws Exception
         assertTrue(validationResult.isSuccessful());
     }
 
+    @Test
+    public void shouldAllowFetchTaskVariablesWithPeriodsInTheName() throws Exception {
+        config = mock(TaskConfig.class);
+        when(config.getValue(FetchTask.REPO)).thenReturn("repo-with.period");
+        when(config.getValue(FetchTask.PACKAGE)).thenReturn("package-with.period");
+        mockEnvironmentVariables = Maps.<String, String>builder()
+            .with(AWS_SECRET_ACCESS_KEY, secretKey)
+            .with(AWS_ACCESS_KEY_ID, accessId)
+            .with(GO_ARTIFACTS_S3_BUCKET, bucket)
+            .with("GO_PACKAGE_REPO_WITH_PERIOD_PACKAGE_WITH_PERIOD_LABEL", "20.1")
+            .with("GO_REPO_REPO_WITH_PERIOD_PACKAGE_WITH_PERIOD_S3_BUCKET", bucket)
+            .with("GO_PACKAGE_REPO_WITH_PERIOD_PACKAGE_WITH_PERIOD_PIPELINE_NAME", "TestPublish")
+            .with("GO_PACKAGE_REPO_WITH_PERIOD_PACKAGE_WITH_PERIOD_STAGE_NAME", "defaultStage")
+            .with("GO_PACKAGE_REPO_WITH_PERIOD_PACKAGE_WITH_PERIOD_JOB_NAME", "defaultJob");
+
+        fetchConfig = new FetchConfig(config, mockContext(mockEnvironmentVariables.build()), goEnvironmentForTest);
+        ValidationResult validationResult = fetchConfig.validate();
+        assertTrue(validationResult.isSuccessful());
+    }
+
+
+    @Test
+    public void shouldAllowFetchTaskVariablesWithSpecialCharactersInTheName() throws Exception {
+        config = mock(TaskConfig.class);
+        when(config.getValue(FetchTask.REPO)).thenReturn("repo-with`~!@#$%^&*()-+=[{]}\\|;:'\",<.>/?");
+        when(config.getValue(FetchTask.PACKAGE)).thenReturn("package-with`~!@#$%^&*()-+=[{]}\\|;:'\",<.>/?");
+        mockEnvironmentVariables = Maps.<String, String>builder()
+                .with(AWS_SECRET_ACCESS_KEY, secretKey)
+                .with(AWS_ACCESS_KEY_ID, accessId)
+                .with(GO_ARTIFACTS_S3_BUCKET, bucket)
+                .with("GO_PACKAGE_REPO_WITH________________________________PACKAGE_WITH________________________________LABEL", "20.1")
+                .with("GO_REPO_REPO_WITH________________________________PACKAGE_WITH________________________________S3_BUCKET", bucket)
+                .with("GO_PACKAGE_REPO_WITH________________________________PACKAGE_WITH________________________________PIPELINE_NAME", "TestPublish")
+                .with("GO_PACKAGE_REPO_WITH________________________________PACKAGE_WITH________________________________STAGE_NAME", "defaultStage")
+                .with("GO_PACKAGE_REPO_WITH________________________________PACKAGE_WITH________________________________JOB_NAME", "defaultJob");
+
+        fetchConfig = new FetchConfig(config, mockContext(mockEnvironmentVariables.build()), goEnvironmentForTest);
+        ValidationResult validationResult = fetchConfig.validate();
+        assertTrue(validationResult.isSuccessful());
+    }
+
     private TaskExecutionContext mockContext(final Map<String, String> environmentMap) {
         return new MockTaskExecutionContext(environmentMap);
     }