Do not generate nft chains for network policies if FilterTableName is…

… empty As the filter feature isn't always required and VyOS 1.4 does not have fitting nftables chains by default, it is useful that this feature can be disabled by letting FilterTableName empty.
cloudandheat · Jul 4, 2024 · b15a5db · b15a5db
1 parent 5a28338
commit b15a5db
Show file tree

Hide file tree

Showing 3 changed files with 115 additions and 21 deletions.
diff --git a/docs/agent/environments/vyos.md b/docs/agent/environments/vyos.md
@@ -4,7 +4,7 @@ This page describes the requirements for running the LBaaS-agent on a VyOS host.
 
 ## Requirements
 
-- VyOS 1.3 (1.4 probably also works with some config changes)
+- VyOS >= 1.3
 - L3-connection to the k8s-cluster
     - For example by establishing BGP peerings with the kubernetes nodes (see below)
 - At least one configured SNAT or DNAT rule via the VyOS configuration interface 
@@ -24,6 +24,7 @@ This page describes the requirements for running the LBaaS-agent on a VyOS host.
 ```toml
 bind-address="0.0.0.0"
 bind-port=15203
+shared-secret="changeme"
 
 [keepalived]
 enabled=false
@@ -47,8 +48,34 @@ status-command=["true"]
 start-command=["sudo", "nft", "-f", "/var/lib/ch-k8s-lbaas-agent/nftables/lbaas.conf"]
 ```
 
-__Warning:__ With VyOS 1.4, the names of the nftables hook changed.
+### VyOS >= 1.4
 
+```toml
+bind-address="0.0.0.0"
+bind-port=15203
+shared-secret="changeme"
+
+[keepalived]
+enabled=false
+
+[nftables]
+filter-table-name="" # "vyos_filter" in VyOS 1.4, but there are no hook-chains as in 1.3 -> Let empty to disable filtering
+filter-table-type="ip"
+filter-forward-chain="" # Does not exist by default in VyOS 1.4, must be created if wanted
+nat-table-name="vyos_nat"
+nat-prerouting-chain="VYOS_PRE_DNAT_HOOK"
+nat-postrouting-chain="VYOS_PRE_SNAT_HOOK"
+partial-reload=true
+policy-prefix="lbaas-"
+nft-command=["sudo","nft"]
+enable-snat=true
+
+[nftables.service]
+config-file="/var/lib/ch-k8s-lbaas-agent/nftables/lbaas.conf"
+reload-command=["sudo", "nft", "-f", "/var/lib/ch-k8s-lbaas-agent/nftables/lbaas.conf"]
+status-command=["true"]
+start-command=["sudo", "nft", "-f", "/var/lib/ch-k8s-lbaas-agent/nftables/lbaas.conf"]
+```
 
 ## BGP Configuration
 

diff --git a/internal/agent/nftables_generator.go b/internal/agent/nftables_generator.go
@@ -42,6 +42,8 @@ var (
 # When partial reload is enabled, flush chains.
 flush chain ip {{ .NATTableName }} {{ .NATPreroutingChainName }}
 flush chain ip {{ .NATTableName }} {{ .NATPostroutingChainName }}
+
+{{- if ne .FilterTableName "" }}
 flush chain {{ .FilterTableType }} {{ .FilterTableName }} {{ .FilterForwardChainName }}
 
 # Also delete all existing policy chains. 
@@ -53,7 +55,9 @@ add chain {{ $cfg.FilterTableType }} {{ $cfg.FilterTableName }} {{ $chain }}
 delete chain {{ $cfg.FilterTableType }} {{ $cfg.FilterTableName }} {{ $chain }}
 {{- end }}
 {{- end }}
+{{- end }}
 
+{{- if ne .FilterTableName "" }}
 table {{ .FilterTableType }} {{ .FilterTableName }} {
 	chain {{ .FilterForwardChainName }} {
 		{{- range $dest := $cfg.PolicyAssignments }}
@@ -103,6 +107,7 @@ table {{ .FilterTableType }} {{ .FilterTableName }} {
 
 	{{- end }}
 }
+{{- end }}
 
 table ip {{ .NATTableName }} {
 	chain {{ .NATPreroutingChainName }} {
@@ -489,22 +494,26 @@ func (g *NftablesGenerator) GenerateStructuredConfig(m *model.LoadBalancer) (*nf
 		return fwdA.InboundPort < fwdB.InboundPort
 	})
 
-	result.PolicyAssignments = copyPolicyAssignment(m.PolicyAssignments)
-	policies, err := copyNetworkPolicies(m.NetworkPolicies)
-	if err != nil {
-		return nil, err
-	}
-	for _, policy := range policies {
-		result.NetworkPolicies[policy.Name] = policy
-	}
+	if g.Cfg.FilterTableName != "" {
+		// When no filter table name is set, do not generate filter rules for network policies
+		result.PolicyAssignments = copyPolicyAssignment(m.PolicyAssignments)
+		policies, err := copyNetworkPolicies(m.NetworkPolicies)
+		if err != nil {
+			return nil, err
+		}
+		for _, policy := range policies {
+			result.NetworkPolicies[policy.Name] = policy
+		}
 
-	if g.Cfg.PartialReload {
-		// When partial reload is enabled, get all existing policy chain names to delete them in the template
-		result.ExistingPolicyChains, err = getExistingPolicyChains(
-			g.Cfg.NftCommand,
-			g.Cfg.FilterTableName,
-			g.Cfg.FilterTableType,
-			g.Cfg.PolicyPrefix)
+		if g.Cfg.PartialReload {
+			// When partial reload is enabled, get all existing policy chain names to delete
+			// them in the template
+			result.ExistingPolicyChains, err = getExistingPolicyChains(
+				g.Cfg.NftCommand,
+				g.Cfg.FilterTableName,
+				g.Cfg.FilterTableType,
+				g.Cfg.PolicyPrefix)
+		}
 	}
 
 	return result, nil

diff --git a/internal/agent/nftables_generator_test.go b/internal/agent/nftables_generator_test.go
@@ -27,16 +27,21 @@ import (
 	"github.com/cloudandheat/ch-k8s-lbaas/internal/model"
 )
 
-func newNftablesGenerator() *NftablesGenerator {
+func newNftablesGenerator(noFilterTable bool) *NftablesGenerator {
 	cfg := &config.Nftables{}
 	config.FillNftablesConfig(cfg)
+
+	if noFilterTable {
+		cfg.FilterTableName = ""
+	}
+
 	return &NftablesGenerator{
 		Cfg: *cfg,
 	}
 }
 
 func TestNftablesStructuredConfigFromEmptyLBModel(t *testing.T) {
-	g := newNftablesGenerator()
+	g := newNftablesGenerator(false)
 
 	m := &model.LoadBalancer{
 		Ingress: []model.IngressIP{},
@@ -58,7 +63,7 @@ func TestNftablesStructuredConfigFromEmptyLBModel(t *testing.T) {
 }
 
 func TestNftablesStructuredConfigFromNonEmptyLBModel(t *testing.T) {
-	g := newNftablesGenerator()
+	g := newNftablesGenerator(false)
 
 	m := &model.LoadBalancer{
 		Ingress: []model.IngressIP{
@@ -222,7 +227,7 @@ func TestNftablesStructuredConfigFromNonEmptyLBModel(t *testing.T) {
 }
 
 func TestNftablesStructuredConfigSortsAddresses(t *testing.T) {
-	g := newNftablesGenerator()
+	g := newNftablesGenerator(false)
 
 	m := &model.LoadBalancer{
 		Ingress: []model.IngressIP{
@@ -330,3 +335,56 @@ func TestFilterNftablesChainListByPrefix(t *testing.T) {
 
 	assert.Equal(t, []string{"Prefix-TestChain1"}, filteredChains)
 }
+
+func TestNftablesStructuredConfigWithDisabledPolicies(t *testing.T) {
+	g := newNftablesGenerator(true)
+
+	m := &model.LoadBalancer{
+		Ingress: []model.IngressIP{
+			{
+				Address: "172.23.42.1",
+				Ports: []model.PortForward{
+					{
+						InboundPort:          80,
+						Protocol:             corev1.ProtocolTCP,
+						DestinationPort:      30080,
+						DestinationAddresses: []string{"192.168.0.1", "192.168.0.2"},
+					},
+				},
+			},
+		},
+		NetworkPolicies: []model.NetworkPolicy{
+			{
+				Name: "block-range",
+				AllowedIngresses: []model.AllowedIngress{
+					{
+						IPBlockFilters: []model.IPBlockFilter{
+							{
+								Allow: "0.0.0.0/0",
+								Block: []string{
+									"192.168.2.0/24",
+								},
+							},
+						},
+					},
+				},
+			},
+		},
+		PolicyAssignments: []model.PolicyAssignment{
+			{
+				Address: "192.168.0.2",
+				NetworkPolicies: []string{
+					"block-range",
+				},
+			},
+		},
+	}
+
+	scfg, err := g.GenerateStructuredConfig(m)
+	assert.Nil(t, err)
+	assert.NotNil(t, scfg)
+	assert.NotNil(t, scfg.Forwards)
+	assert.Equal(t, 1, len(scfg.Forwards))
+	assert.NotNil(t, scfg.NetworkPolicies)
+	assert.Equal(t, 0, len(scfg.NetworkPolicies))
+}