Skip to content

Commit 7dbfffc

Browse files
abrarshivaniabrarshivani
committed
Reject incomplete image specs in ImagePath
When a CR component sets repository or version but leaves repository, image or version empty, ImagePath/imagePath previously concatenated the parts into a structurally invalid reference such as "/image:tag", "repo/:tag" or "repo/image:", which only failed much later at image pull with an opaque error. Validate the branch that builds "repo/image:tag"/"@digest" and return a clear error when repository, image or version is empty, in both image.ImagePath (internal/image) and the duplicated api/nvidia/v1 imagePath used by v1.ImagePath. The error uses %s (not %q) so the field values stay readable in the operator's JSON logs. The kbld/carvel passthrough (repository and version both empty) and the env-var fallback are unchanged. Add tests covering the valid tag/digest, kbld passthrough, env fallback, all-empty error, incomplete-spec and unsupported-type paths. Signed-off-by: Abrar Shivani <ashivani@nvidia.com>
1 parent 7adc442 commit 7dbfffc

4 files changed

Lines changed: 207 additions & 0 deletions

File tree

‎api/nvidia/v1/clusterpolicy_types.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2061,6 +2061,9 @@ func imagePath(repository string, image string, version string, imagePathEnvName
20612061
crdImagePath = image
20622062
}
20632063
} else {
2064+
if repository == "" || image == "" || version == "" {
2065+
return "", fmt.Errorf("invalid image specification: repository, image and version must all be set (repository=%s, image=%s, version=%s)", repository, image, version)
2066+
}
20642067
// use @ if image digest is specified instead of tag
20652068
if strings.HasPrefix(version, "sha256:") {
20662069
crdImagePath = repository + "/" + image + "@" + version

‎api/nvidia/v1/clusterpolicy_types_test.go‎

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
/**
2+
# Copyright (c) NVIDIA CORPORATION. All rights reserved.
3+
#
4+
# Licensed under the Apache License, Version 2.0 (the "License");
5+
# you may not use this file except in compliance with the License.
6+
# You may obtain a copy of the License at
7+
#
8+
# http://www.apache.org/licenses/LICENSE-2.0
9+
#
10+
# Unless required by applicable law or agreed to in writing, software
11+
# distributed under the License is distributed on an "AS IS" BASIS,
12+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
# See the License for the specific language governing permissions and
14+
# limitations under the License.
15+
**/
16+
17+
package v1
18+
19+
import (
20+
"testing"
21+
22+
"github.com/stretchr/testify/assert"
23+
"github.com/stretchr/testify/require"
24+
)
25+
26+
func TestImagePath(t *testing.T) {
27+
t.Run("valid spec builds repo/image:tag", func(t *testing.T) {
28+
path, err := ImagePath(&DriverSpec{Repository: "nvcr.io/nvidia", Image: "driver", Version: "535"})
29+
require.NoError(t, err)
30+
assert.Equal(t, "nvcr.io/nvidia/driver:535", path)
31+
})
32+
33+
t.Run("sha256 version builds a digest reference", func(t *testing.T) {
34+
path, err := ImagePath(&DriverSpec{Repository: "nvcr.io/nvidia", Image: "driver", Version: "sha256:abc"})
35+
require.NoError(t, err)
36+
assert.Equal(t, "nvcr.io/nvidia/driver@sha256:abc", path)
37+
})
38+
39+
t.Run("kbld passthrough when repository and version are empty", func(t *testing.T) {
40+
path, err := ImagePath(&DriverSpec{Image: "nvcr.io/nvidia/driver@sha256:abc"})
41+
require.NoError(t, err)
42+
assert.Equal(t, "nvcr.io/nvidia/driver@sha256:abc", path)
43+
})
44+
45+
t.Run("incomplete spec (empty repository) is rejected", func(t *testing.T) {
46+
// A partial CR spec must fail fast; an env value must not rescue it.
47+
t.Setenv("DRIVER_IMAGE", "from-env/driver:v9")
48+
path, err := ImagePath(&DriverSpec{Image: "driver", Version: "535"})
49+
require.Error(t, err)
50+
assert.Empty(t, path)
51+
assert.ErrorContains(t, err, "invalid image specification")
52+
})
53+
54+
t.Run("incomplete spec (empty image) is rejected", func(t *testing.T) {
55+
path, err := ImagePath(&DriverSpec{Repository: "nvcr.io/nvidia", Version: "535"})
56+
require.Error(t, err)
57+
assert.Empty(t, path)
58+
assert.ErrorContains(t, err, "invalid image specification")
59+
})
60+
61+
t.Run("incomplete spec (empty version) is rejected", func(t *testing.T) {
62+
path, err := ImagePath(&DriverSpec{Repository: "nvcr.io/nvidia", Image: "driver"})
63+
require.Error(t, err)
64+
assert.Empty(t, path)
65+
assert.ErrorContains(t, err, "invalid image specification")
66+
})
67+
68+
t.Run("unsupported spec type errors", func(t *testing.T) {
69+
path, err := ImagePath("not-a-spec")
70+
require.Error(t, err)
71+
assert.Empty(t, path)
72+
assert.ErrorContains(t, err, "invalid type to construct image path")
73+
})
74+
}

‎internal/image/image.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,9 @@ func ImagePath(repository string, image string, version string, imagePathEnvName
3232
crdImagePath = image
3333
}
3434
} else {
35+
if repository == "" || image == "" || version == "" {
36+
return "", fmt.Errorf("invalid image specification: repository, image and version must all be set (repository=%s, image=%s, version=%s)", repository, image, version)
37+
}
3538
// use @ if image digest is specified instead of tag
3639
if strings.HasPrefix(version, "sha256:") {
3740
crdImagePath = repository + "/" + image + "@" + version

‎internal/image/imagepath_validation_test.go‎

Lines changed: 127 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,127 @@
1+
/**
2+
# Copyright (c) NVIDIA CORPORATION. All rights reserved.
3+
#
4+
# Licensed under the Apache License, Version 2.0 (the "License");
5+
# you may not use this file except in compliance with the License.
6+
# You may obtain a copy of the License at
7+
#
8+
# http://www.apache.org/licenses/LICENSE-2.0
9+
#
10+
# Unless required by applicable law or agreed to in writing, software
11+
# distributed under the License is distributed on an "AS IS" BASIS,
12+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
# See the License for the specific language governing permissions and
14+
# limitations under the License.
15+
**/
16+
17+
package image
18+
19+
import (
20+
"testing"
21+
22+
"github.com/stretchr/testify/assert"
23+
"github.com/stretchr/testify/require"
24+
)
25+
26+
func TestImagePath_ValidSpec(t *testing.T) {
27+
const envName = "TEST_IMAGE_PATH_VALID"
28+
29+
t.Run("repository, image and tag version resolve to repo/image:tag", func(t *testing.T) {
30+
path, err := ImagePath("nvcr.io/nvidia", "gpu-operator", "v1.0.0", envName)
31+
require.NoError(t, err)
32+
assert.Equal(t, "nvcr.io/nvidia/gpu-operator:v1.0.0", path)
33+
})
34+
35+
t.Run("sha256 version resolves to a digest reference", func(t *testing.T) {
36+
path, err := ImagePath("nvcr.io/nvidia", "gpu-operator", "sha256:cafe", envName)
37+
require.NoError(t, err)
38+
assert.Equal(t, "nvcr.io/nvidia/gpu-operator@sha256:cafe", path)
39+
})
40+
}
41+
42+
func TestImagePath_InvalidSpec(t *testing.T) {
43+
const envName = "TEST_IMAGE_PATH_ENV_INVALID"
44+
45+
testCases := []struct {
46+
description string
47+
repository string
48+
image string
49+
version string
50+
}{
51+
{
52+
description: "empty repository with tag version",
53+
repository: "",
54+
image: "gpu-operator",
55+
version: "v1.0.0",
56+
},
57+
{
58+
description: "empty repository with digest version",
59+
repository: "",
60+
image: "gpu-operator",
61+
version: "sha256:cafe",
62+
},
63+
{
64+
description: "empty image with repository and version set",
65+
repository: "nvcr.io/nvidia",
66+
image: "",
67+
version: "v1.0.0",
68+
},
69+
{
70+
description: "empty repository and image with tag version",
71+
repository: "",
72+
image: "",
73+
version: "v1.0.0",
74+
},
75+
{
76+
description: "empty repository and image with digest version",
77+
repository: "",
78+
image: "",
79+
version: "sha256:abc",
80+
},
81+
{
82+
description: "empty version with repository and image set",
83+
repository: "nvcr.io/nvidia",
84+
image: "gpu-operator",
85+
version: "",
86+
},
87+
}
88+
89+
for _, tc := range testCases {
90+
t.Run(tc.description, func(t *testing.T) {
91+
// An env value must not rescue an incomplete spec.
92+
t.Setenv(envName, "from-env/op:v9")
93+
94+
path, err := ImagePath(tc.repository, tc.image, tc.version, envName)
95+
96+
require.Error(t, err)
97+
assert.Empty(t, path)
98+
assert.ErrorContains(t, err, "invalid image specification")
99+
})
100+
}
101+
}
102+
103+
func TestImagePath_KbldPassthrough(t *testing.T) {
104+
const envName = "TEST_IMAGE_PATH_KBLD"
105+
106+
path, err := ImagePath("", "nvcr.io/nvidia/driver@sha256:cafe", "", envName)
107+
require.NoError(t, err)
108+
assert.Equal(t, "nvcr.io/nvidia/driver@sha256:cafe", path)
109+
}
110+
111+
func TestImagePath_EnvFallback(t *testing.T) {
112+
const envName = "TEST_IMAGE_PATH_ENV_FALLBACK"
113+
t.Setenv(envName, "from-env/op:v9")
114+
115+
path, err := ImagePath("", "", "", envName)
116+
require.NoError(t, err)
117+
assert.Equal(t, "from-env/op:v9", path)
118+
}
119+
120+
func TestImagePath_EmptyError(t *testing.T) {
121+
const envName = "TEST_IMAGE_PATH_EMPTY"
122+
123+
path, err := ImagePath("", "", "", envName)
124+
require.Error(t, err)
125+
assert.Empty(t, path)
126+
assert.ErrorContains(t, err, "empty image path provided through both CR and ENV "+envName)
127+
}

0 commit comments

Comments
 (0)