Repository: spegel-org/spegel
Pull request #824 (2025-04-15 07:49:33)

Fix improper image string formatting and expand tests
During the refactoring of OCI images some error was introduced in the image string formatting. This doesn't have an functional impact as it is only used in errors and logs but still causes confusion.

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/spegel-org/spegel/pull/824?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=spegel-org) Report
All modified and coverable lines are covered by tests :white_check_mark:

| [Files with missing lines](https://app.codecov.io/gh/spegel-org/spegel/pull/824?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=spegel-org) | Coverage Δ | |
|---|---|---|
| [pkg/oci/image.go](https://app.codecov.io/gh/spegel-org/spegel/pull/824?src=pr&el=tree&filepath=pkg%2Foci%2Fimage.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=spegel-org#diff-cGtnL29jaS9pbWFnZS5nbw==) | `70.00% <100.00%> (+11.11%)` | :arrow_up: |

<details><summary> :rocket: New features to boost your workflow: </summary>

- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
</details>

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 521d5a1..18c848f 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -21,6 +21,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 
+- [#824](https://github.com/spegel-org/spegel/pull/824) Fix improper image string formatting and expand tests.
+
 ### Security
 
 ## v0.1.1
diff --git a/pkg/oci/image.go b/pkg/oci/image.go
index f806c37..7d1949d 100644
--- a/pkg/oci/image.go
+++ b/pkg/oci/image.go
@@ -50,7 +50,7 @@ func (i Image) String() string {
 	if i.Digest != "" {
 		digest = "@" + i.Digest.String()
 	}
-	return fmt.Sprintf("%s/%s%s@%s", i.Registry, i.Repository, tag, digest)
+	return fmt.Sprintf("%s/%s%s%s", i.Registry, i.Repository, tag, digest)
 }
 
 func (i Image) TagName() (string, bool) {


Test patch:
diff --git a/pkg/oci/image_test.go b/pkg/oci/image_test.go
index 98d5fbe..e0757dd 100644
--- a/pkg/oci/image_test.go
+++ b/pkg/oci/image_test.go
@@ -16,6 +16,7 @@ func TestParseImageRequireDigest(t *testing.T) {
 		image              string
 		expectedRepository string
 		expectedTag        string
+		expectedString     string
 		expectedDigest     digest.Digest
 		expectedIsLatest   bool
 		digestInImage      bool
@@ -28,6 +29,7 @@ func TestParseImageRequireDigest(t *testing.T) {
 			expectedTag:        "latest",
 			expectedDigest:     digest.Digest("sha256:c0669ef34cdc14332c0f1ab0c2c01acb91d96014b172f1a76f3a39e63d1f0bda"),
 			expectedIsLatest:   true,
+			expectedString:     "library/ubuntu:latest@sha256:c0669ef34cdc14332c0f1ab0c2c01acb91d96014b172f1a76f3a39e63d1f0bda",
 		},
 		{
 			name:               "Only tag",
@@ -37,6 +39,7 @@ func TestParseImageRequireDigest(t *testing.T) {
 			expectedTag:        "3.18.0",
 			expectedDigest:     digest.Digest("sha256:c0669ef34cdc14332c0f1ab0c2c01acb91d96014b172f1a76f3a39e63d1f0bda"),
 			expectedIsLatest:   false,
+			expectedString:     "library/alpine:3.18.0@sha256:c0669ef34cdc14332c0f1ab0c2c01acb91d96014b172f1a76f3a39e63d1f0bda",
 		},
 		{
 			name:               "Tag and digest",
@@ -46,6 +49,7 @@ func TestParseImageRequireDigest(t *testing.T) {
 			expectedTag:        "3.18.0",
 			expectedDigest:     digest.Digest("sha256:c0669ef34cdc14332c0f1ab0c2c01acb91d96014b172f1a76f3a39e63d1f0bda"),
 			expectedIsLatest:   false,
+			expectedString:     "jetstack/cert-manager-controller:3.18.0@sha256:c0669ef34cdc14332c0f1ab0c2c01acb91d96014b172f1a76f3a39e63d1f0bda",
 		},
 		{
 			name:               "Only digest",
@@ -55,6 +59,7 @@ func TestParseImageRequireDigest(t *testing.T) {
 			expectedTag:        "",
 			expectedDigest:     digest.Digest("sha256:c0669ef34cdc14332c0f1ab0c2c01acb91d96014b172f1a76f3a39e63d1f0bda"),
 			expectedIsLatest:   false,
+			expectedString:     "fluxcd/helm-controller@sha256:c0669ef34cdc14332c0f1ab0c2c01acb91d96014b172f1a76f3a39e63d1f0bda",
 		},
 		{
 			name:               "Digest only in extra digest",
@@ -63,6 +68,7 @@ func TestParseImageRequireDigest(t *testing.T) {
 			expectedRepository: "foo/bar",
 			expectedDigest:     digest.Digest("sha256:c0669ef34cdc14332c0f1ab0c2c01acb91d96014b172f1a76f3a39e63d1f0bda"),
 			expectedIsLatest:   false,
+			expectedString:     "foo/bar@sha256:c0669ef34cdc14332c0f1ab0c2c01acb91d96014b172f1a76f3a39e63d1f0bda",
 		},
 	}
 	registries := []string{"docker.io", "quay.io", "ghcr.com", "127.0.0.1"}
@@ -91,22 +97,100 @@ func TestParseImageRequireDigest(t *testing.T) {
 						require.True(t, ok)
 						require.Equal(t, registry+"/"+tt.expectedRepository+":"+tt.expectedTag, tagName)
 					}
+					require.Equal(t, fmt.Sprintf("%s/%s", registry, tt.expectedString), img.String())
 				}
 			})
 		}
 	}
 }
 
-func TestParseImageDigestDoesNotMatch(t *testing.T) {
+func TestParseImageRequireDigestErrors(t *testing.T) {
 	t.Parallel()
 
-	_, err := ParseImageRequireDigest("quay.io/jetstack/cert-manager-webhook@sha256:13fd9eaadb4e491ef0e1d82de60cb199f5ad2ea5a3f8e0c19fdf31d91175b9cb", digest.Digest("sha256:ec4306b243d98cce7c3b1f994f2dae660059ef521b2b24588cfdc950bd816d4c"))
-	require.EqualError(t, err, "invalid digest set does not match parsed digest: quay.io/jetstack/cert-manager-webhook@sha256:13fd9eaadb4e491ef0e1d82de60cb199f5ad2ea5a3f8e0c19fdf31d91175b9cb sha256:13fd9eaadb4e491ef0e1d82de60cb199f5ad2ea5a3f8e0c19fdf31d91175b9cb")
+	tests := []struct {
+		name          string
+		s             string
+		dgst          digest.Digest
+		expectedError string
+	}{
+		{
+			name:          "digests do not match",
+			s:             "quay.io/jetstack/cert-manager-webhook@sha256:13fd9eaadb4e491ef0e1d82de60cb199f5ad2ea5a3f8e0c19fdf31d91175b9cb",
+			dgst:          digest.Digest("sha256:ec4306b243d98cce7c3b1f994f2dae660059ef521b2b24588cfdc950bd816d4c"),
+			expectedError: "invalid digest set does not match parsed digest: quay.io/jetstack/cert-manager-webhook@sha256:13fd9eaadb4e491ef0e1d82de60cb199f5ad2ea5a3f8e0c19fdf31d91175b9cb sha256:13fd9eaadb4e491ef0e1d82de60cb199f5ad2ea5a3f8e0c19fdf31d91175b9cb",
+		},
+		{
+			name:          "no tag or digest",
+			s:             "ghcr.io/spegel-org/spegel",
+			dgst:          "",
+			expectedError: "image needs to contain a digest",
+		},
+		{
+			name:          "reference contains protocol",
+			s:             "https://example.com/test:latest",
+			dgst:          "",
+			expectedError: "invalid reference",
+		},
+		{
+			name:          "unparsable url",
+			s:             "example%#$.com/foo",
+			dgst:          "",
+			expectedError: "parse \"dummy://example%\": invalid URL escape \"%\"",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			_, err := ParseImageRequireDigest(tt.s, tt.dgst)
+			require.EqualError(t, err, tt.expectedError)
+		})
+	}
 }
 
-func TestParseImageNoTagOrDigest(t *testing.T) {
+func TestNewImageErrors(t *testing.T) {
 	t.Parallel()
 
-	_, err := ParseImageRequireDigest("ghcr.io/spegel-org/spegel", digest.Digest(""))
-	require.EqualError(t, err, "image needs to contain a digest")
+	// TODO (phillebaba): Add test case for no digest or tag. One needs to be set.
+	tests := []struct {
+		name          string
+		registry      string
+		repository    string
+		tag           string
+		dgst          digest.Digest
+		expectedError string
+	}{
+		{
+			name:          "missing registry",
+			registry:      "",
+			repository:    "foo/bar",
+			tag:           "latest",
+			dgst:          digest.Digest("sha256:ec4306b243d98cce7c3b1f994f2dae660059ef521b2b24588cfdc950bd816d4c"),
+			expectedError: "image needs to contain a registry",
+		},
+		{
+			name:          "missing repository",
+			registry:      "example.com",
+			repository:    "",
+			tag:           "latest",
+			dgst:          digest.Digest("sha256:ec4306b243d98cce7c3b1f994f2dae660059ef521b2b24588cfdc950bd816d4c"),
+			expectedError: "image needs to contain a repository",
+		},
+		{
+			name:          "invalid digest",
+			registry:      "example.com",
+			repository:    "foo/bar",
+			tag:           "latest",
+			dgst:          digest.Digest("test"),
+			expectedError: "invalid checksum digest format",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			_, err := NewImage(tt.registry, tt.repository, tt.tag, tt.dgst)
+			require.EqualError(t, err, tt.expectedError)
+		})
+	}
 }