Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 14 commits into from
Sep 17, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
In progress
  • Loading branch information
doxiao committed Sep 11, 2020
commit 568ff48197becbcf77dfdafe45f3942ace5f79e9
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,9 @@
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 {
private static final String TOKEN_MARKER = "$(";
protected final DomainPresenceInfo info;

StepContextBase(DomainPresenceInfo info) {
Expand Down Expand Up @@ -122,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 && result.contains(TOKEN_MARKER) && entry.getValue() != null) {
result = result.replace(String.format("%s%s)", TOKEN_MARKER, 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());
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,16 @@
* 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.
*/
Expand Down Expand Up @@ -685,16 +695,17 @@ private void addInvalidMountPathsForPodSpec(V1PodSpec podSpec) {
}

private void checkValidMountPath(V1VolumeMount mount) {
if (!new File(mount.getMountPath()).isAbsolute()
&& !skipValidation(mount.getMountPath())) {
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, "$(");
StringTokenizer nameList = new StringTokenizer(mountPath, TOKEN_START_MARKER);
if (!nameList.hasMoreElements()) {
return false;
}
Expand All @@ -708,7 +719,7 @@ private boolean skipValidation(String mountPath) {
}

private boolean noMatchingEnvVarName(Set<String> varNames, String token) {
int index = token.indexOf(")");
int index = token.indexOf(TOKEN_END_MARKER);
if (index != -1) {
String str = token.substring(0, index);
// IntrospectorJobEnvVars.isReserved() checks env vars in ServerEnvVars too
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -332,17 +332,6 @@ public void whenDomainHasValueFromEnvironmentItemsWithVariables_createAdminPodSt
);
}

@Test
public void whenDomainAdminServerHasAdditionalVolumeMountsWithVariables_createAdminPodStartupWithSubstitutions() {
configureAdminServer()
.withAdditionalVolumeMount("volume1", RAW_MOUNT_PATH_1);

assertThat(
getCreatedPodSpecContainer().getVolumeMounts(),
allOf(
hasVolumeMount("volume1", END_MOUNT_PATH_1)));
}

@Test
public void whenDomainHasValueFromEnvironmentItemsWithVariables_createPodShouldNotChangeTheirValues()
throws Exception {
Expand All @@ -366,7 +355,7 @@ public void whenDomainHasValueFromEnvironmentItemsWithVariables_createPodShouldN
}

@Test
public void whenDomainAdminServerHasAdditionalVolumesWithVariables_createManagedPodWithSubstitutions() {
public void whenAdminServerHasAdditionalVolumesWithReservedVariables_createAdminPodStartupWithSubstitutions() {
configureAdminServer()
.withAdditionalVolume("volume1", "/source-$(SERVER_NAME)")
.withAdditionalVolume("volume2", "/source-$(DOMAIN_NAME)");
Expand All @@ -379,7 +368,18 @@ public void whenDomainAdminServerHasAdditionalVolumesWithVariables_createManaged
}

@Test
public void whenDomainHasAdditionalVolumesWithVariablesValue_dontReportValidationError() {
public void whenAdminServerHasAdditionalVolumeMountsWithReservedVariables_createAdminPodStartupWithSubstitutions() {
configureAdminServer()
.withAdditionalVolumeMount("volume1", RAW_MOUNT_PATH_1);

assertThat(
getCreatedPodSpecContainer().getVolumeMounts(),
allOf(
hasVolumeMount("volume1", END_MOUNT_PATH_1)));
}

@Test
public void whenDomainHasAdditionalVolumesWithCustomVariables_createAdminPodStartupWithSubstitutions() {
resourceLookup.defineResource(OVERRIDES_CM_NAME_MODEL, KubernetesResourceType.ConfigMap, NS);
resourceLookup.defineResource(OVERRIDES_CM_NAME_IMAGE, KubernetesResourceType.ConfigMap, NS);

Expand All @@ -397,7 +397,7 @@ public void whenDomainHasAdditionalVolumesWithVariablesValue_dontReportValidatio
}

@Test
public void whenDomainHasAdditionalVolumesWithVariablesInvalidValue_reportValidationError() {
public void whenDomainHasAdditionalVolumesWithCustomVariablesInvalidValue_reportValidationError() {
resourceLookup.defineResource(OVERRIDES_CM_NAME_MODEL, KubernetesResourceType.ConfigMap, NS);
resourceLookup.defineResource(OVERRIDES_CM_NAME_IMAGE, KubernetesResourceType.ConfigMap, NS);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -445,7 +445,7 @@ public void introspectorPodStartupWithNullAdminUsernamePasswordEnvVarValues() {
}

@Test
public void whenDomainHasAdditionalVolumesWithVariables_createIntrospectorPodWithSubstitutions() {
public void whenDomainHasAdditionalVolumesWithReservedVariables_createIntrospectorPodWithSubstitutions() {
configureDomain()
.withAdditionalVolumeMount("volume2", "/source-$(DOMAIN_UID)");
runCreateJob();
Expand All @@ -455,7 +455,7 @@ public void whenDomainHasAdditionalVolumesWithVariables_createIntrospectorPodWit
}

@Test
public void whenDomainHasAdditionalVolumesWithVariablesValue_dontReportValidationError() {
public void whenDomainHasAdditionalVolumesWithCustomVariables_createIntrospectorPodWithSubstitutions() {
resourceLookup.defineResource(SECRET_NAME, KubernetesResourceType.Secret, NS);
resourceLookup.defineResource(OVERRIDES_CM_NAME_MODEL, KubernetesResourceType.ConfigMap, NS);
resourceLookup.defineResource(OVERRIDES_CM_NAME_IMAGE, KubernetesResourceType.ConfigMap, NS);
Expand All @@ -473,7 +473,7 @@ public void whenDomainHasAdditionalVolumesWithVariablesValue_dontReportValidatio
}

@Test
public void whenDomainHasAdditionalVolumesWithVariablesInvalidValue_reportValidationError() {
public void whenDomainHasAdditionalVolumesWithCustomVariablesInvalidValue_reportValidationError() {
resourceLookup.defineResource(SECRET_NAME, KubernetesResourceType.Secret, NS);
resourceLookup.defineResource(OVERRIDES_CM_NAME_MODEL, KubernetesResourceType.ConfigMap, NS);
resourceLookup.defineResource(OVERRIDES_CM_NAME_IMAGE, KubernetesResourceType.ConfigMap, NS);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ public void whenPacketHasValueFromEnvironmentItemsWithVariables_createManagedPod
}

@Test
public void whenClusterHasAdditionalVolumesWithVariables_createManagedPodWithSubstitutions() {
public void whenClusterHasAdditionalVolumesWithReservedVariables_createManagedPodWithSubstitutions() {
testSupport.addToPacket(ProcessingConstants.CLUSTER_NAME, CLUSTER_NAME);
getConfigurator()
.configureCluster(CLUSTER_NAME)
Expand All @@ -218,7 +218,7 @@ public void whenClusterHasAdditionalVolumesWithVariables_createManagedPodWithSub
}

@Test
public void whenDomainHasAdditionalVolumesWithVariables_createManagedPodWithSubstitutions() {
public void whenDomainHasAdditionalVolumesWithReservedVariables_createManagedPodWithSubstitutions() {
getConfigurator()
.withAdditionalVolume("volume1", "/source-$(SERVER_NAME)")
.withAdditionalVolume("volume2", "/source-$(DOMAIN_NAME)");
Expand All @@ -231,7 +231,7 @@ public void whenDomainHasAdditionalVolumesWithVariables_createManagedPodWithSubs
}

@Test
public void whenDomainHasAdditionalVolumesWithVariablesValue_dontReportValidationError() {
public void whenDomainHasAdditionalVolumesWithCustomVariables_createManagedPodWithSubstitutions() {
resourceLookup.defineResource(SECRET_NAME, KubernetesResourceType.Secret, NS);
resourceLookup.defineResource(OVERRIDES_CM_NAME_MODEL, KubernetesResourceType.ConfigMap, NS);
resourceLookup.defineResource(OVERRIDES_CM_NAME_IMAGE, KubernetesResourceType.ConfigMap, NS);
Expand All @@ -253,7 +253,7 @@ public void whenDomainHasAdditionalVolumesWithVariablesValue_dontReportValidatio
}

@Test
public void whenDomainHasAdditionalVolumesWithVariablesInvalidValue_reportValidationError() {
public void whenDomainHasAdditionalVolumesWithCustomVariablesInvalidValue_reportValidationError() {
resourceLookup.defineResource(SECRET_NAME, KubernetesResourceType.Secret, NS);
resourceLookup.defineResource(OVERRIDES_CM_NAME_MODEL, KubernetesResourceType.ConfigMap, NS);
resourceLookup.defineResource(OVERRIDES_CM_NAME_IMAGE, KubernetesResourceType.ConfigMap, NS);
Expand Down