improve validation messages

Author: russgold

model/src/main/java/oracle/kubernetes/weblogic/domain/model/Domain.java

@@ -480,41 +480,82 @@ public List<String> getValidationFailures() {
   }
 
   class Validator {
+    static final String DUPLICATE_SERVER_NAME_FOUND
+          = "More than one item under spec.managedServers in the domain resource has DNS-1123 name '%s'";
+    static final String DUPLICATE_CLUSTER_NAME_FOUND
+          = "More than one item under spec.clusters in the domain resource has DNS-1123 name '%s'";
+    static final String LOG_HOME_PATH_NOT_MOUNTED
+          = "No volume mount contains path for log home '%s', %s in the domain resource";
+    static final String BAD_VOLUME_MOUNT_PATH
+          = "The mount path '%s', in entry '%s' of domain resource additionalVolumeMounts, is not valid";
+    
     private List<String> failures = new ArrayList<>();
     private Set<String> clusterNames = new HashSet<>();
     private Set<String> serverNames = new HashSet<>();
 
     List<String> getValidationFailures() {
       addDuplicateNames();
+      addInvalidMountPaths();
       addUnmappedLogHome();
 
       return failures;
     }
 
     private void addDuplicateNames() {
-      getSpec().getManagedServers().stream().map(ManagedServer::getServerName).forEach(this::checkDuplicateServerName);
-      getSpec().getClusters().stream().map(Cluster::getClusterName).forEach(this::checkDuplicateClusterName);
+      getSpec().getManagedServers()
+            .stream()
+            .map(ManagedServer::getServerName)
+            .map(this::toDns1123LegalName)
+            .forEach(this::checkDuplicateServerName);
+      getSpec().getClusters()
+            .stream()
+            .map(Cluster::getClusterName)
+            .map(this::toDns1123LegalName)
+            .forEach(this::checkDuplicateClusterName);
+    }
+
+    /**
+     * Converts value to nearest DNS-1123 legal name, which can be used as a Kubernetes identifier.
+     *
+     * @param value Input value
+     * @return nearest DNS-1123 legal name
+     */
+    String toDns1123LegalName(String value) {
+      return value.toLowerCase().replace('_', '-');
     }
 
     private void checkDuplicateServerName(String s) {
       if (serverNames.contains(s))
-        failures.add(String.format("More than one server is named '%s'", s));
+        failures.add(String.format(DUPLICATE_SERVER_NAME_FOUND, s));
       else
         serverNames.add(s);
     }
 
     private void checkDuplicateClusterName(String s) {
       if (clusterNames.contains(s))
-        failures.add(String.format("More than one cluster is named '%s'", s));
+        failures.add(String.format(DUPLICATE_CLUSTER_NAME_FOUND, s));
       else
         clusterNames.add(s);
     }
 
+    private void addInvalidMountPaths() {
+      getSpec().getAdditionalVolumeMounts().forEach(this::checkValidMountPath);
+    }
+
+    private void checkValidMountPath(V1VolumeMount mount) {
+      if (!new File(mount.getMountPath()).isAbsolute())
+        failures.add(String.format(BAD_VOLUME_MOUNT_PATH, mount.getMountPath(), mount.getName()));
+    }
+
     private void addUnmappedLogHome() {
       if (!isLogHomeEnabled()) return;
 
       if (getSpec().getAdditionalVolumeMounts().stream().map(V1VolumeMount::getMountPath).noneMatch(this::mapsLogHome))
-        failures.add("No volume mount contains path for log home");
+        failures.add(String.format(LOG_HOME_PATH_NOT_MOUNTED, getLogHome(), getLogHomeSource()));
+    }
+
+    private String getLogHomeSource() {
+      return getSpec().getLogHome() == null ? "implicit" : "specified";
     }
 
     private boolean mapsLogHome(String mountPath) {

model/src/test/java/oracle/kubernetes/weblogic/domain/model/DomainValidationTest.java

@@ -29,7 +29,15 @@ public void whenManagerServerSpecsHaveDuplicateNames_reportError() {
     domain.getSpec().getManagedServers().add(new ManagedServer().withServerName("ms1"));
     domain.getSpec().getManagedServers().add(new ManagedServer().withServerName("ms1"));
 
-    assertThat(domain.getValidationFailures(), contains(stringContainsInOrder("server", "ms1")));
+    assertThat(domain.getValidationFailures(), contains(stringContainsInOrder("managedServers", "ms1")));
+  }
+
+  @Test
+  public void whenManagerServerSpecsHaveDns1123DuplicateNames_reportError() {
+    domain.getSpec().getManagedServers().add(new ManagedServer().withServerName("Server-1"));
+    domain.getSpec().getManagedServers().add(new ManagedServer().withServerName("server_1"));
+
+    assertThat(domain.getValidationFailures(), contains(stringContainsInOrder("managedServers", "server-1")));
   }
 
   @Test
@@ -45,7 +53,15 @@ public void whenClusterSpecsHaveDuplicateNames_reportError() {
     domain.getSpec().getClusters().add(new Cluster().withClusterName("cluster1"));
     domain.getSpec().getClusters().add(new Cluster().withClusterName("cluster1"));
 
-    assertThat(domain.getValidationFailures(), contains(stringContainsInOrder("cluster", "cluster1")));
+    assertThat(domain.getValidationFailures(), contains(stringContainsInOrder("clusters", "cluster1")));
+  }
+
+  @Test
+  public void whenClusterSpecsHaveDns1123DuplicateNames_reportError() {
+    domain.getSpec().getClusters().add(new Cluster().withClusterName("Cluster-1"));
+    domain.getSpec().getClusters().add(new Cluster().withClusterName("cluster_1"));
+
+    assertThat(domain.getValidationFailures(), contains(stringContainsInOrder("clusters", "cluster-1")));
   }
 
   @Test
@@ -55,6 +71,13 @@ public void whenLogHomeDisabled_dontReportError() {
     assertThat(domain.getValidationFailures(), empty());
   }
 
+  @Test
+  public void whenVolumeMountHasNonValidPath_reportError() {
+    configureDomain(domain).withAdditionalVolumeMount("sharedlogs", "shared/logs");
+
+    assertThat(domain.getValidationFailures(), contains(stringContainsInOrder("shared/logs", "sharedlogs")));
+  }
+
   @Test
   public void whenVolumeMountHasLogHomeDirectory_dontReportError() {
     configureDomain(domain).withLogHomeEnabled(true).withLogHome("/shared/logs/mydomain");

operator/src/test/java/oracle/kubernetes/operator/DomainProcessorTest.java

@@ -285,7 +285,7 @@ public void whenDomainIsNotValid_updateStatus() {
 
     Domain updatedDomain = testSupport.getResourceWithName(DOMAIN, UID);
     assertThat(getStatusReason(updatedDomain), equalTo("ErrBadDomain"));
-    assertThat(getStatusMessage(updatedDomain), stringContainsInOrder("server", "ms1"));
+    assertThat(getStatusMessage(updatedDomain), stringContainsInOrder("managedServers", "ms1"));
   }
 
   private String getStatusReason(Domain updatedDomain) {

operator/src/test/java/oracle/kubernetes/operator/helpers/DomainValidationStepTest.java

@@ -78,7 +78,7 @@ public void whenDomainIsNotValid_updateStatus() {
 
     Domain updatedDomain = testSupport.getResourceWithName(DOMAIN, UID);
     assertThat(getStatusReason(updatedDomain), equalTo("ErrBadDomain"));
-    assertThat(getStatusMessage(updatedDomain), stringContainsInOrder("server", "ms1"));
+    assertThat(getStatusMessage(updatedDomain), stringContainsInOrder("managedServers", "ms1"));
   }
 
   private String getStatusReason(Domain updatedDomain) {