From 47b0116633705901330a72908c2fbe1d1becb462 Mon Sep 17 00:00:00 2001 From: EC2 Default User Date: Thu, 8 Aug 2024 18:35:15 +0000 Subject: [PATCH] networking: refactor building nomad bridge config Co-authored-by: Martina Santangelo --- client/allocrunner/cni/bridge.go | 86 ++++++++++++++++++ client/allocrunner/cni/bridge_test.go | 31 +++++++ client/allocrunner/cni/plugins.go | 58 +++++++++++++ client/allocrunner/networking_bridge_linux.go | 87 +++++-------------- .../networking_bridge_linux_test.go | 19 ++-- .../test_fixtures/bad_input.conflist.json | 44 ++++++++++ .../test_fixtures/consul-cni.conflist.json | 48 ++++++++++ .../test_fixtures/empty.conflist.json | 44 ++++++++++ .../test_fixtures/hairpin.conflist.json | 44 ++++++++++ 9 files changed, 386 insertions(+), 75 deletions(-) create mode 100644 client/allocrunner/cni/bridge.go create mode 100644 client/allocrunner/cni/bridge_test.go create mode 100644 client/allocrunner/cni/plugins.go create mode 100644 client/allocrunner/test_fixtures/bad_input.conflist.json create mode 100644 client/allocrunner/test_fixtures/consul-cni.conflist.json create mode 100644 client/allocrunner/test_fixtures/empty.conflist.json create mode 100644 client/allocrunner/test_fixtures/hairpin.conflist.json diff --git a/client/allocrunner/cni/bridge.go b/client/allocrunner/cni/bridge.go new file mode 100644 index 000000000000..7554303c9a9c --- /dev/null +++ b/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, + } +} diff --git a/client/allocrunner/cni/bridge_test.go b/client/allocrunner/cni/bridge_test.go new file mode 100644 index 000000000000..2656e3ade392 --- /dev/null +++ b/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)) +} diff --git a/client/allocrunner/cni/plugins.go b/client/allocrunner/cni/plugins.go new file mode 100644 index 000000000000..091208ca3ba2 --- /dev/null +++ b/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"` +} diff --git a/client/allocrunner/networking_bridge_linux.go b/client/allocrunner/networking_bridge_linux.go index 13e78920c378..1d326e67ec8e 100644 --- a/client/allocrunner/networking_bridge_linux.go +++ b/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" - } -` diff --git a/client/allocrunner/networking_bridge_linux_test.go b/client/allocrunner/networking_bridge_linux_test.go index 82eaf497c465..16d9d4d903a5 100644 --- a/client/allocrunner/networking_bridge_linux_test.go +++ b/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") }) } } diff --git a/client/allocrunner/test_fixtures/bad_input.conflist.json b/client/allocrunner/test_fixtures/bad_input.conflist.json new file mode 100644 index 000000000000..f9c9be906ce9 --- /dev/null +++ b/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 + } + ] +} diff --git a/client/allocrunner/test_fixtures/consul-cni.conflist.json b/client/allocrunner/test_fixtures/consul-cni.conflist.json new file mode 100644 index 000000000000..3fe7b270edbf --- /dev/null +++ b/client/allocrunner/test_fixtures/consul-cni.conflist.json @@ -0,0 +1,48 @@ +{ + "cniVersion": "0.4.0", + "name": "nomad", + "plugins": [ + { + "type": "loopback" + }, + { + "type": "bridge", + "bridge": "nomad", + "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 + }, + { + "type": "consul-cni", + "log_level": "debug" + } + ] +} diff --git a/client/allocrunner/test_fixtures/empty.conflist.json b/client/allocrunner/test_fixtures/empty.conflist.json new file mode 100644 index 000000000000..484feb446744 --- /dev/null +++ b/client/allocrunner/test_fixtures/empty.conflist.json @@ -0,0 +1,44 @@ +{ + "cniVersion": "0.4.0", + "name": "nomad", + "plugins": [ + { + "type": "loopback" + }, + { + "type": "bridge", + "bridge": "", + "ipMasq": true, + "isGateway": true, + "forceAddress": true, + "hairpinMode": false, + "ipam": { + "type": "host-local", + "ranges": [ + [ + { + "subnet": "" + } + ] + ], + "routes": [ + { + "dst": "0.0.0.0/0" + } + ] + } + }, + { + "type": "firewall", + "backend": "iptables", + "iptablesAdminChainName": "NOMAD-ADMIN" + }, + { + "type": "portmap", + "capabilities": { + "portMappings": true + }, + "snat": true + } + ] +} diff --git a/client/allocrunner/test_fixtures/hairpin.conflist.json b/client/allocrunner/test_fixtures/hairpin.conflist.json new file mode 100644 index 000000000000..1e584c6c115f --- /dev/null +++ b/client/allocrunner/test_fixtures/hairpin.conflist.json @@ -0,0 +1,44 @@ +{ + "cniVersion": "0.4.0", + "name": "nomad", + "plugins": [ + { + "type": "loopback" + }, + { + "type": "bridge", + "bridge": "nomad", + "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 + } + ] +}