Add basic producer package by elezar · Pull Request #308 · cncf-tags/container-device-interface

elezar · 2026-02-18T16:14:35Z

This change revisits the idea of a separate producer package that was discussed prior to the v1.0.0 release.

Instead of relying on a separate module at this stage, I am using a new package at pkg/cdi-producer.

This is still a rough draft and I need to add better documentation to the API.

I have also created NVIDIA/nvidia-container-toolkit#1665 where I demonstrate the use of the API.

elezar · 2026-02-18T16:17:27Z

pkg/cdi/spec.go

 		priority: priority,
 	}

-	if ext := filepath.Ext(spec.path); ext != ".yaml" && ext != ".json" {


I don't think we should be setting this in newSpec since we also call this when READING a spec. In which case we either do this TWICE, or modify the path before reading.

elezar · 2026-02-18T16:18:29Z

pkg/cdi/cache.go

-	var (
-		specDir string
-		path    string
-		err     error
-	)


Personal preference, but declaring these up front add additional vertical space to a function. Happy to revert if there is a strong opinion.

elezar · 2026-02-18T16:19:16Z

pkg/cdi/cache.go

-	specDir, _ = c.highestPrioritySpecDir()
+	specDir, _ := c.highestPrioritySpecDir()
 	if specDir == "" {
 		return errors.New("no Spec directories to remove from")


One question here: Should this really be an error when removing a spec? If this fails is that not essentially the same as "Not Found" ... we could even return a wrapped NotFound here.

Yes, I think returning a wrapped appropriate error (maybe an fs.ErrNotExist) would probably be the right thing here, giving the caller an easy way to identify and ignore it at will.

Created #309 since this is separate from the producer API.

Signed-off-by: Evan Lezar <elezar@nvidia.com>

klihub · 2026-02-19T13:47:47Z

pkg/cdi/producer/save.go

+		if err != nil {
+			return nil, err
+		}
+		data = append([]byte("---\n"), data...)


Why do we add this document separator in front ?

This is to allow cat /var/run/*.yaml work as expected.

See here #126

(note that with the options we could now make this opt-out)

klihub · 2026-02-19T13:55:02Z

pkg/cdi/spec.go

 type validator interface {
 	Validate(*cdi.Spec) error
 }


nit: since this is now also defined in the newly added package, wouldn't it make sense define it as an exported type in one and have an alias for it in the other with a type Validator = $PKG.Validator ?

I didn't want this PR to become about validation. We already have #247 open for this, and I would rather address reworking validation in a follow-up to keep the scope of this PR limited to actually writing specs.

klihub · 2026-02-19T13:56:50Z

pkg/cdi/spec.go

+		// we assume a json format.
+		producer.WithOutputFormat("json"),
+		producer.WithOverwrite(overwrite),
+		producer.WithPermissions(0),


nit: Isn't this the zero-value so we could just omit it ?

The current implementation has permissions set to 0644 by default. I can change this though.

Update: My motivation for changing the default to 0644 in the producer package is that this generally does the right thing from a general producer perspective. The spec files NEED to be world readable to be generally useful since we have no control over the user that the consumer is running as.

I think in the long run we should consider deprecating the cache.WriteSpec() function and require that producers explicitly use the producer API.

klihub · 2026-02-19T14:05:49Z

pkg/cdi-producer/save.go

+}
+
+func assertNotDirectory(path string) error {
+	info, err := os.Lstat(path)


Shouldn't this be os.Stat ? Now this will pass the check if path is a symlink pointing to a directory, but what happens later, and is it intentional ?

It was intentional, but I think I was getting ahead of myself in terms of path traversal errors. Let me change this to os.Stat for simplicity since this doesn't actually handle the link case explicitly.

klihub · 2026-02-19T15:40:22Z

pkg/cdi/producer/renamein_linux.go

@@ -14,7 +14,7 @@
   limitations under the License.
 */

-package cdi
+package producer


nit: this new package could also be placed under pkg/cdi/producer.

Yes, I was uncertain of where to store this. In the longer term, it may be useful to have this be a separate go module so that producers don't need to import all the unwanted dependencies. (e.g. spec generator).

I moved this back to pkg/cdi/producer.

Just a note that I think having it under pkg/cdi/producer shouldn't prevent it from being its own go module... although it might be that the location would be a bit strange if it were.

That's true. I don't want to go into the module discussion in the first iteration though.

This change addes a new producer subpackage to cdi. The purpose of this package is to handle use-cases that do not require a CDI cache and are only concerned with generating and outputing CDI specifications. Signed-off-by: Evan Lezar <elezar@nvidia.com>

Signed-off-by: Evan Lezar <elezar@nvidia.com>

elezar commented Feb 18, 2026

View reviewed changes

elezar force-pushed the improve-cache-save branch 6 times, most recently from 68087b7 to 6fc7db9 Compare February 18, 2026 21:08

test: Fix tests on darwin

3b02a92

Signed-off-by: Evan Lezar <elezar@nvidia.com>

elezar force-pushed the improve-cache-save branch from 6fc7db9 to 33d83be Compare February 18, 2026 21:38

elezar mentioned this pull request Feb 18, 2026

TOFIX: Use new container-device-interface producer API NVIDIA/nvidia-container-toolkit#1665

Draft

elezar requested review from bart0sh and klihub February 19, 2026 08:24

klihub reviewed Feb 19, 2026

View reviewed changes

elezar mentioned this pull request Feb 19, 2026

Return a wrapped fs.ErrNotExist if no spec directories are defined #309

Open

elezar force-pushed the improve-cache-save branch 2 times, most recently from 82ed0f3 to 437610e Compare February 20, 2026 08:01

elezar added 3 commits February 20, 2026 09:05

Add cdi/producer package

07ab0e4

This change addes a new producer subpackage to cdi. The purpose of this package is to handle use-cases that do not require a CDI cache and are only concerned with generating and outputing CDI specifications. Signed-off-by: Evan Lezar <elezar@nvidia.com>

Add WithValidator option to producer API

dd8a82c

Signed-off-by: Evan Lezar <elezar@nvidia.com>

Add a content-based spec validator

3f95ba4

Signed-off-by: Evan Lezar <elezar@nvidia.com>

elezar force-pushed the improve-cache-save branch from 437610e to 3f95ba4 Compare February 20, 2026 08:08

elezar marked this pull request as ready for review February 20, 2026 13:29

Conversation

elezar commented Feb 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

klihub Feb 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

elezar Feb 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

klihub Feb 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Comments

elezar commented Feb 18, 2026 •

edited

Loading

klihub Feb 19, 2026 •

edited

Loading

elezar Feb 19, 2026 •

edited

Loading

klihub Feb 19, 2026 •

edited

Loading