Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit d0ad607

Browse files
committedJun 19, 2020
[sdk-stamps] Remove addition of sdk labels from crd
1 parent 92eef88 commit d0ad607

File tree

15 files changed

+91
-262
lines changed

15 files changed

+91
-262
lines changed
 
+1-1
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
11
entries:
2-
- description: Add sdk annotations to bundle resources (CRDs, CSVs, `annotations.yaml` and `bundle.dockerfile`).
2+
- description: Add sdk annotations to bundle resources (CSVs, `annotations.yaml` and `bundle.dockerfile`).
33
kind: "addition"

‎cmd/operator-sdk/add/api.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -197,7 +197,7 @@ func doAnsibleAPIScaffold() error {
197197
if err != nil {
198198
return fmt.Errorf("new ansible api scaffold failed: %v", err)
199199
}
200-
if err = genutil.GenerateCRDNonGo("", *r, apiFlags.CrdVersion, projutil.OperatorTypeAnsible); err != nil {
200+
if err = genutil.GenerateCRDNonGo("", *r, apiFlags.CrdVersion); err != nil {
201201
return err
202202
}
203203

‎cmd/operator-sdk/bundle/create.go

+4-4
Original file line numberDiff line numberDiff line change
@@ -341,12 +341,12 @@ func copyScorecardConfig() error {
341341
}
342342

343343
func addLabelsToDockerfile(filename string, metricAnnotation map[string]string) error {
344-
var sdkMetricContent string
344+
var sdkMetricContent strings.Builder
345345
for key, value := range metricAnnotation {
346-
sdkMetricContent += fmt.Sprintf("LABEL %s=%s\n", key, value)
346+
sdkMetricContent.WriteString(fmt.Sprintf("LABEL %s=%s\n", key, value))
347347
}
348348

349-
err := projutil.RewriteFileContents(filename, "LABEL", sdkMetricContent)
349+
err := projutil.RewriteFileContents(filename, "LABEL", sdkMetricContent.String())
350350
if err != nil {
351351
return fmt.Errorf("error rewriting dockerfile with metric labels, %v", err)
352352
}
@@ -366,7 +366,7 @@ func rewriteBundleImageContents(rootDir string) error {
366366
return fmt.Errorf("error writing metric labels to annotations.yaml: %v", err)
367367
}
368368

369-
// CopyScorecardConfig to bundle.Dockerfile
369+
// Add a COPY for the scorecard config to bundle.Dockerfile.
370370
if err := copyScorecardConfig(); err != nil {
371371
return fmt.Errorf("error copying scorecardConfig to bundle image, %v", err)
372372
}

‎cmd/operator-sdk/new/cmd.go

+1-2
Original file line numberDiff line numberDiff line change
@@ -268,8 +268,7 @@ func doAnsibleScaffold() error {
268268
return fmt.Errorf("new ansible scaffold failed: %v", err)
269269
}
270270

271-
if err = genutil.GenerateCRDNonGo(projectName, *resource,
272-
apiFlags.CrdVersion, projutil.OperatorTypeAnsible); err != nil {
271+
if err = genutil.GenerateCRDNonGo(projectName, *resource, apiFlags.CrdVersion); err != nil {
273272
return err
274273
}
275274

‎hack/generate/test-framework/gen-test-framework.sh

-6
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,3 @@ go mod tidy
2222
# Run gen commands
2323
../../build/operator-sdk generate k8s
2424
../../build/operator-sdk generate crds
25-
26-
# Remove sdk labels/annotations from crds
27-
sed -i "/annotations/d" deploy/crds/cache.example.com_memcacheds_crd.yaml
28-
sed -i "/operators.operatorframework.io/d" deploy/crds/cache.example.com_memcacheds_crd.yaml
29-
sed -i "/annotations/d" deploy/crds/cache.example.com_memcachedrs_crd.yaml
30-
sed -i "/operators.operatorframework.io/d" deploy/crds/cache.example.com_memcachedrs_crd.yaml

‎internal/generate/clusterserviceversion/bases/clusterserviceversion.go

-19
Original file line numberDiff line numberDiff line change
@@ -70,9 +70,6 @@ func (b ClusterServiceVersion) GetBase() (base *v1alpha1.ClusterServiceVersion,
7070
base = b.makeNewBase()
7171
}
7272

73-
// Add sdk labels to csv
74-
setSDKAnnotations(base)
75-
7673
// Interactively fill in UI metadata.
7774
if b.Interactive {
7875
meta := &uiMetadata{}
@@ -92,22 +89,6 @@ func (b ClusterServiceVersion) GetBase() (base *v1alpha1.ClusterServiceVersion,
9289
return base, nil
9390
}
9491

95-
// setSDKAnnotations adds SDK metric labels to the base if they do not exist. It assumes that
96-
// if sdk Builder annotation is not present, then all sdk labels are also not populated in csv.
97-
func setSDKAnnotations(csv *v1alpha1.ClusterServiceVersion) {
98-
annotations := csv.GetAnnotations()
99-
if annotations == nil {
100-
annotations = make(map[string]string)
101-
}
102-
103-
if _, exist := annotations[projutil.OperatorBuilder]; !exist {
104-
for key, value := range projutil.MakeOperatorMetricLabels() {
105-
annotations[key] = value
106-
}
107-
}
108-
csv.SetAnnotations(annotations)
109-
}
110-
11192
// setDefaults sets default values in b using b's existing values.
11293
func (b *ClusterServiceVersion) setDefaults() {
11394
if b.DisplayName == "" {

‎internal/generate/clusterserviceversion/clusterserviceversion.go

+20
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121
"strings"
2222

2323
"github.com/blang/semver"
24+
"github.com/operator-framework/api/pkg/operators/v1alpha1"
2425
operatorsv1alpha1 "github.com/operator-framework/api/pkg/operators/v1alpha1"
2526
"github.com/operator-framework/operator-registry/pkg/lib/bundle"
2627
"k8s.io/apimachinery/pkg/runtime/schema"
@@ -160,13 +161,29 @@ func (g *Generator) Generate(cfg *config.Config, opts ...Option) (err error) {
160161
return err
161162
}
162163

164+
// Add sdk labels to csv
165+
setSDKAnnotations(csv)
166+
163167
w, err := g.getWriter()
164168
if err != nil {
165169
return err
166170
}
167171
return genutil.WriteObject(w, csv)
168172
}
169173

174+
// setSDKAnnotations adds SDK metric labels to the base if they do not exist.
175+
func setSDKAnnotations(csv *v1alpha1.ClusterServiceVersion) {
176+
annotations := csv.GetAnnotations()
177+
if annotations == nil {
178+
annotations = make(map[string]string)
179+
}
180+
181+
for key, value := range projutil.MakeOperatorMetricLabels() {
182+
annotations[key] = value
183+
}
184+
csv.SetAnnotations(annotations)
185+
}
186+
170187
// LegacyOption is a function that modifies a Generator for legacy project layouts.
171188
type LegacyOption Option
172189

@@ -204,6 +221,9 @@ func (g *Generator) GenerateLegacy(opts ...LegacyOption) (err error) {
204221
return err
205222
}
206223

224+
// Add sdk labels to csv
225+
setSDKAnnotations(csv)
226+
207227
w, err := g.getWriter()
208228
if err != nil {
209229
return err

‎internal/generate/clusterserviceversion/clusterserviceversion_test.go

+28-46
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,7 @@ var _ = Describe("Generating a ClusterServiceVersion", func() {
141141
}
142142
Expect(g.Generate(cfg, opts...)).ToNot(HaveOccurred())
143143
outputFile := filepath.Join(tmp, "bases", makeCSVFileName(operatorName))
144-
outputFileContents := removeSDKLabelsFromCSVString(string(readFileHelper(outputFile)))
144+
outputFileContents := readFileHelper(outputFile)
145145
Expect(outputFile).To(BeAnExistingFile())
146146
Expect(outputFileContents).To(MatchYAML(baseCSVUIMetaStr))
147147
})
@@ -178,7 +178,7 @@ var _ = Describe("Generating a ClusterServiceVersion", func() {
178178
}
179179
Expect(g.Generate(cfg, opts...)).ToNot(HaveOccurred())
180180
outputFile := filepath.Join(tmp, bundle.ManifestsDir, makeCSVFileName(operatorName))
181-
outputFileContents := removeSDKLabelsFromCSVString(string(readFileHelper(outputFile)))
181+
outputFileContents := readFileHelper(outputFile)
182182
Expect(outputFile).To(BeAnExistingFile())
183183
Expect(outputFileContents).To(MatchYAML(newCSVStr))
184184
})
@@ -195,8 +195,9 @@ var _ = Describe("Generating a ClusterServiceVersion", func() {
195195
}
196196
Expect(g.Generate(cfg, opts...)).ToNot(HaveOccurred())
197197
outputFile := filepath.Join(tmp, g.Version, makeCSVFileName(operatorName))
198+
outputFileContents := readFileHelper(outputFile)
198199
Expect(outputFile).To(BeAnExistingFile())
199-
Expect(string(readFileHelper(outputFile))).To(MatchYAML(newCSVStr))
200+
Expect(outputFileContents).To(MatchYAML(newCSVStr))
200201
})
201202

202203
It("should write a ClusterServiceVersion manifest to a legacy bundle file", func() {
@@ -212,7 +213,7 @@ var _ = Describe("Generating a ClusterServiceVersion", func() {
212213
}
213214
Expect(g.GenerateLegacy(opts...)).ToNot(HaveOccurred())
214215
outputFile := filepath.Join(tmp, bundle.ManifestsDir, makeCSVFileName(operatorName))
215-
outputFileContents := removeSDKLabelsFromCSVString(string(readFileHelper(outputFile)))
216+
outputFileContents := readFileHelper(outputFile)
216217
Expect(outputFile).To(BeAnExistingFile())
217218
Expect(outputFileContents).To(MatchYAML(newCSVStr))
218219
})
@@ -280,46 +281,28 @@ var _ = Describe("Generating a ClusterServiceVersion", func() {
280281
})
281282
})
282283

283-
Context("to create a new", func() {
284-
285-
Context("bundle base", func() {
286-
It("should return the default base object", func() {
287-
g = Generator{
288-
OperatorName: operatorName,
289-
OperatorType: operatorType,
290-
config: cfg,
291-
getBase: makeBaseGetter(baseCSV),
292-
}
293-
csv, err := g.generate()
294-
Expect(err).ToNot(HaveOccurred())
295-
Expect(csv).To(Equal(baseCSV))
296-
})
297-
It("should return a base object with customresourcedefinitions", func() {
298-
g = Generator{
299-
OperatorName: operatorName,
300-
OperatorType: operatorType,
301-
config: cfg,
302-
getBase: makeBaseGetter(baseCSVUIMeta),
303-
}
304-
csv, err := g.generate()
305-
Expect(err).ToNot(HaveOccurred())
306-
Expect(csv).To(Equal(baseCSVUIMeta))
307-
})
284+
Context("to create a new base ClusterServiceVersion", func() {
285+
It("should return the default base object", func() {
286+
g = Generator{
287+
OperatorName: operatorName,
288+
OperatorType: operatorType,
289+
config: cfg,
290+
getBase: makeBaseGetter(baseCSV),
291+
}
292+
csv, err := g.generate()
293+
Expect(err).ToNot(HaveOccurred())
294+
Expect(csv).To(Equal(baseCSV))
308295
})
309-
Context("bundle", func() {
310-
It("should return the expected object", func() {
311-
g = Generator{
312-
OperatorName: operatorName,
313-
OperatorType: operatorType,
314-
Version: version,
315-
Collector: col,
316-
config: cfg,
317-
getBase: makeBaseGetter(baseCSVUIMeta),
318-
}
319-
csv, err := g.generate()
320-
Expect(err).ToNot(HaveOccurred())
321-
Expect(csv).To(Equal(newCSV))
322-
})
296+
It("should return a base object with customresourcedefinitions", func() {
297+
g = Generator{
298+
OperatorName: operatorName,
299+
OperatorType: operatorType,
300+
config: cfg,
301+
getBase: makeBaseGetter(baseCSVUIMeta),
302+
}
303+
csv, err := g.generate()
304+
Expect(err).ToNot(HaveOccurred())
305+
Expect(csv).To(Equal(baseCSVUIMeta))
323306
})
324307
})
325308

@@ -380,7 +363,6 @@ var _ = Describe("Generating a ClusterServiceVersion", func() {
380363
Expect(csv).To(Equal(upgradeCSV(newCSV, g.OperatorName, g.Version)))
381364
})
382365
})
383-
384366
})
385367

386368
})
@@ -445,10 +427,10 @@ func initTestCSVsHelper() {
445427
ExpectWithOffset(1, err).ToNot(HaveOccurred())
446428
}
447429

448-
func readFileHelper(path string) []byte {
430+
func readFileHelper(path string) string {
449431
b, err := ioutil.ReadFile(path)
450432
ExpectWithOffset(1, err).ToNot(HaveOccurred())
451-
return b
433+
return removeSDKLabelsFromCSVString(string(b))
452434
}
453435

454436
func modifyCSVDepImageHelper(tag string) func(csv *v1alpha1.ClusterServiceVersion) {

‎internal/generate/crd/crd.go

+3-26
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,6 @@ import (
4040
"github.com/operator-framework/operator-sdk/internal/scaffold"
4141
"github.com/operator-framework/operator-sdk/internal/util/fileutil"
4242
"github.com/operator-framework/operator-sdk/internal/util/k8sutil"
43-
"github.com/operator-framework/operator-sdk/internal/util/projutil"
4443
)
4544

4645
const DefaultCRDVersion = "v1"
@@ -68,8 +67,6 @@ type Generator struct {
6867
// ApisDir is for the location of the API types directory e.g "pkg/apis"
6968
// The CSV annotation comments will be parsed from the types under this path.
7069
ApisDir string
71-
// OperatorType refers to the type of operator (go/ansible/helm)
72-
OperatorType string
7370
}
7471

7572
func (g Generator) validate() error {
@@ -188,8 +185,9 @@ func (g Generator) generateGo() (map[string][]byte, error) {
188185
// just remove it.
189186
annotations := crd.GetAnnotations()
190187
delete(annotations, "controller-gen.kubebuilder.io/version")
191-
// Add sdk related labels to the cached annotations
192-
g.addSDKLabelsToAnnotations(annotations)
188+
if len(annotations) == 0 {
189+
annotations = nil
190+
}
193191
crd.SetAnnotations(annotations)
194192
b, err := k8sutil.GetObjectBytes(&crd, yaml.Marshal)
195193
if err != nil {
@@ -275,7 +273,6 @@ func (g Generator) generateNonGo() (map[string][]byte, error) {
275273

276274
sort.Sort(k8sutil.CRDVersions(crd.Spec.Versions))
277275
setCRDStorageVersion(crd)
278-
g.setSDKLabels(crd)
279276
if err := checkCRDVersions(crd); err != nil {
280277
return nil, fmt.Errorf("invalid version in CRD %s: %w", crd.GetName(), err)
281278
}
@@ -374,26 +371,6 @@ func setCRDStorageVersion(crd *apiextv1beta1.CustomResourceDefinition) {
374371
crd.Spec.Versions[0].Storage = true
375372
}
376373

377-
func (g Generator) setSDKLabels(crd *apiextv1beta1.CustomResourceDefinition) {
378-
annotations := crd.GetAnnotations()
379-
if annotations == nil {
380-
annotations = make(map[string]string)
381-
}
382-
g.addSDKLabelsToAnnotations(annotations)
383-
crd.SetAnnotations(annotations)
384-
}
385-
386-
func (g Generator) addSDKLabelsToAnnotations(mapAnnotations map[string]string) {
387-
for label, value := range projutil.MakeOperatorMetricLabels() {
388-
mapAnnotations[label] = value
389-
}
390-
// Modifying operator type for ansible and helm legacy operators, as during
391-
// scaffolding the project directories are not written on disk
392-
if g.OperatorType != projutil.OperatorTypeGo {
393-
mapAnnotations[projutil.OperatorLayout] = g.OperatorType
394-
}
395-
}
396-
397374
// checkCRDVersions ensures version(s) generated for a CRD are in valid format.
398375
// From the Kubernetes CRD docs:
399376
//

‎internal/generate/crd/crd_test.go

-17
Original file line numberDiff line numberDiff line change
@@ -132,13 +132,9 @@ func TestGenerate(t *testing.T) {
132132
}
133133

134134
func TestCRDGo(t *testing.T) {
135-
// Setting the OperatorType to be "unknown", so that it is propagated in the
136-
// sdk labels. The project layout cannot be inferred here as the entire
137-
// project scaffolding is not done.
138135
g := Generator{
139136
IsOperatorGo: true,
140137
ApisDir: filepath.Join(testGoDataDir, scaffold.ApisDir),
141-
OperatorType: "unknown",
142138
}
143139

144140
r, err := scaffold.NewResource(testAPIVersion, testKind)
@@ -216,7 +212,6 @@ func TestCRDNonGo(t *testing.T) {
216212
Resource: *r,
217213
CRDVersion: c.crdVersion,
218214
IsOperatorGo: false,
219-
OperatorType: "unknown",
220215
}
221216
fileMap, err := g.generateNonGo()
222217
if err != nil {
@@ -236,9 +231,6 @@ func TestCRDNonGo(t *testing.T) {
236231
const crdNonGoDefaultExpV1beta1 = `apiVersion: apiextensions.k8s.io/v1beta1
237232
kind: CustomResourceDefinition
238233
metadata:
239-
annotations:
240-
operators.operatorframework.io/builder: operator-sdk-unknown
241-
operators.operatorframework.io/project_layout: unknown
242234
name: memcacheds.cache.example.com
243235
spec:
244236
group: cache.example.com
@@ -265,9 +257,6 @@ spec:
265257
const crdNonGoDefaultExpV1 = `apiVersion: apiextensions.k8s.io/v1
266258
kind: CustomResourceDefinition
267259
metadata:
268-
annotations:
269-
operators.operatorframework.io/builder: operator-sdk-unknown
270-
operators.operatorframework.io/project_layout: unknown
271260
name: memcacheds.cache.example.com
272261
spec:
273262
group: cache.example.com
@@ -294,9 +283,6 @@ spec:
294283
const crdCustomExpV1beta1 = `apiVersion: apiextensions.k8s.io/v1beta1
295284
kind: CustomResourceDefinition
296285
metadata:
297-
annotations:
298-
operators.operatorframework.io/builder: operator-sdk-unknown
299-
operators.operatorframework.io/project_layout: unknown
300286
name: memcacheds.cache.example.com
301287
spec:
302288
group: cache.example.com
@@ -358,9 +344,6 @@ spec:
358344
const crdCustomExpV1 = `apiVersion: apiextensions.k8s.io/v1
359345
kind: CustomResourceDefinition
360346
metadata:
361-
annotations:
362-
operators.operatorframework.io/builder: operator-sdk-unknown
363-
operators.operatorframework.io/project_layout: unknown
364347
name: memcacheds.cache.example.com
365348
spec:
366349
group: cache.example.com

0 commit comments

Comments
 (0)
Please sign in to comment.