diff --git a/operator/apis/lifecycle/v1alpha3/common/common.go b/operator/apis/lifecycle/v1alpha3/common/common.go index 1fee458789..13872a60cb 100644 --- a/operator/apis/lifecycle/v1alpha3/common/common.go +++ b/operator/apis/lifecycle/v1alpha3/common/common.go @@ -9,6 +9,7 @@ import ( operatorcommon "github.com/keptn/lifecycle-toolkit/operator/common" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/metric" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) const WorkloadAnnotation = "keptn.sh/workload" @@ -200,3 +201,8 @@ func MergeMaps(m1 map[string]string, m2 map[string]string) map[string]string { } return merged } + +// IsOwnerSupported returns whether the owner of the given object is supported to be considered a KeptnWorklooad +func IsOwnerSupported(owner metav1.OwnerReference) bool { + return owner.Kind == "ReplicaSet" || owner.Kind == "Deployment" || owner.Kind == "StatefulSet" || owner.Kind == "DaemonSet" || owner.Kind == "Rollout" +} diff --git a/operator/apis/lifecycle/v1alpha3/common/common_test.go b/operator/apis/lifecycle/v1alpha3/common/common_test.go index 293f3f9251..c39eaed9fe 100644 --- a/operator/apis/lifecycle/v1alpha3/common/common_test.go +++ b/operator/apis/lifecycle/v1alpha3/common/common_test.go @@ -5,6 +5,7 @@ import ( "testing" "github.com/stretchr/testify/require" + v1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) const ExtraLongName = "loooooooooooooooooooooo00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000ooooooo01234567891234567890123456789" @@ -415,3 +416,76 @@ func Test_MergeMaps(t *testing.T) { }) } } + +func TestIsOwnerSupported(t *testing.T) { + type args struct { + owner v1.OwnerReference + } + tests := []struct { + name string + args args + want bool + }{ + { + name: "Deployment -> true", + args: args{ + owner: v1.OwnerReference{ + Kind: "Deployment", + }, + }, + want: true, + }, + { + name: "DaemonSet-> true", + args: args{ + owner: v1.OwnerReference{ + Kind: "DaemonSet", + }, + }, + want: true, + }, + { + name: "ReplicaSet-> true", + args: args{ + owner: v1.OwnerReference{ + Kind: "ReplicaSet", + }, + }, + want: true, + }, + { + name: "StatefulSet-> true", + args: args{ + owner: v1.OwnerReference{ + Kind: "StatefulSet", + }, + }, + want: true, + }, + { + name: "Rollout-> true", + args: args{ + owner: v1.OwnerReference{ + Kind: "Rollout", + }, + }, + want: true, + }, + { + name: "Job-> false", + args: args{ + owner: v1.OwnerReference{ + Kind: "Job", + }, + }, + want: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := IsOwnerSupported(tt.args.owner); got != tt.want { + t.Errorf("IsOwnerSupported() = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/operator/webhooks/pod_mutator/pod_mutating_webhook.go b/operator/webhooks/pod_mutator/pod_mutating_webhook.go index 3a43b61368..593862d48c 100644 --- a/operator/webhooks/pod_mutator/pod_mutating_webhook.go +++ b/operator/webhooks/pod_mutator/pod_mutating_webhook.go @@ -81,6 +81,15 @@ func (a *PodMutatingWebhook) Handle(ctx context.Context, req admission.Request) return admission.Allowed("namespace is not enabled for lifecycle controller") } + // check the OwnerReference of the pod to see if it is supported and intended to be managed by KLT + ownerRef := a.getOwnerReference(pod.ObjectMeta) + + if ownerRef.Kind == "" { + msg := "owner of pod is not supported by lifecycle controller" + logger.Info(msg, "namespace", req.Namespace, "pod", req.Name) + return admission.Allowed(msg) + } + logger.Info(fmt.Sprintf("Pod annotations: %v", pod.Annotations)) podIsAnnotated := a.isPodAnnotated(pod) @@ -139,7 +148,7 @@ func (a *PodMutatingWebhook) isPodAnnotated(pod *corev1.Pod) bool { } func (a *PodMutatingWebhook) copyAnnotationsIfParentAnnotated(ctx context.Context, req *admission.Request, pod *corev1.Pod) bool { - podOwner := a.getOwnerReference(&pod.ObjectMeta) + podOwner := a.getOwnerReference(pod.ObjectMeta) if podOwner.UID == "" { return false } @@ -152,7 +161,7 @@ func (a *PodMutatingWebhook) copyAnnotationsIfParentAnnotated(ctx context.Contex } a.Log.Info("Done fetching RS") - rsOwner := a.getOwnerReference(&rs.ObjectMeta) + rsOwner := a.getOwnerReference(rs.ObjectMeta) if rsOwner.UID == "" { return false } @@ -381,7 +390,7 @@ func (a *PodMutatingWebhook) generateWorkload(ctx context.Context, pod *corev1.P traceContextCarrier := propagation.MapCarrier{} otel.GetTextMapPropagator().Inject(ctx, traceContextCarrier) - ownerRef := a.getOwnerReference(&pod.ObjectMeta) + ownerRef := a.getOwnerReference(pod.ObjectMeta) return &klcv1alpha3.KeptnWorkload{ ObjectMeta: metav1.ObjectMeta{ @@ -441,11 +450,11 @@ func (a *PodMutatingWebhook) getAppName(pod *corev1.Pod) string { return strings.ToLower(applicationName) } -func (a *PodMutatingWebhook) getOwnerReference(resource *metav1.ObjectMeta) metav1.OwnerReference { +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" || owner.Kind == "Rollout" { + if apicommon.IsOwnerSupported(owner) { reference.UID = owner.UID reference.Kind = owner.Kind reference.Name = owner.Name diff --git a/operator/webhooks/pod_mutator/pod_mutating_webhook_test.go b/operator/webhooks/pod_mutator/pod_mutating_webhook_test.go index 7562b1d18b..764a79781c 100644 --- a/operator/webhooks/pod_mutator/pod_mutating_webhook_test.go +++ b/operator/webhooks/pod_mutator/pod_mutating_webhook_test.go @@ -17,6 +17,7 @@ import ( admissionv1 "k8s.io/api/admission/v1" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" @@ -34,7 +35,7 @@ func TestPodMutatingWebhook_getOwnerReference(t *testing.T) { Log logr.Logger } type args struct { - resource *metav1.ObjectMeta + resource metav1.ObjectMeta } tests := []struct { name string @@ -45,7 +46,7 @@ func TestPodMutatingWebhook_getOwnerReference(t *testing.T) { { name: "Test simple return when UID and Kind is set", args: args{ - resource: &metav1.ObjectMeta{ + resource: metav1.ObjectMeta{ UID: "the-pod-uid", OwnerReferences: []metav1.OwnerReference{ { @@ -65,7 +66,7 @@ func TestPodMutatingWebhook_getOwnerReference(t *testing.T) { { name: "Test return is input argument if owner is not found", args: args{ - resource: &metav1.ObjectMeta{ + resource: metav1.ObjectMeta{ UID: "the-pod-uid", OwnerReferences: []metav1.OwnerReference{ { @@ -1007,6 +1008,102 @@ func TestPodMutatingWebhook_Handle_DisabledNamespace(t *testing.T) { require.True(t, resp.Allowed) } +func TestPodMutatingWebhook_Handle_UnsupportedOwner(t *testing.T) { + fakeClient := fakeclient.NewClient(&corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "default", + Annotations: map[string]string{ + apicommon.NamespaceEnabledAnnotation: "enabled", + }, + }, + }) + + tr := &fakeclient.ITracerMock{StartFunc: func(ctx context.Context, spanName string, opts ...trace.SpanStartOption) (context.Context, trace.Span) { + return ctx, trace.SpanFromContext(ctx) + }} + + decoder, err := admission.NewDecoder(runtime.NewScheme()) + require.Nil(t, err) + + wh := &PodMutatingWebhook{ + Client: fakeClient, + Tracer: tr, + Decoder: decoder, + EventSender: controllercommon.NewEventSender(record.NewFakeRecorder(100)), + Log: testr.New(t), + } + + pod := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "example-pod", + Namespace: "default", + Annotations: map[string]string{ + apicommon.WorkloadAnnotation: "my-workload", + apicommon.VersionAnnotation: "0.1", + }, + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: "batchv1", + Kind: "Job", + Name: "my-job", + UID: "1234", + }, + }, + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "example-container", + Image: "nginx", + }, + }, + }, + } + + // Convert the Pod object to a byte array + podBytes, err := json.Marshal(pod) + require.Nil(t, err) + + // Create an AdmissionRequest object + request := admissionv1.AdmissionRequest{ + UID: "12345", + Kind: metav1.GroupVersionKind{Group: "", Version: "v1", Kind: "Pod"}, + Operation: admissionv1.Create, + Object: runtime.RawExtension{ + Raw: podBytes, + }, + Namespace: "default", + } + + resp := wh.Handle(context.TODO(), admission.Request{ + AdmissionRequest: request, + }) + + require.NotNil(t, resp) + require.True(t, resp.Allowed) + + // if we get an unsupported owner for the pod, we expect not to have any KLT resources to have been created + kacr := &klcv1alpha3.KeptnAppCreationRequest{} + + err = fakeClient.Get(context.Background(), types.NamespacedName{ + Namespace: "default", + Name: "my-workload", + }, kacr) + + require.NotNil(t, err) + require.True(t, errors.IsNotFound(err)) + + workload := &klcv1alpha3.KeptnWorkload{} + + err = fakeClient.Get(context.TODO(), types.NamespacedName{ + Namespace: "default", + Name: "my-workload-my-workload", + }, workload) + + require.NotNil(t, err) + require.True(t, errors.IsNotFound(err)) +} + func TestPodMutatingWebhook_Handle_SingleService(t *testing.T) { fakeClient := fakeclient.NewClient(&corev1.Namespace{ ObjectMeta: metav1.ObjectMeta{ diff --git a/test/integration/unsupported-owner-of-pod/00-assert.yaml b/test/integration/unsupported-owner-of-pod/00-assert.yaml new file mode 100644 index 0000000000..1280385103 --- /dev/null +++ b/test/integration/unsupported-owner-of-pod/00-assert.yaml @@ -0,0 +1,5 @@ +apiVersion: v1 +kind: Pod +metadata: + labels: + app: test diff --git a/test/integration/unsupported-owner-of-pod/00-install.yaml b/test/integration/unsupported-owner-of-pod/00-install.yaml new file mode 100644 index 0000000000..f933121e7e --- /dev/null +++ b/test/integration/unsupported-owner-of-pod/00-install.yaml @@ -0,0 +1,19 @@ +--- +apiVersion: batch/v1 +kind: Job +metadata: + name: test +spec: + template: + metadata: + labels: + app: test + annotations: + keptn.sh/workload: waiter + keptn.sh/version: "0.4" + spec: + restartPolicy: Never + containers: + - name: init-myservice + image: busybox:1.36.1 + command: ['sh', '-c', 'sleep 60'] diff --git a/test/integration/unsupported-owner-of-pod/00-teststep.yaml b/test/integration/unsupported-owner-of-pod/00-teststep.yaml new file mode 100644 index 0000000000..ad4f1d95d5 --- /dev/null +++ b/test/integration/unsupported-owner-of-pod/00-teststep.yaml @@ -0,0 +1,4 @@ +apiVersion: kuttl.dev/v1 +kind: TestStep +commands: + - script: kubectl annotate ns $NAMESPACE keptn.sh/lifecycle-toolkit='enabled'