Merge pull request #2682 from abrarshivani/imagepath-validation

Reject incomplete image specs in ImagePath

Author: abrarshivani

api/nvidia/v1/clusterpolicy_types.go

@@ -2061,6 +2061,9 @@ func imagePath(repository string, image string, version string, imagePathEnvName
 			crdImagePath = image
 		}
 	} else {
+		if repository == "" || image == "" || version == "" {
+			return "", fmt.Errorf("invalid image specification: repository, image and version must all be set (repository=%s, image=%s, version=%s)", repository, image, version)
+		}
 		// use @ if image digest is specified instead of tag
 		if strings.HasPrefix(version, "sha256:") {
 			crdImagePath = repository + "/" + image + "@" + version

api/nvidia/v1/clusterpolicy_types_test.go

@@ -0,0 +1,74 @@
+/**
+# Copyright (c) NVIDIA CORPORATION.  All rights reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+**/
+
+package v1
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestImagePath(t *testing.T) {
+	t.Run("valid spec builds repo/image:tag", func(t *testing.T) {
+		path, err := ImagePath(&DriverSpec{Repository: "nvcr.io/nvidia", Image: "driver", Version: "535"})
+		require.NoError(t, err)
+		assert.Equal(t, "nvcr.io/nvidia/driver:535", path)
+	})
+
+	t.Run("sha256 version builds a digest reference", func(t *testing.T) {
+		path, err := ImagePath(&DriverSpec{Repository: "nvcr.io/nvidia", Image: "driver", Version: "sha256:abc"})
+		require.NoError(t, err)
+		assert.Equal(t, "nvcr.io/nvidia/driver@sha256:abc", path)
+	})
+
+	t.Run("kbld passthrough when repository and version are empty", func(t *testing.T) {
+		path, err := ImagePath(&DriverSpec{Image: "nvcr.io/nvidia/driver@sha256:abc"})
+		require.NoError(t, err)
+		assert.Equal(t, "nvcr.io/nvidia/driver@sha256:abc", path)
+	})
+
+	t.Run("incomplete spec (empty repository) is rejected", func(t *testing.T) {
+		// A partial CR spec must fail fast; an env value must not rescue it.
+		t.Setenv("DRIVER_IMAGE", "from-env/driver:v9")
+		path, err := ImagePath(&DriverSpec{Image: "driver", Version: "535"})
+		require.Error(t, err)
+		assert.Empty(t, path)
+		assert.ErrorContains(t, err, "invalid image specification")
+	})
+
+	t.Run("incomplete spec (empty image) is rejected", func(t *testing.T) {
+		path, err := ImagePath(&DriverSpec{Repository: "nvcr.io/nvidia", Version: "535"})
+		require.Error(t, err)
+		assert.Empty(t, path)
+		assert.ErrorContains(t, err, "invalid image specification")
+	})
+
+	t.Run("incomplete spec (empty version) is rejected", func(t *testing.T) {
+		path, err := ImagePath(&DriverSpec{Repository: "nvcr.io/nvidia", Image: "driver"})
+		require.Error(t, err)
+		assert.Empty(t, path)
+		assert.ErrorContains(t, err, "invalid image specification")
+	})
+
+	t.Run("unsupported spec type errors", func(t *testing.T) {
+		path, err := ImagePath("not-a-spec")
+		require.Error(t, err)
+		assert.Empty(t, path)
+		assert.ErrorContains(t, err, "invalid type to construct image path")
+	})
+}

internal/image/image.go

@@ -32,6 +32,9 @@ func ImagePath(repository string, image string, version string, imagePathEnvName
 			crdImagePath = image
 		}
 	} else {
+		if repository == "" || image == "" || version == "" {
+			return "", fmt.Errorf("invalid image specification: repository, image and version must all be set (repository=%s, image=%s, version=%s)", repository, image, version)
+		}
 		// use @ if image digest is specified instead of tag
 		if strings.HasPrefix(version, "sha256:") {
 			crdImagePath = repository + "/" + image + "@" + version

internal/image/imagepath_validation_test.go

@@ -0,0 +1,127 @@
+/**
+# Copyright (c) NVIDIA CORPORATION.  All rights reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+**/
+
+package image
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestImagePath_ValidSpec(t *testing.T) {
+	const envName = "TEST_IMAGE_PATH_VALID"
+
+	t.Run("repository, image and tag version resolve to repo/image:tag", func(t *testing.T) {
+		path, err := ImagePath("nvcr.io/nvidia", "gpu-operator", "v1.0.0", envName)
+		require.NoError(t, err)
+		assert.Equal(t, "nvcr.io/nvidia/gpu-operator:v1.0.0", path)
+	})
+
+	t.Run("sha256 version resolves to a digest reference", func(t *testing.T) {
+		path, err := ImagePath("nvcr.io/nvidia", "gpu-operator", "sha256:cafe", envName)
+		require.NoError(t, err)
+		assert.Equal(t, "nvcr.io/nvidia/gpu-operator@sha256:cafe", path)
+	})
+}
+
+func TestImagePath_InvalidSpec(t *testing.T) {
+	const envName = "TEST_IMAGE_PATH_ENV_INVALID"
+
+	testCases := []struct {
+		description string
+		repository  string
+		image       string
+		version     string
+	}{
+		{
+			description: "empty repository with tag version",
+			repository:  "",
+			image:       "gpu-operator",
+			version:     "v1.0.0",
+		},
+		{
+			description: "empty repository with digest version",
+			repository:  "",
+			image:       "gpu-operator",
+			version:     "sha256:cafe",
+		},
+		{
+			description: "empty image with repository and version set",
+			repository:  "nvcr.io/nvidia",
+			image:       "",
+			version:     "v1.0.0",
+		},
+		{
+			description: "empty repository and image with tag version",
+			repository:  "",
+			image:       "",
+			version:     "v1.0.0",
+		},
+		{
+			description: "empty repository and image with digest version",
+			repository:  "",
+			image:       "",
+			version:     "sha256:abc",
+		},
+		{
+			description: "empty version with repository and image set",
+			repository:  "nvcr.io/nvidia",
+			image:       "gpu-operator",
+			version:     "",
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.description, func(t *testing.T) {
+			// An env value must not rescue an incomplete spec.
+			t.Setenv(envName, "from-env/op:v9")
+
+			path, err := ImagePath(tc.repository, tc.image, tc.version, envName)
+
+			require.Error(t, err)
+			assert.Empty(t, path)
+			assert.ErrorContains(t, err, "invalid image specification")
+		})
+	}
+}
+
+func TestImagePath_KbldPassthrough(t *testing.T) {
+	const envName = "TEST_IMAGE_PATH_KBLD"
+
+	path, err := ImagePath("", "nvcr.io/nvidia/driver@sha256:cafe", "", envName)
+	require.NoError(t, err)
+	assert.Equal(t, "nvcr.io/nvidia/driver@sha256:cafe", path)
+}
+
+func TestImagePath_EnvFallback(t *testing.T) {
+	const envName = "TEST_IMAGE_PATH_ENV_FALLBACK"
+	t.Setenv(envName, "from-env/op:v9")
+
+	path, err := ImagePath("", "", "", envName)
+	require.NoError(t, err)
+	assert.Equal(t, "from-env/op:v9", path)
+}
+
+func TestImagePath_EmptyError(t *testing.T) {
+	const envName = "TEST_IMAGE_PATH_EMPTY"
+
+	path, err := ImagePath("", "", "", envName)
+	require.Error(t, err)
+	assert.Empty(t, path)
+	assert.ErrorContains(t, err, "empty image path provided through both CR and ENV "+envName)
+}