Skip to content

Commit

Permalink
[sdk-stamps] Remove addition of sdk labels from crd
Browse files Browse the repository at this point in the history
  • Loading branch information
varshaprasad96 committed Jun 24, 2020
1 parent c08b01d commit 3ee2066
Show file tree
Hide file tree
Showing 19 changed files with 136 additions and 280 deletions.
2 changes: 1 addition & 1 deletion changelog/fragments/add-sdk-stamps.yaml
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
entries:
- description: Add sdk annotations to bundle resources (CRDs, CSVs, `annotations.yaml` and `bundle.dockerfile`).
- description: Add sdk annotations to bundle resources (CSVs, `annotations.yaml` and `bundle.dockerfile`).
kind: "addition"
2 changes: 1 addition & 1 deletion cmd/operator-sdk/add/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ func doAnsibleAPIScaffold() error {
if err != nil {
return fmt.Errorf("new ansible api scaffold failed: %v", err)
}
if err = genutil.GenerateCRDNonGo("", *r, apiFlags.CrdVersion, projutil.OperatorTypeAnsible); err != nil {
if err = genutil.GenerateCRDNonGo("", *r, apiFlags.CrdVersion); err != nil {
return err
}

Expand Down
12 changes: 6 additions & 6 deletions cmd/operator-sdk/bundle/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ func (c bundleCreateCmd) runGenerate() error {
rootDir = filepath.Dir(c.directory)
}

if err = rewriteBundleImageContents(rootDir); err != nil {
if err = RewriteBundleImageContents(rootDir); err != nil {
return err
}
return nil
Expand Down Expand Up @@ -341,19 +341,19 @@ func copyScorecardConfig() error {
}

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

err := projutil.RewriteFileContents(filename, "LABEL", sdkMetricContent)
err := projutil.RewriteFileContents(filename, "LABEL", sdkMetricContent.String())
if err != nil {
return fmt.Errorf("error rewriting dockerfile with metric labels, %v", err)
}
return nil
}

func rewriteBundleImageContents(rootDir string) error {
func RewriteBundleImageContents(rootDir string) error {
metricLabels := projutil.MakeBundleMetricsLabels()

// write metric labels to bundle.Dockerfile
Expand All @@ -366,7 +366,7 @@ func rewriteBundleImageContents(rootDir string) error {
return fmt.Errorf("error writing metric labels to annotations.yaml: %v", err)
}

// CopyScorecardConfig to bundle.Dockerfile
// Add a COPY for the scorecard config to bundle.Dockerfile.
if err := copyScorecardConfig(); err != nil {
return fmt.Errorf("error copying scorecardConfig to bundle image, %v", err)
}
Expand Down
11 changes: 11 additions & 0 deletions cmd/operator-sdk/generate/bundle/bundle.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"github.com/operator-framework/operator-registry/pkg/lib/bundle"
"sigs.k8s.io/kubebuilder/pkg/model/config"

genbundle "github.com/operator-framework/operator-sdk/cmd/operator-sdk/bundle"
genutil "github.com/operator-framework/operator-sdk/cmd/operator-sdk/generate/internal"
gencsv "github.com/operator-framework/operator-sdk/internal/generate/clusterserviceversion"
"github.com/operator-framework/operator-sdk/internal/generate/collector"
Expand Down Expand Up @@ -259,5 +260,15 @@ func (c bundleCmd) generateMetadata(manifestsDir, outputDir string) error {
if err != nil {
return fmt.Errorf("error generating bundle metadata: %v", err)
}
if c.overwrite {
rootDir := outputDir
if rootDir == "" {
rootDir = filepath.Dir(manifestsDir)
}

if err = genbundle.RewriteBundleImageContents(rootDir); err != nil {
return err
}
}
return nil
}
3 changes: 1 addition & 2 deletions cmd/operator-sdk/new/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -268,8 +268,7 @@ func doAnsibleScaffold() error {
return fmt.Errorf("new ansible scaffold failed: %v", err)
}

if err = genutil.GenerateCRDNonGo(projectName, *resource,
apiFlags.CrdVersion, projutil.OperatorTypeAnsible); err != nil {
if err = genutil.GenerateCRDNonGo(projectName, *resource, apiFlags.CrdVersion); err != nil {
return err
}

Expand Down
6 changes: 0 additions & 6 deletions hack/generate/test-framework/gen-test-framework.sh
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,3 @@ go mod tidy
# Run gen commands
../../build/operator-sdk generate k8s
../../build/operator-sdk generate crds

# Remove sdk labels/annotations from crds
sed -i "/annotations/d" deploy/crds/cache.example.com_memcacheds_crd.yaml
sed -i "/operators.operatorframework.io/d" deploy/crds/cache.example.com_memcacheds_crd.yaml
sed -i "/annotations/d" deploy/crds/cache.example.com_memcachedrs_crd.yaml
sed -i "/operators.operatorframework.io/d" deploy/crds/cache.example.com_memcachedrs_crd.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -70,9 +70,6 @@ func (b ClusterServiceVersion) GetBase() (base *v1alpha1.ClusterServiceVersion,
base = b.makeNewBase()
}

// Add sdk labels to csv
setSDKAnnotations(base)

// Interactively fill in UI metadata.
if b.Interactive {
meta := &uiMetadata{}
Expand All @@ -92,22 +89,6 @@ func (b ClusterServiceVersion) GetBase() (base *v1alpha1.ClusterServiceVersion,
return base, nil
}

// setSDKAnnotations adds SDK metric labels to the base if they do not exist. It assumes that
// if sdk Builder annotation is not present, then all sdk labels are also not populated in csv.
func setSDKAnnotations(csv *v1alpha1.ClusterServiceVersion) {
annotations := csv.GetAnnotations()
if annotations == nil {
annotations = make(map[string]string)
}

if _, exist := annotations[projutil.OperatorBuilder]; !exist {
for key, value := range projutil.MakeOperatorMetricLabels() {
annotations[key] = value
}
}
csv.SetAnnotations(annotations)
}

// setDefaults sets default values in b using b's existing values.
func (b *ClusterServiceVersion) setDefaults() {
if b.DisplayName == "" {
Expand Down
20 changes: 20 additions & 0 deletions internal/generate/clusterserviceversion/clusterserviceversion.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"strings"

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

// Add sdk labels to csv
setSDKAnnotations(csv)

w, err := g.getWriter()
if err != nil {
return err
}
return genutil.WriteObject(w, csv)
}

// setSDKAnnotations adds SDK metric labels to the base if they do not exist.
func setSDKAnnotations(csv *v1alpha1.ClusterServiceVersion) {
annotations := csv.GetAnnotations()
if annotations == nil {
annotations = make(map[string]string)
}

for key, value := range projutil.MakeOperatorMetricLabels() {
annotations[key] = value
}
csv.SetAnnotations(annotations)
}

// LegacyOption is a function that modifies a Generator for legacy project layouts.
type LegacyOption Option

Expand Down Expand Up @@ -204,6 +221,9 @@ func (g *Generator) GenerateLegacy(opts ...LegacyOption) (err error) {
return err
}

// Add sdk labels to csv
setSDKAnnotations(csv)

w, err := g.getWriter()
if err != nil {
return err
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,9 +141,8 @@ var _ = Describe("Generating a ClusterServiceVersion", func() {
}
Expect(g.Generate(cfg, opts...)).ToNot(HaveOccurred())
outputFile := filepath.Join(tmp, "bases", makeCSVFileName(operatorName))
outputFileContents := removeSDKLabelsFromCSVString(string(readFileHelper(outputFile)))
Expect(outputFile).To(BeAnExistingFile())
Expect(outputFileContents).To(MatchYAML(baseCSVUIMetaStr))
Expect(readFileHelper(outputFile)).To(MatchYAML(baseCSVUIMetaStr))
})
It("should have sdk labels in annotations", func() {
g = Generator{
Expand All @@ -160,7 +159,7 @@ var _ = Describe("Generating a ClusterServiceVersion", func() {
Expect(err).ToNot(HaveOccurred())
Expect(outputFile).To(BeAnExistingFile())

annotations := outputCSV.ObjectMeta.Annotations
annotations := outputCSV.GetAnnotations()
Expect(annotations).ToNot(BeNil())
Expect(annotations).Should(HaveKey(projutil.OperatorBuilder))
Expect(annotations).Should(HaveKey(projutil.OperatorLayout))
Expand All @@ -178,9 +177,8 @@ var _ = Describe("Generating a ClusterServiceVersion", func() {
}
Expect(g.Generate(cfg, opts...)).ToNot(HaveOccurred())
outputFile := filepath.Join(tmp, bundle.ManifestsDir, makeCSVFileName(operatorName))
outputFileContents := removeSDKLabelsFromCSVString(string(readFileHelper(outputFile)))
Expect(outputFile).To(BeAnExistingFile())
Expect(outputFileContents).To(MatchYAML(newCSVStr))
Expect(readFileHelper(outputFile)).To(MatchYAML(newCSVStr))
})
It("should write a ClusterServiceVersion manifest to a package file", func() {
g = Generator{
Expand All @@ -196,7 +194,7 @@ var _ = Describe("Generating a ClusterServiceVersion", func() {
Expect(g.Generate(cfg, opts...)).ToNot(HaveOccurred())
outputFile := filepath.Join(tmp, g.Version, makeCSVFileName(operatorName))
Expect(outputFile).To(BeAnExistingFile())
Expect(string(readFileHelper(outputFile))).To(MatchYAML(newCSVStr))
Expect(readFileHelper(outputFile)).To(MatchYAML(newCSVStr))
})

It("should write a ClusterServiceVersion manifest to a legacy bundle file", func() {
Expand All @@ -212,9 +210,8 @@ var _ = Describe("Generating a ClusterServiceVersion", func() {
}
Expect(g.GenerateLegacy(opts...)).ToNot(HaveOccurred())
outputFile := filepath.Join(tmp, bundle.ManifestsDir, makeCSVFileName(operatorName))
outputFileContents := removeSDKLabelsFromCSVString(string(readFileHelper(outputFile)))
Expect(outputFile).To(BeAnExistingFile())
Expect(outputFileContents).To(MatchYAML(newCSVStr))
Expect(readFileHelper(outputFile)).To(MatchYAML(newCSVStr))
})
It("should write a ClusterServiceVersion manifest as a legacy package file", func() {
g = Generator{
Expand Down Expand Up @@ -280,46 +277,28 @@ var _ = Describe("Generating a ClusterServiceVersion", func() {
})
})

Context("to create a new", func() {

Context("bundle base", func() {
It("should return the default base object", func() {
g = Generator{
OperatorName: operatorName,
OperatorType: operatorType,
config: cfg,
getBase: makeBaseGetter(baseCSV),
}
csv, err := g.generate()
Expect(err).ToNot(HaveOccurred())
Expect(csv).To(Equal(baseCSV))
})
It("should return a base object with customresourcedefinitions", func() {
g = Generator{
OperatorName: operatorName,
OperatorType: operatorType,
config: cfg,
getBase: makeBaseGetter(baseCSVUIMeta),
}
csv, err := g.generate()
Expect(err).ToNot(HaveOccurred())
Expect(csv).To(Equal(baseCSVUIMeta))
})
Context("to create a new base ClusterServiceVersion", func() {
It("should return the default base object", func() {
g = Generator{
OperatorName: operatorName,
OperatorType: operatorType,
config: cfg,
getBase: makeBaseGetter(baseCSV),
}
csv, err := g.generate()
Expect(err).ToNot(HaveOccurred())
Expect(csv).To(Equal(baseCSV))
})
Context("bundle", func() {
It("should return the expected object", func() {
g = Generator{
OperatorName: operatorName,
OperatorType: operatorType,
Version: version,
Collector: col,
config: cfg,
getBase: makeBaseGetter(baseCSVUIMeta),
}
csv, err := g.generate()
Expect(err).ToNot(HaveOccurred())
Expect(csv).To(Equal(newCSV))
})
It("should return a base object with customresourcedefinitions", func() {
g = Generator{
OperatorName: operatorName,
OperatorType: operatorType,
config: cfg,
getBase: makeBaseGetter(baseCSVUIMeta),
}
csv, err := g.generate()
Expect(err).ToNot(HaveOccurred())
Expect(csv).To(Equal(baseCSVUIMeta))
})
})

Expand Down Expand Up @@ -380,7 +359,6 @@ var _ = Describe("Generating a ClusterServiceVersion", func() {
Expect(csv).To(Equal(upgradeCSV(newCSV, g.OperatorName, g.Version)))
})
})

})

})
Expand Down Expand Up @@ -445,10 +423,10 @@ func initTestCSVsHelper() {
ExpectWithOffset(1, err).ToNot(HaveOccurred())
}

func readFileHelper(path string) []byte {
func readFileHelper(path string) string {
b, err := ioutil.ReadFile(path)
ExpectWithOffset(1, err).ToNot(HaveOccurred())
return b
return removeSDKLabelsFromCSVString(string(b))
}

func modifyCSVDepImageHelper(tag string) func(csv *v1alpha1.ClusterServiceVersion) {
Expand Down
29 changes: 3 additions & 26 deletions internal/generate/crd/crd.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ import (
"github.com/operator-framework/operator-sdk/internal/scaffold"
"github.com/operator-framework/operator-sdk/internal/util/fileutil"
"github.com/operator-framework/operator-sdk/internal/util/k8sutil"
"github.com/operator-framework/operator-sdk/internal/util/projutil"
)

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

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

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

func (g Generator) setSDKLabels(crd *apiextv1beta1.CustomResourceDefinition) {
annotations := crd.GetAnnotations()
if annotations == nil {
annotations = make(map[string]string)
}
g.addSDKLabelsToAnnotations(annotations)
crd.SetAnnotations(annotations)
}

func (g Generator) addSDKLabelsToAnnotations(mapAnnotations map[string]string) {
for label, value := range projutil.MakeOperatorMetricLabels() {
mapAnnotations[label] = value
}
// Modifying operator type for ansible and helm legacy operators, as during
// scaffolding the project directories are not written on disk
if g.OperatorType != projutil.OperatorTypeGo {
mapAnnotations[projutil.OperatorLayout] = g.OperatorType
}
}

// checkCRDVersions ensures version(s) generated for a CRD are in valid format.
// From the Kubernetes CRD docs:
//
Expand Down
Loading

0 comments on commit 3ee2066

Please sign in to comment.