diff --git a/e2e/assets/gitrepo/helm-recreate-drift-gitrepo.yaml b/e2e/assets/gitrepo/helm-recreate-drift-gitrepo.yaml deleted file mode 100644 index e455fdc1ef..0000000000 --- a/e2e/assets/gitrepo/helm-recreate-drift-gitrepo.yaml +++ /dev/null @@ -1,10 +0,0 @@ -kind: GitRepo -apiVersion: fleet.cattle.io/v1alpha1 -metadata: - name: {{ .Name }} -spec: - repo: {{ .Repo }} - branch: {{ .Branch }} - clientSecretName: git-auth - paths: - - helm-recreate-drift diff --git a/e2e/assets/helm-recreate-drift/chart/Chart.yaml b/e2e/assets/helm-recreate-drift/chart/Chart.yaml deleted file mode 100644 index 18f9874dd2..0000000000 --- a/e2e/assets/helm-recreate-drift/chart/Chart.yaml +++ /dev/null @@ -1,6 +0,0 @@ -apiVersion: v2 -name: helm-v4-drift-test-chart -description: Test chart with RollingUpdate deployment strategy for Helm v4 drift detection -type: application -version: 0.1.0 -appVersion: "1.0.0" diff --git a/e2e/assets/helm-recreate-drift/chart/templates/deployment.yaml b/e2e/assets/helm-recreate-drift/chart/templates/deployment.yaml deleted file mode 100644 index c0db5ebd1f..0000000000 --- a/e2e/assets/helm-recreate-drift/chart/templates/deployment.yaml +++ /dev/null @@ -1,21 +0,0 @@ -apiVersion: apps/v1 -kind: Deployment -metadata: - name: test-deployment -spec: - replicas: {{ .Values.replicas | default 1 }} - selector: - matchLabels: - app: test-app - strategy: - type: RollingUpdate - template: - metadata: - labels: - app: test-app - spec: - containers: - - name: nginx - image: nginx:1.14.2 - ports: - - containerPort: 80 diff --git a/e2e/assets/helm-recreate-drift/fleet.yaml b/e2e/assets/helm-recreate-drift/fleet.yaml deleted file mode 100644 index 6de3035abe..0000000000 --- a/e2e/assets/helm-recreate-drift/fleet.yaml +++ /dev/null @@ -1,7 +0,0 @@ -namespace: helm-recreate-drift-test - -helm: - chart: chart - releaseName: recreate-strategy-test - values: - replicas: 1 diff --git a/e2e/single-cluster/helm_recreate_drift_test.go b/e2e/single-cluster/helm_recreate_drift_test.go deleted file mode 100644 index 19590d4998..0000000000 --- a/e2e/single-cluster/helm_recreate_drift_test.go +++ /dev/null @@ -1,146 +0,0 @@ -package singlecluster_test - -import ( - "fmt" - "math/rand" - "os" - "path" - "strings" - - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" - - "github.com/rancher/fleet/e2e/testenv" - "github.com/rancher/fleet/e2e/testenv/githelper" - "github.com/rancher/fleet/e2e/testenv/kubectl" -) - -var _ = Describe("Helm v4 Null Field Drift Detection", Label("infra-setup"), func() { - var ( - tmpDir string - clonedir string - k kubectl.Command - gh *githelper.Git - localRepoName string - inClusterRepoURL string - gitrepoName string - r = rand.New(rand.NewSource(GinkgoRandomSeed())) - gitServerPort int - gitProtocol string - ) - - BeforeEach(func() { - k = env.Kubectl.Namespace(env.Namespace) - gitServerPort = 8080 - gitProtocol = "http" - localRepoName = "repo" - }) - - JustBeforeEach(func() { - // Build git repo URL reachable _within_ the cluster, for the GitRepo - host := githelper.BuildGitHostname() - - addr, err := githelper.GetExternalRepoAddr(env, gitServerPort, localRepoName) - Expect(err).ToNot(HaveOccurred()) - addr = strings.Replace(addr, "http://", fmt.Sprintf("%s://", gitProtocol), 1) - gh = githelper.NewHTTP(addr) - - inClusterRepoURL = gh.GetInClusterURL(host, gitServerPort, localRepoName) - - tmpDir, _ = os.MkdirTemp("", "fleet-") - clonedir = path.Join(tmpDir, localRepoName) - - gitrepoName = testenv.RandomFilename("helm-recreate-drift", r) - }) - - AfterEach(func() { - _ = os.RemoveAll(tmpDir) - - _, _ = k.Delete("gitrepo", gitrepoName) - _, _ = k.Delete("secret", "git-auth") - _, _ = k.Delete("namespace", "helm-recreate-drift-test") - - // Check that the bundle deployment resource has been deleted - Eventually(func(g Gomega) { - out, _ := k.Get( - "bundledeployments", - "-A", - "-l", - fmt.Sprintf("fleet.cattle.io/repo-name=%s", gitrepoName), - ) - g.Expect(out).To(ContainSubstring("No resources found")) - }).Should(Succeed()) - }) - - When("deploying a Helm chart with fields omitted by Helm v4", func() { - JustBeforeEach(func() { - // Create git auth secret - _, err := k.Create( - "secret", "generic", "git-auth", "--type", "kubernetes.io/basic-auth", - "--from-literal=username="+os.Getenv("GIT_HTTP_USER"), - "--from-literal=password="+os.Getenv("GIT_HTTP_PASSWORD"), - ) - Expect(err).ToNot(HaveOccurred()) - - err = testenv.ApplyTemplate(k, testenv.AssetPath("gitrepo/helm-recreate-drift-gitrepo.yaml"), struct { - Name string - Repo string - Branch string - }{ - Name: gitrepoName, - Repo: inClusterRepoURL, - Branch: gh.Branch, - }) - Expect(err).ToNot(HaveOccurred()) - - _, err = gh.Create(clonedir, testenv.AssetPath("helm-recreate-drift"), "helm-recreate-drift") - Expect(err).ToNot(HaveOccurred()) - }) - - It("should not detect drift from Helm v4 rendering", func() { - By("checking the bundle is ready") - Eventually(func(g Gomega) { - out, err := k.Get("bundles", "-A", "-l", fmt.Sprintf("fleet.cattle.io/repo-name=%s", gitrepoName)) - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(out).To(ContainSubstring("fleet-local")) - }, testenv.MediumTimeout, testenv.ShortTimeout).Should(Succeed()) - - By("checking the bundle deployment is ready") - var bundleDeploymentName, bdNamespace string - Eventually(func(g Gomega) { - out, err := k.Get("bundledeployments", "-A", "-o", "jsonpath={.items[0]['metadata.name','metadata.namespace']}", "-l", fmt.Sprintf("fleet.cattle.io/repo-name=%s", gitrepoName)) - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(out).ToNot(BeEmpty()) - - data := strings.Split(out, " ") - g.Expect(data).To(HaveLen(2)) - bundleDeploymentName, bdNamespace = data[0], data[1] - }, testenv.MediumTimeout, testenv.ShortTimeout).Should(Succeed()) - - Eventually(func(g Gomega) { - out, err := k.Namespace(bdNamespace).Get("bundledeployments", bundleDeploymentName, "-o", "jsonpath={.status.ready}") - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(out).To(Equal("true")) - }, testenv.MediumTimeout, testenv.ShortTimeout).Should(Succeed()) - - By("checking the deployment has RollingUpdate strategy") - Eventually(func(g Gomega) { - out, err := k.Namespace("helm-recreate-drift-test").Get("deployments", "test-deployment", "-o", "jsonpath={.spec.strategy.type}") - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(out).To(Equal("RollingUpdate")) - }, testenv.MediumTimeout, testenv.ShortTimeout).Should(Succeed()) - - By("verifying that no drift is detected") - Eventually(func(g Gomega) { - out, err := k.Namespace(bdNamespace).Get("bundledeployments", bundleDeploymentName, "-o", "jsonpath={.status.nonModified}") - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(out).To(Equal("true"), "BundleDeployment should not show drift from Helm v4 rendering") - - // Also check that modifiedStatus is empty - out, err = k.Namespace(bdNamespace).Get("bundledeployments", bundleDeploymentName, "-o", "jsonpath={.status.modifiedStatus}") - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(out).To(BeEmpty(), "modifiedStatus should be empty when no drift is detected") - }, testenv.MediumTimeout, testenv.ShortTimeout).Should(Succeed()) - }) - }) -}) diff --git a/go.mod b/go.mod index 34c74eb808..8dbe5d8437 100644 --- a/go.mod +++ b/go.mod @@ -8,6 +8,35 @@ replace ( github.com/imdario/mergo => github.com/imdario/mergo v1.0.2 github.com/rancher/fleet/pkg/apis => ./pkg/apis gopkg.in/go-playground/webhooks.v6 => github.com/go-playground/webhooks/v6 v6.4.0 + k8s.io/api => k8s.io/api v0.34.6 + k8s.io/apiextensions-apiserver => k8s.io/apiextensions-apiserver v0.34.6 + k8s.io/apimachinery => k8s.io/apimachinery v0.34.6 + k8s.io/apiserver => k8s.io/apiserver v0.34.6 + k8s.io/cli-runtime => k8s.io/cli-runtime v0.34.6 + k8s.io/client-go => k8s.io/client-go v0.34.6 + k8s.io/cloud-provider => k8s.io/cloud-provider v0.34.6 + k8s.io/cluster-bootstrap => k8s.io/cluster-bootstrap v0.34.6 + k8s.io/code-generator => k8s.io/code-generator v0.34.6 + k8s.io/component-base => k8s.io/component-base v0.34.6 + k8s.io/component-helpers => k8s.io/component-helpers v0.34.6 + k8s.io/controller-manager => k8s.io/controller-manager v0.34.6 + k8s.io/cri-api => k8s.io/cri-api v0.34.6 + k8s.io/cri-client => k8s.io/cri-client v0.34.6 + k8s.io/csi-translation-lib => k8s.io/csi-translation-lib v0.34.6 + k8s.io/dynamic-resource-allocation => k8s.io/dynamic-resource-allocation v0.34.6 + k8s.io/endpointslice => k8s.io/endpointslice v0.34.6 + k8s.io/externaljwt => k8s.io/externaljwt v0.34.6 + k8s.io/kms => k8s.io/kms v0.34.6 + k8s.io/kube-aggregator => k8s.io/kube-aggregator v0.34.6 + k8s.io/kube-controller-manager => k8s.io/kube-controller-manager v0.34.6 + k8s.io/kube-proxy => k8s.io/kube-proxy v0.34.6 + k8s.io/kube-scheduler => k8s.io/kube-scheduler v0.34.6 + k8s.io/kubectl => k8s.io/kubectl v0.34.6 + k8s.io/kubelet => k8s.io/kubelet v0.34.6 + k8s.io/metrics => k8s.io/metrics v0.34.6 + k8s.io/mount-utils => k8s.io/mount-utils v0.34.6 + k8s.io/pod-security-admission => k8s.io/pod-security-admission v0.34.6 + k8s.io/sample-apiserver => k8s.io/sample-apiserver v0.34.6 ) require ( @@ -60,6 +89,7 @@ require ( k8s.io/klog/v2 v2.130.1 k8s.io/kube-openapi v0.0.0-20250710124328-f3f2b991d03b k8s.io/kubectl v0.34.6 + k8s.io/kubernetes v1.34.6 k8s.io/utils v0.0.0-20250604170112-4c0f3b243397 oras.land/oras-go/v2 v2.6.0 sigs.k8s.io/cli-utils v0.37.2 @@ -94,6 +124,7 @@ require ( github.com/containerd/stargz-snapshotter/estargz v0.16.3 // indirect github.com/cpuguy83/go-md2man/v2 v2.0.7 // indirect github.com/cyphar/filepath-securejoin v0.4.1 // indirect + github.com/distribution/reference v0.6.0 // indirect github.com/docker/cli v29.2.0+incompatible // indirect github.com/docker/distribution v2.8.3+incompatible // indirect github.com/docker/docker-credential-helpers v0.9.3 // indirect @@ -177,6 +208,8 @@ require ( github.com/x448/float16 v0.8.4 // indirect github.com/xanzy/ssh-agent v0.3.3 // indirect github.com/xlab/treeprint v1.2.0 // indirect + go.opentelemetry.io/otel v1.39.0 // indirect + go.opentelemetry.io/otel/trace v1.39.0 // indirect go.uber.org/automaxprocs v1.6.0 // indirect go.uber.org/multierr v1.11.0 // indirect go.yaml.in/yaml/v2 v2.4.3 // indirect @@ -202,6 +235,8 @@ require ( k8s.io/apiserver v0.34.6 // indirect k8s.io/code-generator v0.34.6 // indirect k8s.io/component-base v0.34.6 // indirect + k8s.io/component-helpers v0.34.6 // indirect + k8s.io/controller-manager v0.34.6 // indirect k8s.io/gengo v0.0.0-20250130153323-76c5745d3511 // indirect k8s.io/gengo/v2 v2.0.0-20250604051438-85fd79dbfd9f // indirect k8s.io/helm v2.17.0+incompatible // indirect diff --git a/go.sum b/go.sum index 14022c76ae..b5b0f51cff 100644 --- a/go.sum +++ b/go.sum @@ -606,6 +606,10 @@ k8s.io/code-generator v0.34.6 h1:Sff8VcHxpVj/1tvYE7CgL1X7/hHjH2Nnu2//BNVUAaY= k8s.io/code-generator v0.34.6/go.mod h1:21o4G2tzuXrgpwntJeWA5Nt+H9ADnXvoQ2fwKlfGkfE= k8s.io/component-base v0.34.6 h1:+hkR1hJI2Bsfp5SbTGmGcKdkGZoa0aX68eMFGeZPUIg= k8s.io/component-base v0.34.6/go.mod h1:D2fBgLp3Rwe4+P0JTLhRPDJWrWddwTu/aoBT0FMQ75M= +k8s.io/component-helpers v0.34.6 h1:fLYgk0Jpgr7ykcQUL8bomqKtqMCV8lnfqV3iyhjHdsQ= +k8s.io/component-helpers v0.34.6/go.mod h1:eAQxT3xRPZ56H4gLILpuXWKwC92FWhYMOsEtWLQgK9o= +k8s.io/controller-manager v0.34.6 h1:e+qcnaGyOThkEMPL+GJ8IiPRDuSPDLgc9FbpvShcOl0= +k8s.io/controller-manager v0.34.6/go.mod h1:PTc8yq8l80jp7dGFh8spdJVKAzNCcS13CceylpG7bMs= k8s.io/gengo v0.0.0-20250130153323-76c5745d3511 h1:4eL6zr5VCj71nu2nOuQ6j6m/kqh5WueXBN8daZkNe90= k8s.io/gengo v0.0.0-20250130153323-76c5745d3511/go.mod h1:FiNAH4ZV3gBg2Kwh89tzAEV2be7d5xI0vBa/VySYy3E= k8s.io/gengo/v2 v2.0.0-20250604051438-85fd79dbfd9f h1:SLb+kxmzfA87x4E4brQzB33VBbT2+x7Zq9ROIHmGn9Q= @@ -619,6 +623,8 @@ k8s.io/kube-openapi v0.0.0-20250710124328-f3f2b991d03b h1:MloQ9/bdJyIu9lb1PzujOP k8s.io/kube-openapi v0.0.0-20250710124328-f3f2b991d03b/go.mod h1:UZ2yyWbFTpuhSbFhv24aGNOdoRdJZgsIObGBUaYVsts= k8s.io/kubectl v0.34.6 h1:IxLtdBssejoWWeG7zCDiDc2E+aKG6S69ue/kCOHZi6I= k8s.io/kubectl v0.34.6/go.mod h1:Vw5UnMi+lgzXv5sfBycy/BdkPP451DmvarxnMJaMJQA= +k8s.io/kubernetes v1.34.6 h1:zghBAjObluxBc5bkVXXvTA+fn9dkYSIq1QvzTAOWvsw= +k8s.io/kubernetes v1.34.6/go.mod h1:m6pZk6a179pRo2wsTiCPORJ86iOEQmfIzUvtyEF8BwA= k8s.io/utils v0.0.0-20250604170112-4c0f3b243397 h1:hwvWFiBzdWw1FhfY1FooPn3kzWuJ8tmbZBHi4zVsl1Y= k8s.io/utils v0.0.0-20250604170112-4c0f3b243397/go.mod h1:OLgZIPagt7ERELqWJFomSt595RzquPNLL48iOWgYOg0= oras.land/oras-go/v2 v2.6.0 h1:X4ELRsiGkrbeox69+9tzTu492FMUu7zJQW6eJU+I2oc= diff --git a/integrationtests/go.mod b/integrationtests/go.mod index 9a8795b1d4..dc3aec7d2a 100644 --- a/integrationtests/go.mod +++ b/integrationtests/go.mod @@ -7,6 +7,35 @@ toolchain go1.25.9 replace ( github.com/rancher/fleet => ../ github.com/rancher/fleet/pkg/apis => ../pkg/apis + k8s.io/api => k8s.io/api v0.34.6 + k8s.io/apiextensions-apiserver => k8s.io/apiextensions-apiserver v0.34.6 + k8s.io/apimachinery => k8s.io/apimachinery v0.34.6 + k8s.io/apiserver => k8s.io/apiserver v0.34.6 + k8s.io/cli-runtime => k8s.io/cli-runtime v0.34.6 + k8s.io/client-go => k8s.io/client-go v0.34.6 + k8s.io/cloud-provider => k8s.io/cloud-provider v0.34.6 + k8s.io/cluster-bootstrap => k8s.io/cluster-bootstrap v0.34.6 + k8s.io/code-generator => k8s.io/code-generator v0.34.6 + k8s.io/component-base => k8s.io/component-base v0.34.6 + k8s.io/component-helpers => k8s.io/component-helpers v0.34.6 + k8s.io/controller-manager => k8s.io/controller-manager v0.34.6 + k8s.io/cri-api => k8s.io/cri-api v0.34.6 + k8s.io/cri-client => k8s.io/cri-client v0.34.6 + k8s.io/csi-translation-lib => k8s.io/csi-translation-lib v0.34.6 + k8s.io/dynamic-resource-allocation => k8s.io/dynamic-resource-allocation v0.34.6 + k8s.io/endpointslice => k8s.io/endpointslice v0.34.6 + k8s.io/externaljwt => k8s.io/externaljwt v0.34.6 + k8s.io/kms => k8s.io/kms v0.34.6 + k8s.io/kube-aggregator => k8s.io/kube-aggregator v0.34.6 + k8s.io/kube-controller-manager => k8s.io/kube-controller-manager v0.34.6 + k8s.io/kube-proxy => k8s.io/kube-proxy v0.34.6 + k8s.io/kube-scheduler => k8s.io/kube-scheduler v0.34.6 + k8s.io/kubectl => k8s.io/kubectl v0.34.6 + k8s.io/kubelet => k8s.io/kubelet v0.34.6 + k8s.io/metrics => k8s.io/metrics v0.34.6 + k8s.io/mount-utils => k8s.io/mount-utils v0.34.6 + k8s.io/pod-security-admission => k8s.io/pod-security-admission v0.34.6 + k8s.io/sample-apiserver => k8s.io/sample-apiserver v0.34.6 ) require ( @@ -217,8 +246,11 @@ require ( k8s.io/apiserver v0.34.6 // indirect k8s.io/cli-runtime v0.34.6 // indirect k8s.io/component-base v0.34.6 // indirect + k8s.io/component-helpers v0.34.6 // indirect + k8s.io/controller-manager v0.34.6 // indirect k8s.io/klog/v2 v2.130.1 // indirect k8s.io/kube-openapi v0.0.0-20250710124328-f3f2b991d03b // indirect + k8s.io/kubernetes v1.34.6 // indirect k8s.io/utils v0.0.0-20250604170112-4c0f3b243397 // indirect oras.land/oras-go/v2 v2.6.0 // indirect sigs.k8s.io/cli-utils v0.37.2 // indirect diff --git a/integrationtests/go.sum b/integrationtests/go.sum index e9759512f3..01321a9c4b 100644 --- a/integrationtests/go.sum +++ b/integrationtests/go.sum @@ -616,12 +616,18 @@ k8s.io/client-go v0.34.6 h1:8aF4tJiZolSdliT5nhJnBx49Om2ET3Tn3/JKKpJk4gI= k8s.io/client-go v0.34.6/go.mod h1:ZntANq4HsaiOD0rIhLHTdZT/aLkv4NVyI/glqocESTQ= k8s.io/component-base v0.34.6 h1:+hkR1hJI2Bsfp5SbTGmGcKdkGZoa0aX68eMFGeZPUIg= k8s.io/component-base v0.34.6/go.mod h1:D2fBgLp3Rwe4+P0JTLhRPDJWrWddwTu/aoBT0FMQ75M= +k8s.io/component-helpers v0.34.6 h1:fLYgk0Jpgr7ykcQUL8bomqKtqMCV8lnfqV3iyhjHdsQ= +k8s.io/component-helpers v0.34.6/go.mod h1:eAQxT3xRPZ56H4gLILpuXWKwC92FWhYMOsEtWLQgK9o= +k8s.io/controller-manager v0.34.6 h1:e+qcnaGyOThkEMPL+GJ8IiPRDuSPDLgc9FbpvShcOl0= +k8s.io/controller-manager v0.34.6/go.mod h1:PTc8yq8l80jp7dGFh8spdJVKAzNCcS13CceylpG7bMs= k8s.io/klog/v2 v2.130.1 h1:n9Xl7H1Xvksem4KFG4PYbdQCQxqc/tTUyrgXaOhHSzk= k8s.io/klog/v2 v2.130.1/go.mod h1:3Jpz1GvMt720eyJH1ckRHK1EDfpxISzJ7I9OYgaDtPE= k8s.io/kube-openapi v0.0.0-20250710124328-f3f2b991d03b h1:MloQ9/bdJyIu9lb1PzujOPolHyvO06MXG5TUIj2mNAA= k8s.io/kube-openapi v0.0.0-20250710124328-f3f2b991d03b/go.mod h1:UZ2yyWbFTpuhSbFhv24aGNOdoRdJZgsIObGBUaYVsts= k8s.io/kubectl v0.34.6 h1:IxLtdBssejoWWeG7zCDiDc2E+aKG6S69ue/kCOHZi6I= k8s.io/kubectl v0.34.6/go.mod h1:Vw5UnMi+lgzXv5sfBycy/BdkPP451DmvarxnMJaMJQA= +k8s.io/kubernetes v1.34.6 h1:zghBAjObluxBc5bkVXXvTA+fn9dkYSIq1QvzTAOWvsw= +k8s.io/kubernetes v1.34.6/go.mod h1:m6pZk6a179pRo2wsTiCPORJ86iOEQmfIzUvtyEF8BwA= k8s.io/utils v0.0.0-20250604170112-4c0f3b243397 h1:hwvWFiBzdWw1FhfY1FooPn3kzWuJ8tmbZBHi4zVsl1Y= k8s.io/utils v0.0.0-20250604170112-4c0f3b243397/go.mod h1:OLgZIPagt7ERELqWJFomSt595RzquPNLL48iOWgYOg0= oras.land/oras-go/v2 v2.6.0 h1:X4ELRsiGkrbeox69+9tzTu492FMUu7zJQW6eJU+I2oc= diff --git a/internal/cmd/agent/deployer/desiredset/diff.go b/internal/cmd/agent/deployer/desiredset/diff.go index 628241ccf7..1def4b7b4c 100644 --- a/internal/cmd/agent/deployer/desiredset/diff.go +++ b/internal/cmd/agent/deployer/desiredset/diff.go @@ -1,11 +1,8 @@ package desiredset import ( - "bytes" "encoding/json" - "fmt" "slices" - "strings" jsonpatch "github.com/evanphx/json-patch" @@ -106,20 +103,6 @@ func Diff(plan Plan, bd *fleet.BundleDeployment, ns string, objs ...runtime.Obje errs = append(errs, err) continue } - - // Some normalization operations, unlike those called from Diff (vendored ArgoCD code), must only be applied - // to actual, in-cluster objects, not to desired ones. - emptied, err := normalizeActual(live, actualObj.(*unstructured.Unstructured), key, &patch) - if err != nil { - errs = append(errs, err) - continue - } - - if emptied { - delete(plan.Update[gvk], key) - continue - } - // this will overwrite an existing entry in the Update map plan.Update.Set(gvk, key.Namespace, key.Name, string(patch)) } @@ -190,280 +173,3 @@ func newNormalizers(live objectset.ObjectByGVK, bd *fleet.BundleDeployment) (dif return normalizers.New(live, ignoreNormalizer, knownTypesNorm, jsonPatchNorm), nil } - -// normalizeActual encapsulates patch normalization operations which are only run against a live object (uActual), -// possibly requiring knowledge of other live resources. -func normalizeActual( - live objectset.ObjectByGVK, - uActual *unstructured.Unstructured, - key objectset.ObjectKey, - patch *[]byte, -) (bool, error) { - emptied, err := normalizeReplicasPatch(live, uActual, key, patch) - if err != nil || emptied { - return emptied, err - } - - return normalizeNullPatch(key, patch) -} - -// normalizeReplicasPatch handles removal of a diff patch's `.spec.replicas` field on a Deployment or a StatefulSet. -// Processing involves checking whether live objects include HPAs referencing the object. -// Both v1 and v2 of the autoscaling API are supported. -// This can be called safely against any unstructured object: if the object turns out not to represent a Deployment nor a -// StatefulSet, this function is a no-op. -// Returns a boolean indicating whether the resulting patch is empty, and any error which may have happened in the -// process. -func normalizeReplicasPatch( - live objectset.ObjectByGVK, - uActual *unstructured.Unstructured, - key objectset.ObjectKey, - patch *[]byte, -) (bool, error) { - actualGVK := uActual.GroupVersionKind() - if actualGVK.Group != "apps" { - return false, nil - } - - if actualGVK.Kind != "Deployment" && actualGVK.Kind != "StatefulSet" { - return false, nil - } - - var patchData map[string]any - if err := json.Unmarshal(*patch, &patchData); err != nil { - return false, fmt.Errorf("failed to unmarshal patch for %s/%s: %v: %w", key.Namespace, key.Name, *patch, err) - } - - patchSpec, patchHasSpec := patchData["spec"] - if !patchHasSpec { - // No need to even check HPAs for replicas - return false, nil - } - - // What differs between v1 and v2 is the set of supported metrics for scaling (with memory and custom metrics - // included in v2); this is irrelevant to the logic at play here: we are only interested in values of replica - // counts, not in what triggers their updates. - supportedVersions := []string{"v2", "v1"} - - failFieldNotFound := func(k objectset.ObjectKey, fieldName string) error { - return fmt.Errorf("malformed HPA %s/%s: field %q not found", k.Namespace, k.Name, fieldName) - } - - // a non-nil error would mean that the field has an unexpected type; this cannot happen as per the Deployment - // and StatefulSet APIs. - actualReplicas, found, _ := unstructured.NestedInt64(uActual.Object, "spec", "replicas") - if !found { - return false, failFieldNotFound(key, ".spec.replicas") - } - - for _, v := range supportedVersions { - gvk := schema.GroupVersionKind{ - Group: "autoscaling", - Version: v, - Kind: "HorizontalPodAutoscaler", - } - - for k, o := range live[gvk] { - if k.Namespace != key.Namespace { - continue - } - - un, ok := o.(*unstructured.Unstructured) - if !ok { - continue - } - - // in each case of extraction of HPA fields below, a non-nil error would mean that the field has - // an unexpected type; this cannot happen as per the HPA API. - minRepField, found, _ := unstructured.NestedInt64(un.Object, "spec", "minReplicas") - if !found { - minRepField = 1 - } - - maxRepField, found, _ := unstructured.NestedInt64(un.Object, "spec", "maxReplicas") - if !found { - return false, failFieldNotFound(k, ".spec.maxReplicas") - } - - refObjField, found, _ := unstructured.NestedMap(un.Object, "spec", "scaleTargetRef") - if !found { - return false, failFieldNotFound(k, ".spec.scaleTargetRef") - } - - // apiVersion can be empty - refVersion, _, _ := unstructured.NestedString(refObjField, "apiVersion") - - refKind, found, _ := unstructured.NestedString(refObjField, "kind") - if !found { - return false, failFieldNotFound(k, ".spec.scaleTargetRef.kind") - } - - refName, found, _ := unstructured.NestedString(refObjField, "name") - if !found { - return false, failFieldNotFound(k, ".spec.scaleTargetRef.name") - } - - if refVersion != "" { - groupVersion := strings.Split(refVersion, "/") - var group, version string - switch len(groupVersion) { - case 1: - version = groupVersion[0] - case 2: - group = groupVersion[0] - version = groupVersion[1] - default: - continue - } - - if actualGVK.Version != version || actualGVK.Group != group { - continue - } - } - - if actualGVK.Kind != refKind { - continue - } - - if key.Name != refName { - continue - } - - if actualReplicas < minRepField || actualReplicas > maxRepField { - return false, nil - } - - // No need to check if the field actually exists, as we've been through that above. - spec, ok := patchSpec.(map[string]any) - if !ok { - return false, fmt.Errorf("malformed spec for %s %s/%s", refKind, k.Namespace, k.Name) - } - delete(spec, "replicas") - - if len(patchData) == 1 /* spec only */ && len(spec) == 0 { - // no more fields in the diff - return true, nil - } - - p, err := json.Marshal(patchData) - if err != nil { - return false, fmt.Errorf("failed to marshal patch after removing replicas field: %w", err) - } - - *patch = p - - return false, nil - } - } - - return false, nil -} - -// normalizeNullPatch removes null-valued fields from merge patches generated by -// diffing desired against live resources. -// -// Context: In merge patches, null means deleting a field. Helm v4 can emit explicit -// null values for omitted fields (e.g., spec.strategy.rollingUpdate: null) where -// Helm v3 omitted those fields entirely. When Kubernetes applies defaulting to live -// objects, this creates false drift: the patch wants to delete a field that was -// server-side defaulted. -// -// Solution: Strip all null-valued fields from patches. This preserves meaningful drift -// detection while ignoring defaulting-only differences, restoring Helm v3-like behavior. -// -// Returns true if the patch becomes empty after removing nulls (no real drift). -func normalizeNullPatch( - key objectset.ObjectKey, - patch *[]byte, -) (bool, error) { - // Fast-path: skip expensive processing if patch doesn't contain "null" - // This avoids unmarshal/walk/marshal overhead for patches with no nulls - if !bytes.Contains(*patch, []byte("null")) { - return false, nil - } - - var patchData map[string]any - if err := json.Unmarshal(*patch, &patchData); err != nil { - // Format patch as string and truncate if too large for logging - patchStr := string(*patch) - const maxPatchLogLen = 1024 - if len(patchStr) > maxPatchLogLen { - patchStr = patchStr[:maxPatchLogLen] + "...(truncated)" - } - return false, fmt.Errorf("failed to unmarshal patch for %s/%s: %q: %w", key.Namespace, key.Name, patchStr, err) - } - - // Recursively remove all null values from the patch tree - cleaned, _ := removeNullPatchFields(patchData).(map[string]any) - if len(cleaned) == 0 { - // Patch contained only nulls - no meaningful drift - return true, nil - } - - p, err := json.Marshal(cleaned) - if err != nil { - return false, fmt.Errorf("failed to marshal patch after removing null fields: %w", err) - } - - *patch = p - return false, nil -} - -// removeNullPatchFields recursively removes null values from patch data structures. -// It handles both maps (objects) and slices (arrays), preserving non-null values. -// -// Map behavior: -// - Skips entries with nil values -// - Skips entries that become empty maps after cleaning -// -// Slice behavior: -// - Skips nil elements -// - Skips elements that become empty maps after cleaning -// - Preserves non-map elements (strings, numbers, etc.) -func removeNullPatchFields(value any) any { - switch v := value.(type) { - case map[string]any: - result := make(map[string]any, len(v)) - for key, item := range v { - // Skip null values in maps - if item == nil { - continue - } - - // Recursively clean nested structures - cleaned := removeNullPatchFields(item) - - // Skip maps that became empty after cleaning (contained only nulls) - if cleanedMap, ok := cleaned.(map[string]any); ok && len(cleanedMap) == 0 { - continue - } - - result[key] = cleaned - } - return result - - case []any: - result := make([]any, 0, len(v)) - for i := range v { - // Skip null elements in arrays - if v[i] == nil { - continue - } - - // Recursively clean nested structures - cleaned := removeNullPatchFields(v[i]) - - // Skip maps that became empty after cleaning - if cleanedMap, ok := cleaned.(map[string]any); ok && len(cleanedMap) == 0 { - continue - } - - result = append(result, cleaned) - } - return result - - default: - // Leaf values (strings, numbers, bools) pass through unchanged - return v - } -} diff --git a/internal/cmd/agent/deployer/desiredset/diff_null_test.go b/internal/cmd/agent/deployer/desiredset/diff_null_test.go deleted file mode 100644 index 3045f8a0a6..0000000000 --- a/internal/cmd/agent/deployer/desiredset/diff_null_test.go +++ /dev/null @@ -1,191 +0,0 @@ -package desiredset - -import ( - "encoding/json" - "testing" - - "github.com/rancher/fleet/internal/cmd/agent/deployer/objectset" -) - -// Test_Diff_NullPatch validates normalizeNullPatch behavior across various -// scenarios including nested nulls, arrays with nulls, and edge cases. -func Test_Diff_NullPatch(t *testing.T) { - key := objectset.ObjectKey{Name: "test", Namespace: "ns"} - tests := []struct { - name string - patch string - expectPatch string - expectEmpty bool - expectErr bool - }{ - { - name: "keeps_patch_without_nulls", - patch: `{"metadata":{"labels":{"a":"b"}}}`, - expectPatch: `{"metadata":{"labels":{"a":"b"}}}`, - }, - { - name: "fast_path_no_nulls_complex_patch", - patch: `{"spec":{"replicas":3,"strategy":{"type":"RollingUpdate"},"template":{"metadata":{"labels":{"app":"test"}},"spec":{"containers":[{"name":"nginx","image":"nginx:1.14.2"}]}}}}`, - expectPatch: `{"spec":{"replicas":3,"strategy":{"type":"RollingUpdate"},"template":{"metadata":{"labels":{"app":"test"}},"spec":{"containers":[{"name":"nginx","image":"nginx:1.14.2"}]}}}}`, - }, - { - name: "removes_null_field", - patch: `{"spec":{"strategy":{"rollingUpdate":null,"type":"RollingUpdate"}}}`, - expectPatch: `{"spec":{"strategy":{"type":"RollingUpdate"}}}`, - }, - { - name: "removes_nested_null_fields", - patch: `{"spec":{"template":{"spec":{"securityContext":null,"containers":[{"name":"c1","image":"nginx"}]}}}}`, - expectPatch: `{"spec":{"template":{"spec":{"containers":[{"name":"c1","image":"nginx"}]}}}}`, - }, - { - name: "removes_null_elements_from_arrays", - patch: `{"spec":{"tolerations":[{"key":"a","operator":null},null,{"key":"b"}]}}`, - expectPatch: `{"spec":{"tolerations":[{"key":"a"},{"key":"b"}]}}`, - }, - { - name: "removes_multiple_nulls_across_tree", - patch: `{"spec":{"foo":null,"bar":{"baz":null,"keep":"x"}},"metadata":{"annotations":null}}`, - expectPatch: `{"spec":{"bar":{"keep":"x"}}}`, - }, - { - name: "empties_patch_when_only_nulls", - patch: `{"spec":{"strategy":{"rollingUpdate":null}}}`, - expectEmpty: true, - }, - { - name: "fails_on_malformed_json_with_null", - patch: `{"spec":null,"bad":`, - expectErr: true, - }, - } - - for _, tc := range tests { - t.Run(tc.name, func(t *testing.T) { - patch := []byte(tc.patch) - emptied, err := normalizeNullPatch(key, &patch) - - if tc.expectErr { - if err == nil { - t.Fatal("expected error, got nil") - } - return - } - - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - - if emptied != tc.expectEmpty { - t.Fatalf("emptied = %v, want %v", emptied, tc.expectEmpty) - } - - if tc.expectEmpty { - return - } - - assertPatchJSONEqual(t, string(patch), tc.expectPatch) - }) - } -} - -// Test_Diff_RemoveNullPatchFields validates the recursive null removal logic -// with a complex nested structure containing maps, arrays, and null values. -func Test_Diff_RemoveNullPatchFields(t *testing.T) { - input := map[string]any{ - "spec": map[string]any{ - "list": []any{ - map[string]any{"name": "a", "value": nil}, - nil, - "text", - }, - "empty": map[string]any{"foo": nil}, - }, - "metadata": map[string]any{"labels": map[string]any{"x": "y"}}, - } - - cleanedAny := removeNullPatchFields(input) - cleaned, ok := cleanedAny.(map[string]any) - if !ok { - t.Fatalf("cleaned type = %T, want map[string]any", cleanedAny) - } - - expected := map[string]any{ - "spec": map[string]any{ - "list": []any{ - map[string]any{"name": "a"}, - "text", - }, - }, - "metadata": map[string]any{"labels": map[string]any{"x": "y"}}, - } - - gotJSON, err := json.Marshal(cleaned) - if err != nil { - t.Fatalf("failed to marshal cleaned: %v", err) - } - wantJSON, err := json.Marshal(expected) - if err != nil { - t.Fatalf("failed to marshal expected: %v", err) - } - - if string(gotJSON) != string(wantJSON) { - t.Fatalf("cleaned mismatch\ngot: %s\nwant: %s", gotJSON, wantJSON) - } -} - -// assertPatchJSONEqual compares two JSON strings for semantic equality, -// normalizing formatting differences through unmarshal/marshal cycles. -func assertPatchJSONEqual(t *testing.T, got, want string) { - t.Helper() - - var gotObj map[string]any - if err := json.Unmarshal([]byte(got), &gotObj); err != nil { - t.Fatalf("failed to unmarshal got json: %v", err) - } - - var wantObj map[string]any - if err := json.Unmarshal([]byte(want), &wantObj); err != nil { - t.Fatalf("failed to unmarshal want json: %v", err) - } - - gotNorm, err := json.Marshal(gotObj) - if err != nil { - t.Fatalf("failed to marshal got object: %v", err) - } - wantNorm, err := json.Marshal(wantObj) - if err != nil { - t.Fatalf("failed to marshal want object: %v", err) - } - - if string(gotNorm) != string(wantNorm) { - t.Fatalf("json mismatch\ngot: %s\nwant: %s", gotNorm, wantNorm) - } -} - -// Benchmark_Diff_NullPatch_WithNulls benchmarks normalizeNullPatch with a patch containing nulls. -func Benchmark_Diff_NullPatch_WithNulls(b *testing.B) { - key := objectset.ObjectKey{Name: "test", Namespace: "ns"} - patch := []byte(`{"spec":{"strategy":{"rollingUpdate":null,"type":"RollingUpdate"},"template":{"spec":{"securityContext":null,"containers":[{"name":"nginx","resources":null}]}}}}`) - - b.ResetTimer() - for i := 0; i < b.N; i++ { - patchCopy := make([]byte, len(patch)) - copy(patchCopy, patch) - _, _ = normalizeNullPatch(key, &patchCopy) - } -} - -// Benchmark_Diff_NullPatch_WithoutNulls benchmarks normalizeNullPatch with a patch containing no nulls. -// This should be significantly faster due to the fast-path optimization. -func Benchmark_Diff_NullPatch_WithoutNulls(b *testing.B) { - key := objectset.ObjectKey{Name: "test", Namespace: "ns"} - patch := []byte(`{"spec":{"replicas":3,"strategy":{"type":"RollingUpdate"},"template":{"metadata":{"labels":{"app":"test"}},"spec":{"containers":[{"name":"nginx","image":"nginx:1.14.2"}]}}}}`) - - b.ResetTimer() - for i := 0; i < b.N; i++ { - patchCopy := make([]byte, len(patch)) - copy(patchCopy, patch) - _, _ = normalizeNullPatch(key, &patchCopy) - } -} diff --git a/internal/cmd/agent/deployer/internal/diff/scheme/scheme.go b/internal/cmd/agent/deployer/internal/diff/scheme/scheme.go index a2bc212614..91bd2a9211 100644 --- a/internal/cmd/agent/deployer/internal/diff/scheme/scheme.go +++ b/internal/cmd/agent/deployer/internal/diff/scheme/scheme.go @@ -1,9 +1,30 @@ // +vendored https://github.com/argoproj/gitops-engine/blob/master/pkg/utils/kube/scheme/scheme.go -// Modified to use k8s.io/client-go/kubernetes/scheme instead of k8s.io/kubernetes due to compilation issues in k8s.io/kubernetes. package scheme import ( - kubescheme "k8s.io/client-go/kubernetes/scheme" + "k8s.io/kubernetes/pkg/api/legacyscheme" + + _ "k8s.io/kubernetes/pkg/apis/admission/install" + _ "k8s.io/kubernetes/pkg/apis/admissionregistration/install" + _ "k8s.io/kubernetes/pkg/apis/apps/install" + _ "k8s.io/kubernetes/pkg/apis/authentication/install" + _ "k8s.io/kubernetes/pkg/apis/authorization/install" + _ "k8s.io/kubernetes/pkg/apis/autoscaling/install" + _ "k8s.io/kubernetes/pkg/apis/batch/install" + _ "k8s.io/kubernetes/pkg/apis/certificates/install" + _ "k8s.io/kubernetes/pkg/apis/coordination/install" + _ "k8s.io/kubernetes/pkg/apis/core/install" + _ "k8s.io/kubernetes/pkg/apis/discovery/install" + _ "k8s.io/kubernetes/pkg/apis/events/install" + _ "k8s.io/kubernetes/pkg/apis/extensions/install" + _ "k8s.io/kubernetes/pkg/apis/flowcontrol/install" + _ "k8s.io/kubernetes/pkg/apis/imagepolicy/install" + _ "k8s.io/kubernetes/pkg/apis/networking/install" + _ "k8s.io/kubernetes/pkg/apis/node/install" + _ "k8s.io/kubernetes/pkg/apis/policy/install" + _ "k8s.io/kubernetes/pkg/apis/rbac/install" + _ "k8s.io/kubernetes/pkg/apis/scheduling/install" + _ "k8s.io/kubernetes/pkg/apis/storage/install" ) -var Scheme = kubescheme.Scheme +var Scheme = legacyscheme.Scheme