Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add new validator for deprecate images #380

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 70 additions & 0 deletions pkg/validation/internal/deprecated_images.go
Original file line number Diff line number Diff line change
@@ -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",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It allows us add more images/messages in the future if we need to do so

}

// 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
}
138 changes: 138 additions & 0 deletions pkg/validation/internal/deprecated_images_test.go
Original file line number Diff line number Diff line change
@@ -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))
})
}
}
4 changes: 4 additions & 0 deletions pkg/validation/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -93,6 +96,7 @@ var AllValidators = interfaces.Validators{
AlphaDeprecatedAPIsValidator,
GoodPracticesValidator,
MultipleArchitecturesValidator,
ImageDeprecateValidator,
}

var DefaultBundleValidators = interfaces.Validators{
Expand Down
Loading