diff --git a/.changelog/23538.txt b/.changelog/23538.txt new file mode 100644 index 00000000000..0db0b224238 --- /dev/null +++ b/.changelog/23538.txt @@ -0,0 +1,3 @@ +```release-note:improvement +cni: allow users to input CNI args in job specification +``` diff --git a/api/resources.go b/api/resources.go index a6fe60d20eb..5e1adce500e 100644 --- a/api/resources.go +++ b/api/resources.go @@ -143,6 +143,9 @@ type DNSConfig struct { Searches []string `mapstructure:"searches" hcl:"searches,optional"` Options []string `mapstructure:"options" hcl:"options,optional"` } +type CNIConfig struct { + Args map[string]string `hcl:"args,optional"` +} // NetworkResource is used to describe required network // resources of a given task. @@ -160,7 +163,8 @@ type NetworkResource struct { // XXX Deprecated. Please do not use. The field will be removed in Nomad // 0.13 and is only being kept to allow any references to be removed before // then. - MBits *int `hcl:"mbits,optional"` + MBits *int `hcl:"mbits,optional"` + CNI *CNIConfig `hcl:"cni,block"` } // COMPAT(0.13) diff --git a/client/allocrunner/networking_cni.go b/client/allocrunner/networking_cni.go index ba967e92549..a15a336204f 100644 --- a/client/allocrunner/networking_cni.go +++ b/client/allocrunner/networking_cni.go @@ -102,6 +102,18 @@ const ( ConsulIPTablesConfigEnvVar = "CONSUL_IPTABLES_CONFIG" ) +// Adds user inputted custom CNI args to cniArgs map +func addCustomCNIArgs(networks []*structs.NetworkResource, cniArgs map[string]string) { + for _, net := range networks { + if net.CNI == nil { + continue + } + for k, v := range net.CNI.Args { + cniArgs[k] = v + } + } +} + // Setup calls the CNI plugins with the add action func (c *cniNetworkConfigurator) Setup(ctx context.Context, alloc *structs.Allocation, spec *drivers.NetworkIsolationSpec) (*structs.AllocNetworkStatus, error) { if err := c.ensureCNIInitialized(); err != nil { @@ -114,6 +126,10 @@ func (c *cniNetworkConfigurator) Setup(ctx context.Context, alloc *structs.Alloc "IgnoreUnknown": "true", } + tg := alloc.Job.LookupTaskGroup(alloc.TaskGroup) + + addCustomCNIArgs(tg.Networks, cniArgs) + portMaps := getPortMapping(alloc, c.ignorePortMappingHostIP) tproxyArgs, err := c.setupTransparentProxyArgs(alloc, spec, portMaps) diff --git a/client/allocrunner/networking_cni_test.go b/client/allocrunner/networking_cni_test.go index d58787d693a..31b4452aea3 100644 --- a/client/allocrunner/networking_cni_test.go +++ b/client/allocrunner/networking_cni_test.go @@ -205,6 +205,53 @@ func TestCNI_cniToAllocNet_Invalid(t *testing.T) { require.Nil(t, allocNet) } +func TestCNI_addCustomCNIArgs(t *testing.T) { + ci.Parallel(t) + cniArgs := map[string]string{ + "default": "yup", + } + + networkWithArgs := []*structs.NetworkResource{{ + CNI: &structs.CNIConfig{ + Args: map[string]string{ + "first_arg": "example", + "new_arg": "example_2", + }, + }, + }} + networkWithoutArgs := []*structs.NetworkResource{{ + Mode: "bridge", + }} + testCases := []struct { + name string + network []*structs.NetworkResource + expectMap map[string]string + }{ + { + name: "cni args not specified", + network: networkWithoutArgs, + expectMap: map[string]string{ + "default": "yup", + }, + }, { + name: "cni args specified", + network: networkWithArgs, + expectMap: map[string]string{ + "default": "yup", + "first_arg": "example", + "new_arg": "example_2", + }, + }, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + addCustomCNIArgs(tc.network, cniArgs) + test.Eq(t, tc.expectMap, cniArgs) + + }) + } +} + func TestCNI_setupTproxyArgs(t *testing.T) { ci.Parallel(t) diff --git a/client/taskenv/network.go b/client/taskenv/network.go index a59f96d3d91..51653855103 100644 --- a/client/taskenv/network.go +++ b/client/taskenv/network.go @@ -13,6 +13,7 @@ import ( // Current interoperable fields: // - Hostname // - DNS +// - CNI func InterpolateNetworks(taskEnv *TaskEnv, networks structs.Networks) structs.Networks { // Guard against not having a valid taskEnv. This can be the case if the @@ -32,6 +33,12 @@ func InterpolateNetworks(taskEnv *TaskEnv, networks structs.Networks) structs.Ne interpolated[i].DNS.Searches = taskEnv.ParseAndReplace(interpolated[i].DNS.Searches) interpolated[i].DNS.Options = taskEnv.ParseAndReplace(interpolated[i].DNS.Options) } + if interpolated[i].CNI != nil { + for k, v := range interpolated[i].CNI.Args { + interpolated[i].CNI.Args[k] = taskEnv.ReplaceEnv(v) + + } + } } return interpolated diff --git a/client/taskenv/network_test.go b/client/taskenv/network_test.go index a63fd5fef8f..5ff3016c1a1 100644 --- a/client/taskenv/network_test.go +++ b/client/taskenv/network_test.go @@ -84,6 +84,30 @@ func Test_InterpolateNetworks(t *testing.T) { }, name: "interpolated dns servers", }, + { + inputTaskEnv: testEnv, + inputNetworks: structs.Networks{ + { + CNI: &structs.CNIConfig{ + Args: map[string]string{ + "static": "example", + "second": "${foo}-opt", + }, + }, + }, + }, + expectedOutputNetworks: structs.Networks{ + { + CNI: &structs.CNIConfig{ + Args: map[string]string{ + "static": "example", + "second": "bar-opt", + }, + }, + }, + }, + name: "interpolated and non-interpolated cni args", + }, } for _, tc := range testCases { diff --git a/command/agent/job_endpoint.go b/command/agent/job_endpoint.go index b9de05a9157..465bc727275 100644 --- a/command/agent/job_endpoint.go +++ b/command/agent/job_endpoint.go @@ -1555,6 +1555,11 @@ func ApiNetworkResourceToStructs(in []*api.NetworkResource) []*structs.NetworkRe Options: nw.DNS.Options, } } + if nw.CNI != nil { + out[i].CNI = &structs.CNIConfig{ + Args: nw.CNI.Args, + } + } if l := len(nw.DynamicPorts); l != 0 { out[i].DynamicPorts = make([]structs.Port, l) diff --git a/nomad/structs/cni_config.go b/nomad/structs/cni_config.go new file mode 100644 index 00000000000..e821ebef13b --- /dev/null +++ b/nomad/structs/cni_config.go @@ -0,0 +1,32 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: BUSL-1.1 + +package structs + +import ( + "maps" +) + +type CNIConfig struct { + Args map[string]string +} + +func (d *CNIConfig) Copy() *CNIConfig { + if d == nil { + return nil + } + newMap := make(map[string]string) + for k, v := range d.Args { + newMap[k] = v + } + return &CNIConfig{ + Args: newMap, + } +} + +func (d *CNIConfig) Equal(o *CNIConfig) bool { + if d == nil || o == nil { + return d == o + } + return maps.Equal(d.Args, o.Args) +} diff --git a/nomad/structs/cni_config_test.go b/nomad/structs/cni_config_test.go new file mode 100644 index 00000000000..e7cf78b8819 --- /dev/null +++ b/nomad/structs/cni_config_test.go @@ -0,0 +1,28 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: BUSL-1.1 + +package structs + +import ( + "github.com/hashicorp/nomad/ci" + "github.com/shoenig/test/must" + "testing" +) + +func TestCNIConfig_Equal(t *testing.T) { + ci.Parallel(t) + + must.Equal[*CNIConfig](t, nil, nil) + must.NotEqual[*CNIConfig](t, nil, new(CNIConfig)) + must.NotEqual[*CNIConfig](t, nil, &CNIConfig{Args: map[string]string{"first": "second"}}) + + must.StructEqual(t, &CNIConfig{ + Args: map[string]string{ + "arg": "example_1", + "new_arg": "example_2", + }, + }, []must.Tweak[*CNIConfig]{{ + Field: "Args", + Apply: func(c *CNIConfig) { c.Args = map[string]string{"different": "arg"} }, + }}) +} diff --git a/nomad/structs/diff.go b/nomad/structs/diff.go index 5b75d8cb9bb..c00f150983d 100644 --- a/nomad/structs/diff.go +++ b/nomad/structs/diff.go @@ -2663,6 +2663,10 @@ func (n *NetworkResource) Diff(other *NetworkResource, contextual bool) *ObjectD diff.Objects = append(diff.Objects, dnsDiff) } + if cniDiff := n.CNI.Diff(other.CNI, contextual); cniDiff != nil { + diff.Objects = append(diff.Objects, cniDiff) + } + return diff } @@ -2706,6 +2710,21 @@ func (d *DNSConfig) Diff(other *DNSConfig, contextual bool) *ObjectDiff { return diff } +// Diff returns a diff of two CNIConfig structs +func (d *CNIConfig) Diff(other *CNIConfig, contextual bool) *ObjectDiff { + if d == nil { + d = &CNIConfig{} + } + if other == nil { + other = &CNIConfig{} + } + if d.Equal(other) { + return nil + } + + return primitiveObjectDiff(d.Args, other.Args, nil, "CNIConfig", contextual) +} + func disconectStrategyDiffs(old, new *DisconnectStrategy, contextual bool) *ObjectDiff { diff := &ObjectDiff{Type: DiffTypeNone, Name: "Disconnect"} var oldDisconnectFlat, newDisconnectFlat map[string]string diff --git a/nomad/structs/diff_test.go b/nomad/structs/diff_test.go index 15b095889d1..d24b4b6af9e 100644 --- a/nomad/structs/diff_test.go +++ b/nomad/structs/diff_test.go @@ -5221,6 +5221,251 @@ func TestTaskGroupDiff(t *testing.T) { }, }, }, + {TestCase: "TaskGroup CNI added", + Contextual: false, + Old: &TaskGroup{ + Networks: Networks{}, + }, + New: &TaskGroup{ + Networks: Networks{ + { + CNI: &CNIConfig{ + Args: map[string]string{ + "example": "example1", + }, + }, + }, + }, + }, + Expected: &TaskGroupDiff{ + Type: DiffTypeEdited, + Objects: []*ObjectDiff{ + { + Type: DiffTypeAdded, + Name: "Network", + Objects: []*ObjectDiff{ + + { + Type: DiffTypeAdded, + Name: "CNIConfig", + Fields: []*FieldDiff{ + { + Type: DiffTypeAdded, + Name: "example", + Old: "", + New: "example1", + }, + }, + }, + }, + }, + }, + }, + }, + {TestCase: "TaskGroup CNI deleted", + Contextual: false, + Old: &TaskGroup{ + Networks: Networks{ + { + CNI: &CNIConfig{ + Args: map[string]string{ + "example": "example1", + }, + }, + }, + }, + }, + New: &TaskGroup{ + Networks: Networks{}, + }, + Expected: &TaskGroupDiff{ + Type: DiffTypeEdited, + Objects: []*ObjectDiff{ + { + Type: DiffTypeDeleted, + Name: "Network", + Objects: []*ObjectDiff{ + + { + Type: DiffTypeDeleted, + Name: "CNIConfig", + Fields: []*FieldDiff{ + { + Type: DiffTypeDeleted, + Name: "example", + Old: "example1", + New: "", + }, + }, + }, + }, + }, + }, + }, + }, + {TestCase: "TaskGroup CNI edited", + Contextual: false, + Old: &TaskGroup{ + Networks: Networks{ + { + CNI: &CNIConfig{ + Args: map[string]string{ + "example": "example1", + }, + }, + }, + }, + }, + New: &TaskGroup{ + Networks: Networks{ + { + CNI: &CNIConfig{ + Args: map[string]string{ + "example": "example2", + }, + }, + }, + }, + }, + Expected: &TaskGroupDiff{ + Type: DiffTypeEdited, + Objects: []*ObjectDiff{ + { + Type: DiffTypeAdded, + Name: "Network", + Objects: []*ObjectDiff{ + + { + Type: DiffTypeAdded, + Name: "CNIConfig", + Fields: []*FieldDiff{ + { + Type: DiffTypeAdded, + Name: "example", + Old: "", + New: "example2", + }, + }, + }, + }, + }, + { + Type: DiffTypeDeleted, + Name: "Network", + Objects: []*ObjectDiff{ + + { + Type: DiffTypeDeleted, + Name: "CNIConfig", + Fields: []*FieldDiff{ + { + Type: DiffTypeDeleted, + Name: "example", + Old: "example1", + New: "", + }, + }, + }, + }, + }, + }, + }, + }, + {TestCase: "Editing Networks deletes and re-adds", + Contextual: false, + Old: &TaskGroup{ + Networks: Networks{ + { + DNS: &DNSConfig{ + Servers: []string{"1.1.1.1"}, + }, + }, + }, + }, + New: &TaskGroup{ + Networks: Networks{ + { + DNS: &DNSConfig{ + Servers: []string{"1.1.1.1"}, + }, + DynamicPorts: []Port{ + { + Label: "bar", + To: 8081, + HostNetwork: "public", + }, + }, + }, + }, + }, + Expected: &TaskGroupDiff{ + Type: DiffTypeEdited, + Objects: []*ObjectDiff{ + { + Type: DiffTypeAdded, + Name: "Network", + Objects: []*ObjectDiff{ + + { + Type: DiffTypeAdded, + Name: "Dynamic Port", + Fields: []*FieldDiff{ + { + Type: DiffTypeAdded, + Name: "HostNetwork", + Old: "", + New: "public", + }, + { + Type: DiffTypeAdded, + Name: "Label", + Old: "", + New: "bar", + }, + { + Type: DiffTypeAdded, + Name: "To", + Old: "", + New: "8081", + }, + }, + }, + { + Type: DiffTypeAdded, + Name: "DNS", + Fields: []*FieldDiff{ + { + Type: DiffTypeAdded, + Name: "Servers", + Old: "", + New: "1.1.1.1", + }, + }, + }, + }, + }, + { + Type: DiffTypeDeleted, + Name: "Network", + Objects: []*ObjectDiff{ + + { + Type: DiffTypeDeleted, + Name: "DNS", + Fields: []*FieldDiff{ + { + Type: DiffTypeDeleted, + Name: "Servers", + Old: "1.1.1.1", + New: "", + }, + }, + }, + }, + }, + }, + }, + }, } for i, c := range cases { diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index e0315654d1b..ebd81514fb6 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -2872,6 +2872,7 @@ type NetworkResource struct { DNS *DNSConfig // DNS Configuration ReservedPorts []Port // Host Reserved ports DynamicPorts []Port // Host Dynamically assigned ports + CNI *CNIConfig // CNIConfig Configuration } func (n *NetworkResource) Hash() uint32 { @@ -7147,6 +7148,7 @@ func (tg *TaskGroup) validateNetworks() error { portLabels := make(map[string]string) // host_network -> static port tracking staticPortsIndex := make(map[string]map[int]string) + cniArgKeys := set.New[string](len(tg.Networks)) for _, net := range tg.Networks { for _, port := range append(net.ReservedPorts, net.DynamicPorts...) { @@ -7186,6 +7188,26 @@ func (tg *TaskGroup) validateNetworks() error { mErr.Errors = append(mErr.Errors, err) } } + // Validate the cniArgs in each network resource. Make sure there are no duplicate Args in + // different network resources or illegal characters (;) in key or value ;) + if net.CNI != nil { + for k, v := range net.CNI.Args { + if cniArgKeys.Contains(k) { + err := fmt.Errorf("duplicate CNI arg %q", k) + mErr.Errors = append(mErr.Errors, err) + } else { + cniArgKeys.Insert(k) + } + if strings.Contains(k, ";") { + err := fmt.Errorf("invalid ';' character in CNI arg key %q", k) + mErr.Errors = append(mErr.Errors, err) + } + if strings.Contains(v, ";") { + err := fmt.Errorf("invalid ';' character in CNI arg value %q", v) + mErr.Errors = append(mErr.Errors, err) + } + } + } // Validate the hostname field to be a valid DNS name. If the parameter // looks like it includes an interpolation value, we skip this. It diff --git a/nomad/structs/structs_test.go b/nomad/structs/structs_test.go index 0c81555e78b..acc17ec1a18 100644 --- a/nomad/structs/structs_test.go +++ b/nomad/structs/structs_test.go @@ -2015,6 +2015,58 @@ func TestTaskGroupNetwork_Validate(t *testing.T) { }, ErrContains: "Hostname is not a valid DNS name", }, + { + TG: &TaskGroup{ + Name: "testing-duplicate-cni-arg-keys", + Networks: []*NetworkResource{ + { + CNI: &CNIConfig{Args: map[string]string{"static": "first_value"}}, + }, + { + CNI: &CNIConfig{Args: map[string]string{"static": "new_value"}}, + }, + }, + }, + ErrContains: "duplicate CNI arg", + }, + { + TG: &TaskGroup{ + Name: "testing-valid-cni-arg-keys", + Networks: []*NetworkResource{ + { + CNI: &CNIConfig{Args: map[string]string{"static": "first_value"}}, + }, + { + CNI: &CNIConfig{Args: map[string]string{"new_key": "new_value"}}, + }, + { + CNI: &CNIConfig{Args: map[string]string{"newest_key": "new_value", "second_key": "second_value"}}, + }, + }, + }, + }, + { + TG: &TaskGroup{ + Name: "testing-invalid-character-cni-arg-key", + Networks: []*NetworkResource{ + { + CNI: &CNIConfig{Args: map[string]string{"static;": "first_value"}}, + }, + }, + }, + ErrContains: "invalid ';' character in CNI arg key \"static;", + }, + { + TG: &TaskGroup{ + Name: "testing-invalid-character-cni-arg-value", + Networks: []*NetworkResource{ + { + CNI: &CNIConfig{Args: map[string]string{"static": "first_value;"}}, + }, + }, + }, + ErrContains: "invalid ';' character in CNI arg value \"first_value;", + }, } for i := range cases { @@ -2186,6 +2238,7 @@ func TestTask_Validate_Resources(t *testing.T) { HostNetwork: "loopback", }, }, + CNI: &CNIConfig{map[string]string{"static": "new_val"}}, }, }, }, @@ -2318,6 +2371,7 @@ func TestNetworkResource_Copy(t *testing.T) { HostNetwork: "public", }, }, + CNI: &CNIConfig{Args: map[string]string{"foo": "bar", "hello": "world"}}, }, name: "fully populated input check", }, diff --git a/website/content/docs/job-specification/network.mdx b/website/content/docs/job-specification/network.mdx index 67b3f9080e6..6003535308a 100644 --- a/website/content/docs/job-specification/network.mdx +++ b/website/content/docs/job-specification/network.mdx @@ -89,6 +89,8 @@ All other operating systems use the `host` networking mode. DNS configuration from the client host. DNS configuration is only supported on Linux clients at this time. Note that if you are using a `mode="cni/*`, these values will override any DNS configuration the CNI plugins return. +- `cni` ([CNIConfig](#cni-parameters): nil) - Sets the custom CNI + arguments for a network configuration per allocation, for use with `mode="cni/*`. ### `port` Parameters @@ -128,6 +130,14 @@ The label of the port is just text - it has no special meaning to Nomad. These parameters support [interpolation](/nomad/docs/runtime/interpolation). +## `cni` Parameters + +- `args` `(map: nil)` - Sets CNI arguments for network configuration. + These get turned into `CNI_ARGS` per the + [CNI spec](https://www.cni.dev/docs/spec/#parameters). + +These parameters support [interpolation](/nomad/docs/runtime/interpolation). + ## `network` Examples The following examples only show the `network` blocks. Remember that the @@ -294,6 +304,24 @@ network { The Nomad client will build the correct [capabilities arguments](https://github.com/containernetworking/cni/blob/v0.8.0/CONVENTIONS.md#well-known-capabilities) for the portmap plugin based on the defined port blocks. +### CNI Args + +The following example specifies CNI args for the custom CNI plugin specified above. + +```hcl +network { + mode = "cni/mynet" + port "http" { + to = 8080 + } + cni { + args = { + "nomad.region" : "${node.region}" + } + } +} +``` + ### Host Networks In some cases a port should only be allocated to a specific interface or address on the host.