refactor(transparent-proxy): adjust tproxy iptables API (#10185)

- Rename "commands" to "rules"

  Rules are more flexible if we want to introduce some additional
  logic for example to remove particular iptables rule or check
  if it exists. Before we would end up with a list of commands
  inside chains, which would make it harder if we in example would
  like to check if the rules we want to add/remove already exists,
  because we would have to iterate over all "commands" inside chains
  and create new `Command`s for check (-C|--check) or delete.
  
  We also won't need short/long parameters in rules anymore

- Get rid of rules.Append/Insert and add rules.NewRule

- Change rule position param type from `int` to `uint`

  iptables -I|--insert doesen't allow to pass negative numbers as
  rule positions

- Rename chain.Insert/Append/AppendIf methods

  Chains don't contain commands anymore. They contain rules.
  By changing names of these methods we give ourselves more
  flexibility about what we want to do with the rules inside chains.

- Put flag string literals in consts package for rules (-I|--insert etc.)

- Move regular chain and table names to consts package

- Pass table name to rules

  Table name might be used to generate command arguments for checking
  or removing rules.

- Change receiver name from "b" to "c" in Chain (b was incorrect)

- Make `Rule` parameter names concistent

  In other places we are using `chain` and `table` instead of
  `chainName` and `tableName`

- Rename Build methods to BuildForRestore

  `Build` is vague and `BuildForRestore` is more correct as returned
  strings are intended for `iptables-restore` usage only.

- Rename variable "cmds" to "lines" in Chain.BuildForRestore

- Change parameter builder's Build method to return []string instead of string

  We can use it then as a list of parameters for iptables execution

- Allow chain constructors to return and pass through errors

  You shouldn't be able to create a Chain without a name or without
  providing a table to which the chain belongs to.

- Rename "table" package/directory to "tables" for consistency

- Rename "chain" package/directory to "chains" for consistency

- Replace TableBuilder with Table interface

  Having this intermediate structure was cumbersome and unnecessary.
  I removed BuildForRestore method from TableBuilder and replaced it with
  the function BuildRulesForRestore which accepts Table interface. It
  makes it simpler in the future to introduce logic for cleanup and/or
  check of existing iptables rules.

Signed-off-by: Bart Smykla <bartek@smykla.com>

pkg/transparentproxy/config/config.go

@@ -138,28 +138,28 @@ type Config struct {
 
 // ShouldDropInvalidPackets is just a convenience function which can be used in
 // iptables conditional command generations instead of inlining anonymous functions
-// i.e. AppendIf(ShouldDropInvalidPackets, Match(...), Jump(Drop()))
+// i.e. AddRuleIf(ShouldDropInvalidPackets, Match(...), Jump(Drop()))
 func (c Config) ShouldDropInvalidPackets() bool {
 	return c.DropInvalidPackets
 }
 
 // ShouldRedirectDNS is just a convenience function which can be used in
 // iptables conditional command generations instead of inlining anonymous functions
-// i.e. AppendIf(ShouldRedirectDNS, Match(...), Jump(Drop()))
+// i.e. AddRuleIf(ShouldRedirectDNS, Match(...), Jump(Drop()))
 func (c Config) ShouldRedirectDNS() bool {
 	return c.Redirect.DNS.Enabled
 }
 
 // ShouldFallbackDNSToUpstreamChain is just a convenience function which can be used in
 // iptables conditional command generations instead of inlining anonymous functions
-// i.e. AppendIf(ShouldFallbackDNSToUpstreamChain, Match(...), Jump(Drop()))
+// i.e. AddRuleIf(ShouldFallbackDNSToUpstreamChain, Match(...), Jump(Drop()))
 func (c Config) ShouldFallbackDNSToUpstreamChain() bool {
 	return c.Redirect.DNS.UpstreamTargetChain != ""
 }
 
 // ShouldCaptureAllDNS is just a convenience function which can be used in
 // iptables conditional command generations instead of inlining anonymous functions
-// i.e. AppendIf(ShouldCaptureAllDNS, Match(...), Jump(Drop()))
+// i.e. AddRuleIf(ShouldCaptureAllDNS, Match(...), Jump(Drop()))
 func (c Config) ShouldCaptureAllDNS() bool {
 	return c.Redirect.DNS.CaptureAll
 }

pkg/transparentproxy/iptables/builder/builder.go

@@ -6,14 +6,15 @@ import (
 	"fmt"
 	"net"
 	"os"
+	"slices"
 	"strings"
 	"time"
 
 	"github.com/pkg/errors"
 	"github.com/vishvananda/netlink"
 
 	"github.com/kumahq/kuma/pkg/transparentproxy/config"
-	"github.com/kumahq/kuma/pkg/transparentproxy/iptables/table"
+	"github.com/kumahq/kuma/pkg/transparentproxy/iptables/tables"
 	"github.com/kumahq/kuma/pkg/util/pointer"
 )
 
@@ -23,15 +24,15 @@ const (
 )
 
 type IPTables struct {
-	raw    *table.RawTable
-	nat    *table.NatTable
-	mangle *table.MangleTable
+	raw    *tables.RawTable
+	nat    *tables.NatTable
+	mangle *tables.MangleTable
 }
 
 func newIPTables(
-	raw *table.RawTable,
-	nat *table.NatTable,
-	mangle *table.MangleTable,
+	raw *tables.RawTable,
+	nat *tables.NatTable,
+	mangle *tables.MangleTable,
 ) *IPTables {
 	return &IPTables{
 		raw:    raw,
@@ -40,33 +41,22 @@ func newIPTables(
 	}
 }
 
-func (t *IPTables) Build(verbose bool) string {
-	var tables []string
-
-	raw := t.raw.Build(verbose)
-	if raw != "" {
-		tables = append(tables, raw)
-	}
-
-	nat := t.nat.Build(verbose)
-	if nat != "" {
-		tables = append(tables, nat)
-	}
-
-	mangle := t.mangle.Build(verbose)
-	if mangle != "" {
-		tables = append(tables, mangle)
-	}
+func (t *IPTables) BuildForRestore(verbose bool) string {
+	result := slices.DeleteFunc([]string{
+		tables.BuildRulesForRestore(t.raw, verbose),
+		tables.BuildRulesForRestore(t.nat, verbose),
+		tables.BuildRulesForRestore(t.mangle, verbose),
+	}, func(s string) bool { return s == "" })
 
 	separator := "\n"
 	if verbose {
 		separator = "\n\n"
 	}
 
-	return strings.Join(tables, separator) + "\n"
+	return strings.Join(result, separator) + "\n"
 }
 
-func BuildIPTables(
+func BuildIPTablesForRestore(
 	cfg config.Config,
 	dnsServers []string,
 	ipv6 bool,
@@ -88,7 +78,7 @@ func BuildIPTables(
 		buildRawTable(cfg, dnsServers, iptablesExecutablePath),
 		natTable,
 		buildMangleTable(cfg),
-	).Build(cfg.Verbose), nil
+	).BuildForRestore(cfg.Verbose), nil
 }
 
 // runtimeOutput is the file (should be os.Stdout by default) where we can dump generated
@@ -205,7 +195,7 @@ func (r *restorer) tryRestoreIPTables(
 		r.cfg.Redirect.DNS.UpstreamTargetChain = "DOCKER_OUTPUT"
 	}
 
-	rules, err := BuildIPTables(r.cfg, r.dnsServers, r.ipv6, executables.Iptables.Path)
+	rules, err := BuildIPTablesForRestore(r.cfg, r.dnsServers, r.ipv6, executables.Iptables.Path)
 	if err != nil {
 		return "", fmt.Errorf("unable to build iptable rules: %s", err)
 	}

pkg/transparentproxy/iptables/builder/builder_table_mangle.go

@@ -4,14 +4,14 @@ import (
 	"github.com/kumahq/kuma/pkg/transparentproxy/config"
 	. "github.com/kumahq/kuma/pkg/transparentproxy/iptables/parameters"
 	. "github.com/kumahq/kuma/pkg/transparentproxy/iptables/parameters/match/conntrack"
-	"github.com/kumahq/kuma/pkg/transparentproxy/iptables/table"
+	"github.com/kumahq/kuma/pkg/transparentproxy/iptables/tables"
 )
 
-func buildMangleTable(cfg config.Config) *table.MangleTable {
-	mangle := table.Mangle()
+func buildMangleTable(cfg config.Config) *tables.MangleTable {
+	mangle := tables.Mangle()
 
 	mangle.Prerouting().
-		AppendIf(cfg.ShouldDropInvalidPackets,
+		AddRuleIf(cfg.ShouldDropInvalidPackets,
 			Match(Conntrack(Ctstate(INVALID))),
 			Jump(Drop()),
 		)