Skip to content

Owls 91563 - Changes to validate the container port names and truncate automatically generated long names #2542

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.

Sign up for GitHub

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 4 commits into from
Sep 23, 2021
Merged

Owls 91563 - Changes to validate the container port names and truncate automatically generated long names #2542

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ddaaa3b
Changes for OWLS-91563 to validate container port name and truncate g…
ankedia Sep 22, 2021
da34f21
Minor changes.
ankedia Sep 22, 2021
a3cae56
Use existing portNamePrefix variable.
ankedia Sep 22, 2021
f9d576a
Added validation error message for illegal channel name length as per…
ankedia Sep 23, 2021
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
3 changes: 3 additions & 0 deletions operator/src/main/java/oracle/kubernetes/operator/helpers/LegalNames.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,9 @@ public class LegalNames {
// The maximum length of a legal DNS label name
public static final int LEGAL_DNS_LABEL_NAME_MAX_LENGTH = 63;

// The maximum length of a container port name
public static final int LEGAL_CONTAINER_PORT_NAME_MAX_LENGTH = 15;

private static final String DNS_NAME_REGEXP = "[a-z0-9]([-a-z0-9]*[a-z0-9])?";
private static final Pattern DNS_NAME_PATTERN = Pattern.compile(DNS_NAME_REGEXP);

Expand Down
33 changes: 32 additions & 1 deletion operator/src/main/java/oracle/kubernetes/operator/helpers/PodStepContext.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@
import oracle.kubernetes.weblogic.domain.model.ServerSpec;
import oracle.kubernetes.weblogic.domain.model.Shutdown;
import org.apache.commons.lang3.builder.EqualsBuilder;
import org.jetbrains.annotations.NotNull;

import static oracle.kubernetes.operator.EventConstants.ROLL_REASON_DOMAIN_RESOURCE_CHANGED;
import static oracle.kubernetes.operator.EventConstants.ROLL_REASON_WEBLOGIC_CONFIGURATION_CHANGED;
Expand All @@ -91,6 +92,7 @@
import static oracle.kubernetes.operator.helpers.CompatibilityCheck.CompatibilityScope.UNKNOWN;
import static oracle.kubernetes.operator.helpers.EventHelper.EventItem.DOMAIN_ROLL_STARTING;
import static oracle.kubernetes.operator.helpers.EventHelper.EventItem.POD_CYCLE_STARTING;
import static oracle.kubernetes.operator.helpers.LegalNames.LEGAL_CONTAINER_PORT_NAME_MAX_LENGTH;

public abstract class PodStepContext extends BasePodStepContext {

Expand Down Expand Up @@ -295,7 +297,7 @@ private boolean isSipProtocol(NetworkAccessPoint nap) {
}

private void addContainerPort(List<V1ContainerPort> ports, NetworkAccessPoint nap) {
String name = LegalNames.toDns1123LegalName(nap.getName());
String name = createContainerPortName(ports, LegalNames.toDns1123LegalName(nap.getName()));
addContainerPort(ports, name, nap.getListenPort(), "TCP");

if (isSipProtocol(nap)) {
Expand All @@ -306,10 +308,39 @@ private void addContainerPort(List<V1ContainerPort> ports, NetworkAccessPoint na
private void addContainerPort(List<V1ContainerPort> ports, String name,
@Nullable Integer listenPort, String protocol) {
if (listenPort != null) {
name = createContainerPortName(ports, name);
ports.add(new V1ContainerPort().name(name).containerPort(listenPort).protocol(protocol));
}
}

private String createContainerPortName(List<V1ContainerPort> ports, String name) {
//Container port names can be a maximum of 15 characters in length
if (name.length() > LEGAL_CONTAINER_PORT_NAME_MAX_LENGTH) {
String portNamePrefix = getPortNamePrefix(name);
// Find ports with the name having the same first 12 characters
List<V1ContainerPort> containerPortsWithSamePrefix = ports.stream().filter(port ->
portNamePrefix.equals(getPortNamePrefix(port.getName()))).collect(Collectors.toList());
int index = containerPortsWithSamePrefix.size() + 1;
String indexStr = String.valueOf(index);
// zero fill to the left for single digit index (e.g. 01)
if (index < 10) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess we're hoping that no one ever has 100 channels :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All kidding aside... It is extremely unlikely that there will be over 100 channels that are also longer than the maximum name, but we ought to do something here like generate a specific error message.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:) In addition to 100 channels having longer than the maximum name, these 100 channels also need to have the same first 12 characters. Anyways, I'll look into adding a specific error message.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added an error message when more than 100 channels are configured with long names in f9d576a .

indexStr = "0" + index;
} else if (index >= 100) {
LOGGER.severe(MessageKeys.ILLEGAL_NETWORK_CHANNEL_NAME_LENGTH, getDomainUid(), getServerName(),
name, LEGAL_CONTAINER_PORT_NAME_MAX_LENGTH);
return name;
}
name = portNamePrefix + "-" + indexStr;
}
return name;
}

@NotNull
private String getPortNamePrefix(String name) {
// Use first 12 characters of port name as prefix due to 15 character port name limit
return name.substring(0, 12);
}

Integer getListenPort() {
return Optional.ofNullable(scan).map(WlsServerConfig::getListenPort).orElse(null);
}
Expand Down
3 changes: 2 additions & 1 deletion operator/src/main/java/oracle/kubernetes/operator/logging/MessageKeys.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,6 @@ public class MessageKeys {
public static final String LOG_WAITING_COUNT = "WLSKO-0193";
public static final String INTERNAL_IDENTITY_INITIALIZATION_FAILED = "WLSKO-0194";


// domain status messages
public static final String DUPLICATE_SERVER_NAME_FOUND = "WLSDO-0001";
public static final String DUPLICATE_CLUSTER_NAME_FOUND = "WLSDO-0002";
Expand Down Expand Up @@ -176,6 +175,8 @@ public class MessageKeys {
public static final String MONITORING_EXPORTER_CONFLICT_DYNAMIC_CLUSTER = "WLSDO-0028";
public static final String INVALID_LIVENESS_PROBE_SUCCESS_THRESHOLD_VALUE = "WLSDO-0029";
public static final String RESERVED_CONTAINER_NAME = "WLSDO-0030";
public static final String ILLEGAL_CONTAINER_PORT_NAME_LENGTH = "WLSDO-0031";
public static final String ILLEGAL_NETWORK_CHANNEL_NAME_LENGTH = "WLSDO-0032";

private MessageKeys() {
}
Expand Down
14 changes: 14 additions & 0 deletions operator/src/main/java/oracle/kubernetes/operator/wlsconfig/WlsDomainConfig.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -407,6 +407,20 @@ public WlsDomainConfig withAdminServer(String adminServerName, String listenAddr
return this;
}

/**
* Build the domain config with a WLS server.
* @param server WLS server configuration
* @param isAdmin Specifies if the server is Admin or managed server.
* @return domain config
*/
public WlsDomainConfig withServer(WlsServerConfig server, boolean isAdmin) {
if (isAdmin) {
setAdminServerName(server.getName());
}
getServers().add(server);
return this;
}

public WlsDomainConfig addWlsServer(String name, String listenAddress, int port) {
getServers().add(new WlsServerConfig(name, listenAddress, port));
return this;
Expand Down
29 changes: 29 additions & 0 deletions operator/src/main/java/oracle/kubernetes/weblogic/domain/model/Domain.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import com.google.gson.annotations.SerializedName;
import io.kubernetes.client.common.KubernetesObject;
import io.kubernetes.client.openapi.models.V1Container;
import io.kubernetes.client.openapi.models.V1ContainerPort;
import io.kubernetes.client.openapi.models.V1EnvVar;
import io.kubernetes.client.openapi.models.V1LocalObjectReference;
import io.kubernetes.client.openapi.models.V1ObjectMeta;
Expand Down Expand Up @@ -833,6 +834,7 @@ List<String> getValidationFailures(KubernetesResourceLookup kubernetesResources)
addDuplicateAuxiliaryImageVolumeNames();
verifyLivenessProbeSuccessThreshold();
verifyContainerNameValidInPodSpec();
verifyContainerPortNameValidInPodSpec();

return failures;
}
Expand Down Expand Up @@ -1118,6 +1120,33 @@ private void isContainerNameReserved(V1Container container, String prefix) {
}
}

private void verifyContainerPortNameValidInPodSpec() {
getAdminServerSpec().getContainers().forEach(container ->
areContainerPortNamesValid(container, ADMIN_SERVER_POD_SPEC_PREFIX + ".containers"));
getSpec().getClusters().forEach(cluster ->
cluster.getContainers().forEach(container ->
areContainerPortNamesValid(container, CLUSTER_SPEC_PREFIX + "[" + cluster.getClusterName()
+ "].serverPod.containers")));
getSpec().getManagedServers().forEach(managedServer ->
managedServer.getContainers().forEach(container ->
areContainerPortNamesValid(container, MS_SPEC_PREFIX + "[" + managedServer.getServerName()
+ "].serverPod.containers")));
}

private void areContainerPortNamesValid(V1Container container, String prefix) {
Optional.ofNullable(container.getPorts()).ifPresent(portList ->
portList.forEach(port -> checkPortNameLength(port, container.getName(), prefix)));
}

private void checkPortNameLength(V1ContainerPort port, String name, String prefix) {
if (port.getName().length() > LegalNames.LEGAL_CONTAINER_PORT_NAME_MAX_LENGTH) {
failures.add(DomainValidationMessages.exceedMaxContainerPortName(
getDomainUid(),
prefix + "." + name,
port.getName()));
}
}

@Nonnull
private Set<String> getEnvNames() {
return Optional.ofNullable(spec.getEnv()).stream()
Expand Down
8 changes: 8 additions & 0 deletions operator/src/main/java/oracle/kubernetes/weblogic/domain/model/DomainValidationMessages.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
import oracle.kubernetes.operator.logging.MessageKeys;
import oracle.kubernetes.utils.OperatorUtils;

import static oracle.kubernetes.operator.helpers.LegalNames.LEGAL_CONTAINER_PORT_NAME_MAX_LENGTH;

class DomainValidationMessages {

/**
Expand Down Expand Up @@ -174,4 +176,10 @@ public static String invalidLivenessProbeSuccessThresholdValue(int value, String
public static String reservedContainerName(String name, String prefix) {
return getMessage(MessageKeys.RESERVED_CONTAINER_NAME, name, prefix);
}

public static String exceedMaxContainerPortName(String domainUid, String containerName, String portName) {
return getMessage(MessageKeys.ILLEGAL_CONTAINER_PORT_NAME_LENGTH, domainUid, containerName, portName,
LEGAL_CONTAINER_PORT_NAME_MAX_LENGTH);
}

}
3 changes: 3 additions & 0 deletions operator/src/main/resources/Operator.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,9 @@ WLSDO-0027=The Monitoring Exporter port ''{0}'' specified by ''spec.monitoringEx
WLSDO-0028=The Monitoring Exporter port ''{0}'' specified by ''spec.monitoringExporter.port'' conflicts with Cluster ''{1}'' dynamic server template port ''{2}''
WLSDO-0029=Invalid value ''{0}'' for the liveness probe success threshold under ''{1}''. The liveness probe successThreshold value must be 1.
WLSDO-0030=The container name ''{0}'' specified under ''{1}'' is reserved for use by the operator.
WLSDO-0031=Container port name ''{2}'' for domain with domainUID ''{0}'' and container name ''{1}'' exceeds maximum allowed length ''{3}''.
WLSDO-0032=Network channel name ''{2}'' for domain with domainUID ''{0}'' and server with \
name ''{1}'' exceeds maximum allowed length ''{3}''. Please specify a shorter channel name.

oneEnvVar=variable
multipleEnvVars=variables
Expand Down
40 changes: 40 additions & 0 deletions operator/src/test/java/oracle/kubernetes/operator/helpers/PodHelperTestBase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,7 @@
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.hasEntry;
import static org.hamcrest.Matchers.hasItem;
import static org.hamcrest.Matchers.hasItems;
import static org.hamcrest.Matchers.hasKey;
import static org.hamcrest.Matchers.hasSize;
import static org.hamcrest.Matchers.is;
Expand Down Expand Up @@ -201,6 +202,8 @@ public abstract class PodHelperTestBase extends DomainValidationBaseTest {
private static final int READ_AND_EXECUTE_MODE = 0555;
private static final String TEST_PRODUCT_VERSION = "unit-test";
private static final String NOOP_EXPORTER_CONFIG = "queries:\n";
public static final String LONG_CHANNEL_NAME = "Very_Long_Channel_Name";
public static final String TRUNCATED_PORT_NAME_PREFIX = "very-long-ch";

final TerminalStep terminalStep = new TerminalStep();
private final Domain domain = createDomain();
Expand Down Expand Up @@ -378,6 +381,43 @@ void whenPodCreatedWithAdminNap_prometheusAnnotationsSpecifyPlainTextPort() {
hasEntry("prometheus.io/scrape", "true")));
}

@Test
void whenPodCreatedWithAdminNapNameExceedingMaxPortNameLength_podContainerCreatedWithTruncatedPortName() {
getServerTopology().addNetworkAccessPoint(new NetworkAccessPoint(LONG_CHANNEL_NAME, "admin", 8001, 8001));
assertThat(
getContainerPorts(),
hasItem(createContainerPort(TRUNCATED_PORT_NAME_PREFIX + "-01")));
}

private List<V1ContainerPort> getContainerPorts() {
return getCreatedPod().getSpec().getContainers().stream()
.filter(c -> c.getName().equals(WLS_CONTAINER_NAME)).findFirst().map(c -> c.getPorts()).orElse(null);
}

private V1ContainerPort createContainerPort(String portName) {
return new V1ContainerPort().name(portName).containerPort(8001).protocol("TCP");
}

@Test
void whenPodCreatedWithMultipleNapsWithNamesExceedingMaxPortNameLength_podContainerCreatedWithTruncatedPortNames() {
getServerTopology().addNetworkAccessPoint(new NetworkAccessPoint(LONG_CHANNEL_NAME + "1", "admin", 8001, 8001));
getServerTopology().addNetworkAccessPoint(new NetworkAccessPoint(LONG_CHANNEL_NAME + "11", "admin", 8001, 8001));
assertThat(
getContainerPorts(),
hasItems(createContainerPort(TRUNCATED_PORT_NAME_PREFIX + "-01"),
createContainerPort(TRUNCATED_PORT_NAME_PREFIX + "-02")));
}

@Test
void whenPodCreatedWithMultipleNapsSomeWithNamesExceedingMaxPortNameLength_podContainerCreatedWithMixedPortNames() {
getServerTopology().addNetworkAccessPoint(new NetworkAccessPoint(LONG_CHANNEL_NAME, "admin", 8001, 8001));
getServerTopology().addNetworkAccessPoint(new NetworkAccessPoint("My_Channel_Name", "admin", 8001, 8001));
assertThat(
getContainerPorts(),
hasItems(createContainerPort(TRUNCATED_PORT_NAME_PREFIX + "-01"),
createContainerPort("my-channel-name")));
}

protected DomainConfigurator defineExporterConfiguration() {
return configureDomain()
.withMonitoringExporterConfiguration(NOOP_EXPORTER_CONFIG)
Expand Down
38 changes: 38 additions & 0 deletions operator/src/test/java/oracle/kubernetes/weblogic/domain/model/DomainValidationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,13 @@
package oracle.kubernetes.weblogic.domain.model;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;

import com.meterware.simplestub.Memento;
import io.kubernetes.client.openapi.models.V1Container;
import io.kubernetes.client.openapi.models.V1ContainerPort;
import io.kubernetes.client.openapi.models.V1LocalObjectReference;
import oracle.kubernetes.operator.TuningParameters;
import oracle.kubernetes.operator.helpers.KubernetesTestSupport;
Expand Down Expand Up @@ -50,6 +52,7 @@ class DomainValidationTest extends DomainValidationBaseTest {
private static final String BAD_MOUNT_PATH_3 = "$()DOMAIN_HOME/servers/SERVER_NAME";
public static final String TEST_VOLUME_NAME = "test";
public static final String WRONG_VOLUME_NAME = "BadVolume";
public static final String LONG_CONTAINER_PORT_NAME = "long-container-port-name";

private final Domain domain = createTestDomain();
private final KubernetesTestSupport testSupport = new KubernetesTestSupport();
Expand Down Expand Up @@ -530,6 +533,41 @@ void whenReservedContainerNameUsedForManagedServer_reportError() {
"is reserved", "operator")));
}

@Test
void whenContainerPortNameExceedsMaxLength_ForAdminServerContainer_reportError() {
configureDomain(domain)
.withContainer(new V1Container().name("Test")
.ports(Arrays.asList(new V1ContainerPort().name(LONG_CONTAINER_PORT_NAME))));

assertThat(domain.getValidationFailures(resourceLookup), contains(stringContainsInOrder(
"Container port name ", LONG_CONTAINER_PORT_NAME, "domainUID", UID, "adminServer", "Test",
"exceeds maximum allowed length '15'")));
}

@Test
void whenContainerPortNameExceedsMaxLength_ForClusteredServerContainer_reportError() {
configureDomain(domain)
.configureCluster("cluster-1")
.withContainer(new V1Container().name("Test")
.ports(Arrays.asList(new V1ContainerPort().name(LONG_CONTAINER_PORT_NAME))));

assertThat(domain.getValidationFailures(resourceLookup), contains(stringContainsInOrder(
"Container port name ", LONG_CONTAINER_PORT_NAME, "domainUID", UID, "cluster-1", "Test",
"exceeds maximum allowed length '15'")));
}

@Test
void whenContainerPortNameExceedsMaxLength_ForManagedServerContainer_reportError() {
configureDomain(domain)
.configureServer("managed-server1")
.withContainer(new V1Container().name("Test")
.ports(Arrays.asList(new V1ContainerPort().name(LONG_CONTAINER_PORT_NAME))));

assertThat(domain.getValidationFailures(resourceLookup), contains(stringContainsInOrder(
"Container port name ", LONG_CONTAINER_PORT_NAME, "domainUID", UID, "managed-server1", "Test",
"exceeds maximum allowed length '15'")));
}

@Test
void whenWebLogicCredentialsSecretNameNotFound_reportError() {
resourceLookup.undefineResource(SECRET_NAME, KubernetesResourceType.Secret, NS);
Expand Down