Skip to content

Commit ee8bcb7

Browse files
committed
fix: do not normalize diff into move and copy operations in GenericKubernetesResourceMatcher
An added value equal to an unchanged value (for example a Service targetPort defaulted to its port) became a copy operation and caused a false mismatch. A removed value re-added under an ignored path became a move operation and was ignored. matchStatus already disables this normalization.
1 parent b0ef77f commit ee8bcb7

2 files changed

Lines changed: 44 additions & 1 deletion

File tree

‎operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericKubernetesResourceMatcher.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,9 @@ public static <R extends HasMetadata, P extends HasMetadata> Matcher.Result<R> m
183183
final var kubernetesSerialization = context.getClient().getKubernetesSerialization();
184184
var desiredNode = kubernetesSerialization.convertValue(desired, JsonNode.class);
185185
var actualNode = kubernetesSerialization.convertValue(actualResource, JsonNode.class);
186-
var wholeDiffJsonPatch = JsonDiff.asJson(desiredNode, actualNode);
186+
// move and copy operations would hide additions and removals from the checks below
187+
var wholeDiffJsonPatch =
188+
JsonDiff.asJson(desiredNode, actualNode, DiffFlags.dontNormalizeOpIntoMoveAndCopy());
187189

188190
boolean matched = true;
189191
for (int i = 0; i < wholeDiffJsonPatch.size() && matched; i++) {

‎operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericKubernetesResourceMatcherTest.java‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,47 @@ void matchesAdditiveOnlyChanges() {
7777
.isTrue();
7878
}
7979

80+
@Test
81+
void matchesAdditiveChangesWithValueEqualToAnExistingValue() {
82+
var desired =
83+
new ServiceBuilder()
84+
.withNewMetadata()
85+
.withName("test")
86+
.endMetadata()
87+
.withNewSpec()
88+
.addNewPort()
89+
.withPort(80)
90+
.endPort()
91+
.endSpec()
92+
.build();
93+
// the API server defaults targetPort to the value of port
94+
var actual =
95+
new ServiceBuilder(desired)
96+
.editSpec()
97+
.editFirstPort()
98+
.withTargetPort(new IntOrString(80))
99+
.endPort()
100+
.endSpec()
101+
.build();
102+
assertThat(GenericKubernetesResourceMatcher.match(desired, actual, context).matched())
103+
.withFailMessage(
104+
"Additive changes should not cause a mismatch even if the added value equals an"
105+
+ " existing one")
106+
.isTrue();
107+
}
108+
109+
@Test
110+
void doesNotMatchRemovedValueAddedBackUnderIgnoredPath() {
111+
actual = createDeployment();
112+
// revisionHistoryLimit is 10 in desired, the added priority has the same value
113+
actual.getSpec().setRevisionHistoryLimit(null);
114+
actual.getSpec().getTemplate().getSpec().setPriority(10);
115+
assertThat(
116+
match(dependentResource, actual, null, context, false, "/spec/template/spec").matched())
117+
.withFailMessage("Removing values outside of ignored paths should lead to a mismatch")
118+
.isFalse();
119+
}
120+
80121
@Test
81122
void matchesWithStrongSpecEquality() {
82123
actual.getSpec().getTemplate().getMetadata().getLabels().put("new-key", "val");

0 commit comments

Comments
 (0)