Skip to content

Commit 52640cf

Browse files
russgoldrussgold
committed
exclude internal cert from checksum and incompatibility computation
1 parent 68648b7 commit 52640cf

File tree

3 files changed

+105
-10
lines changed

3 files changed

+105
-10
lines changed

‎operator/src/main/java/oracle/kubernetes/operator/helpers/PodCompatibility.java

Lines changed: 35 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
import static oracle.kubernetes.operator.VersionConstants.DEFAULT_DOMAIN_VERSION;
1111
import static oracle.kubernetes.operator.helpers.PodCompatibility.asSet;
1212
import static oracle.kubernetes.operator.helpers.PodCompatibility.getMissingElements;
13+
import static oracle.kubernetes.operator.helpers.PodHelper.AdminPodStepContext.INTERNAL_OPERATOR_CERT_ENV;
1314

1415
import io.kubernetes.client.custom.Quantity;
1516
import io.kubernetes.client.models.V1Container;
@@ -21,6 +22,7 @@
2122
import java.lang.reflect.InvocationTargetException;
2223
import java.lang.reflect.Method;
2324
import java.util.ArrayList;
25+
import java.util.Arrays;
2426
import java.util.Collection;
2527
import java.util.Collections;
2628
import java.util.HashMap;
@@ -162,7 +164,7 @@ static class ContainerCompatibility extends CollectiveCompatibility {
162164
add(new EqualResources(expected.getResources(), actual.getResources()));
163165
addSets("volumeMounts", expected.getVolumeMounts(), actual.getVolumeMounts());
164166
addSets("ports", expected.getPorts(), actual.getPorts());
165-
addSets("env", expected.getEnv(), actual.getEnv());
167+
addSetsIgnoring("env", expected.getEnv(), actual.getEnv(), INTERNAL_OPERATOR_CERT_ENV);
166168
addSets("envFrom", expected.getEnvFrom(), actual.getEnvFrom());
167169
}
168170

@@ -275,7 +277,7 @@ static <T> Set<T> asSet(Collection<T> collection) {
275277

276278
static <T> Set<T> getMissingElements(Collection<T> expected, Collection<T> actual) {
277279
Set<T> missing = asSet(expected);
278-
missing.removeAll(actual);
280+
if (actual != null) missing.removeAll(actual);
279281
return missing;
280282
}
281283
}
@@ -284,6 +286,10 @@ interface CompatibilityCheck {
284286
boolean isCompatible();
285287

286288
String getIncompatibility();
289+
290+
default CompatibilityCheck ignoring(String... keys) {
291+
return this;
292+
}
287293
}
288294

289295
abstract class CollectiveCompatibility implements CompatibilityCheck {
@@ -316,6 +322,11 @@ <T> void addSets(String description, List<T> expected, List<T> actual) {
316322
add(CheckFactory.create(description, expected, actual));
317323
}
318324

325+
@SuppressWarnings("SameParameterValue")
326+
<T> void addSetsIgnoring(String description, List<T> expected, List<T> actual, String... keys) {
327+
add(CheckFactory.create(description, expected, actual).ignoring(keys));
328+
}
329+
319330
protected <T> void add(String description, T expected, T actual) {
320331
add(new Equality(description, expected, actual));
321332
}
@@ -363,7 +374,7 @@ private static <T> boolean canBeMap(List<T> list) {
363374
}
364375

365376
private static <T> Map<String, T> asMap(List<T> values) {
366-
if (values == null) return null;
377+
if (values == null) return Collections.emptyMap();
367378
Map<String, T> result = new HashMap<>();
368379
for (T value : values) {
369380
String key = getKey(value);
@@ -412,6 +423,7 @@ class CompatibleMaps<K, V> implements CompatibilityCheck {
412423
private final String description;
413424
private final Map<K, V> expected;
414425
private final Map<K, V> actual;
426+
private List<String> ignoredKeys = new ArrayList<>();
415427

416428
CompatibleMaps(String description, Map<K, V> expected, Map<K, V> actual) {
417429
this.description = description;
@@ -421,12 +433,22 @@ class CompatibleMaps<K, V> implements CompatibilityCheck {
421433

422434
@Override
423435
public boolean isCompatible() {
424-
for (K key : expected.keySet())
425-
if (!actual.containsKey(key) || !Objects.equals(expected.get(key), actual.get(key)))
426-
return false;
436+
for (K key : expected.keySet()) if (isKeyToCheck(key) && isIncompatible(key)) return false;
427437
return true;
428438
}
429439

440+
private boolean isKeyToCheck(K key) {
441+
return !ignoredKeys.contains(key.toString());
442+
}
443+
444+
private boolean isIncompatible(K key) {
445+
return !actual.containsKey(key) || valuesDiffer(key);
446+
}
447+
448+
private boolean valuesDiffer(K key) {
449+
return !Objects.equals(expected.get(key), actual.get(key));
450+
}
451+
430452
@Override
431453
public String getIncompatibility() {
432454
StringBuilder sb = new StringBuilder();
@@ -436,12 +458,18 @@ public String getIncompatibility() {
436458
sb.append(String.format("actual %s has no entry for '%s'%n", description, missingKeys));
437459

438460
for (K key : expected.keySet())
439-
if (actual.containsKey(key) && !Objects.equals(expected.get(key), actual.get(key)))
461+
if (isKeyToCheck(key) && actual.containsKey(key) && valuesDiffer(key))
440462
sb.append(
441463
String.format(
442464
"actual %s has entry '%s' with value '%s' rather than '%s'%n",
443465
description, key, actual.get(key), expected.get(key)));
444466

445467
return sb.toString();
446468
}
469+
470+
@Override
471+
public CompatibilityCheck ignoring(String... keys) {
472+
ignoredKeys.addAll(Arrays.asList(keys));
473+
return this;
474+
}
447475
}

‎operator/src/main/java/oracle/kubernetes/operator/helpers/PodHelper.java

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
package oracle.kubernetes.operator.helpers;
66

7+
import io.kubernetes.client.models.V1Container;
78
import io.kubernetes.client.models.V1DeleteOptions;
89
import io.kubernetes.client.models.V1EnvVar;
910
import io.kubernetes.client.models.V1ObjectMeta;
@@ -12,7 +13,9 @@
1213
import java.util.ArrayList;
1314
import java.util.List;
1415
import java.util.Map;
16+
import java.util.Optional;
1517
import oracle.kubernetes.operator.DomainStatusUpdater;
18+
import oracle.kubernetes.operator.KubernetesConstants;
1619
import oracle.kubernetes.operator.LabelConstants;
1720
import oracle.kubernetes.operator.PodAwaiterStepFactory;
1821
import oracle.kubernetes.operator.ProcessingConstants;
@@ -31,7 +34,7 @@ private PodHelper() {}
3134

3235
static class AdminPodStepContext extends PodStepContext {
3336
private static final String INTERNAL_OPERATOR_CERT_FILE = "internalOperatorCert";
34-
private static final String INTERNAL_OPERATOR_CERT_ENV = "INTERNAL_OPERATOR_CERT";
37+
static final String INTERNAL_OPERATOR_CERT_ENV = "INTERNAL_OPERATOR_CERT";
3538

3639
AdminPodStepContext(Step conflictStep, Packet packet) {
3740
super(conflictStep, packet);
@@ -84,6 +87,28 @@ String getPodReplacedMessageKey() {
8487
return MessageKeys.ADMIN_POD_REPLACED;
8588
}
8689

90+
@Override
91+
V1Pod withNonHashedElements(V1Pod pod) {
92+
V1Pod v1Pod = super.withNonHashedElements(pod);
93+
getContainer(v1Pod).ifPresent(c -> c.addEnvItem(internalCertEnvValue()));
94+
95+
return v1Pod;
96+
}
97+
98+
private V1EnvVar internalCertEnvValue() {
99+
return new V1EnvVar()
100+
.name(INTERNAL_OPERATOR_CERT_ENV)
101+
.value(getInternalOperatorCertFile(TuningParameters.getInstance()));
102+
}
103+
104+
private Optional<V1Container> getContainer(V1Pod v1Pod) {
105+
return v1Pod.getSpec().getContainers().stream().filter(this::isK8sContainer).findFirst();
106+
}
107+
108+
private boolean isK8sContainer(V1Container c) {
109+
return KubernetesConstants.CONTAINER_NAME.equals(c.getName());
110+
}
111+
87112
@Override
88113
protected V1PodSpec createSpec(TuningParameters tuningParameters) {
89114
return super.createSpec(tuningParameters).hostname(getPodName());
@@ -92,7 +117,6 @@ protected V1PodSpec createSpec(TuningParameters tuningParameters) {
92117
@Override
93118
List<V1EnvVar> getConfiguredEnvVars(TuningParameters tuningParameters) {
94119
List<V1EnvVar> vars = new ArrayList<>(getServerSpec().getEnvironmentVariables());
95-
addEnvVar(vars, INTERNAL_OPERATOR_CERT_ENV, getInternalOperatorCertFile(tuningParameters));
96120
overrideContainerWeblogicEnvVars(vars);
97121
return vars;
98122
}

‎operator/src/test/java/oracle/kubernetes/operator/helpers/PodCompatibilityTest.java

Lines changed: 44 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,11 @@
44

55
package oracle.kubernetes.operator.helpers;
66

7+
import static oracle.kubernetes.operator.helpers.PodHelper.AdminPodStepContext.INTERNAL_OPERATOR_CERT_ENV;
8+
import static org.hamcrest.Matchers.blankOrNullString;
79
import static org.hamcrest.Matchers.both;
810
import static org.hamcrest.Matchers.containsString;
11+
import static org.hamcrest.Matchers.emptyOrNullString;
912
import static org.hamcrest.Matchers.is;
1013
import static org.hamcrest.Matchers.not;
1114
import static org.junit.Assert.assertThat;
@@ -15,6 +18,7 @@
1518
import io.kubernetes.client.custom.Quantity;
1619
import io.kubernetes.client.models.V1Container;
1720
import io.kubernetes.client.models.V1ContainerPort;
21+
import io.kubernetes.client.models.V1EnvVar;
1822
import io.kubernetes.client.models.V1Probe;
1923
import io.kubernetes.client.models.V1ResourceRequirements;
2024
import java.util.Arrays;
@@ -67,14 +71,41 @@ public void whenResourcesDontMatch_createErrorMessage() {
6771
public void whenPortsDontMatch_createErrorMessage() {
6872
PodCompatibility.ContainerCompatibility compatibility =
6973
new PodCompatibility.ContainerCompatibility(
70-
new V1Container().name("test").addPortsItem(new V1ContainerPort().containerPort(1100)),
74+
new V1Container().addPortsItem(new V1ContainerPort().containerPort(1100)),
7175
new V1Container().addPortsItem(new V1ContainerPort().containerPort(1234)));
7276

7377
assertThat(
7478
compatibility.getIncompatibility(),
7579
both(containsString("1100")).and(not(containsString("1234"))));
7680
}
7781

82+
@Test
83+
public void whenEnvVarsDontMatch_createErrorMessage() {
84+
String name = "aa";
85+
String value = "bb";
86+
PodCompatibility.ContainerCompatibility compatibility =
87+
new PodCompatibility.ContainerCompatibility(
88+
new V1Container().addEnvItem(envVar(name, value)),
89+
new V1Container().addEnvItem(envVar("aa", "cc")));
90+
91+
assertThat(
92+
compatibility.getIncompatibility(), both(containsString("aa")).and(containsString("bb")));
93+
}
94+
95+
private V1EnvVar envVar(String name, String value) {
96+
return new V1EnvVar().name(name).value(value);
97+
}
98+
99+
@Test
100+
public void whenOnlyCertificateForEnvVarsDontMatch_dontCreateErrorMessage() {
101+
PodCompatibility.ContainerCompatibility compatibility =
102+
new PodCompatibility.ContainerCompatibility(
103+
new V1Container().name("test").addEnvItem(envVar(INTERNAL_OPERATOR_CERT_ENV, "bb")),
104+
new V1Container());
105+
106+
assertThat(compatibility.getIncompatibility(), blankOrNullString());
107+
}
108+
78109
@Test
79110
public void whenExpectedSubsetOfActual_reportCompatible() {
80111
CompatibilityCheck check =
@@ -117,6 +148,18 @@ public void whenCanBeMapsAndExpectedAndActualDifferentValues_reportChangedElemen
117148
assertThat(check.getIncompatibility(), not(containsString("alpha")));
118149
}
119150

151+
@Test
152+
public void ignoreCertForComparisons() {
153+
CompatibilityCheck check =
154+
CheckFactory.create(
155+
"envVars",
156+
Arrays.asList(object("alpha", 1), object(INTERNAL_OPERATOR_CERT_ENV, 3)),
157+
Arrays.asList(object(INTERNAL_OPERATOR_CERT_ENV, 700), object("alpha", 1)))
158+
.ignoring(INTERNAL_OPERATOR_CERT_ENV);
159+
160+
assertThat(check.getIncompatibility(), emptyOrNullString());
161+
}
162+
120163
private Object object(String name, int value) {
121164
return new ObjectWithName(name, value);
122165
}

0 commit comments

Comments
 (0)