Repository: moby/moby
File: daemon/pkg/opts/nri_opts.go (Go)

Code before:
package opts

import (
	"bytes"
	"encoding/csv"
	"encoding/json"
	"fmt"
	"strconv"
	"strings"
)

type NRIConfig struct {
	Enable           bool   `json:"enable,omitempty"`
	PluginPath       string `json:"plugin-path,omitempty"`
	PluginConfigPath string `json:"plugin-config-path,omitempty"`
	SocketPath       string `json:"socket-path,omitempty"`
}

func (c *NRIConfig) UnmarshalJSON(raw []byte) error {
	dec := json.NewDecoder(bytes.NewReader(raw))
	dec.DisallowUnknownFields()
	type nc *NRIConfig // prevent recursion
	if err := dec.Decode(nc(c)); err != nil {
		return err
	}
	return nil
}

// NRIConfigOpt is a NamedOption and flags.Value for NRI configuration parsing.
type NRIConfigOpt struct {
	Val *NRIConfig
}

func NewNamedNRIOptsRef(val *NRIConfig) *NRIConfigOpt {
	return &NRIConfigOpt{Val: val}
}

func (p *NRIConfigOpt) Set(value string) error {
	csvReader := csv.NewReader(strings.NewReader(value))
	fields, err := csvReader.Read()
	if err != nil {

Review comment (suggestion):
Just considering that ... maybe these should be a `[]string` to allow specifying paths (plural) for discovering plugins. As you (correctly) called out; NRI plugins for (say) CRI / kubernetes may not be applicable to non-kubernetes workloads (so having separate locations for docker and non-docker probably makes sense), there could be cases where they ARE expected to be applicable to both.

In the current situation in this PR, a user can configure these paths to match containerd's paths, in which case they're shared, but wouldn't be able to express "share all the containerd NRI plugins _AND_ look or dockerd-specific plugins in `<this location>`". If we make these a slice of paths, that's something that could be expressed; i.e., the dockerd-specific locations to be _expanding_ the list of NRI plugins that are applied without that meaning "create a copy of all plugins in both places".

In general, I'm still curious if NRI implements / should implement (or define) conditionals; with CDI, the user is in control whether CDI should be applied (and thus modify the OCI spec); for NRI that's not the case; it's unilaterally modifying the behavior of dockerd (containers) to patch the OCI config. I guess that's mostly by design, as NRI is intended to extend the runtime, and of course it could be left to the plugin to handle these conditions, but wondering if there's something to define / formalise on behavior (which could include "disable all NRI for this container / pod").

Code after:
package opts

import (
	"bytes"
	"encoding/csv"
	"encoding/json"
	"fmt"
	"strconv"
	"strings"
)

type NRIOpts struct {
	Enable           bool   `json:"enable,omitempty"`
	PluginPath       string `json:"plugin-path,omitempty"`
	PluginConfigPath string `json:"plugin-config-path,omitempty"`
	SocketPath       string `json:"socket-path,omitempty"`
}

func (c *NRIOpts) UnmarshalJSON(raw []byte) error {
	dec := json.NewDecoder(bytes.NewReader(raw))
	dec.DisallowUnknownFields()
	type nc *NRIOpts // prevent recursion
	if err := dec.Decode(nc(c)); err != nil {
		return err
	}
	return nil
}

// NamedNRIOpts is a NamedOption and flags.Value for NRI configuration parsing.
type NamedNRIOpts struct {
	Val *NRIOpts
}

func NewNamedNRIOptsRef(val *NRIOpts) *NamedNRIOpts {
	return &NamedNRIOpts{Val: val}
}

func (p *NamedNRIOpts) Set(value string) error {
	csvReader := csv.NewReader(strings.NewReader(value))
	fields, err := csvReader.Read()
	if err != nil {