Expand validation to annotations

- Add validation function for well known case sensitive annotation names
- Use new validation function in CSV and OperatorGroups
- Add minimal OperatorGroup validator calling new function
- Add / update unit tests and test data
- update usage readme

Signed-off-by: John Hunkins <[email protected]>
(upstream api commit: 1ce77c819f6b2672adf424884af076b4de282632)

Author: jchunkins

staging/api/README.md

@@ -17,4 +17,4 @@ You can install the `operator-verify` tool from source using:
 
 To verify your ClusterServiceVersion yaml,
 
-`$ operator-verify verify /path/to/filename.yaml`
+`$ operator-verify manifests /path/to/filename.yaml`

staging/api/pkg/operators/v1alpha1/clusterserviceversion_types.go

@@ -22,6 +22,7 @@ const (
 	ClusterServiceVersionKind           = "ClusterServiceVersion"
 	OperatorGroupNamespaceAnnotationKey = "olm.operatorNamespace"
 	InstallStrategyNameDeployment       = "deployment"
+	SkipRangeAnnotationKey              = "olm.skipRange"
 )
 
 // InstallModeType is a supported type of install mode for CSV installation

staging/api/pkg/validation/internal/annotations.go

@@ -0,0 +1,53 @@
+package internal
+
+import (
+	"fmt"
+	"strings"
+
+	v1 "github.com/operator-framework/api/pkg/operators/v1"
+	"github.com/operator-framework/api/pkg/operators/v1alpha1"
+	"github.com/operator-framework/api/pkg/validation/errors"
+)
+
+// CaseSensitiveAnnotationKeySet is a set of annotation keys that are case sensitive
+// and can be used for validation purposes. The key is always lowercase and the value
+// contains the expected case sensitive string. This may not be an exhaustive list.
+var CaseSensitiveAnnotationKeySet = map[string]string{
+
+	strings.ToLower(v1.OperatorGroupAnnotationKey):             v1.OperatorGroupAnnotationKey,
+	strings.ToLower(v1.OperatorGroupNamespaceAnnotationKey):    v1.OperatorGroupNamespaceAnnotationKey,
+	strings.ToLower(v1.OperatorGroupTargetsAnnotationKey):      v1.OperatorGroupTargetsAnnotationKey,
+	strings.ToLower(v1.OperatorGroupProvidedAPIsAnnotationKey): v1.OperatorGroupProvidedAPIsAnnotationKey,
+	strings.ToLower(v1alpha1.SkipRangeAnnotationKey):           v1alpha1.SkipRangeAnnotationKey,
+}
+
+/*
+ValidateAnnotationNames will check annotation keys to ensure they are using
+proper case. Uses CaseSensitiveAnnotationKeySet as a source for keys
+which are known to be case sensitive. This function can be used anywhere
+annotations need to be checked for case sensitivity.
+
+Arguments
+
+• annotations: annotations map usually obtained from ObjectMeta.GetAnnotations()
+
+• value: is the field or file that caused an error or warning
+
+Returns
+
+• errs: Any errors that may have been detected with the annotation keys provided
+*/
+func ValidateAnnotationNames(annotations map[string]string, value interface{}) (errs []errors.Error) {
+	// for every annotation provided
+	for annotationKey := range annotations {
+		// check the case sensitive key set for a matching lowercase annotation
+		if knownCaseSensitiveKey, ok := CaseSensitiveAnnotationKeySet[strings.ToLower(annotationKey)]; ok {
+			// we have a case-insensitive match... now check to see if the case is really correct
+			if annotationKey != knownCaseSensitiveKey {
+				// annotation key supplied is invalid due to bad case.
+				errs = append(errs, errors.ErrFailedValidation(fmt.Sprintf("provided annotation %s uses wrong case and should be %s instead", annotationKey, knownCaseSensitiveKey), value))
+			}
+		}
+	}
+	return errs
+}

staging/api/pkg/validation/internal/csv.go

@@ -46,6 +46,8 @@ func validateCSV(csv *v1alpha1.ClusterServiceVersion) errors.ManifestResult {
 	result.Add(validateInstallModes(csv)...)
 	// check missing optional/mandatory fields.
 	result.Add(checkFields(*csv)...)
+	// validate case sensitive annotation names
+	result.Add(ValidateAnnotationNames(csv.GetAnnotations(), csv.GetName())...)
 	return result
 }
 

staging/api/pkg/validation/internal/csv_test.go

@@ -49,6 +49,18 @@ func TestValidateCSV(t *testing.T) {
 			},
 			filepath.Join("testdata", "incorrect.csv.with.conversion.webhook.yaml"),
 		},
+		{
+			validatorFuncTest{
+				description: "invalid annotation name for csv",
+				wantErr:     true,
+				errors: []errors.Error{
+					errors.ErrFailedValidation("provided annotation olm.skiprange uses wrong case and should be olm.skipRange instead", "etcdoperator.v0.9.0"),
+					errors.ErrFailedValidation("provided annotation olm.operatorgroup uses wrong case and should be olm.operatorGroup instead", "etcdoperator.v0.9.0"),
+					errors.ErrFailedValidation("provided annotation olm.operatornamespace uses wrong case and should be olm.operatorNamespace instead", "etcdoperator.v0.9.0"),
+				},
+			},
+			filepath.Join("testdata", "badAnnotationNames.csv.yaml"),
+		},
 	}
 	for _, c := range cases {
 		b, err := ioutil.ReadFile(c.csvPath)

staging/api/pkg/validation/internal/operatorgroup.go

@@ -0,0 +1,35 @@
+package internal
+
+import (
+	operatorsv1 "github.com/operator-framework/api/pkg/operators/v1"
+	operatorsv1alpha2 "github.com/operator-framework/api/pkg/operators/v1alpha2"
+	"github.com/operator-framework/api/pkg/validation/errors"
+	interfaces "github.com/operator-framework/api/pkg/validation/interfaces"
+)
+
+// OperatorGroupValidator is a validator for OperatorGroup
+var OperatorGroupValidator interfaces.Validator = interfaces.ValidatorFunc(validateOperatorGroups)
+
+func validateOperatorGroups(objs ...interface{}) (results []errors.ManifestResult) {
+	for _, obj := range objs {
+		switch v := obj.(type) {
+		case *operatorsv1.OperatorGroup:
+			results = append(results, validateOperatorGroupV1(v))
+		case *operatorsv1alpha2.OperatorGroup:
+			results = append(results, validateOperatorGroupV1Alpha2(v))
+		}
+	}
+	return results
+}
+
+func validateOperatorGroupV1Alpha2(operatorGroup *operatorsv1alpha2.OperatorGroup) (result errors.ManifestResult) {
+	// validate case sensitive annotation names
+	result.Add(ValidateAnnotationNames(operatorGroup.GetAnnotations(), operatorGroup.GetName())...)
+	return result
+}
+
+func validateOperatorGroupV1(operatorGroup *operatorsv1.OperatorGroup) (result errors.ManifestResult) {
+	// validate case sensitive annotation names
+	result.Add(ValidateAnnotationNames(operatorGroup.GetAnnotations(), operatorGroup.GetName())...)
+	return result
+}

staging/api/pkg/validation/internal/operatorgroup_test.go

@@ -0,0 +1,47 @@
+package internal
+
+import (
+	"io/ioutil"
+	"path/filepath"
+	"testing"
+
+	"github.com/ghodss/yaml"
+	operatorsv1 "github.com/operator-framework/api/pkg/operators/v1"
+	"github.com/operator-framework/api/pkg/validation/errors"
+)
+
+func TestValidateOperatorGroup(t *testing.T) {
+	cases := []struct {
+		validatorFuncTest
+		operatorGroupPath string
+	}{
+		{
+			validatorFuncTest{
+				description: "successfully validated",
+			},
+			filepath.Join("testdata", "correct.og.yaml"),
+		},
+		{
+			validatorFuncTest{
+				description: "invalid annotation name for operator group",
+				wantErr:     true,
+				errors: []errors.Error{
+					errors.ErrFailedValidation("provided annotation olm.providedapis uses wrong case and should be olm.providedAPIs instead", "nginx-hbvsw"),
+				},
+			},
+			filepath.Join("testdata", "badAnnotationNames.og.yaml"),
+		},
+	}
+	for _, c := range cases {
+		b, err := ioutil.ReadFile(c.operatorGroupPath)
+		if err != nil {
+			t.Fatalf("Error reading OperatorGroup path %s: %v", c.operatorGroupPath, err)
+		}
+		og := operatorsv1.OperatorGroup{}
+		if err = yaml.Unmarshal(b, &og); err != nil {
+			t.Fatalf("Error unmarshalling OperatorGroup at path %s: %v", c.operatorGroupPath, err)
+		}
+		result := validateOperatorGroupV1(&og)
+		c.check(t, result)
+	}
+}