fix(canary-config): Ensure that PUT applies same validation as POST. (#…

…215)
spinnaker · Jan 24, 2018 · 10cdaab · 10cdaab
1 parent f73186f
commit 10cdaab
Showing 1 changed file with 18 additions and 12 deletions.
diff --git a/kayenta-web/src/main/java/com/netflix/kayenta/controllers/CanaryConfigController.java b/kayenta-web/src/main/java/com/netflix/kayenta/controllers/CanaryConfigController.java
@@ -98,18 +98,7 @@ public CanaryConfigUpdateResponse storeCanaryConfig(@RequestParam(required = fal
     String canaryConfigId = UUID.randomUUID() + "";
 
     // TODO(duftler): Serialize the canary config within a canary run?
-    if (StringUtils.isEmpty(canaryConfig.getName())) {
-      throw new IllegalArgumentException("Canary config must specify a name.");
-    } else if (canaryConfig.getApplications() == null || canaryConfig.getApplications().size() == 0) {
-      throw new IllegalArgumentException("Canary config must specify at least one application.");
-    }
-
-    String canaryConfigName = canaryConfig.getName();
-
-    if (!canaryConfigNamePattern.matcher(canaryConfigName).matches()) {
-      throw new IllegalArgumentException("Canary config cannot be named '" + canaryConfigName +
-        "'. Names must contain only letters, numbers, dashes (-) and underscores (_).");
-    }
+    validateNameAndApplicationAttributes(canaryConfig);
 
     try {
       configurationService.loadObject(resolvedConfigurationAccountName, ObjectType.CANARY_CONFIG, canaryConfigId);
@@ -138,6 +127,8 @@ public CanaryConfigUpdateResponse updateCanaryConfig(@RequestParam(required = fa
     canaryConfig.setUpdatedTimestamp(System.currentTimeMillis());
     canaryConfig.setUpdatedTimestampIso(Instant.ofEpochMilli(canaryConfig.getUpdatedTimestamp()).toString());
 
+    validateNameAndApplicationAttributes(canaryConfig);
+
     try {
       configurationService.loadObject(resolvedConfigurationAccountName, ObjectType.CANARY_CONFIG, canaryConfigId);
     } catch (Exception e) {
@@ -149,6 +140,21 @@ public CanaryConfigUpdateResponse updateCanaryConfig(@RequestParam(required = fa
     return CanaryConfigUpdateResponse.builder().canaryConfigId(canaryConfigId).build();
   }
 
+  private static void validateNameAndApplicationAttributes(@RequestBody CanaryConfig canaryConfig) {
+    if (StringUtils.isEmpty(canaryConfig.getName())) {
+      throw new IllegalArgumentException("Canary config must specify a name.");
+    } else if (canaryConfig.getApplications() == null || canaryConfig.getApplications().size() == 0) {
+      throw new IllegalArgumentException("Canary config must specify at least one application.");
+    }
+
+    String canaryConfigName = canaryConfig.getName();
+
+    if (!canaryConfigNamePattern.matcher(canaryConfigName).matches()) {
+      throw new IllegalArgumentException("Canary config cannot be named '" + canaryConfigName +
+        "'. Names must contain only letters, numbers, dashes (-) and underscores (_).");
+    }
+  }
+
   @ApiOperation(value = "Delete a canary config")
   @RequestMapping(value = "/{canaryConfigId:.+}", method = RequestMethod.DELETE)
   public void deleteCanaryConfig(@RequestParam(required = false) final String configurationAccountName,