From dd879d80769085168c5069a91f8c5bcb907f08bb Mon Sep 17 00:00:00 2001 From: Camila Macedo <7708031+camilamacedo86@users.noreply.github.com> Date: Mon, 9 Dec 2024 08:51:20 +0000 Subject: [PATCH] Add new validator for deprecate images This validator will help us to warning users about the usage of deprecated images found in the CSV --- pkg/validation/internal/deprecated_images.go | 70 +++++++++ .../internal/deprecated_images_test.go | 138 ++++++++++++++++++ pkg/validation/validation.go | 4 + 3 files changed, 212 insertions(+) create mode 100644 pkg/validation/internal/deprecated_images.go create mode 100644 pkg/validation/internal/deprecated_images_test.go diff --git a/pkg/validation/internal/deprecated_images.go b/pkg/validation/internal/deprecated_images.go new file mode 100644 index 000000000..19054aec4 --- /dev/null +++ b/pkg/validation/internal/deprecated_images.go @@ -0,0 +1,70 @@ +package internal + +import ( + "fmt" + "strings" + + "github.com/operator-framework/api/pkg/manifests" + "github.com/operator-framework/api/pkg/validation/errors" + interfaces "github.com/operator-framework/api/pkg/validation/interfaces" +) + +// DeprecatedImages contains a list of deprecated images and their respective messages +var DeprecatedImages = map[string]string{ + "gcr.io/kubebuilder/kube-rbac-proxy": "Your bundle uses the image `gcr.io/kubebuilder/kube-rbac-proxy`. This upstream image is deprecated and may become unavailable at any point. \n\nPlease use an equivalent image from a trusted source or update your approach to protect the metrics endpoint. \n\nFor further information: https://github.com/kubernetes-sigs/kubebuilder/discussions/3907", +} + +// ImageDeprecateValidator implements Validator to validate bundle objects for deprecated image usage. +var ImageDeprecateValidator interfaces.Validator = interfaces.ValidatorFunc(validateImageDeprecateValidator) + +// validateImageDeprecateValidator checks for the presence of deprecated images in the bundle's CSV and deployment specs. +func validateImageDeprecateValidator(objs ...interface{}) (results []errors.ManifestResult) { + for _, obj := range objs { + switch v := obj.(type) { + case *manifests.Bundle: + results = append(results, validateDeprecatedImage(v)) + } + } + return results +} + +// validateDeprecatedImage checks for deprecated images in both the CSV `RelatedImages` and deployment specs. +func validateDeprecatedImage(bundle *manifests.Bundle) errors.ManifestResult { + result := errors.ManifestResult{} + if bundle == nil { + result.Add(errors.ErrInvalidBundle("Bundle is nil", nil)) + return result + } + + result.Name = bundle.Name + + if bundle.CSV != nil { + for _, relatedImage := range bundle.CSV.Spec.RelatedImages { + for deprecatedImage, message := range DeprecatedImages { + if strings.Contains(relatedImage.Image, deprecatedImage) { + result.Add(errors.WarnFailedValidation( + fmt.Sprintf(message, relatedImage.Image), + relatedImage.Name, + )) + } + } + } + } + + if bundle.CSV != nil { + for _, deploymentSpec := range bundle.CSV.Spec.InstallStrategy.StrategySpec.DeploymentSpecs { + for _, container := range deploymentSpec.Spec.Template.Spec.Containers { + for deprecatedImage, message := range DeprecatedImages { + if strings.Contains(container.Image, deprecatedImage) { + result.Add(errors.WarnFailedValidation( + fmt.Sprintf(message, container.Image), + deploymentSpec.Name, + )) + } + } + } + } + } + + return result +} diff --git a/pkg/validation/internal/deprecated_images_test.go b/pkg/validation/internal/deprecated_images_test.go new file mode 100644 index 000000000..1584633df --- /dev/null +++ b/pkg/validation/internal/deprecated_images_test.go @@ -0,0 +1,138 @@ +package internal + +import ( + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" + "reflect" + "strings" + "testing" + + "github.com/operator-framework/api/pkg/manifests" + operatorsv1alpha1 "github.com/operator-framework/api/pkg/operators/v1alpha1" + "github.com/stretchr/testify/require" +) + +func TestValidateDeprecatedImage(t *testing.T) { + type args struct { + bundleDir string + } + tests := []struct { + name string + args args + modifyBundle func(bundle *manifests.Bundle) + wantErr []string + want map[string]string + }{ + { + name: "should return no error or warning for a valid bundle", + args: args{ + bundleDir: "./testdata/valid_bundle", + }, + modifyBundle: func(bundle *manifests.Bundle) { + // Do not modify the bundle + }, + wantErr: []string{}, + want: map[string]string{}, + }, + { + name: "should detect deprecated image in RelatedImages", + args: args{ + bundleDir: "./testdata/valid_bundle", + }, + modifyBundle: func(bundle *manifests.Bundle) { + bundle.CSV.Spec.RelatedImages = append(bundle.CSV.Spec.RelatedImages, operatorsv1alpha1.RelatedImage{ + Name: "kube-rbac-proxy", + Image: "gcr.io/kubebuilder/kube-rbac-proxy", + }) + }, + wantErr: []string{ + "Your bundle uses the image `gcr.io/kubebuilder/kube-rbac-proxy`", + }, + want: map[string]string{ + "RelatedImage": "gcr.io/kubebuilder/kube-rbac-proxy", + }, + }, + { + name: "should detect deprecated image in DeploymentSpecs", + args: args{ + bundleDir: "./testdata/valid_bundle", + }, + modifyBundle: func(bundle *manifests.Bundle) { + bundle.CSV.Spec.InstallStrategy.StrategySpec.DeploymentSpecs = append( + bundle.CSV.Spec.InstallStrategy.StrategySpec.DeploymentSpecs, + operatorsv1alpha1.StrategyDeploymentSpec{ + Name: "test-deployment", + Spec: appsv1.DeploymentSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "kube-rbac-proxy", + Image: "gcr.io/kubebuilder/kube-rbac-proxy", + }, + }, + }, + }, + }, + }, + ) + }, + wantErr: []string{ + "Your bundle uses the image `gcr.io/kubebuilder/kube-rbac-proxy`", + }, + want: map[string]string{ + "DeploymentSpec": "gcr.io/kubebuilder/kube-rbac-proxy", + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + bundle, err := manifests.GetBundleFromDir(tt.args.bundleDir) + require.NoError(t, err) + + tt.modifyBundle(bundle) + result := validateDeprecatedImage(bundle) + + var gotWarnings []string + for _, warn := range result.Warnings { + gotWarnings = append(gotWarnings, warn.Error()) + } + + for _, expectedErr := range tt.wantErr { + found := false + for _, gotWarning := range gotWarnings { + if strings.Contains(gotWarning, expectedErr) { + found = true + break + } + } + if !found { + t.Errorf("Expected warning containing '%s' but did not find it", expectedErr) + } + } + + gotImages := make(map[string]string) + if bundle.CSV != nil { + for _, relatedImage := range bundle.CSV.Spec.RelatedImages { + for deprecatedImage := range DeprecatedImages { + if relatedImage.Image == deprecatedImage { + gotImages["RelatedImage"] = deprecatedImage + } + } + } + for _, deploymentSpec := range bundle.CSV.Spec.InstallStrategy.StrategySpec.DeploymentSpecs { + for _, container := range deploymentSpec.Spec.Template.Spec.Containers { + for deprecatedImage := range DeprecatedImages { + if container.Image == deprecatedImage { + gotImages["DeploymentSpec"] = deprecatedImage + } + } + } + } + } + + require.True(t, reflect.DeepEqual(tt.want, gotImages)) + }) + } +} diff --git a/pkg/validation/validation.go b/pkg/validation/validation.go index d98237e05..8e19f0057 100644 --- a/pkg/validation/validation.go +++ b/pkg/validation/validation.go @@ -78,6 +78,9 @@ var GoodPracticesValidator = internal.GoodPracticesValidator // information check: https://olm.operatorframework.io/docs/advanced-tasks/ship-operator-supporting-multiarch/ var MultipleArchitecturesValidator = internal.MultipleArchitecturesValidator +// ImageDeprecateValidator implements Validator to validate Images Deprecated found in the CSV configuration. +var ImageDeprecateValidator = internal.ImageDeprecateValidator + // AllValidators implements Validator to validate all Operator manifest types. var AllValidators = interfaces.Validators{ PackageManifestValidator, @@ -93,6 +96,7 @@ var AllValidators = interfaces.Validators{ AlphaDeprecatedAPIsValidator, GoodPracticesValidator, MultipleArchitecturesValidator, + ImageDeprecateValidator, } var DefaultBundleValidators = interfaces.Validators{