OWLS-80038 and OWLS-80090: fix mountPath validation and token substitution (#1911)

* Skip volume mount path validation if it contains valid tokens

* Check admin serverPod's additional volume mount paths too in domain validation

* Check cluster serverPod's additional mount paths too in domain validation

* check mountpath validation after token substitution is performed

* cleanup

* fine tuning

* minor cleanup

* In progress

* WIP

* minor change

* Modify a test name

Author: doxiao

operator/src/main/java/oracle/kubernetes/operator/helpers/DomainValidationSteps.java

@@ -10,6 +10,7 @@
 import io.kubernetes.client.openapi.models.V1ConfigMap;
 import io.kubernetes.client.openapi.models.V1ConfigMapList;
 import io.kubernetes.client.openapi.models.V1ObjectMeta;
+import io.kubernetes.client.openapi.models.V1PodSpec;
 import io.kubernetes.client.openapi.models.V1Secret;
 import io.kubernetes.client.openapi.models.V1SecretList;
 import oracle.kubernetes.operator.DomainStatusUpdater;
@@ -43,6 +44,10 @@ public static Step createDomainValidationSteps(String namespace, Step next) {
               new DomainValidationStep(next));
   }
 
+  public static Step createAdditionalDomainValidationSteps(V1PodSpec podSpec) {
+    return new DomainAdditionalValidationStep(podSpec);
+  }
+
   private static Step createListSecretsStep(String domainNamespace) {
     return new CallBuilder().listSecretsAsync(domainNamespace, new ListSecretsResponseStep());
   }
@@ -102,6 +107,34 @@ private String perLine(List<String> validationFailures) {
 
   }
 
+  static class DomainAdditionalValidationStep extends Step {
+    V1PodSpec podSpec;
+
+    DomainAdditionalValidationStep(V1PodSpec podSpec) {
+      this.podSpec = podSpec;
+    }
+
+    @Override
+    public NextAction apply(Packet packet) {
+      DomainPresenceInfo info = packet.getSpi(DomainPresenceInfo.class);
+      Domain domain = info.getDomain();
+      List<String> validationFailures = domain.getAdditionalValidationFailures(podSpec);
+
+      if (validationFailures.isEmpty()) {
+        return doNext(packet);
+      }
+
+      LOGGER.severe(DOMAIN_VALIDATION_FAILED, domain.getDomainUid(), perLine(validationFailures));
+      Step step = DomainStatusUpdater.createFailedStep(BAD_DOMAIN, perLine(validationFailures), null);
+      return doNext(step, packet);
+    }
+
+    private String perLine(List<String> validationFailures) {
+      return String.join(lineSeparator(), validationFailures);
+    }
+
+  }
+
   static class ValidateDomainTopologyStep extends Step {
 
     ValidateDomainTopologyStep(Step next) {

operator/src/main/java/oracle/kubernetes/operator/helpers/JobHelper.java

@@ -346,6 +346,8 @@ public NextAction apply(Packet packet) {
 
         return doNext(
             Step.chain(
+                DomainValidationSteps.createAdditionalDomainValidationSteps(
+                    context.getJobModel().getSpec().getTemplate().getSpec()),
                 createProgressingStep(info, INSPECTING_DOMAIN_PROGRESS_REASON, true, null),
                 context.createNewJob(null),
                 readDomainIntrospectorPodLogStep(null),

operator/src/main/java/oracle/kubernetes/operator/helpers/JobStepContext.java

@@ -62,7 +62,7 @@ void init() {
 
   // ------------------------ data methods ----------------------------
 
-  private V1Job getJobModel() {
+  protected V1Job getJobModel() {
     return jobModel;
   }
 

operator/src/main/java/oracle/kubernetes/operator/helpers/PodStepContext.java

@@ -267,7 +267,9 @@ private void setRecordedPod(V1Pod pod) {
    * @return a step to be scheduled.
    */
   Step verifyPod(Step next) {
-    return new VerifyPodStep(next);
+    return Step.chain(
+        DomainValidationSteps.createAdditionalDomainValidationSteps(podModel.getSpec()),
+        new VerifyPodStep(next));
   }
 
   /**

operator/src/main/java/oracle/kubernetes/operator/helpers/StepContextBase.java

@@ -17,18 +17,20 @@
 import oracle.kubernetes.operator.Pair;
 import oracle.kubernetes.weblogic.domain.model.Domain;
 
+import static oracle.kubernetes.weblogic.domain.model.Domain.TOKEN_END_MARKER;
+
 public abstract class StepContextBase implements StepContextConstants {
   protected final DomainPresenceInfo info;
 
   StepContextBase(DomainPresenceInfo info) {
     this.info = info;
   }
 
-  protected <T> T doDeepSubstitution(final Map<String, String> substitutionVariables, T obj) {
+  <T> T doDeepSubstitution(final Map<String, String> substitutionVariables, T obj) {
     return doDeepSubstitution(substitutionVariables, obj, false);
   }
 
-  protected <T> T doDeepSubstitution(final Map<String, String> substitutionVariables, T obj, boolean requiresDns1123) {
+  private <T> T doDeepSubstitution(final Map<String, String> substitutionVariables, T obj, boolean requiresDns1123) {
     if (obj instanceof String) {
       return (T) translate(substitutionVariables, (String) obj, requiresDns1123);
     } else if (obj instanceof List) {
@@ -74,7 +76,7 @@ protected <T> T doDeepSubstitution(final Map<String, String> substitutionVariabl
     return obj;
   }
 
-  boolean isDns1123Required(Method method) {
+  private boolean isDns1123Required(Method method) {
     // value requires to be in DNS1123 if the value is for a name, which is assumed to be
     // name for a kubernetes object
     return LegalNames.isDns1123Required(method.getName().substring(3));
@@ -121,8 +123,8 @@ private String translate(final Map<String, String> substitutionVariables, String
   private String translate(final Map<String, String> substitutionVariables, String rawValue, boolean requiresDns1123) {
     String result = rawValue;
     for (Map.Entry<String, String> entry : substitutionVariables.entrySet()) {
-      if (result != null && entry.getValue() != null) {
-        result = result.replace(String.format("$(%s)", entry.getKey()),
+      if (result != null && result.contains(Domain.TOKEN_START_MARKER) && entry.getValue() != null) {
+        result = result.replace(String.format("%s%s%s", Domain.TOKEN_START_MARKER, entry.getKey(), TOKEN_END_MARKER),
             requiresDns1123 ? LegalNames.toDns1123LegalName(entry.getValue()) : entry.getValue());
       }
     }

operator/src/main/java/oracle/kubernetes/weblogic/domain/AdminServerConfigurator.java

@@ -3,10 +3,13 @@
 
 package oracle.kubernetes.weblogic.domain;
 
+import oracle.kubernetes.weblogic.domain.model.AdminServer;
 import oracle.kubernetes.weblogic.domain.model.AdminService;
 
 @SuppressWarnings("UnusedReturnValue")
 public interface AdminServerConfigurator extends ServerConfigurator {
 
   AdminService configureAdminService();
+
+  AdminServer getAdminServer();
 }

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

@@ -11,6 +11,7 @@
 import java.util.List;
 import java.util.Optional;
 import java.util.Set;
+import java.util.StringTokenizer;
 import java.util.function.Predicate;
 import java.util.stream.Collectors;
 import javax.annotation.Nonnull;
@@ -23,6 +24,7 @@
 import io.kubernetes.client.openapi.models.V1EnvVar;
 import io.kubernetes.client.openapi.models.V1LocalObjectReference;
 import io.kubernetes.client.openapi.models.V1ObjectMeta;
+import io.kubernetes.client.openapi.models.V1PodSpec;
 import io.kubernetes.client.openapi.models.V1SecretReference;
 import io.kubernetes.client.openapi.models.V1VolumeMount;
 import oracle.kubernetes.json.Description;
@@ -35,10 +37,22 @@
 import org.apache.commons.lang3.builder.HashCodeBuilder;
 import org.apache.commons.lang3.builder.ToStringBuilder;
 
+import static java.util.stream.Collectors.toSet;
+
 /**
  * Domain represents a WebLogic domain and how it will be realized in the Kubernetes cluster.
  */
 public class Domain implements KubernetesObject {
+  /**
+   * The starting marker of a token that needs to be substituted with a matching env var.
+   */
+  public static final String TOKEN_START_MARKER = "$(";
+
+  /**
+   * The ending marker of a token that needs to be substituted with a matching env var.
+   */
+  public static final String TOKEN_END_MARKER = ")";
+
   /**
    * The pattern for computing the default shared logs directory.
    */
@@ -595,6 +609,10 @@ public List<String> getValidationFailures(KubernetesResourceLookup kubernetesRes
     return new Validator().getValidationFailures(kubernetesResources);
   }
 
+  public List<String> getAdditionalValidationFailures(V1PodSpec podSpec) {
+    return new Validator().getAdditionalValidationFailures(podSpec);
+  }
+
   class Validator {
     private final List<String> failures = new ArrayList<>();
     private final Set<String> clusterNames = new HashSet<>();
@@ -614,6 +632,11 @@ List<String> getValidationFailures(KubernetesResourceLookup kubernetesResources)
       return failures;
     }
 
+    public List<String> getAdditionalValidationFailures(V1PodSpec podSpec) {
+      addInvalidMountPathsForPodSpec(podSpec);
+      return failures;
+    }
+
     private void addDuplicateNames() {
       getSpec().getManagedServers()
           .stream()
@@ -655,14 +678,60 @@ private void checkDuplicateClusterName(String clusterName) {
 
     private void addInvalidMountPaths() {
       getSpec().getAdditionalVolumeMounts().forEach(this::checkValidMountPath);
+      if (getSpec().getAdminServer() != null) {
+        getSpec().getAdminServer().getAdditionalVolumeMounts().forEach(this::checkValidMountPath);
+      }
+      if (getSpec().getClusters() != null) {
+        getSpec().getClusters().forEach(
+            cluster -> cluster.getAdditionalVolumeMounts().forEach(this::checkValidMountPath));
+      }
+    }
+
+    private void addInvalidMountPathsForPodSpec(V1PodSpec podSpec) {
+      podSpec.getContainers()
+          .forEach(container ->
+              Optional.ofNullable(container.getVolumeMounts())
+                  .ifPresent(volumes -> volumes.forEach(this::checkValidMountPath)));
     }
 
     private void checkValidMountPath(V1VolumeMount mount) {
+      if (skipValidation(mount.getMountPath())) {
+        return;
+      }
+
       if (!new File(mount.getMountPath()).isAbsolute()) {
         failures.add(DomainValidationMessages.badVolumeMountPath(mount));
       }
     }
 
+    private boolean skipValidation(String mountPath) {
+      List<V1EnvVar> envVars = spec.getEnv();
+      Set<String> varNames = envVars.stream().map(V1EnvVar::getName).collect(toSet());
+      StringTokenizer nameList = new StringTokenizer(mountPath, TOKEN_START_MARKER);
+      if (!nameList.hasMoreElements()) {
+        return false;
+      }
+      while (nameList.hasMoreElements()) {
+        String token = nameList.nextToken();
+        if (noMatchingEnvVarName(varNames, token)) {
+          return false;
+        }
+      }
+      return true;
+    }
+
+    private boolean noMatchingEnvVarName(Set<String> varNames, String token) {
+      int index = token.indexOf(TOKEN_END_MARKER);
+      if (index != -1) {
+        String str = token.substring(0, index);
+        // IntrospectorJobEnvVars.isReserved() checks env vars in ServerEnvVars too
+        if (varNames.contains(str) || IntrospectorJobEnvVars.isReserved(str)) {
+          return false;
+        }
+      }
+      return true;
+    }
+
     private void addUnmappedLogHome() {
       if (!isLogHomeEnabled()) {
         return;

operator/src/main/java/oracle/kubernetes/weblogic/domain/model/DomainCommonConfigurator.java

@@ -29,7 +29,7 @@ public class DomainCommonConfigurator extends DomainConfigurator {
   public DomainCommonConfigurator() {
   }
 
-  DomainCommonConfigurator(@Nonnull Domain domain) {
+  public DomainCommonConfigurator(@Nonnull Domain domain) {
     super(domain);
     setApiVersion(domain);
   }
@@ -386,6 +386,10 @@ class AdminServerConfiguratorImpl extends ServerConfiguratorImpl
     public AdminService configureAdminService() {
       return adminServer.createAdminService();
     }
+
+    public AdminServer getAdminServer() { 
+      return adminServer; 
+    }
   }
 
   class ServerConfiguratorImpl implements ServerConfigurator {

operator/src/main/java/oracle/kubernetes/weblogic/domain/model/IntrospectorJobEnvVars.java

@@ -35,7 +35,6 @@ public class IntrospectorJobEnvVars {
    */
   public static final String OPSS_WALLETFILE_SECRET_NAME = "OPSS_WALLETFILE_SECRET_NAME";
 
-
   /**
    * The credentials used by the introspection job - wdt encryption passphrase.
    */
@@ -46,7 +45,6 @@ public class IntrospectorJobEnvVars {
    */
   public static final String RUNTIME_ENCRYPTION_SECRET_NAME = "RUNTIME_ENCRYPTION_SECRET_NAME";
 
-
   /**
    * The domain source type.
    */
@@ -83,5 +81,8 @@ static boolean isReserved(String name) {
     return ServerEnvVars.isReserved(name) || RESERVED_NAMES.contains(name);
   }
 
-  private static final List<String> RESERVED_NAMES = Arrays.asList(NAMESPACE, INTROSPECT_HOME, CREDENTIALS_SECRET_NAME);
+  private static final List<String> RESERVED_NAMES = Arrays.asList(
+      NAMESPACE, INTROSPECT_HOME, CREDENTIALS_SECRET_NAME, OPSS_KEY_SECRET_NAME, OPSS_WALLETFILE_SECRET_NAME,
+      RUNTIME_ENCRYPTION_SECRET_NAME, WDT_DOMAIN_TYPE, DOMAIN_SOURCE_TYPE, ISTIO_ENABLED, ISTIO_READINESS_PORT,
+      ISTIO_POD_NAMESPACE, WDT_MODEL_HOME);
 }

operator/src/main/java/oracle/kubernetes/weblogic/domain/model/ServerEnvVars.java

@@ -64,8 +64,8 @@ public class ServerEnvVars {
 
   private static final List<String> RESERVED_NAMES = Arrays.asList(
         DOMAIN_UID, DOMAIN_NAME, DOMAIN_HOME, NODEMGR_HOME, SERVER_NAME, SERVICE_NAME,
-        ADMIN_NAME, AS_SERVICE_NAME, ADMIN_PORT, ADMIN_PORT_SECURE,
-        LOG_HOME, SERVER_OUT_IN_POD_LOG);
+        ADMIN_NAME, AS_SERVICE_NAME, ADMIN_PORT, ADMIN_PORT_SECURE, ADMIN_SERVER_PORT_SECURE,
+        LOG_HOME, SERVER_OUT_IN_POD_LOG, DATA_HOME, ACCESS_LOG_IN_LOG_HOME, DYNAMIC_CONFIG_OVERRIDE);
 
   static boolean isReserved(String name) {
     return RESERVED_NAMES.contains(name);