From 8b49a8443048b4c84600f192c590214162c61cac Mon Sep 17 00:00:00 2001 From: Shinnosuke Sawada-Dazai Date: Wed, 30 Oct 2024 12:59:13 +0900 Subject: [PATCH] Implement k8s manifest diff (#5298) Signed-off-by: Shinnosuke Sawada-Dazai --- .../plugin/kubernetes/provider/diff.go | 84 ++++++ .../plugin/kubernetes/provider/diff_test.go | 239 ++++++++++++++++++ .../plugin/kubernetes/provider/diffutil.go | 120 +++++++++ .../kubernetes/provider/diffutil_test.go | 218 ++++++++++++++++ .../plugin/kubernetes/provider/resource.go | 65 ++++- 5 files changed, 721 insertions(+), 5 deletions(-) create mode 100644 pkg/app/pipedv1/plugin/kubernetes/provider/diff.go create mode 100644 pkg/app/pipedv1/plugin/kubernetes/provider/diff_test.go create mode 100644 pkg/app/pipedv1/plugin/kubernetes/provider/diffutil.go create mode 100644 pkg/app/pipedv1/plugin/kubernetes/provider/diffutil_test.go diff --git a/pkg/app/pipedv1/plugin/kubernetes/provider/diff.go b/pkg/app/pipedv1/plugin/kubernetes/provider/diff.go new file mode 100644 index 0000000000..3f83ffb0e3 --- /dev/null +++ b/pkg/app/pipedv1/plugin/kubernetes/provider/diff.go @@ -0,0 +1,84 @@ +// Copyright 2024 The PipeCD Authors. +// +// 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 provider + +import ( + "go.uber.org/zap" + v1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" + + "github.com/pipe-cd/pipecd/pkg/plugin/diff" +) + + +func Diff(old, new Manifest, logger *zap.Logger, opts ...diff.Option) (*diff.Result, error) { + if old.Key.IsSecret() && new.Key.IsSecret() { + var err error + old.Body, err = normalizeNewSecret(old.Body, new.Body) + if err != nil { + return nil, err + } + } + + key := old.Key.String() + + normalizedOld, err := remarshal(old.Body) + if err != nil { + logger.Info("compare manifests directly since it was unable to remarshal old Kubernetes manifest to normalize special fields", zap.Error(err)) + return diff.DiffUnstructureds(*old.Body, *new.Body, key, opts...) + } + + normalizedNew, err := remarshal(new.Body) + if err != nil { + logger.Info("compare manifests directly since it was unable to remarshal new Kubernetes manifest to normalize special fields", zap.Error(err)) + return diff.DiffUnstructureds(*old.Body, *new.Body, key, opts...) + } + + return diff.DiffUnstructureds(*normalizedOld, *normalizedNew, key, opts...) +} + +func normalizeNewSecret(old, new *unstructured.Unstructured) (*unstructured.Unstructured, error) { + var o, n v1.Secret + runtime.DefaultUnstructuredConverter.FromUnstructured(old.Object, &o) + runtime.DefaultUnstructuredConverter.FromUnstructured(new.Object, &n) + + // Move as much as possible fields from `o.Data` to `o.StringData` to make `o` close to `n` to minimize the diff. + for k, v := range o.Data { + // Skip if the field also exists in StringData. + if _, ok := o.StringData[k]; ok { + continue + } + + if _, ok := n.StringData[k]; !ok { + continue + } + + if o.StringData == nil { + o.StringData = make(map[string]string) + } + + // If the field is existing in `n.StringData`, we should move that field from `o.Data` to `o.StringData` + o.StringData[k] = string(v) + delete(o.Data, k) + } + + newO, err := runtime.DefaultUnstructuredConverter.ToUnstructured(&o) + if err != nil { + return nil, err + } + + return &unstructured.Unstructured{Object: newO}, nil +} diff --git a/pkg/app/pipedv1/plugin/kubernetes/provider/diff_test.go b/pkg/app/pipedv1/plugin/kubernetes/provider/diff_test.go new file mode 100644 index 0000000000..fd25a9070d --- /dev/null +++ b/pkg/app/pipedv1/plugin/kubernetes/provider/diff_test.go @@ -0,0 +1,239 @@ +// Copyright 2024 The PipeCD Authors. +// +// 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 provider + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.uber.org/zap" + + "github.com/pipe-cd/pipecd/pkg/plugin/diff" +) + +func TestDiff(t *testing.T) { + t.Parallel() + + testcases := []struct { + name string + manifests string + expected string + diffNum int + falsePositive bool + }{ + { + name: "Secret no diff 1", + manifests: `apiVersion: apps/v1 +kind: Secret +metadata: + name: secret-management +--- +apiVersion: apps/v1 +kind: Secret +metadata: + name: secret-management +`, + expected: "", + diffNum: 0, + }, + { + name: "Secret no diff 2", + manifests: `apiVersion: apps/v1 +kind: Secret +metadata: + name: secret-management +data: + password: hoge +stringData: + foo: bar +--- +apiVersion: apps/v1 +kind: Secret +metadata: + name: secret-management +data: + password: hoge +stringData: + foo: bar +`, + expected: "", + diffNum: 0, + }, + { + name: "Secret no diff with merge", + manifests: `apiVersion: apps/v1 +kind: Secret +metadata: + name: secret-management +data: + password: hoge + foo: YmFy +--- +apiVersion: apps/v1 +kind: Secret +metadata: + name: secret-management +data: + password: hoge +stringData: + foo: bar +`, + expected: "", + diffNum: 0, + }, + { + name: "Secret no diff override false-positive", + manifests: `apiVersion: apps/v1 +kind: Secret +metadata: + name: secret-management +data: + password: hoge + foo: YmFy +--- +apiVersion: apps/v1 +kind: Secret +metadata: + name: secret-management +data: + password: hoge + foo: Zm9v +stringData: + foo: bar +`, + expected: "", + diffNum: 0, + falsePositive: true, + }, + { + name: "Secret has diff", + manifests: `apiVersion: apps/v1 +kind: Secret +metadata: + name: secret-management +data: + foo: YmFy +--- +apiVersion: apps/v1 +kind: Secret +metadata: + name: secret-management +data: + password: hoge +stringData: + foo: bar +`, + expected: ` #data ++ data: ++ password: hoge + +`, + diffNum: 1, + }, + { + name: "Pod no diff 1", + manifests: `apiVersion: v1 +kind: Pod +metadata: + name: static-web + labels: + role: myrole +spec: + containers: + - name: web + image: nginx + resources: + limits: + memory: "2Gi" +--- +apiVersion: v1 +kind: Pod +metadata: + name: static-web + labels: + role: myrole +spec: + containers: + - name: web + image: nginx + ports: + resources: + limits: + memory: "2Gi" +`, + expected: "", + diffNum: 0, + falsePositive: false, + }, + { + name: "Pod no diff 2", + manifests: `apiVersion: v1 +kind: Pod +metadata: + name: static-web + labels: + role: myrole +spec: + containers: + - name: web + image: nginx + resources: + limits: + memory: "1536Mi" +--- +apiVersion: v1 +kind: Pod +metadata: + name: static-web + labels: + role: myrole +spec: + containers: + - name: web + image: nginx + ports: + resources: + limits: + memory: "1.5Gi" +`, + expected: "", + diffNum: 0, + falsePositive: false, + }, + } + + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + manifests, err := ParseManifests(tc.manifests) + require.NoError(t, err) + require.Equal(t, 2, len(manifests)) + old, new := manifests[0], manifests[1] + + result, err := Diff(old, new, zap.NewNop(), diff.WithEquateEmpty(), diff.WithIgnoreAddingMapKeys(), diff.WithCompareNumberAndNumericString()) + require.NoError(t, err) + + renderer := diff.NewRenderer(diff.WithLeftPadding(1)) + ds := renderer.Render(result.Nodes()) + if tc.falsePositive { + assert.NotEqual(t, tc.diffNum, result.NumNodes()) + assert.NotEqual(t, tc.expected, ds) + } else { + assert.Equal(t, tc.diffNum, result.NumNodes()) + assert.Equal(t, tc.expected, ds) + } + }) + } +} diff --git a/pkg/app/pipedv1/plugin/kubernetes/provider/diffutil.go b/pkg/app/pipedv1/plugin/kubernetes/provider/diffutil.go new file mode 100644 index 0000000000..4ff4c731a7 --- /dev/null +++ b/pkg/app/pipedv1/plugin/kubernetes/provider/diffutil.go @@ -0,0 +1,120 @@ +// Copyright 2024 The PipeCD Authors. +// +// 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 provider + +import ( + "bytes" + "encoding/json" + "reflect" + + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/kubernetes/scheme" +) + +// All functions in this file is borrowed from argocd/gitops-engine and modified +// All function except `remarshal` is borrowed from +// https://github.com/argoproj/gitops-engine/blob/0bc2f8c395f67123156d4ce6b667bf730618307f/pkg/utils/json/json.go +// and `remarshal` function is borrowed from +// https://github.com/argoproj/gitops-engine/blob/b0c5e00ccfa5d1e73087a18dc59e2e4c72f5f175/pkg/diff/diff.go#L685-L723 + +// https://github.com/ksonnet/ksonnet/blob/master/pkg/kubecfg/diff.go +func removeFields(config, live interface{}) interface{} { + switch c := config.(type) { + case map[string]interface{}: + l, ok := live.(map[string]interface{}) + if ok { + return removeMapFields(c, l) + } + return live + case []interface{}: + l, ok := live.([]interface{}) + if ok { + return removeListFields(c, l) + } + return live + default: + return live + } + +} + +// removeMapFields remove all non-existent fields in the live that don't exist in the config +func removeMapFields(config, live map[string]interface{}) map[string]interface{} { + result := map[string]interface{}{} + for k, v1 := range config { + v2, ok := live[k] + if !ok { + continue + } + if v2 != nil { + v2 = removeFields(v1, v2) + } + result[k] = v2 + } + return result +} + +func removeListFields(config, live []interface{}) []interface{} { + // If live is longer than config, then the extra elements at the end of the + // list will be returned as-is so they appear in the diff. + result := make([]interface{}, 0, len(live)) + for i, v2 := range live { + if len(config) > i { + if v2 != nil { + v2 = removeFields(config[i], v2) + } + result = append(result, v2) + } else { + result = append(result, v2) + } + } + return result +} + +// remarshal checks resource kind and version and re-marshal using corresponding struct custom marshaller. +// This ensures that expected resource state is formatter same as actual resource state in kubernetes +// and allows to find differences between actual and target states more accurately. +// Remarshalling also strips any type information (e.g. float64 vs. int) from the unstructured +// object. This is important for diffing since it will cause godiff to report a false difference. +func remarshal(obj *unstructured.Unstructured) (*unstructured.Unstructured, error) { + data, err := json.Marshal(obj) + if err != nil { + return nil, err + } + item, err := scheme.Scheme.New(obj.GroupVersionKind()) + if err != nil { + // This is common. the scheme is not registered + return nil, err + } + // This will drop any omitempty fields, perform resource conversion etc... + unmarshalledObj := reflect.New(reflect.TypeOf(item).Elem()).Interface() + // Unmarshal data into unmarshalledObj, but detect if there are any unknown fields that are not + // found in the target GVK object. + decoder := json.NewDecoder(bytes.NewReader(data)) + decoder.DisallowUnknownFields() + if err := decoder.Decode(&unmarshalledObj); err != nil { + // Likely a field present in obj that is not present in the GVK type, or user + // may have specified an invalid spec in git, so return original object + return nil, err + } + unstrBody, err := runtime.DefaultUnstructuredConverter.ToUnstructured(unmarshalledObj) + if err != nil { + return nil, err + } + // Remove all default values specified by custom formatter (e.g. creationTimestamp) + unstrBody = removeMapFields(obj.Object, unstrBody) + return &unstructured.Unstructured{Object: unstrBody}, nil +} diff --git a/pkg/app/pipedv1/plugin/kubernetes/provider/diffutil_test.go b/pkg/app/pipedv1/plugin/kubernetes/provider/diffutil_test.go new file mode 100644 index 0000000000..223e37bd15 --- /dev/null +++ b/pkg/app/pipedv1/plugin/kubernetes/provider/diffutil_test.go @@ -0,0 +1,218 @@ +// Copyright 2024 The PipeCD Authors. +// +// 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 provider + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestRemoveMapFields(t *testing.T) { + t.Parallel() + + testcases := []struct { + name string + config map[string]interface{} + live map[string]interface{} + expected map[string]interface{} + }{ + { + name: "Empty map", + config: make(map[string]interface{}, 0), + live: make(map[string]interface{}, 0), + expected: make(map[string]interface{}, 0), + }, + { + name: "Not nested 1", + config: map[string]interface{}{ + "key a": "value a", + }, + live: map[string]interface{}{ + "key a": "value a", + "key b": "value b", + }, + expected: map[string]interface{}{ + "key a": "value a", + }, + }, + { + name: "Not nested 2", + config: map[string]interface{}{ + "key a": "value a", + "key b": "value b", + }, + live: map[string]interface{}{ + "key a": "value a", + }, + expected: map[string]interface{}{ + "key a": "value a", + }, + }, + { + name: "Nested live deleted", + config: map[string]interface{}{ + "key a": "value a", + }, + live: map[string]interface{}{ + "key a": "value a", + "key b": map[string]interface{}{ + "nested key a": "nested value a", + }, + }, + expected: map[string]interface{}{ + "key a": "value a", + }, + }, + { + name: "Nested same", + config: map[string]interface{}{ + "key a": "value a", + "key b": map[string]interface{}{ + "nested key a": "nested value a", + }, + }, + live: map[string]interface{}{ + "key a": "value a", + "key b": map[string]interface{}{ + "nested key a": "nested value a", + }, + }, + expected: map[string]interface{}{ + "key a": "value a", + "key b": map[string]interface{}{ + "nested key a": "nested value a", + }, + }, + }, + { + name: "Nested nested live deleted", + config: map[string]interface{}{ + "key a": "value a", + "key b": map[string]interface{}{ + "nested key a": "nested value a", + }, + }, + live: map[string]interface{}{ + "key a": "value a", + "key b": map[string]interface{}{ + "nested key a": "nested value a", + "nested key b": "nested value b", + }, + }, + expected: map[string]interface{}{ + "key a": "value a", + "key b": map[string]interface{}{ + "nested key a": "nested value a", + }, + }, + }, + { + name: "Nested array", + config: map[string]interface{}{ + "key a": "value a", + "key b": []interface{}{ + "a", "b", 3, + }, + }, + live: map[string]interface{}{ + "key a": "value a", + "key b": []interface{}{ + "a", "b", 3, + }, + }, + expected: map[string]interface{}{ + "key a": "value a", + "key b": []interface{}{ + "a", "b", 3, + }, + }, + }, + { + name: "Nested array 2", + config: map[string]interface{}{ + "key a": "value a", + "key b": []interface{}{ + "a", "b", 3, 4, + }, + }, + live: map[string]interface{}{ + "key a": "value a", + "key b": []interface{}{ + "a", "b", 3, + }, + }, + expected: map[string]interface{}{ + "key a": "value a", + "key b": []interface{}{ + "a", "b", 3, + }, + }, + }, + { + name: "Nested array remain", + config: map[string]interface{}{ + "key a": "value a", + "key b": []interface{}{ + "a", "b", + }, + }, + live: map[string]interface{}{ + "key a": "value a", + "key b": []interface{}{ + "a", "b", map[string]interface{}{ + "aa": "aa", + }, + }, + }, + expected: map[string]interface{}{ + "key a": "value a", + "key b": []interface{}{ + "a", "b", map[string]interface{}{ + "aa": "aa", + }, + }, + }, + }, + { + name: "Nested array same", + config: map[string]interface{}{ + "key a": "value a", + "key b": []interface{}{ + "a", "b", 3, + }, + }, + live: map[string]interface{}{ + "key a": "value a", + "key b": []interface{}{ + "b", "a", 3, + }, + }, + expected: map[string]interface{}{ + "key a": "value a", + "key b": []interface{}{ + "b", "a", 3, + }, + }, + }, + } + + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + removed := removeMapFields(tc.config, tc.live) + assert.Equal(t, tc.expected, removed) + }) + } +} diff --git a/pkg/app/pipedv1/plugin/kubernetes/provider/resource.go b/pkg/app/pipedv1/plugin/kubernetes/provider/resource.go index 3337e1d4d6..35e5f009aa 100644 --- a/pkg/app/pipedv1/plugin/kubernetes/provider/resource.go +++ b/pkg/app/pipedv1/plugin/kubernetes/provider/resource.go @@ -20,15 +20,53 @@ import ( "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" ) +var builtInAPIVersions = map[string]struct{}{ + "admissionregistration.k8s.io/v1": {}, + "admissionregistration.k8s.io/v1beta1": {}, + "apiextensions.k8s.io/v1": {}, + "apiextensions.k8s.io/v1beta1": {}, + "apiregistration.k8s.io/v1": {}, + "apiregistration.k8s.io/v1beta1": {}, + "apps/v1": {}, + "authentication.k8s.io/v1": {}, + "authentication.k8s.io/v1beta1": {}, + "authorization.k8s.io/v1": {}, + "authorization.k8s.io/v1beta1": {}, + "autoscaling/v1": {}, + "autoscaling/v2beta1": {}, + "autoscaling/v2beta2": {}, + "batch/v1": {}, + "batch/v1beta1": {}, + "certificates.k8s.io/v1beta1": {}, + "coordination.k8s.io/v1": {}, + "coordination.k8s.io/v1beta1": {}, + "extensions/v1beta1": {}, + "internal.autoscaling.k8s.io/v1alpha1": {}, + "metrics.k8s.io/v1beta1": {}, + "networking.k8s.io/v1": {}, + "networking.k8s.io/v1beta1": {}, + "node.k8s.io/v1beta1": {}, + "policy/v1": {}, + "policy/v1beta1": {}, + "rbac.authorization.k8s.io/v1": {}, + "rbac.authorization.k8s.io/v1beta1": {}, + "scheduling.k8s.io/v1": {}, + "scheduling.k8s.io/v1beta1": {}, + "storage.k8s.io/v1": {}, + "storage.k8s.io/v1beta1": {}, + "v1": {}, +} - -const KindDeployment = "Deployment" +const ( + KindDeployment = "Deployment" + KindSecret = "Secret" +) type ResourceKey struct { APIVersion string - Kind string - Namespace string - Name string + Kind string + Namespace string + Name string } func (k ResourceKey) String() string { @@ -48,3 +86,20 @@ func MakeResourceKey(obj *unstructured.Unstructured) ResourceKey { } return k } + +func (k ResourceKey) IsSecret() bool { + if k.Kind != KindSecret { + return false + } + if !IsKubernetesBuiltInResource(k.APIVersion) { + return false + } + return true +} + +func IsKubernetesBuiltInResource(apiVersion string) bool { + _, ok := builtInAPIVersions[apiVersion] + // TODO: Change the way to detect whether an APIVersion is built-in or not + // rather than depending on this fixed list. + return ok +}