The default channel info/annotation is optional & not required

The default channel annotation is no longer required as default
channel information is now optional. Users may provide default
channel if they intend to update default channel information in
the index/DB.

The default channel may still be infered if package.yaml is present.
If the default channel cannot be infered, then the default channel
will be omitted.

Bundle validation will no longer error out if default channel
annotation is missing.

Signed-off-by: Vu Dinh <[email protected]>

Author: dinhxuanvu

docs/design/operator-bundle.md

@@ -45,7 +45,7 @@ We use the following labels to annotate the operator bundle image.
     * The value `metadata.v1` implies that this bundle has operator metadata.
 * The label `operators.operatorframework.io.bundle.package.v1` reflects the package name of the bundle.
 * The label `operators.operatorframework.io.bundle.channels.v1` reflects the list of channels the bundle is subscribing to when added into an operator registry
-* The label `operators.operatorframework.io.bundle.channel.default.v1` reflects the default channel an operator should be subscribed to when installed from a registry
+* The label `operators.operatorframework.io.bundle.channel.default.v1` reflects the default channel an operator should be subscribed to when installed from a registry. This label is optional if the default channel has been set by previous bundles and the default channel is unchanged for this bundle.
 
 The labels will also be put inside a YAML file, as shown below.
 
@@ -240,7 +240,7 @@ $ ./opm alpha bundle build --directory /test/0.1.0/ --tag quay.io/coreos/test-op
 --image-builder podman --package test-operator --channels stable,beta --default stable
 ```
 
-The `--package` or `-p` is the name of package for the operator such as `etcd` which maps `channels` to a particular application definition. `channels` allow package authors to write different upgrade paths for different users (e.g. `beta` vs. `stable`). The `channels` list is provided via `--channels` or `-c` flag. Multiple `channels` are separated by a comma (`,`). The default channel is provided optionally via `--default` or `-e` flag. If the default channel is not provided, the first channel in channel list is selected as default.
+The `--package` or `-p` is the name of package for the operator such as `etcd` which maps `channels` to a particular application definition. `channels` allow package authors to write different upgrade paths for different users (e.g. `beta` vs. `stable`). The `channels` list is provided via `--channels` or `-c` flag. Multiple `channels` are separated by a comma (`,`). The default channel is provided optionally via `--default` or `-e` flag.
 
 *Notes:*
 * If there is `Dockerfile` existing in the directory, it will be overwritten.

pkg/lib/bundle/generate.go

@@ -112,9 +112,6 @@ func GenerateFunc(directory, outputDir, packageName, channels, channelDefault st
 
 		if channelDefault == "" {
 			channelDefault = i.GetDefaultChannel()
-			if !containsString(strings.Split(channels, ","), channelDefault) {
-				channelDefault = ""
-			}
 			log.Infof("Inferred default channel: %s", channelDefault)
 		}
 	}
@@ -299,16 +296,18 @@ func ValidateAnnotations(existing, expected []byte) error {
 func GenerateAnnotations(mediaType, manifests, metadata, packageName, channels, channelDefault string) ([]byte, error) {
 	annotations := &AnnotationMetadata{
 		Annotations: map[string]string{
-			MediatypeLabel:      mediaType,
-			ManifestsLabel:      manifests,
-			MetadataLabel:       metadata,
-			PackageLabel:        packageName,
-			ChannelsLabel:       channels,
-			ChannelDefaultLabel: channelDefault,
+			MediatypeLabel: mediaType,
+			ManifestsLabel: manifests,
+			MetadataLabel:  metadata,
+			PackageLabel:   packageName,
+			ChannelsLabel:  channels,
 		},
 	}
 
-	annotations.Annotations[ChannelDefaultLabel] = channelDefault
+	// Only add defaultChannel annotation if present
+	if channelDefault != "" {
+		annotations.Annotations[ChannelDefaultLabel] = channelDefault
+	}
 
 	afile, err := yaml.Marshal(annotations)
 	if err != nil {
@@ -343,7 +342,11 @@ func GenerateDockerfile(mediaType, manifests, metadata, copyManifestDir, copyMet
 	fileContent += fmt.Sprintf("LABEL %s=%s\n", MetadataLabel, metadata)
 	fileContent += fmt.Sprintf("LABEL %s=%s\n", PackageLabel, packageName)
 	fileContent += fmt.Sprintf("LABEL %s=%s\n", ChannelsLabel, channels)
-	fileContent += fmt.Sprintf("LABEL %s=%s\n\n", ChannelDefaultLabel, channelDefault)
+
+	// Only add defaultChannel annotation if present
+	if channelDefault != "" {
+		fileContent += fmt.Sprintf("LABEL %s=%s\n\n", ChannelDefaultLabel, channelDefault)
+	}
 
 	// CONTENT
 	fileContent += fmt.Sprintf("COPY %s %s\n", relativeManifestDirectory, "/manifests/")
@@ -352,7 +355,7 @@ func GenerateDockerfile(mediaType, manifests, metadata, copyManifestDir, copyMet
 	return []byte(fileContent), nil
 }
 
-// Write `fileName` file with `content` into a `directory`
+// WriteFile writes `fileName` file with `content` into a `directory`
 // Note: Will overwrite the existing `fileName` file if it exists
 func WriteFile(fileName, directory string, content []byte) error {
 	if _, err := os.Stat(directory); os.IsNotExist(err) {

pkg/lib/bundle/generate_test.go

@@ -170,7 +170,6 @@ func TestGenerateDockerfileFunc(t *testing.T) {
 		"LABEL operators.operatorframework.io.bundle.metadata.v1=%s\n"+
 		"LABEL operators.operatorframework.io.bundle.package.v1=test4\n"+
 		"LABEL operators.operatorframework.io.bundle.channels.v1=test5\n"+
-		"LABEL operators.operatorframework.io.bundle.channel.default.v1=\n\n"+
 		"COPY test2 /manifests/\n"+
 		"COPY metadata /metadata/\n", MetadataDir)
 
@@ -255,7 +254,7 @@ func TestGenerateFunc(t *testing.T) {
 	os.Remove(filepath.Join("./", DockerFile))
 
 	output := fmt.Sprintf("annotations:\n" +
-		"  operators.operatorframework.io.bundle.channel.default.v1: \"\"\n" +
+		"  operators.operatorframework.io.bundle.channel.default.v1: alpha\n" +
 		"  operators.operatorframework.io.bundle.channels.v1: beta\n" +
 		"  operators.operatorframework.io.bundle.manifests.v1: manifests/\n" +
 		"  operators.operatorframework.io.bundle.mediatype.v1: registry+v1\n" +

pkg/lib/bundle/validate.go

@@ -180,7 +180,7 @@ func validateAnnotations(mediaType string, fileAnnotations *AnnotationMetadata)
 
 	for label, item := range annotations {
 		val, ok := fileAnnotations.Annotations[label]
-		if !ok {
+		if !ok && label != ChannelDefaultLabel {
 			aErr := fmt.Errorf("Missing annotation %q", label)
 			validationErrors = append(validationErrors, aErr)
 		}
@@ -205,11 +205,12 @@ func validateAnnotations(mediaType string, fileAnnotations *AnnotationMetadata)
 			if val == "" {
 				aErr := fmt.Errorf("Expecting annotation %q to have non-empty value", label)
 				validationErrors = append(validationErrors, aErr)
-			} else {
-				annotations[label] = val
 			}
 		case ChannelDefaultLabel:
-			annotations[label] = val
+			if val == "" {
+				aErr := fmt.Errorf("Expecting annotation %q to have non-empty value", label)
+				validationErrors = append(validationErrors, aErr)
+			}
 		}
 	}
 

pkg/registry/types.go

@@ -333,12 +333,5 @@ func (a *AnnotationsFile) GetChannels() []string {
 
 // GetDefaultChannelName returns the name of the default channel
 func (a *AnnotationsFile) GetDefaultChannelName() string {
-	if a.Annotations.DefaultChannelName != "" {
-		return a.Annotations.DefaultChannelName
-	}
-	channels := a.GetChannels()
-	if len(channels) == 1 {
-		return channels[0]
-	}
-	return ""
+	return a.Annotations.DefaultChannelName
 }