Merge 0a34379 into backport/ticket-23830/definitely-current-crab

hashicorp · Nov 19, 2024 · f4216db · f4216db
2 parents 78592ce + 0a34379
commit f4216db
Show file tree

Hide file tree

Showing 4 changed files with 78 additions and 7 deletions.
diff --git a/.changelog/24319.txt b/.changelog/24319.txt
@@ -0,0 +1,3 @@
+```release-note:improvement
+cni: Add Nomad specific workload information to CNI_ARGS
+```
diff --git a/client/allocrunner/networking_cni.go b/client/allocrunner/networking_cni.go
@@ -28,6 +28,7 @@ import (
 	consulIPTables "github.com/hashicorp/consul/sdk/iptables"
 	log "github.com/hashicorp/go-hclog"
 	"github.com/hashicorp/go-set/v3"
+	"github.com/hashicorp/nomad/client/taskenv"
 	"github.com/hashicorp/nomad/helper"
 	"github.com/hashicorp/nomad/helper/envoy"
 	"github.com/hashicorp/nomad/nomad/structs"
@@ -117,6 +118,26 @@ func addCustomCNIArgs(networks []*structs.NetworkResource, cniArgs map[string]st
 	}
 }
 
+func addNomadWorkloadCNIArgs(logger log.Logger, alloc *structs.Allocation, cniArgs map[string]string) {
+	for key, value := range map[string]string{
+		// these are the very same keys that are used to build task env vars
+		taskenv.Region:    alloc.Job.Region, // NOMAD_REGION
+		taskenv.Namespace: alloc.Namespace,  // NOMAD_NAMESPACE
+		taskenv.JobID:     alloc.Job.ID,     // NOMAD_JOB_ID
+		taskenv.GroupName: alloc.TaskGroup,  // NOMAD_GROUP_NAME
+		taskenv.AllocID:   alloc.ID,         // NOMAD_ALLOC_ID
+	} {
+		// job ID and group name may contain ";" but CNI_ARGS are ";"-separated
+		// per the spec, so they may not be used in arg keys or values.
+		if strings.Contains(value, ";") {
+			logger.Warn("Skipping CNI arg because it contains a semicolon",
+				"key", key, "value", value)
+		} else {
+			cniArgs[key] = value
+		}
+	}
+}
+
 // 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 {
@@ -133,6 +154,9 @@ func (c *cniNetworkConfigurator) Setup(ctx context.Context, alloc *structs.Alloc
 
 	addCustomCNIArgs(tg.Networks, cniArgs)
 
+	// Add NOMAD_* after custom args so it cannot be overridden.
+	addNomadWorkloadCNIArgs(c.logger, alloc, 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
@@ -137,7 +137,12 @@ func TestSetup(t *testing.T) {
 				Address:       "99.99.99.99",
 			},
 			expectArgs: map[string]string{
-				"IgnoreUnknown": "true",
+				"IgnoreUnknown":    "true",
+				"NOMAD_ALLOC_ID":   "7cd08c6c-86c8-0bfa-f7ca-338466447711",
+				"NOMAD_GROUP_NAME": "web",
+				"NOMAD_JOB_ID":     "mock-service",
+				"NOMAD_NAMESPACE":  "default",
+				"NOMAD_REGION":     "global",
 			},
 		},
 		{
@@ -148,7 +153,12 @@ func TestSetup(t *testing.T) {
 				Address:       "99.99.99.99",
 			},
 			expectArgs: map[string]string{
-				"IgnoreUnknown": "true",
+				"IgnoreUnknown":    "true",
+				"NOMAD_ALLOC_ID":   "7cd08c6c-86c8-0bfa-f7ca-338466447711",
+				"NOMAD_GROUP_NAME": "web",
+				"NOMAD_JOB_ID":     "mock-service",
+				"NOMAD_NAMESPACE":  "default",
+				"NOMAD_REGION":     "global",
 			},
 		},
 		{
@@ -174,9 +184,31 @@ func TestSetup(t *testing.T) {
 				Address:       "99.99.99.99",
 			},
 			expectArgs: map[string]string{
-				"IgnoreUnknown": "true",
-				"first_arg":     "example",
-				"new_arg":       "example_2",
+				"IgnoreUnknown":    "true",
+				"first_arg":        "example",
+				"new_arg":          "example_2",
+				"NOMAD_ALLOC_ID":   "7cd08c6c-86c8-0bfa-f7ca-338466447711",
+				"NOMAD_GROUP_NAME": "web",
+				"NOMAD_JOB_ID":     "mock-service",
+				"NOMAD_NAMESPACE":  "default",
+				"NOMAD_REGION":     "global",
+			},
+		},
+		{
+			name: "cni workload with invalid job id and namespace",
+			modAlloc: func(a *structs.Allocation) {
+				a.Job.ID = "this;does;not;work"
+				a.Namespace = "no;chance"
+			},
+			expectResult: &structs.AllocNetworkStatus{
+				InterfaceName: "eth0",
+				Address:       "99.99.99.99",
+			},
+			expectArgs: map[string]string{
+				"IgnoreUnknown":    "true",
+				"NOMAD_ALLOC_ID":   "7cd08c6c-86c8-0bfa-f7ca-338466447711",
+				"NOMAD_GROUP_NAME": "web",
+				"NOMAD_REGION":     "global",
 			},
 		},
 		{
@@ -215,6 +247,11 @@ func TestSetup(t *testing.T) {
 				"IgnoreUnknown":          "true",
 				"extra_arg":              "example",
 				"CONSUL_IPTABLES_CONFIG": `{"ConsulDNSIP":"192.168.1.117","ConsulDNSPort":8600,"ProxyUserID":"101","ProxyInboundPort":9999,"ProxyOutboundPort":15001,"ExcludeInboundPorts":["9002"],"ExcludeOutboundPorts":null,"ExcludeOutboundCIDRs":null,"ExcludeUIDs":null,"NetNS":"/var/run/docker/netns/nonsense-ns","IptablesProvider":null}`,
+				"NOMAD_ALLOC_ID":         "7cd08c6c-86c8-0bfa-f7ca-338466447711",
+				"NOMAD_GROUP_NAME":       "web",
+				"NOMAD_JOB_ID":           "mock-service",
+				"NOMAD_NAMESPACE":        "default",
+				"NOMAD_REGION":           "global",
 			},
 		},
 	}
@@ -246,6 +283,8 @@ func TestSetup(t *testing.T) {
 			}
 
 			alloc := mock.ConnectAlloc()
+			alloc.ID = "7cd08c6c-86c8-0bfa-f7ca-338466447711" // Fix the ID for easier testing
+			alloc.Job.ID = "mock-service"                     // Fix the ID for easier testing
 			if tc.modAlloc != nil {
 				tc.modAlloc(alloc)
 			}

diff --git a/website/content/docs/networking/cni.mdx b/website/content/docs/networking/cni.mdx
@@ -250,8 +250,13 @@ To specify that a job should use a CNI network, set the task group's network
 that have fingerprinted a CNI configuration with the given name. For example, to
 use the configuration named `mynet`, you should set the task group's network
 mode to `cni/mynet`. Nodes that have a network configuration defining a network
-named `mynet` in their `cni_config_dir` are eligible to run the workload.
-
+named `mynet` in their `cni_config_dir` are eligible to run the workload. Nomad
+additionally supplies the following arguments via `CNI_ARGS` to the CNI network:
+`NOMAD_REGION`, `NOMAD_NAMESPACE`, `NOMAD_JOB_ID`, `NOMAD_GROUP_NAME`, and
+`NOMAD_ALLOC_ID`. Since the `CNI_ARGS` do not allow values to contain a semicolon
+Nomad will not set keys where the value contains a semicolon (this could happen
+with the job ID). CNI plugins utilizing `NOMAD_*` CNI arguments are advised to
+apply a defensive policy or simply error out.
 
 [cni-spec]: https://www.cni.dev/docs/spec/
 [cni-plugin-docs]: https://www.cni.dev/plugins/current/