Skip to content

Commit 491ea01

Browse files
openshift-merge-robotopenshift-merge-robot
authored
Merge pull request #278 from timflannagan/sync/04-05
sync: Update staging directories 04-05
2 parents 7e4eda4 + 87cb590 commit 491ea01

File tree

24 files changed

+845
-316
lines changed

24 files changed

+845
-316
lines changed

‎staging/api/pkg/manifests/bundleloader.go

Lines changed: 33 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,10 @@ import (
1919

2020
// bundleLoader loads a bundle directory from disk
2121
type bundleLoader struct {
22-
dir string
23-
bundle *Bundle
24-
foundCSV bool
22+
dir string
23+
bundle *Bundle
24+
foundCSV bool
25+
annotationsFile AnnotationsFile
2526
}
2627

2728
func NewBundleLoader(dir string) bundleLoader {
@@ -37,6 +38,7 @@ func (b *bundleLoader) LoadBundle() error {
3738
}
3839

3940
errs = append(errs, b.calculateCompressedBundleSize())
41+
b.addChannelsFromAnnotationsFile()
4042

4143
if !b.foundCSV {
4244
errs = append(errs, fmt.Errorf("unable to find a csv in bundle directory %s", b.dir))
@@ -47,6 +49,21 @@ func (b *bundleLoader) LoadBundle() error {
4749
return utilerrors.NewAggregate(errs)
4850
}
4951

52+
// Add values from the annotations when the values are not loaded
53+
func (b *bundleLoader) addChannelsFromAnnotationsFile() {
54+
// Note that they will not get load for Bundle Format directories
55+
// and PackageManifest should not have the annotationsFile. However,
56+
// the following check to ensure that channels and default channels
57+
// are empty before set the annotations is just an extra precaution
58+
channels := strings.Split(b.annotationsFile.Annotations.Channels, ",")
59+
if len(channels) > 0 && len(b.bundle.Channels) == 0 {
60+
b.bundle.Channels = channels
61+
}
62+
if len(b.annotationsFile.Annotations.DefaultChannelName) > 0 && len(b.bundle.DefaultChannel) == 0 {
63+
b.bundle.DefaultChannel = b.annotationsFile.Annotations.DefaultChannelName
64+
}
65+
}
66+
5067
// Compress the bundle to check its size
5168
func (b *bundleLoader) calculateCompressedBundleSize() error {
5269
if b.bundle == nil {
@@ -108,6 +125,19 @@ func (b *bundleLoader) LoadBundleWalkFunc(path string, f os.FileInfo, err error)
108125
return nil
109126
}
110127

128+
annotationsFile := AnnotationsFile{}
129+
if strings.HasPrefix(f.Name(), "annotations") {
130+
annFile, err := os.ReadFile(path)
131+
if err != nil {
132+
return err
133+
}
134+
if err := yaml.Unmarshal(annFile, &annotationsFile); err == nil {
135+
b.annotationsFile = annotationsFile
136+
} else {
137+
return fmt.Errorf("unable to load the annotations file %s: %s", path, err)
138+
}
139+
}
140+
111141
fileReader, err := os.Open(path)
112142
if err != nil {
113143
return fmt.Errorf("unable to load file %s: %s", path, err)

‎staging/api/pkg/validation/internal/good_practices.go

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package internal
33
import (
44
goerrors "errors"
55
"fmt"
6+
"strings"
67

78
"github.com/operator-framework/api/pkg/manifests"
89
operatorsv1alpha1 "github.com/operator-framework/api/pkg/operators/v1alpha1"
@@ -17,6 +18,9 @@ import (
1718
// This validator will raise an WARNING when:
1819
//
1920
// - The resources request for CPU and/or Memory are not defined for any of the containers found in the CSV
21+
//
22+
// - The channel names seems are not following the convention https://olm.operatorframework.io/docs/best-practices/channel-naming/
23+
//
2024
var GoodPracticesValidator interfaces.Validator = interfaces.ValidatorFunc(goodPracticesValidator)
2125

2226
func goodPracticesValidator(objs ...interface{}) (results []errors.ManifestResult) {
@@ -50,7 +54,14 @@ func validateGoodPracticesFrom(bundle *manifests.Bundle) errors.ManifestResult {
5054
for _, warn := range warns {
5155
result.Add(errors.WarnFailedValidation(warn.Error(), bundle.CSV.GetName()))
5256
}
57+
for _, warn := range validateCrdDescriptions(bundle.CSV.Spec.CustomResourceDefinitions) {
58+
result.Add(errors.WarnFailedValidation(warn.Error(), bundle.CSV.GetName()))
59+
}
5360

61+
channels := append(bundle.Channels, bundle.DefaultChannel)
62+
if warn := validateHubChannels(channels); warn != nil {
63+
result.Add(errors.WarnFailedValidation(warn.Error(), bundle.CSV.GetName()))
64+
}
5465
return result
5566
}
5667

@@ -75,3 +86,61 @@ func validateResourceRequests(csv *operatorsv1alpha1.ClusterServiceVersion) (err
7586
}
7687
return errs, warns
7788
}
89+
90+
// validateHubChannels will check the channels. The motivation for the following check is to ensure that operators
91+
// authors knows if their operator bundles are or not respecting the Naming Convention Rules.
92+
// However, the operator authors still able to choose the names as please them.
93+
func validateHubChannels(channels []string) error {
94+
const candidate = "candidate"
95+
const stable = "stable"
96+
const fast = "fast"
97+
98+
channels = getUniqueValues(channels)
99+
var channelsNotFollowingConventional []string
100+
for _, channel := range channels {
101+
if !strings.HasPrefix(channel, candidate) &&
102+
!strings.HasPrefix(channel, stable) &&
103+
!strings.HasPrefix(channel, fast) &&
104+
channel != "" {
105+
channelsNotFollowingConventional = append(channelsNotFollowingConventional, channel)
106+
}
107+
108+
}
109+
110+
if len(channelsNotFollowingConventional) > 0 {
111+
return fmt.Errorf("channel(s) %+q are not following the recommended naming convention: "+
112+
"https://olm.operatorframework.io/docs/best-practices/channel-naming",
113+
channelsNotFollowingConventional)
114+
}
115+
116+
return nil
117+
}
118+
119+
// getUniqueValues return the values without duplicates
120+
func getUniqueValues(array []string) []string {
121+
var result []string
122+
uniqueValues := make(map[string]string)
123+
for _, n := range array {
124+
uniqueValues[strings.TrimSpace(n)] = ""
125+
}
126+
127+
for k, _ := range uniqueValues {
128+
result = append(result, k)
129+
}
130+
return result
131+
}
132+
133+
// validateCrdDescrptions ensures that all CRDs defined in the bundle have non-empty descriptions.
134+
func validateCrdDescriptions(crds operatorsv1alpha1.CustomResourceDefinitions) []error {
135+
f := func(crds []operatorsv1alpha1.CRDDescription, relation string) []error {
136+
errors := make([]error, 0, len(crds))
137+
for _, crd := range crds {
138+
if crd.Description == "" {
139+
errors = append(errors, fmt.Errorf("%s CRD %q has an empty description", relation, crd.Name))
140+
}
141+
}
142+
return errors
143+
}
144+
145+
return append(f(crds.Owned, "owned"), f(crds.Required, "required")...)
146+
}

‎staging/api/pkg/validation/internal/good_practices_test.go

Lines changed: 68 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,19 @@
11
package internal
22

33
import (
4+
"testing"
5+
46
"github.com/operator-framework/api/pkg/manifests"
57
"github.com/stretchr/testify/require"
6-
"testing"
78
)
89

910
func Test_ValidateGoodPractices(t *testing.T) {
1011
bundleWithDeploymentSpecEmpty, _ := manifests.GetBundleFromDir("./testdata/valid_bundle")
1112
bundleWithDeploymentSpecEmpty.CSV.Spec.InstallStrategy.StrategySpec.DeploymentSpecs = nil
1213

14+
bundleWithMissingCrdDescription, _ := manifests.GetBundleFromDir("./testdata/valid_bundle")
15+
bundleWithMissingCrdDescription.CSV.Spec.CustomResourceDefinitions.Owned[0].Description = ""
16+
1317
type args struct {
1418
bundleDir string
1519
bundle *manifests.Bundle
@@ -62,6 +66,22 @@ func Test_ValidateGoodPractices(t *testing.T) {
6266
},
6367
errStrings: []string{"Error: Value etcdoperator.v0.9.4: unable to find a deployment to install in the CSV"},
6468
},
69+
{
70+
name: "should raise an warn when the channel does not follows the convention",
71+
wantWarning: true,
72+
args: args{
73+
bundleDir: "./testdata/bundle_with_metadata",
74+
},
75+
warnStrings: []string{"Warning: Value memcached-operator.v0.0.1: channel(s) [\"alpha\"] are not following the recommended naming convention: https://olm.operatorframework.io/docs/best-practices/channel-naming"},
76+
},
77+
{
78+
name: "should raise a warn when a CRD does not have a description",
79+
wantWarning: true,
80+
args: args{
81+
bundle: bundleWithMissingCrdDescription,
82+
},
83+
warnStrings: []string{"Warning: Value etcdoperator.v0.9.4: owned CRD \"etcdclusters.etcd.database.coreos.com\" has an empty description"},
84+
},
6585
}
6686

6787
for _, tt := range tests {
@@ -92,3 +112,50 @@ func Test_ValidateGoodPractices(t *testing.T) {
92112
})
93113
}
94114
}
115+
116+
func TestValidateHubChannels(t *testing.T) {
117+
type args struct {
118+
channels []string
119+
}
120+
tests := []struct {
121+
name string
122+
args args
123+
wantWarn bool
124+
warnStrings []string
125+
}{
126+
{
127+
name: "should not return warning when the channel names following the convention",
128+
args: args{
129+
channels: []string{"fast", "candidate"},
130+
},
131+
wantWarn: false,
132+
},
133+
{
134+
name: "should return warning when the channel names are NOT following the convention",
135+
args: args{
136+
channels: []string{"mychannel-4.5"},
137+
},
138+
wantWarn: true,
139+
warnStrings: []string{"channel(s) [\"mychannel-4.5\"] are not following the recommended naming convention: https://olm.operatorframework.io/docs/best-practices/channel-naming"},
140+
},
141+
{
142+
name: "should return warning when has 1 channel NOT following the convention along the others which follows up",
143+
args: args{
144+
channels: []string{"alpha", "fast-v2.1", "candidate-v2.2"},
145+
},
146+
wantWarn: true,
147+
warnStrings: []string{"channel(s) [\"alpha\"] are not following the recommended naming convention: https://olm.operatorframework.io/docs/best-practices/channel-naming"},
148+
},
149+
}
150+
for _, tt := range tests {
151+
t.Run(tt.name, func(t *testing.T) {
152+
err := validateHubChannels(tt.args.channels)
153+
if (err != nil) != tt.wantWarn {
154+
t.Errorf("validateHubChannels() error = %v, wantWarn %v", err, tt.wantWarn)
155+
}
156+
if len(tt.warnStrings) > 0 {
157+
require.Contains(t, tt.warnStrings, err.Error())
158+
}
159+
})
160+
}
161+
}

‎staging/api/pkg/validation/internal/operatorhub.go

Lines changed: 0 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -215,40 +215,9 @@ func validateBundleOperatorHub(bundle *manifests.Bundle, k8sVersion string) erro
215215
result.Add(errors.WarnFailedValidation(warn.Error(), bundle.CSV.GetName()))
216216
}
217217

218-
if warn := validateHubChannels(bundle.Channels); warn != nil {
219-
result.Add(errors.WarnFailedValidation(warn.Error(), bundle.CSV.GetName()))
220-
}
221-
222218
return result
223219
}
224220

225-
// validateHubChannels will check the channels. The motivation for the following check is to ensure that operators
226-
// authors knows if their operator bundles are or not respecting the Naming Convention Rules.
227-
// However, the operator authors still able to choose the names as please them.
228-
func validateHubChannels(channels []string) error {
229-
const candidate = "candidate"
230-
const stable = "stable"
231-
const fast = "fast"
232-
233-
var channelsNotFollowingConventional []string
234-
for _, channel := range channels {
235-
if !strings.HasPrefix(channel, candidate) &&
236-
!strings.HasPrefix(channel, stable) &&
237-
!strings.HasPrefix(channel, fast) {
238-
channelsNotFollowingConventional = append(channelsNotFollowingConventional, channel)
239-
}
240-
241-
}
242-
243-
if len(channelsNotFollowingConventional) > 0 {
244-
return fmt.Errorf("channel(s) %+q are not following the recommended naming convention: "+
245-
"https://olm.operatorframework.io/docs/best-practices/channel-naming",
246-
channelsNotFollowingConventional)
247-
}
248-
249-
return nil
250-
}
251-
252221
// validateHubCSVSpec will check the CSV against the criteria to publish an
253222
// operator bundle in the OperatorHub.io
254223
func validateHubCSVSpec(csv v1alpha1.ClusterServiceVersion) CSVChecks {

‎staging/api/pkg/validation/internal/operatorhub_test.go

Lines changed: 0 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -342,36 +342,3 @@ func TestCheckSpecMinKubeVersion(t *testing.T) {
342342
})
343343
}
344344
}
345-
346-
func TestValidateHubChannels(t *testing.T) {
347-
type args struct {
348-
channels []string
349-
}
350-
tests := []struct {
351-
name string
352-
args args
353-
wantWarn bool
354-
}{
355-
{
356-
name: "should not return warning when the channel names following the convention",
357-
args: args{
358-
channels: []string{"fast", "candidate"},
359-
},
360-
wantWarn: false,
361-
},
362-
{
363-
name: "should return warning when the channel names are NOT following the convention",
364-
args: args{
365-
channels: []string{"mychannel-4.5"},
366-
},
367-
wantWarn: true,
368-
},
369-
}
370-
for _, tt := range tests {
371-
t.Run(tt.name, func(t *testing.T) {
372-
if err := validateHubChannels(tt.args.channels); (err != nil) != tt.wantWarn {
373-
t.Errorf("validateHubChannels() error = %v, wantWarn %v", err, tt.wantWarn)
374-
}
375-
})
376-
}
377-
}

‎staging/api/pkg/validation/internal/testdata/bundle_with_metadata/manifests/cache.example.com_memcacheds.yaml

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
apiVersion: apiextensions.k8s.io/v1
2+
kind: CustomResourceDefinition
3+
metadata:
4+
annotations:
5+
controller-gen.kubebuilder.io/version: v0.8.0
6+
creationTimestamp: null
7+
name: memcacheds.cache.example.com
8+
spec:
9+
group: cache.example.com
10+
names:
11+
kind: Memcached
12+
listKind: MemcachedList
13+
plural: memcacheds
14+
singular: bundle_with_metadata
15+
scope: Namespaced
16+
versions:
17+
- name: v1alpha1
18+
schema:
19+
openAPIV3Schema:
20+
description: Memcached is the Schema for the memcacheds API
21+
properties:
22+
apiVersion:
23+
description: 'APIVersion defines the versioned schema of this representation
24+
of an object. Servers should convert recognized schemas to the latest
25+
internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources'
26+
type: string
27+
kind:
28+
description: 'Kind is a string value representing the REST resource this
29+
object represents. Servers may infer this from the endpoint the client
30+
submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds'
31+
type: string
32+
metadata:
33+
type: object
34+
spec:
35+
description: MemcachedSpec defines the desired state of Memcached
36+
properties:
37+
foo:
38+
description: Foo is an example field of Memcached. Edit memcached_types.go
39+
to remove/update
40+
type: string
41+
size:
42+
description: Size defines the number of Memcached instances
43+
format: int32
44+
type: integer
45+
type: object
46+
status:
47+
description: MemcachedStatus defines the observed state of Memcached
48+
properties:
49+
nodes:
50+
description: Nodes store the name of the pods which are running Memcached
51+
instances
52+
items:
53+
type: string
54+
type: array
55+
type: object
56+
type: object
57+
served: true
58+
storage: true
59+
subresources:
60+
status: {}
61+
status:
62+
acceptedNames:
63+
kind: ""
64+
plural: ""
65+
conditions: []
66+
storedVersions: []

0 commit comments

Comments
 (0)