From 3013925a6e68a13ae51d4b9bdf8ad7afb4b82d1b Mon Sep 17 00:00:00 2001 From: Moritz Wiesinger Date: Thu, 3 Nov 2022 13:43:59 +0100 Subject: [PATCH 01/41] first implementation steps Signed-off-by: Moritz Wiesinger --- operator/webhooks/pod_mutating_webhook.go | 138 +++++++++++++++++++++- 1 file changed, 133 insertions(+), 5 deletions(-) diff --git a/operator/webhooks/pod_mutating_webhook.go b/operator/webhooks/pod_mutating_webhook.go index 16e671b7bc..db66d1f750 100644 --- a/operator/webhooks/pod_mutating_webhook.go +++ b/operator/webhooks/pod_mutating_webhook.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "fmt" + appsv1 "k8s.io/api/apps/v1" "net/http" "reflect" "strings" @@ -77,12 +78,13 @@ func (a *PodMutatingWebhook) Handle(ctx context.Context, req admission.Request) logger.Info(fmt.Sprintf("Pod annotations: %v", pod.Annotations)) - isAnnotated, err := a.isKeptnAnnotated(pod) + podIsAnnotated, err := a.isPodAnnotated(pod) + parentIsAnnotated, err := a.isParentAnnotated(ctx, &req, pod) if err != nil { span.SetStatus(codes.Error, "Invalid annotations") return admission.Errored(http.StatusBadRequest, err) } - if isAnnotated { + if podIsAnnotated { logger.Info("Resource is annotated with Keptn annotations, using Keptn scheduler") pod.Spec.SchedulerName = "keptn-scheduler" logger.Info("Annotations", "annotations", pod.Annotations) @@ -128,7 +130,7 @@ func (a *PodMutatingWebhook) InjectDecoder(d *admission.Decoder) error { return nil } -func (a *PodMutatingWebhook) isKeptnAnnotated(pod *corev1.Pod) (bool, error) { +func (a *PodMutatingWebhook) isPodAnnotated(pod *corev1.Pod) (bool, error) { workload, gotWorkloadAnnotation := getLabelOrAnnotation(pod, common.WorkloadAnnotation, common.K8sRecommendedWorkloadAnnotations) version, gotVersionAnnotation := getLabelOrAnnotation(pod, common.VersionAnnotation, common.K8sRecommendedVersionAnnotations) @@ -148,6 +150,93 @@ func (a *PodMutatingWebhook) isKeptnAnnotated(pod *corev1.Pod) (bool, error) { return false, nil } +func (a *PodMutatingWebhook) isParentAnnotated(ctx context.Context, req *admission.Request, pod *corev1.Pod) (bool, error) { + owner := a.getReplicaSetOfPod(pod) + if owner.UID == pod.UID { + return false, nil + } + + rsl := &appsv1.ReplicaSetList{} + if err := a.Client.List(ctx, rsl, client.InNamespace(req.Namespace)); err != nil { + return false, nil + } + + rs := appsv1.ReplicaSet{} + if len(rsl.Items) != 0 { + for _, rs = range rsl.Items { + if rs.UID == owner.UID { + break + } + } + } + + rsOwner := a.getOwnerOfReplicaSet(&rs) + dpList := &appsv1.DeploymentList{} + stsList := &appsv1.StatefulSetList{} + dsList := &appsv1.DaemonSetList{} + + if err := a.Client.List(ctx, dpList, client.InNamespace(req.Namespace)); err != nil { + return false, nil + } + if err := a.Client.List(ctx, stsList, client.InNamespace(req.Namespace)); err != nil { + return false, nil + } + if err := a.Client.List(ctx, dsList, client.InNamespace(req.Namespace)); err != nil { + return false, nil + } + + dp := appsv1.Deployment{} + if len(dpList.Items) != 0 { + for _, dp = range dpList.Items { + if dp.UID == rsOwner.UID { + break + } + } + } + + sts := appsv1.StatefulSet{} + if len(stsList.Items) != 0 { + for _, sts = range stsList.Items { + if sts.UID == rsOwner.UID { + break + } + } + } + + ds := appsv1.DaemonSet{} + if len(dsList.Items) != 0 { + for _, ds = range dsList.Items { + if ds.UID == rsOwner.UID { + break + } + } + } + + if dp.UID == rsOwner.UID { + workload, gotWorkloadAnnotation := getLabelOrAnnotationFromDeployment(&dp, common.WorkloadAnnotation, common.K8sRecommendedWorkloadAnnotations) + version, gotVersionAnnotation := getLabelOrAnnotationFromDeployment(&dp, common.VersionAnnotation, common.K8sRecommendedVersionAnnotations) + + if len(workload) > common.MaxWorkloadNameLength || len(version) > common.MaxVersionLength { + return false, common.ErrTooLongAnnotations + } + + if gotWorkloadAnnotation { + if !gotVersionAnnotation { + if len(pod.Annotations) == 0 { + pod.Annotations = make(map[string]string) + } + pod.Annotations[common.VersionAnnotation] = a.calculateVersion(pod) + } + return true, nil + } + } else if sts.UID == rsOwner.UID { + // TODO + } else if ds.UID == rsOwner.UID { + // TODO + } + return false, nil +} + func (a *PodMutatingWebhook) isAppAnnotationPresent(pod *corev1.Pod) (bool, error) { app, gotAppAnnotation := getLabelOrAnnotation(pod, common.AppAnnotation, common.K8sRecommendedAppAnnotations) @@ -334,7 +423,7 @@ func (a *PodMutatingWebhook) generateWorkload(ctx context.Context, pod *corev1.P Spec: klcv1alpha1.KeptnWorkloadSpec{ AppName: applicationName, Version: version, - ResourceReference: a.getResourceReference(pod), + ResourceReference: a.getReplicaSetOfPod(pod), PreDeploymentTasks: preDeploymentTasks, PostDeploymentTasks: postDeploymentTasks, PreDeploymentEvaluations: preDeploymentEvaluation, @@ -385,7 +474,7 @@ func (a *PodMutatingWebhook) getAppName(pod *corev1.Pod) string { return strings.ToLower(applicationName) } -func (a *PodMutatingWebhook) getResourceReference(pod *corev1.Pod) klcv1alpha1.ResourceReference { +func (a *PodMutatingWebhook) getReplicaSetOfPod(pod *corev1.Pod) klcv1alpha1.ResourceReference { reference := klcv1alpha1.ResourceReference{ UID: pod.UID, Kind: pod.Kind, @@ -401,6 +490,22 @@ func (a *PodMutatingWebhook) getResourceReference(pod *corev1.Pod) klcv1alpha1.R return reference } +func (a *PodMutatingWebhook) getOwnerOfReplicaSet(rs *appsv1.ReplicaSet) klcv1alpha1.ResourceReference { + reference := klcv1alpha1.ResourceReference{ + UID: rs.UID, + Kind: rs.Kind, + } + if len(rs.OwnerReferences) != 0 { + for _, o := range rs.OwnerReferences { + if o.Kind == "Deployment" || o.Kind == "StatefulSet" || o.Kind == "DaemonSet" { + reference.UID = o.UID + reference.Kind = o.Kind + } + } + } + return reference +} + func getLabelOrAnnotation(pod *corev1.Pod, primaryAnnotation string, secondaryAnnotation string) (string, bool) { if pod.Annotations[primaryAnnotation] != "" { return pod.Annotations[primaryAnnotation], true @@ -423,3 +528,26 @@ func getLabelOrAnnotation(pod *corev1.Pod, primaryAnnotation string, secondaryAn } return "", false } + +func getLabelOrAnnotationFromDeployment(dp *appsv1.Deployment, primaryAnnotation string, secondaryAnnotation string) (string, bool) { + if dp.Annotations[primaryAnnotation] != "" { + return dp.Annotations[primaryAnnotation], true + } + + if dp.Labels[primaryAnnotation] != "" { + return dp.Labels[primaryAnnotation], true + } + + if secondaryAnnotation == "" { + return "", false + } + + if dp.Annotations[secondaryAnnotation] != "" { + return dp.Annotations[secondaryAnnotation], true + } + + if dp.Labels[secondaryAnnotation] != "" { + return dp.Labels[secondaryAnnotation], true + } + return "", false +} From c72f5d2e8fc35bc1c421f353536c9ae16e448298 Mon Sep 17 00:00:00 2001 From: Moritz Wiesinger Date: Thu, 3 Nov 2022 14:29:28 +0100 Subject: [PATCH 02/41] fix typo in readme Signed-off-by: Moritz Wiesinger --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 763e85c7d3..0db3424356 100644 --- a/README.md +++ b/README.md @@ -59,7 +59,7 @@ For this reason, the Keptn Lifecycle Toolkit is agnostic to deployment tools tha ## How to use The Keptn Lifecycle Toolkit monitors manifests that have been applied against the Kubernetes API and reacts if it finds a workload with special annotations/labels. -For this, you should annotate your [Workload](https://kubernetes.io/docs/concepts/workloads/) with (at least) the following two annotations: +For this, you should annotate your [Workload](https://kubernetes.io/docs/concepts/workloads/) with (at least) the following annotations: ```yaml keptn.sh/app: myAwesomeAppName From 82867fae37aad8797fccde2fc05bdfff91a64f98 Mon Sep 17 00:00:00 2001 From: Moritz Wiesinger Date: Thu, 3 Nov 2022 15:36:10 +0100 Subject: [PATCH 03/41] add label copying for dp, sts and ds Signed-off-by: Moritz Wiesinger --- operator/webhooks/pod_mutating_webhook.go | 113 ++++++++++++++++++++-- 1 file changed, 106 insertions(+), 7 deletions(-) diff --git a/operator/webhooks/pod_mutating_webhook.go b/operator/webhooks/pod_mutating_webhook.go index db66d1f750..e598dded3f 100644 --- a/operator/webhooks/pod_mutating_webhook.go +++ b/operator/webhooks/pod_mutating_webhook.go @@ -79,7 +79,11 @@ func (a *PodMutatingWebhook) Handle(ctx context.Context, req admission.Request) logger.Info(fmt.Sprintf("Pod annotations: %v", pod.Annotations)) podIsAnnotated, err := a.isPodAnnotated(pod) - parentIsAnnotated, err := a.isParentAnnotated(ctx, &req, pod) + + if err == nil && !podIsAnnotated { + isParentAnnotated, err := a.copyAnnotationsIfParentAnnotated(ctx, &req, pod) + } + if err != nil { span.SetStatus(codes.Error, "Invalid annotations") return admission.Errored(http.StatusBadRequest, err) @@ -150,7 +154,7 @@ func (a *PodMutatingWebhook) isPodAnnotated(pod *corev1.Pod) (bool, error) { return false, nil } -func (a *PodMutatingWebhook) isParentAnnotated(ctx context.Context, req *admission.Request, pod *corev1.Pod) (bool, error) { +func (a *PodMutatingWebhook) copyAnnotationsIfParentAnnotated(ctx context.Context, req *admission.Request, pod *corev1.Pod) (bool, error) { owner := a.getReplicaSetOfPod(pod) if owner.UID == pod.UID { return false, nil @@ -212,9 +216,12 @@ func (a *PodMutatingWebhook) isParentAnnotated(ctx context.Context, req *admissi } } + var workload, version string + var gotWorkloadAnnotation, gotVersionAnnotation bool + if dp.UID == rsOwner.UID { - workload, gotWorkloadAnnotation := getLabelOrAnnotationFromDeployment(&dp, common.WorkloadAnnotation, common.K8sRecommendedWorkloadAnnotations) - version, gotVersionAnnotation := getLabelOrAnnotationFromDeployment(&dp, common.VersionAnnotation, common.K8sRecommendedVersionAnnotations) + workload, gotWorkloadAnnotation = getLabelOrAnnotationFromDeployment(&dp, common.WorkloadAnnotation, common.K8sRecommendedWorkloadAnnotations) + version, gotVersionAnnotation = getLabelOrAnnotationFromDeployment(&dp, common.VersionAnnotation, common.K8sRecommendedVersionAnnotations) if len(workload) > common.MaxWorkloadNameLength || len(version) > common.MaxVersionLength { return false, common.ErrTooLongAnnotations @@ -226,15 +233,61 @@ func (a *PodMutatingWebhook) isParentAnnotated(ctx context.Context, req *admissi pod.Annotations = make(map[string]string) } pod.Annotations[common.VersionAnnotation] = a.calculateVersion(pod) + } else { + pod.Annotations[common.VersionAnnotation] = dp.Annotations[common.VersionAnnotation] } + + pod.Annotations[common.WorkloadAnnotation] = dp.Annotations[common.WorkloadAnnotation] return true, nil } + return false, nil } else if sts.UID == rsOwner.UID { - // TODO + workload, gotWorkloadAnnotation = getLabelOrAnnotationFromStatefulSet(&sts, common.WorkloadAnnotation, common.K8sRecommendedWorkloadAnnotations) + version, gotVersionAnnotation = getLabelOrAnnotationFromStatefulSet(&sts, common.VersionAnnotation, common.K8sRecommendedVersionAnnotations) + + if len(workload) > common.MaxWorkloadNameLength || len(version) > common.MaxVersionLength { + return false, common.ErrTooLongAnnotations + } + + if gotWorkloadAnnotation { + if !gotVersionAnnotation { + if len(pod.Annotations) == 0 { + pod.Annotations = make(map[string]string) + } + pod.Annotations[common.VersionAnnotation] = a.calculateVersion(pod) + } else { + pod.Annotations[common.VersionAnnotation] = sts.Annotations[common.VersionAnnotation] + } + + pod.Annotations[common.WorkloadAnnotation] = sts.Annotations[common.WorkloadAnnotation] + return true, nil + } + return false, nil } else if ds.UID == rsOwner.UID { - // TODO + workload, gotWorkloadAnnotation = getLabelOrAnnotationFromDaemonSet(&ds, common.WorkloadAnnotation, common.K8sRecommendedWorkloadAnnotations) + version, gotVersionAnnotation = getLabelOrAnnotationFromDaemonSet(&ds, common.VersionAnnotation, common.K8sRecommendedVersionAnnotations) + + if len(workload) > common.MaxWorkloadNameLength || len(version) > common.MaxVersionLength { + return false, common.ErrTooLongAnnotations + } + + if gotWorkloadAnnotation { + if !gotVersionAnnotation { + if len(pod.Annotations) == 0 { + pod.Annotations = make(map[string]string) + } + pod.Annotations[common.VersionAnnotation] = a.calculateVersion(pod) + } else { + pod.Annotations[common.VersionAnnotation] = ds.Annotations[common.VersionAnnotation] + } + + pod.Annotations[common.WorkloadAnnotation] = ds.Annotations[common.WorkloadAnnotation] + return true, nil + } + return false, nil + } else { + return false, nil } - return false, nil } func (a *PodMutatingWebhook) isAppAnnotationPresent(pod *corev1.Pod) (bool, error) { @@ -551,3 +604,49 @@ func getLabelOrAnnotationFromDeployment(dp *appsv1.Deployment, primaryAnnotation } return "", false } + +func getLabelOrAnnotationFromStatefulSet(sts *appsv1.StatefulSet, primaryAnnotation string, secondaryAnnotation string) (string, bool) { + if sts.Annotations[primaryAnnotation] != "" { + return sts.Annotations[primaryAnnotation], true + } + + if sts.Labels[primaryAnnotation] != "" { + return sts.Labels[primaryAnnotation], true + } + + if secondaryAnnotation == "" { + return "", false + } + + if sts.Annotations[secondaryAnnotation] != "" { + return sts.Annotations[secondaryAnnotation], true + } + + if sts.Labels[secondaryAnnotation] != "" { + return sts.Labels[secondaryAnnotation], true + } + return "", false +} + +func getLabelOrAnnotationFromDaemonSet(ds *appsv1.DaemonSet, primaryAnnotation string, secondaryAnnotation string) (string, bool) { + if ds.Annotations[primaryAnnotation] != "" { + return ds.Annotations[primaryAnnotation], true + } + + if ds.Labels[primaryAnnotation] != "" { + return ds.Labels[primaryAnnotation], true + } + + if secondaryAnnotation == "" { + return "", false + } + + if ds.Annotations[secondaryAnnotation] != "" { + return ds.Annotations[secondaryAnnotation], true + } + + if ds.Labels[secondaryAnnotation] != "" { + return ds.Labels[secondaryAnnotation], true + } + return "", false +} From 55b580a6f2f2d8f2511073ce057612498828da13 Mon Sep 17 00:00:00 2001 From: Moritz Wiesinger Date: Thu, 3 Nov 2022 15:37:01 +0100 Subject: [PATCH 04/41] go fmt Signed-off-by: Moritz Wiesinger --- operator/api/v1alpha1/groupversion_info.go | 4 ++-- scheduler/cmd/scheduler/main_test.go | 25 +++++++++++----------- 2 files changed, 14 insertions(+), 15 deletions(-) diff --git a/operator/api/v1alpha1/groupversion_info.go b/operator/api/v1alpha1/groupversion_info.go index 911c854d36..d342beaf07 100644 --- a/operator/api/v1alpha1/groupversion_info.go +++ b/operator/api/v1alpha1/groupversion_info.go @@ -15,8 +15,8 @@ limitations under the License. */ // Package v1alpha1 contains API Schema definitions for the lifecycle v1alpha1 API group -//+kubebuilder:object:generate=true -//+groupName=lifecycle.keptn.sh +// +kubebuilder:object:generate=true +// +groupName=lifecycle.keptn.sh package v1alpha1 import ( diff --git a/scheduler/cmd/scheduler/main_test.go b/scheduler/cmd/scheduler/main_test.go index 7f7412a325..b54b130910 100644 --- a/scheduler/cmd/scheduler/main_test.go +++ b/scheduler/cmd/scheduler/main_test.go @@ -1,19 +1,18 @@ -///* -//Copyright 2020 The Kubernetes Authors. +// /* +// Copyright 2020 The Kubernetes 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 +// 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. -//*/ +// 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 main // From 2f0862c93a70947c71880d01886237369b5c1b27 Mon Sep 17 00:00:00 2001 From: Moritz Wiesinger Date: Mon, 7 Nov 2022 11:19:52 +0100 Subject: [PATCH 05/41] remove duplicate methods in favor of generalized ones Signed-off-by: Moritz Wiesinger --- operator/webhooks/pod_mutating_webhook.go | 130 ++++++---------------- 1 file changed, 31 insertions(+), 99 deletions(-) diff --git a/operator/webhooks/pod_mutating_webhook.go b/operator/webhooks/pod_mutating_webhook.go index e598dded3f..e5f917b49d 100644 --- a/operator/webhooks/pod_mutating_webhook.go +++ b/operator/webhooks/pod_mutating_webhook.go @@ -81,7 +81,7 @@ func (a *PodMutatingWebhook) Handle(ctx context.Context, req admission.Request) podIsAnnotated, err := a.isPodAnnotated(pod) if err == nil && !podIsAnnotated { - isParentAnnotated, err := a.copyAnnotationsIfParentAnnotated(ctx, &req, pod) + _, _ = a.copyAnnotationsIfParentAnnotated(ctx, &req, pod) } if err != nil { @@ -135,8 +135,8 @@ func (a *PodMutatingWebhook) InjectDecoder(d *admission.Decoder) error { } func (a *PodMutatingWebhook) isPodAnnotated(pod *corev1.Pod) (bool, error) { - workload, gotWorkloadAnnotation := getLabelOrAnnotation(pod, common.WorkloadAnnotation, common.K8sRecommendedWorkloadAnnotations) - version, gotVersionAnnotation := getLabelOrAnnotation(pod, common.VersionAnnotation, common.K8sRecommendedVersionAnnotations) + workload, gotWorkloadAnnotation := getLabelOrAnnotation(&pod.ObjectMeta, common.WorkloadAnnotation, common.K8sRecommendedWorkloadAnnotations) + version, gotVersionAnnotation := getLabelOrAnnotation(&pod.ObjectMeta, common.VersionAnnotation, common.K8sRecommendedVersionAnnotations) if len(workload) > common.MaxWorkloadNameLength || len(version) > common.MaxVersionLength { return false, ErrTooLongAnnotations @@ -148,6 +148,7 @@ func (a *PodMutatingWebhook) isPodAnnotated(pod *corev1.Pod) (bool, error) { pod.Annotations = make(map[string]string) } pod.Annotations[common.VersionAnnotation] = a.calculateVersion(pod) + pod.GetAnnotations() } return true, nil } @@ -220,8 +221,8 @@ func (a *PodMutatingWebhook) copyAnnotationsIfParentAnnotated(ctx context.Contex var gotWorkloadAnnotation, gotVersionAnnotation bool if dp.UID == rsOwner.UID { - workload, gotWorkloadAnnotation = getLabelOrAnnotationFromDeployment(&dp, common.WorkloadAnnotation, common.K8sRecommendedWorkloadAnnotations) - version, gotVersionAnnotation = getLabelOrAnnotationFromDeployment(&dp, common.VersionAnnotation, common.K8sRecommendedVersionAnnotations) + workload, gotWorkloadAnnotation = getLabelOrAnnotation(&dp.ObjectMeta, common.WorkloadAnnotation, common.K8sRecommendedWorkloadAnnotations) + version, gotVersionAnnotation = getLabelOrAnnotation(&dp.ObjectMeta, common.VersionAnnotation, common.K8sRecommendedVersionAnnotations) if len(workload) > common.MaxWorkloadNameLength || len(version) > common.MaxVersionLength { return false, common.ErrTooLongAnnotations @@ -242,8 +243,8 @@ func (a *PodMutatingWebhook) copyAnnotationsIfParentAnnotated(ctx context.Contex } return false, nil } else if sts.UID == rsOwner.UID { - workload, gotWorkloadAnnotation = getLabelOrAnnotationFromStatefulSet(&sts, common.WorkloadAnnotation, common.K8sRecommendedWorkloadAnnotations) - version, gotVersionAnnotation = getLabelOrAnnotationFromStatefulSet(&sts, common.VersionAnnotation, common.K8sRecommendedVersionAnnotations) + workload, gotWorkloadAnnotation = getLabelOrAnnotation(&sts.ObjectMeta, common.WorkloadAnnotation, common.K8sRecommendedWorkloadAnnotations) + version, gotVersionAnnotation = getLabelOrAnnotation(&sts.ObjectMeta, common.VersionAnnotation, common.K8sRecommendedVersionAnnotations) if len(workload) > common.MaxWorkloadNameLength || len(version) > common.MaxVersionLength { return false, common.ErrTooLongAnnotations @@ -264,8 +265,8 @@ func (a *PodMutatingWebhook) copyAnnotationsIfParentAnnotated(ctx context.Contex } return false, nil } else if ds.UID == rsOwner.UID { - workload, gotWorkloadAnnotation = getLabelOrAnnotationFromDaemonSet(&ds, common.WorkloadAnnotation, common.K8sRecommendedWorkloadAnnotations) - version, gotVersionAnnotation = getLabelOrAnnotationFromDaemonSet(&ds, common.VersionAnnotation, common.K8sRecommendedVersionAnnotations) + workload, gotWorkloadAnnotation = getLabelOrAnnotation(&ds.ObjectMeta, common.WorkloadAnnotation, common.K8sRecommendedWorkloadAnnotations) + version, gotVersionAnnotation = getLabelOrAnnotation(&ds.ObjectMeta, common.VersionAnnotation, common.K8sRecommendedVersionAnnotations) if len(workload) > common.MaxWorkloadNameLength || len(version) > common.MaxVersionLength { return false, common.ErrTooLongAnnotations @@ -291,7 +292,7 @@ func (a *PodMutatingWebhook) copyAnnotationsIfParentAnnotated(ctx context.Contex } func (a *PodMutatingWebhook) isAppAnnotationPresent(pod *corev1.Pod) (bool, error) { - app, gotAppAnnotation := getLabelOrAnnotation(pod, common.AppAnnotation, common.K8sRecommendedAppAnnotations) + app, gotAppAnnotation := getLabelOrAnnotation(&pod.ObjectMeta, common.AppAnnotation, common.K8sRecommendedAppAnnotations) if gotAppAnnotation { if len(app) > common.MaxAppNameLength { @@ -303,7 +304,7 @@ func (a *PodMutatingWebhook) isAppAnnotationPresent(pod *corev1.Pod) (bool, erro if len(pod.Annotations) == 0 { pod.Annotations = make(map[string]string) } - pod.Annotations[common.AppAnnotation], _ = getLabelOrAnnotation(pod, common.WorkloadAnnotation, common.K8sRecommendedWorkloadAnnotations) + pod.Annotations[common.AppAnnotation], _ = getLabelOrAnnotation(&pod.ObjectMeta, common.WorkloadAnnotation, common.K8sRecommendedWorkloadAnnotations) return false, nil } @@ -438,27 +439,27 @@ func (a *PodMutatingWebhook) handleApp(ctx context.Context, logger logr.Logger, } func (a *PodMutatingWebhook) generateWorkload(ctx context.Context, pod *corev1.Pod, namespace string) *klcv1alpha1.KeptnWorkload { - version, _ := getLabelOrAnnotation(pod, common.VersionAnnotation, common.K8sRecommendedVersionAnnotations) - applicationName, _ := getLabelOrAnnotation(pod, common.AppAnnotation, common.K8sRecommendedAppAnnotations) + version, _ := getLabelOrAnnotation(&pod.ObjectMeta, common.VersionAnnotation, common.K8sRecommendedVersionAnnotations) + applicationName, _ := getLabelOrAnnotation(&pod.ObjectMeta, common.AppAnnotation, common.K8sRecommendedAppAnnotations) var preDeploymentTasks []string var postDeploymentTasks []string var preDeploymentEvaluation []string var postDeploymentEvaluation []string - if annotations, found := getLabelOrAnnotation(pod, common.PreDeploymentTaskAnnotation, ""); found { + if annotations, found := getLabelOrAnnotation(&pod.ObjectMeta, common.PreDeploymentTaskAnnotation, ""); found { preDeploymentTasks = strings.Split(annotations, ",") } - if annotations, found := getLabelOrAnnotation(pod, common.PostDeploymentTaskAnnotation, ""); found { + if annotations, found := getLabelOrAnnotation(&pod.ObjectMeta, common.PostDeploymentTaskAnnotation, ""); found { postDeploymentTasks = strings.Split(annotations, ",") } - if annotations, found := getLabelOrAnnotation(pod, common.PreDeploymentEvaluationAnnotation, ""); found { + if annotations, found := getLabelOrAnnotation(&pod.ObjectMeta, common.PreDeploymentEvaluationAnnotation, ""); found { preDeploymentEvaluation = strings.Split(annotations, ",") } - if annotations, found := getLabelOrAnnotation(pod, common.PostDeploymentEvaluationAnnotation, ""); found { + if annotations, found := getLabelOrAnnotation(&pod.ObjectMeta, common.PostDeploymentEvaluationAnnotation, ""); found { postDeploymentEvaluation = strings.Split(annotations, ",") } @@ -486,7 +487,7 @@ func (a *PodMutatingWebhook) generateWorkload(ctx context.Context, pod *corev1.P } func (a *PodMutatingWebhook) generateApp(ctx context.Context, pod *corev1.Pod, namespace string) *klcv1alpha1.KeptnApp { - version, _ := getLabelOrAnnotation(pod, common.VersionAnnotation, common.K8sRecommendedVersionAnnotations) + version, _ := getLabelOrAnnotation(&pod.ObjectMeta, common.VersionAnnotation, common.K8sRecommendedVersionAnnotations) appName := a.getAppName(pod) // create TraceContext @@ -517,13 +518,13 @@ func (a *PodMutatingWebhook) generateApp(ctx context.Context, pod *corev1.Pod, n } func (a *PodMutatingWebhook) getWorkloadName(pod *corev1.Pod) string { - workloadName, _ := getLabelOrAnnotation(pod, common.WorkloadAnnotation, common.K8sRecommendedWorkloadAnnotations) - applicationName, _ := getLabelOrAnnotation(pod, common.AppAnnotation, common.K8sRecommendedAppAnnotations) + workloadName, _ := getLabelOrAnnotation(&pod.ObjectMeta, common.WorkloadAnnotation, common.K8sRecommendedWorkloadAnnotations) + applicationName, _ := getLabelOrAnnotation(&pod.ObjectMeta, common.AppAnnotation, common.K8sRecommendedAppAnnotations) return strings.ToLower(applicationName + "-" + workloadName) } func (a *PodMutatingWebhook) getAppName(pod *corev1.Pod) string { - applicationName, _ := getLabelOrAnnotation(pod, common.AppAnnotation, common.K8sRecommendedAppAnnotations) + applicationName, _ := getLabelOrAnnotation(&pod.ObjectMeta, common.AppAnnotation, common.K8sRecommendedAppAnnotations) return strings.ToLower(applicationName) } @@ -559,94 +560,25 @@ func (a *PodMutatingWebhook) getOwnerOfReplicaSet(rs *appsv1.ReplicaSet) klcv1al return reference } -func getLabelOrAnnotation(pod *corev1.Pod, primaryAnnotation string, secondaryAnnotation string) (string, bool) { - if pod.Annotations[primaryAnnotation] != "" { - return pod.Annotations[primaryAnnotation], true +func getLabelOrAnnotation(resource *metav1.ObjectMeta, primaryAnnotation string, secondaryAnnotation string) (string, bool) { + if resource.Annotations[primaryAnnotation] != "" { + return resource.Annotations[primaryAnnotation], true } - if pod.Labels[primaryAnnotation] != "" { - return pod.Labels[primaryAnnotation], true + if resource.Labels[primaryAnnotation] != "" { + return resource.Labels[primaryAnnotation], true } if secondaryAnnotation == "" { return "", false } - if pod.Annotations[secondaryAnnotation] != "" { - return pod.Annotations[secondaryAnnotation], true + if resource.Annotations[secondaryAnnotation] != "" { + return resource.Annotations[secondaryAnnotation], true } - if pod.Labels[secondaryAnnotation] != "" { - return pod.Labels[secondaryAnnotation], true - } - return "", false -} - -func getLabelOrAnnotationFromDeployment(dp *appsv1.Deployment, primaryAnnotation string, secondaryAnnotation string) (string, bool) { - if dp.Annotations[primaryAnnotation] != "" { - return dp.Annotations[primaryAnnotation], true - } - - if dp.Labels[primaryAnnotation] != "" { - return dp.Labels[primaryAnnotation], true - } - - if secondaryAnnotation == "" { - return "", false - } - - if dp.Annotations[secondaryAnnotation] != "" { - return dp.Annotations[secondaryAnnotation], true - } - - if dp.Labels[secondaryAnnotation] != "" { - return dp.Labels[secondaryAnnotation], true - } - return "", false -} - -func getLabelOrAnnotationFromStatefulSet(sts *appsv1.StatefulSet, primaryAnnotation string, secondaryAnnotation string) (string, bool) { - if sts.Annotations[primaryAnnotation] != "" { - return sts.Annotations[primaryAnnotation], true - } - - if sts.Labels[primaryAnnotation] != "" { - return sts.Labels[primaryAnnotation], true - } - - if secondaryAnnotation == "" { - return "", false - } - - if sts.Annotations[secondaryAnnotation] != "" { - return sts.Annotations[secondaryAnnotation], true - } - - if sts.Labels[secondaryAnnotation] != "" { - return sts.Labels[secondaryAnnotation], true - } - return "", false -} - -func getLabelOrAnnotationFromDaemonSet(ds *appsv1.DaemonSet, primaryAnnotation string, secondaryAnnotation string) (string, bool) { - if ds.Annotations[primaryAnnotation] != "" { - return ds.Annotations[primaryAnnotation], true - } - - if ds.Labels[primaryAnnotation] != "" { - return ds.Labels[primaryAnnotation], true - } - - if secondaryAnnotation == "" { - return "", false - } - - if ds.Annotations[secondaryAnnotation] != "" { - return ds.Annotations[secondaryAnnotation], true - } - - if ds.Labels[secondaryAnnotation] != "" { - return ds.Labels[secondaryAnnotation], true + if resource.Labels[secondaryAnnotation] != "" { + return resource.Labels[secondaryAnnotation], true } return "", false } From 058fca49e94ce81b7833af39318a43527a8c0bd7 Mon Sep 17 00:00:00 2001 From: Moritz Wiesinger Date: Mon, 7 Nov 2022 11:39:37 +0100 Subject: [PATCH 06/41] remove unneeded length checks Signed-off-by: Moritz Wiesinger --- operator/webhooks/pod_mutating_webhook.go | 24 +++++++++-------------- 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/operator/webhooks/pod_mutating_webhook.go b/operator/webhooks/pod_mutating_webhook.go index e5f917b49d..861482f1d7 100644 --- a/operator/webhooks/pod_mutating_webhook.go +++ b/operator/webhooks/pod_mutating_webhook.go @@ -191,29 +191,23 @@ func (a *PodMutatingWebhook) copyAnnotationsIfParentAnnotated(ctx context.Contex } dp := appsv1.Deployment{} - if len(dpList.Items) != 0 { - for _, dp = range dpList.Items { - if dp.UID == rsOwner.UID { - break - } + for _, dp = range dpList.Items { + if dp.UID == rsOwner.UID { + break } } sts := appsv1.StatefulSet{} - if len(stsList.Items) != 0 { - for _, sts = range stsList.Items { - if sts.UID == rsOwner.UID { - break - } + for _, sts = range stsList.Items { + if sts.UID == rsOwner.UID { + break } } ds := appsv1.DaemonSet{} - if len(dsList.Items) != 0 { - for _, ds = range dsList.Items { - if ds.UID == rsOwner.UID { - break - } + for _, ds = range dsList.Items { + if ds.UID == rsOwner.UID { + break } } From 5c59ec86fd35159d7df54205703141e7e58010b8 Mon Sep 17 00:00:00 2001 From: Moritz Wiesinger Date: Mon, 7 Nov 2022 13:13:06 +0100 Subject: [PATCH 07/41] move code to separate function to reduce complexity Signed-off-by: Moritz Wiesinger --- operator/webhooks/pod_mutating_webhook.go | 90 +++++++---------------- 1 file changed, 27 insertions(+), 63 deletions(-) diff --git a/operator/webhooks/pod_mutating_webhook.go b/operator/webhooks/pod_mutating_webhook.go index 861482f1d7..f0f09adf74 100644 --- a/operator/webhooks/pod_mutating_webhook.go +++ b/operator/webhooks/pod_mutating_webhook.go @@ -211,78 +211,42 @@ func (a *PodMutatingWebhook) copyAnnotationsIfParentAnnotated(ctx context.Contex } } - var workload, version string - var gotWorkloadAnnotation, gotVersionAnnotation bool - if dp.UID == rsOwner.UID { - workload, gotWorkloadAnnotation = getLabelOrAnnotation(&dp.ObjectMeta, common.WorkloadAnnotation, common.K8sRecommendedWorkloadAnnotations) - version, gotVersionAnnotation = getLabelOrAnnotation(&dp.ObjectMeta, common.VersionAnnotation, common.K8sRecommendedVersionAnnotations) - - if len(workload) > common.MaxWorkloadNameLength || len(version) > common.MaxVersionLength { - return false, common.ErrTooLongAnnotations - } - - if gotWorkloadAnnotation { - if !gotVersionAnnotation { - if len(pod.Annotations) == 0 { - pod.Annotations = make(map[string]string) - } - pod.Annotations[common.VersionAnnotation] = a.calculateVersion(pod) - } else { - pod.Annotations[common.VersionAnnotation] = dp.Annotations[common.VersionAnnotation] - } - - pod.Annotations[common.WorkloadAnnotation] = dp.Annotations[common.WorkloadAnnotation] - return true, nil - } - return false, nil + return a.copyResourceLabelsIfPresent(&dp.ObjectMeta, pod) } else if sts.UID == rsOwner.UID { - workload, gotWorkloadAnnotation = getLabelOrAnnotation(&sts.ObjectMeta, common.WorkloadAnnotation, common.K8sRecommendedWorkloadAnnotations) - version, gotVersionAnnotation = getLabelOrAnnotation(&sts.ObjectMeta, common.VersionAnnotation, common.K8sRecommendedVersionAnnotations) - - if len(workload) > common.MaxWorkloadNameLength || len(version) > common.MaxVersionLength { - return false, common.ErrTooLongAnnotations - } + return a.copyResourceLabelsIfPresent(&sts.ObjectMeta, pod) + } else if ds.UID == rsOwner.UID { + return a.copyResourceLabelsIfPresent(&ds.ObjectMeta, pod) + } else { + return false, nil + } +} - if gotWorkloadAnnotation { - if !gotVersionAnnotation { - if len(pod.Annotations) == 0 { - pod.Annotations = make(map[string]string) - } - pod.Annotations[common.VersionAnnotation] = a.calculateVersion(pod) - } else { - pod.Annotations[common.VersionAnnotation] = sts.Annotations[common.VersionAnnotation] - } +func (a *PodMutatingWebhook) copyResourceLabelsIfPresent(sourceResource *metav1.ObjectMeta, targetPod *corev1.Pod) (bool, error) { + var workload, version string + var gotWorkloadAnnotation, gotVersionAnnotation bool - pod.Annotations[common.WorkloadAnnotation] = sts.Annotations[common.WorkloadAnnotation] - return true, nil - } - return false, nil - } else if ds.UID == rsOwner.UID { - workload, gotWorkloadAnnotation = getLabelOrAnnotation(&ds.ObjectMeta, common.WorkloadAnnotation, common.K8sRecommendedWorkloadAnnotations) - version, gotVersionAnnotation = getLabelOrAnnotation(&ds.ObjectMeta, common.VersionAnnotation, common.K8sRecommendedVersionAnnotations) + workload, gotWorkloadAnnotation = getLabelOrAnnotation(sourceResource, common.WorkloadAnnotation, common.K8sRecommendedWorkloadAnnotations) + version, gotVersionAnnotation = getLabelOrAnnotation(sourceResource, common.VersionAnnotation, common.K8sRecommendedVersionAnnotations) - if len(workload) > common.MaxWorkloadNameLength || len(version) > common.MaxVersionLength { - return false, common.ErrTooLongAnnotations - } + if len(workload) > common.MaxWorkloadNameLength || len(version) > common.MaxVersionLength { + return false, common.ErrTooLongAnnotations + } - if gotWorkloadAnnotation { - if !gotVersionAnnotation { - if len(pod.Annotations) == 0 { - pod.Annotations = make(map[string]string) - } - pod.Annotations[common.VersionAnnotation] = a.calculateVersion(pod) - } else { - pod.Annotations[common.VersionAnnotation] = ds.Annotations[common.VersionAnnotation] + if gotWorkloadAnnotation { + if !gotVersionAnnotation { + if len(targetPod.Annotations) == 0 { + targetPod.Annotations = make(map[string]string) } - - pod.Annotations[common.WorkloadAnnotation] = ds.Annotations[common.WorkloadAnnotation] - return true, nil + targetPod.Annotations[common.VersionAnnotation] = a.calculateVersion(targetPod) + } else { + targetPod.Annotations[common.VersionAnnotation] = sourceResource.Annotations[common.VersionAnnotation] } - return false, nil - } else { - return false, nil + + targetPod.Annotations[common.WorkloadAnnotation] = sourceResource.Annotations[common.WorkloadAnnotation] + return true, nil } + return false, nil } func (a *PodMutatingWebhook) isAppAnnotationPresent(pod *corev1.Pod) (bool, error) { From 7c8352b7d9779ed355a0e183101b3947a044128b Mon Sep 17 00:00:00 2001 From: Moritz Wiesinger Date: Mon, 7 Nov 2022 13:41:05 +0100 Subject: [PATCH 08/41] rename to kltv1alpha1 Signed-off-by: Moritz Wiesinger --- operator/webhooks/pod_mutating_webhook.go | 34 +++++++++++++---------- 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/operator/webhooks/pod_mutating_webhook.go b/operator/webhooks/pod_mutating_webhook.go index f0f09adf74..77b79d7759 100644 --- a/operator/webhooks/pod_mutating_webhook.go +++ b/operator/webhooks/pod_mutating_webhook.go @@ -10,7 +10,7 @@ import ( "strings" "github.com/go-logr/logr" - klcv1alpha1 "github.com/keptn/lifecycle-toolkit/operator/api/v1alpha1" + kltv1alpha1 "github.com/keptn/lifecycle-toolkit/operator/api/v1alpha1" "github.com/keptn/lifecycle-toolkit/operator/api/v1alpha1/common" "github.com/keptn/lifecycle-toolkit/operator/api/v1alpha1/semconv" "go.opentelemetry.io/otel" @@ -81,7 +81,7 @@ func (a *PodMutatingWebhook) Handle(ctx context.Context, req admission.Request) podIsAnnotated, err := a.isPodAnnotated(pod) if err == nil && !podIsAnnotated { - _, _ = a.copyAnnotationsIfParentAnnotated(ctx, &req, pod) + _, err = a.copyAnnotationsIfParentAnnotated(ctx, &req, pod) } if err != nil { @@ -299,7 +299,7 @@ func (a *PodMutatingWebhook) handleWorkload(ctx context.Context, logger logr.Log logger.Info("Searching for workload") - workload := &klcv1alpha1.KeptnWorkload{} + workload := &kltv1alpha1.KeptnWorkload{} err := a.Client.Get(ctx, types.NamespacedName{Namespace: namespace, Name: newWorkload.Name}, workload) if errors.IsNotFound(err) { logger.Info("Creating workload", "workload", workload.Name) @@ -353,7 +353,7 @@ func (a *PodMutatingWebhook) handleApp(ctx context.Context, logger logr.Logger, logger.Info("Searching for app") - app := &klcv1alpha1.KeptnApp{} + app := &kltv1alpha1.KeptnApp{} err := a.Client.Get(ctx, types.NamespacedName{Namespace: namespace, Name: newApp.Name}, app) if errors.IsNotFound(err) { logger.Info("Creating app", "app", app.Name) @@ -396,7 +396,7 @@ func (a *PodMutatingWebhook) handleApp(ctx context.Context, logger logr.Logger, return nil } -func (a *PodMutatingWebhook) generateWorkload(ctx context.Context, pod *corev1.Pod, namespace string) *klcv1alpha1.KeptnWorkload { +func (a *PodMutatingWebhook) generateWorkload(ctx context.Context, pod *corev1.Pod, namespace string) *kltv1alpha1.KeptnWorkload { version, _ := getLabelOrAnnotation(&pod.ObjectMeta, common.VersionAnnotation, common.K8sRecommendedVersionAnnotations) applicationName, _ := getLabelOrAnnotation(&pod.ObjectMeta, common.AppAnnotation, common.K8sRecommendedAppAnnotations) @@ -426,13 +426,13 @@ func (a *PodMutatingWebhook) generateWorkload(ctx context.Context, pod *corev1.P traceContextCarrier := propagation.MapCarrier{} otel.GetTextMapPropagator().Inject(ctx, traceContextCarrier) - return &klcv1alpha1.KeptnWorkload{ + return &kltv1alpha1.KeptnWorkload{ ObjectMeta: metav1.ObjectMeta{ Name: a.getWorkloadName(pod), Namespace: namespace, Annotations: traceContextCarrier, }, - Spec: klcv1alpha1.KeptnWorkloadSpec{ + Spec: kltv1alpha1.KeptnWorkloadSpec{ AppName: applicationName, Version: version, ResourceReference: a.getReplicaSetOfPod(pod), @@ -444,7 +444,7 @@ func (a *PodMutatingWebhook) generateWorkload(ctx context.Context, pod *corev1.P } } -func (a *PodMutatingWebhook) generateApp(ctx context.Context, pod *corev1.Pod, namespace string) *klcv1alpha1.KeptnApp { +func (a *PodMutatingWebhook) generateApp(ctx context.Context, pod *corev1.Pod, namespace string) *kltv1alpha1.KeptnApp { version, _ := getLabelOrAnnotation(&pod.ObjectMeta, common.VersionAnnotation, common.K8sRecommendedVersionAnnotations) appName := a.getAppName(pod) @@ -453,19 +453,19 @@ func (a *PodMutatingWebhook) generateApp(ctx context.Context, pod *corev1.Pod, n traceContextCarrier := propagation.MapCarrier{} otel.GetTextMapPropagator().Inject(ctx, traceContextCarrier) - return &klcv1alpha1.KeptnApp{ + return &kltv1alpha1.KeptnApp{ ObjectMeta: metav1.ObjectMeta{ Name: appName, Namespace: namespace, Annotations: traceContextCarrier, }, - Spec: klcv1alpha1.KeptnAppSpec{ + Spec: kltv1alpha1.KeptnAppSpec{ Version: version, PreDeploymentTasks: []string{}, PostDeploymentTasks: []string{}, PreDeploymentEvaluations: []string{}, PostDeploymentEvaluations: []string{}, - Workloads: []klcv1alpha1.KeptnWorkloadRef{ + Workloads: []kltv1alpha1.KeptnWorkloadRef{ { Name: appName, Version: version, @@ -486,8 +486,8 @@ func (a *PodMutatingWebhook) getAppName(pod *corev1.Pod) string { return strings.ToLower(applicationName) } -func (a *PodMutatingWebhook) getReplicaSetOfPod(pod *corev1.Pod) klcv1alpha1.ResourceReference { - reference := klcv1alpha1.ResourceReference{ +func (a *PodMutatingWebhook) getReplicaSetOfPod(pod *corev1.Pod) kltv1alpha1.ResourceReference { + reference := kltv1alpha1.ResourceReference{ UID: pod.UID, Kind: pod.Kind, } @@ -502,8 +502,8 @@ func (a *PodMutatingWebhook) getReplicaSetOfPod(pod *corev1.Pod) klcv1alpha1.Res return reference } -func (a *PodMutatingWebhook) getOwnerOfReplicaSet(rs *appsv1.ReplicaSet) klcv1alpha1.ResourceReference { - reference := klcv1alpha1.ResourceReference{ +func (a *PodMutatingWebhook) getOwnerOfReplicaSet(rs *appsv1.ReplicaSet) kltv1alpha1.ResourceReference { + reference := kltv1alpha1.ResourceReference{ UID: rs.UID, Kind: rs.Kind, } @@ -540,3 +540,7 @@ func getLabelOrAnnotation(resource *metav1.ObjectMeta, primaryAnnotation string, } return "", false } + +func getResourceByUID[resourceType *appsv1.Deployment | *appsv1.StatefulSet | *appsv1.DaemonSet](resource resourceType, UID string) (resourceType, error) { + +} From 35273a0d8bd9b0bd92272e6a31bf2c3e611338f1 Mon Sep 17 00:00:00 2001 From: Moritz Wiesinger Date: Mon, 7 Nov 2022 13:56:01 +0100 Subject: [PATCH 09/41] small refactoring, adjust return values Signed-off-by: Moritz Wiesinger --- operator/webhooks/pod_mutating_webhook.go | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/operator/webhooks/pod_mutating_webhook.go b/operator/webhooks/pod_mutating_webhook.go index 77b79d7759..4d370aa958 100644 --- a/operator/webhooks/pod_mutating_webhook.go +++ b/operator/webhooks/pod_mutating_webhook.go @@ -81,7 +81,7 @@ func (a *PodMutatingWebhook) Handle(ctx context.Context, req admission.Request) podIsAnnotated, err := a.isPodAnnotated(pod) if err == nil && !podIsAnnotated { - _, err = a.copyAnnotationsIfParentAnnotated(ctx, &req, pod) + podIsAnnotated, err = a.copyAnnotationsIfParentAnnotated(ctx, &req, pod) } if err != nil { @@ -223,27 +223,28 @@ func (a *PodMutatingWebhook) copyAnnotationsIfParentAnnotated(ctx context.Contex } func (a *PodMutatingWebhook) copyResourceLabelsIfPresent(sourceResource *metav1.ObjectMeta, targetPod *corev1.Pod) (bool, error) { - var workload, version string + var workloadName, version string var gotWorkloadAnnotation, gotVersionAnnotation bool - workload, gotWorkloadAnnotation = getLabelOrAnnotation(sourceResource, common.WorkloadAnnotation, common.K8sRecommendedWorkloadAnnotations) + workloadName, gotWorkloadAnnotation = getLabelOrAnnotation(sourceResource, common.WorkloadAnnotation, common.K8sRecommendedWorkloadAnnotations) version, gotVersionAnnotation = getLabelOrAnnotation(sourceResource, common.VersionAnnotation, common.K8sRecommendedVersionAnnotations) - if len(workload) > common.MaxWorkloadNameLength || len(version) > common.MaxVersionLength { + if len(workloadName) > common.MaxWorkloadNameLength || len(version) > common.MaxVersionLength { return false, common.ErrTooLongAnnotations } if gotWorkloadAnnotation { + if len(targetPod.Annotations) == 0 { + targetPod.Annotations = make(map[string]string) + } + + targetPod.Annotations[common.WorkloadAnnotation] = sourceResource.Annotations[common.WorkloadAnnotation] + if !gotVersionAnnotation { - if len(targetPod.Annotations) == 0 { - targetPod.Annotations = make(map[string]string) - } targetPod.Annotations[common.VersionAnnotation] = a.calculateVersion(targetPod) } else { targetPod.Annotations[common.VersionAnnotation] = sourceResource.Annotations[common.VersionAnnotation] } - - targetPod.Annotations[common.WorkloadAnnotation] = sourceResource.Annotations[common.WorkloadAnnotation] return true, nil } return false, nil @@ -540,7 +541,3 @@ func getLabelOrAnnotation(resource *metav1.ObjectMeta, primaryAnnotation string, } return "", false } - -func getResourceByUID[resourceType *appsv1.Deployment | *appsv1.StatefulSet | *appsv1.DaemonSet](resource resourceType, UID string) (resourceType, error) { - -} From fe414847ac53b6f8086f84e46b7ee907975e8992 Mon Sep 17 00:00:00 2001 From: Moritz Wiesinger Date: Mon, 7 Nov 2022 13:58:47 +0100 Subject: [PATCH 10/41] remove unneeded initialization Signed-off-by: Moritz Wiesinger --- operator/webhooks/pod_mutating_webhook.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/operator/webhooks/pod_mutating_webhook.go b/operator/webhooks/pod_mutating_webhook.go index 4d370aa958..a3b691444d 100644 --- a/operator/webhooks/pod_mutating_webhook.go +++ b/operator/webhooks/pod_mutating_webhook.go @@ -234,10 +234,6 @@ func (a *PodMutatingWebhook) copyResourceLabelsIfPresent(sourceResource *metav1. } if gotWorkloadAnnotation { - if len(targetPod.Annotations) == 0 { - targetPod.Annotations = make(map[string]string) - } - targetPod.Annotations[common.WorkloadAnnotation] = sourceResource.Annotations[common.WorkloadAnnotation] if !gotVersionAnnotation { From 3debb0c7059425a22128ae6a29c44591fb942f79 Mon Sep 17 00:00:00 2001 From: Moritz Wiesinger Date: Mon, 7 Nov 2022 15:43:23 +0100 Subject: [PATCH 11/41] initialize map correctly Signed-off-by: Moritz Wiesinger --- operator/webhooks/pod_mutating_webhook.go | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/operator/webhooks/pod_mutating_webhook.go b/operator/webhooks/pod_mutating_webhook.go index a3b691444d..d5449723a4 100644 --- a/operator/webhooks/pod_mutating_webhook.go +++ b/operator/webhooks/pod_mutating_webhook.go @@ -32,6 +32,7 @@ import ( // +kubebuilder:webhook:path=/mutate-v1-pod,mutating=true,failurePolicy=fail,groups="",resources=pods,verbs=create;update,versions=v1,name=mpod.keptn.sh,admissionReviewVersions=v1,sideEffects=None //+kubebuilder:rbac:groups=core,resources=namespaces,verbs=get;list;watch +//+kubebuilder:rbac:groups=apps,resources=deployments;statefulsets;daemonsets,verbs=get;list;watch // PodMutatingWebhook annotates Pods type PodMutatingWebhook struct { @@ -79,8 +80,10 @@ func (a *PodMutatingWebhook) Handle(ctx context.Context, req admission.Request) logger.Info(fmt.Sprintf("Pod annotations: %v", pod.Annotations)) podIsAnnotated, err := a.isPodAnnotated(pod) + logger.Info("Checked if pod is annotated.") if err == nil && !podIsAnnotated { + logger.Info("Pod is not annotated, check for parent annotations...") podIsAnnotated, err = a.copyAnnotationsIfParentAnnotated(ctx, &req, pod) } @@ -158,6 +161,7 @@ func (a *PodMutatingWebhook) isPodAnnotated(pod *corev1.Pod) (bool, error) { func (a *PodMutatingWebhook) copyAnnotationsIfParentAnnotated(ctx context.Context, req *admission.Request, pod *corev1.Pod) (bool, error) { owner := a.getReplicaSetOfPod(pod) if owner.UID == pod.UID { + a.Log.Info("owner UID equals pod UID") return false, nil } @@ -174,6 +178,7 @@ func (a *PodMutatingWebhook) copyAnnotationsIfParentAnnotated(ctx context.Contex } } } + a.Log.Info("Done looking for RS") rsOwner := a.getOwnerOfReplicaSet(&rs) dpList := &appsv1.DeploymentList{} @@ -210,12 +215,16 @@ func (a *PodMutatingWebhook) copyAnnotationsIfParentAnnotated(ctx context.Contex break } } + a.Log.Info("Done looking for Parents of RS") if dp.UID == rsOwner.UID { + a.Log.Info("Copying from DP") return a.copyResourceLabelsIfPresent(&dp.ObjectMeta, pod) } else if sts.UID == rsOwner.UID { + a.Log.Info("Copying from STS") return a.copyResourceLabelsIfPresent(&sts.ObjectMeta, pod) } else if ds.UID == rsOwner.UID { + a.Log.Info("Copying from DS") return a.copyResourceLabelsIfPresent(&ds.ObjectMeta, pod) } else { return false, nil @@ -234,6 +243,10 @@ func (a *PodMutatingWebhook) copyResourceLabelsIfPresent(sourceResource *metav1. } if gotWorkloadAnnotation { + if len(targetPod.Annotations) == 0 { + targetPod.Annotations = make(map[string]string) + } + targetPod.Annotations[common.WorkloadAnnotation] = sourceResource.Annotations[common.WorkloadAnnotation] if !gotVersionAnnotation { @@ -241,6 +254,7 @@ func (a *PodMutatingWebhook) copyResourceLabelsIfPresent(sourceResource *metav1. } else { targetPod.Annotations[common.VersionAnnotation] = sourceResource.Annotations[common.VersionAnnotation] } + a.Log.Info("Copied stuff from DP") return true, nil } return false, nil From 6022a74e4da7aef08ca06306f42b403743b6a1c0 Mon Sep 17 00:00:00 2001 From: Moritz Wiesinger Date: Mon, 7 Nov 2022 16:21:29 +0100 Subject: [PATCH 12/41] add label copy for all pre/post deployment/evaluation labels Signed-off-by: Moritz Wiesinger --- operator/webhooks/pod_mutating_webhook.go | 39 +++++++++++++++++------ 1 file changed, 29 insertions(+), 10 deletions(-) diff --git a/operator/webhooks/pod_mutating_webhook.go b/operator/webhooks/pod_mutating_webhook.go index d5449723a4..a80c6f94e5 100644 --- a/operator/webhooks/pod_mutating_webhook.go +++ b/operator/webhooks/pod_mutating_webhook.go @@ -232,29 +232,48 @@ func (a *PodMutatingWebhook) copyAnnotationsIfParentAnnotated(ctx context.Contex } func (a *PodMutatingWebhook) copyResourceLabelsIfPresent(sourceResource *metav1.ObjectMeta, targetPod *corev1.Pod) (bool, error) { - var workloadName, version string - var gotWorkloadAnnotation, gotVersionAnnotation bool + var workloadName, version, preDeploymentChecks, postDeploymentChecks, preEvaluationChecks, postEvaluationChecks string + var gotWorkloadName, gotVersion, gotPreDeploymentChecks, gotPostDeploymentChecks, gotPreEvaluationChecks, gotPostEvaluationChecks bool - workloadName, gotWorkloadAnnotation = getLabelOrAnnotation(sourceResource, common.WorkloadAnnotation, common.K8sRecommendedWorkloadAnnotations) - version, gotVersionAnnotation = getLabelOrAnnotation(sourceResource, common.VersionAnnotation, common.K8sRecommendedVersionAnnotations) + workloadName, gotWorkloadName = getLabelOrAnnotation(sourceResource, common.WorkloadAnnotation, common.K8sRecommendedWorkloadAnnotations) + version, gotVersion = getLabelOrAnnotation(sourceResource, common.VersionAnnotation, common.K8sRecommendedVersionAnnotations) + preDeploymentChecks, gotPreDeploymentChecks = getLabelOrAnnotation(sourceResource, common.PreDeploymentTaskAnnotation, "") + postDeploymentChecks, gotPreDeploymentChecks = getLabelOrAnnotation(sourceResource, common.PostDeploymentTaskAnnotation, "") + preEvaluationChecks, gotPreEvaluationChecks = getLabelOrAnnotation(sourceResource, common.PreDeploymentEvaluationAnnotation, "") + postEvaluationChecks, gotPostEvaluationChecks = getLabelOrAnnotation(sourceResource, common.PostDeploymentEvaluationAnnotation, "") if len(workloadName) > common.MaxWorkloadNameLength || len(version) > common.MaxVersionLength { return false, common.ErrTooLongAnnotations } - if gotWorkloadAnnotation { - if len(targetPod.Annotations) == 0 { - targetPod.Annotations = make(map[string]string) - } + if len(targetPod.Annotations) == 0 { + targetPod.Annotations = make(map[string]string) + } + if gotWorkloadName { targetPod.Annotations[common.WorkloadAnnotation] = sourceResource.Annotations[common.WorkloadAnnotation] - if !gotVersionAnnotation { + if !gotVersion { targetPod.Annotations[common.VersionAnnotation] = a.calculateVersion(targetPod) } else { targetPod.Annotations[common.VersionAnnotation] = sourceResource.Annotations[common.VersionAnnotation] } - a.Log.Info("Copied stuff from DP") + + if gotPreDeploymentChecks { + targetPod.Annotations[common.PreDeploymentTaskAnnotation] = preDeploymentChecks + } + + if gotPostDeploymentChecks { + targetPod.Annotations[common.PostDeploymentTaskAnnotation] = postDeploymentChecks + } + + if gotPreEvaluationChecks { + targetPod.Annotations[common.PreDeploymentEvaluationAnnotation] = preEvaluationChecks + } + + if gotPostEvaluationChecks { + targetPod.Annotations[common.PostDeploymentEvaluationAnnotation] = postEvaluationChecks + } return true, nil } return false, nil From 0d07b5c58823a28b046552baa4aa9d3bd746c274 Mon Sep 17 00:00:00 2001 From: Moritz Wiesinger Date: Tue, 8 Nov 2022 08:27:04 +0100 Subject: [PATCH 13/41] add namespace to single service example Signed-off-by: Moritz Wiesinger --- examples/single-service/deploy.yaml | 4 ++-- examples/single-service/pre-deployment-tasks.yaml | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/examples/single-service/deploy.yaml b/examples/single-service/deploy.yaml index 128095792e..ee320411a6 100644 --- a/examples/single-service/deploy.yaml +++ b/examples/single-service/deploy.yaml @@ -2,7 +2,7 @@ apiVersion: v1 kind: Namespace metadata: - name: single-service + name: klt-test annotations: keptn.sh/lifecycle-toolkit: "enabled" --- @@ -13,7 +13,7 @@ metadata: labels: app: test name: test - namespace: single-service + namespace: klt-test spec: replicas: 1 selector: diff --git a/examples/single-service/pre-deployment-tasks.yaml b/examples/single-service/pre-deployment-tasks.yaml index 400cebe08a..05fd031d55 100644 --- a/examples/single-service/pre-deployment-tasks.yaml +++ b/examples/single-service/pre-deployment-tasks.yaml @@ -2,7 +2,7 @@ apiVersion: lifecycle.keptn.sh/v1alpha1 kind: KeptnTaskDefinition metadata: name: pre-deployment-hello - namespace: single-service + namespace: klt-test spec: function: inline: From d32f6c9ec22d740b67a0f327d2bb342a21880e8b Mon Sep 17 00:00:00 2001 From: Moritz Wiesinger Date: Tue, 8 Nov 2022 08:29:50 +0100 Subject: [PATCH 14/41] add permission to list apps.v1 resources to operator Signed-off-by: Moritz Wiesinger --- operator/config/rbac/role.yaml | 8 ++++++++ operator/webhooks/pod_mutating_webhook.go | 4 ++-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/operator/config/rbac/role.yaml b/operator/config/rbac/role.yaml index b95972b03f..d908e7703a 100644 --- a/operator/config/rbac/role.yaml +++ b/operator/config/rbac/role.yaml @@ -5,6 +5,14 @@ metadata: creationTimestamp: null name: manager-role rules: +- apiGroups: + - apps + resources: + - daemonsets + - deployments + - statefulsets + verbs: + - list - apiGroups: - apps resources: diff --git a/operator/webhooks/pod_mutating_webhook.go b/operator/webhooks/pod_mutating_webhook.go index a80c6f94e5..0eeb0c4d71 100644 --- a/operator/webhooks/pod_mutating_webhook.go +++ b/operator/webhooks/pod_mutating_webhook.go @@ -32,7 +32,7 @@ import ( // +kubebuilder:webhook:path=/mutate-v1-pod,mutating=true,failurePolicy=fail,groups="",resources=pods,verbs=create;update,versions=v1,name=mpod.keptn.sh,admissionReviewVersions=v1,sideEffects=None //+kubebuilder:rbac:groups=core,resources=namespaces,verbs=get;list;watch -//+kubebuilder:rbac:groups=apps,resources=deployments;statefulsets;daemonsets,verbs=get;list;watch +//+kubebuilder:rbac:groups=apps,resources=deployments;statefulsets;daemonsets,verbs=list // PodMutatingWebhook annotates Pods type PodMutatingWebhook struct { @@ -270,7 +270,7 @@ func (a *PodMutatingWebhook) copyResourceLabelsIfPresent(sourceResource *metav1. if gotPreEvaluationChecks { targetPod.Annotations[common.PreDeploymentEvaluationAnnotation] = preEvaluationChecks } - + if gotPostEvaluationChecks { targetPod.Annotations[common.PostDeploymentEvaluationAnnotation] = postEvaluationChecks } From 27f8c8ab1479f07ac8017b34cad56434fd54614c Mon Sep 17 00:00:00 2001 From: Moritz Wiesinger Date: Tue, 8 Nov 2022 11:39:57 +0100 Subject: [PATCH 15/41] add unit test file for webhook Signed-off-by: Moritz Wiesinger --- .../webhooks/pod_mutating_webhook_test.go | 54 +++++++++++++++++++ 1 file changed, 54 insertions(+) create mode 100644 operator/webhooks/pod_mutating_webhook_test.go diff --git a/operator/webhooks/pod_mutating_webhook_test.go b/operator/webhooks/pod_mutating_webhook_test.go new file mode 100644 index 0000000000..629dbd2ddf --- /dev/null +++ b/operator/webhooks/pod_mutating_webhook_test.go @@ -0,0 +1,54 @@ +package webhooks + +import ( + "github.com/go-logr/logr" + "go.opentelemetry.io/otel/trace" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/tools/record" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + "testing" +) + +func TestPodMutatingWebhook_copyResourceLabelsIfPresent(t *testing.T) { + type fields struct { + Client client.Client + Tracer trace.Tracer + decoder *admission.Decoder + Recorder record.EventRecorder + Log logr.Logger + } + type args struct { + sourceResource *metav1.ObjectMeta + targetPod *corev1.Pod + } + tests := []struct { + name string + fields fields + args args + want bool + wantErr bool + }{ + // TODO: Add test cases. + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + a := &PodMutatingWebhook{ + Client: tt.fields.Client, + Tracer: tt.fields.Tracer, + decoder: tt.fields.decoder, + Recorder: tt.fields.Recorder, + Log: tt.fields.Log, + } + got, err := a.copyResourceLabelsIfPresent(tt.args.sourceResource, tt.args.targetPod) + if (err != nil) != tt.wantErr { + t.Errorf("copyResourceLabelsIfPresent() error = %v, wantErr %v", err, tt.wantErr) + return + } + if got != tt.want { + t.Errorf("copyResourceLabelsIfPresent() got = %v, want %v", got, tt.want) + } + }) + } +} From 25669e20d6cfd53656093938cd6e1304088e4371 Mon Sep 17 00:00:00 2001 From: Moritz Wiesinger Date: Tue, 8 Nov 2022 12:52:09 +0100 Subject: [PATCH 16/41] first unit tests for webhook Signed-off-by: Moritz Wiesinger --- .../webhooks/pod_mutating_webhook_test.go | 148 ++++++++++++++++-- 1 file changed, 133 insertions(+), 15 deletions(-) diff --git a/operator/webhooks/pod_mutating_webhook_test.go b/operator/webhooks/pod_mutating_webhook_test.go index 629dbd2ddf..1426134eeb 100644 --- a/operator/webhooks/pod_mutating_webhook_test.go +++ b/operator/webhooks/pod_mutating_webhook_test.go @@ -2,16 +2,19 @@ package webhooks import ( "github.com/go-logr/logr" + "github.com/keptn/lifecycle-toolkit/operator/api/v1alpha1" "go.opentelemetry.io/otel/trace" + appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/tools/record" + "reflect" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" "testing" ) -func TestPodMutatingWebhook_copyResourceLabelsIfPresent(t *testing.T) { +func TestPodMutatingWebhook_getOwnerOfReplicaSet(t *testing.T) { type fields struct { Client client.Client Tracer trace.Tracer @@ -20,17 +23,58 @@ func TestPodMutatingWebhook_copyResourceLabelsIfPresent(t *testing.T) { Log logr.Logger } type args struct { - sourceResource *metav1.ObjectMeta - targetPod *corev1.Pod + rs *appsv1.ReplicaSet } tests := []struct { - name string - fields fields - args args - want bool - wantErr bool + name string + fields fields + args args + want v1alpha1.ResourceReference }{ - // TODO: Add test cases. + { + name: "Test simple return when UID and Kind is set", + fields: fields{}, + args: args{ + rs: &appsv1.ReplicaSet{ + ObjectMeta: metav1.ObjectMeta{ + OwnerReferences: []metav1.OwnerReference{ + { + Kind: "Deployment", + UID: "someUID-123456", + }, + }, + }, + }, + }, + want: v1alpha1.ResourceReference{ + UID: "someUID-123456", + Kind: "Deployment", + }, + }, + { + name: "Test return is input argument if owner is not found", + fields: fields{}, + args: args{ + rs: &appsv1.ReplicaSet{ + TypeMeta: metav1.TypeMeta{ + Kind: "ReplicaSet", + }, + ObjectMeta: metav1.ObjectMeta{ + UID: "replicaset-UID-abc123", + OwnerReferences: []metav1.OwnerReference{ + { + Kind: "SomeNonExistentType", + UID: "someUID-123456", + }, + }, + }, + }, + }, + want: v1alpha1.ResourceReference{ + Kind: "ReplicaSet", + UID: "replicaset-UID-abc123", + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -41,13 +85,87 @@ func TestPodMutatingWebhook_copyResourceLabelsIfPresent(t *testing.T) { Recorder: tt.fields.Recorder, Log: tt.fields.Log, } - got, err := a.copyResourceLabelsIfPresent(tt.args.sourceResource, tt.args.targetPod) - if (err != nil) != tt.wantErr { - t.Errorf("copyResourceLabelsIfPresent() error = %v, wantErr %v", err, tt.wantErr) - return + if got := a.getOwnerOfReplicaSet(tt.args.rs); !reflect.DeepEqual(got, tt.want) { + t.Errorf("getOwnerOfReplicaSet() = %v, want %v", got, tt.want) } - if got != tt.want { - t.Errorf("copyResourceLabelsIfPresent() got = %v, want %v", got, tt.want) + }) + } +} + +func TestPodMutatingWebhook_getReplicaSetOfPod(t *testing.T) { + type fields struct { + Client client.Client + Tracer trace.Tracer + decoder *admission.Decoder + Recorder record.EventRecorder + Log logr.Logger + } + type args struct { + pod *corev1.Pod + } + tests := []struct { + name string + fields fields + args args + want v1alpha1.ResourceReference + }{ + { + name: "Test simple return when UID and Kind is set", + fields: fields{}, + args: args{ + pod: &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + UID: "the-pod-uid", + OwnerReferences: []metav1.OwnerReference{ + { + Kind: "ReplicaSet", + UID: "the-replicaset-uid", + }, + }, + }, + }, + }, + want: v1alpha1.ResourceReference{ + UID: "the-replicaset-uid", + Kind: "ReplicaSet", + }, + }, + { + name: "Test return is input argument if owner is not found", + fields: fields{}, + args: args{ + pod: &corev1.Pod{ + TypeMeta: metav1.TypeMeta{ + Kind: "Pod", + }, + ObjectMeta: metav1.ObjectMeta{ + UID: "the-pod-uid", + OwnerReferences: []metav1.OwnerReference{ + { + Kind: "SomeNonExistentType", + UID: "the-replicaset-uid", + }, + }, + }, + }, + }, + want: v1alpha1.ResourceReference{ + UID: "the-pod-uid", + Kind: "Pod", + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + a := &PodMutatingWebhook{ + Client: tt.fields.Client, + Tracer: tt.fields.Tracer, + decoder: tt.fields.decoder, + Recorder: tt.fields.Recorder, + Log: tt.fields.Log, + } + if got := a.getReplicaSetOfPod(tt.args.pod); !reflect.DeepEqual(got, tt.want) { + t.Errorf("getReplicaSetOfPod() = %v, want %v", got, tt.want) } }) } From daed32026091cf7fd594264cee806b11253463fa Mon Sep 17 00:00:00 2001 From: Moritz Wiesinger Date: Tue, 8 Nov 2022 13:09:17 +0100 Subject: [PATCH 17/41] more tests Signed-off-by: Moritz Wiesinger --- .../webhooks/pod_mutating_webhook_test.go | 168 +++++++++++++++++- 1 file changed, 160 insertions(+), 8 deletions(-) diff --git a/operator/webhooks/pod_mutating_webhook_test.go b/operator/webhooks/pod_mutating_webhook_test.go index 1426134eeb..c30d50badf 100644 --- a/operator/webhooks/pod_mutating_webhook_test.go +++ b/operator/webhooks/pod_mutating_webhook_test.go @@ -32,8 +32,7 @@ func TestPodMutatingWebhook_getOwnerOfReplicaSet(t *testing.T) { want v1alpha1.ResourceReference }{ { - name: "Test simple return when UID and Kind is set", - fields: fields{}, + name: "Test simple return when UID and Kind is set", args: args{ rs: &appsv1.ReplicaSet{ ObjectMeta: metav1.ObjectMeta{ @@ -52,8 +51,7 @@ func TestPodMutatingWebhook_getOwnerOfReplicaSet(t *testing.T) { }, }, { - name: "Test return is input argument if owner is not found", - fields: fields{}, + name: "Test return is input argument if owner is not found", args: args{ rs: &appsv1.ReplicaSet{ TypeMeta: metav1.TypeMeta{ @@ -110,8 +108,7 @@ func TestPodMutatingWebhook_getReplicaSetOfPod(t *testing.T) { want v1alpha1.ResourceReference }{ { - name: "Test simple return when UID and Kind is set", - fields: fields{}, + name: "Test simple return when UID and Kind is set", args: args{ pod: &corev1.Pod{ ObjectMeta: metav1.ObjectMeta{ @@ -131,8 +128,7 @@ func TestPodMutatingWebhook_getReplicaSetOfPod(t *testing.T) { }, }, { - name: "Test return is input argument if owner is not found", - fields: fields{}, + name: "Test return is input argument if owner is not found", args: args{ pod: &corev1.Pod{ TypeMeta: metav1.TypeMeta{ @@ -170,3 +166,159 @@ func TestPodMutatingWebhook_getReplicaSetOfPod(t *testing.T) { }) } } + +func TestPodMutatingWebhook_getAppName(t *testing.T) { + type fields struct { + Client client.Client + Tracer trace.Tracer + decoder *admission.Decoder + Recorder record.EventRecorder + Log logr.Logger + } + type args struct { + pod *corev1.Pod + } + tests := []struct { + name string + fields fields + args args + want string + }{ + { + name: "Return keptn app name in lower case when annotation is set", + args: args{ + pod: &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + "keptn.sh/app": "SOME-APP-NAME", + }, + }, + }, + }, + want: "some-app-name", + }, + { + name: "Return keptn app name in lower case when label is set", + args: args{ + pod: &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + "keptn.sh/app": "SOME-APP-NAME", + }, + }, + }, + }, + want: "some-app-name", + }, + { + name: "Return keptn app name from annotation in lower case when annotation and label is set", + args: args{ + pod: &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + "keptn.sh/app": "SOME-APP-NAME-ANNOTATION", + }, + Labels: map[string]string{ + "keptn.sh/app": "SOME-APP-NAME-LABEL", + }, + }, + }, + }, + want: "some-app-name-annotation", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + a := &PodMutatingWebhook{ + Client: tt.fields.Client, + Tracer: tt.fields.Tracer, + decoder: tt.fields.decoder, + Recorder: tt.fields.Recorder, + Log: tt.fields.Log, + } + if got := a.getAppName(tt.args.pod); got != tt.want { + t.Errorf("getAppName() = %v, want %v", got, tt.want) + } + }) + } +} + +func TestPodMutatingWebhook_getWorkloadName(t *testing.T) { + type fields struct { + Client client.Client + Tracer trace.Tracer + decoder *admission.Decoder + Recorder record.EventRecorder + Log logr.Logger + } + type args struct { + pod *corev1.Pod + } + tests := []struct { + name string + fields fields + args args + want string + }{ + { + name: "Return concatenated app name and workload name in lower case when annotations are set", + args: args{ + pod: &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + "keptn.sh/app": "SOME-APP-NAME", + "keptn.sh/workload": "SOME-WORKLOAD-NAME", + }, + }, + }, + }, + want: "some-app-name-some-workload-name", + }, + { + name: "Return concatenated app name and workload name in lower case when labels are set", + args: args{ + pod: &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + "keptn.sh/app": "SOME-APP-NAME", + "keptn.sh/workload": "SOME-WORKLOAD-NAME", + }, + }, + }, + }, + want: "some-app-name-some-workload-name", + }, + { + name: "Return concatenated keptn app name and workload name from annotation in lower case when annotations and labels are set", + args: args{ + pod: &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + "keptn.sh/app": "SOME-APP-NAME-ANNOTATION", + "keptn.sh/workload": "SOME-WORKLOAD-NAME-ANNOTATION", + }, + Labels: map[string]string{ + "keptn.sh/app": "SOME-APP-NAME-LABEL", + "keptn.sh/workload": "SOME-WORKLOAD-NAME-LABEL", + }, + }, + }, + }, + want: "some-app-name-annotation-some-workload-name-annotation", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + a := &PodMutatingWebhook{ + Client: tt.fields.Client, + Tracer: tt.fields.Tracer, + decoder: tt.fields.decoder, + Recorder: tt.fields.Recorder, + Log: tt.fields.Log, + } + if got := a.getWorkloadName(tt.args.pod); got != tt.want { + t.Errorf("getWorkloadName() = %v, want %v", got, tt.want) + } + }) + } +} From 6c5bcdb7ebf85b00eb305524179b9ff31ee7b21f Mon Sep 17 00:00:00 2001 From: Moritz Wiesinger Date: Tue, 8 Nov 2022 13:26:59 +0100 Subject: [PATCH 18/41] even more tests Signed-off-by: Moritz Wiesinger --- .../webhooks/pod_mutating_webhook_test.go | 135 ++++++++++++++++-- 1 file changed, 123 insertions(+), 12 deletions(-) diff --git a/operator/webhooks/pod_mutating_webhook_test.go b/operator/webhooks/pod_mutating_webhook_test.go index c30d50badf..683f09801f 100644 --- a/operator/webhooks/pod_mutating_webhook_test.go +++ b/operator/webhooks/pod_mutating_webhook_test.go @@ -3,6 +3,7 @@ package webhooks import ( "github.com/go-logr/logr" "github.com/keptn/lifecycle-toolkit/operator/api/v1alpha1" + "github.com/keptn/lifecycle-toolkit/operator/api/v1alpha1/common" "go.opentelemetry.io/otel/trace" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" @@ -190,7 +191,7 @@ func TestPodMutatingWebhook_getAppName(t *testing.T) { pod: &corev1.Pod{ ObjectMeta: metav1.ObjectMeta{ Annotations: map[string]string{ - "keptn.sh/app": "SOME-APP-NAME", + common.AppAnnotation: "SOME-APP-NAME", }, }, }, @@ -203,7 +204,7 @@ func TestPodMutatingWebhook_getAppName(t *testing.T) { pod: &corev1.Pod{ ObjectMeta: metav1.ObjectMeta{ Labels: map[string]string{ - "keptn.sh/app": "SOME-APP-NAME", + common.AppAnnotation: "SOME-APP-NAME", }, }, }, @@ -216,10 +217,10 @@ func TestPodMutatingWebhook_getAppName(t *testing.T) { pod: &corev1.Pod{ ObjectMeta: metav1.ObjectMeta{ Annotations: map[string]string{ - "keptn.sh/app": "SOME-APP-NAME-ANNOTATION", + common.AppAnnotation: "SOME-APP-NAME-ANNOTATION", }, Labels: map[string]string{ - "keptn.sh/app": "SOME-APP-NAME-LABEL", + common.AppAnnotation: "SOME-APP-NAME-LABEL", }, }, }, @@ -266,8 +267,8 @@ func TestPodMutatingWebhook_getWorkloadName(t *testing.T) { pod: &corev1.Pod{ ObjectMeta: metav1.ObjectMeta{ Annotations: map[string]string{ - "keptn.sh/app": "SOME-APP-NAME", - "keptn.sh/workload": "SOME-WORKLOAD-NAME", + common.AppAnnotation: "SOME-APP-NAME", + common.WorkloadAnnotation: "SOME-WORKLOAD-NAME", }, }, }, @@ -280,8 +281,8 @@ func TestPodMutatingWebhook_getWorkloadName(t *testing.T) { pod: &corev1.Pod{ ObjectMeta: metav1.ObjectMeta{ Labels: map[string]string{ - "keptn.sh/app": "SOME-APP-NAME", - "keptn.sh/workload": "SOME-WORKLOAD-NAME", + common.AppAnnotation: "SOME-APP-NAME", + common.WorkloadAnnotation: "SOME-WORKLOAD-NAME", }, }, }, @@ -294,12 +295,12 @@ func TestPodMutatingWebhook_getWorkloadName(t *testing.T) { pod: &corev1.Pod{ ObjectMeta: metav1.ObjectMeta{ Annotations: map[string]string{ - "keptn.sh/app": "SOME-APP-NAME-ANNOTATION", - "keptn.sh/workload": "SOME-WORKLOAD-NAME-ANNOTATION", + common.AppAnnotation: "SOME-APP-NAME-ANNOTATION", + common.WorkloadAnnotation: "SOME-WORKLOAD-NAME-ANNOTATION", }, Labels: map[string]string{ - "keptn.sh/app": "SOME-APP-NAME-LABEL", - "keptn.sh/workload": "SOME-WORKLOAD-NAME-LABEL", + common.AppAnnotation: "SOME-APP-NAME-LABEL", + common.WorkloadAnnotation: "SOME-WORKLOAD-NAME-LABEL", }, }, }, @@ -322,3 +323,113 @@ func TestPodMutatingWebhook_getWorkloadName(t *testing.T) { }) } } + +func Test_getLabelOrAnnotation(t *testing.T) { + type args struct { + resource *metav1.ObjectMeta + primaryAnnotation string + secondaryAnnotation string + } + tests := []struct { + name string + args args + want string + want1 bool + }{ + { + name: "Test if primary annotation is returned from annotations", + args: args{ + resource: &metav1.ObjectMeta{ + Annotations: map[string]string{ + common.AppAnnotation: "some-app-name", + }, + }, + primaryAnnotation: common.AppAnnotation, + secondaryAnnotation: common.K8sRecommendedAppAnnotations, + }, + want: "some-app-name", + want1: true, + }, + { + name: "Test if secondary annotation is returned from annotations", + args: args{ + resource: &metav1.ObjectMeta{ + Annotations: map[string]string{ + common.K8sRecommendedAppAnnotations: "some-app-name", + }, + }, + primaryAnnotation: common.AppAnnotation, + secondaryAnnotation: common.K8sRecommendedAppAnnotations, + }, + want: "some-app-name", + want1: true, + }, + { + name: "Test if primary annotation is returned from labels", + args: args{ + resource: &metav1.ObjectMeta{ + Labels: map[string]string{ + common.AppAnnotation: "some-app-name", + }, + }, + primaryAnnotation: common.AppAnnotation, + secondaryAnnotation: common.K8sRecommendedAppAnnotations, + }, + want: "some-app-name", + want1: true, + }, + { + name: "Test if secondary annotation is returned from labels", + args: args{ + resource: &metav1.ObjectMeta{ + Labels: map[string]string{ + common.K8sRecommendedAppAnnotations: "some-app-name", + }, + }, + primaryAnnotation: common.AppAnnotation, + secondaryAnnotation: common.K8sRecommendedAppAnnotations, + }, + want: "some-app-name", + want1: true, + }, + { + name: "Test that empty string is returned when no annotations or labels are found", + args: args{ + resource: &metav1.ObjectMeta{ + Annotations: map[string]string{ + "some-other-annotation": "some-app-name", + }, + }, + primaryAnnotation: common.AppAnnotation, + secondaryAnnotation: common.K8sRecommendedAppAnnotations, + }, + want: "", + want1: false, + }, + { + name: "Test that empty string is returned when primary annotation cannot be found and secondary annotation is empty", + args: args{ + resource: &metav1.ObjectMeta{ + Annotations: map[string]string{ + "some-other-annotation": "some-app-name", + }, + }, + primaryAnnotation: common.AppAnnotation, + secondaryAnnotation: "", + }, + want: "", + want1: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, got1 := getLabelOrAnnotation(tt.args.resource, tt.args.primaryAnnotation, tt.args.secondaryAnnotation) + if got != tt.want { + t.Errorf("getLabelOrAnnotation() got = %v, want %v", got, tt.want) + } + if got1 != tt.want1 { + t.Errorf("getLabelOrAnnotation() got1 = %v, want %v", got1, tt.want1) + } + }) + } +} From 744291204dd18f9b0c5f5f0224250843aecf2449 Mon Sep 17 00:00:00 2001 From: Moritz Wiesinger Date: Tue, 8 Nov 2022 13:50:26 +0100 Subject: [PATCH 19/41] moooooooooooore Signed-off-by: Moritz Wiesinger --- .../webhooks/pod_mutating_webhook_test.go | 97 +++++++++++++++++++ 1 file changed, 97 insertions(+) diff --git a/operator/webhooks/pod_mutating_webhook_test.go b/operator/webhooks/pod_mutating_webhook_test.go index 683f09801f..db8539f923 100644 --- a/operator/webhooks/pod_mutating_webhook_test.go +++ b/operator/webhooks/pod_mutating_webhook_test.go @@ -433,3 +433,100 @@ func Test_getLabelOrAnnotation(t *testing.T) { }) } } + +func TestPodMutatingWebhook_isPodAnnotated(t *testing.T) { + type fields struct { + Client client.Client + Tracer trace.Tracer + decoder *admission.Decoder + Recorder record.EventRecorder + Log logr.Logger + } + type args struct { + pod *corev1.Pod + } + tests := []struct { + name string + fields fields + args args + want bool + wantErr bool + }{ + { + name: "Test error when workload name is too long", + args: args{ + pod: &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + common.AppAnnotation: "SOME-APP-NAME-ANNOTATION", + common.WorkloadAnnotation: "workload-name-that-is-too-loooooooooooooooooooooooooooooooooooooooooooooooooong", + }, + }, + }, + }, + want: false, + wantErr: true, + }, + { + name: "Test return true when pod has workload annotation", + args: args{ + pod: &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + common.WorkloadAnnotation: "some-workload-name", + }, + }, + }, + }, + want: true, + wantErr: false, + }, + { + name: "Test return true and initialize annotations when labels are set", + args: args{ + pod: &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + common.WorkloadAnnotation: "some-workload-name", + }, + }, + }, + }, + want: true, + wantErr: false, + }, + { + name: "Test return false when annotations and labels are not set", + args: args{ + pod: &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + "some-other-label": "some-value", + }, + }, + }, + }, + want: false, + wantErr: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + a := &PodMutatingWebhook{ + Client: tt.fields.Client, + Tracer: tt.fields.Tracer, + decoder: tt.fields.decoder, + Recorder: tt.fields.Recorder, + Log: tt.fields.Log, + } + got, err := a.isPodAnnotated(tt.args.pod) + if (err != nil) != tt.wantErr { + t.Errorf("isPodAnnotated() error = %v, wantErr %v", err, tt.wantErr) + return + } + if got != tt.want { + t.Errorf("isPodAnnotated() got = %v, want %v", got, tt.want) + } + }) + } +} From 872d7cb65843611317479ae4b509d0cc237aad38 Mon Sep 17 00:00:00 2001 From: Moritz Wiesinger Date: Tue, 8 Nov 2022 15:15:06 +0100 Subject: [PATCH 20/41] check if pod reference got updated correctly Signed-off-by: Moritz Wiesinger --- .../webhooks/pod_mutating_webhook_test.go | 39 ++++++++++++++++--- 1 file changed, 34 insertions(+), 5 deletions(-) diff --git a/operator/webhooks/pod_mutating_webhook_test.go b/operator/webhooks/pod_mutating_webhook_test.go index db8539f923..e304dbe032 100644 --- a/operator/webhooks/pod_mutating_webhook_test.go +++ b/operator/webhooks/pod_mutating_webhook_test.go @@ -4,6 +4,7 @@ import ( "github.com/go-logr/logr" "github.com/keptn/lifecycle-toolkit/operator/api/v1alpha1" "github.com/keptn/lifecycle-toolkit/operator/api/v1alpha1/common" + "github.com/stretchr/testify/require" "go.opentelemetry.io/otel/trace" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" @@ -446,11 +447,12 @@ func TestPodMutatingWebhook_isPodAnnotated(t *testing.T) { pod *corev1.Pod } tests := []struct { - name string - fields fields - args args - want bool - wantErr bool + name string + fields fields + args args + want bool + wantErr bool + wantedPod *corev1.Pod }{ { name: "Test error when workload name is too long", @@ -485,6 +487,13 @@ func TestPodMutatingWebhook_isPodAnnotated(t *testing.T) { name: "Test return true and initialize annotations when labels are set", args: args{ pod: &corev1.Pod{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Image: "some-image:v1", + }, + }, + }, ObjectMeta: metav1.ObjectMeta{ Labels: map[string]string{ common.WorkloadAnnotation: "some-workload-name", @@ -494,6 +503,23 @@ func TestPodMutatingWebhook_isPodAnnotated(t *testing.T) { }, want: true, wantErr: false, + wantedPod: &corev1.Pod{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Image: "some-image:v1", + }, + }, + }, + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + common.WorkloadAnnotation: "some-workload-name", + }, + Annotations: map[string]string{ + common.VersionAnnotation: "v1", + }, + }, + }, }, { name: "Test return false when annotations and labels are not set", @@ -527,6 +553,9 @@ func TestPodMutatingWebhook_isPodAnnotated(t *testing.T) { if got != tt.want { t.Errorf("isPodAnnotated() got = %v, want %v", got, tt.want) } + if tt.wantedPod != nil { + require.Equal(t, tt.wantedPod, tt.args.pod) + } }) } } From cb0166ff36d39892304c07f61a4d7f1698e056be Mon Sep 17 00:00:00 2001 From: Moritz Wiesinger Date: Wed, 9 Nov 2022 09:36:52 +0100 Subject: [PATCH 21/41] add error when parent rs of pod cannot be found Signed-off-by: Moritz Wiesinger --- operator/webhooks/pod_mutating_webhook.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/operator/webhooks/pod_mutating_webhook.go b/operator/webhooks/pod_mutating_webhook.go index 0eeb0c4d71..f5d7158b29 100644 --- a/operator/webhooks/pod_mutating_webhook.go +++ b/operator/webhooks/pod_mutating_webhook.go @@ -151,7 +151,6 @@ func (a *PodMutatingWebhook) isPodAnnotated(pod *corev1.Pod) (bool, error) { pod.Annotations = make(map[string]string) } pod.Annotations[common.VersionAnnotation] = a.calculateVersion(pod) - pod.GetAnnotations() } return true, nil } @@ -161,7 +160,7 @@ func (a *PodMutatingWebhook) isPodAnnotated(pod *corev1.Pod) (bool, error) { func (a *PodMutatingWebhook) copyAnnotationsIfParentAnnotated(ctx context.Context, req *admission.Request, pod *corev1.Pod) (bool, error) { owner := a.getReplicaSetOfPod(pod) if owner.UID == pod.UID { - a.Log.Info("owner UID equals pod UID") + a.Log.Info("owner UID equals pod UID") return false, nil } @@ -181,6 +180,11 @@ func (a *PodMutatingWebhook) copyAnnotationsIfParentAnnotated(ctx context.Contex a.Log.Info("Done looking for RS") rsOwner := a.getOwnerOfReplicaSet(&rs) + + if rsOwner.UID == "" { + return false, common.ErrReplicaSetNotFound + } + dpList := &appsv1.DeploymentList{} stsList := &appsv1.StatefulSetList{} dsList := &appsv1.DaemonSetList{} From 75bc13d45ecb03adaa0146eeb29e8c0a732d9e11 Mon Sep 17 00:00:00 2001 From: Moritz Wiesinger Date: Wed, 9 Nov 2022 09:38:04 +0100 Subject: [PATCH 22/41] typo Signed-off-by: Moritz Wiesinger --- operator/controllers/keptnapp/controller_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/operator/controllers/keptnapp/controller_test.go b/operator/controllers/keptnapp/controller_test.go index b2c0bf450d..c84dc2ed21 100644 --- a/operator/controllers/keptnapp/controller_test.go +++ b/operator/controllers/keptnapp/controller_test.go @@ -17,7 +17,7 @@ import ( "testing" ) -//EXample Unit test on help function +// Example Unit test on help function func TestKeptnAppReconciler_createAppVersionSuccess(t *testing.T) { app := &lfcv1alpha1.KeptnApp{ @@ -135,7 +135,6 @@ func setupReconciler(t *testing.T) (*KeptnAppReconciler, chan string, *fake.ITra fakeClient, err := fake.NewClient() if err != nil { t.Errorf("Reconcile() error when setting up fake client %v", err) - } recorder := record.NewFakeRecorder(100) r := &KeptnAppReconciler{ From eea85a771165882a72d9d1f52969bff782ae3596 Mon Sep 17 00:00:00 2001 From: Moritz Wiesinger Date: Wed, 9 Nov 2022 10:32:18 +0100 Subject: [PATCH 23/41] more tests and added error if rs of pod doesnt have an owner Signed-off-by: Moritz Wiesinger --- operator/webhooks/pod_mutating_webhook.go | 6 +- .../webhooks/pod_mutating_webhook_test.go | 302 +++++++++++++++++- 2 files changed, 302 insertions(+), 6 deletions(-) diff --git a/operator/webhooks/pod_mutating_webhook.go b/operator/webhooks/pod_mutating_webhook.go index f5d7158b29..e43fd6ff37 100644 --- a/operator/webhooks/pod_mutating_webhook.go +++ b/operator/webhooks/pod_mutating_webhook.go @@ -537,10 +537,8 @@ func (a *PodMutatingWebhook) getReplicaSetOfPod(pod *corev1.Pod) kltv1alpha1.Res } func (a *PodMutatingWebhook) getOwnerOfReplicaSet(rs *appsv1.ReplicaSet) kltv1alpha1.ResourceReference { - reference := kltv1alpha1.ResourceReference{ - UID: rs.UID, - Kind: rs.Kind, - } + reference := kltv1alpha1.ResourceReference{} + if len(rs.OwnerReferences) != 0 { for _, o := range rs.OwnerReferences { if o.Kind == "Deployment" || o.Kind == "StatefulSet" || o.Kind == "DaemonSet" { diff --git a/operator/webhooks/pod_mutating_webhook_test.go b/operator/webhooks/pod_mutating_webhook_test.go index e304dbe032..1172e19b4c 100644 --- a/operator/webhooks/pod_mutating_webhook_test.go +++ b/operator/webhooks/pod_mutating_webhook_test.go @@ -1,14 +1,19 @@ package webhooks import ( + "context" "github.com/go-logr/logr" + "github.com/go-logr/logr/testr" "github.com/keptn/lifecycle-toolkit/operator/api/v1alpha1" "github.com/keptn/lifecycle-toolkit/operator/api/v1alpha1/common" + "github.com/keptn/lifecycle-toolkit/operator/controllers/common/fake" "github.com/stretchr/testify/require" "go.opentelemetry.io/otel/trace" + admissionv1 "k8s.io/api/admission/v1" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/tools/record" "reflect" "sigs.k8s.io/controller-runtime/pkg/client" @@ -71,8 +76,8 @@ func TestPodMutatingWebhook_getOwnerOfReplicaSet(t *testing.T) { }, }, want: v1alpha1.ResourceReference{ - Kind: "ReplicaSet", - UID: "replicaset-UID-abc123", + Kind: "", + UID: "", }, }, } @@ -559,3 +564,296 @@ func TestPodMutatingWebhook_isPodAnnotated(t *testing.T) { }) } } + +func TestPodMutatingWebhook_copyAnnotationsIfParentAnnotated(t *testing.T) { + testNamespace := "test-namespace" + rsUidWithDpOwner := types.UID("this-is-the-replicaset-with-dp-owner") + rsUidWithStsOwner := types.UID("this-is-the-replicaset-with-sts-owner") + rsUidWithDsOwner := types.UID("this-is-the-replicaset-with-ds-owner") + rsUidWithNoOwner := types.UID("this-is-the-replicaset-with-no-owner") + + fakeClient, err := fake.NewClient() + + if err != nil { + t.Errorf("Error when setting up fake client %v", err) + } + + rsWithDpOwner := &appsv1.ReplicaSet{ + TypeMeta: metav1.TypeMeta{ + Kind: "ReplicaSet", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "test-replicaset1", + UID: rsUidWithDpOwner, + Namespace: testNamespace, + OwnerReferences: []metav1.OwnerReference{ + { + Kind: "Deployment", + Name: "this-is-the-deployment", + UID: "this-is-the-deployment-uid", + }, + }, + }, + } + rsWithStsOwner := &appsv1.ReplicaSet{ + TypeMeta: metav1.TypeMeta{ + Kind: "ReplicaSet", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "test-replicaset2", + UID: rsUidWithStsOwner, + Namespace: testNamespace, + OwnerReferences: []metav1.OwnerReference{ + { + Kind: "Deployment", + Name: "this-is-the-deployment", + UID: "this-is-the-stateful-set-uid", + }, + }, + }, + } + rsWithDsOwner := &appsv1.ReplicaSet{ + TypeMeta: metav1.TypeMeta{ + Kind: "ReplicaSet", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "test-replicaset3", + UID: rsUidWithDsOwner, + Namespace: testNamespace, + OwnerReferences: []metav1.OwnerReference{ + { + Kind: "Deployment", + Name: "this-is-the-deployment", + UID: "this-is-the-daemonset-uid", + }, + }, + }, + } + testRsWithNoOwner := &appsv1.ReplicaSet{ + TypeMeta: metav1.TypeMeta{ + Kind: "ReplicaSet", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "test-replicaset4", + UID: rsUidWithNoOwner, + Namespace: testNamespace, + }, + } + testDp := &appsv1.Deployment{ + TypeMeta: metav1.TypeMeta{ + Kind: "Deployment", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "test-deployment", + UID: "this-is-the-deployment-uid", + Namespace: testNamespace, + }, + } + testSts := &appsv1.StatefulSet{ + TypeMeta: metav1.TypeMeta{ + Kind: "StatefulSet", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "test-stateful-set", + UID: "this-is-the-stateful-set-uid", + Namespace: testNamespace, + }, + } + testDs := &appsv1.DaemonSet{ + TypeMeta: metav1.TypeMeta{ + Kind: "DaemonSet", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "test-daemonset", + UID: "this-is-the-daemonset-uid", + Namespace: testNamespace, + }, + } + + err = fakeClient.Create(context.TODO(), rsWithDpOwner) + err = fakeClient.Create(context.TODO(), rsWithStsOwner) + err = fakeClient.Create(context.TODO(), rsWithDsOwner) + err = fakeClient.Create(context.TODO(), testRsWithNoOwner) + err = fakeClient.Create(context.TODO(), testDp) + err = fakeClient.Create(context.TODO(), testSts) + err = fakeClient.Create(context.TODO(), testDs) + + if err != nil { + t.Errorf("Error when creating objects in fake client %v", err) + } + + type fields struct { + Client client.Client + Tracer trace.Tracer + decoder *admission.Decoder + Recorder record.EventRecorder + Log logr.Logger + } + type args struct { + ctx context.Context + req *admission.Request + pod *corev1.Pod + } + tests := []struct { + name string + fields fields + args args + want bool + wantErr bool + }{ + { + name: "Test that nothing happens if owner UID is pod UID", + fields: fields{ + Log: testr.New(t), + }, + args: args{ + ctx: context.TODO(), + req: nil, + pod: &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + UID: "some-uid", + OwnerReferences: []metav1.OwnerReference{ + { + UID: "some-uid", + Kind: "ReplicaSet", + }, + }, + }, + }, + }, + want: false, + wantErr: false, + }, + { + name: "Test fetching of replicaset owner of pod and deployment owner of replicaset", + fields: fields{ + Log: testr.New(t), + Client: fakeClient, + }, + args: args{ + ctx: context.TODO(), + req: &admission.Request{ + AdmissionRequest: admissionv1.AdmissionRequest{ + Namespace: testNamespace, + }, + }, + pod: &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + UID: "this-is-the-pod-uid", + OwnerReferences: []metav1.OwnerReference{ + { + UID: rsUidWithDpOwner, + Kind: "ReplicaSet", + }, + }, + }, + }, + }, + want: false, + wantErr: false, + }, + { + name: "Test fetching of replicaset owner of pod and statefulset owner of replicaset", + fields: fields{ + Log: testr.New(t), + Client: fakeClient, + }, + args: args{ + ctx: context.TODO(), + req: &admission.Request{ + AdmissionRequest: admissionv1.AdmissionRequest{ + Namespace: testNamespace, + }, + }, + pod: &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + UID: "this-is-the-pod-uid", + OwnerReferences: []metav1.OwnerReference{ + { + UID: rsUidWithStsOwner, + Kind: "ReplicaSet", + }, + }, + }, + }, + }, + want: false, + wantErr: false, + }, + { + name: "Test fetching of replicaset owner of pod and daemonset owner of replicaset", + fields: fields{ + Log: testr.New(t), + Client: fakeClient, + }, + args: args{ + ctx: context.TODO(), + req: &admission.Request{ + AdmissionRequest: admissionv1.AdmissionRequest{ + Namespace: testNamespace, + }, + }, + pod: &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + UID: "this-is-the-pod-uid", + OwnerReferences: []metav1.OwnerReference{ + { + UID: rsUidWithDsOwner, + Kind: "ReplicaSet", + }, + }, + }, + }, + }, + want: false, + wantErr: false, + }, + { + name: "Test that error is return when we get a pod with replicaset without owner", + fields: fields{ + Log: testr.New(t), + Client: fakeClient, + }, + args: args{ + ctx: context.TODO(), + req: &admission.Request{ + AdmissionRequest: admissionv1.AdmissionRequest{ + Namespace: testNamespace, + }, + }, + pod: &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + UID: "this-is-the-pod-uid", + OwnerReferences: []metav1.OwnerReference{ + { + UID: rsUidWithNoOwner, + Kind: "ReplicaSet", + }, + }, + }, + }, + }, + want: false, + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + a := &PodMutatingWebhook{ + Client: tt.fields.Client, + Tracer: tt.fields.Tracer, + decoder: tt.fields.decoder, + Recorder: tt.fields.Recorder, + Log: tt.fields.Log, + } + got, err := a.copyAnnotationsIfParentAnnotated(tt.args.ctx, tt.args.req, tt.args.pod) + if (err != nil) != tt.wantErr { + t.Errorf("copyAnnotationsIfParentAnnotated() error = %v, wantErr %v", err, tt.wantErr) + return + } + if got != tt.want { + t.Errorf("copyAnnotationsIfParentAnnotated() got = %v, want %v", got, tt.want) + } + }) + } +} From 8a95ee3f54df80a623da9dc577ac5e1c533ebe00 Mon Sep 17 00:00:00 2001 From: Moritz Wiesinger Date: Wed, 9 Nov 2022 10:50:29 +0100 Subject: [PATCH 24/41] fix behaviour when no owner is found Signed-off-by: Moritz Wiesinger --- operator/webhooks/pod_mutating_webhook.go | 2 +- operator/webhooks/pod_mutating_webhook_test.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/operator/webhooks/pod_mutating_webhook.go b/operator/webhooks/pod_mutating_webhook.go index e43fd6ff37..f7bb864e55 100644 --- a/operator/webhooks/pod_mutating_webhook.go +++ b/operator/webhooks/pod_mutating_webhook.go @@ -182,7 +182,7 @@ func (a *PodMutatingWebhook) copyAnnotationsIfParentAnnotated(ctx context.Contex rsOwner := a.getOwnerOfReplicaSet(&rs) if rsOwner.UID == "" { - return false, common.ErrReplicaSetNotFound + return false, nil } dpList := &appsv1.DeploymentList{} diff --git a/operator/webhooks/pod_mutating_webhook_test.go b/operator/webhooks/pod_mutating_webhook_test.go index 1172e19b4c..32cf220c28 100644 --- a/operator/webhooks/pod_mutating_webhook_test.go +++ b/operator/webhooks/pod_mutating_webhook_test.go @@ -809,7 +809,7 @@ func TestPodMutatingWebhook_copyAnnotationsIfParentAnnotated(t *testing.T) { wantErr: false, }, { - name: "Test that error is return when we get a pod with replicaset without owner", + name: "Test that method returns without doing anything when we get a pod with replicaset without owner", fields: fields{ Log: testr.New(t), Client: fakeClient, @@ -834,7 +834,7 @@ func TestPodMutatingWebhook_copyAnnotationsIfParentAnnotated(t *testing.T) { }, }, want: false, - wantErr: true, + wantErr: false, }, } for _, tt := range tests { From 37c3e482e346e969f5dd2f73383405a52c05b241 Mon Sep 17 00:00:00 2001 From: Moritz Wiesinger Date: Wed, 9 Nov 2022 11:21:50 +0100 Subject: [PATCH 25/41] more tests, also copy over app name Signed-off-by: Moritz Wiesinger --- operator/webhooks/pod_mutating_webhook.go | 15 +- .../webhooks/pod_mutating_webhook_test.go | 184 ++++++++++++++++++ 2 files changed, 194 insertions(+), 5 deletions(-) diff --git a/operator/webhooks/pod_mutating_webhook.go b/operator/webhooks/pod_mutating_webhook.go index f7bb864e55..9cfb71dd8d 100644 --- a/operator/webhooks/pod_mutating_webhook.go +++ b/operator/webhooks/pod_mutating_webhook.go @@ -236,13 +236,14 @@ func (a *PodMutatingWebhook) copyAnnotationsIfParentAnnotated(ctx context.Contex } func (a *PodMutatingWebhook) copyResourceLabelsIfPresent(sourceResource *metav1.ObjectMeta, targetPod *corev1.Pod) (bool, error) { - var workloadName, version, preDeploymentChecks, postDeploymentChecks, preEvaluationChecks, postEvaluationChecks string - var gotWorkloadName, gotVersion, gotPreDeploymentChecks, gotPostDeploymentChecks, gotPreEvaluationChecks, gotPostEvaluationChecks bool + var workloadName, appName, version, preDeploymentChecks, postDeploymentChecks, preEvaluationChecks, postEvaluationChecks string + var gotWorkloadName, gotAppName, gotVersion, gotPreDeploymentChecks, gotPostDeploymentChecks, gotPreEvaluationChecks, gotPostEvaluationChecks bool workloadName, gotWorkloadName = getLabelOrAnnotation(sourceResource, common.WorkloadAnnotation, common.K8sRecommendedWorkloadAnnotations) + appName, gotAppName = getLabelOrAnnotation(sourceResource, common.AppAnnotation, common.K8sRecommendedAppAnnotations) version, gotVersion = getLabelOrAnnotation(sourceResource, common.VersionAnnotation, common.K8sRecommendedVersionAnnotations) preDeploymentChecks, gotPreDeploymentChecks = getLabelOrAnnotation(sourceResource, common.PreDeploymentTaskAnnotation, "") - postDeploymentChecks, gotPreDeploymentChecks = getLabelOrAnnotation(sourceResource, common.PostDeploymentTaskAnnotation, "") + postDeploymentChecks, gotPostDeploymentChecks = getLabelOrAnnotation(sourceResource, common.PostDeploymentTaskAnnotation, "") preEvaluationChecks, gotPreEvaluationChecks = getLabelOrAnnotation(sourceResource, common.PreDeploymentEvaluationAnnotation, "") postEvaluationChecks, gotPostEvaluationChecks = getLabelOrAnnotation(sourceResource, common.PostDeploymentEvaluationAnnotation, "") @@ -255,12 +256,16 @@ func (a *PodMutatingWebhook) copyResourceLabelsIfPresent(sourceResource *metav1. } if gotWorkloadName { - targetPod.Annotations[common.WorkloadAnnotation] = sourceResource.Annotations[common.WorkloadAnnotation] + targetPod.Annotations[common.WorkloadAnnotation] = workloadName if !gotVersion { targetPod.Annotations[common.VersionAnnotation] = a.calculateVersion(targetPod) } else { - targetPod.Annotations[common.VersionAnnotation] = sourceResource.Annotations[common.VersionAnnotation] + targetPod.Annotations[common.VersionAnnotation] = version + } + + if gotAppName { + targetPod.Annotations[common.AppAnnotation] = appName } if gotPreDeploymentChecks { diff --git a/operator/webhooks/pod_mutating_webhook_test.go b/operator/webhooks/pod_mutating_webhook_test.go index 32cf220c28..4837d9adf0 100644 --- a/operator/webhooks/pod_mutating_webhook_test.go +++ b/operator/webhooks/pod_mutating_webhook_test.go @@ -857,3 +857,187 @@ func TestPodMutatingWebhook_copyAnnotationsIfParentAnnotated(t *testing.T) { }) } } + +func TestPodMutatingWebhook_copyResourceLabelsIfPresent(t *testing.T) { + type fields struct { + Client client.Client + Tracer trace.Tracer + decoder *admission.Decoder + Recorder record.EventRecorder + Log logr.Logger + } + type args struct { + sourceResource *metav1.ObjectMeta + targetPod *corev1.Pod + } + tests := []struct { + name string + fields fields + args args + want bool + wantErr bool + wantedPod *corev1.Pod + }{ + { + name: "Test that annotations get copied from source to target", + args: args{ + sourceResource: &metav1.ObjectMeta{ + Name: "testSourceObject", + Annotations: map[string]string{ + common.WorkloadAnnotation: "some-workload-name", + common.AppAnnotation: "some-app-name", + common.VersionAnnotation: "v1.0.0", + common.PreDeploymentTaskAnnotation: "some-pre-deployment-task", + common.PostDeploymentTaskAnnotation: "some-post-deployment-task", + common.PreDeploymentEvaluationAnnotation: "some-pre-deployment-evaluation", + common.PostDeploymentEvaluationAnnotation: "some-post-deployment-evaluation", + }, + }, + targetPod: &corev1.Pod{ + TypeMeta: metav1.TypeMeta{}, + ObjectMeta: metav1.ObjectMeta{}, + Spec: corev1.PodSpec{}, + Status: corev1.PodStatus{}, + }, + }, + want: true, + wantErr: false, + wantedPod: &corev1.Pod{ + TypeMeta: metav1.TypeMeta{}, + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + common.WorkloadAnnotation: "some-workload-name", + common.AppAnnotation: "some-app-name", + common.VersionAnnotation: "v1.0.0", + common.PreDeploymentTaskAnnotation: "some-pre-deployment-task", + common.PostDeploymentTaskAnnotation: "some-post-deployment-task", + common.PreDeploymentEvaluationAnnotation: "some-pre-deployment-evaluation", + common.PostDeploymentEvaluationAnnotation: "some-post-deployment-evaluation", + }, + }, + }, + }, + { + name: "Test that source labels get copied to target annotations", + args: args{ + sourceResource: &metav1.ObjectMeta{ + Name: "testSourceObject", + Labels: map[string]string{ + common.WorkloadAnnotation: "some-workload-name", + common.AppAnnotation: "some-app-name", + common.VersionAnnotation: "v1.0.0", + common.PreDeploymentTaskAnnotation: "some-pre-deployment-task", + common.PostDeploymentTaskAnnotation: "some-post-deployment-task", + common.PreDeploymentEvaluationAnnotation: "some-pre-deployment-evaluation", + common.PostDeploymentEvaluationAnnotation: "some-post-deployment-evaluation", + }, + }, + targetPod: &corev1.Pod{}, + }, + want: true, + wantErr: false, + wantedPod: &corev1.Pod{ + TypeMeta: metav1.TypeMeta{}, + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + common.WorkloadAnnotation: "some-workload-name", + common.AppAnnotation: "some-app-name", + common.VersionAnnotation: "v1.0.0", + common.PreDeploymentTaskAnnotation: "some-pre-deployment-task", + common.PostDeploymentTaskAnnotation: "some-post-deployment-task", + common.PreDeploymentEvaluationAnnotation: "some-pre-deployment-evaluation", + common.PostDeploymentEvaluationAnnotation: "some-post-deployment-evaluation", + }, + }, + }, + }, + { + name: "Test that version label is generated correctly and rest is copied", + args: args{ + sourceResource: &metav1.ObjectMeta{ + Name: "testSourceObject", + Labels: map[string]string{ + common.WorkloadAnnotation: "some-workload-name", + common.AppAnnotation: "some-app-name", + common.PreDeploymentTaskAnnotation: "some-pre-deployment-task", + common.PostDeploymentTaskAnnotation: "some-post-deployment-task", + common.PreDeploymentEvaluationAnnotation: "some-pre-deployment-evaluation", + common.PostDeploymentEvaluationAnnotation: "some-post-deployment-evaluation", + }, + }, + targetPod: &corev1.Pod{ + TypeMeta: metav1.TypeMeta{}, + ObjectMeta: metav1.ObjectMeta{}, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Image: "some-image:v1.0.0", + }, + }, + }, + Status: corev1.PodStatus{}, + }, + }, + want: true, + wantErr: false, + wantedPod: &corev1.Pod{ + TypeMeta: metav1.TypeMeta{}, + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + common.WorkloadAnnotation: "some-workload-name", + common.AppAnnotation: "some-app-name", + common.VersionAnnotation: "v1.0.0", + common.PreDeploymentTaskAnnotation: "some-pre-deployment-task", + common.PostDeploymentTaskAnnotation: "some-post-deployment-task", + common.PreDeploymentEvaluationAnnotation: "some-pre-deployment-evaluation", + common.PostDeploymentEvaluationAnnotation: "some-post-deployment-evaluation", + }, + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Image: "some-image:v1.0.0", + }, + }, + }, + Status: corev1.PodStatus{}, + }, + }, + { + name: "Test that error is return with too long workload name", + args: args{ + sourceResource: &metav1.ObjectMeta{ + Name: "testSourceObject", + Labels: map[string]string{ + common.WorkloadAnnotation: "some-workload-name-that-is-very-looooooooooooooooooooooong", + }, + }, + targetPod: &corev1.Pod{}, + }, + want: false, + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + a := &PodMutatingWebhook{ + Client: tt.fields.Client, + Tracer: tt.fields.Tracer, + decoder: tt.fields.decoder, + Recorder: tt.fields.Recorder, + Log: tt.fields.Log, + } + got, err := a.copyResourceLabelsIfPresent(tt.args.sourceResource, tt.args.targetPod) + if (err != nil) != tt.wantErr { + t.Errorf("copyResourceLabelsIfPresent() error = %v, wantErr %v", err, tt.wantErr) + return + } + if got != tt.want { + t.Errorf("copyResourceLabelsIfPresent() got = %v, want %v", got, tt.want) + } + if tt.wantedPod != nil { + require.Equal(t, tt.wantedPod, tt.args.targetPod) + } + }) + } +} From 4a1b565b3f121589da46d526eb5b60bf1a1ac105 Mon Sep 17 00:00:00 2001 From: Moritz Wiesinger Date: Wed, 9 Nov 2022 11:30:46 +0100 Subject: [PATCH 26/41] fix rebase artifact Signed-off-by: Moritz Wiesinger --- operator/webhooks/pod_mutating_webhook.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/operator/webhooks/pod_mutating_webhook.go b/operator/webhooks/pod_mutating_webhook.go index 9cfb71dd8d..3bb1ddabd1 100644 --- a/operator/webhooks/pod_mutating_webhook.go +++ b/operator/webhooks/pod_mutating_webhook.go @@ -248,7 +248,7 @@ func (a *PodMutatingWebhook) copyResourceLabelsIfPresent(sourceResource *metav1. postEvaluationChecks, gotPostEvaluationChecks = getLabelOrAnnotation(sourceResource, common.PostDeploymentEvaluationAnnotation, "") if len(workloadName) > common.MaxWorkloadNameLength || len(version) > common.MaxVersionLength { - return false, common.ErrTooLongAnnotations + return false, ErrTooLongAnnotations } if len(targetPod.Annotations) == 0 { From 4ab3b212e0cd9a4d4317a1bf8f458533b6767023 Mon Sep 17 00:00:00 2001 From: Moritz Wiesinger Date: Wed, 9 Nov 2022 12:51:51 +0100 Subject: [PATCH 27/41] more tests Signed-off-by: Moritz Wiesinger --- .../webhooks/pod_mutating_webhook_test.go | 102 ++++++++++++++++++ 1 file changed, 102 insertions(+) diff --git a/operator/webhooks/pod_mutating_webhook_test.go b/operator/webhooks/pod_mutating_webhook_test.go index 4837d9adf0..56c25f30e6 100644 --- a/operator/webhooks/pod_mutating_webhook_test.go +++ b/operator/webhooks/pod_mutating_webhook_test.go @@ -1041,3 +1041,105 @@ func TestPodMutatingWebhook_copyResourceLabelsIfPresent(t *testing.T) { }) } } + +func TestPodMutatingWebhook_isAppAnnotationPresent(t *testing.T) { + type fields struct { + Client client.Client + Tracer trace.Tracer + decoder *admission.Decoder + Recorder record.EventRecorder + Log logr.Logger + } + type args struct { + pod *corev1.Pod + } + tests := []struct { + name string + fields fields + args args + want bool + wantErr bool + wantedPod *corev1.Pod + }{ + { + name: "Test return true when app annotation is present", + args: args{ + pod: &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + common.AppAnnotation: "some-app-name", + }, + }, + }, + }, + want: true, + wantErr: false, + }, + { + name: "Test return false when app annotation is not present", + args: args{ + pod: &corev1.Pod{}, + }, + want: false, + wantErr: false, + }, + { + name: "Test return error when app annotation is too long", + args: args{ + pod: &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + common.AppAnnotation: "some-app-annotation-that-is-very-looooooooooooooooooooong", + }, + }, + }, + }, + want: false, + wantErr: true, + }, + { + name: "Test that app name is copied when only workload name is present", + args: args{ + pod: &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + common.WorkloadAnnotation: "some-workload-name", + }, + }, + }, + }, + want: false, + wantErr: false, + wantedPod: &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + common.AppAnnotation: "some-workload-name", + common.WorkloadAnnotation: "some-workload-name", + }, + }, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + a := &PodMutatingWebhook{ + Client: tt.fields.Client, + Tracer: tt.fields.Tracer, + decoder: tt.fields.decoder, + Recorder: tt.fields.Recorder, + Log: tt.fields.Log, + } + got, err := a.isAppAnnotationPresent(tt.args.pod) + if (err != nil) != tt.wantErr { + t.Errorf("isAppAnnotationPresent() error = %v, wantErr %v", err, tt.wantErr) + return + } + if got != tt.want { + t.Errorf("isAppAnnotationPresent() got = %v, want %v", got, tt.want) + } + if tt.wantedPod != nil { + require.Equal(t, tt.wantedPod, tt.args.pod) + } + }) + } +} From b4de62d91dd8eb32dc15aaa4eacdc239f9971e55 Mon Sep 17 00:00:00 2001 From: Moritz Wiesinger Date: Wed, 9 Nov 2022 13:52:20 +0100 Subject: [PATCH 28/41] optimise finding of owner a little bit Signed-off-by: Moritz Wiesinger --- operator/webhooks/pod_mutating_webhook.go | 39 +++++++---------------- 1 file changed, 12 insertions(+), 27 deletions(-) diff --git a/operator/webhooks/pod_mutating_webhook.go b/operator/webhooks/pod_mutating_webhook.go index 3bb1ddabd1..2077042e1d 100644 --- a/operator/webhooks/pod_mutating_webhook.go +++ b/operator/webhooks/pod_mutating_webhook.go @@ -186,53 +186,38 @@ func (a *PodMutatingWebhook) copyAnnotationsIfParentAnnotated(ctx context.Contex } dpList := &appsv1.DeploymentList{} - stsList := &appsv1.StatefulSetList{} - dsList := &appsv1.DaemonSetList{} - if err := a.Client.List(ctx, dpList, client.InNamespace(req.Namespace)); err != nil { return false, nil } - if err := a.Client.List(ctx, stsList, client.InNamespace(req.Namespace)); err != nil { - return false, nil - } - if err := a.Client.List(ctx, dsList, client.InNamespace(req.Namespace)); err != nil { - return false, nil - } - dp := appsv1.Deployment{} for _, dp = range dpList.Items { if dp.UID == rsOwner.UID { - break + return a.copyResourceLabelsIfPresent(&dp.ObjectMeta, pod) } } + stsList := &appsv1.StatefulSetList{} + if err := a.Client.List(ctx, stsList, client.InNamespace(req.Namespace)); err != nil { + return false, nil + } sts := appsv1.StatefulSet{} for _, sts = range stsList.Items { if sts.UID == rsOwner.UID { - break + return a.copyResourceLabelsIfPresent(&sts.ObjectMeta, pod) } } + dsList := &appsv1.DaemonSetList{} + if err := a.Client.List(ctx, dsList, client.InNamespace(req.Namespace)); err != nil { + return false, nil + } ds := appsv1.DaemonSet{} for _, ds = range dsList.Items { if ds.UID == rsOwner.UID { - break + return a.copyResourceLabelsIfPresent(&ds.ObjectMeta, pod) } } - a.Log.Info("Done looking for Parents of RS") - - if dp.UID == rsOwner.UID { - a.Log.Info("Copying from DP") - return a.copyResourceLabelsIfPresent(&dp.ObjectMeta, pod) - } else if sts.UID == rsOwner.UID { - a.Log.Info("Copying from STS") - return a.copyResourceLabelsIfPresent(&sts.ObjectMeta, pod) - } else if ds.UID == rsOwner.UID { - a.Log.Info("Copying from DS") - return a.copyResourceLabelsIfPresent(&ds.ObjectMeta, pod) - } else { - return false, nil - } + return false, nil } func (a *PodMutatingWebhook) copyResourceLabelsIfPresent(sourceResource *metav1.ObjectMeta, targetPod *corev1.Pod) (bool, error) { From 6b19285df5cc416f62ebcfa6ad1cfa5527f1c5da Mon Sep 17 00:00:00 2001 From: Moritz Wiesinger Date: Wed, 9 Nov 2022 14:33:06 +0100 Subject: [PATCH 29/41] fix some refactoring things Signed-off-by: Moritz Wiesinger --- operator/webhooks/pod_mutating_webhook.go | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/operator/webhooks/pod_mutating_webhook.go b/operator/webhooks/pod_mutating_webhook.go index 2077042e1d..66d40f0987 100644 --- a/operator/webhooks/pod_mutating_webhook.go +++ b/operator/webhooks/pod_mutating_webhook.go @@ -10,7 +10,7 @@ import ( "strings" "github.com/go-logr/logr" - kltv1alpha1 "github.com/keptn/lifecycle-toolkit/operator/api/v1alpha1" + klcv1alpha1 "github.com/keptn/lifecycle-toolkit/operator/api/v1alpha1" "github.com/keptn/lifecycle-toolkit/operator/api/v1alpha1/common" "github.com/keptn/lifecycle-toolkit/operator/api/v1alpha1/semconv" "go.opentelemetry.io/otel" @@ -323,7 +323,7 @@ func (a *PodMutatingWebhook) handleWorkload(ctx context.Context, logger logr.Log logger.Info("Searching for workload") - workload := &kltv1alpha1.KeptnWorkload{} + workload := &klcv1alpha1.KeptnWorkload{} err := a.Client.Get(ctx, types.NamespacedName{Namespace: namespace, Name: newWorkload.Name}, workload) if errors.IsNotFound(err) { logger.Info("Creating workload", "workload", workload.Name) @@ -377,7 +377,7 @@ func (a *PodMutatingWebhook) handleApp(ctx context.Context, logger logr.Logger, logger.Info("Searching for app") - app := &kltv1alpha1.KeptnApp{} + app := &klcv1alpha1.KeptnApp{} err := a.Client.Get(ctx, types.NamespacedName{Namespace: namespace, Name: newApp.Name}, app) if errors.IsNotFound(err) { logger.Info("Creating app", "app", app.Name) @@ -420,7 +420,7 @@ func (a *PodMutatingWebhook) handleApp(ctx context.Context, logger logr.Logger, return nil } -func (a *PodMutatingWebhook) generateWorkload(ctx context.Context, pod *corev1.Pod, namespace string) *kltv1alpha1.KeptnWorkload { +func (a *PodMutatingWebhook) generateWorkload(ctx context.Context, pod *corev1.Pod, namespace string) *klcv1alpha1.KeptnWorkload { version, _ := getLabelOrAnnotation(&pod.ObjectMeta, common.VersionAnnotation, common.K8sRecommendedVersionAnnotations) applicationName, _ := getLabelOrAnnotation(&pod.ObjectMeta, common.AppAnnotation, common.K8sRecommendedAppAnnotations) @@ -450,13 +450,13 @@ func (a *PodMutatingWebhook) generateWorkload(ctx context.Context, pod *corev1.P traceContextCarrier := propagation.MapCarrier{} otel.GetTextMapPropagator().Inject(ctx, traceContextCarrier) - return &kltv1alpha1.KeptnWorkload{ + return &klcv1alpha1.KeptnWorkload{ ObjectMeta: metav1.ObjectMeta{ Name: a.getWorkloadName(pod), Namespace: namespace, Annotations: traceContextCarrier, }, - Spec: kltv1alpha1.KeptnWorkloadSpec{ + Spec: klcv1alpha1.KeptnWorkloadSpec{ AppName: applicationName, Version: version, ResourceReference: a.getReplicaSetOfPod(pod), @@ -468,7 +468,7 @@ func (a *PodMutatingWebhook) generateWorkload(ctx context.Context, pod *corev1.P } } -func (a *PodMutatingWebhook) generateApp(ctx context.Context, pod *corev1.Pod, namespace string) *kltv1alpha1.KeptnApp { +func (a *PodMutatingWebhook) generateApp(ctx context.Context, pod *corev1.Pod, namespace string) *klcv1alpha1.KeptnApp { version, _ := getLabelOrAnnotation(&pod.ObjectMeta, common.VersionAnnotation, common.K8sRecommendedVersionAnnotations) appName := a.getAppName(pod) @@ -477,19 +477,19 @@ func (a *PodMutatingWebhook) generateApp(ctx context.Context, pod *corev1.Pod, n traceContextCarrier := propagation.MapCarrier{} otel.GetTextMapPropagator().Inject(ctx, traceContextCarrier) - return &kltv1alpha1.KeptnApp{ + return &klcv1alpha1.KeptnApp{ ObjectMeta: metav1.ObjectMeta{ Name: appName, Namespace: namespace, Annotations: traceContextCarrier, }, - Spec: kltv1alpha1.KeptnAppSpec{ + Spec: klcv1alpha1.KeptnAppSpec{ Version: version, PreDeploymentTasks: []string{}, PostDeploymentTasks: []string{}, PreDeploymentEvaluations: []string{}, PostDeploymentEvaluations: []string{}, - Workloads: []kltv1alpha1.KeptnWorkloadRef{ + Workloads: []klcv1alpha1.KeptnWorkloadRef{ { Name: appName, Version: version, From 9925ebb4a87f89e66e36db845a05eddf70b8cbeb Mon Sep 17 00:00:00 2001 From: Moritz Wiesinger Date: Wed, 9 Nov 2022 14:33:14 +0100 Subject: [PATCH 30/41] go fmt Signed-off-by: Moritz Wiesinger --- operator/controllers/common/fake/tracer.go | 23 +++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/operator/controllers/common/fake/tracer.go b/operator/controllers/common/fake/tracer.go index 9bdc5314dc..5934e8ba0b 100644 --- a/operator/controllers/common/fake/tracer.go +++ b/operator/controllers/common/fake/tracer.go @@ -11,19 +11,19 @@ import ( // ITracerMock is a mock implementation of common.ITracer. // -// func TestSomethingThatUsesITracer(t *testing.T) { +// func TestSomethingThatUsesITracer(t *testing.T) { // -// // make and configure a mocked common.ITracer -// mockedITracer := &ITracerMock{ -// StartFunc: func(ctx context.Context, spanName string, opts ...trace.SpanStartOption) (context.Context, trace.Span) { -// panic("mock out the Start method") -// }, -// } +// // make and configure a mocked common.ITracer +// mockedITracer := &ITracerMock{ +// StartFunc: func(ctx context.Context, spanName string, opts ...trace.SpanStartOption) (context.Context, trace.Span) { +// panic("mock out the Start method") +// }, +// } // -// // use mockedITracer in code that requires common.ITracer -// // and then make assertions. +// // use mockedITracer in code that requires common.ITracer +// // and then make assertions. // -// } +// } type ITracerMock struct { // StartFunc mocks the Start method. StartFunc func(ctx context.Context, spanName string, opts ...trace.SpanStartOption) (context.Context, trace.Span) @@ -65,7 +65,8 @@ func (mock *ITracerMock) Start(ctx context.Context, spanName string, opts ...tra // StartCalls gets all the calls that were made to Start. // Check the length with: -// len(mockedITracer.StartCalls()) +// +// len(mockedITracer.StartCalls()) func (mock *ITracerMock) StartCalls() []struct { Ctx context.Context SpanName string From 237824876930fa75b498abfa9fe15ebf19c26477 Mon Sep 17 00:00:00 2001 From: Moritz Wiesinger Date: Wed, 9 Nov 2022 14:48:57 +0100 Subject: [PATCH 31/41] update the whole process with switch case and change behaviour Signed-off-by: Moritz Wiesinger --- operator/webhooks/pod_mutating_webhook.go | 137 ++++++++++------------ 1 file changed, 64 insertions(+), 73 deletions(-) diff --git a/operator/webhooks/pod_mutating_webhook.go b/operator/webhooks/pod_mutating_webhook.go index 66d40f0987..892496f087 100644 --- a/operator/webhooks/pod_mutating_webhook.go +++ b/operator/webhooks/pod_mutating_webhook.go @@ -158,64 +158,72 @@ func (a *PodMutatingWebhook) isPodAnnotated(pod *corev1.Pod) (bool, error) { } func (a *PodMutatingWebhook) copyAnnotationsIfParentAnnotated(ctx context.Context, req *admission.Request, pod *corev1.Pod) (bool, error) { - owner := a.getReplicaSetOfPod(pod) - if owner.UID == pod.UID { - a.Log.Info("owner UID equals pod UID") + podOwner := a.getOwnerReference(&pod.ObjectMeta) + if podOwner.UID == "" { return false, nil } - rsl := &appsv1.ReplicaSetList{} - if err := a.Client.List(ctx, rsl, client.InNamespace(req.Namespace)); err != nil { - return false, nil - } + switch podOwner.Kind { + case "ReplicaSet": + rsl := &appsv1.ReplicaSetList{} + if err := a.Client.List(ctx, rsl, client.InNamespace(req.Namespace)); err != nil { + return false, nil + } - rs := appsv1.ReplicaSet{} - if len(rsl.Items) != 0 { - for _, rs = range rsl.Items { - if rs.UID == owner.UID { - break + rs := appsv1.ReplicaSet{} + if len(rsl.Items) != 0 { + for _, rs = range rsl.Items { + if rs.UID == podOwner.UID { + break + } } } - } - a.Log.Info("Done looking for RS") - - rsOwner := a.getOwnerOfReplicaSet(&rs) + a.Log.Info("Done looking for RS") - if rsOwner.UID == "" { - return false, nil - } - - dpList := &appsv1.DeploymentList{} - if err := a.Client.List(ctx, dpList, client.InNamespace(req.Namespace)); err != nil { - return false, nil - } - dp := appsv1.Deployment{} - for _, dp = range dpList.Items { - if dp.UID == rsOwner.UID { - return a.copyResourceLabelsIfPresent(&dp.ObjectMeta, pod) + rsOwner := a.getOwnerReference(&rs.ObjectMeta) + if rsOwner.UID == "" { + return false, nil } - } - - stsList := &appsv1.StatefulSetList{} - if err := a.Client.List(ctx, stsList, client.InNamespace(req.Namespace)); err != nil { - return false, nil - } - sts := appsv1.StatefulSet{} - for _, sts = range stsList.Items { - if sts.UID == rsOwner.UID { - return a.copyResourceLabelsIfPresent(&sts.ObjectMeta, pod) + dpl := &appsv1.DeploymentList{} + if err := a.Client.List(ctx, dpl, client.InNamespace(req.Namespace)); err != nil { + return false, nil } - } - - dsList := &appsv1.DaemonSetList{} - if err := a.Client.List(ctx, dsList, client.InNamespace(req.Namespace)); err != nil { - return false, nil - } - ds := appsv1.DaemonSet{} - for _, ds = range dsList.Items { - if ds.UID == rsOwner.UID { - return a.copyResourceLabelsIfPresent(&ds.ObjectMeta, pod) + dp := appsv1.Deployment{} + if len(dpl.Items) != 0 { + for _, dp = range dpl.Items { + if dp.UID == podOwner.UID { + return a.copyResourceLabelsIfPresent(&dp.ObjectMeta, pod) + } + } + } + case "StatefulSet": + stsl := &appsv1.StatefulSetList{} + if err := a.Client.List(ctx, stsl, client.InNamespace(req.Namespace)); err != nil { + return false, nil + } + sts := appsv1.StatefulSet{} + if len(stsl.Items) != 0 { + for _, sts = range stsl.Items { + if sts.UID == podOwner.UID { + return a.copyResourceLabelsIfPresent(&sts.ObjectMeta, pod) + } + } } + case "Daemonset": + dsl := &appsv1.DaemonSetList{} + if err := a.Client.List(ctx, dsl, client.InNamespace(req.Namespace)); err != nil { + return false, nil + } + ds := appsv1.DaemonSet{} + if len(dsl.Items) != 0 { + for _, ds = range dsl.Items { + if ds.UID == podOwner.UID { + return a.copyResourceLabelsIfPresent(&ds.ObjectMeta, pod) + } + } + } + default: + return false, nil } return false, nil } @@ -459,7 +467,7 @@ func (a *PodMutatingWebhook) generateWorkload(ctx context.Context, pod *corev1.P Spec: klcv1alpha1.KeptnWorkloadSpec{ AppName: applicationName, Version: version, - ResourceReference: a.getReplicaSetOfPod(pod), + ResourceReference: a.getOwnerReference(&pod.ObjectMeta), PreDeploymentTasks: preDeploymentTasks, PostDeploymentTasks: postDeploymentTasks, PreDeploymentEvaluations: preDeploymentEvaluation, @@ -510,30 +518,13 @@ func (a *PodMutatingWebhook) getAppName(pod *corev1.Pod) string { return strings.ToLower(applicationName) } -func (a *PodMutatingWebhook) getReplicaSetOfPod(pod *corev1.Pod) kltv1alpha1.ResourceReference { - reference := kltv1alpha1.ResourceReference{ - UID: pod.UID, - Kind: pod.Kind, - } - if len(pod.OwnerReferences) != 0 { - for _, o := range pod.OwnerReferences { - if o.Kind == "ReplicaSet" { - reference.UID = o.UID - reference.Kind = o.Kind - } - } - } - return reference -} - -func (a *PodMutatingWebhook) getOwnerOfReplicaSet(rs *appsv1.ReplicaSet) kltv1alpha1.ResourceReference { - reference := kltv1alpha1.ResourceReference{} - - if len(rs.OwnerReferences) != 0 { - for _, o := range rs.OwnerReferences { - if o.Kind == "Deployment" || o.Kind == "StatefulSet" || o.Kind == "DaemonSet" { - reference.UID = o.UID - reference.Kind = o.Kind +func (a *PodMutatingWebhook) getOwnerReference(resource *metav1.ObjectMeta) klcv1alpha1.ResourceReference { + reference := klcv1alpha1.ResourceReference{} + if len(resource.OwnerReferences) != 0 { + for _, owner := range resource.OwnerReferences { + if owner.Kind == "ReplicaSet" || owner.Kind == "Deployment" || owner.Kind == "StatefulSet" || owner.Kind == "DaemonSet" { + reference.UID = owner.UID + reference.Kind = owner.Kind } } } From ccf00c3d0435c7c6292799b71007ec0758020814 Mon Sep 17 00:00:00 2001 From: Moritz Wiesinger Date: Wed, 9 Nov 2022 15:23:00 +0100 Subject: [PATCH 32/41] simplify the whole thing by using get instead of list and search Signed-off-by: Moritz Wiesinger --- operator/webhooks/pod_mutating_webhook.go | 68 +++++++---------------- 1 file changed, 21 insertions(+), 47 deletions(-) diff --git a/operator/webhooks/pod_mutating_webhook.go b/operator/webhooks/pod_mutating_webhook.go index 892496f087..60f0149d6c 100644 --- a/operator/webhooks/pod_mutating_webhook.go +++ b/operator/webhooks/pod_mutating_webhook.go @@ -32,7 +32,7 @@ import ( // +kubebuilder:webhook:path=/mutate-v1-pod,mutating=true,failurePolicy=fail,groups="",resources=pods,verbs=create;update,versions=v1,name=mpod.keptn.sh,admissionReviewVersions=v1,sideEffects=None //+kubebuilder:rbac:groups=core,resources=namespaces,verbs=get;list;watch -//+kubebuilder:rbac:groups=apps,resources=deployments;statefulsets;daemonsets,verbs=list +//+kubebuilder:rbac:groups=apps,resources=deployments;statefulsets;daemonsets,verbs=list;get // PodMutatingWebhook annotates Pods type PodMutatingWebhook struct { @@ -165,67 +165,38 @@ func (a *PodMutatingWebhook) copyAnnotationsIfParentAnnotated(ctx context.Contex switch podOwner.Kind { case "ReplicaSet": - rsl := &appsv1.ReplicaSetList{} - if err := a.Client.List(ctx, rsl, client.InNamespace(req.Namespace)); err != nil { + rs := &appsv1.ReplicaSet{} + if err := a.Client.Get(ctx, types.NamespacedName{Namespace: req.Namespace, Name: podOwner.Name}, rs); err != nil { return false, nil } - - rs := appsv1.ReplicaSet{} - if len(rsl.Items) != 0 { - for _, rs = range rsl.Items { - if rs.UID == podOwner.UID { - break - } - } - } - a.Log.Info("Done looking for RS") + a.Log.Info("Done fetching RS") rsOwner := a.getOwnerReference(&rs.ObjectMeta) if rsOwner.UID == "" { return false, nil } - dpl := &appsv1.DeploymentList{} - if err := a.Client.List(ctx, dpl, client.InNamespace(req.Namespace)); err != nil { + + dp := &appsv1.Deployment{} + if err := a.Client.Get(ctx, types.NamespacedName{Namespace: req.Namespace, Name: rsOwner.Name}, dp); err != nil { return false, nil } - dp := appsv1.Deployment{} - if len(dpl.Items) != 0 { - for _, dp = range dpl.Items { - if dp.UID == podOwner.UID { - return a.copyResourceLabelsIfPresent(&dp.ObjectMeta, pod) - } - } - } + + return a.copyResourceLabelsIfPresent(&dp.ObjectMeta, pod) case "StatefulSet": - stsl := &appsv1.StatefulSetList{} - if err := a.Client.List(ctx, stsl, client.InNamespace(req.Namespace)); err != nil { + sts := &appsv1.StatefulSet{} + if err := a.Client.Get(ctx, types.NamespacedName{Namespace: req.Namespace, Name: podOwner.Name}, sts); err != nil { return false, nil } - sts := appsv1.StatefulSet{} - if len(stsl.Items) != 0 { - for _, sts = range stsl.Items { - if sts.UID == podOwner.UID { - return a.copyResourceLabelsIfPresent(&sts.ObjectMeta, pod) - } - } - } + return a.copyResourceLabelsIfPresent(&sts.ObjectMeta, pod) case "Daemonset": - dsl := &appsv1.DaemonSetList{} - if err := a.Client.List(ctx, dsl, client.InNamespace(req.Namespace)); err != nil { + ds := &appsv1.DaemonSet{} + if err := a.Client.Get(ctx, types.NamespacedName{Namespace: req.Namespace, Name: podOwner.Name}, ds); err != nil { return false, nil } - ds := appsv1.DaemonSet{} - if len(dsl.Items) != 0 { - for _, ds = range dsl.Items { - if ds.UID == podOwner.UID { - return a.copyResourceLabelsIfPresent(&ds.ObjectMeta, pod) - } - } - } + return a.copyResourceLabelsIfPresent(&ds.ObjectMeta, pod) default: return false, nil } - return false, nil } func (a *PodMutatingWebhook) copyResourceLabelsIfPresent(sourceResource *metav1.ObjectMeta, targetPod *corev1.Pod) (bool, error) { @@ -458,6 +429,8 @@ func (a *PodMutatingWebhook) generateWorkload(ctx context.Context, pod *corev1.P traceContextCarrier := propagation.MapCarrier{} otel.GetTextMapPropagator().Inject(ctx, traceContextCarrier) + ownerRef := a.getOwnerReference(&pod.ObjectMeta) + return &klcv1alpha1.KeptnWorkload{ ObjectMeta: metav1.ObjectMeta{ Name: a.getWorkloadName(pod), @@ -467,7 +440,7 @@ func (a *PodMutatingWebhook) generateWorkload(ctx context.Context, pod *corev1.P Spec: klcv1alpha1.KeptnWorkloadSpec{ AppName: applicationName, Version: version, - ResourceReference: a.getOwnerReference(&pod.ObjectMeta), + ResourceReference: klcv1alpha1.ResourceReference{UID: ownerRef.UID, Kind: ownerRef.Kind}, PreDeploymentTasks: preDeploymentTasks, PostDeploymentTasks: postDeploymentTasks, PreDeploymentEvaluations: preDeploymentEvaluation, @@ -518,13 +491,14 @@ func (a *PodMutatingWebhook) getAppName(pod *corev1.Pod) string { return strings.ToLower(applicationName) } -func (a *PodMutatingWebhook) getOwnerReference(resource *metav1.ObjectMeta) klcv1alpha1.ResourceReference { - reference := klcv1alpha1.ResourceReference{} +func (a *PodMutatingWebhook) getOwnerReference(resource *metav1.ObjectMeta) metav1.OwnerReference { + reference := metav1.OwnerReference{} if len(resource.OwnerReferences) != 0 { for _, owner := range resource.OwnerReferences { if owner.Kind == "ReplicaSet" || owner.Kind == "Deployment" || owner.Kind == "StatefulSet" || owner.Kind == "DaemonSet" { reference.UID = owner.UID reference.Kind = owner.Kind + reference.Name = owner.Name } } } From 8881c943d4a0bbb99fd3dad9640557b450e975f8 Mon Sep 17 00:00:00 2001 From: Moritz Wiesinger Date: Wed, 9 Nov 2022 15:24:40 +0100 Subject: [PATCH 33/41] update CRDs accordingly Signed-off-by: Moritz Wiesinger --- operator/config/rbac/role.yaml | 3 ++- operator/webhooks/pod_mutating_webhook.go | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/operator/config/rbac/role.yaml b/operator/config/rbac/role.yaml index d908e7703a..58a9d2b1b1 100644 --- a/operator/config/rbac/role.yaml +++ b/operator/config/rbac/role.yaml @@ -10,9 +10,10 @@ rules: resources: - daemonsets - deployments + - replicasets - statefulsets verbs: - - list + - get - apiGroups: - apps resources: diff --git a/operator/webhooks/pod_mutating_webhook.go b/operator/webhooks/pod_mutating_webhook.go index 60f0149d6c..40dd499830 100644 --- a/operator/webhooks/pod_mutating_webhook.go +++ b/operator/webhooks/pod_mutating_webhook.go @@ -32,7 +32,7 @@ import ( // +kubebuilder:webhook:path=/mutate-v1-pod,mutating=true,failurePolicy=fail,groups="",resources=pods,verbs=create;update,versions=v1,name=mpod.keptn.sh,admissionReviewVersions=v1,sideEffects=None //+kubebuilder:rbac:groups=core,resources=namespaces,verbs=get;list;watch -//+kubebuilder:rbac:groups=apps,resources=deployments;statefulsets;daemonsets,verbs=list;get +//+kubebuilder:rbac:groups=apps,resources=deployments;statefulsets;daemonsets;replicasets,verbs=get // PodMutatingWebhook annotates Pods type PodMutatingWebhook struct { From 450444656dc6f1a2b27e3446add6b49fffd36018 Mon Sep 17 00:00:00 2001 From: Moritz Wiesinger Date: Thu, 10 Nov 2022 11:24:57 +0100 Subject: [PATCH 34/41] fix unit tests Signed-off-by: Moritz Wiesinger --- .../webhooks/pod_mutating_webhook_test.go | 138 ++++-------------- 1 file changed, 31 insertions(+), 107 deletions(-) diff --git a/operator/webhooks/pod_mutating_webhook_test.go b/operator/webhooks/pod_mutating_webhook_test.go index 56c25f30e6..cae7185bc1 100644 --- a/operator/webhooks/pod_mutating_webhook_test.go +++ b/operator/webhooks/pod_mutating_webhook_test.go @@ -4,7 +4,6 @@ import ( "context" "github.com/go-logr/logr" "github.com/go-logr/logr/testr" - "github.com/keptn/lifecycle-toolkit/operator/api/v1alpha1" "github.com/keptn/lifecycle-toolkit/operator/api/v1alpha1/common" "github.com/keptn/lifecycle-toolkit/operator/controllers/common/fake" "github.com/stretchr/testify/require" @@ -21,7 +20,7 @@ import ( "testing" ) -func TestPodMutatingWebhook_getOwnerOfReplicaSet(t *testing.T) { +func TestPodMutatingWebhook_getOwnerReference(t *testing.T) { type fields struct { Client client.Client Tracer trace.Tracer @@ -30,131 +29,51 @@ func TestPodMutatingWebhook_getOwnerOfReplicaSet(t *testing.T) { Log logr.Logger } type args struct { - rs *appsv1.ReplicaSet + resource *metav1.ObjectMeta } tests := []struct { name string fields fields args args - want v1alpha1.ResourceReference + want metav1.OwnerReference }{ { name: "Test simple return when UID and Kind is set", args: args{ - rs: &appsv1.ReplicaSet{ - ObjectMeta: metav1.ObjectMeta{ - OwnerReferences: []metav1.OwnerReference{ - { - Kind: "Deployment", - UID: "someUID-123456", - }, - }, - }, - }, - }, - want: v1alpha1.ResourceReference{ - UID: "someUID-123456", - Kind: "Deployment", - }, - }, - { - name: "Test return is input argument if owner is not found", - args: args{ - rs: &appsv1.ReplicaSet{ - TypeMeta: metav1.TypeMeta{ - Kind: "ReplicaSet", - }, - ObjectMeta: metav1.ObjectMeta{ - UID: "replicaset-UID-abc123", - OwnerReferences: []metav1.OwnerReference{ - { - Kind: "SomeNonExistentType", - UID: "someUID-123456", - }, - }, - }, - }, - }, - want: v1alpha1.ResourceReference{ - Kind: "", - UID: "", - }, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - a := &PodMutatingWebhook{ - Client: tt.fields.Client, - Tracer: tt.fields.Tracer, - decoder: tt.fields.decoder, - Recorder: tt.fields.Recorder, - Log: tt.fields.Log, - } - if got := a.getOwnerOfReplicaSet(tt.args.rs); !reflect.DeepEqual(got, tt.want) { - t.Errorf("getOwnerOfReplicaSet() = %v, want %v", got, tt.want) - } - }) - } -} - -func TestPodMutatingWebhook_getReplicaSetOfPod(t *testing.T) { - type fields struct { - Client client.Client - Tracer trace.Tracer - decoder *admission.Decoder - Recorder record.EventRecorder - Log logr.Logger - } - type args struct { - pod *corev1.Pod - } - tests := []struct { - name string - fields fields - args args - want v1alpha1.ResourceReference - }{ - { - name: "Test simple return when UID and Kind is set", - args: args{ - pod: &corev1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - UID: "the-pod-uid", - OwnerReferences: []metav1.OwnerReference{ - { - Kind: "ReplicaSet", - UID: "the-replicaset-uid", - }, + resource: &metav1.ObjectMeta{ + UID: "the-pod-uid", + OwnerReferences: []metav1.OwnerReference{ + { + Kind: "ReplicaSet", + UID: "the-replicaset-uid", + Name: "some-name", }, }, }, }, - want: v1alpha1.ResourceReference{ + want: metav1.OwnerReference{ UID: "the-replicaset-uid", Kind: "ReplicaSet", + Name: "some-name", }, }, { name: "Test return is input argument if owner is not found", args: args{ - pod: &corev1.Pod{ - TypeMeta: metav1.TypeMeta{ - Kind: "Pod", - }, - ObjectMeta: metav1.ObjectMeta{ - UID: "the-pod-uid", - OwnerReferences: []metav1.OwnerReference{ - { - Kind: "SomeNonExistentType", - UID: "the-replicaset-uid", - }, + resource: &metav1.ObjectMeta{ + UID: "the-pod-uid", + OwnerReferences: []metav1.OwnerReference{ + { + Kind: "SomeNonExistentType", + UID: "the-replicaset-uid", }, }, }, }, - want: v1alpha1.ResourceReference{ - UID: "the-pod-uid", - Kind: "Pod", + want: metav1.OwnerReference{ + UID: "", + Kind: "", + Name: "", }, }, } @@ -167,8 +86,8 @@ func TestPodMutatingWebhook_getReplicaSetOfPod(t *testing.T) { Recorder: tt.fields.Recorder, Log: tt.fields.Log, } - if got := a.getReplicaSetOfPod(tt.args.pod); !reflect.DeepEqual(got, tt.want) { - t.Errorf("getReplicaSetOfPod() = %v, want %v", got, tt.want) + if got := a.getOwnerReference(tt.args.resource); !reflect.DeepEqual(got, tt.want) { + t.Errorf("getOwnerReference() = %v, want %v", got, tt.want) } }) } @@ -704,11 +623,16 @@ func TestPodMutatingWebhook_copyAnnotationsIfParentAnnotated(t *testing.T) { { name: "Test that nothing happens if owner UID is pod UID", fields: fields{ - Log: testr.New(t), + Log: testr.New(t), + Client: fakeClient, }, args: args{ ctx: context.TODO(), - req: nil, + req: &admission.Request{ + AdmissionRequest: admissionv1.AdmissionRequest{ + Namespace: testNamespace, + }, + }, pod: &corev1.Pod{ ObjectMeta: metav1.ObjectMeta{ UID: "some-uid", From 33552ea5276d7d116901ad3526d33a1d781d42bb Mon Sep 17 00:00:00 2001 From: Moritz Wiesinger Date: Thu, 10 Nov 2022 11:52:05 +0100 Subject: [PATCH 35/41] refactoring Signed-off-by: Moritz Wiesinger --- operator/webhooks/pod_mutating_webhook.go | 27 ++++++++++++----------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/operator/webhooks/pod_mutating_webhook.go b/operator/webhooks/pod_mutating_webhook.go index 40dd499830..2e2453f4c0 100644 --- a/operator/webhooks/pod_mutating_webhook.go +++ b/operator/webhooks/pod_mutating_webhook.go @@ -177,28 +177,29 @@ func (a *PodMutatingWebhook) copyAnnotationsIfParentAnnotated(ctx context.Contex } dp := &appsv1.Deployment{} - if err := a.Client.Get(ctx, types.NamespacedName{Namespace: req.Namespace, Name: rsOwner.Name}, dp); err != nil { - return false, nil - } - - return a.copyResourceLabelsIfPresent(&dp.ObjectMeta, pod) + return a.fetchParentObjectAndCopyLabels(ctx, rsOwner.Name, req.Namespace, pod, dp) case "StatefulSet": sts := &appsv1.StatefulSet{} - if err := a.Client.Get(ctx, types.NamespacedName{Namespace: req.Namespace, Name: podOwner.Name}, sts); err != nil { - return false, nil - } - return a.copyResourceLabelsIfPresent(&sts.ObjectMeta, pod) + return a.fetchParentObjectAndCopyLabels(ctx, podOwner.Name, req.Namespace, pod, sts) case "Daemonset": ds := &appsv1.DaemonSet{} - if err := a.Client.Get(ctx, types.NamespacedName{Namespace: req.Namespace, Name: podOwner.Name}, ds); err != nil { - return false, nil - } - return a.copyResourceLabelsIfPresent(&ds.ObjectMeta, pod) + return a.fetchParentObjectAndCopyLabels(ctx, podOwner.Name, req.Namespace, pod, ds) default: return false, nil } } +func (a *PodMutatingWebhook) fetchParentObjectAndCopyLabels(ctx context.Context, name string, namespace string, pod *corev1.Pod, objectContainer client.Object) (bool, error) { + if err := a.Client.Get(ctx, types.NamespacedName{Namespace: namespace, Name: name}, objectContainer); err != nil { + return false, nil + } + objectContainerMetaData := metav1.ObjectMeta{ + Labels: objectContainer.GetLabels(), + Annotations: objectContainer.GetAnnotations(), + } + return a.copyResourceLabelsIfPresent(&objectContainerMetaData, pod) +} + func (a *PodMutatingWebhook) copyResourceLabelsIfPresent(sourceResource *metav1.ObjectMeta, targetPod *corev1.Pod) (bool, error) { var workloadName, appName, version, preDeploymentChecks, postDeploymentChecks, preEvaluationChecks, postEvaluationChecks string var gotWorkloadName, gotAppName, gotVersion, gotPreDeploymentChecks, gotPostDeploymentChecks, gotPreEvaluationChecks, gotPostEvaluationChecks bool From c8372e6a9b7a7ae4acdf5a130fc46f610a4cecae Mon Sep 17 00:00:00 2001 From: Moritz Wiesinger Date: Thu, 10 Nov 2022 12:00:55 +0100 Subject: [PATCH 36/41] move error check as per review request Signed-off-by: Moritz Wiesinger --- operator/webhooks/pod_mutating_webhook.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/operator/webhooks/pod_mutating_webhook.go b/operator/webhooks/pod_mutating_webhook.go index 2e2453f4c0..179a16be80 100644 --- a/operator/webhooks/pod_mutating_webhook.go +++ b/operator/webhooks/pod_mutating_webhook.go @@ -82,15 +82,16 @@ func (a *PodMutatingWebhook) Handle(ctx context.Context, req admission.Request) podIsAnnotated, err := a.isPodAnnotated(pod) logger.Info("Checked if pod is annotated.") + if err != nil { + span.SetStatus(codes.Error, "Invalid annotations") + return admission.Errored(http.StatusBadRequest, err) + } + if err == nil && !podIsAnnotated { logger.Info("Pod is not annotated, check for parent annotations...") podIsAnnotated, err = a.copyAnnotationsIfParentAnnotated(ctx, &req, pod) } - if err != nil { - span.SetStatus(codes.Error, "Invalid annotations") - return admission.Errored(http.StatusBadRequest, err) - } if podIsAnnotated { logger.Info("Resource is annotated with Keptn annotations, using Keptn scheduler") pod.Spec.SchedulerName = "keptn-scheduler" From 6c78d1b2fbe60d2006e1adc84ee9657381520503 Mon Sep 17 00:00:00 2001 From: Moritz Wiesinger Date: Thu, 10 Nov 2022 12:01:36 +0100 Subject: [PATCH 37/41] remove unneeded permissions Signed-off-by: Moritz Wiesinger --- operator/config/rbac/role.yaml | 2 -- operator/webhooks/pod_mutating_webhook.go | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/operator/config/rbac/role.yaml b/operator/config/rbac/role.yaml index 58a9d2b1b1..45d0f9d0af 100644 --- a/operator/config/rbac/role.yaml +++ b/operator/config/rbac/role.yaml @@ -65,8 +65,6 @@ rules: - namespaces verbs: - get - - list - - watch - apiGroups: - "" resources: diff --git a/operator/webhooks/pod_mutating_webhook.go b/operator/webhooks/pod_mutating_webhook.go index 179a16be80..fec22263c1 100644 --- a/operator/webhooks/pod_mutating_webhook.go +++ b/operator/webhooks/pod_mutating_webhook.go @@ -31,7 +31,7 @@ import ( ) // +kubebuilder:webhook:path=/mutate-v1-pod,mutating=true,failurePolicy=fail,groups="",resources=pods,verbs=create;update,versions=v1,name=mpod.keptn.sh,admissionReviewVersions=v1,sideEffects=None -//+kubebuilder:rbac:groups=core,resources=namespaces,verbs=get;list;watch +//+kubebuilder:rbac:groups=core,resources=namespaces,verbs=get //+kubebuilder:rbac:groups=apps,resources=deployments;statefulsets;daemonsets;replicasets,verbs=get // PodMutatingWebhook annotates Pods From 22f9c98b70f01d88f9b74c1ddbe153d68769e357 Mon Sep 17 00:00:00 2001 From: Moritz Wiesinger Date: Thu, 10 Nov 2022 15:52:00 +0100 Subject: [PATCH 38/41] fix typo Signed-off-by: Moritz Wiesinger --- operator/webhooks/pod_mutating_webhook.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/operator/webhooks/pod_mutating_webhook.go b/operator/webhooks/pod_mutating_webhook.go index fec22263c1..0a71be711d 100644 --- a/operator/webhooks/pod_mutating_webhook.go +++ b/operator/webhooks/pod_mutating_webhook.go @@ -182,7 +182,7 @@ func (a *PodMutatingWebhook) copyAnnotationsIfParentAnnotated(ctx context.Contex case "StatefulSet": sts := &appsv1.StatefulSet{} return a.fetchParentObjectAndCopyLabels(ctx, podOwner.Name, req.Namespace, pod, sts) - case "Daemonset": + case "DaemonSet": ds := &appsv1.DaemonSet{} return a.fetchParentObjectAndCopyLabels(ctx, podOwner.Name, req.Namespace, pod, ds) default: From 65736b820033b85d115fb8408ae31877f4fcaf97 Mon Sep 17 00:00:00 2001 From: Moritz Wiesinger Date: Thu, 10 Nov 2022 15:52:08 +0100 Subject: [PATCH 39/41] fix unit tests Signed-off-by: Moritz Wiesinger --- .../webhooks/pod_mutating_webhook_test.go | 77 +++++-------------- 1 file changed, 21 insertions(+), 56 deletions(-) diff --git a/operator/webhooks/pod_mutating_webhook_test.go b/operator/webhooks/pod_mutating_webhook_test.go index cae7185bc1..b173e22eaa 100644 --- a/operator/webhooks/pod_mutating_webhook_test.go +++ b/operator/webhooks/pod_mutating_webhook_test.go @@ -487,9 +487,11 @@ func TestPodMutatingWebhook_isPodAnnotated(t *testing.T) { func TestPodMutatingWebhook_copyAnnotationsIfParentAnnotated(t *testing.T) { testNamespace := "test-namespace" rsUidWithDpOwner := types.UID("this-is-the-replicaset-with-dp-owner") - rsUidWithStsOwner := types.UID("this-is-the-replicaset-with-sts-owner") - rsUidWithDsOwner := types.UID("this-is-the-replicaset-with-ds-owner") rsUidWithNoOwner := types.UID("this-is-the-replicaset-with-no-owner") + testStsUid := types.UID("this-is-the-stateful-set-uid") + tstStsName := "test-stateful-set" + testDsUid := types.UID("this-is-the-daemon-set-uid") + testDsName := "test-daemon-set" fakeClient, err := fake.NewClient() @@ -514,41 +516,8 @@ func TestPodMutatingWebhook_copyAnnotationsIfParentAnnotated(t *testing.T) { }, }, } - rsWithStsOwner := &appsv1.ReplicaSet{ - TypeMeta: metav1.TypeMeta{ - Kind: "ReplicaSet", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "test-replicaset2", - UID: rsUidWithStsOwner, - Namespace: testNamespace, - OwnerReferences: []metav1.OwnerReference{ - { - Kind: "Deployment", - Name: "this-is-the-deployment", - UID: "this-is-the-stateful-set-uid", - }, - }, - }, - } - rsWithDsOwner := &appsv1.ReplicaSet{ - TypeMeta: metav1.TypeMeta{ - Kind: "ReplicaSet", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "test-replicaset3", - UID: rsUidWithDsOwner, - Namespace: testNamespace, - OwnerReferences: []metav1.OwnerReference{ - { - Kind: "Deployment", - Name: "this-is-the-deployment", - UID: "this-is-the-daemonset-uid", - }, - }, - }, - } - testRsWithNoOwner := &appsv1.ReplicaSet{ + // TODO: fix tests where an RS has a STS or DS as owner. they should not have a RS in between + rsWithNoOwner := &appsv1.ReplicaSet{ TypeMeta: metav1.TypeMeta{ Kind: "ReplicaSet", }, @@ -573,8 +542,8 @@ func TestPodMutatingWebhook_copyAnnotationsIfParentAnnotated(t *testing.T) { Kind: "StatefulSet", }, ObjectMeta: metav1.ObjectMeta{ - Name: "test-stateful-set", - UID: "this-is-the-stateful-set-uid", + Name: tstStsName, + UID: testStsUid, Namespace: testNamespace, }, } @@ -583,16 +552,14 @@ func TestPodMutatingWebhook_copyAnnotationsIfParentAnnotated(t *testing.T) { Kind: "DaemonSet", }, ObjectMeta: metav1.ObjectMeta{ - Name: "test-daemonset", - UID: "this-is-the-daemonset-uid", + Name: testDsName, + UID: testDsUid, Namespace: testNamespace, }, } err = fakeClient.Create(context.TODO(), rsWithDpOwner) - err = fakeClient.Create(context.TODO(), rsWithStsOwner) - err = fakeClient.Create(context.TODO(), rsWithDsOwner) - err = fakeClient.Create(context.TODO(), testRsWithNoOwner) + err = fakeClient.Create(context.TODO(), rsWithNoOwner) err = fakeClient.Create(context.TODO(), testDp) err = fakeClient.Create(context.TODO(), testSts) err = fakeClient.Create(context.TODO(), testDs) @@ -636,12 +603,6 @@ func TestPodMutatingWebhook_copyAnnotationsIfParentAnnotated(t *testing.T) { pod: &corev1.Pod{ ObjectMeta: metav1.ObjectMeta{ UID: "some-uid", - OwnerReferences: []metav1.OwnerReference{ - { - UID: "some-uid", - Kind: "ReplicaSet", - }, - }, }, }, }, @@ -666,6 +627,7 @@ func TestPodMutatingWebhook_copyAnnotationsIfParentAnnotated(t *testing.T) { UID: "this-is-the-pod-uid", OwnerReferences: []metav1.OwnerReference{ { + Name: rsWithDpOwner.Name, UID: rsUidWithDpOwner, Kind: "ReplicaSet", }, @@ -677,7 +639,7 @@ func TestPodMutatingWebhook_copyAnnotationsIfParentAnnotated(t *testing.T) { wantErr: false, }, { - name: "Test fetching of replicaset owner of pod and statefulset owner of replicaset", + name: "Test fetching of statefulset owner of pod", fields: fields{ Log: testr.New(t), Client: fakeClient, @@ -694,8 +656,9 @@ func TestPodMutatingWebhook_copyAnnotationsIfParentAnnotated(t *testing.T) { UID: "this-is-the-pod-uid", OwnerReferences: []metav1.OwnerReference{ { - UID: rsUidWithStsOwner, - Kind: "ReplicaSet", + Name: testSts.Name, + UID: testSts.UID, + Kind: testSts.Kind, }, }, }, @@ -705,7 +668,7 @@ func TestPodMutatingWebhook_copyAnnotationsIfParentAnnotated(t *testing.T) { wantErr: false, }, { - name: "Test fetching of replicaset owner of pod and daemonset owner of replicaset", + name: "Test fetching of daemonset owner of pod", fields: fields{ Log: testr.New(t), Client: fakeClient, @@ -722,8 +685,9 @@ func TestPodMutatingWebhook_copyAnnotationsIfParentAnnotated(t *testing.T) { UID: "this-is-the-pod-uid", OwnerReferences: []metav1.OwnerReference{ { - UID: rsUidWithDsOwner, - Kind: "ReplicaSet", + Name: testDs.Name, + UID: testDs.UID, + Kind: testDs.Kind, }, }, }, @@ -750,6 +714,7 @@ func TestPodMutatingWebhook_copyAnnotationsIfParentAnnotated(t *testing.T) { UID: "this-is-the-pod-uid", OwnerReferences: []metav1.OwnerReference{ { + Name: rsWithNoOwner.Name, UID: rsUidWithNoOwner, Kind: "ReplicaSet", }, From 13e8e20cd3c3f2791929a15be5a9466f304a67d7 Mon Sep 17 00:00:00 2001 From: Florian Bacher Date: Mon, 14 Nov 2022 10:17:11 +0100 Subject: [PATCH 40/41] minor cleanups Signed-off-by: Florian Bacher --- operator/webhooks/pod_mutating_webhook.go | 52 +++++++++++------------ 1 file changed, 24 insertions(+), 28 deletions(-) diff --git a/operator/webhooks/pod_mutating_webhook.go b/operator/webhooks/pod_mutating_webhook.go index 0a71be711d..311d73b346 100644 --- a/operator/webhooks/pod_mutating_webhook.go +++ b/operator/webhooks/pod_mutating_webhook.go @@ -87,7 +87,7 @@ func (a *PodMutatingWebhook) Handle(ctx context.Context, req admission.Request) return admission.Errored(http.StatusBadRequest, err) } - if err == nil && !podIsAnnotated { + if !podIsAnnotated { logger.Info("Pod is not annotated, check for parent annotations...") podIsAnnotated, err = a.copyAnnotationsIfParentAnnotated(ctx, &req, pod) } @@ -203,15 +203,15 @@ func (a *PodMutatingWebhook) fetchParentObjectAndCopyLabels(ctx context.Context, func (a *PodMutatingWebhook) copyResourceLabelsIfPresent(sourceResource *metav1.ObjectMeta, targetPod *corev1.Pod) (bool, error) { var workloadName, appName, version, preDeploymentChecks, postDeploymentChecks, preEvaluationChecks, postEvaluationChecks string - var gotWorkloadName, gotAppName, gotVersion, gotPreDeploymentChecks, gotPostDeploymentChecks, gotPreEvaluationChecks, gotPostEvaluationChecks bool + var gotWorkloadName, gotVersion bool workloadName, gotWorkloadName = getLabelOrAnnotation(sourceResource, common.WorkloadAnnotation, common.K8sRecommendedWorkloadAnnotations) - appName, gotAppName = getLabelOrAnnotation(sourceResource, common.AppAnnotation, common.K8sRecommendedAppAnnotations) + appName, _ = getLabelOrAnnotation(sourceResource, common.AppAnnotation, common.K8sRecommendedAppAnnotations) version, gotVersion = getLabelOrAnnotation(sourceResource, common.VersionAnnotation, common.K8sRecommendedVersionAnnotations) - preDeploymentChecks, gotPreDeploymentChecks = getLabelOrAnnotation(sourceResource, common.PreDeploymentTaskAnnotation, "") - postDeploymentChecks, gotPostDeploymentChecks = getLabelOrAnnotation(sourceResource, common.PostDeploymentTaskAnnotation, "") - preEvaluationChecks, gotPreEvaluationChecks = getLabelOrAnnotation(sourceResource, common.PreDeploymentEvaluationAnnotation, "") - postEvaluationChecks, gotPostEvaluationChecks = getLabelOrAnnotation(sourceResource, common.PostDeploymentEvaluationAnnotation, "") + preDeploymentChecks, _ = getLabelOrAnnotation(sourceResource, common.PreDeploymentTaskAnnotation, "") + postDeploymentChecks, _ = getLabelOrAnnotation(sourceResource, common.PostDeploymentTaskAnnotation, "") + preEvaluationChecks, _ = getLabelOrAnnotation(sourceResource, common.PreDeploymentEvaluationAnnotation, "") + postEvaluationChecks, _ = getLabelOrAnnotation(sourceResource, common.PostDeploymentEvaluationAnnotation, "") if len(workloadName) > common.MaxWorkloadNameLength || len(version) > common.MaxVersionLength { return false, ErrTooLongAnnotations @@ -222,33 +222,20 @@ func (a *PodMutatingWebhook) copyResourceLabelsIfPresent(sourceResource *metav1. } if gotWorkloadName { - targetPod.Annotations[common.WorkloadAnnotation] = workloadName + setMapKey(targetPod.Annotations, common.WorkloadAnnotation, workloadName) if !gotVersion { - targetPod.Annotations[common.VersionAnnotation] = a.calculateVersion(targetPod) + setMapKey(targetPod.Annotations, common.VersionAnnotation, a.calculateVersion(targetPod)) } else { - targetPod.Annotations[common.VersionAnnotation] = version + setMapKey(targetPod.Annotations, common.VersionAnnotation, version) } - if gotAppName { - targetPod.Annotations[common.AppAnnotation] = appName - } - - if gotPreDeploymentChecks { - targetPod.Annotations[common.PreDeploymentTaskAnnotation] = preDeploymentChecks - } - - if gotPostDeploymentChecks { - targetPod.Annotations[common.PostDeploymentTaskAnnotation] = postDeploymentChecks - } - - if gotPreEvaluationChecks { - targetPod.Annotations[common.PreDeploymentEvaluationAnnotation] = preEvaluationChecks - } + setMapKey(targetPod.Annotations, common.AppAnnotation, appName) + setMapKey(targetPod.Annotations, common.PreDeploymentTaskAnnotation, preDeploymentChecks) + setMapKey(targetPod.Annotations, common.PostDeploymentTaskAnnotation, postDeploymentChecks) + setMapKey(targetPod.Annotations, common.PreDeploymentEvaluationAnnotation, preEvaluationChecks) + setMapKey(targetPod.Annotations, common.PostDeploymentEvaluationAnnotation, postEvaluationChecks) - if gotPostEvaluationChecks { - targetPod.Annotations[common.PostDeploymentEvaluationAnnotation] = postEvaluationChecks - } return true, nil } return false, nil @@ -529,3 +516,12 @@ func getLabelOrAnnotation(resource *metav1.ObjectMeta, primaryAnnotation string, } return "", false } + +func setMapKey(myMap map[string]string, key, value string) { + if myMap == nil { + return + } + if value != "" { + myMap[key] = value + } +} From 64bcceadbbbe86992f06ae30c3b2baf656d2d6bb Mon Sep 17 00:00:00 2001 From: Florian Bacher Date: Mon, 14 Nov 2022 10:24:42 +0100 Subject: [PATCH 41/41] pr review Signed-off-by: Florian Bacher --- operator/webhooks/pod_mutating_webhook_test.go | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/operator/webhooks/pod_mutating_webhook_test.go b/operator/webhooks/pod_mutating_webhook_test.go index b173e22eaa..ed74b1004a 100644 --- a/operator/webhooks/pod_mutating_webhook_test.go +++ b/operator/webhooks/pod_mutating_webhook_test.go @@ -493,12 +493,6 @@ func TestPodMutatingWebhook_copyAnnotationsIfParentAnnotated(t *testing.T) { testDsUid := types.UID("this-is-the-daemon-set-uid") testDsName := "test-daemon-set" - fakeClient, err := fake.NewClient() - - if err != nil { - t.Errorf("Error when setting up fake client %v", err) - } - rsWithDpOwner := &appsv1.ReplicaSet{ TypeMeta: metav1.TypeMeta{ Kind: "ReplicaSet", @@ -558,11 +552,7 @@ func TestPodMutatingWebhook_copyAnnotationsIfParentAnnotated(t *testing.T) { }, } - err = fakeClient.Create(context.TODO(), rsWithDpOwner) - err = fakeClient.Create(context.TODO(), rsWithNoOwner) - err = fakeClient.Create(context.TODO(), testDp) - err = fakeClient.Create(context.TODO(), testSts) - err = fakeClient.Create(context.TODO(), testDs) + fakeClient, err := fake.NewClient(rsWithDpOwner, rsWithNoOwner, testDp, testSts, testDs) if err != nil { t.Errorf("Error when creating objects in fake client %v", err)