networking: refactor building nomad bridge config

Co-authored-by: Martina Santangelo <[email protected]>

client/allocrunner/cni/bridge.go

@@ -0,0 +1,86 @@
+// Copyright (c) HashiCorp, Inc.
+// SPDX-License-Identifier: BUSL-1.1
+
+package cni
+
+import "encoding/json"
+
+// Conflist is the .conflist format of CNI network config.
+type Conflist struct {
+	CniVersion string `json:"cniVersion"`
+	Name       string `json:"name"`
+	Plugins    []any  `json:"plugins"`
+}
+
+// Json produces indented json of the conflist.
+func (b Conflist) Json() ([]byte, error) {
+	return json.MarshalIndent(b, "", "\t")
+}
+
+// NomadBridgeConfig determines the contents of the Conflist.
+type NomadBridgeConfig struct {
+	BridgeName     string
+	AdminChainName string
+	IPv4Subnet     string
+	HairpinMode    bool
+	ConsulCNI      bool
+}
+
+// NewNomadBridgeConflist produces a full Conflist from the config.
+func NewNomadBridgeConflist(conf NomadBridgeConfig) Conflist {
+	// Update website/content/docs/networking/cni.mdx when the bridge config
+	// is modified. The json versions of the config can be found in
+	// client/allocrunner/test_fixtures/*.conflist.json
+	// If CNI plugins are added or versions need to be updated for new fields,
+	// add a new constraint to nomad/job_endpoint_hooks.go
+
+	ipRanges := [][]Range{
+		{{Subnet: conf.IPv4Subnet}},
+	}
+	ipRoutes := []Route{
+		{Dst: "0.0.0.0/0"},
+	}
+
+	plugins := []any{
+		Generic{
+			Type: "loopback",
+		},
+		Bridge{
+			Type:         "bridge",
+			Bridgename:   conf.BridgeName,
+			IpMasq:       true,
+			IsGateway:    true,
+			ForceAddress: true,
+			HairpinMode:  conf.HairpinMode,
+			Ipam: IPAM{
+				Type:   "host-local",
+				Ranges: ipRanges,
+				Routes: ipRoutes,
+			},
+		},
+		Firewall{
+			Type:           "firewall",
+			Backend:        "iptables",
+			AdminChainName: conf.AdminChainName,
+		},
+		Portmap{
+			Type: "portmap",
+			Capabilities: PortmapCapabilities{
+				Portmappings: true,
+			},
+			Snat: true,
+		},
+	}
+	if conf.ConsulCNI {
+		plugins = append(plugins, ConsulCNI{
+			Type:     "consul-cni",
+			LogLevel: "debug",
+		})
+	}
+
+	return Conflist{
+		CniVersion: "0.4.0",
+		Name:       "nomad",
+		Plugins:    plugins,
+	}
+}

client/allocrunner/cni/bridge_test.go

@@ -0,0 +1,31 @@
+// Copyright (c) HashiCorp, Inc.
+// SPDX-License-Identifier: BUSL-1.1
+
+package cni
+
+import (
+	"testing"
+
+	"github.com/shoenig/test/must"
+)
+
+func TestConflist_Json(t *testing.T) {
+	conf := &Conflist{
+		CniVersion: "0.0.1",
+		Name:       "test-config",
+		Plugins: []any{
+			Generic{Type: "test-plugin"},
+		},
+	}
+	bts, err := conf.Json()
+	must.NoError(t, err)
+	must.Eq(t, `{
+	"cniVersion": "0.0.1",
+	"name": "test-config",
+	"plugins": [
+		{
+			"type": "test-plugin"
+		}
+	]
+}`, string(bts))
+}

client/allocrunner/cni/plugins.go

@@ -0,0 +1,58 @@
+// Copyright (c) HashiCorp, Inc.
+// SPDX-License-Identifier: BUSL-1.1
+
+package cni
+
+// Generic has the one key that all plugins must have: "type"
+type Generic struct {
+	Type string `json:"type"`
+}
+
+// Bridge is the subset of options that we use to configure the "bridge" plugin.
+// https://www.cni.dev/plugins/current/main/bridge/
+type Bridge struct {
+	Type         string `json:"type"`
+	Bridgename   string `json:"bridge"`
+	IpMasq       bool   `json:"ipMasq"`
+	IsGateway    bool   `json:"isGateway"`
+	ForceAddress bool   `json:"forceAddress"`
+	HairpinMode  bool   `json:"hairpinMode"`
+	Ipam         IPAM   `json:"ipam"`
+}
+type IPAM struct {
+	Type   string    `json:"type"`
+	Ranges [][]Range `json:"ranges"`
+	Routes []Route   `json:"routes"`
+}
+type Range struct {
+	Subnet string `json:"subnet"`
+}
+type Route struct {
+	Dst string `json:"dst"`
+}
+
+// Firewall is the "firewall" plugin.
+// https://www.cni.dev/plugins/current/meta/firewall/
+type Firewall struct {
+	Type           string `json:"type"`
+	Backend        string `json:"backend"`
+	AdminChainName string `json:"iptablesAdminChainName"`
+}
+
+// Portmap is the "portmap" plugin.
+// https://www.cni.dev/plugins/current/meta/portmap/
+type Portmap struct {
+	Type         string              `json:"type"`
+	Capabilities PortmapCapabilities `json:"capabilities"`
+	Snat         bool                `json:"snat"`
+}
+type PortmapCapabilities struct {
+	Portmappings bool `json:"portMappings"`
+}
+
+// ConsulCNI is the "consul-cni" plugin used for transparent proxy.
+// https://github.com/hashicorp/consul-k8s/blob/main/control-plane/cni/main.go
+type ConsulCNI struct {
+	Type     string `json:"type"`
+	LogLevel string `json:"log_level"`
+}

client/allocrunner/networking_bridge_linux.go

@@ -9,6 +9,7 @@ import (
 
 	"github.com/coreos/go-iptables/iptables"
 	hclog "github.com/hashicorp/go-hclog"
+	"github.com/hashicorp/nomad/client/allocrunner/cni"
 	"github.com/hashicorp/nomad/nomad/structs"
 	"github.com/hashicorp/nomad/plugins/drivers"
 )
@@ -60,16 +61,24 @@ func newBridgeNetworkConfigurator(log hclog.Logger, alloc *structs.Allocation, b
 	}
 
 	var netCfg []byte
+	var err error
 
 	tg := alloc.Job.LookupTaskGroup(alloc.TaskGroup)
 	for _, svc := range tg.Services {
 		if svc.Connect.HasTransparentProxy() {
-			netCfg = buildNomadBridgeNetConfig(*b, true)
+			netCfg, err = buildNomadBridgeNetConfig(*b, true)
+			if err != nil {
+				return nil, err
+			}
+
 			break
 		}
 	}
 	if netCfg == nil {
-		netCfg = buildNomadBridgeNetConfig(*b, false)
+		netCfg, err = buildNomadBridgeNetConfig(*b, false)
+		if err != nil {
+			return nil, err
+		}
 	}
 
 	parser := &cniConfParser{
@@ -156,69 +165,13 @@ func (b *bridgeNetworkConfigurator) Teardown(ctx context.Context, alloc *structs
 	return b.cni.Teardown(ctx, alloc, spec)
 }
 
-func buildNomadBridgeNetConfig(b bridgeNetworkConfigurator, withConsulCNI bool) []byte {
-	var consulCNI string
-	if withConsulCNI {
-		consulCNI = consulCNIBlock
-	}
-
-	return []byte(fmt.Sprintf(nomadCNIConfigTemplate,
-		b.bridgeName,
-		b.hairpinMode,
-		b.allocSubnet,
-		cniAdminChainName,
-		consulCNI,
-	))
-}
-
-// Update website/content/docs/networking/cni.mdx when the bridge configuration
-// is modified. If CNI plugins are added or versions need to be updated for new
-// fields, add a new constraint to nomad/job_endpoint_hooks.go
-const nomadCNIConfigTemplate = `{
-	"cniVersion": "0.4.0",
-	"name": "nomad",
-	"plugins": [
-		{
-			"type": "loopback"
-		},
-		{
-			"type": "bridge",
-			"bridge": %q,
-			"ipMasq": true,
-			"isGateway": true,
-			"forceAddress": true,
-			"hairpinMode": %v,
-			"ipam": {
-				"type": "host-local",
-				"ranges": [
-					[
-						{
-							"subnet": %q
-						}
-					]
-				],
-				"routes": [
-					{ "dst": "0.0.0.0/0" }
-				]
-			}
-		},
-		{
-			"type": "firewall",
-			"backend": "iptables",
-			"iptablesAdminChainName": %q
-		},
-		{
-			"type": "portmap",
-			"capabilities": {"portMappings": true},
-			"snat": true
-		}%s
-	]
+func buildNomadBridgeNetConfig(b bridgeNetworkConfigurator, withConsulCNI bool) ([]byte, error) {
+	conf := cni.NewNomadBridgeConflist(cni.NomadBridgeConfig{
+		BridgeName:     b.bridgeName,
+		AdminChainName: cniAdminChainName,
+		IPv4Subnet:     b.allocSubnet,
+		HairpinMode:    b.hairpinMode,
+		ConsulCNI:      withConsulCNI,
+	})
+	return conf.Json()
 }
-`
-
-const consulCNIBlock = `,
-		{
-			"type": "consul-cni",
-			"log_level": "debug"
-		}
-`

client/allocrunner/networking_bridge_linux_test.go

@@ -5,6 +5,8 @@ package allocrunner
 
 import (
 	"encoding/json"
+	"os"
+	"path/filepath"
 	"testing"
 
 	"github.com/hashicorp/nomad/ci"
@@ -51,16 +53,17 @@ func Test_buildNomadBridgeNetConfig(t *testing.T) {
 	}
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
-			tc := tc
-			ci.Parallel(t)
-			bCfg := buildNomadBridgeNetConfig(*tc.b, tc.withConsulCNI)
+			bCfg, err := buildNomadBridgeNetConfig(*tc.b, tc.withConsulCNI)
+			must.NoError(t, err)
+
 			// Validate that the JSON created is rational
 			must.True(t, json.Valid(bCfg))
-			if tc.withConsulCNI {
-				must.StrContains(t, string(bCfg), "consul-cni")
-			} else {
-				must.StrNotContains(t, string(bCfg), "consul-cni")
-			}
+
+			// and that it matches golden expectations
+			goldenFile := filepath.Join("test_fixtures", tc.name+".conflist.json")
+			expect, err := os.ReadFile(goldenFile)
+			must.NoError(t, err)
+			must.Eq(t, string(expect), string(bCfg)+"\n")
 		})
 	}
 }

client/allocrunner/test_fixtures/bad_input.conflist.json

@@ -0,0 +1,44 @@
+{
+	"cniVersion": "0.4.0",
+	"name": "nomad",
+	"plugins": [
+		{
+			"type": "loopback"
+		},
+		{
+			"type": "bridge",
+			"bridge": "bad\"",
+			"ipMasq": true,
+			"isGateway": true,
+			"forceAddress": true,
+			"hairpinMode": true,
+			"ipam": {
+				"type": "host-local",
+				"ranges": [
+					[
+						{
+							"subnet": "172.26.64.0/20"
+						}
+					]
+				],
+				"routes": [
+					{
+						"dst": "0.0.0.0/0"
+					}
+				]
+			}
+		},
+		{
+			"type": "firewall",
+			"backend": "iptables",
+			"iptablesAdminChainName": "NOMAD-ADMIN"
+		},
+		{
+			"type": "portmap",
+			"capabilities": {
+				"portMappings": true
+			},
+			"snat": true
+		}
+	]
+}