Skip to content

OWLS-76832 (Resolves #1232) - Error using variable in pod template if value has underscore #1286

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 12 commits into from
Oct 8, 2019
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,27 @@ These examples show two valid YAML syntax options for arrays.
You must include the `default` namespace in the list if you want the operator to monitor both the `default` namespace and some other namespaces.
{{% /notice %}}

##### `dns1123Fields`

When the operator processes variable references in the domain resource, such as `$(SERVER_NAME)`,
it will convert them into DNS-1123 legal values if the variables are referenced
in fields that require their values to be conforming to DNS-1123. A default list of such field
names can be found inside the class `Legalnames`
in the `oracle.kubernetes.operator.helpers package`.
For example, a value `$(SERVER_NAME)-volume` is specified in a field in a domain resource that
requires DNS-1123. Suppose the name of the server is `managed_server1` which contains an illegal
underscore character. Upon variable substitution, the operator will replace it with a hyphen, and
the resulting value will become `managed-server1-volume`.

A comma separated list of field names can be supplied in `dns1123Fields` to customize the list of
field names, or it can be left commented out to use the default list of field names.

This property is optional.

Example:
```
dns1123Fields: "name, claimName, volumeName"
```
#### Elastic Stack integration

##### `elkIntegrationEnabled`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@ data:
{{- end }}
serviceaccount: {{ .serviceAccount | quote }}
targetNamespaces: {{ .domainNamespaces | uniq | sortAlpha | join "," | quote }}
{{- if .dns1123Fields }}
dns1123Fields: {{ .dns1123Fields | quote }}
{{- end }}
kind: "ConfigMap"
metadata:
labels:
Expand Down
11 changes: 10 additions & 1 deletion kubernetes/charts/weblogic-operator/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,15 @@ elasticSearchHost: "elasticsearch.default.svc.cluster.local"
# This parameter is ignored if 'elkIntegrationEnabled' is false.
elasticSearchPort: 9200

# dns1123Fields overrides the default list of field names that the operator
# converts to DNS-1123 legal values when replacing variable references in the
# domain resource. The default list can be found inside the class Legalnames
# in the oracle.kubernetes.operator.helpers package.
# Supply a comma seperated list of field names to customize the list of fields
# such as "name, claimName, volumeName", or leave it commented out to use
# the default list of field names.
# dns1123Fields: ""

# Istio service mesh support is experimental.
# istioEnabled specifies whether or not the domain is deployed under an Istio service mesh.
istioEnabled: false
istioEnabled: false
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,13 @@

package oracle.kubernetes.operator.helpers;

import java.util.ArrayList;
import java.util.Collection;
import java.util.Optional;
import java.util.StringTokenizer;

import oracle.kubernetes.operator.TuningParameters;

/** A class to create DNS-1123 legal names for Kubernetes objects. */
public class LegalNames {

Expand All @@ -11,6 +18,34 @@ public class LegalNames {
private static final String DOMAIN_INTROSPECTOR_JOB_PATTERN = "%s-introspect-domain-job";
private static final String EXTERNAL_SERVICE_PATTERN = "%s-%s-external";

public static final String DNS_1123_FIELDS_PARAM = "dns1123Fields";

// Fields that requires values to be DNS1123 legal
private static final String[] DEFAULT_DNS1123_FIELDS = {
"ClaimName", // V1PersistentVolumeClaimVolumeSource
"ClusterName", // V1ObjectMetaData
"ContainerName", // V1ResourceFieldSelector
"ExternalName", // V1ServiceSpec
"GenerateName", // V1ObjectMetaData
"MetricName", // V2beta1PodsMetricSource, etc
"Name",
"NodeName", // V1PodSpec, etc
// "NominatedNodeName", // V1PodStatus - excluded since it is not used within V1PodSpec
"PersistentVolumeName",// V1VolumeAttachmentSource, etc
"PriorityClassName", // V1PodSpec
"RuntimeClassName", // V1PodSpec
"SchedulerName", // V1PodSpec
"ScopeName", // V1ScopedResourceSelectorRequirement
"SecretName", // V1SecretVolumeSource, etc
"ServiceAccountName", // V1PodSpec
"ServiceName", // NetworkingV1beta1IngressBackend, etc
"SingularName", // V1APIResource
"StorageClassName", // V1PersistentVolumeSpec, V1PersistentVolumeClaimSpec
"VolumeName" // V1PersistentVolumeClaimSpec, etc
};

static String[] dns1123Fields;

public static String toServerServiceName(String domainUid, String serverName) {
return toServerName(domainUid, serverName);
}
Expand Down Expand Up @@ -48,4 +83,57 @@ public static String toExternalServiceName(String domainUid, String serverName)
public static String toDns1123LegalName(String value) {
return value.toLowerCase().replace('_', '-');
}

/**
* Returns a list of field names of fields that needs to be in DNS-1123 format from the
* "dns1123Fields" tuning parameter, if it is configured with a comma delimited values
* containing field names.
*
* @return String array containing a list of fields that are required to be * in DNS-1123 format,
* or null if "dns1123Fields" tuning parameter is not configured.
*/
private static String[] getConfiguredDns1123Fields() {
String configuredValue = TuningParameters.getInstance().get(DNS_1123_FIELDS_PARAM);

if (configuredValue == null) {
return null;
}

Collection<String> fields = new ArrayList<>();

if (configuredValue != null) {
StringTokenizer st = new StringTokenizer(configuredValue, ",");
while (st.hasMoreTokens()) {
fields.add(st.nextToken().trim());
}
}
String[] fieldsArray = new String[fields.size()];
return fields.toArray(fieldsArray);
}


private static String[] getDns1123Fields() {
if (dns1123Fields == null) {
dns1123Fields = Optional.ofNullable(getConfiguredDns1123Fields()).orElse(DEFAULT_DNS1123_FIELDS);
}
return dns1123Fields;
}

/**
* Returns true if the value in the field is required to be DNS-1123 legal.
*
* @param fieldName Name of the field to be checked
* @return true if the value needs to be DNS1123 legal, false otherwise
*/
public static boolean isDNS1123Required(String fieldName) {
if (fieldName != null) {
for (String dns1123Field: getDns1123Fields()) {
if (dns1123Field.equalsIgnoreCase(fieldName)) {
return true;
}
}
}
return false;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -160,8 +160,9 @@ static List<V1EnvVar> createCopy(List<V1EnvVar> envVars) {
ArrayList<V1EnvVar> copy = new ArrayList<>();
if (envVars != null) {
for (V1EnvVar envVar : envVars) {
// note that a deep copy of valueFrom is not needed here as, unlike with value, we are
// not doing any modifications or macro substitutions on the valueFrom fields
// note that a deep copy of valueFrom is not needed here as, unlike with value, the
// new V1EnvVarFrom objects would be created by the doDeepSubstitutions() method in
// StepContextBase class.
copy.add(new V1EnvVar()
.name(envVar.getName())
.value(envVar.getValue())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,9 +94,7 @@ final List<V1EnvVar> getEnvironmentVariables(TuningParameters tuningParameters)
vars, "USER_MEM_ARGS", "-XX:+UseContainerSupport -Djava.security.egd=file:/dev/./urandom");

hideAdminUserCredentials(vars);
doSubstitution(varsToSubVariables(vars), vars);

return vars;
return doDeepSubstitution(varsToSubVariables(vars), vars);
}

protected Map<String, String> varsToSubVariables(List<V1EnvVar> vars) {
Expand All @@ -110,15 +108,13 @@ protected Map<String, String> varsToSubVariables(List<V1EnvVar> vars) {
return substitutionVariables;
}

protected void doSubstitution(final Map<String, String> substitutionVariables, List<V1EnvVar> vars) {
for (V1EnvVar var : vars) {
var.setValue(translate(substitutionVariables, var.getValue()));
}
protected <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) {
protected <T> T doDeepSubstitution(final Map<String, String> substitutionVariables, T obj, boolean requiresDNS1123) {
if (obj instanceof String) {
return (T) translate(substitutionVariables, (String) obj);
return (T) translate(substitutionVariables, (String) obj, requiresDNS1123);
} else if (obj instanceof List) {
List<Object> result = new ArrayList<>();
for (Object o : (List) obj) {
Expand All @@ -144,7 +140,11 @@ protected <T> T doDeepSubstitution(final Map<String, String> substitutionVariabl
for (Pair<Method, Method> item : typeBeans) {
item.getRight()
.invoke(
subObj, doDeepSubstitution(substitutionVariables, item.getLeft().invoke(obj)));
subObj,
doDeepSubstitution(
substitutionVariables,
item.getLeft().invoke(obj),
isDNS1123Required(item.getLeft())));
}
return subObj;
} catch (NoSuchMethodException
Expand All @@ -158,6 +158,12 @@ protected <T> T doDeepSubstitution(final Map<String, String> substitutionVariabl
return obj;
}

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));
}

private static final String MODELS_PACKAGE = V1Pod.class.getPackageName();
private static final String DOMAIN_MODEL_PACKAGE = Domain.class.getPackageName();

Expand Down Expand Up @@ -193,10 +199,15 @@ private List<Pair<Method, Method>> typeBeans(Class cls) {
}

private String translate(final Map<String, String> substitutionVariables, String rawValue) {
return translate(substitutionVariables, rawValue, false);
}

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()), entry.getValue());
result = result.replace(String.format("$(%s)", entry.getKey()),
requiresDNS1123 ? LegalNames.toDns1123LegalName(entry.getValue()) : entry.getValue());
}
}
return result;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,8 @@ protected DomainSpec getDomainSpec() {

public abstract DomainConfigurator withAdditionalVolume(String name, String path);

public abstract DomainConfigurator withAdditionalPVClaimVolume(String name, String claimName);

public abstract DomainConfigurator withAdditionalVolumeMount(String name, String path);

public abstract DomainConfigurator withInitContainer(V1Container initContainer);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,10 @@ void addAdditionalVolume(String name, String path) {
serverPod.addAdditionalVolume(name, path);
}

void addAdditionalPVClaimVolume(String name, String claimName) {
serverPod.addAdditionalPVClaimVolume(name, claimName);
}

public List<V1VolumeMount> getAdditionalVolumeMounts() {
return serverPod.getAdditionalVolumeMounts();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,12 @@ public DomainConfigurator withAdditionalVolume(String name, String path) {
return this;
}

@Override
public DomainConfigurator withAdditionalPVClaimVolume(String name, String claimName) {
getDomainSpec().addAdditionalPVClaimVolume(name, claimName);
return this;
}

@Override
public DomainConfigurator withAdditionalVolumeMount(String name, String path) {
getDomainSpec().addAdditionalVolumeMount(name, path);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import io.kubernetes.client.models.V1HostPathVolumeSource;
import io.kubernetes.client.models.V1NodeAffinity;
import io.kubernetes.client.models.V1NodeSelector;
import io.kubernetes.client.models.V1PersistentVolumeClaimVolumeSource;
import io.kubernetes.client.models.V1PodAffinity;
import io.kubernetes.client.models.V1PodAntiAffinity;
import io.kubernetes.client.models.V1PodReadinessGate;
Expand Down Expand Up @@ -608,6 +609,13 @@ private void addAdditionalVolume(V1Volume var) {
volumes.add(var);
}

void addAdditionalPVClaimVolume(String name, String claimName) {
addAdditionalVolume(
new V1Volume().name(name).persistentVolumeClaim(
new V1PersistentVolumeClaimVolumeSource().claimName(claimName))
);
}

void addAdditionalVolumeMount(String name, String path) {
addAdditionalVolumeMount(new V1VolumeMount().name(name).mountPath(path));
}
Expand Down
Loading