Skip to content

Commit 34b559e

Browse files
authored
feat: strong equality matcher (#1733)
1 parent 9abcb37 commit 34b559e

File tree

2 files changed

+49
-2
lines changed

2 files changed

+49
-2
lines changed

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

Lines changed: 44 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,11 +29,40 @@ static <R extends HasMetadata, P extends HasMetadata> Matcher<R, P> matcherFor(
2929
@Override
3030
public Result<R> match(R actualResource, P primary, Context<P> context) {
3131
var desired = dependentResource.desired(primary, context);
32-
return match(desired, actualResource, false);
32+
return match(desired, actualResource, false, false);
3333
}
3434

3535
public static <R extends HasMetadata> Result<R> match(R desired, R actualResource,
3636
boolean considerMetadata) {
37+
return match(desired, actualResource, considerMetadata, false);
38+
}
39+
40+
/**
41+
* Determines whether the specified actual resource matches the specified desired resource,
42+
* possibly considering metadata and deeper equality checks.
43+
*
44+
* @param desired the desired resource
45+
* @param actualResource the actual resource
46+
* @param considerMetadata {@code true} if labels and annotations will be checked for equality,
47+
* {@code false} otherwise (meaning that metadata changes will be ignored for matching
48+
* purposes)
49+
* @param equality if {@code false}, the algorithm checks if the properties in the desired
50+
* resource spec are same as in the actual resource spec. The reason is that admission
51+
* controllers and default Kubernetes controllers might add default values to some
52+
* properties which are not set in the desired resources' spec and comparing it with simple
53+
* equality check would mean that such resource will not match (while conceptually should).
54+
* However, there is an issue with this for example if desired spec contains a list of
55+
* values and a value is removed, this still will match the actual state from previous
56+
* reconciliation. Setting this parameter to {@code true}, will match the resources only if
57+
* all properties and values are equal. This could be implemented also by overriding equals
58+
* method of spec, should be done as an optimization - this implementation does not require
59+
* that.
60+
*
61+
* @return results of matching
62+
* @param <R> resource
63+
*/
64+
public static <R extends HasMetadata> Result<R> match(R desired, R actualResource,
65+
boolean considerMetadata, boolean equality) {
3766
if (considerMetadata) {
3867
final var desiredMetadata = desired.getMetadata();
3968
final var actualMetadata = actualResource.getMetadata();
@@ -61,6 +90,12 @@ public static <R extends HasMetadata> Result<R> match(R desired, R actualResourc
6190
var desiredSpecNode = objectMapper.valueToTree(ReconcilerUtils.getSpec(desired));
6291
var actualSpecNode = objectMapper.valueToTree(ReconcilerUtils.getSpec(actualResource));
6392
var diffJsonPatch = JsonDiff.asJson(desiredSpecNode, actualSpecNode);
93+
// In case of equality is set to true, no diffs are allowed, so we return early if diffs exist
94+
// On contrary (if equality is false), "add" is allowed for cases when for some
95+
// resources Kubernetes fills-in values into spec.
96+
if (equality && diffJsonPatch.size() > 0) {
97+
return Result.computed(false, desired);
98+
}
6499
for (int i = 0; i < diffJsonPatch.size(); i++) {
65100
String operation = diffJsonPatch.get(i).get("op").asText();
66101
if (!operation.equals("add")) {
@@ -90,10 +125,17 @@ public static <R extends HasMetadata> Result<R> match(R desired, R actualResourc
90125
* @param <P> the type of primary resources associated with the secondary resources we want to
91126
* match
92127
*/
128+
public static <R extends HasMetadata, P extends HasMetadata> Result<R> match(
129+
KubernetesDependentResource<R, P> dependentResource, R actualResource, P primary,
130+
Context<P> context, boolean considerMetadata, boolean strongEquality) {
131+
final var desired = dependentResource.desired(primary, context);
132+
return match(desired, actualResource, considerMetadata, strongEquality);
133+
}
134+
93135
public static <R extends HasMetadata, P extends HasMetadata> Result<R> match(
94136
KubernetesDependentResource<R, P> dependentResource, R actualResource, P primary,
95137
Context<P> context, boolean considerMetadata) {
96138
final var desired = dependentResource.desired(primary, context);
97-
return match(desired, actualResource, considerMetadata);
139+
return match(desired, actualResource, considerMetadata, false);
98140
}
99141
}

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,11 @@ void checksIfDesiredValuesAreTheSame() {
4343
.withFailMessage("Additive changes should be ok")
4444
.isTrue();
4545

46+
assertThat(GenericKubernetesResourceMatcher
47+
.match(dependentResource, actual, null, context, true, true).matched())
48+
.withFailMessage("Strong equality does not ignore additive changes on spec")
49+
.isFalse();
50+
4651
actual = createDeployment();
4752
assertThat(matcher.match(actual, createPrimary("removed"), context).matched())
4853
.withFailMessage("Removed value should not be ok")

0 commit comments

Comments
 (0)