diff --git a/lifecycle-operator/apis/lifecycle/v1alpha3/common/common.go b/lifecycle-operator/apis/lifecycle/v1alpha3/common/common.go index d38ac542a5..31f1c2312d 100644 --- a/lifecycle-operator/apis/lifecycle/v1alpha3/common/common.go +++ b/lifecycle-operator/apis/lifecycle/v1alpha3/common/common.go @@ -32,6 +32,7 @@ const CreateAppEvalSpanName = "create_%s_app_evaluation" const CreateWorkloadEvalSpanName = "create_%s_deployment_evaluation" const AppTypeAnnotation = "keptn.sh/app-type" const KeptnGate = "keptn-prechecks-gate" +const ContainerNameAnnotation = "keptn.sh/container" const MinKeptnNameLen = 80 const MaxK8sObjectLength = 253 diff --git a/lifecycle-operator/webhooks/pod_mutator/handlers/objectmeta.go b/lifecycle-operator/webhooks/pod_mutator/handlers/objectmeta.go index 60d37e15a7..aac607428c 100644 --- a/lifecycle-operator/webhooks/pod_mutator/handlers/objectmeta.go +++ b/lifecycle-operator/webhooks/pod_mutator/handlers/objectmeta.go @@ -94,24 +94,44 @@ func initEmptyAnnotations(meta *metav1.ObjectMeta, size int) { } } -func calculateVersion(pod *corev1.Pod) string { +func getImageVersion(image string) (string, error) { + splitImage := strings.Split(image, ":") + lenImg := len(splitImage) - 1 + if lenImg >= 1 && splitImage[lenImg] != "" && splitImage[lenImg] != "latest" { + return splitImage[lenImg], nil + } + return "", fmt.Errorf("Invalid image version") +} + +func calculateVersion(pod *corev1.Pod, containerName string) (string, error) { if len(pod.Spec.Containers) == 1 { - image := strings.Split(pod.Spec.Containers[0].Image, ":") - lenImg := len(image) - 1 - if lenImg >= 1 && image[lenImg] != "" && image[lenImg] != "latest" { - return image[lenImg] + if containerName != "" && pod.Spec.Containers[0].Name != containerName { + return "", fmt.Errorf("The container name '%s' specified in %s does not match the name of the container in the pod", containerName, apicommon.ContainerNameAnnotation) } + return getImageVersion(pod.Spec.Containers[0].Image) } name := "" + containerFound := false for _, item := range pod.Spec.Containers { + if item.Name == containerName { + containerFound = true + version, err := getImageVersion(item.Image) + if err == nil { + return version, nil + } + } name = name + item.Name + item.Image for _, e := range item.Env { name = name + e.Name + e.Value } } + if containerName != "" && !containerFound { + return "", fmt.Errorf("The container name '%s' specified in %s does not match any containers in the pod", containerName, apicommon.ContainerNameAnnotation) + } + h := fnv.New32a() h.Write([]byte(name)) - return fmt.Sprint(h.Sum32()) + return fmt.Sprint(h.Sum32()), nil } diff --git a/lifecycle-operator/webhooks/pod_mutator/handlers/objectmeta_test.go b/lifecycle-operator/webhooks/pod_mutator/handlers/objectmeta_test.go index e7be06d458..4cf9783538 100644 --- a/lifecycle-operator/webhooks/pod_mutator/handlers/objectmeta_test.go +++ b/lifecycle-operator/webhooks/pod_mutator/handlers/objectmeta_test.go @@ -187,12 +187,59 @@ func TestGetLabelOrAnnotation(t *testing.T) { } } -func TestCalculateVersion(t *testing.T) { +func TestGetImageVersion(t *testing.T) { + tests := []struct { + name string + image string + want string + wantErr bool + }{ + { + name: "Return image version when version is present", + image: "my-image:1.0.0", + want: "1.0.0", + wantErr: false, + }, + { + name: "Return error when image version is not present", + image: "my-image", + want: "", + wantErr: true, + }, + { + name: "Return error when image version is empty", + image: "my-image:", + want: "", + wantErr: true, + }, + { + name: "Return error when image version is latest", + image: "my-image:latest", + want: "", + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := getImageVersion(tt.image) + if (err != nil) != tt.wantErr { + t.Errorf("getImageVersion() error = %v, wantErr %v", err, tt.wantErr) + return + } + if got != tt.want { + t.Errorf("getImageVersion() = %v, want %v", got, tt.want) + } + }) + } +} +func Test_calculateVersion(t *testing.T) { tests := []struct { - name string - pod *corev1.Pod - want string + name string + pod *corev1.Pod + containerName string + want string + wantErr bool }{ { name: "simple tag", @@ -202,7 +249,9 @@ func TestCalculateVersion(t *testing.T) { {Image: "ciao:1.0.0"}, }, }}, - want: "1.0.0", + containerName: "", + want: "1.0.0", + wantErr: false, }, { name: "local registry", pod: &corev1.Pod{ @@ -211,7 +260,28 @@ func TestCalculateVersion(t *testing.T) { {Image: "localhost:5000/node-web-app:1.0.0"}, }, }}, - want: "1.0.0", + containerName: "", + want: "1.0.0", + wantErr: false, + }, + { + name: "single container with annotation mismatch", + pod: &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pod-name", + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "container-name", + Image: "image:tag1", + }, + }, + }, + }, + containerName: "not-container-name", + want: "", + wantErr: true, }, { name: "multiple containers", @@ -230,12 +300,66 @@ func TestCalculateVersion(t *testing.T) { }}, }, }}, - want: "1253120182", //the hash of ciaopeppetest12 + containerName: "", + want: "1253120182", //the hash of ciaopeppetest12 + wantErr: false, + }, + { + name: "multiple containers with annotation", + pod: &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pod-name", + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "container-name", + Image: "image:tag1", + }, + { + Name: "container-name2", + Image: "image:tag2", + }, + }, + }, + }, + containerName: "container-name2", + want: "tag2", + wantErr: false, + }, + { + name: "multiple containers with annotation mismatch", + pod: &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pod-name", + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "container-name", + Image: "image:tag1", + }, + { + Name: "container-name2", + Image: "image:tag2", + }, + }, + }, + }, + containerName: "not-container-name", + want: "", + wantErr: true, }, } + for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if got := calculateVersion(tt.pod); got != tt.want { + got, err := calculateVersion(tt.pod, tt.containerName) + if (err != nil) != tt.wantErr { + t.Errorf("calculateVersion() error = %v, wantErr %v", err, tt.wantErr) + return + } + if got != tt.want { t.Errorf("calculateVersion() = %v, want %v", got, tt.want) } }) diff --git a/lifecycle-operator/webhooks/pod_mutator/handlers/pod_annotation.go b/lifecycle-operator/webhooks/pod_mutator/handlers/pod_annotation.go index 082ca28ea7..9c6f20ecec 100644 --- a/lifecycle-operator/webhooks/pod_mutator/handlers/pod_annotation.go +++ b/lifecycle-operator/webhooks/pod_mutator/handlers/pod_annotation.go @@ -2,6 +2,7 @@ package handlers import ( "context" + "log" argov1alpha1 "github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1" "github.com/go-logr/logr" @@ -94,12 +95,18 @@ func copyResourceLabelsIfPresent(sourceResource *metav1.ObjectMeta, targetPod *c postDeploymentChecks, _ = GetLabelOrAnnotation(sourceResource, apicommon.PostDeploymentTaskAnnotation, "") preEvaluationChecks, _ = GetLabelOrAnnotation(sourceResource, apicommon.PreDeploymentEvaluationAnnotation, "") postEvaluationChecks, _ = GetLabelOrAnnotation(sourceResource, apicommon.PostDeploymentEvaluationAnnotation, "") + containerName, _ := GetLabelOrAnnotation(sourceResource, apicommon.ContainerNameAnnotation, "") if gotWorkloadName { setMapKey(targetPod.Annotations, apicommon.WorkloadAnnotation, workloadName) if !gotVersion { - setMapKey(targetPod.Annotations, apicommon.VersionAnnotation, calculateVersion(targetPod)) + version, err := calculateVersion(targetPod, containerName) + if err != nil { + log.Println(err) + return false + } + setMapKey(targetPod.Annotations, apicommon.VersionAnnotation, version) } else { setMapKey(targetPod.Annotations, apicommon.VersionAnnotation, version) } @@ -116,13 +123,19 @@ func copyResourceLabelsIfPresent(sourceResource *metav1.ObjectMeta, targetPod *c } func isPodAnnotated(pod *corev1.Pod) bool { + containerName, _ := GetLabelOrAnnotation(&pod.ObjectMeta, apicommon.ContainerNameAnnotation, "") _, gotWorkloadAnnotation := GetLabelOrAnnotation(&pod.ObjectMeta, apicommon.WorkloadAnnotation, apicommon.K8sRecommendedWorkloadAnnotations) _, gotVersionAnnotation := GetLabelOrAnnotation(&pod.ObjectMeta, apicommon.VersionAnnotation, apicommon.K8sRecommendedVersionAnnotations) if gotWorkloadAnnotation { if !gotVersionAnnotation { initEmptyAnnotations(&pod.ObjectMeta, 1) - pod.Annotations[apicommon.VersionAnnotation] = calculateVersion(pod) + version, err := calculateVersion(pod, containerName) + if err != nil { + log.Println(err) + } else { + pod.Annotations[apicommon.VersionAnnotation] = version + } } return true } diff --git a/lifecycle-operator/webhooks/pod_mutator/handlers/pod_annotation_test.go b/lifecycle-operator/webhooks/pod_mutator/handlers/pod_annotation_test.go index f9604a0b21..645e3f121a 100644 --- a/lifecycle-operator/webhooks/pod_mutator/handlers/pod_annotation_test.go +++ b/lifecycle-operator/webhooks/pod_mutator/handlers/pod_annotation_test.go @@ -636,6 +636,49 @@ func TestIsPodAnnotated(t *testing.T) { }, want: false, }, + { + name: "Test return false when container annotation does not match container name", + args: args{ + pod: &corev1.Pod{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "container-name", + }, + }, + }, + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + apicommon.ContainerNameAnnotation: "not-container-name", + }, + }, + }, + }, + want: false, + }, + { + name: "Test return false when container annotation does not match any container name", + args: args{ + pod: &corev1.Pod{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "container-name", + }, + { + Name: "container-name2", + }, + }, + }, + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + apicommon.ContainerNameAnnotation: "not-container-name", + }, + }, + }, + }, + want: false, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/scheduler/pkg/klcpermit/workflow_manager.go b/scheduler/pkg/klcpermit/workflow_manager.go index 1658a2e7a4..51e212941d 100644 --- a/scheduler/pkg/klcpermit/workflow_manager.go +++ b/scheduler/pkg/klcpermit/workflow_manager.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "hash/fnv" + "log" "strings" "github.com/keptn/lifecycle-toolkit/scheduler/pkg/tracing" @@ -50,6 +51,7 @@ const AppAnnotation = "keptn.sh/app" const K8sRecommendedWorkloadAnnotations = "app.kubernetes.io/name" const K8sRecommendedVersionAnnotations = "app.kubernetes.io/version" const K8sRecommendedAppAnnotations = "app.kubernetes.io/part-of" +const ContainerNameAnnotation = "keptn.sh/container" type Manager interface { Permit(context.Context, *corev1.Pod) Status @@ -182,8 +184,15 @@ func getCRDName(pod *corev1.Pod) string { application, _ := getLabelOrAnnotation(pod, AppAnnotation, K8sRecommendedAppAnnotations) workload, _ := getLabelOrAnnotation(pod, WorkloadAnnotation, K8sRecommendedWorkloadAnnotations) version, versionExists := getLabelOrAnnotation(pod, VersionAnnotation, K8sRecommendedVersionAnnotations) + containerName, _ := getLabelOrAnnotation(pod, ContainerNameAnnotation, "") if !versionExists { - version = calculateVersion(pod) + + var err error + + version, err = calculateVersion(pod, containerName) + if err != nil { + log.Println(err) + } } return createResourceName(MaxK8sObjectLength, MinKLTNameLen, application, workload, version) } @@ -206,24 +215,44 @@ func getLabelOrAnnotation(pod *corev1.Pod, primaryAnnotation string, secondaryAn return "", false } -func calculateVersion(pod *corev1.Pod) string { - name := "" +func getImageVersion(image string) (string, error) { + splitImage := strings.Split(image, ":") + lenImg := len(splitImage) - 1 + if lenImg >= 1 && splitImage[lenImg] != "" && splitImage[lenImg] != "latest" { + return splitImage[lenImg], nil + } + return "", fmt.Errorf("Invalid image version") +} +func calculateVersion(pod *corev1.Pod, containerName string) (string, error) { if len(pod.Spec.Containers) == 1 { - image := strings.Split(pod.Spec.Containers[0].Image, ":") - if len(image) > 0 && image[1] != "" && image[1] != "latest" { - return image[1] + if containerName != "" && pod.Spec.Containers[0].Name != containerName { + return "", fmt.Errorf("The container name '%s' specified in %s does not match the name of the container in the pod", containerName, ContainerNameAnnotation) } + return getImageVersion(pod.Spec.Containers[0].Image) } + name := "" + containerFound := false for _, item := range pod.Spec.Containers { + if item.Name == containerName { + containerFound = true + version, err := getImageVersion(item.Image) + if err == nil { + return version, nil + } + } name = name + item.Name + item.Image for _, e := range item.Env { name = name + e.Name + e.Value } } + if containerName != "" && !containerFound { + return "", fmt.Errorf("The container name '%s' specified in %s does not match any containers in the pod", containerName, ContainerNameAnnotation) + } + h := fnv.New32a() h.Write([]byte(name)) - return fmt.Sprint(h.Sum32()) + return fmt.Sprint(h.Sum32()), nil } diff --git a/scheduler/pkg/klcpermit/workflow_manager_test.go b/scheduler/pkg/klcpermit/workflow_manager_test.go index 50beac799f..6c6b8fb302 100644 --- a/scheduler/pkg/klcpermit/workflow_manager_test.go +++ b/scheduler/pkg/klcpermit/workflow_manager_test.go @@ -374,16 +374,66 @@ func Test_getSpan_unbindSpan(t *testing.T) { } +func TestGetImageVersion(t *testing.T) { + tests := []struct { + name string + image string + want string + wantErr bool + }{ + { + name: "Return image version when version is present", + image: "my-image:1.0.0", + want: "1.0.0", + wantErr: false, + }, + { + name: "Return error when image version is not present", + image: "my-image", + want: "", + wantErr: true, + }, + { + name: "Return error when image version is empty", + image: "my-image:", + want: "", + wantErr: true, + }, + { + name: "Return error when image version is latest", + image: "my-image:latest", + want: "", + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := getImageVersion(tt.image) + if (err != nil) != tt.wantErr { + t.Errorf("getImageVersion() error = %v, wantErr %v", err, tt.wantErr) + return + } + if got != tt.want { + t.Errorf("getImageVersion() = %v, want %v", got, tt.want) + } + }) + } +} + func Test_calculateVersion(t *testing.T) { tests := []struct { - name string - pod *corev1.Pod - want string + name string + pod *corev1.Pod + containerName string + want string + wantErr bool }{ { - name: "empty pod", - pod: &corev1.Pod{}, - want: "2166136261", + name: "empty pod", + pod: &corev1.Pod{}, + containerName: "", + want: "2166136261", + wantErr: false, }, { name: "no containers", @@ -392,7 +442,9 @@ func Test_calculateVersion(t *testing.T) { Name: "pod-name", }, }, - want: "2166136261", + containerName: "", + want: "2166136261", + wantErr: false, }, { name: "single container", @@ -409,7 +461,9 @@ func Test_calculateVersion(t *testing.T) { }, }, }, - want: "tag", + containerName: "", + want: "tag", + wantErr: false, }, { name: "single container latest tag", @@ -426,7 +480,28 @@ func Test_calculateVersion(t *testing.T) { }, }, }, - want: "2894867514", + containerName: "", + want: "", + wantErr: true, + }, + { + name: "single container annotation mismatch", + pod: &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pod-name", + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "container-name", + Image: "image:latest", + }, + }, + }, + }, + containerName: "not-container-name", + want: "", + wantErr: true, }, { name: "multiple containers", @@ -447,7 +522,55 @@ func Test_calculateVersion(t *testing.T) { }, }, }, - want: "3235658121", + containerName: "", + want: "3235658121", + wantErr: false, + }, + { + name: "multiple containers with annotation", + pod: &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pod-name", + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "container-name", + Image: "image:tag1", + }, + { + Name: "container-name2", + Image: "image:tag2", + }, + }, + }, + }, + containerName: "container-name2", + want: "tag2", + wantErr: false, + }, + { + name: "multiple containers with annotation mismatch", + pod: &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pod-name", + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "container-name", + Image: "image:tag1", + }, + { + Name: "container-name2", + Image: "image:tag2", + }, + }, + }, + }, + containerName: "not-container-name", + want: "", + wantErr: true, }, { name: "multiple containers with env", @@ -488,14 +611,22 @@ func Test_calculateVersion(t *testing.T) { }, }, }, - want: "2484568705", + containerName: "", + want: "2484568705", + wantErr: false, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got := calculateVersion(tt.pod) - require.Equal(t, tt.want, got) + got, err := calculateVersion(tt.pod, tt.containerName) + if (err != nil) != tt.wantErr { + t.Errorf("calculateVersion() error = %v, wantErr %v", err, tt.wantErr) + return + } + if got != tt.want { + t.Errorf("calculateVersion() = %v, want %v", got, tt.want) + } }) } }