Fix OCPBUGS-11840: ParseImageReference supports cases where both tag …

…and digest are present in a ref (#633)

pkg/api/v1alpha2/types_config.go

@@ -186,34 +186,39 @@ func (o Operator) GetUniqueName() (string, error) {
 // from the imageName.
 // It can handle both remote and local images.
 func ParseImageReference(imageName string) (string, string, string, string, string) {
-	registry, org, repo, tag, sha := "", "", "", "", ""
+	registry, namespace, repo, tag, sha := "", "", "", "", ""
 	imageName = TrimProtocol(imageName)
 	imageName = strings.TrimPrefix(imageName, "/")
 	imageName = strings.TrimSuffix(imageName, "/")
-	tmp := strings.Split(imageName, "/")
+	pathComponents := strings.Split(imageName, "/")
 
-	if len(tmp) > 1 {
-		registry = tmp[0]
+	// For more than 2 pathComponents, the first must be the registry
+	if len(pathComponents) > 1 {
+		registry = pathComponents[0]
 	}
-
-	img := strings.Split(tmp[len(tmp)-1], ":")
-	if len(tmp) > 2 {
-		org = strings.Join(tmp[1:len(tmp)-1], "/")
+	// For more than 3 pathComponents, everything in between registry (first) and
+	// repository (last) is considered to be namespace or organisation
+	if len(pathComponents) > 2 {
+		namespace = strings.Join(pathComponents[1:len(pathComponents)-1], "/")
 	}
+
+	// It is best to split first on digest, as the `:` might
+	// exist for the tag or for the digest
+	img := strings.Split(pathComponents[len(pathComponents)-1], "@sha256:")
+	// The first element in the slice will always exist (repository)
+	repo = img[0]
 	if len(img) > 1 {
-		if strings.Contains(img[0], "@") {
-			nm := strings.Split(img[0], "@")
-			repo = nm[0]
-			sha = img[1]
-		} else {
-			repo = img[0]
-			tag = img[1]
-		}
-	} else {
-		repo = img[0]
+		sha = img[1]
+	}
+
+	// We check now for the existance of a tag
+	if strings.Contains(repo, ":") {
+		nm := strings.Split(repo, ":")
+		repo = nm[0]
+		tag = nm[1]
 	}
 
-	return registry, org, repo, tag, sha
+	return registry, namespace, repo, tag, sha
 }
 
 // trimProtocol removes oci://, file:// or docker:// from

pkg/api/v1alpha2/types_config_test.go

@@ -6,6 +6,58 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
+func TestParseImageReference(t *testing.T) {
+	type spec struct {
+		desc          string
+		input         string
+		expRegistry   string
+		expNamespace  string
+		expRepository string
+		expDigest     string
+		expTag        string
+	}
+
+	specs := []spec{
+		{
+			desc:          "remote image with both tag and digest",
+			input:         "cp.icr.io/cp/cpd/postgresql:13.7@sha256:e05434bfdb4b306fbc2e697112e1343907e368eb5f348c1779562d31b9f32ac5",
+			expRegistry:   "cp.icr.io",
+			expNamespace:  "cp/cpd",
+			expRepository: "postgresql",
+			expDigest:     "e05434bfdb4b306fbc2e697112e1343907e368eb5f348c1779562d31b9f32ac5",
+			expTag:        "13.7",
+		},
+		{
+			desc:          "remote image with tag",
+			input:         "quay.io/redhatgov/oc-mirror-dev:foo-bundle-v0.3.1",
+			expRegistry:   "quay.io",
+			expNamespace:  "redhatgov",
+			expRepository: "oc-mirror-dev",
+			expDigest:     "",
+			expTag:        "foo-bundle-v0.3.1",
+		},
+		{
+			desc:          "remote image with digest",
+			input:         "quay.io/redhatgov/oc-mirror-dev@sha256:7e1e74b87a503e95db5203334917856f61aece90a72e8d53a9fd903344eb78a5",
+			expRegistry:   "quay.io",
+			expNamespace:  "redhatgov",
+			expRepository: "oc-mirror-dev",
+			expDigest:     "7e1e74b87a503e95db5203334917856f61aece90a72e8d53a9fd903344eb78a5",
+			expTag:        "",
+		},
+	}
+
+	for _, s := range specs {
+		t.Run(s.desc, func(t *testing.T) {
+			reg, ns, repo, tag, id := ParseImageReference(s.input)
+			require.Equal(t, s.expRegistry, reg)
+			require.Equal(t, s.expNamespace, ns)
+			require.Equal(t, s.expRepository, repo)
+			require.Equal(t, s.expTag, tag)
+			require.Equal(t, s.expDigest, id)
+		})
+	}
+}
 func TestGetUniqueName(t *testing.T) {
 	type spec struct {
 		desc     string