[sdk-stamps] add sdk stamps to bundle images

[varshaprasad96]

Description of the change:
Add SDK related metric labels and annotations to bundle images

Motivation for the change:
To enable OLM track SDK Operators with help of these stamps on bundle resources.

Things to do:

• Add stamps to bundle.Dockerfile and annotaions.yaml
• Add stamps csv (during generate csv command)
  • Write test cases to verify the stamps on CSV
• Add stamps to crd (during generate crds command)

Note:
Adding of labels to CRDs through controller-gen for new project layouts will be done in a new PR. Adding labels through built-in crd-gen for legacy layouts is removed from this PR as of now, and will be included in a follow up later.

Co-authored by: @estroz

[varshaprasad96]

@estroz @joelanford @jmrodri Would be helpful to have initial reviews

[varshaprasad96]

Added this function here so that it remains consistent by adding scorecard labels and metrics labels in create.go

[estroz]

Initial review

[estroz]

Suggested change

	"gopkg.in/yaml.v2"
	"sigs.k8s.io/yaml"

[varshaprasad96]

Switching over to gopkg.in/yaml.v2. k8s-yaml interprets and appends the name of the field defined in yaml struct while marshaling and unmarshaling, whereas gopkg-yaml uses the yaml annotation defined in the struct. Example - https://play.golang.org/p/uQ_J4oZBbCR

[camilamacedo86]

We are trying to remove the "gopkg.in/yaml.v2". See : #2790
So, we should not add new code with it.

[varshaprasad96]

The operator-registry is still using gopkg.in/yaml.v2 for generation of annotation.yaml. Here: https://github.com/operator-framework/operator-registry/blob/master/pkg/lib/bundle/generate.go#L12. And this function is trying to essentially replicate their process.

[varshaprasad96]

The unit tests do pass with "sigs.k8s.io/yaml". However, it would fail in the scenario where we create bundle image using files already present on disk.
Explanation is as below:

To reproduce the scenario:

1. First create bundle files on disk with operator-sdk bundle create command and --generate-only flag.
2. Now create bundle image and push it to the registry (ie use the eg. command: operator-sdk bundle create quay.io/example/memcached-operator:latest --directory path/to/manifests --package memcached-operator)

The error which we get would be:
Unmatched number of fields. Expected 6 vs existing 0. It fails here.

Cause of error:
When we use sigs.k8s.io/yaml, the key in annotations.yaml file would be the name of the field in AnnotationMetadata struct. The annotations.yaml which would be rewritten after adding the sdk stamps, would be like this:

Annotations:
  operators.operatorframework.io.bundle.channel.default.v1: stable
  operators.operatorframework.io.bundle.channels.v1: stable
  operators.operatorframework.io.bundle.manifests.v1: manifests/

Now during unmarshalling, ie here, it would not be able to interpret this yaml because it has the key Annotations not annotations. The gopkg at operator-registry expects the key in yaml file to be annotations.

If we use "gopkg.in/yaml.v2", the annotations.yaml which we re-write, would take the marker value (name of field) defined in yaml struct, yaml:"annotations" as it does in operator-registry and it would unmarshall properly.

I have created a simple example to show the difference between both the packages in this playground: https://play.golang.org/p/uQ_J4oZBbCR

This scenario is being checked in one of the CI tests.

[camilamacedo86]

See that your example will work if we replace the yaml for JSON

Something int yaml:"somethingGreat,omitempty"
With
Something int json:"somethingGreat,omitempty"

Then, see that also it will work with yaml if we replace the Unmarshal/Marchal for JSONToYAML/YAMLToJSON. See: https://play.golang.org/p/D2guhgb7cvc

In SDK, we read/update/delete the files and use the lib to verify them. In this way, no matter the dep/lib that we use to manipulate it, all should work fine if at the end we have the same result. Then, I understand that if we change the impl here to use the above it should work.

However, if you still just facing problems with. could you please add a comment with a //todo: replace it for "sigs.k8s.io/yaml". Operator-registry defines the annotations with yaml format?

[varshaprasad96]

Adding //todo: because of the dependency on operator-registry. OR defines annotations.

[joelanford]

This could likely be fixed by adding json:"annotations" to the struct tag in the operator-registry repo.

So it would look like:

type AnnotationMetadata struct {
	Annotations map[string]string `yaml:"annotations" json:"annotations"``
}

Obviously this would require an upstream PR to be merged and for us to update our dependency to a new commit/version, so we shouldn't block this PR for that.

[varshaprasad96]

Will continue with above and have an upstream PR in parallel. Ill have a follow-up PR to remove the TODO.

[estroz]

A few minor changes required

[estroz]

To make this check robust, you need to check and write to each file individually, since one may exist without the other.

[varshaprasad96]

Yes, but I was thinking of rewriting both of them even if one exists, because until tweaked manually GenerateFunc of OR is going to write both and also generate bundle provides a --metadata option which writes both. In the sense, if there is a scenario where only one of them exists, doesn't it mean that either it has been manually deleted or there is some error (though I assume its taken care of in respective commands create/generate) and wouldn't rewriting both of them together in a clean state be a better option in such a scenario?

[estroz]

/lgtm