Skip to content

Commit 595f8d2

Browse files
committed
consolidating the diffs
also adding a test that removes a field not present in the desired
1 parent 2fbd8a0 commit 595f8d2

File tree

2 files changed

+26
-62
lines changed

2 files changed

+26
-62
lines changed

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

Lines changed: 9 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -188,33 +188,17 @@ public static <R extends HasMetadata, P extends HasMetadata> Result<R> match(R d
188188
"Equality should be false in case of ignore list provided");
189189
}
190190

191-
if (considerMetadata) {
192-
Optional<Result<R>> res =
193-
matchMetadata(desired, actualResource, labelsAndAnnotationsEquality, context);
194-
if (res.isPresent()) {
195-
return res.orElseThrow();
196-
}
197-
}
198-
199-
final var matched = match(actualResource, desired, specEquality, context, ignoredPaths);
200-
return Result.computed(matched, desired);
201-
}
202-
203-
private static <R extends HasMetadata> boolean match(R actual, R desired, boolean equality,
204-
Context<?> context,
205-
String[] ignoredPaths) {
206-
207191
final var kubernetesSerialization = context.getClient().getKubernetesSerialization();
208192
var desiredNode = kubernetesSerialization.convertValue(desired, JsonNode.class);
209-
var actualNode = kubernetesSerialization.convertValue(actual, JsonNode.class);
193+
var actualNode = kubernetesSerialization.convertValue(actualResource, JsonNode.class);
210194
var wholeDiffJsonPatch = JsonDiff.asJson(desiredNode, actualNode);
211195

212-
final List<String> ignoreList =
213-
ignoredPaths != null && ignoredPaths.length > 0 ? Arrays.asList(ignoredPaths)
214-
: Collections.emptyList();
215-
boolean specMatch = match(equality, wholeDiffJsonPatch, ignoreList, SPEC);
196+
boolean specMatch = match(specEquality, wholeDiffJsonPatch, ignoreList, SPEC);
216197
if (!specMatch) {
217-
return false;
198+
return Result.computed(false, desired);
199+
}
200+
if (considerMetadata && !match(labelsAndAnnotationsEquality, wholeDiffJsonPatch, ignoreList, METADATA_LABELS, METADATA_ANNOTATIONS)) {
201+
return Result.computed(false, desired);
218202
}
219203
// expect everything else to be equal
220204
var names = desiredNode.fieldNames();
@@ -225,7 +209,8 @@ private static <R extends HasMetadata> boolean match(R actual, R desired, boolea
225209
prefixes.add(prefix);
226210
}
227211
}
228-
return match(true, wholeDiffJsonPatch, ignoreList, prefixes.toArray(String[]::new));
212+
boolean matched = match(true, wholeDiffJsonPatch, ignoreList, prefixes.toArray(String[]::new));
213+
return Result.computed(matched, desired);
229214
}
230215

231216
private static boolean match(boolean equality, JsonNode wholeDiffJsonPatch,
@@ -241,49 +226,11 @@ private static boolean match(boolean equality, JsonNode wholeDiffJsonPatch,
241226
return false;
242227
}
243228
if (!ignoreList.isEmpty()) {
244-
return allDiffsOnIgnoreList(diffJsonPatch, ignoreList);
229+
return diffJsonPatch.stream().allMatch(n -> nodeIsChildOf(n, ignoreList));
245230
}
246231
return allDiffsAreAddOps(diffJsonPatch);
247232
}
248233

249-
private static boolean allDiffsOnIgnoreList(List<JsonNode> metadataJSonDiffs,
250-
List<String> ignoreList) {
251-
if (metadataJSonDiffs.isEmpty()) {
252-
return false;
253-
}
254-
return metadataJSonDiffs.stream().allMatch(n -> nodeIsChildOf(n, ignoreList));
255-
}
256-
257-
private static <R extends HasMetadata, P extends HasMetadata> Optional<Result<R>> matchMetadata(
258-
R desired,
259-
R actualResource,
260-
boolean labelsAndAnnotationsEquality, Context<P> context) {
261-
262-
if (labelsAndAnnotationsEquality) {
263-
final var desiredMetadata = desired.getMetadata();
264-
final var actualMetadata = actualResource.getMetadata();
265-
266-
final var matched =
267-
Objects.equals(desiredMetadata.getAnnotations(), actualMetadata.getAnnotations()) &&
268-
Objects.equals(desiredMetadata.getLabels(), actualMetadata.getLabels());
269-
if (!matched) {
270-
return Optional.of(Result.computed(false, desired));
271-
}
272-
} else {
273-
final var objectMapper = context.getClient().getKubernetesSerialization();
274-
var desiredNode = objectMapper.convertValue(desired, JsonNode.class);
275-
var actualNode = objectMapper.convertValue(actualResource, JsonNode.class);
276-
var wholeDiffJsonPatch = JsonDiff.asJson(desiredNode, actualNode);
277-
var metadataJSonDiffs = getDiffsImpactingPathsWithPrefixes(wholeDiffJsonPatch,
278-
METADATA_LABELS,
279-
METADATA_ANNOTATIONS);
280-
if (!allDiffsAreAddOps(metadataJSonDiffs)) {
281-
return Optional.of(Result.computed(false, desired));
282-
}
283-
}
284-
return Optional.empty();
285-
}
286-
287234
static boolean nodeIsChildOf(JsonNode n, List<String> prefixes) {
288235
var path = getPath(n);
289236
return prefixes.stream().anyMatch(path::startsWith);

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

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import java.util.HashMap;
44
import java.util.List;
5+
import java.util.Map;
56

67
import org.junit.jupiter.api.BeforeAll;
78
import org.junit.jupiter.api.Test;
@@ -99,6 +100,22 @@ void checkSecret() {
99100
assertThat(secret.getData()).containsOnlyKeys("foo");
100101
}
101102

103+
@Test
104+
void checkSeviceAccount() {
105+
var processor = GenericResourceUpdaterMatcher.updaterMatcherFor(ServiceAccount.class);
106+
var desired = new ServiceAccountBuilder()
107+
.withMetadata(new ObjectMetaBuilder().addToLabels("new", "label").build())
108+
.build();
109+
var actual = new ServiceAccountBuilder()
110+
.withMetadata(new ObjectMetaBuilder().addToLabels("a", "label").build())
111+
.withImagePullSecrets(new LocalObjectReferenceBuilder().withName("secret").build())
112+
.build();
113+
114+
final var serviceAccount = processor.updateResource(actual, desired, context);
115+
assertThat(serviceAccount.getMetadata().getLabels()).isEqualTo(Map.of("a", "label", "new", "label"));
116+
assertThat(serviceAccount.getImagePullSecrets()).isNullOrEmpty();
117+
}
118+
102119
Deployment createDeployment() {
103120
return ReconcilerUtils.loadYaml(
104121
Deployment.class, GenericResourceUpdaterMatcherTest.class, "nginx-deployment.yaml");

0 commit comments

Comments
 (0)