From fbb97cbaa6ccf87a923a14d9484731ad71871748 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Kalke?= <56382792+MichalKalke@users.noreply.github.com> Date: Thu, 21 Dec 2023 15:16:08 +0100 Subject: [PATCH] Including failed images in the textual error message (#149) * Including failed images in the textual error message * Bump images --- charts/warden/values.yaml | 4 +- internal/admission/defaulting.go | 26 ++-- internal/admission/defaulting_test.go | 30 ++-- internal/admission/validation.go | 8 +- internal/admission/validation_test.go | 4 +- .../{admission => annotations}/annotations.go | 3 +- internal/controllers/pod_controller.go | 8 +- .../validate/mocks/ImageValidatorService.go | 15 +- internal/validate/mocks/NotaryRepoClient.go | 138 ++++++++++++++++-- internal/validate/mocks/PodValidator.go | 20 ++- internal/validate/mocks/RepoFactory.go | 20 ++- internal/validate/pod.go | 28 ++-- internal/validate/pod_test.go | 65 ++++++--- tests/{operator_test.go => warden_test.go} | 19 ++- 14 files changed, 295 insertions(+), 93 deletions(-) rename internal/{admission => annotations}/annotations.go (65%) rename tests/{operator_test.go => warden_test.go} (79%) diff --git a/charts/warden/values.yaml b/charts/warden/values.yaml index a756aa79..c5441a84 100644 --- a/charts/warden/values.yaml +++ b/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 diff --git a/internal/admission/defaulting.go b/internal/admission/defaulting.go index d4c8bc64..0fde00f0 100644 --- a/internal/admission/defaulting.go +++ b/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,23 +194,26 @@ 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: @@ -217,10 +221,10 @@ func podMarkersForValidationResult(result validate.ValidationResult, strictMode } } -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 } diff --git a/internal/admission/defaulting_test.go b/internal/admission/defaulting_test.go index 6c51d319..4f327a38 100644 --- a/internal/admission/defaulting_test.go +++ b/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", + }, + }) +} diff --git a/internal/admission/validation.go b/internal/admission/validation.go index 430eb8b6..c329ea97 100644 --- a/internal/admission/validation.go +++ b/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") } diff --git a/internal/admission/validation_test.go b/internal/admission/validation_test.go index 1818a661..c0439882 100644 --- a/internal/admission/validation_test.go +++ b/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), diff --git a/internal/admission/annotations.go b/internal/annotations/annotations.go similarity index 65% rename from internal/admission/annotations.go rename to internal/annotations/annotations.go index 0e802ed6..141a8261 100644 --- a/internal/admission/annotations.go +++ b/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" ) diff --git a/internal/controllers/pod_controller.go b/internal/controllers/pod_controller.go index d30a4d19..3ff7c6ea 100644 --- a/internal/controllers/pod_controller.go +++ b/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 "" diff --git a/internal/validate/mocks/ImageValidatorService.go b/internal/validate/mocks/ImageValidatorService.go index ae4b1731..1111081e 100644 --- a/internal/validate/mocks/ImageValidatorService.go +++ b/internal/validate/mocks/ImageValidatorService.go @@ -1,4 +1,4 @@ -// Code generated by mockery v2.16.0. DO NOT EDIT. +// Code generated by mockery v2.39.0. DO NOT EDIT. package mocks @@ -17,6 +17,10 @@ type ImageValidatorService struct { func (_m *ImageValidatorService) Validate(ctx context.Context, image string) error { ret := _m.Called(ctx, image) + if len(ret) == 0 { + panic("no return value specified for Validate") + } + var r0 error if rf, ok := ret.Get(0).(func(context.Context, string) error); ok { r0 = rf(ctx, image) @@ -27,13 +31,12 @@ func (_m *ImageValidatorService) Validate(ctx context.Context, image string) err return r0 } -type mockConstructorTestingTNewImageValidatorService interface { +// NewImageValidatorService creates a new instance of ImageValidatorService. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. +// The first argument is typically a *testing.T value. +func NewImageValidatorService(t interface { mock.TestingT Cleanup(func()) -} - -// NewImageValidatorService creates a new instance of ImageValidatorService. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. -func NewImageValidatorService(t mockConstructorTestingTNewImageValidatorService) *ImageValidatorService { +}) *ImageValidatorService { mock := &ImageValidatorService{} mock.Mock.Test(t) diff --git a/internal/validate/mocks/NotaryRepoClient.go b/internal/validate/mocks/NotaryRepoClient.go index f585e521..c9c0c4a0 100644 --- a/internal/validate/mocks/NotaryRepoClient.go +++ b/internal/validate/mocks/NotaryRepoClient.go @@ -1,4 +1,4 @@ -// Code generated by mockery v2.16.0. DO NOT EDIT. +// Code generated by mockery v2.39.0. DO NOT EDIT. package mocks @@ -22,6 +22,10 @@ type NotaryRepoClient struct { func (_m *NotaryRepoClient) AddDelegation(name data.RoleName, delegationKeys []data.PublicKey, paths []string) error { ret := _m.Called(name, delegationKeys, paths) + if len(ret) == 0 { + panic("no return value specified for AddDelegation") + } + var r0 error if rf, ok := ret.Get(0).(func(data.RoleName, []data.PublicKey, []string) error); ok { r0 = rf(name, delegationKeys, paths) @@ -36,6 +40,10 @@ func (_m *NotaryRepoClient) AddDelegation(name data.RoleName, delegationKeys []d func (_m *NotaryRepoClient) AddDelegationPaths(name data.RoleName, paths []string) error { ret := _m.Called(name, paths) + if len(ret) == 0 { + panic("no return value specified for AddDelegationPaths") + } + var r0 error if rf, ok := ret.Get(0).(func(data.RoleName, []string) error); ok { r0 = rf(name, paths) @@ -50,6 +58,10 @@ func (_m *NotaryRepoClient) AddDelegationPaths(name data.RoleName, paths []strin func (_m *NotaryRepoClient) AddDelegationRoleAndKeys(name data.RoleName, delegationKeys []data.PublicKey) error { ret := _m.Called(name, delegationKeys) + if len(ret) == 0 { + panic("no return value specified for AddDelegationRoleAndKeys") + } + var r0 error if rf, ok := ret.Get(0).(func(data.RoleName, []data.PublicKey) error); ok { r0 = rf(name, delegationKeys) @@ -71,6 +83,10 @@ func (_m *NotaryRepoClient) AddTarget(target *client.Target, roles ...data.RoleN _ca = append(_ca, _va...) ret := _m.Called(_ca...) + if len(ret) == 0 { + panic("no return value specified for AddTarget") + } + var r0 error if rf, ok := ret.Get(0).(func(*client.Target, ...data.RoleName) error); ok { r0 = rf(target, roles...) @@ -85,6 +101,10 @@ func (_m *NotaryRepoClient) AddTarget(target *client.Target, roles ...data.RoleN func (_m *NotaryRepoClient) ClearDelegationPaths(name data.RoleName) error { ret := _m.Called(name) + if len(ret) == 0 { + panic("no return value specified for ClearDelegationPaths") + } + var r0 error if rf, ok := ret.Get(0).(func(data.RoleName) error); ok { r0 = rf(name) @@ -99,7 +119,15 @@ func (_m *NotaryRepoClient) ClearDelegationPaths(name data.RoleName) error { func (_m *NotaryRepoClient) GetAllTargetMetadataByName(name string) ([]client.TargetSignedStruct, error) { ret := _m.Called(name) + if len(ret) == 0 { + panic("no return value specified for GetAllTargetMetadataByName") + } + var r0 []client.TargetSignedStruct + var r1 error + if rf, ok := ret.Get(0).(func(string) ([]client.TargetSignedStruct, error)); ok { + return rf(name) + } if rf, ok := ret.Get(0).(func(string) []client.TargetSignedStruct); ok { r0 = rf(name) } else { @@ -108,7 +136,6 @@ func (_m *NotaryRepoClient) GetAllTargetMetadataByName(name string) ([]client.Ta } } - var r1 error if rf, ok := ret.Get(1).(func(string) error); ok { r1 = rf(name) } else { @@ -122,7 +149,15 @@ func (_m *NotaryRepoClient) GetAllTargetMetadataByName(name string) ([]client.Ta func (_m *NotaryRepoClient) GetChangelist() (changelist.Changelist, error) { ret := _m.Called() + if len(ret) == 0 { + panic("no return value specified for GetChangelist") + } + var r0 changelist.Changelist + var r1 error + if rf, ok := ret.Get(0).(func() (changelist.Changelist, error)); ok { + return rf() + } if rf, ok := ret.Get(0).(func() changelist.Changelist); ok { r0 = rf() } else { @@ -131,7 +166,6 @@ func (_m *NotaryRepoClient) GetChangelist() (changelist.Changelist, error) { } } - var r1 error if rf, ok := ret.Get(1).(func() error); ok { r1 = rf() } else { @@ -145,6 +179,10 @@ func (_m *NotaryRepoClient) GetChangelist() (changelist.Changelist, error) { func (_m *NotaryRepoClient) GetCryptoService() signed.CryptoService { ret := _m.Called() + if len(ret) == 0 { + panic("no return value specified for GetCryptoService") + } + var r0 signed.CryptoService if rf, ok := ret.Get(0).(func() signed.CryptoService); ok { r0 = rf() @@ -161,7 +199,15 @@ func (_m *NotaryRepoClient) GetCryptoService() signed.CryptoService { func (_m *NotaryRepoClient) GetDelegationRoles() ([]data.Role, error) { ret := _m.Called() + if len(ret) == 0 { + panic("no return value specified for GetDelegationRoles") + } + var r0 []data.Role + var r1 error + if rf, ok := ret.Get(0).(func() ([]data.Role, error)); ok { + return rf() + } if rf, ok := ret.Get(0).(func() []data.Role); ok { r0 = rf() } else { @@ -170,7 +216,6 @@ func (_m *NotaryRepoClient) GetDelegationRoles() ([]data.Role, error) { } } - var r1 error if rf, ok := ret.Get(1).(func() error); ok { r1 = rf() } else { @@ -184,6 +229,10 @@ func (_m *NotaryRepoClient) GetDelegationRoles() ([]data.Role, error) { func (_m *NotaryRepoClient) GetGUN() data.GUN { ret := _m.Called() + if len(ret) == 0 { + panic("no return value specified for GetGUN") + } + var r0 data.GUN if rf, ok := ret.Get(0).(func() data.GUN); ok { r0 = rf() @@ -205,7 +254,15 @@ func (_m *NotaryRepoClient) GetTargetByName(name string, roles ...data.RoleName) _ca = append(_ca, _va...) ret := _m.Called(_ca...) + if len(ret) == 0 { + panic("no return value specified for GetTargetByName") + } + var r0 *client.TargetWithRole + var r1 error + if rf, ok := ret.Get(0).(func(string, ...data.RoleName) (*client.TargetWithRole, error)); ok { + return rf(name, roles...) + } if rf, ok := ret.Get(0).(func(string, ...data.RoleName) *client.TargetWithRole); ok { r0 = rf(name, roles...) } else { @@ -214,7 +271,6 @@ func (_m *NotaryRepoClient) GetTargetByName(name string, roles ...data.RoleName) } } - var r1 error if rf, ok := ret.Get(1).(func(string, ...data.RoleName) error); ok { r1 = rf(name, roles...) } else { @@ -235,6 +291,10 @@ func (_m *NotaryRepoClient) Initialize(rootKeyIDs []string, serverManagedRoles . _ca = append(_ca, _va...) ret := _m.Called(_ca...) + if len(ret) == 0 { + panic("no return value specified for Initialize") + } + var r0 error if rf, ok := ret.Get(0).(func([]string, ...data.RoleName) error); ok { r0 = rf(rootKeyIDs, serverManagedRoles...) @@ -256,6 +316,10 @@ func (_m *NotaryRepoClient) InitializeWithCertificate(rootKeyIDs []string, rootC _ca = append(_ca, _va...) ret := _m.Called(_ca...) + if len(ret) == 0 { + panic("no return value specified for InitializeWithCertificate") + } + var r0 error if rf, ok := ret.Get(0).(func([]string, []data.PublicKey, ...data.RoleName) error); ok { r0 = rf(rootKeyIDs, rootCerts, serverManagedRoles...) @@ -270,7 +334,15 @@ func (_m *NotaryRepoClient) InitializeWithCertificate(rootKeyIDs []string, rootC func (_m *NotaryRepoClient) ListRoles() ([]client.RoleWithSignatures, error) { ret := _m.Called() + if len(ret) == 0 { + panic("no return value specified for ListRoles") + } + var r0 []client.RoleWithSignatures + var r1 error + if rf, ok := ret.Get(0).(func() ([]client.RoleWithSignatures, error)); ok { + return rf() + } if rf, ok := ret.Get(0).(func() []client.RoleWithSignatures); ok { r0 = rf() } else { @@ -279,7 +351,6 @@ func (_m *NotaryRepoClient) ListRoles() ([]client.RoleWithSignatures, error) { } } - var r1 error if rf, ok := ret.Get(1).(func() error); ok { r1 = rf() } else { @@ -299,7 +370,15 @@ func (_m *NotaryRepoClient) ListTargets(roles ...data.RoleName) ([]*client.Targe _ca = append(_ca, _va...) ret := _m.Called(_ca...) + if len(ret) == 0 { + panic("no return value specified for ListTargets") + } + var r0 []*client.TargetWithRole + var r1 error + if rf, ok := ret.Get(0).(func(...data.RoleName) ([]*client.TargetWithRole, error)); ok { + return rf(roles...) + } if rf, ok := ret.Get(0).(func(...data.RoleName) []*client.TargetWithRole); ok { r0 = rf(roles...) } else { @@ -308,7 +387,6 @@ func (_m *NotaryRepoClient) ListTargets(roles ...data.RoleName) ([]*client.Targe } } - var r1 error if rf, ok := ret.Get(1).(func(...data.RoleName) error); ok { r1 = rf(roles...) } else { @@ -322,6 +400,10 @@ func (_m *NotaryRepoClient) ListTargets(roles ...data.RoleName) ([]*client.Targe func (_m *NotaryRepoClient) Publish() error { ret := _m.Called() + if len(ret) == 0 { + panic("no return value specified for Publish") + } + var r0 error if rf, ok := ret.Get(0).(func() error); ok { r0 = rf() @@ -336,6 +418,10 @@ func (_m *NotaryRepoClient) Publish() error { func (_m *NotaryRepoClient) RemoveDelegationKeys(name data.RoleName, keyIDs []string) error { ret := _m.Called(name, keyIDs) + if len(ret) == 0 { + panic("no return value specified for RemoveDelegationKeys") + } + var r0 error if rf, ok := ret.Get(0).(func(data.RoleName, []string) error); ok { r0 = rf(name, keyIDs) @@ -350,6 +436,10 @@ func (_m *NotaryRepoClient) RemoveDelegationKeys(name data.RoleName, keyIDs []st func (_m *NotaryRepoClient) RemoveDelegationKeysAndPaths(name data.RoleName, keyIDs []string, paths []string) error { ret := _m.Called(name, keyIDs, paths) + if len(ret) == 0 { + panic("no return value specified for RemoveDelegationKeysAndPaths") + } + var r0 error if rf, ok := ret.Get(0).(func(data.RoleName, []string, []string) error); ok { r0 = rf(name, keyIDs, paths) @@ -364,6 +454,10 @@ func (_m *NotaryRepoClient) RemoveDelegationKeysAndPaths(name data.RoleName, key func (_m *NotaryRepoClient) RemoveDelegationPaths(name data.RoleName, paths []string) error { ret := _m.Called(name, paths) + if len(ret) == 0 { + panic("no return value specified for RemoveDelegationPaths") + } + var r0 error if rf, ok := ret.Get(0).(func(data.RoleName, []string) error); ok { r0 = rf(name, paths) @@ -378,6 +472,10 @@ func (_m *NotaryRepoClient) RemoveDelegationPaths(name data.RoleName, paths []st func (_m *NotaryRepoClient) RemoveDelegationRole(name data.RoleName) error { ret := _m.Called(name) + if len(ret) == 0 { + panic("no return value specified for RemoveDelegationRole") + } + var r0 error if rf, ok := ret.Get(0).(func(data.RoleName) error); ok { r0 = rf(name) @@ -399,6 +497,10 @@ func (_m *NotaryRepoClient) RemoveTarget(targetName string, roles ...data.RoleNa _ca = append(_ca, _va...) ret := _m.Called(_ca...) + if len(ret) == 0 { + panic("no return value specified for RemoveTarget") + } + var r0 error if rf, ok := ret.Get(0).(func(string, ...data.RoleName) error); ok { r0 = rf(targetName, roles...) @@ -413,6 +515,10 @@ func (_m *NotaryRepoClient) RemoveTarget(targetName string, roles ...data.RoleNa func (_m *NotaryRepoClient) RotateKey(role data.RoleName, serverManagesKey bool, keyList []string) error { ret := _m.Called(role, serverManagesKey, keyList) + if len(ret) == 0 { + panic("no return value specified for RotateKey") + } + var r0 error if rf, ok := ret.Get(0).(func(data.RoleName, bool, []string) error); ok { r0 = rf(role, serverManagesKey, keyList) @@ -438,7 +544,15 @@ func (_m *NotaryRepoClient) Witness(roles ...data.RoleName) ([]data.RoleName, er _ca = append(_ca, _va...) ret := _m.Called(_ca...) + if len(ret) == 0 { + panic("no return value specified for Witness") + } + var r0 []data.RoleName + var r1 error + if rf, ok := ret.Get(0).(func(...data.RoleName) ([]data.RoleName, error)); ok { + return rf(roles...) + } if rf, ok := ret.Get(0).(func(...data.RoleName) []data.RoleName); ok { r0 = rf(roles...) } else { @@ -447,7 +561,6 @@ func (_m *NotaryRepoClient) Witness(roles ...data.RoleName) ([]data.RoleName, er } } - var r1 error if rf, ok := ret.Get(1).(func(...data.RoleName) error); ok { r1 = rf(roles...) } else { @@ -457,13 +570,12 @@ func (_m *NotaryRepoClient) Witness(roles ...data.RoleName) ([]data.RoleName, er return r0, r1 } -type mockConstructorTestingTNewNotaryRepoClient interface { +// NewNotaryRepoClient creates a new instance of NotaryRepoClient. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. +// The first argument is typically a *testing.T value. +func NewNotaryRepoClient(t interface { mock.TestingT Cleanup(func()) -} - -// NewNotaryRepoClient creates a new instance of NotaryRepoClient. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. -func NewNotaryRepoClient(t mockConstructorTestingTNewNotaryRepoClient) *NotaryRepoClient { +}) *NotaryRepoClient { mock := &NotaryRepoClient{} mock.Mock.Test(t) diff --git a/internal/validate/mocks/PodValidator.go b/internal/validate/mocks/PodValidator.go index 3f77fa6d..dcfe67ef 100644 --- a/internal/validate/mocks/PodValidator.go +++ b/internal/validate/mocks/PodValidator.go @@ -1,4 +1,4 @@ -// Code generated by mockery v2.16.0. DO NOT EDIT. +// Code generated by mockery v2.39.0. DO NOT EDIT. package mocks @@ -20,14 +20,21 @@ type PodValidator struct { func (_m *PodValidator) ValidatePod(ctx context.Context, pod *v1.Pod, ns *v1.Namespace) (validate.ValidationResult, error) { ret := _m.Called(ctx, pod, ns) + if len(ret) == 0 { + panic("no return value specified for ValidatePod") + } + var r0 validate.ValidationResult + var r1 error + if rf, ok := ret.Get(0).(func(context.Context, *v1.Pod, *v1.Namespace) (validate.ValidationResult, error)); ok { + return rf(ctx, pod, ns) + } if rf, ok := ret.Get(0).(func(context.Context, *v1.Pod, *v1.Namespace) validate.ValidationResult); ok { r0 = rf(ctx, pod, ns) } else { r0 = ret.Get(0).(validate.ValidationResult) } - var r1 error if rf, ok := ret.Get(1).(func(context.Context, *v1.Pod, *v1.Namespace) error); ok { r1 = rf(ctx, pod, ns) } else { @@ -37,13 +44,12 @@ func (_m *PodValidator) ValidatePod(ctx context.Context, pod *v1.Pod, ns *v1.Nam return r0, r1 } -type mockConstructorTestingTNewPodValidator interface { +// NewPodValidator creates a new instance of PodValidator. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. +// The first argument is typically a *testing.T value. +func NewPodValidator(t interface { mock.TestingT Cleanup(func()) -} - -// NewPodValidator creates a new instance of PodValidator. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. -func NewPodValidator(t mockConstructorTestingTNewPodValidator) *PodValidator { +}) *PodValidator { mock := &PodValidator{} mock.Mock.Test(t) diff --git a/internal/validate/mocks/RepoFactory.go b/internal/validate/mocks/RepoFactory.go index 4512b8d5..42055b1f 100644 --- a/internal/validate/mocks/RepoFactory.go +++ b/internal/validate/mocks/RepoFactory.go @@ -1,4 +1,4 @@ -// Code generated by mockery v2.16.0. DO NOT EDIT. +// Code generated by mockery v2.39.0. DO NOT EDIT. package mocks @@ -16,7 +16,15 @@ type RepoFactory struct { func (_m *RepoFactory) NewRepoClient(_a0 string, _a1 validate.NotaryConfig) (validate.NotaryRepoClient, error) { ret := _m.Called(_a0, _a1) + if len(ret) == 0 { + panic("no return value specified for NewRepoClient") + } + var r0 validate.NotaryRepoClient + var r1 error + if rf, ok := ret.Get(0).(func(string, validate.NotaryConfig) (validate.NotaryRepoClient, error)); ok { + return rf(_a0, _a1) + } if rf, ok := ret.Get(0).(func(string, validate.NotaryConfig) validate.NotaryRepoClient); ok { r0 = rf(_a0, _a1) } else { @@ -25,7 +33,6 @@ func (_m *RepoFactory) NewRepoClient(_a0 string, _a1 validate.NotaryConfig) (val } } - var r1 error if rf, ok := ret.Get(1).(func(string, validate.NotaryConfig) error); ok { r1 = rf(_a0, _a1) } else { @@ -35,13 +42,12 @@ func (_m *RepoFactory) NewRepoClient(_a0 string, _a1 validate.NotaryConfig) (val return r0, r1 } -type mockConstructorTestingTNewRepoFactory interface { +// NewRepoFactory creates a new instance of RepoFactory. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. +// The first argument is typically a *testing.T value. +func NewRepoFactory(t interface { mock.TestingT Cleanup(func()) -} - -// NewRepoFactory creates a new instance of RepoFactory. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. -func NewRepoFactory(t mockConstructorTestingTNewRepoFactory) *RepoFactory { +}) *RepoFactory { mock := &RepoFactory{} mock.Mock.Test(t) diff --git a/internal/validate/pod.go b/internal/validate/pod.go index f7fa3787..6e516348 100644 --- a/internal/validate/pod.go +++ b/internal/validate/pod.go @@ -8,13 +8,18 @@ import ( corev1 "k8s.io/api/core/v1" ) -type ValidationResult string +type ValidationStatus string + +type ValidationResult struct { + Status ValidationStatus + InvalidImages []string +} const ( - Invalid ValidationResult = "Invalid" - ServiceUnavailable ValidationResult = "ServiceUnavailable" - Valid ValidationResult = "Valid" - NoAction ValidationResult = "NoAction" + Invalid ValidationStatus = "Invalid" + ServiceUnavailable ValidationStatus = "ServiceUnavailable" + Valid ValidationStatus = "Valid" + NoAction ValidationStatus = "NoAction" ) //go:generate mockery --name PodValidator @@ -42,33 +47,33 @@ func (a *podValidator) ValidatePod(ctx context.Context, pod *corev1.Pod, ns *cor logger := helpers.LoggerFromCtx(ctx) if ns.Name != pod.Namespace { - return Invalid, errors.New("pod namespace mismatch with given namespace") + return ValidationResult{Invalid, nil}, errors.New("pod namespace mismatch with given namespace") } - matched := make(map[string]ValidationResult) - images := getAllImages(pod) admitResult := Valid + invalidImages := []string{} + for s := range images { result, err := a.validateImage(ctx, s) - matched[s] = result if result != Valid { admitResult = result + invalidImages = append(invalidImages, s) logger.With("image", s).Info(err.Error()) } } - return admitResult, nil + return ValidationResult{admitResult, invalidImages}, nil } func IsValidationEnabledForNS(ns *corev1.Namespace) bool { return ns.GetLabels()[pkg.NamespaceValidationLabel] == pkg.NamespaceValidationEnabled } -func (a *podValidator) validateImage(ctx context.Context, image string) (ValidationResult, error) { +func (a *podValidator) validateImage(ctx context.Context, image string) (ValidationStatus, error) { err := a.Validator.Validate(ctx, image) if err != nil { if pkg.ErrorCode(err) == pkg.UnknownResult { @@ -87,4 +92,3 @@ func getAllImages(pod *corev1.Pod) map[string]struct{} { } return images } - diff --git a/internal/validate/pod_test.go b/internal/validate/pod_test.go index 1a70140d..57ca760f 100644 --- a/internal/validate/pod_test.go +++ b/internal/validate/pod_test.go @@ -21,6 +21,8 @@ func TestValidatePod(t *testing.T) { validContainer := v1.Container{Name: "valid-image", Image: validImage} invalidImage := "invalidImage" invalidContainer := v1.Container{Name: "invalid-image", Image: invalidImage} + invalidImage2 := "invalidImage2" + invalidContainer2 := v1.Container{Name: "invalid-image2", Image: invalidImage2} longResp := "long" longRespContainer := v1.Container{Name: "invalid-image", Image: longResp} @@ -41,9 +43,10 @@ func TestValidatePod(t *testing.T) { }) testCases := []struct { - name string - pod *v1.Pod - expectedResult validate.ValidationResult + name string + pod *v1.Pod + expectedStatus validate.ValidationStatus + expectedFailedImages []string }{ { name: "pod has valid image", @@ -51,7 +54,8 @@ func TestValidatePod(t *testing.T) { Spec: v1.PodSpec{Containers: []v1.Container{ validContainer, }}}, - expectedResult: validate.Valid, + expectedStatus: validate.Valid, + expectedFailedImages: []string{}, }, { name: "pod has valid images", @@ -59,21 +63,42 @@ func TestValidatePod(t *testing.T) { Spec: v1.PodSpec{Containers: []v1.Container{ validContainer, validContainer, }}}, - expectedResult: validate.Valid, + expectedStatus: validate.Valid, + expectedFailedImages: []string{}, }, { name: "pod has invalid image", pod: &v1.Pod{ObjectMeta: metav1.ObjectMeta{Namespace: testNs}, Spec: v1.PodSpec{Containers: []v1.Container{ invalidContainer}}}, - expectedResult: validate.Invalid, + expectedStatus: validate.Invalid, + expectedFailedImages: []string{invalidImage}, + }, + { + name: "pod has two invalid images", + pod: &v1.Pod{ObjectMeta: metav1.ObjectMeta{Namespace: testNs}, + Spec: v1.PodSpec{Containers: []v1.Container{ + invalidContainer, + invalidContainer2}}}, + expectedStatus: validate.Invalid, + expectedFailedImages: []string{invalidImage, invalidImage2}, + }, + { + name: "pod has one invalid image", + pod: &v1.Pod{ObjectMeta: metav1.ObjectMeta{Namespace: testNs}, + Spec: v1.PodSpec{Containers: []v1.Container{ + validContainer, + invalidContainer}}}, + expectedStatus: validate.Invalid, + expectedFailedImages: []string{invalidImage}, }, { name: "image validator timeout", pod: &v1.Pod{ObjectMeta: metav1.ObjectMeta{Namespace: testNs}, Spec: v1.PodSpec{Containers: []v1.Container{ longRespContainer}}}, - expectedResult: validate.ServiceUnavailable, + expectedStatus: validate.ServiceUnavailable, + expectedFailedImages: []string{longResp}, }, { name: "pod has invalid image among otters", @@ -81,7 +106,8 @@ func TestValidatePod(t *testing.T) { Spec: v1.PodSpec{Containers: []v1.Container{ validContainer, validContainer, validContainer, invalidContainer, }}}, - expectedResult: validate.Invalid, + expectedStatus: validate.Invalid, + expectedFailedImages: []string{invalidImage}, }, { name: "pod has valid image in initContainers and containers", @@ -90,7 +116,8 @@ func TestValidatePod(t *testing.T) { InitContainers: []v1.Container{validContainer}, Containers: []v1.Container{validContainer}, }}, - expectedResult: validate.Valid, + expectedStatus: validate.Valid, + expectedFailedImages: []string{}, }, { name: "pod has invalid image in initContainers", @@ -99,7 +126,8 @@ func TestValidatePod(t *testing.T) { InitContainers: []v1.Container{invalidContainer}, Containers: []v1.Container{validContainer}, }}, - expectedResult: validate.Invalid, + expectedStatus: validate.Invalid, + expectedFailedImages: []string{invalidImage}, }, { name: "pod has invalid image among others images in initContainers", @@ -108,7 +136,8 @@ func TestValidatePod(t *testing.T) { InitContainers: []v1.Container{validContainer, validContainer, invalidContainer}, Containers: []v1.Container{validContainer}, }}, - expectedResult: validate.Invalid, + expectedStatus: validate.Invalid, + expectedFailedImages: []string{invalidImage}, }, } @@ -120,18 +149,20 @@ func TestValidatePod(t *testing.T) { pkg.NamespaceValidationLabel: pkg.NamespaceValidationEnabled, }}} - mockValidator := mocks.ImageValidatorService{} - mockValidator.Mock.On("Validate", mock.Anything, invalidImage).Return(errors.New("Invalid image")) - mockValidator.Mock.On("Validate", mock.Anything, validImage).Return(nil) - mockValidator.Mock.On("Validate", mock.Anything, longResp).Return(pkg.NewUnknownResultErr(nil)) + validatorSvcMock := mocks.ImageValidatorService{} + validatorSvcMock.Mock.On("Validate", mock.Anything, invalidImage).Return(errors.New("Invalid image")) + validatorSvcMock.Mock.On("Validate", mock.Anything, invalidImage2).Return(errors.New("Invalid image")) + validatorSvcMock.Mock.On("Validate", mock.Anything, validImage).Return(nil) + validatorSvcMock.Mock.On("Validate", mock.Anything, longResp).Return(pkg.NewUnknownResultErr(nil)) - podValidator := validate.NewPodValidator(&mockValidator) + podValidator := validate.NewPodValidator(&validatorSvcMock) //WHEN result, err := podValidator.ValidatePod(context.TODO(), testCase.pod, ns) //THEN require.NoError(t, err) - require.Equal(t, testCase.expectedResult, result) + require.Equal(t, testCase.expectedStatus, result.Status) + require.EqualValues(t, testCase.expectedFailedImages, result.InvalidImages) }) } } diff --git a/tests/operator_test.go b/tests/warden_test.go similarity index 79% rename from tests/operator_test.go rename to tests/warden_test.go index c447e542..1c897e9e 100644 --- a/tests/operator_test.go +++ b/tests/warden_test.go @@ -3,6 +3,7 @@ package tests import ( + "fmt" "github.com/kyma-project/warden/pkg" "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" @@ -48,7 +49,21 @@ func Test_PodInsideVerifiedNamespaceWithUntrustedImage_ShouldBeRejected(t *testi pod := tc.Pod().WithContainer(container).Build() err := tc.Create(pod) require.Error(t, err) - require.ErrorContains(t, err, "Pod images validation failed") + require.ErrorContains(t, err, fmt.Sprintf("Pod images %s validation failed", UntrustedImageName)) +} + +func Test_PodInsideVerifiedNamespaceWithUntrustedImages_ShouldBeRejected(t *testing.T) { + tc := th.NewTestContext(t, "warden-verified-namespace-untrusted-images"). + ValidationEnabled(true). + Initialize() + defer tc.Destroy() + + container1 := corev1.Container{Name: "test-container1", Image: UntrustedImageName} + container2 := corev1.Container{Name: "test-container2", Image: "nginx:1.24.0-perl"} + pod := tc.Pod().WithContainer(container1).WithContainer(container2).Build() + err := tc.Create(pod) + require.Error(t, err) + require.ErrorContains(t, err, fmt.Sprintf("Pod images %s, %s validation failed", UntrustedImageName, "nginx:1.24.0-perl")) } func Test_PodInsideVerifiedNamespaceWithTrustedImage_ShouldBeCreatedWithValidationLabel(t *testing.T) { @@ -105,5 +120,5 @@ func Test_UpdateVerifiedPodWithUntrustedImage_ShouldBeRejected(t *testing.T) { pod.Spec.Containers[0].Image = UntrustedImageName err = tc.Update(pod) require.Error(t, err) - require.ErrorContains(t, err, "Pod images validation failed") + require.ErrorContains(t, err, fmt.Sprintf("Pod images %s validation failed", UntrustedImageName)) }