Skip to content

improve: matcher always considers metadata #2273

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.

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 1 commit into from
Mar 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ static <R extends HasMetadata, P extends HasMetadata> Matcher<R, P> matcherFor(
@Override
public Result<R> match(R actualResource, P primary, Context<P> context) {
var desired = dependentResource.desired(primary, context);
return match(desired, actualResource, false, false, false, context);
return match(desired, actualResource, false, false, context);
}

/**
Expand All @@ -67,9 +67,6 @@ public Result<R> match(R actualResource, P primary, Context<P> context) {
*
* @param desired the desired resource
* @param actualResource the actual resource
* @param considerLabelsAndAnnotations {@code true} if labels and annotations will be checked for
* equality, {@code false} otherwise (meaning that metadata changes will be ignored for
* matching purposes)
* @param labelsAndAnnotationsEquality if true labels and annotation match exactly in the actual
* and desired state if false, additional elements are allowed in actual annotations.
* Considered only if considerLabelsAndAnnotations is true.
Expand All @@ -89,9 +86,9 @@ public Result<R> match(R actualResource, P primary, Context<P> context) {
*/
public static <R extends HasMetadata, P extends HasMetadata> Result<R> match(R desired,
R actualResource,
boolean considerLabelsAndAnnotations, boolean labelsAndAnnotationsEquality,
boolean labelsAndAnnotationsEquality,
boolean valuesEquality, Context<P> context) {
return match(desired, actualResource, considerLabelsAndAnnotations,
return match(desired, actualResource,
labelsAndAnnotationsEquality, valuesEquality, context, EMPTY_ARRAY);
}

Expand All @@ -101,9 +98,6 @@ public static <R extends HasMetadata, P extends HasMetadata> Result<R> match(R d
*
* @param desired the desired resource
* @param actualResource the actual resource
* @param considerLabelsAndAnnotations {@code true} if labels and annotations will be checked for
* equality, {@code false} otherwise (meaning that metadata changes will be ignored for
* matching purposes)
* @param labelsAndAnnotationsEquality if true labels and annotation match exactly in the actual
* and desired state if false, additional elements are allowed in actual annotations.
* Considered only if considerLabelsAndAnnotations is true.
Expand All @@ -116,9 +110,9 @@ public static <R extends HasMetadata, P extends HasMetadata> Result<R> match(R d
*/
public static <R extends HasMetadata, P extends HasMetadata> Result<R> match(R desired,
R actualResource,
boolean considerLabelsAndAnnotations, boolean labelsAndAnnotationsEquality,
boolean labelsAndAnnotationsEquality,
Context<P> context, String... ignorePaths) {
return match(desired, actualResource, considerLabelsAndAnnotations,
return match(desired, actualResource,
labelsAndAnnotationsEquality, false, context, ignorePaths);
}

Expand All @@ -133,9 +127,6 @@ public static <R extends HasMetadata, P extends HasMetadata> Result<R> match(R d
* matches the desired state or not
* @param primary the primary resource from which we want to compute the desired state
* @param context the {@link Context} instance within which this method is called
* @param considerLabelsAndAnnotations {@code true} to consider the metadata of the actual
* resource when determining if it matches the desired state, {@code false} if matching
* should occur only considering the spec of the resources
* @param labelsAndAnnotationsEquality if true labels and annotation match exactly in the actual
* and desired state if false, additional elements are allowed in actual annotations.
* Considered only if considerLabelsAndAnnotations is true.
Expand All @@ -150,28 +141,28 @@ public static <R extends HasMetadata, P extends HasMetadata> Result<R> match(R d
*/
public static <R extends HasMetadata, P extends HasMetadata> Result<R> match(
KubernetesDependentResource<R, P> dependentResource, R actualResource, P primary,
Context<P> context, boolean considerLabelsAndAnnotations,
Context<P> context,
boolean labelsAndAnnotationsEquality,
String... ignorePaths) {
final var desired = dependentResource.desired(primary, context);
return match(desired, actualResource, considerLabelsAndAnnotations,
return match(desired, actualResource,
labelsAndAnnotationsEquality, context,
ignorePaths);
}

public static <R extends HasMetadata, P extends HasMetadata> Result<R> match(
KubernetesDependentResource<R, P> dependentResource, R actualResource, P primary,
Context<P> context, boolean considerLabelsAndAnnotations,
Context<P> context,
boolean specEquality,
boolean labelsAndAnnotationsEquality,
boolean specEquality) {
String... ignorePaths) {
final var desired = dependentResource.desired(primary, context);
return match(desired, actualResource, considerLabelsAndAnnotations,
labelsAndAnnotationsEquality, specEquality, context);
return match(desired, actualResource,
labelsAndAnnotationsEquality, specEquality, context, ignorePaths);
}

public static <R extends HasMetadata, P extends HasMetadata> Result<R> match(R desired,
R actualResource,
boolean considerMetadata, boolean labelsAndAnnotationsEquality, boolean valuesEquality,
R actualResource, boolean labelsAndAnnotationsEquality, boolean valuesEquality,
Context<P> context,
String... ignoredPaths) {
final List<String> ignoreList =
Expand All @@ -195,8 +186,7 @@ public static <R extends HasMetadata, P extends HasMetadata> Result<R> match(R d
matched = match(valuesEquality, node, ignoreList);
} else if (nodeIsChildOf(node, List.of(METADATA))) {
// conditionally consider labels and annotations
if (considerMetadata
&& nodeIsChildOf(node, List.of(METADATA_LABELS, METADATA_ANNOTATIONS))) {
if (nodeIsChildOf(node, List.of(METADATA_LABELS, METADATA_ANNOTATIONS))) {
matched = match(labelsAndAnnotationsEquality, node, Collections.emptyList());
}
} else if (!nodeIsChildOf(node, IGNORED_FIELDS)) {
Expand Down Expand Up @@ -227,20 +217,4 @@ static String getPath(JsonNode n) {
return n.get(PATH).asText();
}

@Deprecated(forRemoval = true)
public static <R extends HasMetadata, P extends HasMetadata> Result<R> match(
KubernetesDependentResource<R, P> dependentResource, R actualResource, P primary,
Context<P> context, boolean considerLabelsAndAnnotations, boolean specEquality) {
final var desired = dependentResource.desired(primary, context);
return match(desired, actualResource, considerLabelsAndAnnotations, specEquality, context);
}

@Deprecated(forRemoval = true)
public static <R extends HasMetadata, P extends HasMetadata> Result<R> match(
KubernetesDependentResource<R, P> dependentResource, R actualResource, P primary,
Context<P> context, boolean considerLabelsAndAnnotations, String... ignorePaths) {
final var desired = dependentResource.desired(primary, context);
return match(desired, actualResource, considerLabelsAndAnnotations, true, context, ignorePaths);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ public R updateResource(R actual, R desired, Context<?> context) {

@Override
public boolean matches(R actual, R desired, Context<?> context) {
return GenericKubernetesResourceMatcher.match(desired, actual, true,
return GenericKubernetesResourceMatcher.match(desired, actual,
false, false, context).matched();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,7 @@ void matchesAdditiveOnlyChanges() {
@Test
void matchesWithStrongSpecEquality() {
actual.getSpec().getTemplate().getMetadata().getLabels().put("new-key", "val");
assertThat(match(dependentResource, actual, null, context, true, true,
true)
assertThat(match(desired, actual, true, true, context)
.matched())
.withFailMessage("Adding values should fail matching when strong equality is required")
.isFalse();
Expand Down Expand Up @@ -127,11 +126,11 @@ void matchesMetadata() {
.withFailMessage("Annotations shouldn't matter when metadata is not considered")
.isTrue();

assertThat(match(dependentResource, actual, null, context, true, true, true).matched())
assertThat(match(desired, actual, true, true, context).matched())
.withFailMessage("Annotations should matter when metadata is considered")
.isFalse();

assertThat(match(dependentResource, actual, null, context, true, false).matched())
assertThat(match(desired, actual, false, false, context).matched())
.withFailMessage(
"Should match when strong equality is not considered and only additive changes are made")
.isTrue();
Expand All @@ -157,7 +156,7 @@ void matchConfigMap() {
var actual = createConfigMap();
actual.getData().put("key2", "val2");

var match = GenericKubernetesResourceMatcher.match(desired, actual, true,
var match = GenericKubernetesResourceMatcher.match(desired, actual,
true, false, context);
assertThat(match.matched()).isTrue();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,10 @@ protected Service desired(SSALegacyMatcherCustomResource primary,
@Override
public Result<Service> match(Service actualResource, SSALegacyMatcherCustomResource primary,
Context<SSALegacyMatcherCustomResource> context) {
var desired = desired(primary, context);

return GenericKubernetesResourceMatcher.match(this, actualResource, primary, context,
true, false, false);
false, false);
}

// override just to check the exec count
Expand Down