Including failed images in the textual error message (#149)

* Including failed images in the textual error message

* Bump images

charts/warden/values.yaml

@@ -20,7 +20,7 @@ global:
   wardenPriorityClassName: warden-priority
   wardenPriorityClassValue: 2000000
   operator:
-    image: europe-docker.pkg.dev/kyma-project/prod/warden/operator:v20231011-fe44448b
+    image: europe-docker.pkg.dev/kyma-project/dev/warden/operator:PR-149
     resources:
       requests:
         cpu: 10m
@@ -30,7 +30,7 @@ global:
         memory: 160Mi
 
   admission:
-    image: europe-docker.pkg.dev/kyma-project/prod/warden/admission:v20231011-fe44448b
+    image: europe-docker.pkg.dev/kyma-project/dev/warden/admission:PR-149
     resources:
       requests:
         cpu: 10m

internal/admission/defaulting.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"github.com/kyma-project/warden/internal/annotations"
 	"github.com/kyma-project/warden/internal/helpers"
 	"github.com/kyma-project/warden/internal/validate"
 	"github.com/kyma-project/warden/pkg"
@@ -15,6 +16,7 @@ import (
 	"net/http"
 	k8sclient "sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
+	"strings"
 	"time"
 )
 
@@ -77,7 +79,7 @@ func (w *DefaultingWebHook) handle(ctx context.Context, req admission.Request) a
 	if err != nil {
 		return admission.Errored(http.StatusInternalServerError, err)
 	}
-	if result == validate.NoAction {
+	if result.Status == validate.NoAction {
 		return admission.Allowed("validation is not enabled for pod")
 	}
 	res := w.createResponse(ctx, req, result, pod, logger)
@@ -107,8 +109,7 @@ func (w DefaultingWebHook) handleTimeout(ctx context.Context, timeoutErr error,
 	msg := fmt.Sprintf("request exceeded desired timeout: %s, reason: %s", w.timeout.String(), timeoutErr.Error())
 	logger := helpers.LoggerFromCtx(ctx)
 	logger.Info(msg)
-
-	res := w.createResponse(ctx, req, validate.ServiceUnavailable, pod, logger)
+	res := w.createResponse(ctx, req, validate.ValidationResult{Status: validate.ServiceUnavailable}, pod, logger)
 	res.Result = &metav1.Status{Message: msg}
 	return res
 }
@@ -173,7 +174,7 @@ func (w *DefaultingWebHook) InjectDecoder(decoder *admission.Decoder) error {
 }
 
 func markPod(ctx context.Context, result validate.ValidationResult, pod *corev1.Pod, strictMode bool) *corev1.Pod {
-	label, annotation := podMarkersForValidationResult(result, strictMode)
+	label, annotation := podMarkersForValidationResult(result.Status, strictMode)
 	helpers.LoggerFromCtx(ctx).Infof("pod was labeled: `%s` and annotated: `%s`", label, annotation)
 	if label == "" && annotation == "" {
 		return pod
@@ -193,34 +194,37 @@ func markPod(ctx context.Context, result validate.ValidationResult, pod *corev1.
 		if markedPod.Annotations == nil {
 			markedPod.Annotations = map[string]string{}
 		}
-		markedPod.Annotations[PodValidationRejectAnnotation] = annotation
+		markedPod.Annotations[annotations.PodValidationRejectAnnotation] = annotation
+		if result.InvalidImages != nil {
+			markedPod.Annotations[annotations.InvalidImagesAnnotation] = strings.Join(result.InvalidImages, ", ")
+		}
 	}
 	return markedPod
 }
 
-func podMarkersForValidationResult(result validate.ValidationResult, strictMode bool) (label string, annotation string) {
+func podMarkersForValidationResult(result validate.ValidationStatus, strictMode bool) (label string, annotation string) {
 	switch result {
 	case validate.NoAction:
 		return "", ""
 	case validate.Invalid:
-		return pkg.ValidationStatusFailed, ValidationReject
+		return pkg.ValidationStatusFailed, annotations.ValidationReject
 	case validate.Valid:
 		return pkg.ValidationStatusSuccess, ""
 	case validate.ServiceUnavailable:
 		annotation = ""
 		if strictMode {
-			annotation = ValidationReject
+			annotation = annotations.ValidationReject
 		}
 		return pkg.ValidationStatusPending, annotation
 	default:
 		return pkg.ValidationStatusPending, ""
 	}
 }
 
-func removeInternalAnnotation(ctx context.Context, annotations map[string]string) bool {
+func removeInternalAnnotation(ctx context.Context, source map[string]string) bool {
 	logger := helpers.LoggerFromCtx(ctx)
-	if _, ok := annotations[PodValidationRejectAnnotation]; ok {
-		delete(annotations, PodValidationRejectAnnotation)
+	if _, ok := source[annotations.PodValidationRejectAnnotation]; ok {
+		delete(source, annotations.PodValidationRejectAnnotation)
 		logger.Debug("Internal Annotation deleted")
 		return true
 	}

internal/admission/defaulting_test.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"encoding/json"
 	"errors"
+	"github.com/kyma-project/warden/internal/annotations"
 	"github.com/kyma-project/warden/internal/test_helpers"
 	"github.com/kyma-project/warden/internal/validate"
 	"github.com/kyma-project/warden/internal/validate/mocks"
@@ -63,7 +64,7 @@ func TestTimeout(t *testing.T) {
 		validationSvc := mocks.NewPodValidator(t)
 		validationSvc.On("ValidatePod", mock.Anything, mock.Anything, mock.Anything).
 			After(timeout/2).
-			Return(validate.Valid, nil).Once()
+			Return(validate.ValidationResult{Status: validate.Valid}, nil).Once()
 		defer validationSvc.AssertExpectations(t)
 		webhook := NewDefaultingWebhook(client, validationSvc, timeout, StrictModeOff, logger.Sugar())
 		require.NoError(t, webhook.InjectDecoder(decoder))
@@ -81,7 +82,7 @@ func TestTimeout(t *testing.T) {
 		validationSvc := mocks.NewPodValidator(t)
 		validationSvc.On("ValidatePod", mock.Anything, mock.Anything, mock.Anything).
 			After(timeout*2).
-			Return(validate.Valid, nil).Once()
+			Return(validate.ValidationResult{Status: validate.Valid}, nil).Once()
 		defer validationSvc.AssertExpectations(t)
 		webhook := NewDefaultingWebhook(client, validationSvc, timeout, StrictModeOff, logger.Sugar())
 		require.NoError(t, webhook.InjectDecoder(decoder))
@@ -100,7 +101,7 @@ func TestTimeout(t *testing.T) {
 		validationSvc := mocks.NewPodValidator(t)
 		validationSvc.On("ValidatePod", mock.Anything, mock.Anything, mock.Anything).
 			After(timeout*2).
-			Return(validate.Valid, nil).Once()
+			Return(validate.ValidationResult{Status: validate.Valid}, nil).Once()
 		defer validationSvc.AssertExpectations(t)
 		webhook := NewDefaultingWebhook(client, validationSvc, timeout, StrictModeOn, logger.Sugar())
 		require.NoError(t, webhook.InjectDecoder(decoder))
@@ -187,7 +188,7 @@ func TestFlow_OutputStatuses_ForPodValidationResult(t *testing.T) {
 		mockPodValidator := validate.NewPodValidator(&mockImageValidator)
 
 		pod := newPodFix(nsName, nil)
-		pod.ObjectMeta.Annotations = map[string]string{PodValidationRejectAnnotation: ValidationReject}
+		pod.ObjectMeta.Annotations = map[string]string{annotations.PodValidationRejectAnnotation: annotations.ValidationReject}
 		req := newRequestFix(t, pod, admissionv1.Create)
 		client := fake.NewClientBuilder().WithScheme(scheme).WithObjects(&ns).Build()
 
@@ -207,7 +208,7 @@ func TestFlow_OutputStatuses_ForPodValidationResult(t *testing.T) {
 	t.Run("when pod labeled by ns controller with pending label and annotation reject should remove the annotation", func(t *testing.T) {
 		//GIVEN
 		pod := newPodFix(nsName, map[string]string{pkg.PodValidationLabel: pkg.ValidationStatusPending})
-		pod.ObjectMeta.Annotations = map[string]string{PodValidationRejectAnnotation: ValidationReject}
+		pod.ObjectMeta.Annotations = map[string]string{annotations.PodValidationRejectAnnotation: annotations.ValidationReject}
 		req := newRequestFix(t, pod, admissionv1.Update)
 		client := fake.NewClientBuilder().WithScheme(scheme).WithObjects(&ns).Build()
 
@@ -223,7 +224,7 @@ func TestFlow_OutputStatuses_ForPodValidationResult(t *testing.T) {
 		require.ElementsMatch(t, withRemovedAnnotation([]jsonpatch.JsonPatchOperation{}), res.Patches)
 	})
 
-	t.Run("when invalid image should return failed and annotation reject", func(t *testing.T) {
+	t.Run("when invalid image should return failed and annotation reject with images list", func(t *testing.T) {
 		//GIVEN
 		mockImageValidator := mocks.ImageValidatorService{}
 		mockImageValidator.Mock.On("Validate", mock.Anything, "test:test").
@@ -244,14 +245,14 @@ func TestFlow_OutputStatuses_ForPodValidationResult(t *testing.T) {
 		mockImageValidator.AssertNumberOfCalls(t, "Validate", 1)
 		require.NotNil(t, res)
 		require.True(t, res.AdmissionResponse.Allowed)
-		require.ElementsMatch(t, withAddRejectAnnotation(patchWithAddLabel(pkg.ValidationStatusFailed)), res.Patches)
+		require.ElementsMatch(t, withAddRejectAndImagesAnnotation(patchWithAddLabel(pkg.ValidationStatusFailed)), res.Patches)
 	})
 
 	t.Run("when service unavailable and strict mode on should return pending and annotation reject", func(t *testing.T) {
 		//GIVEN
 		mockPodValidator := mocks.NewPodValidator(t)
 		mockPodValidator.On("ValidatePod", mock.Anything, mock.Anything, mock.Anything).
-			Return(validate.ServiceUnavailable, nil)
+			Return(validate.ValidationResult{Status: validate.ServiceUnavailable}, nil)
 		defer mockPodValidator.AssertExpectations(t)
 
 		pod := newPodFix(nsName, nil)
@@ -275,7 +276,7 @@ func TestFlow_OutputStatuses_ForPodValidationResult(t *testing.T) {
 		//GIVEN
 		mockPodValidator := mocks.NewPodValidator(t)
 		mockPodValidator.On("ValidatePod", mock.Anything, mock.Anything, mock.Anything).
-			Return(validate.ServiceUnavailable, nil)
+			Return(validate.ValidationResult{Status: validate.ServiceUnavailable}, nil)
 		defer mockPodValidator.AssertExpectations(t)
 
 		pod := newPodFix(nsName, nil)
@@ -516,3 +517,14 @@ func withAddRejectAnnotation(patch []jsonpatch.JsonPatchOperation) []jsonpatch.J
 		},
 	})
 }
+
+func withAddRejectAndImagesAnnotation(patch []jsonpatch.JsonPatchOperation) []jsonpatch.JsonPatchOperation {
+	return append(patch, jsonpatch.JsonPatchOperation{
+		Operation: "add",
+		Path:      "/metadata/annotations",
+		Value: map[string]interface{}{
+			"pods.warden.kyma-project.io/invalid-images":  "test:test",
+			"pods.warden.kyma-project.io/validate-reject": "reject",
+		},
+	})
+}

internal/admission/validation.go

@@ -2,6 +2,8 @@ package admission
 
 import (
 	"context"
+	"fmt"
+	"github.com/kyma-project/warden/internal/annotations"
 	"github.com/kyma-project/warden/internal/helpers"
 	"github.com/pkg/errors"
 	"go.uber.org/zap"
@@ -51,11 +53,15 @@ func (w *ValidationWebhook) handle(ctx context.Context, req admission.Request) a
 		return admission.Allowed("nothing to do")
 	}
 
-	if pod.Annotations[PodValidationRejectAnnotation] != ValidationReject {
+	if pod.Annotations[annotations.PodValidationRejectAnnotation] != annotations.ValidationReject {
 		return admission.Allowed("nothing to do")
 	}
 
 	logger.Info("Pod images validation failed")
+	if _, ok := pod.Annotations[annotations.InvalidImagesAnnotation]; ok {
+		return admission.Denied(fmt.Sprintf("Pod images %s validation failed", pod.Annotations[annotations.InvalidImagesAnnotation]))
+	}
+
 	return admission.Denied("Pod images validation failed")
 }
 

internal/admission/validation_test.go

@@ -3,6 +3,7 @@ package admission
 import (
 	"context"
 	"encoding/json"
+	"github.com/kyma-project/warden/internal/annotations"
 	"github.com/kyma-project/warden/internal/test_helpers"
 	"github.com/kyma-project/warden/pkg"
 	"github.com/stretchr/testify/assert"
@@ -36,7 +37,8 @@ func TestValidationWebhook(t *testing.T) {
 			pod: &corev1.Pod{
 				ObjectMeta: metav1.ObjectMeta{Name: "test-pod",
 					Annotations: map[string]string{
-						PodValidationRejectAnnotation: ValidationReject,
+						annotations.PodValidationRejectAnnotation: annotations.ValidationReject,
+						annotations.InvalidImagesAnnotation:       annotations.InvalidImagesAnnotation,
 					}},
 			},
 			expectedStatus:  int32(http.StatusForbidden),

internal/admission/annotations.go → internal/annotations/annotations.go

@@ -1,7 +1,8 @@
-package admission
+package annotations
 
 const (
 	// Reject is used to pass status between webhooks
 	PodValidationRejectAnnotation = "pods.warden.kyma-project.io/validate-reject"
+	InvalidImagesAnnotation       = "pods.warden.kyma-project.io/invalid-images"
 	ValidationReject              = "reject"
 )

internal/controllers/pod_controller.go

@@ -131,7 +131,7 @@ func (r *PodReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.R
 	return shouldRetry, nil
 }
 
-func (r *PodReconciler) checkPod(ctx context.Context, pod *corev1.Pod) (validate.ValidationResult, error) {
+func (r *PodReconciler) checkPod(ctx context.Context, pod *corev1.Pod) (validate.ValidationStatus, error) {
 	var ns corev1.Namespace
 	if err := r.client.Get(ctx, client.ObjectKey{Name: pod.Namespace}, &ns); err != nil {
 		return validate.NoAction, err
@@ -142,10 +142,10 @@ func (r *PodReconciler) checkPod(ctx context.Context, pod *corev1.Pod) (validate
 		return validate.NoAction, err
 	}
 
-	return result, nil
+	return result.Status, nil
 }
 
-func (r *PodReconciler) labelPod(ctx context.Context, pod corev1.Pod, result validate.ValidationResult) error {
+func (r *PodReconciler) labelPod(ctx context.Context, pod corev1.Pod, result validate.ValidationStatus) error {
 
 	resultLabel := labelForValidationResult(result)
 	if resultLabel == "" {
@@ -199,7 +199,7 @@ func (r *PodReconciler) isValidationEnabledForNS(namespace string) bool {
 	return validate.IsValidationEnabledForNS(&ns)
 }
 
-func labelForValidationResult(result validate.ValidationResult) string {
+func labelForValidationResult(result validate.ValidationStatus) string {
 	switch result {
 	case validate.NoAction:
 		return ""