Ensure all disconnected annotations are accounted for when reviewing …

…guidelines

Signed-off-by: Jose R. Gonzalez <[email protected]>

internal/csv/csv.go

@@ -24,15 +24,17 @@ const (
 	CSIAnnotation                    = "features.operators.openshift.io/csi"
 )
 
-// SupportsDisconnected accepts a stringified list of supported features
+// SupportsDisconnectedViaInfrastructureFeatures accepts a stringified list of supported features
 // and returns true if "disconnected" is listed as a supported feature.
 //
 // E.g. '["disconnected"]'.
 //
 // This is case insensitive, as each infrastructure
 // is normalized before checking. A failure to unmarshal this structure
 // returns false.
-func SupportsDisconnected(infrastructureFeatures string) bool {
+//
+// This is a legacy annotation and will be superceded by the DisconnectedAnnotation in the future.
+func SupportsDisconnectedViaInfrastructureFeatures(infrastructureFeatures string) bool {
 	var features []string
 
 	err := json.Unmarshal([]byte(infrastructureFeatures), &features)
@@ -49,13 +51,28 @@ func SupportsDisconnected(infrastructureFeatures string) bool {
 	return false
 }
 
+// SupportsDisconnected accepts the value of the DisconnectedAnnotation and returns whether it is
+// set to the exact value of "true", which is the expected value indicating support. For this annotation,
+// an explicit value of "false" is treated the same as any other non-true value.
+func SupportsDisconnected(annotationValue string) bool {
+	return annotationValue == "true"
+}
+
 // HasInfrastructureFeaturesAnnotation returns true if the infrastructure-features annotation
-// exists in the .metadata.annotations block of csv.
+// exists in the .metadata.annotations block of csv. This is the legacy annotation
+// superceded by the DisconnectedAnnotation
 func HasInfrastructureFeaturesAnnotation(csv *operatorsv1alpha1.ClusterServiceVersion) bool {
 	_, ok := csv.GetAnnotations()[InfrastructureFeaturesAnnotation]
 	return ok
 }
 
+// HasDisconnectedAnnotation returns true if the disconnected annotation exists in the
+// .metadata.annotations block of csv.
+func HasDisconnectedAnnotation(csv *operatorsv1alpha1.ClusterServiceVersion) bool {
+	_, ok := csv.GetAnnotations()[DisconnectedAnnotation]
+	return ok
+}
+
 // HasRelatedImages returns true if the length of the .spec.relatedImages section of
 // csv is greater than 0.
 func HasRelatedImages(csv *operatorsv1alpha1.ClusterServiceVersion) bool {

internal/csv/csv_test.go

@@ -12,10 +12,37 @@ import (
 )
 
 var _ = Describe("CSV Functions", func() {
+	DescribeTable(
+		"Validating the independent disconnected annotation values",
+		func(value string, expected bool) {
+			actual := SupportsDisconnected(value)
+			Expect(actual).To(Equal(expected))
+		},
+		Entry("set to \"true\"", "true", true),
+		Entry("set to \"false\"", "false", false),
+		Entry("set to \"TRUE\"", "TRUE", false),
+		Entry("set to a random string", "abc", false),
+	)
+
+	When("Checking CSVs for the Disconnected Annotation", func() {
+		var csv operatorsv1alpha1.ClusterServiceVersion
+		It("Should return false if the annotation is missing", func() {
+			annotations := map[string]string{}
+			csv.Annotations = annotations
+			Expect(HasDisconnectedAnnotation(&csv)).To(BeFalse())
+		})
+
+		It("Should return true if the annotation is present, regardless of its value", func() {
+			annotations := map[string]string{DisconnectedAnnotation: "foo"}
+			csv.Annotations = annotations
+			Expect(HasDisconnectedAnnotation(&csv)).To(BeTrue())
+		})
+	})
+
 	DescribeTable(
 		"Validating infrastructure features list disconnected",
 		func(featurelist string, expected bool) {
-			actual := SupportsDisconnected(featurelist)
+			actual := SupportsDisconnectedViaInfrastructureFeatures(featurelist)
 			Expect(actual).To(Equal(expected))
 		},
 		Entry("disconnected in all caps", `["DISCONNECTED"]`, true),

internal/policy/operator/restricted_network_aware.go

@@ -37,14 +37,18 @@ func (p FollowsRestrictedNetworkEnablementGuidelines) validate(ctx context.Conte
 		return false, err
 	}
 
+	restrictedNetworkSupport := false
 	// If the CSV does not claim to support disconnected environments, there's no reason to check that it followed guidelines.
-	if !libcsv.HasInfrastructureFeaturesAnnotation(csv) {
-		logger.Info("this operator does not have the infrastructure-features annotation. You can safely ignore this if you your operator is not intended to be restricted-network aware.")
-		return false, nil
+	if libcsv.HasDisconnectedAnnotation(csv) && libcsv.SupportsDisconnected(csv.Annotations[libcsv.DisconnectedAnnotation]) {
+		restrictedNetworkSupport = true
+	}
+
+	if libcsv.HasInfrastructureFeaturesAnnotation(csv) && libcsv.SupportsDisconnectedViaInfrastructureFeatures(csv.Annotations[libcsv.InfrastructureFeaturesAnnotation]) {
+		restrictedNetworkSupport = true
 	}
 
-	if !libcsv.SupportsDisconnected(csv.Annotations[libcsv.InfrastructureFeaturesAnnotation]) {
-		logger.Info("the infrastructure-features enabled for this operator did not include the \"disconnected\" identifier. You can safely ignore this if you your operator is not intended to be restricted-network aware.")
+	if !restrictedNetworkSupport {
+		logger.Info("this operator does not indicate it supports installation into restricted networks. This is safe to ignore if you are not intending to deploy in these environments.")
 		return false, nil
 	}