Skip to content

Commit

Permalink
pkg/adaptation: eliminate configuration file.
Browse files Browse the repository at this point in the history 
Eliminate the global NRI configuration file. It had one
remaining option for controlling whether NRI listens to
connection attempts from plugins launched independently
from NRI itself.

To make up for the removed configuration file option,
introduce a WithDisabledExternalConnections() option to
allow controlling external connections. Also introduce
a WithPluginConfigPath() option for controlling which
directory is used to look up optional plugin-specific
configuration for NRI-launched plugins. This used to be
deducted from the location of the now defunct config
file.

Signed-off-by: Krisztian Litkey <[email protected]>
  • Loading branch information
@klihub
klihub committed Feb 1, 2023
1 parent b3cabde commit cb3e20a
Show file tree
Hide file tree
Showing 7 changed files with 86 additions and 309 deletions.
45 changes: 19 additions & 26 deletions pkg/adaptation/adaptation.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,12 @@ import (
)

const (
// DefaultConfigPath is the default path to the NRI configuration.
DefaultConfigPath = "/etc/nri/nri.conf"
// DefaultPluginPath is the default path to search for NRI plugins.
DefaultPluginPath = "/opt/nri/plugins"
// DefaultSocketPath is the default socket path for external plugins.
DefaultSocketPath = api.DefaultSocketPath
// PluginConfigDir is the drop-in directory for NRI-launched plugin configuration.
DefaultPluginConfigPath = "/etc/nri/conf.d"
)

// SyncFn is a container runtime function for state synchronization.
Expand All @@ -54,12 +54,12 @@ type Adaptation struct {
sync.Mutex
name string
version string
configPath string
dropinPath string
pluginPath string
socketPath string
dontListen bool
syncFn SyncFn
updateFn UpdateFn
cfg *Config
listener net.Listener
plugins []*plugin
}
Expand All @@ -72,35 +72,34 @@ var (
// Option to apply to the NRI runtime.
type Option func(*Adaptation) error

// WithConfigPath returns an option to override the default NRI config path.
func WithConfigPath(path string) Option {
// WithPluginPath returns an option to override the default NRI plugin path.
func WithPluginPath(path string) Option {
return func(r *Adaptation) error {
r.configPath = path
r.pluginPath = path
return nil
}
}

// WithConfig returns an option to provide a pre-parsed NRI configuration.
func WithConfig(cfg *Config) Option {
// WithPluginConfigPath returns an option to override the default NRI plugin config path.
func WithPluginConfigPath(path string) Option {
return func(r *Adaptation) error {
r.cfg = cfg
r.configPath = cfg.path
r.dropinPath = path
return nil
}
}

// WithPluginPath returns an option to override the default NRI plugin path.
func WithPluginPath(path string) Option {
// WithSocketPath returns an option to override the default NRI socket path.
func WithSocketPath(path string) Option {
return func(r *Adaptation) error {
r.pluginPath = path
r.socketPath = path
return nil
}
}

// WithSocketPath returns an option to override the default NRI socket path.
func WithSocketPath(path string) Option {
// WithDisabledExternalConnections returns an options to disable accepting plugin connections.
func WithDisabledExternalConnections() Option {
return func(r *Adaptation) error {
r.socketPath = path
r.dontListen = true
return nil
}
}
Expand All @@ -121,8 +120,8 @@ func New(name, version string, syncFn SyncFn, updateFn UpdateFn, opts ...Option)
version: version,
syncFn: syncFn,
updateFn: updateFn,
configPath: DefaultConfigPath,
pluginPath: DefaultPluginPath,
dropinPath: DefaultPluginConfigPath,
socketPath: DefaultSocketPath,
}

Expand All @@ -132,12 +131,6 @@ func New(name, version string, syncFn SyncFn, updateFn UpdateFn, opts ...Option)
}
}

if r.cfg == nil {
if r.cfg, err = ReadConfig(r.configPath); err != nil {
return nil, err
}
}

log.Infof(noCtx, "runtime interface created")

return r, nil
Expand Down Expand Up @@ -374,7 +367,7 @@ func (r *Adaptation) removeClosedPlugins() {
}

func (r *Adaptation) startListener() error {
if r.cfg.DisableConnections {
if r.dontListen {
log.Infof(noCtx, "connection from external plugins disabled")
return nil
}
Expand Down Expand Up @@ -481,7 +474,7 @@ func (r *Adaptation) discoverPlugins() ([]string, []string, []string, error) {
r.pluginPath, err)
}

cfg, err := r.cfg.getPluginConfig(idx, base)
cfg, err := r.getPluginConfig(idx, base)
if err != nil {
return nil, nil, nil, fmt.Errorf("failed to discover plugins in %s: %w",
r.pluginPath, err)
Expand Down
116 changes: 19 additions & 97 deletions pkg/adaptation/adaptation_suite_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,27 +41,9 @@ var _ = Describe("Configuration", func() {
s.Cleanup()
})

When("invalid", func() {
var (
invalidConfig = "xyzzy gibberish foobar"
)

BeforeEach(func() {
s.Prepare(invalidConfig, &mockRuntime{})
})

It("should prevent startup with an error", func() {
Expect(s.runtime.Start(s.dir)).ToNot(Succeed())
})
})

When("empty", func() {
var (
emptyConfig = ""
)

When("no (extra) options given", func() {
BeforeEach(func() {
s.Prepare(emptyConfig, &mockRuntime{}, &mockPlugin{idx: "00", name: "test"})
s.Prepare(&mockRuntime{}, &mockPlugin{idx: "00", name: "test"})
})

It("should allow startup", func() {
Expand All @@ -81,12 +63,17 @@ var _ = Describe("Configuration", func() {
})

When("external connections are explicitly disabled", func() {
var (
config = "disableConnections: true"
)
var ()

BeforeEach(func() {
s.Prepare(config, &mockRuntime{}, &mockPlugin{idx: "00", name: "test"})
s.Prepare(
&mockRuntime{
options: []nri.Option{
nri.WithDisabledExternalConnections(),
},
},
&mockPlugin{idx: "00", name: "test"},
)
})

It("should prevent plugins from connecting", func() {
Expand All @@ -98,28 +85,6 @@ var _ = Describe("Configuration", func() {
Expect(plugin.Start(s.dir)).ToNot(Succeed())
})
})

When("external connections are explicitly enabled", func() {
var (
config = "disableConnections: false"
)

BeforeEach(func() {
s.Prepare(config, &mockRuntime{}, &mockPlugin{idx: "00", name: "test"})
})

It("should allow plugins to connect", func() {
var (
runtime = s.runtime
plugin = s.plugins[0]
timeout = time.After(startupTimeout)
)
Expect(runtime.Start(s.dir)).To(Succeed())
Expect(plugin.Start(s.dir)).To(Succeed())
Expect(plugin.Wait(PluginSynchronized, timeout)).To(Succeed())
})
})

})

var _ = Describe("Adaptation", func() {
Expand All @@ -135,15 +100,13 @@ var _ = Describe("Adaptation", func() {
var (
dir = GinkgoT().TempDir()
etc = filepath.Join(dir, "etc", "nri")
cfg = filepath.Join(etc, "nri.conf")
)

Expect(os.MkdirAll(etc, 0o755)).To(Succeed())
Expect(os.WriteFile(cfg, []byte(""), 0o644)).To(Succeed())

r, err := nri.New("mockRuntime", "0.0.1", syncFn, updateFn,
nri.WithConfigPath(filepath.Join(dir, "etc", "nri", "nri.conf")),
nri.WithPluginPath(filepath.Join(dir, "opt", "nri", "plugins")),
nri.WithPluginConfigPath(filepath.Join(dir, "etc", "nri", "conf.d")),
nri.WithSocketPath(filepath.Join(dir, "nri.sock")),
)

Expand All @@ -164,15 +127,13 @@ var _ = Describe("Adaptation", func() {
var (
dir = GinkgoT().TempDir()
etc = filepath.Join(dir, "etc", "nri")
cfg = filepath.Join(etc, "nri.conf")
)

Expect(os.MkdirAll(etc, 0o755)).To(Succeed())
Expect(os.WriteFile(cfg, []byte(""), 0o644)).To(Succeed())

r, err := nri.New("mockRuntime", "0.0.1", syncFn, updateFn,
nri.WithConfigPath(filepath.Join(dir, "etc", "nri", "nri.conf")),
nri.WithPluginPath(filepath.Join(dir, "opt", "nri", "plugins")),
nri.WithPluginConfigPath(filepath.Join(dir, "etc", "nri", "conf.d")),
nri.WithSocketPath(filepath.Join(dir, "nri.sock")),
)

Expand All @@ -188,11 +149,7 @@ var _ = Describe("Plugin connection", func() {
)

BeforeEach(func() {
var (
config = ""
)

s.Prepare(config,
s.Prepare(
&mockRuntime{
pods: map[string]*api.PodSandbox{
"pod0": {
Expand Down Expand Up @@ -285,11 +242,7 @@ var _ = Describe("Pod and container requests and events", func() {

When("there are no plugins", func() {
BeforeEach(func() {
var (
config = ""
)

s.Prepare(config, &mockRuntime{})
s.Prepare(&mockRuntime{})
})

It("should always succeed", func() {
Expand Down Expand Up @@ -330,11 +283,7 @@ var _ = Describe("Pod and container requests and events", func() {

When("when there are plugins", func() {
BeforeEach(func() {
var (
config = ""
)

s.Prepare(config, &mockRuntime{}, &mockPlugin{idx: "00", name: "test"})
s.Prepare(&mockRuntime{}, &mockPlugin{idx: "00", name: "test"})
})

DescribeTable("should honor plugins' event subscriptions",
Expand Down Expand Up @@ -405,12 +354,7 @@ var _ = Describe("Pod and container requests and events", func() {

When("when there are multiple plugins", func() {
BeforeEach(func() {
var (
config = ""
)

s.Prepare(
config,
&mockRuntime{},
&mockPlugin{idx: "20", name: "test"},
&mockPlugin{idx: "99", name: "foo"},
Expand Down Expand Up @@ -584,11 +528,7 @@ var _ = Describe("Plugin container creation adjustments", func() {

When("there is a single plugin", func() {
BeforeEach(func() {
var (
config = ""
)

s.Prepare(config, &mockRuntime{}, &mockPlugin{idx: "00", name: "test"})
s.Prepare(&mockRuntime{}, &mockPlugin{idx: "00", name: "test"})
})

DescribeTable("should be successfully collected without conflicts",
Expand Down Expand Up @@ -770,12 +710,7 @@ var _ = Describe("Plugin container creation adjustments", func() {

When("there are multiple plugins", func() {
BeforeEach(func() {
var (
config = ""
)

s.Prepare(
config,
&mockRuntime{},
&mockPlugin{idx: "10", name: "foo"},
&mockPlugin{idx: "00", name: "bar"},
Expand Down Expand Up @@ -956,11 +891,7 @@ var _ = Describe("Plugin container updates during creation", func() {

When("there is a single plugin", func() {
BeforeEach(func() {
var (
config = ""
)

s.Prepare(config, &mockRuntime{}, &mockPlugin{idx: "00", name: "test"})
s.Prepare(&mockRuntime{}, &mockPlugin{idx: "00", name: "test"})
})

DescribeTable("should be successfully collected without conflicts",
Expand Down Expand Up @@ -1110,12 +1041,7 @@ var _ = Describe("Plugin container updates during creation", func() {

When("there are multiple plugins", func() {
BeforeEach(func() {
var (
config = ""
)

s.Prepare(
config,
&mockRuntime{},
&mockPlugin{idx: "10", name: "foo"},
&mockPlugin{idx: "00", name: "bar"},
Expand Down Expand Up @@ -1287,11 +1213,7 @@ var _ = Describe("Unsolicited container update requests", func() {

When("when there are plugins", func() {
BeforeEach(func() {
var (
config = ""
)

s.Prepare(config, &mockRuntime{}, &mockPlugin{idx: "00", name: "test"})
s.Prepare(&mockRuntime{}, &mockPlugin{idx: "00", name: "test"})
})

It("should be delivered, without crash/panic", func() {
Expand Down
Loading

0 comments on commit cb3e20a

Please sign in to comment.