Repository: redhat-openshift-ecosystem/openshift-preflight
Pull request #291 (2021-10-08 17:22:22)

Remove tag from container when listing tags
When listing tags from a remote repository, Crane will throw an error if there
is already a tag on the URI (which makes sense, as we're trying to get a list
of tags!). This strips the tag before passing to Crane. Adds a unit test for
this.

Fixes #290

Signed-off-by: Brad P. Crochet <brad@redhat.com>


Patch:
diff --git a/README.md b/README.md
index 5b8eee8..84adf91 100644
--- a/README.md
+++ b/README.md
@@ -62,33 +62,12 @@ preflight check operator quay.io/example-namespace/example-operator:0.0.1
 
 For more detailed usage examples, see [Recipes](docs/RECIPES.md).
 
-For more information on how to configure the execution of `preflight`, see
-[CONFIG](docs/CONFIG.md)
-
 ### Authenticating to Registries
 
 The `preflight` command will automatically utilize a credential file at
 `$DOCKER_CONFIG/config.json` (default: `~/.docker/config.json`) to access images
 in private registries.
 
-#### Remote Checks
-
-In some cases (e.g. *DeployableByOLM*), `preflight` will also pass credentials
-to the cluster used for testing (i.e. the cluster that is accessible through the
-current-context of the provided `KUBECONFIG`).
-
-We anticipate that the credentials in `$DOCKER_CONFIG/config.json` may contain
-more access than what is needed for `preflight` execution. To avoid passing more
-credentials than needed into a cluster for those checks, `preflight` will also
-accept a full path to a dockerconfigjson that should be passed through to a
-remote cluster via the `PFLT_DOCKERCONFIG` environment variable.
-
-If this variable is unset, `preflight` will assume that the images in scope
-(e.g. PFLT_INDEXIMAGE value, and the test target itself) are already accessible
-from the cluster used for testing.
-
-#### Podman Users
-
 [Podman](https://podman.io/) stores credentials at
 `${XDG_RUNTIME_DIR}/containers/auth.json`, which can also be used by executing
 the following:
@@ -98,6 +77,9 @@ ln -sf ${XDG_RUNTIME_DIR}/containers/auth.json ${XDG_RUNTIME_DIR}/containers/con
 DOCKER_CONFIG=${XDG_RUNTIME_DIR}/containers
 ```
 
+For more information on how to configure the execution of `preflight`, see
+[CONFIG](docs/CONFIG.md)
+
 ## Installation
 
 Before installing `preflight`, ensure that the [required dependencies](#requirements) have been installed on the local machine.
diff --git a/certification/internal/policy/container/base_on_ubi.go b/certification/internal/policy/container/base_on_ubi.go
index d435a9a..31c15af 100644
--- a/certification/internal/policy/container/base_on_ubi.go
+++ b/certification/internal/policy/container/base_on_ubi.go
@@ -49,22 +49,20 @@ func (p *BasedOnUBICheck) getOsReleaseContents(path string) ([]string, error) {
 }
 
 func (p *BasedOnUBICheck) validate(labels map[string]string, osRelease []string) (bool, error) {
-	var hasRHELID, hasRHELName bool
+	var hasRHELID, hasRHELName, hasUbiComponentLabel bool
 	for _, value := range osRelease {
-		line := strings.Split(value, "=")
-		if line[0] == "ID" && line[1] == `"rhel"` {
-			log.Trace("Has RHEL ID")
+		if strings.HasPrefix(value, `ID="rhel"`) {
 			hasRHELID = true
-			continue
-		}
-		if line[0] == "NAME" && line[1] == `"Red Hat Enterprise Linux"` {
-			log.Trace("Has RHEL Name")
+		} else if strings.HasPrefix(value, `NAME="Red Hat Enterprise Linux"`) {
 			hasRHELName = true
-			continue
 		}
 	}
 
-	if hasRHELID && hasRHELName {
+	if component, exists := labels["com.redhat.component"]; exists {
+		hasUbiComponentLabel = strings.Contains(component, "ubi")
+	}
+
+	if hasRHELID && hasRHELName && hasUbiComponentLabel {
 		return true, nil
 	}
 
diff --git a/certification/internal/policy/container/has_unique_tag.go b/certification/internal/policy/container/has_unique_tag.go
index 114182a..15b958c 100644
--- a/certification/internal/policy/container/has_unique_tag.go
+++ b/certification/internal/policy/container/has_unique_tag.go
@@ -19,7 +19,12 @@ type HasUniqueTagCheck struct {
 }
 
 func (p *HasUniqueTagCheck) Validate(imgRef certification.ImageReference) (bool, error) {
-	tags, err := p.getDataToValidate(imgRef.ImageURI)
+	cleanUri := imgRef.ImageURI
+	badCharIndex := strings.IndexAny(cleanUri, "@:")
+	if badCharIndex > -1 {
+		cleanUri = imgRef.ImageURI[:badCharIndex]
+	}
+	tags, err := p.getDataToValidate(cleanUri)
 	if err != nil {
 		return false, err
 	}
diff --git a/docs/CONFIG.md b/docs/CONFIG.md
index b4896e8..5f3e2d1 100644
--- a/docs/CONFIG.md
+++ b/docs/CONFIG.md
@@ -22,8 +22,6 @@ is called.
 |`PFLT_NAMESPACE`|env|The namespace to use when running [OperatorSDK Scorecard](https://sdk.operatorframework.io/docs/testing-operators/scorecard/)|optional|[default](https://github.com/redhat-openshift-ecosystem/openshift-preflight/blob/main/cmd/defaults.go#L8)|
 |`PFLT_SERVICEACCOUNT`|env|The service account to use when running [OperatorSDK Scorecard](https://sdk.operatorframework.io/docs/testing-operators/scorecard/)|optional|[default](https://github.com/redhat-openshift-ecosystem/openshift-preflight/blob/main/cmd/defaults.go#L9)|
 |`PFLT_INDEXIMAGE`|env|The index image to use when testing that an operator is `DeployableByOLM`|required|-|
-|`PFLT_DOCKERCONFIG`|env|The full path to a dockerconfigjson file, which is pushed to the target test cluster to access images in private repositories in the `DeployableByOLM`. If empty, no secret is created and the resource is assumed to be public.|optional|-|
-
 
 For information on how to build an index image, see [BUILDING_AN_INDEX.md](BUILDING_AN_INDEX.md).
 
diff --git a/docs/RECIPES.md b/docs/RECIPES.md
index 32e4bd5..9f29776 100644
--- a/docs/RECIPES.md
+++ b/docs/RECIPES.md
@@ -31,6 +31,7 @@ test asset by executing the following.
 ```bash
 export KUBECONFIG=/path/to/your/kubeconfig 
 export PFLT_INDEXIMAGE=registry.example.org/your-namespace/your-index-image:sometag
+export PFLT_DOCKERCONFIG=~/.docker/config.json
 preflight check operator registry.example.org/your-namespace/your-bundle-image:sometag
 ```
 
@@ -58,6 +59,7 @@ $CONTAINER_TOOL run \
   --env KUBECONFIG=/kubeconfig \
   --env PFLT_LOGLEVEL=trace \
   --env PFLT_INDEXIMAGE=registry.example.org/your-namespace/your-index-image:sometag \
+  --env PFLT_DOCKERCONFIG=~/.docker/config.json \
   --env PFLT_ARTIFACTS=/artifacts \
   --env PFLT_LOGFILE=/artifacts/preflight.log \
   -v /some/path/on/your/host/artifacts:/artifacts \
@@ -109,6 +111,8 @@ spec:
             value: trace
           - name: PFLT_INDEXIMAGE
             value: "registry.example.org/your-namespace/your-index-image:sometag"
+          - name: PFLT_DOCKERCONFIG
+            value: "/root/.docker/config.json"
           - name: PFLT_LOGFILE
             value: "/artifacts/preflight.log"
           - name: PFLT_ARTIFACTS


Test patch:
diff --git a/certification/internal/policy/container/base_on_ubi_test.go b/certification/internal/policy/container/base_on_ubi_test.go
index e0b1839..1e29424 100644
--- a/certification/internal/policy/container/base_on_ubi_test.go
+++ b/certification/internal/policy/container/base_on_ubi_test.go
@@ -82,7 +82,7 @@ NAME="Red Hat Enterprise Linux"
 	})
 	Describe("Checking for UBI as a base", func() {
 		Context("When it is based on UBI", func() {
-			Context("and has the correct os-release", func() {
+			Context("and has the correct labels and os-release", func() {
 				It("should pass Validate", func() {
 					ok, err := basedOnUbiCheck.Validate(imageRef)
 					Expect(err).ToNot(HaveOccurred())
@@ -91,7 +91,7 @@ NAME="Red Hat Enterprise Linux"
 			})
 		})
 		Context("When it is not based on UBI", func() {
-			Context("and has a bad os-release", func() {
+			Context("and has the correct labels but bad os-release", func() {
 				JustBeforeEach(func() {
 					err := os.WriteFile(filepath.Join(imageRef.ImageFSPath, "etc", osrelease), []byte("Not a good file"), 0644)
 					Expect(err).ToNot(HaveOccurred())
@@ -102,6 +102,32 @@ NAME="Red Hat Enterprise Linux"
 					Expect(ok).To(BeFalse())
 				})
 			})
+			Context("and has does not have correct labels but good os-release", func() {
+				JustBeforeEach(func() {
+					fakeImage := fakecranev1.FakeImage{
+						ConfigFileStub: badLabelsConfigFile,
+					}
+					imageRef.ImageInfo = &fakeImage
+				})
+				It("should not pass Validate", func() {
+					ok, err := basedOnUbiCheck.Validate(imageRef)
+					Expect(err).ToNot(HaveOccurred())
+					Expect(ok).To(BeFalse())
+				})
+			})
+			Context("and it is missing the correct label", func() {
+				JustBeforeEach(func() {
+					fakeImage := fakecranev1.FakeImage{
+						ConfigFileStub: missingUbiLableConfigFile,
+					}
+					imageRef.ImageInfo = &fakeImage
+				})
+				It("should not pass Validate", func() {
+					ok, err := basedOnUbiCheck.Validate(imageRef)
+					Expect(err).ToNot(HaveOccurred())
+					Expect(ok).To(BeFalse())
+				})
+			})
 			Context("and os-release is missing", func() {
 				JustBeforeEach(func() {
 					os.Remove(filepath.Join(imageRef.ImageFSPath, "etc", osrelease))
diff --git a/certification/internal/policy/container/has_unique_tag_test.go b/certification/internal/policy/container/has_unique_tag_test.go
index 6a4e192..0ccc82e 100644
--- a/certification/internal/policy/container/has_unique_tag_test.go
+++ b/certification/internal/policy/container/has_unique_tag_test.go
@@ -1,6 +1,9 @@
 package container
 
 import (
+	"errors"
+	"strings"
+
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
 	"github.com/redhat-openshift-ecosystem/openshift-preflight/certification"
@@ -15,7 +18,11 @@ type fakeTagLister struct {
 	Tags []string
 }
 
-func (ftl *fakeTagLister) ListTags(_ string) ([]string, error) {
+func (ftl *fakeTagLister) ListTags(imageUri string) ([]string, error) {
+	if strings.ContainsAny(imageUri, "@:") {
+		// This would be an error from Crane
+		return nil, errors.New("repository can only contain the runes `abcdefghijklmnopqrstuvwxyz0123456789_-./`")
+	}
 	return ftl.Tags, nil
 }
 
@@ -45,6 +52,26 @@ var _ = Describe("UniqueTag", func() {
 				Expect(ok).To(BeFalse())
 			})
 		})
+		Context("When a tag is provided", func() {
+			BeforeEach(func() {
+				hasUniqueTagCheck = *NewHasUniqueTagCheck(&fakeTagLister{Tags: validImageTags()})
+			})
+			It("should pass Validate", func() {
+				ok, err := hasUniqueTagCheck.Validate(certification.ImageReference{ImageURI: "dummy/image:atag"})
+				Expect(err).ToNot(HaveOccurred())
+				Expect(ok).To(BeTrue())
+			})
+		})
+		Context("When sha256 is used", func() {
+			BeforeEach(func() {
+				hasUniqueTagCheck = *NewHasUniqueTagCheck(&fakeTagLister{Tags: validImageTags()})
+			})
+			It("should pass Validate", func() {
+				ok, err := hasUniqueTagCheck.Validate(certification.ImageReference{ImageURI: "dummy/image@sha256:thisisasha256"})
+				Expect(err).ToNot(HaveOccurred())
+				Expect(ok).To(BeTrue())
+			})
+		})
 	})
 })