From cd1cd06022b5b4e98de944c92950bb69901c9241 Mon Sep 17 00:00:00 2001 From: Steve Hawkins Date: Wed, 7 Dec 2022 16:43:05 -0500 Subject: [PATCH] fix #4363: refining patch logic and adding metadata compare utility --- CHANGELOG.md | 2 + .../client/utils/ResourceCompare.java | 24 ++++++++ .../client/utils/ResourceCompareTest.java | 7 +++ .../client/dsl/internal/ObjectMetaMixIn.java | 58 ------------------- .../client/dsl/internal/PatchUtils.java | 54 ++++++++--------- 5 files changed, 57 insertions(+), 88 deletions(-) delete mode 100644 kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/ObjectMetaMixIn.java diff --git a/CHANGELOG.md b/CHANGELOG.md index 886feadd01d..aa9ce48a5e1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,7 @@ * Fix #4201: Removed sendAsync from the individual http client implementations * Fix #4355: for exec, attach, upload, and copy operations the container id/name will be validated or chosen prior to the remote call. You may also use the kubectl.kubernetes.io/default-container annotation to specify the default container. * Fix #4530: generalizing the Serialization logic to allow for primitive values and clarifying the type expectations. +* Fix #4363: exposed ResourceCompare.metadataChanged #### Dependency Upgrade @@ -35,6 +36,7 @@ * Fix #4515: files located at the root of jars named model.properties, e.g. core.properties, have been removed * Fix #4579: the implicit registration of resource and list types that happens when using the resource(class) methods has been removed. This makes the behavior of the client more predictable as that was an undocumented side-effect. If you expect to see instances of a custom type from an untyped api call - typically KubernetesClient.load, KubernetesClient.resourceList, KubernetesClient.resource(InputStream|String), then you must either create a META-INF/services/io.fabric8.kubernetes.api.model.KubernetesResource file (see above #3923), or make calls to KubernetesDeserializer.registerCustomKind - however since KubernetesDeserializer is an internal class that mechanism is not preferred. * Fix #4597: remove the deprecated support for `javax.validation.constraints.NotNull` in the `crd-generator`, to mark a property as `required` it needs to be annotated with `io.fabric8.generator.annotation.Required` +* Fix #4363: deprecated existing ResourceCompare methods such as compareKubernetesResource as they are not for general use ### 6.2.0 (2022-10-20) diff --git a/kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/utils/ResourceCompare.java b/kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/utils/ResourceCompare.java index d111de99230..6d8af451c78 100644 --- a/kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/utils/ResourceCompare.java +++ b/kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/utils/ResourceCompare.java @@ -19,10 +19,13 @@ import com.fasterxml.jackson.core.type.TypeReference; import com.fasterxml.jackson.databind.ObjectMapper; import io.fabric8.kubernetes.api.model.KubernetesList; +import io.fabric8.kubernetes.api.model.ObjectMeta; +import io.fabric8.kubernetes.api.model.ObjectMetaBuilder; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Objects; public class ResourceCompare { private ResourceCompare() { @@ -47,6 +50,7 @@ private ResourceCompare() { * * @return boolean value whether both resources are actually equal or not */ + @Deprecated public static boolean equals(T left, T right) { ObjectMapper jsonMapper = Serialization.jsonMapper(); if (left == null && right == null) { @@ -66,6 +70,7 @@ public static boolean equals(T left, T right) { } } + @Deprecated public static boolean compareKubernetesList(Map leftJson, Map rightJson) { List> leftItems = (List>) leftJson.get(ITEMS); List> rightItems = (List>) rightJson.get(ITEMS); @@ -85,6 +90,7 @@ public static boolean compareKubernetesList(Map leftJson, Map leftJson, Map rightJson) { return isEqualMetadata(leftJson, rightJson) && isEqualSpec(leftJson, rightJson); @@ -139,4 +145,22 @@ private static boolean isLeftMapSupersetOfRight(Map leftMap, Map } return true; } + + /** + * Compare the two metadata objects after they have been stripped of api server managed fields, such as uid + * + * @return true if the metadata are the same ignoring + */ + public static boolean metadataChanged(ObjectMeta object1, ObjectMeta object2) { + return !Objects.equals(withoutRuntimeState(object1), withoutRuntimeState(object2)); + } + + static ObjectMeta withoutRuntimeState(ObjectMeta meta) { + if (meta == null) { + return null; + } + ObjectMetaBuilder builder = new ObjectMetaBuilder(meta); + return builder.withCreationTimestamp(null).withDeletionTimestamp(null).withGeneration(null).withResourceVersion(null) + .withSelfLink(null).withUid(null).build(); + } } diff --git a/kubernetes-client-api/src/test/java/io/fabric8/kubernetes/client/utils/ResourceCompareTest.java b/kubernetes-client-api/src/test/java/io/fabric8/kubernetes/client/utils/ResourceCompareTest.java index 3bdf8c8f3bc..376c6578d43 100644 --- a/kubernetes-client-api/src/test/java/io/fabric8/kubernetes/client/utils/ResourceCompareTest.java +++ b/kubernetes-client-api/src/test/java/io/fabric8/kubernetes/client/utils/ResourceCompareTest.java @@ -18,6 +18,7 @@ import io.fabric8.kubernetes.api.model.IntOrString; import io.fabric8.kubernetes.api.model.KubernetesList; import io.fabric8.kubernetes.api.model.KubernetesListBuilder; +import io.fabric8.kubernetes.api.model.ObjectMetaBuilder; import io.fabric8.kubernetes.api.model.Pod; import io.fabric8.kubernetes.api.model.PodBuilder; import io.fabric8.kubernetes.api.model.ReplicationController; @@ -149,4 +150,10 @@ void testEqualsWhenOneResourceIsNull() { void testEqualsWhenBothNull() { assertTrue(ResourceCompare.equals(null, null)); } + + @Test + void testMetadataChanged() { + assertFalse(ResourceCompare.metadataChanged(new ObjectMetaBuilder().withResourceVersion("1").build(), + new ObjectMetaBuilder().withResourceVersion("2").build())); + } } diff --git a/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/ObjectMetaMixIn.java b/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/ObjectMetaMixIn.java deleted file mode 100644 index 227131e3b03..00000000000 --- a/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/ObjectMetaMixIn.java +++ /dev/null @@ -1,58 +0,0 @@ -/** - * Copyright (C) 2015 Red Hat, Inc. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package io.fabric8.kubernetes.client.dsl.internal; - -import com.fasterxml.jackson.annotation.JsonIgnore; -import io.fabric8.kubernetes.api.model.ObjectMeta; - -public abstract class ObjectMetaMixIn extends ObjectMeta { - @JsonIgnore - private String creationTimestamp; - @JsonIgnore - private String deletionTimestamp; - @JsonIgnore - private Long generation; - @JsonIgnore - private String resourceVersion; - @JsonIgnore - private String selfLink; - @JsonIgnore - private String uid; - - @Override - @JsonIgnore - public abstract String getCreationTimestamp(); - - @Override - @JsonIgnore - public abstract String getDeletionTimestamp(); - - @Override - @JsonIgnore - public abstract Long getGeneration(); - - @Override - @JsonIgnore - public abstract String getResourceVersion(); - - @Override - @JsonIgnore - public abstract String getSelfLink(); - - @Override - @JsonIgnore - public abstract String getUid(); -} diff --git a/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/PatchUtils.java b/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/PatchUtils.java index 804f58f6876..18474f5255f 100644 --- a/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/PatchUtils.java +++ b/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/PatchUtils.java @@ -19,13 +19,13 @@ import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.SerializationFeature; -import io.fabric8.kubernetes.api.model.ObjectMeta; +import com.fasterxml.jackson.databind.node.ObjectNode; import io.fabric8.kubernetes.client.KubernetesClientException; +import io.fabric8.kubernetes.client.utils.ResourceCompare; import io.fabric8.kubernetes.client.utils.Serialization; import io.fabric8.zjsonpatch.JsonDiff; -import java.util.Map; -import java.util.function.BiFunction; +import java.util.Optional; public class PatchUtils { @@ -39,51 +39,45 @@ private PatchUtils() { private static class SingletonHolder { public static final ObjectMapper patchMapper; - public static final ObjectMapper yamlMapper; static { patchMapper = Serialization.jsonMapper().copy(); - patchMapper.addMixIn(ObjectMeta.class, ObjectMetaMixIn.class); + // if this isn't set the patches are still correct, but not as compact for some reason - array values are added individually patchMapper.setConfig(patchMapper.getSerializationConfig().without(SerializationFeature.WRITE_EMPTY_JSON_ARRAYS)); - - yamlMapper = Serialization.yamlMapper().copy(); - yamlMapper.addMixIn(ObjectMeta.class, ObjectMetaMixIn.class); - yamlMapper.setConfig(yamlMapper.getSerializationConfig().without(SerializationFeature.WRITE_EMPTY_JSON_ARRAYS)); } } public static String withoutRuntimeState(Object object, Format format, boolean omitStatus) { - return withoutRuntimeState(object, format, omitStatus, (m, v) -> { - try { - return m.writeValueAsString(v); - } catch (JsonProcessingException e) { - throw KubernetesClientException.launderThrowable(e); - } - }); + ObjectMapper mapper = format == Format.JSON ? Serialization.jsonMapper() : Serialization.yamlMapper(); + try { + return mapper.writeValueAsString(withoutRuntimeState(object, omitStatus)); + } catch (JsonProcessingException e) { + throw KubernetesClientException.launderThrowable(e); + } } + /** + * See also {@link ResourceCompare#withoutRuntimeState} + */ static JsonNode withoutRuntimeState(Object object, boolean omitStatus) { - return withoutRuntimeState(object, Format.JSON, omitStatus, (m, v) -> m.convertValue(v, JsonNode.class)); - } - - static T withoutRuntimeState(Object object, Format format, boolean omitStatus, - BiFunction result) { - ObjectMapper mapper = SingletonHolder.patchMapper; - if (format == Format.YAML) { - mapper = SingletonHolder.yamlMapper; - } - Object value = object; + ObjectNode raw = SingletonHolder.patchMapper.convertValue(object, ObjectNode.class); + Optional.ofNullable(raw.get("metadata")).filter(ObjectNode.class::isInstance).map(ObjectNode.class::cast).ifPresent(m -> { + m.remove("creationTimestamp"); + m.remove("deletionTimestamp"); + m.remove("generation"); + m.remove("resourceVersion"); + m.remove("selfLink"); + m.remove("uid"); + }); if (omitStatus) { - Map raw = mapper.convertValue(object, Map.class); raw.remove("status"); - value = raw; } - return result.apply(mapper, value); + return raw; } public static String jsonDiff(Object current, Object updated, boolean omitStatus) { try { - return SingletonHolder.patchMapper.writeValueAsString( + return Serialization.jsonMapper().writeValueAsString( JsonDiff.asJson(withoutRuntimeState(current, omitStatus), withoutRuntimeState(updated, omitStatus))); } catch (JsonProcessingException e) { throw KubernetesClientException.launderThrowable(e);