From d1fd32a3b3d9a8e8d30905f3fcd11b192dc1760a Mon Sep 17 00:00:00 2001 From: Shinnosuke Sawada-Dazai Date: Fri, 13 Dec 2024 12:23:05 +0900 Subject: [PATCH 1/6] Add annotation key constant Signed-off-by: Shinnosuke Sawada-Dazai --- pkg/app/pipedv1/plugin/kubernetes/provider/kubernetes.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/app/pipedv1/plugin/kubernetes/provider/kubernetes.go b/pkg/app/pipedv1/plugin/kubernetes/provider/kubernetes.go index 11402b398d..bdab6a5758 100644 --- a/pkg/app/pipedv1/plugin/kubernetes/provider/kubernetes.go +++ b/pkg/app/pipedv1/plugin/kubernetes/provider/kubernetes.go @@ -33,7 +33,8 @@ const ( LabelOriginalAPIVersion = "pipecd.dev/original-api-version" // The api version defined in git configuration. e.g. apps/v1 // annotations - AnnotationOrder = "pipecd.dev/order" // The order number of resource used to sort them before using. + AnnotationOrder = "pipecd.dev/order" // The order number of resource used to sort them before using. + AnnotationConfigHash = "pipecd.dev/config-hash" // The hash value of all mouting config resources. // label/annotation values ManagedByPiped = "piped" From 74f2480d77063fb9f4b74ac3c83d49017c4a9ac8 Mon Sep 17 00:00:00 2001 From: Shinnosuke Sawada-Dazai Date: Fri, 13 Dec 2024 12:24:58 +0900 Subject: [PATCH 2/6] Add IsDeployment method to ResourceKey Signed-off-by: Shinnosuke Sawada-Dazai --- pkg/app/pipedv1/plugin/kubernetes/provider/resource.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/pkg/app/pipedv1/plugin/kubernetes/provider/resource.go b/pkg/app/pipedv1/plugin/kubernetes/provider/resource.go index ce9b919519..f626afeb58 100644 --- a/pkg/app/pipedv1/plugin/kubernetes/provider/resource.go +++ b/pkg/app/pipedv1/plugin/kubernetes/provider/resource.go @@ -90,6 +90,16 @@ func MakeResourceKey(obj *unstructured.Unstructured) ResourceKey { return k } +func (k ResourceKey) IsDeployment() bool { + if k.Kind != KindDeployment { + return false + } + if !IsKubernetesBuiltInResource(k.APIVersion) { + return false + } + return true +} + func (k ResourceKey) IsConfigMap() bool { if k.Kind != KindConfigMap { return false From 126f53ad03763c09bd764d70400e76b19a5a0b50 Mon Sep 17 00:00:00 2001 From: Shinnosuke Sawada-Dazai Date: Fri, 13 Dec 2024 12:25:13 +0900 Subject: [PATCH 3/6] Add AddStringMapValues method to Manifest Signed-off-by: Shinnosuke Sawada-Dazai --- .../plugin/kubernetes/provider/manifest.go | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/pkg/app/pipedv1/plugin/kubernetes/provider/manifest.go b/pkg/app/pipedv1/plugin/kubernetes/provider/manifest.go index 8b0447fc1d..4f60065aec 100644 --- a/pkg/app/pipedv1/plugin/kubernetes/provider/manifest.go +++ b/pkg/app/pipedv1/plugin/kubernetes/provider/manifest.go @@ -16,6 +16,7 @@ package provider import ( "encoding/json" + "maps" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "sigs.k8s.io/yaml" @@ -69,3 +70,18 @@ func (m Manifest) AddAnnotations(annotations map[string]string) { } m.Body.SetAnnotations(annos) } + +// AddStringMapValues adds or overrides the given key-values into the string map +// that can be found at the specified fields. +func (m Manifest) AddStringMapValues(values map[string]string, fields ...string) error { + curMap, _, err := unstructured.NestedStringMap(m.Body.Object, fields...) + if err != nil { + return err + } + + if curMap == nil { + return unstructured.SetNestedStringMap(m.Body.Object, values, fields...) + } + maps.Copy(curMap, values) + return unstructured.SetNestedStringMap(m.Body.Object, curMap, fields...) +} From f01cd52c61175a591185f932db0ac0a3dd78cdd2 Mon Sep 17 00:00:00 2001 From: Shinnosuke Sawada-Dazai Date: Fri, 13 Dec 2024 12:26:03 +0900 Subject: [PATCH 4/6] Implement config hash annotation to Kubernetes deployment manifests Signed-off-by: Shinnosuke Sawada-Dazai --- .../plugin/kubernetes/deployment/annotate.go | 110 ++++++ .../kubernetes/deployment/annotate_test.go | 350 ++++++++++++++++++ 2 files changed, 460 insertions(+) create mode 100644 pkg/app/pipedv1/plugin/kubernetes/deployment/annotate.go create mode 100644 pkg/app/pipedv1/plugin/kubernetes/deployment/annotate_test.go diff --git a/pkg/app/pipedv1/plugin/kubernetes/deployment/annotate.go b/pkg/app/pipedv1/plugin/kubernetes/deployment/annotate.go new file mode 100644 index 0000000000..973682c8b8 --- /dev/null +++ b/pkg/app/pipedv1/plugin/kubernetes/deployment/annotate.go @@ -0,0 +1,110 @@ +// 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 deployment + +import ( + "github.com/pipe-cd/pipecd/pkg/app/pipedv1/plugin/kubernetes/provider" +) + +// annotateConfigHash appends a hash annotation into the workload manifests. +// The hash value is calculated by hashing the content of all configmaps/secrets +// that are referenced by the workload. +// This appending ensures that the workload should be restarted when +// one of its configurations changed. +func annotateConfigHash(manifests []provider.Manifest) error { + if len(manifests) == 0 { + return nil + } + + configMaps := make(map[string]provider.Manifest) + secrets := make(map[string]provider.Manifest) + for _, m := range manifests { + if m.Key.IsConfigMap() { + configMaps[m.Key.Name] = m + continue + } + if m.Key.IsSecret() { + secrets[m.Key.Name] = m + } + } + + // This application is not containing any config manifests + // so nothing to do. + if len(configMaps)+len(secrets) == 0 { + return nil + } + + for _, m := range manifests { + if m.Key.IsDeployment() { + if err := annotateConfigHashToWorkload(m, configMaps, secrets); err != nil { + return err + } + + // TODO: Add support for other workload types, such as StatefulSet, DaemonSet, etc. + } + } + + return nil +} + +func annotateConfigHashToWorkload(m provider.Manifest, managedConfigMaps, managedSecrets map[string]provider.Manifest) error { + configMaps := provider.FindReferencingConfigMaps(m.Body) + secrets := provider.FindReferencingSecrets(m.Body) + + // The deployment is not referencing any config resources. + if len(configMaps)+len(secrets) == 0 { + return nil + } + + cfgs := make([]provider.Manifest, 0, len(configMaps)+len(secrets)) + for _, cm := range configMaps { + m, ok := managedConfigMaps[cm] + if !ok { + // We do not return error here because the deployment may use + // a config resource that is not managed by PipeCD. + continue + } + cfgs = append(cfgs, m) + } + for _, s := range secrets { + m, ok := managedSecrets[s] + if !ok { + // We do not return error here because the deployment may use + // a config resource that is not managed by PipeCD. + continue + } + cfgs = append(cfgs, m) + } + + if len(cfgs) == 0 { + return nil + } + + hash, err := provider.HashManifests(cfgs) + if err != nil { + return err + } + + m.AddStringMapValues( + map[string]string{ + provider.AnnotationConfigHash: hash, + }, + "spec", + "template", + "metadata", + "annotations", + ) + return nil +} diff --git a/pkg/app/pipedv1/plugin/kubernetes/deployment/annotate_test.go b/pkg/app/pipedv1/plugin/kubernetes/deployment/annotate_test.go new file mode 100644 index 0000000000..83844f6afc --- /dev/null +++ b/pkg/app/pipedv1/plugin/kubernetes/deployment/annotate_test.go @@ -0,0 +1,350 @@ +// 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 deployment + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/pipe-cd/pipecd/pkg/app/pipedv1/plugin/kubernetes/provider" +) + +func TestAnnotateConfigHash(t *testing.T) { + t.Parallel() + + testcases := []struct { + name string + manifests string + expected string + expectedError error + }{ + { + name: "empty list", + }, + { + name: "one config", + manifests: ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: canary-by-config-change + labels: + app: canary-by-config-change +spec: + replicas: 2 + selector: + matchLabels: + app: canary-by-config-change + pipecd.dev/variant: primary + template: + metadata: + labels: + app: canary-by-config-change + pipecd.dev/variant: primary + spec: + containers: + - name: helloworld + image: gcr.io/pipecd/helloworld:v0.5.0 + args: + - server + ports: + - containerPort: 9085 + volumeMounts: + - name: config + mountPath: /etc/pipecd-config + readOnly: true + volumes: + - name: config + configMap: + name: canary-by-config-change +--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: canary-by-config-change +data: + two: "2" +`, + expected: ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: canary-by-config-change + labels: + app: canary-by-config-change +spec: + replicas: 2 + selector: + matchLabels: + app: canary-by-config-change + pipecd.dev/variant: primary + template: + metadata: + labels: + app: canary-by-config-change + pipecd.dev/variant: primary + annotations: + pipecd.dev/config-hash: 75c9m2btb6 + spec: + containers: + - name: helloworld + image: gcr.io/pipecd/helloworld:v0.5.0 + args: + - server + ports: + - containerPort: 9085 + volumeMounts: + - name: config + mountPath: /etc/pipecd-config + readOnly: true + volumes: + - name: config + configMap: + name: canary-by-config-change +--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: canary-by-config-change +data: + two: "2" +`, + }, + { + name: "multiple configs", + manifests: ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: canary-by-config-change + labels: + app: canary-by-config-change +spec: + replicas: 2 + selector: + matchLabels: + app: canary-by-config-change + pipecd.dev/variant: primary + template: + metadata: + labels: + app: canary-by-config-change + pipecd.dev/variant: primary + spec: + containers: + - name: helloworld + image: gcr.io/pipecd/helloworld:v0.5.0 + args: + - server + ports: + - containerPort: 9085 + volumeMounts: + - name: config + mountPath: /etc/pipecd-config + readOnly: true + volumes: + - name: config + configMap: + name: canary-by-config-change + - name: secret + secret: + secretName: secret-1 + - name: unmanaged-config + configMap: + name: unmanaged-config +--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: canary-by-config-change +data: + two: "2" +--- +apiVersion: v1 +kind: Secret +metadata: + name: secret-1 +type: my-type +data: + "one": "Mg==" +`, + expected: ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: canary-by-config-change + labels: + app: canary-by-config-change +spec: + replicas: 2 + selector: + matchLabels: + app: canary-by-config-change + pipecd.dev/variant: primary + template: + metadata: + labels: + app: canary-by-config-change + pipecd.dev/variant: primary + annotations: + pipecd.dev/config-hash: t7dtkdm455 + spec: + containers: + - name: helloworld + image: gcr.io/pipecd/helloworld:v0.5.0 + args: + - server + ports: + - containerPort: 9085 + volumeMounts: + - name: config + mountPath: /etc/pipecd-config + readOnly: true + volumes: + - name: config + configMap: + name: canary-by-config-change + - name: secret + secret: + secretName: secret-1 + - name: unmanaged-config + configMap: + name: unmanaged-config +--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: canary-by-config-change +data: + two: "2" +--- +apiVersion: v1 +kind: Secret +metadata: + name: secret-1 +type: my-type +data: + "one": "Mg==" +`, + }, + { + name: "one secret", + manifests: ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: canary-by-secret-change + labels: + app: canary-by-secret-change +spec: + replicas: 2 + selector: + matchLabels: + app: canary-by-secret-change + pipecd.dev/variant: primary + template: + metadata: + labels: + app: canary-by-secret-change + pipecd.dev/variant: primary + spec: + containers: + - name: helloworld + image: gcr.io/pipecd/helloworld:v0.5.0 + args: + - server + ports: + - containerPort: 9085 + volumeMounts: + - name: secret + mountPath: /etc/pipecd-secret + readOnly: true + volumes: + - name: secret + secret: + secretName: canary-by-secret-change +--- +apiVersion: v1 +kind: Secret +metadata: + name: canary-by-secret-change +type: Opaque +data: + one: "MQ==" +`, + expected: ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: canary-by-secret-change + labels: + app: canary-by-secret-change +spec: + replicas: 2 + selector: + matchLabels: + app: canary-by-secret-change + pipecd.dev/variant: primary + template: + metadata: + labels: + app: canary-by-secret-change + pipecd.dev/variant: primary + annotations: + pipecd.dev/config-hash: t58h88cd4b + spec: + containers: + - name: helloworld + image: gcr.io/pipecd/helloworld:v0.5.0 + args: + - server + ports: + - containerPort: 9085 + volumeMounts: + - name: secret + mountPath: /etc/pipecd-secret + readOnly: true + volumes: + - name: secret + secret: + secretName: canary-by-secret-change +--- +apiVersion: v1 +kind: Secret +metadata: + name: canary-by-secret-change +type: Opaque +data: + one: "MQ==" +`, + }, + } + + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + manifests, err := provider.ParseManifests(tc.manifests) + require.NoError(t, err) + + expected, err := provider.ParseManifests(tc.expected) + require.NoError(t, err) + + err = annotateConfigHash(manifests) + assert.Equal(t, expected, manifests) + assert.Equal(t, tc.expectedError, err) + }) + } +} From 0b3e61df5eb1eda259bbadcb279e5a2ee32b47af Mon Sep 17 00:00:00 2001 From: Shinnosuke Sawada-Dazai Date: Fri, 13 Dec 2024 13:47:14 +0900 Subject: [PATCH 5/6] Add unit tests for ResourceKey.IsDeployment method Signed-off-by: Shinnosuke Sawada-Dazai --- .../kubernetes/provider/resource_test.go | 71 +++++++++++++++++++ 1 file changed, 71 insertions(+) create mode 100644 pkg/app/pipedv1/plugin/kubernetes/provider/resource_test.go diff --git a/pkg/app/pipedv1/plugin/kubernetes/provider/resource_test.go b/pkg/app/pipedv1/plugin/kubernetes/provider/resource_test.go new file mode 100644 index 0000000000..121d29aa24 --- /dev/null +++ b/pkg/app/pipedv1/plugin/kubernetes/provider/resource_test.go @@ -0,0 +1,71 @@ +// 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" +) + +func TestResourceKey_IsDeployment(t *testing.T) { + tests := []struct { + name string + key ResourceKey + want bool + }{ + { + name: "Deployment with built-in API version", + key: ResourceKey{ + APIVersion: "apps/v1", + Kind: KindDeployment, + Namespace: "default", + Name: "test-deployment", + }, + want: true, + }, + { + name: "Deployment with non built-in API version", + key: ResourceKey{ + APIVersion: "custom/v1", + Kind: KindDeployment, + Namespace: "default", + Name: "test-deployment", + }, + want: false, + }, + { + name: "Non-Deployment kind", + key: ResourceKey{ + APIVersion: "apps/v1", + Kind: KindConfigMap, + Namespace: "default", + Name: "test-configmap", + }, + want: false, + }, + { + name: "Empty ResourceKey", + key: ResourceKey{}, + want: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := tt.key.IsDeployment(); got != tt.want { + t.Errorf("ResourceKey.IsDeployment() = %v, want %v", got, tt.want) + } + }) + } +} From ab992d675d5679a87afa6544eba66b953b3fb3ad Mon Sep 17 00:00:00 2001 From: Shinnosuke Sawada-Dazai Date: Fri, 13 Dec 2024 13:54:44 +0900 Subject: [PATCH 6/6] Add unit tests for AddStringMapValues method in Manifest Signed-off-by: Shinnosuke Sawada-Dazai --- .../kubernetes/provider/manifest_test.go | 112 ++++++++++++++++++ 1 file changed, 112 insertions(+) create mode 100644 pkg/app/pipedv1/plugin/kubernetes/provider/manifest_test.go diff --git a/pkg/app/pipedv1/plugin/kubernetes/provider/manifest_test.go b/pkg/app/pipedv1/plugin/kubernetes/provider/manifest_test.go new file mode 100644 index 0000000000..8f3f5469f2 --- /dev/null +++ b/pkg/app/pipedv1/plugin/kubernetes/provider/manifest_test.go @@ -0,0 +1,112 @@ +// 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/google/go-cmp/cmp" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" +) + +func TestManifest_AddStringMapValues(t *testing.T) { + tests := []struct { + name string + initial map[string]interface{} + values map[string]string + fields []string + expected map[string]interface{} + }{ + { + name: "add new values to empty map", + initial: map[string]interface{}{ + "metadata": map[string]interface{}{ + "annotations": map[string]interface{}{}, + }, + }, + values: map[string]string{ + "key1": "value1", + "key2": "value2", + }, + fields: []string{"metadata", "annotations"}, + expected: map[string]interface{}{ + "metadata": map[string]interface{}{ + "annotations": map[string]interface{}{ + "key1": "value1", + "key2": "value2", + }, + }, + }, + }, + { + name: "override existing values", + initial: map[string]interface{}{ + "metadata": map[string]interface{}{ + "annotations": map[string]interface{}{ + "key1": "oldvalue1", + }, + }, + }, + values: map[string]string{ + "key1": "newvalue1", + "key2": "value2", + }, + fields: []string{"metadata", "annotations"}, + expected: map[string]interface{}{ + "metadata": map[string]interface{}{ + "annotations": map[string]interface{}{ + "key1": "newvalue1", + "key2": "value2", + }, + }, + }, + }, + { + name: "add values to non-existing map", + initial: map[string]interface{}{ + "metadata": map[string]interface{}{}, + }, + values: map[string]string{ + "key1": "value1", + }, + fields: []string{"metadata", "annotations"}, + expected: map[string]interface{}{ + "metadata": map[string]interface{}{ + "annotations": map[string]interface{}{ + "key1": "value1", + }, + }, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + manifest := Manifest{ + Body: &unstructured.Unstructured{ + Object: tt.initial, + }, + } + err := manifest.AddStringMapValues(tt.values, tt.fields...) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if diff := cmp.Diff(tt.expected, manifest.Body.Object); diff != "" { + t.Errorf("unexpected result (-want +got):\n%s", diff) + } + }) + } +}