Skip to content

Commit af1f4d2

Browse files
camilamacedo86camilamacedo86
authored
add new good-practices validator (#197)
1 parent 17c6616 commit af1f4d2

File tree

5 files changed

+196
-7
lines changed

5 files changed

+196
-7
lines changed

‎pkg/validation/internal/good_practices.go

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
package internal
2+
3+
import (
4+
goerrors "errors"
5+
"fmt"
6+
7+
"github.com/operator-framework/api/pkg/manifests"
8+
operatorsv1alpha1 "github.com/operator-framework/api/pkg/operators/v1alpha1"
9+
"github.com/operator-framework/api/pkg/validation/errors"
10+
interfaces "github.com/operator-framework/api/pkg/validation/interfaces"
11+
)
12+
13+
// GoodPracticesValidator validates the bundle against criteria and suggestions defined as
14+
// good practices for bundles under the operator-framework solutions. (You might give a
15+
// look at https://sdk.operatorframework.io/docs/best-practices/)
16+
//
17+
// This validator will raise an WARNING when:
18+
//
19+
// - The resources request for CPU and/or Memory are not defined for any of the containers found in the CSV
20+
var GoodPracticesValidator interfaces.Validator = interfaces.ValidatorFunc(goodPracticesValidator)
21+
22+
func goodPracticesValidator(objs ...interface{}) (results []errors.ManifestResult) {
23+
for _, obj := range objs {
24+
switch v := obj.(type) {
25+
case *manifests.Bundle:
26+
results = append(results, validateGoodPracticesFrom(v))
27+
}
28+
}
29+
return results
30+
}
31+
32+
func validateGoodPracticesFrom(bundle *manifests.Bundle) errors.ManifestResult {
33+
result := errors.ManifestResult{}
34+
if bundle == nil {
35+
result.Add(errors.ErrInvalidBundle("Bundle is nil", nil))
36+
return result
37+
}
38+
39+
result.Name = bundle.Name
40+
41+
if bundle.CSV == nil {
42+
result.Add(errors.ErrInvalidBundle("Bundle csv is nil", bundle.Name))
43+
return result
44+
}
45+
46+
errs, warns := validateResourceRequests(bundle.CSV)
47+
for _, err := range errs {
48+
result.Add(errors.ErrFailedValidation(err.Error(), bundle.CSV.GetName()))
49+
}
50+
for _, warn := range warns {
51+
result.Add(errors.WarnFailedValidation(warn.Error(), bundle.CSV.GetName()))
52+
}
53+
54+
return result
55+
}
56+
57+
// validateResourceRequests will return a WARN when the resource request is not set
58+
func validateResourceRequests(csv *operatorsv1alpha1.ClusterServiceVersion) (errs, warns []error) {
59+
if csv.Spec.InstallStrategy.StrategySpec.DeploymentSpecs == nil {
60+
errs = append(errs, goerrors.New("unable to find a deployment to install in the CSV"))
61+
return errs, warns
62+
}
63+
deploymentSpec := csv.Spec.InstallStrategy.StrategySpec.DeploymentSpecs
64+
65+
for _, dSpec := range deploymentSpec {
66+
for _, c := range dSpec.Spec.Template.Spec.Containers {
67+
if c.Resources.Requests == nil || !(len(c.Resources.Requests.Cpu().String()) != 0 && len(c.Resources.Requests.Memory().String()) != 0) {
68+
msg := fmt.Errorf("unable to find the resource requests for the container %s. It is recommended "+
69+
"to ensure the resource request for CPU and Memory. Be aware that for some clusters configurations "+
70+
"it is required to specify requests or limits for those values. Otherwise, the system or quota may "+
71+
"reject Pod creation. More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/", c.Name)
72+
warns = append(warns, msg)
73+
}
74+
}
75+
}
76+
return errs, warns
77+
}

‎pkg/validation/internal/good_practices_test.go

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
package internal
2+
3+
import (
4+
"github.com/operator-framework/api/pkg/manifests"
5+
"github.com/stretchr/testify/require"
6+
"testing"
7+
)
8+
9+
func Test_ValidateGoodPractices(t *testing.T) {
10+
bundleWithDeploymentSpecEmpty, _ := manifests.GetBundleFromDir("./testdata/valid_bundle")
11+
bundleWithDeploymentSpecEmpty.CSV.Spec.InstallStrategy.StrategySpec.DeploymentSpecs = nil
12+
13+
type args struct {
14+
bundleDir string
15+
bundle *manifests.Bundle
16+
}
17+
tests := []struct {
18+
name string
19+
args args
20+
wantError bool
21+
wantWarning bool
22+
errStrings []string
23+
warnStrings []string
24+
}{
25+
{
26+
name: "should pass successfully when the resource request is set for " +
27+
"all containers defined in the bundle",
28+
args: args{
29+
bundleDir: "./testdata/valid_bundle",
30+
},
31+
},
32+
{
33+
name: "should raise an waring when the resource request is NOT set for any of the containers defined in the bundle",
34+
wantWarning: true,
35+
warnStrings: []string{"Warning: Value memcached-operator.v0.0.1: unable to find the resource requests for the container kube-rbac-proxy. It is recommended to ensure the resource request for CPU and Memory. Be aware that for some clusters configurations it is required to specify requests or limits for those values. Otherwise, the system or quota may reject Pod creation. More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/",
36+
"Warning: Value memcached-operator.v0.0.1: unable to find the resource requests for the container manager. It is recommended to ensure the resource request for CPU and Memory. Be aware that for some clusters configurations it is required to specify requests or limits for those values. Otherwise, the system or quota may reject Pod creation. More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/"},
37+
args: args{
38+
bundleDir: "./testdata/valid_bundle_v1",
39+
},
40+
},
41+
{
42+
name: "should fail when the bundle is nil",
43+
wantError: true,
44+
args: args{
45+
bundle: nil,
46+
},
47+
errStrings: []string{"Error: : Bundle is nil"},
48+
},
49+
{
50+
name: "should fail when the bundle csv is nil",
51+
wantError: true,
52+
args: args{
53+
bundle: &manifests.Bundle{CSV: nil, Name: "test"},
54+
},
55+
errStrings: []string{"Error: Value test: Bundle csv is nil"},
56+
},
57+
{
58+
name: "should fail when the csv.Spec.InstallStrategy.StrategySpec.DeploymentSpecs is nil",
59+
wantError: true,
60+
args: args{
61+
bundle: bundleWithDeploymentSpecEmpty,
62+
},
63+
errStrings: []string{"Error: Value etcdoperator.v0.9.4: unable to find a deployment to install in the CSV"},
64+
},
65+
}
66+
67+
for _, tt := range tests {
68+
t.Run(tt.name, func(t *testing.T) {
69+
var err error
70+
if len(tt.args.bundleDir) > 0 {
71+
tt.args.bundle, err = manifests.GetBundleFromDir(tt.args.bundleDir)
72+
require.NoError(t, err)
73+
}
74+
results := validateGoodPracticesFrom(tt.args.bundle)
75+
require.Equal(t, tt.wantWarning, len(results.Warnings) > 0)
76+
if tt.wantWarning {
77+
require.Equal(t, len(tt.warnStrings), len(results.Warnings))
78+
for _, w := range results.Warnings {
79+
wString := w.Error()
80+
require.Contains(t, tt.warnStrings, wString)
81+
}
82+
}
83+
84+
require.Equal(t, tt.wantError, len(results.Errors) > 0)
85+
if tt.wantError {
86+
require.Equal(t, len(tt.errStrings), len(results.Errors))
87+
for _, err := range results.Errors {
88+
errString := err.Error()
89+
require.Contains(t, tt.errStrings, errString)
90+
}
91+
}
92+
})
93+
}
94+
}

‎pkg/validation/internal/testdata/valid_bundle/etcdoperator.v0.9.4.clusterserviceversion.yaml

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -207,6 +207,13 @@ spec:
207207
fieldPath: metadata.name
208208
image: quay.io/coreos/etcd-operator@sha256:66a37fd61a06a43969854ee6d3e21087a98b93838e284a6086b13917f96b0d9b
209209
name: etcd-operator
210+
resources:
211+
limits:
212+
cpu: 500m
213+
memory: 128Mi
214+
requests:
215+
cpu: 10m
216+
memory: 64Mi
210217
- command:
211218
- etcd-backup-operator
212219
- --create-crd=false
@@ -221,6 +228,13 @@ spec:
221228
fieldPath: metadata.name
222229
image: quay.io/coreos/etcd-operator@sha256:66a37fd61a06a43969854ee6d3e21087a98b93838e284a6086b13917f96b0d9b
223230
name: etcd-backup-operator
231+
resources:
232+
limits:
233+
cpu: 500m
234+
memory: 128Mi
235+
requests:
236+
cpu: 10m
237+
memory: 64Mi
224238
- command:
225239
- etcd-restore-operator
226240
- --create-crd=false
@@ -235,6 +249,13 @@ spec:
235249
fieldPath: metadata.name
236250
image: quay.io/coreos/etcd-operator@sha256:66a37fd61a06a43969854ee6d3e21087a98b93838e284a6086b13917f96b0d9b
237251
name: etcd-restore-operator
252+
resources:
253+
limits:
254+
cpu: 500m
255+
memory: 128Mi
256+
requests:
257+
cpu: 10m
258+
memory: 64Mi
238259
serviceAccountName: etcd-operator
239260
permissions:
240261
- rules:

‎pkg/validation/internal/testdata/valid_bundle_v1/memcached-operator.clusterserviceversion.yaml

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -144,13 +144,6 @@ spec:
144144
port: 8081
145145
initialDelaySeconds: 5
146146
periodSeconds: 10
147-
resources:
148-
limits:
149-
cpu: 100m
150-
memory: 30Mi
151-
requests:
152-
cpu: 100m
153-
memory: 20Mi
154147
securityContext:
155148
allowPrivilegeEscalation: false
156149
securityContext:

‎pkg/validation/validation.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,9 @@ var CommunityOperatorValidator = internal.CommunityOperatorValidator
5050
// for API deprecation requirements.
5151
var AlphaDeprecatedAPIsValidator = internal.AlphaDeprecatedAPIsValidator
5252

53+
// GoodPracticesValidator implements Validator to validate the criteria defined as good practices
54+
var GoodPracticesValidator = internal.GoodPracticesValidator
55+
5356
// AllValidators implements Validator to validate all Operator manifest types.
5457
var AllValidators = interfaces.Validators{
5558
PackageManifestValidator,
@@ -61,6 +64,7 @@ var AllValidators = interfaces.Validators{
6164
OperatorGroupValidator,
6265
CommunityOperatorValidator,
6366
AlphaDeprecatedAPIsValidator,
67+
GoodPracticesValidator,
6468
}
6569

6670
var DefaultBundleValidators = interfaces.Validators{

0 commit comments

Comments
 (0)