From 5c2235103ca759f80ae24ce28862a02192ef633c Mon Sep 17 00:00:00 2001 From: Roberto Santalla Date: Wed, 8 Nov 2023 17:05:15 +0100 Subject: [PATCH 1/5] iptables: refactor helper, add RuleSet --- cmd/agent/commands/grpc.go | 2 +- cmd/agent/commands/http.go | 2 +- cmd/agent/commands/tcpdrop.go | 3 +- pkg/agent/tcpconn/disruptor.go | 9 +- pkg/iptables/iptables.go | 69 ++++++--- pkg/iptables/iptables_test.go | 14 +- pkg/iptables/redirector.go | 263 ++++++++++++++------------------ pkg/iptables/redirector_test.go | 28 ++-- 8 files changed, 191 insertions(+), 199 deletions(-) diff --git a/cmd/agent/commands/grpc.go b/cmd/agent/commands/grpc.go index 08d6afaa..d8e8d8dc 100644 --- a/cmd/agent/commands/grpc.go +++ b/cmd/agent/commands/grpc.go @@ -70,7 +70,7 @@ func BuildGrpcCmd(env runtime.Environment, config *agent.Config) *cobra.Command RedirectPort: port, // to the proxy port. } - redirector, err = iptables.NewTrafficRedirector(tr, env.Executor()) + redirector, err = iptables.NewTrafficRedirector(tr, iptables.New(env.Executor())) if err != nil { return err } diff --git a/cmd/agent/commands/http.go b/cmd/agent/commands/http.go index bd054e37..38b87972 100644 --- a/cmd/agent/commands/http.go +++ b/cmd/agent/commands/http.go @@ -69,7 +69,7 @@ func BuildHTTPCmd(env runtime.Environment, config *agent.Config) *cobra.Command RedirectPort: port, // to the proxy port. } - redirector, err = iptables.NewTrafficRedirector(tr, env.Executor()) + redirector, err = iptables.NewTrafficRedirector(tr, iptables.New(env.Executor())) if err != nil { return err } diff --git a/cmd/agent/commands/tcpdrop.go b/cmd/agent/commands/tcpdrop.go index 07bb722c..8f427a15 100644 --- a/cmd/agent/commands/tcpdrop.go +++ b/cmd/agent/commands/tcpdrop.go @@ -6,6 +6,7 @@ import ( "github.com/grafana/xk6-disruptor/pkg/agent" "github.com/grafana/xk6-disruptor/pkg/agent/tcpconn" + "github.com/grafana/xk6-disruptor/pkg/iptables" "github.com/grafana/xk6-disruptor/pkg/runtime" "github.com/spf13/cobra" ) @@ -38,7 +39,7 @@ func BuildTCPDropCmd(env runtime.Environment, config *agent.Config) *cobra.Comma } disruptor := tcpconn.Disruptor{ - Executor: env.Executor(), + Iptables: iptables.New(env.Executor()), Filter: filter, Dropper: dropper, } diff --git a/pkg/agent/tcpconn/disruptor.go b/pkg/agent/tcpconn/disruptor.go index b09e25a1..12a9d6e3 100644 --- a/pkg/agent/tcpconn/disruptor.go +++ b/pkg/agent/tcpconn/disruptor.go @@ -10,13 +10,12 @@ import ( "github.com/florianl/go-nfqueue" "github.com/grafana/xk6-disruptor/pkg/iptables" - "github.com/grafana/xk6-disruptor/pkg/runtime" ) // Disruptor applies TCP Connection disruptions by dropping connections according to a Dropper. A filter decides which // connections are considered for dropping. type Disruptor struct { - Executor runtime.Executor + Iptables iptables.Iptables Dropper Dropper Filter Filter } @@ -36,13 +35,13 @@ func (d Disruptor) Apply(ctx context.Context, duration time.Duration) error { return ErrDurationTooShort } - iptables := iptables.New(d.Executor) + ruleset := iptables.NewRuleSet(d.Iptables) //nolint:errcheck // Errors while removing rules are not actionable. - defer iptables.Remove() + defer ruleset.Remove() config := randomNFQConfig() for _, r := range d.rules(config) { - err := iptables.Add(r) + err := ruleset.Add(r) if err != nil { return err } diff --git a/pkg/iptables/iptables.go b/pkg/iptables/iptables.go index bad7683d..f0b731b6 100644 --- a/pkg/iptables/iptables.go +++ b/pkg/iptables/iptables.go @@ -1,3 +1,4 @@ +// Package iptables implements objects that manipulate netfilter rules by calling the iptables binary. package iptables import ( @@ -8,40 +9,79 @@ import ( ) // Iptables adds and removes iptables rules by executing the `iptables` binary. -// Add()ed rules are remembered and are automatically removed when Remove is called. type Iptables struct { // Executor is the runtime.Executor used to run the iptables binary. executor runtime.Executor - - rules []Rule } // New returns a new Iptables ready to use. -func New(executor runtime.Executor) *Iptables { - return &Iptables{ +func New(executor runtime.Executor) Iptables { + return Iptables{ executor: executor, } } -// Add adds a rule. Added rule will be remembered and removed later when Remove is called. -func (i *Iptables) Add(r Rule) error { +// Add appends a rule into the corresponding table and chain. +func (i Iptables) Add(r Rule) error { err := i.exec(r.add()) if err != nil { return err } + return nil +} + +// Remove removes an existing rule. If the rule does not exist, an error is returned. +func (i Iptables) Remove(r Rule) error { + err := i.exec(r.remove()) + if err != nil { + return err + } + + return nil +} + +func (i Iptables) exec(args string) error { + out, err := i.executor.Exec("iptables", strings.Split(args, " ")...) + if err != nil { + return fmt.Errorf("%w: %q", err, out) + } + + return nil +} + +// RuleSet is a stateful object that allows adding rules and keeping track of them to remove them later. +type RuleSet struct { + iptables Iptables + rules []Rule +} + +// NewRuleSet builds a RuleSet that uses the provided Iptables instance to add and remove rules. +func NewRuleSet(iptables Iptables) *RuleSet { + return &RuleSet{ + iptables: iptables, + } +} + +// Add adds a rule. Added rule will be remembered and removed later together with other rules when Remove is called. +func (i *RuleSet) Add(r Rule) error { + err := i.iptables.Add(r) + if err != nil { + return err + } + i.rules = append(i.rules, r) return nil } // Remove removes all added rules. If an error occurs, Remove continues to try and remove remaining rules. -func (i *Iptables) Remove() error { +func (i *RuleSet) Remove() error { var errors []error var remaining []Rule for _, rule := range i.rules { - err := i.exec(rule.remove()) + err := i.iptables.Remove(rule) if err != nil { errors = append(errors, err) remaining = append(remaining, rule) @@ -58,15 +98,6 @@ func (i *Iptables) Remove() error { return nil } -func (i *Iptables) exec(args string) error { - out, err := i.executor.Exec("iptables", strings.Split(args, " ")...) - if err != nil { - return fmt.Errorf("%w: %q", err, out) - } - - return nil -} - // Rule is a netfilter/iptables rule. type Rule struct { // Table is the netfilter table to which this rule belongs. It is usually "filter". @@ -74,6 +105,8 @@ type Rule struct { // Chain is the netfilter chain to which this rule belongs. Usual values are "INPUT", "OUTPUT". Chain string // Args is the rest of the netfilter rule. + // Arguments must be space-separated. Using shell-style quotes or backslashes to group more than one space-separated + // word as one argument is not allowed. Args string } diff --git a/pkg/iptables/iptables_test.go b/pkg/iptables/iptables_test.go index 36f8eb80..3ed986bf 100644 --- a/pkg/iptables/iptables_test.go +++ b/pkg/iptables/iptables_test.go @@ -8,21 +8,21 @@ import ( "github.com/grafana/xk6-disruptor/pkg/runtime" ) -func Test_IptablesAddsRemovesRules(t *testing.T) { +func Test_RulesetAddsRemovesRules(t *testing.T) { t.Parallel() exec := runtime.NewFakeExecutor(nil, nil) - ipt := iptables.New(exec) + ruleset := iptables.NewRuleSet(iptables.New(exec)) // Add two rules - err := ipt.Add(iptables.Rule{ + err := ruleset.Add(iptables.Rule{ Table: "table1", Chain: "CHAIN1", Args: "--foo foo --bar bar", }) if err != nil { t.Fatalf("error adding rule: %v", err) } - err = ipt.Add(iptables.Rule{ + err = ruleset.Add(iptables.Rule{ Table: "table2", Chain: "CHAIN2", Args: "--boo boo --baz baz", }) if err != nil { @@ -42,7 +42,7 @@ func Test_IptablesAddsRemovesRules(t *testing.T) { exec.Reset() // Remove the rules. - err = ipt.Remove() + err = ruleset.Remove() if err != nil { t.Fatalf("error removing rules: %v", err) } @@ -60,7 +60,7 @@ func Test_IptablesAddsRemovesRules(t *testing.T) { exec.Reset() // After removing the rules, add a new one. - err = ipt.Add(iptables.Rule{ + err = ruleset.Add(iptables.Rule{ Table: "table3", Chain: "CHAIN3", Args: "--zoo zoo --zap zap", }) if err != nil { @@ -78,7 +78,7 @@ func Test_IptablesAddsRemovesRules(t *testing.T) { exec.Reset() // Remove the rule. - err = ipt.Remove() + err = ruleset.Remove() if err != nil { t.Fatalf("error removing rules: %v", err) } diff --git a/pkg/iptables/redirector.go b/pkg/iptables/redirector.go index 5bb72372..9d067742 100644 --- a/pkg/iptables/redirector.go +++ b/pkg/iptables/redirector.go @@ -4,88 +4,9 @@ package iptables import ( - "errors" "fmt" - "strings" - - "github.com/grafana/xk6-disruptor/pkg/agent/protocol" - "github.com/grafana/xk6-disruptor/pkg/runtime" ) -// The four rules defined in the constants below achieve the following purposes: -// - Redirect traffic to the target application through the proxy, excluding traffic from the proxy itself. -// - Reset existing, non-redirected connections to the target application, except those of the proxy itself. -// Excluding traffic from the proxy from the goals above is not entirely straightforward, mainly because the proxy, -// just like `kubectl port-forward` and sidecars, connect _from_ the loopback address 127.0.0.1. -// -// To achieve this, we take advantage of the fact that the proxy knows the pod IP and connects to it, instead of to the -// loopback address like sidecars and kubectl port-forward does. This allows us to distinguish the proxy traffic from -// port-forwarded traffic, as while both traverse the `lo` interface, the former targets the pod IP while the latter -// targets the loopback IP. -// - -// +-----------+---------------+------------------------+ -// | Interface | From/To | What | -// +-----------+---------------+------------------------+ -// | ! lo | Anywhere | Outside traffic | -// +-----------+---------------+------------------------+ -// | lo | 127.0.0.0/8 | Port-forwarded traffic | -// +-----------+---------------+------------------------+ -// | lo | ! 127.0.0.0/8 | Proxy traffic | -// +-----------+---------------+------------------------+ - -// redirectLocalRule is a netfilter rule that intercepts locally-originated traffic, such as that coming from sidecars -// or `kubectl port-forward, directed to the application and redirects it to the proxy. -// As per https://upload.wikimedia.org/wikipedia/commons/3/37/Netfilter-packet-flow.svg, locally originated traffic -// traverses OUTPUT instead of PREROUTING. -// Traffic created by the proxy itself to the application also traverses this chain, but is not redirected by this rule -// as the proxy targets the pod IP and not the loopback address. -const redirectLocalRule = "OUTPUT " + // For local traffic - "-t nat " + // Traversing the nat table - "-s 127.0.0.0/8 -d 127.0.0.1/32 " + // Coming from and directed to localhost, i.e. not the pod IP. - //nolint:goconst - "-p tcp --dport %d " + // Sent to the upstream application's port - "-j REDIRECT --to-port %d" // Forward it to the proxy address - -// redirectExternalRule is a netfilter rule that intercepts external traffic directed to the application and redirects -// it to the proxy. -// Traffic created by the proxy itself to the application traverses is not redirected by this rule as it traverses the -// OUTPUT chain, not PREROUTING. -const redirectExternalRule = "PREROUTING " + // For remote traffic - "-t nat " + // Traversing the nat table - "! -i lo " + // Not coming form loopback. This is technically not needed, but doesn't hurt and helps readability. - "-p tcp --dport %d " + // Sent to the upstream application's port - "-j REDIRECT --to-port %d" // Forward it to the proxy address - -// resetLocalRule is a netfilter rule that resets established connections (i.e. that have not been redirected) coming -// to and from the loopback address. -// This rule matches connections from sidecars and `kubectl port-forward`. -// Connections from the proxy itself do not match this rule, as although they flow through `lo`, they are directed to -// the pod's external IP and not the loopback address. -const resetLocalRule = "INPUT " + // For traffic traversing the INPUT chain - "-i lo " + // On the loopback interface - "-s 127.0.0.0/8 -d 127.0.0.1/32 " + // Coming from and directed to localhost - "-p tcp --dport %d " + // Directed to the upstream application's port - "-m state --state ESTABLISHED " + // That are already ESTABLISHED, i.e. not before they are redirected - "-j REJECT --reject-with tcp-reset" // Reject it - -// resetExternalRule is a netfilter rule that resets established connections (i.e. that have not been redirected) coming -// from anywhere except the local IP. -// This rule matches external connections to the pod's IP address. -// Connections from the proxy itself do not match this rule, as they flow through the `lo` interface. -const resetExternalRule = "INPUT " + // For traffic traversing the INPUT chain - "! -i lo " + // Not coming form loopback. This is technically not needed, but doesn't hurt and helps readability. - "-p tcp --dport %d " + // Directed to the upstream application's port - "-m state --state ESTABLISHED " + // That are already ESTABLISHED, i.e. not before they are redirected - "-j REJECT --reject-with tcp-reset" // Reject it - -// proxyResetRule is a netfilter rule that rejects traffic to the proxy. -// This rule is set up after injection finishes to kill any leftover connection to the proxy. -// TODO: Run some tests to check if this is really necessary, as the proxy may already be killing conns on termination. -const resetProxyRule = "INPUT " + // Traffic flowing through the INPUT chain - "-p tcp --dport %d " + // Directed to the proxy port - "-j REJECT --reject-with tcp-reset" // Reject it - // TrafficRedirectionSpec specifies the redirection of traffic to a destination type TrafficRedirectionSpec struct { // DestinationPort is the original destination port where the upstream application listens. @@ -95,17 +16,17 @@ type TrafficRedirectionSpec struct { RedirectPort uint } -// trafficRedirect defines an instance of a TrafficRedirector -type redirector struct { +// Redirector is an implementation of TrafficRedirector that uses iptables rules. +type Redirector struct { *TrafficRedirectionSpec - executor runtime.Executor + iptables Iptables } // NewTrafficRedirector creates instances of an iptables traffic redirector func NewTrafficRedirector( tr *TrafficRedirectionSpec, - executor runtime.Executor, -) (protocol.TrafficRedirector, error) { + iptables Iptables, +) (*Redirector, error) { if tr.DestinationPort == 0 || tr.RedirectPort == 0 { return nil, fmt.Errorf("DestinationPort and RedirectPort must be specified") } @@ -118,71 +39,119 @@ func NewTrafficRedirector( ) } - return &redirector{ + return &Redirector{ TrafficRedirectionSpec: tr, - executor: executor, + iptables: iptables, }, nil } -func (tr *redirector) redirectRules() []string { - return []string{ - fmt.Sprintf( - redirectLocalRule, - tr.DestinationPort, - tr.RedirectPort, - ), - fmt.Sprintf( - redirectExternalRule, - tr.DestinationPort, - tr.RedirectPort, - ), +// rules returns the iptables rules that cause traffic to be forwarded according to the spec. +// The four returned rules fulfill two different purposes. +// - Redirect traffic to the target application through the proxy, excluding traffic from the proxy itself. +// - Reset existing, non-redirected connections to the target application, except those of the proxy itself. +// Excluding traffic from the proxy from the goals above is not entirely straightforward, mainly because the proxy, +// just like `kubectl port-forward` and sidecars, connect _from_ the loopback address 127.0.0.1. +// +// To achieve this, we take advantage of the fact that the proxy knows the pod IP and connects to it, instead of to the +// loopback address like sidecars and kubectl port-forward does. This allows us to distinguish the proxy traffic from +// port-forwarded traffic, as while both traverse the `lo` interface, the former targets the pod IP while the latter +// targets the loopback IP. +// +// +-----------+---------------+------------------------+ +// | Interface | From/To | What | +// +-----------+---------------+------------------------+ +// | ! lo | Anywhere | Outside traffic | +// +-----------+---------------+------------------------+ +// | lo | 127.0.0.0/8 | Port-forwarded traffic | +// +-----------+---------------+------------------------+ +// | lo | ! 127.0.0.0/8 | Proxy traffic | +// +-----------+---------------+------------------------+ +func (tr *Redirector) rules() []Rule { + // redirectLocalRule is a netfilter rule that intercepts locally-originated traffic, such as that coming from sidecars + // or `kubectl port-forward, directed to the application and redirects it to the proxy. + // As per https://upload.wikimedia.org/wikipedia/commons/3/37/Netfilter-packet-flow.svg, locally originated traffic + // traverses OUTPUT instead of PREROUTING. + // Traffic created by the proxy itself to the application also traverses this chain, but is not redirected by this rule + // as the proxy targets the pod IP and not the loopback address. + redirectLocalRule := Rule{ + Table: "nat", + Chain: "OUTPUT", // For local traffic + Args: "-s 127.0.0.0/8 -d 127.0.0.1/32 " + // Coming from and directed to localhost, i.e. not the pod IP. + fmt.Sprintf("-p tcp --dport %d ", tr.DestinationPort) + // Sent to the upstream application's port + fmt.Sprintf("-j REDIRECT --to-port %d", tr.RedirectPort), // Forward it to the proxy address } -} -func (tr *redirector) resetRules() []string { - return []string{ - fmt.Sprintf( - resetLocalRule, - tr.DestinationPort, - ), - fmt.Sprintf( - resetExternalRule, - tr.DestinationPort, - ), + // redirectExternalRule is a netfilter rule that intercepts external traffic directed to the application and redirects + // it to the proxy. + // Traffic created by the proxy itself to the application traverses is not redirected by this rule as it traverses the + // OUTPUT chain, not PREROUTING. + redirectExternalRule := Rule{ + Table: "nat", + Chain: "PREROUTING", // For remote traffic + Args: "! -i lo " + // Not coming form loopback. Technically not needed, but doesn't hurt and helps readability. + fmt.Sprintf("-p tcp --dport %d ", tr.DestinationPort) + // Sent to the upstream application's port + fmt.Sprintf("-j REDIRECT --to-port %d", tr.RedirectPort), // Forward it to the proxy address } -} -func (tr *redirector) resetProxyRule() string { - return fmt.Sprintf(resetProxyRule, tr.RedirectPort) -} + // resetLocalRule is a netfilter rule that resets established connections (i.e. that have not been redirected) coming + // to and from the loopback address. + // This rule matches connections from sidecars and `kubectl port-forward`. + // Connections from the proxy itself do not match this rule, as although they flow through `lo`, they are directed to + // the pod's external IP and not the loopback address. + resetLocalRule := Rule{ + Table: "filter", + Chain: "INPUT", // For traffic traversing the INPUT chain + Args: "-i lo " + // On the loopback interface + "-s 127.0.0.0/8 -d 127.0.0.1/32 " + // Coming from and directed to localhost + fmt.Sprintf("-p tcp --dport %d ", tr.DestinationPort) + // Directed to the upstream application's port + "-m state --state ESTABLISHED " + // That are already ESTABLISHED, i.e. not before they are redirected + "-j REJECT --reject-with tcp-reset", // Reject it + } -// execIptables runs performs the specified action ("-A" or "-D") for the supplied rule. -func (tr *redirector) execIptables(action string, rule string) error { - cmd := fmt.Sprintf("%s %s", action, rule) - out, err := tr.executor.Exec("iptables", strings.Split(cmd, " ")...) - if err != nil { - return fmt.Errorf("error executing iptables command %q: %w %s", cmd, err, string(out)) + // resetExternalRule is a netfilter rule that resets established connections (i.e. that have not been redirected) + // coming from anywhere except the local IP. + // This rule matches external connections to the pod's IP address. + // Connections from the proxy itself do not match this rule, as they flow through the `lo` interface. + resetExternalRule := Rule{ + Table: "filter", + Chain: "INPUT", // For traffic traversing the INPUT chain + Args: "! -i lo " + // Not coming form loopback. This is technically not needed as loopback traffic does not + // traverse INPUT, but helps with explicitness. + fmt.Sprintf("-p tcp --dport %d ", tr.DestinationPort) + // Directed to the upstream application's port + "-m state --state ESTABLISHED " + // That are already ESTABLISHED, i.e. not before they are redirected + "-j REJECT --reject-with tcp-reset", // Reject it } - return nil + return []Rule{ + redirectLocalRule, + redirectExternalRule, + resetLocalRule, + resetExternalRule, + } +} + +// proxyResetRule returns a netfilter rule that rejects traffic to the proxy. +// This rule is set up after injection finishes to kill any leftover connection to the proxy. +// TODO: Run some tests to check if this is really necessary, as the proxy may already be killing conns on termination. +func (tr *Redirector) resetProxyRule() Rule { + return Rule{ + Table: "filter", + Chain: "INPUT", + Args: fmt.Sprintf("-p tcp --dport %d ", tr.RedirectPort) + // Directed to the proxy port + "-j REJECT --reject-with tcp-reset", // Reject it + } } // Start applies the TrafficRedirect -func (tr *redirector) Start() error { +func (tr *Redirector) Start() error { // Remove reset rule for the proxy in case it exists from a previous run. - _ = tr.execIptables("-D", tr.resetProxyRule()) + _ = tr.iptables.Remove(tr.resetProxyRule()) - for _, rule := range tr.redirectRules() { - err := tr.execIptables("-A", rule) + // TODO: Use RuleSet instead, which takes care of automatically cleaning the rules. + for _, rule := range tr.rules() { + err := tr.iptables.Add(rule) if err != nil { - return err - } - } - - for _, rule := range tr.resetRules() { - err := tr.execIptables("-A", rule) - if err != nil { - return err + return fmt.Errorf("adding rules: %w", err) } } @@ -191,33 +160,23 @@ func (tr *redirector) Start() error { // Stop stops the TrafficRedirect. // Stop will continue attempting to remove all the rules it deployed even if removing one fails. -// TODO: The error returned does not wrap original errors. -func (tr *redirector) Stop() error { - var errs []string - - // TODO: Replace this homemade error aggregation with errors.Join when we upgrade from Go 1.19 to 1.20. - for _, rule := range tr.redirectRules() { - err := tr.execIptables("-D", rule) - if err != nil { - errs = append(errs, err.Error()) - } - } +func (tr *Redirector) Stop() error { + var errors []error - for _, rule := range tr.resetRules() { - err := tr.execIptables("-D", rule) + for _, rule := range tr.rules() { + err := tr.iptables.Remove(rule) if err != nil { - errs = append(errs, err.Error()) + errors = append(errors, err) } } - // Add rule to terminate any remaining traffic directed to the proxy. - err := tr.execIptables("-A", tr.resetProxyRule()) - if err != nil { - errs = append(errs, err.Error()) + if err := tr.iptables.Add(tr.resetProxyRule()); err != nil { + errors = append(errors, err) } - if len(errs) > 0 { - return errors.New(strings.Join(errs, "; ")) + // TODO: Replace with errors.Join. + if len(errors) > 0 { + return errors[0] } return nil diff --git a/pkg/iptables/redirector_test.go b/pkg/iptables/redirector_test.go index e02ae753..de28a404 100644 --- a/pkg/iptables/redirector_test.go +++ b/pkg/iptables/redirector_test.go @@ -49,7 +49,7 @@ func Test_validateTrafficRedirect(t *testing.T) { executor := runtime.NewFakeExecutor(nil, nil) _, err := NewTrafficRedirector( &tc.redirect, - executor, + New(executor), ) if tc.expectError && err == nil { t.Errorf("error expected but none returned") @@ -83,13 +83,13 @@ func Test_Commands(t *testing.T) { testFunction: func(tr protocol.TrafficRedirector) error { return tr.Start() }, + //nolint:lll expectedCmds: []string{ - "iptables -D INPUT -p tcp --dport 8080 -j REJECT --reject-with tcp-reset", - "iptables -A OUTPUT -t nat -s 127.0.0.0/8 -d 127.0.0.1/32 -p tcp --dport 80 -j REDIRECT --to-port 8080", - "iptables -A PREROUTING -t nat ! -i lo -p tcp --dport 80 -j REDIRECT --to-port 8080", - //nolint:lll - "iptables -A INPUT -i lo -s 127.0.0.0/8 -d 127.0.0.1/32 -p tcp --dport 80 -m state --state ESTABLISHED -j REJECT --reject-with tcp-reset", - "iptables -A INPUT ! -i lo -p tcp --dport 80 -m state --state ESTABLISHED -j REJECT --reject-with tcp-reset", + "iptables -t filter -D INPUT -p tcp --dport 8080 -j REJECT --reject-with tcp-reset", + "iptables -t nat -A OUTPUT -s 127.0.0.0/8 -d 127.0.0.1/32 -p tcp --dport 80 -j REDIRECT --to-port 8080", + "iptables -t nat -A PREROUTING ! -i lo -p tcp --dport 80 -j REDIRECT --to-port 8080", + "iptables -t filter -A INPUT -i lo -s 127.0.0.0/8 -d 127.0.0.1/32 -p tcp --dport 80 -m state --state ESTABLISHED -j REJECT --reject-with tcp-reset", + "iptables -t filter -A INPUT ! -i lo -p tcp --dport 80 -m state --state ESTABLISHED -j REJECT --reject-with tcp-reset", }, expectError: false, fakeError: nil, @@ -104,13 +104,13 @@ func Test_Commands(t *testing.T) { testFunction: func(tr protocol.TrafficRedirector) error { return tr.Stop() }, + //nolint:lll expectedCmds: []string{ - "iptables -D OUTPUT -t nat -s 127.0.0.0/8 -d 127.0.0.1/32 -p tcp --dport 80 -j REDIRECT --to-port 8080", - "iptables -D PREROUTING -t nat ! -i lo -p tcp --dport 80 -j REDIRECT --to-port 8080", - //nolint:lll - "iptables -D INPUT -i lo -s 127.0.0.0/8 -d 127.0.0.1/32 -p tcp --dport 80 -m state --state ESTABLISHED -j REJECT --reject-with tcp-reset", - "iptables -D INPUT ! -i lo -p tcp --dport 80 -m state --state ESTABLISHED -j REJECT --reject-with tcp-reset", - "iptables -A INPUT -p tcp --dport 8080 -j REJECT --reject-with tcp-reset", + "iptables -t nat -D OUTPUT -s 127.0.0.0/8 -d 127.0.0.1/32 -p tcp --dport 80 -j REDIRECT --to-port 8080", + "iptables -t nat -D PREROUTING ! -i lo -p tcp --dport 80 -j REDIRECT --to-port 8080", + "iptables -t filter -D INPUT -i lo -s 127.0.0.0/8 -d 127.0.0.1/32 -p tcp --dport 80 -m state --state ESTABLISHED -j REJECT --reject-with tcp-reset", + "iptables -t filter -D INPUT ! -i lo -p tcp --dport 80 -m state --state ESTABLISHED -j REJECT --reject-with tcp-reset", + "iptables -t filter -A INPUT -p tcp --dport 8080 -j REJECT --reject-with tcp-reset", }, expectError: false, fakeError: nil, @@ -153,7 +153,7 @@ func Test_Commands(t *testing.T) { t.Parallel() executor := runtime.NewFakeExecutor(tc.fakeOutput, tc.fakeError) - redirector, err := NewTrafficRedirector(&tc.redirect, executor) + redirector, err := NewTrafficRedirector(&tc.redirect, New(executor)) if err != nil { t.Errorf("failed creating traffic redirector with error %v", err) return From 33abf218c1ae0c15c53c14dfba5a5a21a12dc812 Mon Sep 17 00:00:00 2001 From: Roberto Santalla Date: Thu, 9 Nov 2023 14:33:05 +0100 Subject: [PATCH 2/5] redirector: move to protocol package --- cmd/agent/commands/grpc.go | 6 ++-- cmd/agent/commands/http.go | 6 ++-- .../protocol}/redirector.go | 29 +++++++++---------- .../protocol}/redirector_test.go | 27 ++++++++--------- 4 files changed, 34 insertions(+), 34 deletions(-) rename pkg/{iptables => agent/protocol}/redirector.go (92%) rename pkg/{iptables => agent/protocol}/redirector_test.go (87%) diff --git a/cmd/agent/commands/grpc.go b/cmd/agent/commands/grpc.go index d8e8d8dc..7d9d3fca 100644 --- a/cmd/agent/commands/grpc.go +++ b/cmd/agent/commands/grpc.go @@ -31,7 +31,7 @@ func BuildGrpcCmd(env runtime.Environment, config *agent.Config) *cobra.Command Long: "Disrupts http request by introducing delays and errors." + " When running as a transparent proxy requires NET_ADMIM capabilities for setting" + " iptable rules.", - RunE: func(cmd *cobra.Command, args []string) error { + RunE: func(cmd *cobra.Command, _ []string) error { if targetPort == 0 { return fmt.Errorf("target port for fault injection is required") } @@ -65,12 +65,12 @@ func BuildGrpcCmd(env runtime.Environment, config *agent.Config) *cobra.Command // Redirect traffic to the proxy var redirector protocol.TrafficRedirector if transparent { - tr := &iptables.TrafficRedirectionSpec{ + tr := &protocol.TrafficRedirectionSpec{ DestinationPort: targetPort, // Redirect traffic from the application (target) port... RedirectPort: port, // to the proxy port. } - redirector, err = iptables.NewTrafficRedirector(tr, iptables.New(env.Executor())) + redirector, err = protocol.NewTrafficRedirector(tr, iptables.New(env.Executor())) if err != nil { return err } diff --git a/cmd/agent/commands/http.go b/cmd/agent/commands/http.go index 38b87972..a21cb55b 100644 --- a/cmd/agent/commands/http.go +++ b/cmd/agent/commands/http.go @@ -30,7 +30,7 @@ func BuildHTTPCmd(env runtime.Environment, config *agent.Config) *cobra.Command Long: "Disrupts http request by introducing delays and errors." + " When running as a transparent proxy requires NET_ADMIM capabilities for setting" + " iptable rules.", - RunE: func(cmd *cobra.Command, args []string) error { + RunE: func(cmd *cobra.Command, _ []string) error { if targetPort == 0 { return fmt.Errorf("target port for fault injection is required") } @@ -64,12 +64,12 @@ func BuildHTTPCmd(env runtime.Environment, config *agent.Config) *cobra.Command // Redirect traffic to the proxy var redirector protocol.TrafficRedirector if transparent { - tr := &iptables.TrafficRedirectionSpec{ + tr := &protocol.TrafficRedirectionSpec{ DestinationPort: targetPort, // Redirect traffic from the application (target) port... RedirectPort: port, // to the proxy port. } - redirector, err = iptables.NewTrafficRedirector(tr, iptables.New(env.Executor())) + redirector, err = protocol.NewTrafficRedirector(tr, iptables.New(env.Executor())) if err != nil { return err } diff --git a/pkg/iptables/redirector.go b/pkg/agent/protocol/redirector.go similarity index 92% rename from pkg/iptables/redirector.go rename to pkg/agent/protocol/redirector.go index 9d067742..4e03d995 100644 --- a/pkg/iptables/redirector.go +++ b/pkg/agent/protocol/redirector.go @@ -1,10 +1,9 @@ -// Package iptables implements helpers for manipulating the iptables. -// Requires the iptables command to be installed. -// Requires 'NET_ADMIN' capabilities for manipulating the iptables. -package iptables +package protocol import ( "fmt" + + "github.com/grafana/xk6-disruptor/pkg/iptables" ) // TrafficRedirectionSpec specifies the redirection of traffic to a destination @@ -19,13 +18,13 @@ type TrafficRedirectionSpec struct { // Redirector is an implementation of TrafficRedirector that uses iptables rules. type Redirector struct { *TrafficRedirectionSpec - iptables Iptables + iptables iptables.Iptables } // NewTrafficRedirector creates instances of an iptables traffic redirector func NewTrafficRedirector( tr *TrafficRedirectionSpec, - iptables Iptables, + iptables iptables.Iptables, ) (*Redirector, error) { if tr.DestinationPort == 0 || tr.RedirectPort == 0 { return nil, fmt.Errorf("DestinationPort and RedirectPort must be specified") @@ -66,14 +65,14 @@ func NewTrafficRedirector( // +-----------+---------------+------------------------+ // | lo | ! 127.0.0.0/8 | Proxy traffic | // +-----------+---------------+------------------------+ -func (tr *Redirector) rules() []Rule { +func (tr *Redirector) rules() []iptables.Rule { // redirectLocalRule is a netfilter rule that intercepts locally-originated traffic, such as that coming from sidecars // or `kubectl port-forward, directed to the application and redirects it to the proxy. // As per https://upload.wikimedia.org/wikipedia/commons/3/37/Netfilter-packet-flow.svg, locally originated traffic // traverses OUTPUT instead of PREROUTING. // Traffic created by the proxy itself to the application also traverses this chain, but is not redirected by this rule // as the proxy targets the pod IP and not the loopback address. - redirectLocalRule := Rule{ + redirectLocalRule := iptables.Rule{ Table: "nat", Chain: "OUTPUT", // For local traffic Args: "-s 127.0.0.0/8 -d 127.0.0.1/32 " + // Coming from and directed to localhost, i.e. not the pod IP. @@ -85,7 +84,7 @@ func (tr *Redirector) rules() []Rule { // it to the proxy. // Traffic created by the proxy itself to the application traverses is not redirected by this rule as it traverses the // OUTPUT chain, not PREROUTING. - redirectExternalRule := Rule{ + redirectExternalRule := iptables.Rule{ Table: "nat", Chain: "PREROUTING", // For remote traffic Args: "! -i lo " + // Not coming form loopback. Technically not needed, but doesn't hurt and helps readability. @@ -98,7 +97,7 @@ func (tr *Redirector) rules() []Rule { // This rule matches connections from sidecars and `kubectl port-forward`. // Connections from the proxy itself do not match this rule, as although they flow through `lo`, they are directed to // the pod's external IP and not the loopback address. - resetLocalRule := Rule{ + resetLocalRule := iptables.Rule{ Table: "filter", Chain: "INPUT", // For traffic traversing the INPUT chain Args: "-i lo " + // On the loopback interface @@ -112,7 +111,7 @@ func (tr *Redirector) rules() []Rule { // coming from anywhere except the local IP. // This rule matches external connections to the pod's IP address. // Connections from the proxy itself do not match this rule, as they flow through the `lo` interface. - resetExternalRule := Rule{ + resetExternalRule := iptables.Rule{ Table: "filter", Chain: "INPUT", // For traffic traversing the INPUT chain Args: "! -i lo " + // Not coming form loopback. This is technically not needed as loopback traffic does not @@ -122,7 +121,7 @@ func (tr *Redirector) rules() []Rule { "-j REJECT --reject-with tcp-reset", // Reject it } - return []Rule{ + return []iptables.Rule{ redirectLocalRule, redirectExternalRule, resetLocalRule, @@ -133,8 +132,8 @@ func (tr *Redirector) rules() []Rule { // proxyResetRule returns a netfilter rule that rejects traffic to the proxy. // This rule is set up after injection finishes to kill any leftover connection to the proxy. // TODO: Run some tests to check if this is really necessary, as the proxy may already be killing conns on termination. -func (tr *Redirector) resetProxyRule() Rule { - return Rule{ +func (tr *Redirector) resetProxyRule() iptables.Rule { + return iptables.Rule{ Table: "filter", Chain: "INPUT", Args: fmt.Sprintf("-p tcp --dport %d ", tr.RedirectPort) + // Directed to the proxy port @@ -147,7 +146,7 @@ func (tr *Redirector) Start() error { // Remove reset rule for the proxy in case it exists from a previous run. _ = tr.iptables.Remove(tr.resetProxyRule()) - // TODO: Use RuleSet instead, which takes care of automatically cleaning the rules. + // TODO: Use iptables.RuleSet instead, which takes care of automatically cleaning the rules. for _, rule := range tr.rules() { err := tr.iptables.Add(rule) if err != nil { diff --git a/pkg/iptables/redirector_test.go b/pkg/agent/protocol/redirector_test.go similarity index 87% rename from pkg/iptables/redirector_test.go rename to pkg/agent/protocol/redirector_test.go index de28a404..fe471c82 100644 --- a/pkg/iptables/redirector_test.go +++ b/pkg/agent/protocol/redirector_test.go @@ -1,4 +1,4 @@ -package iptables +package protocol_test import ( "fmt" @@ -6,6 +6,7 @@ import ( "github.com/google/go-cmp/cmp" "github.com/grafana/xk6-disruptor/pkg/agent/protocol" + "github.com/grafana/xk6-disruptor/pkg/iptables" "github.com/grafana/xk6-disruptor/pkg/runtime" ) @@ -14,12 +15,12 @@ func Test_validateTrafficRedirect(t *testing.T) { TestCases := []struct { title string - redirect TrafficRedirectionSpec + redirect protocol.TrafficRedirectionSpec expectError bool }{ { title: "Valid redirect", - redirect: TrafficRedirectionSpec{ + redirect: protocol.TrafficRedirectionSpec{ DestinationPort: 80, RedirectPort: 8080, }, @@ -27,7 +28,7 @@ func Test_validateTrafficRedirect(t *testing.T) { }, { title: "Same target and proxy port", - redirect: TrafficRedirectionSpec{ + redirect: protocol.TrafficRedirectionSpec{ DestinationPort: 8080, RedirectPort: 8080, }, @@ -35,7 +36,7 @@ func Test_validateTrafficRedirect(t *testing.T) { }, { title: "Ports not specified", - redirect: TrafficRedirectionSpec{}, + redirect: protocol.TrafficRedirectionSpec{}, expectError: true, }, } @@ -47,9 +48,9 @@ func Test_validateTrafficRedirect(t *testing.T) { t.Parallel() executor := runtime.NewFakeExecutor(nil, nil) - _, err := NewTrafficRedirector( + _, err := protocol.NewTrafficRedirector( &tc.redirect, - New(executor), + iptables.New(executor), ) if tc.expectError && err == nil { t.Errorf("error expected but none returned") @@ -67,7 +68,7 @@ func Test_Commands(t *testing.T) { TestCases := []struct { title string - redirect TrafficRedirectionSpec + redirect protocol.TrafficRedirectionSpec expectedCmds []string expectError bool fakeError error @@ -76,7 +77,7 @@ func Test_Commands(t *testing.T) { }{ { title: "Start valid redirect", - redirect: TrafficRedirectionSpec{ + redirect: protocol.TrafficRedirectionSpec{ DestinationPort: 80, RedirectPort: 8080, }, @@ -97,7 +98,7 @@ func Test_Commands(t *testing.T) { }, { title: "Stop active redirect", - redirect: TrafficRedirectionSpec{ + redirect: protocol.TrafficRedirectionSpec{ DestinationPort: 80, RedirectPort: 8080, }, @@ -118,7 +119,7 @@ func Test_Commands(t *testing.T) { }, { title: "Error invoking iptables command in Start", - redirect: TrafficRedirectionSpec{ + redirect: protocol.TrafficRedirectionSpec{ DestinationPort: 80, RedirectPort: 8080, }, @@ -132,7 +133,7 @@ func Test_Commands(t *testing.T) { }, { title: "Error invoking iptables command in Stop", - redirect: TrafficRedirectionSpec{ + redirect: protocol.TrafficRedirectionSpec{ DestinationPort: 80, RedirectPort: 8080, }, @@ -153,7 +154,7 @@ func Test_Commands(t *testing.T) { t.Parallel() executor := runtime.NewFakeExecutor(tc.fakeOutput, tc.fakeError) - redirector, err := NewTrafficRedirector(&tc.redirect, New(executor)) + redirector, err := protocol.NewTrafficRedirector(&tc.redirect, iptables.New(executor)) if err != nil { t.Errorf("failed creating traffic redirector with error %v", err) return From e791a48f5710ce7cecc85148e5acd7c9e6e8eb7c Mon Sep 17 00:00:00 2001 From: Roberto Santalla Date: Fri, 10 Nov 2023 10:47:47 +0100 Subject: [PATCH 3/5] agent: put tests in the same package as the code --- pkg/agent/protocol/redirector_test.go | 35 +++++++++++++-------------- 1 file changed, 17 insertions(+), 18 deletions(-) diff --git a/pkg/agent/protocol/redirector_test.go b/pkg/agent/protocol/redirector_test.go index fe471c82..be2d5353 100644 --- a/pkg/agent/protocol/redirector_test.go +++ b/pkg/agent/protocol/redirector_test.go @@ -1,11 +1,10 @@ -package protocol_test +package protocol import ( "fmt" "testing" "github.com/google/go-cmp/cmp" - "github.com/grafana/xk6-disruptor/pkg/agent/protocol" "github.com/grafana/xk6-disruptor/pkg/iptables" "github.com/grafana/xk6-disruptor/pkg/runtime" ) @@ -15,12 +14,12 @@ func Test_validateTrafficRedirect(t *testing.T) { TestCases := []struct { title string - redirect protocol.TrafficRedirectionSpec + redirect TrafficRedirectionSpec expectError bool }{ { title: "Valid redirect", - redirect: protocol.TrafficRedirectionSpec{ + redirect: TrafficRedirectionSpec{ DestinationPort: 80, RedirectPort: 8080, }, @@ -28,7 +27,7 @@ func Test_validateTrafficRedirect(t *testing.T) { }, { title: "Same target and proxy port", - redirect: protocol.TrafficRedirectionSpec{ + redirect: TrafficRedirectionSpec{ DestinationPort: 8080, RedirectPort: 8080, }, @@ -36,7 +35,7 @@ func Test_validateTrafficRedirect(t *testing.T) { }, { title: "Ports not specified", - redirect: protocol.TrafficRedirectionSpec{}, + redirect: TrafficRedirectionSpec{}, expectError: true, }, } @@ -48,7 +47,7 @@ func Test_validateTrafficRedirect(t *testing.T) { t.Parallel() executor := runtime.NewFakeExecutor(nil, nil) - _, err := protocol.NewTrafficRedirector( + _, err := NewTrafficRedirector( &tc.redirect, iptables.New(executor), ) @@ -68,20 +67,20 @@ func Test_Commands(t *testing.T) { TestCases := []struct { title string - redirect protocol.TrafficRedirectionSpec + redirect TrafficRedirectionSpec expectedCmds []string expectError bool fakeError error fakeOutput []byte - testFunction func(protocol.TrafficRedirector) error + testFunction func(TrafficRedirector) error }{ { title: "Start valid redirect", - redirect: protocol.TrafficRedirectionSpec{ + redirect: TrafficRedirectionSpec{ DestinationPort: 80, RedirectPort: 8080, }, - testFunction: func(tr protocol.TrafficRedirector) error { + testFunction: func(tr TrafficRedirector) error { return tr.Start() }, //nolint:lll @@ -98,11 +97,11 @@ func Test_Commands(t *testing.T) { }, { title: "Stop active redirect", - redirect: protocol.TrafficRedirectionSpec{ + redirect: TrafficRedirectionSpec{ DestinationPort: 80, RedirectPort: 8080, }, - testFunction: func(tr protocol.TrafficRedirector) error { + testFunction: func(tr TrafficRedirector) error { return tr.Stop() }, //nolint:lll @@ -119,11 +118,11 @@ func Test_Commands(t *testing.T) { }, { title: "Error invoking iptables command in Start", - redirect: protocol.TrafficRedirectionSpec{ + redirect: TrafficRedirectionSpec{ DestinationPort: 80, RedirectPort: 8080, }, - testFunction: func(tr protocol.TrafficRedirector) error { + testFunction: func(tr TrafficRedirector) error { return tr.Start() }, expectedCmds: []string{}, @@ -133,11 +132,11 @@ func Test_Commands(t *testing.T) { }, { title: "Error invoking iptables command in Stop", - redirect: protocol.TrafficRedirectionSpec{ + redirect: TrafficRedirectionSpec{ DestinationPort: 80, RedirectPort: 8080, }, - testFunction: func(tr protocol.TrafficRedirector) error { + testFunction: func(tr TrafficRedirector) error { return tr.Stop() }, expectedCmds: []string{}, @@ -154,7 +153,7 @@ func Test_Commands(t *testing.T) { t.Parallel() executor := runtime.NewFakeExecutor(tc.fakeOutput, tc.fakeError) - redirector, err := protocol.NewTrafficRedirector(&tc.redirect, iptables.New(executor)) + redirector, err := NewTrafficRedirector(&tc.redirect, iptables.New(executor)) if err != nil { t.Errorf("failed creating traffic redirector with error %v", err) return From 4dc9fce92edd597601605a2e62418385ffa6f087 Mon Sep 17 00:00:00 2001 From: Roberto Santalla Date: Thu, 9 Nov 2023 14:50:05 +0100 Subject: [PATCH 4/5] iptables: add unit test for iptables.Iptables --- pkg/iptables/iptables_test.go | 75 +++++++++++++++++++++++++++++++++++ 1 file changed, 75 insertions(+) diff --git a/pkg/iptables/iptables_test.go b/pkg/iptables/iptables_test.go index 3ed986bf..f3c4f5e3 100644 --- a/pkg/iptables/iptables_test.go +++ b/pkg/iptables/iptables_test.go @@ -1,6 +1,7 @@ package iptables_test import ( + "errors" "testing" "github.com/google/go-cmp/cmp" @@ -8,6 +9,80 @@ import ( "github.com/grafana/xk6-disruptor/pkg/runtime" ) +func Test_Iptables(t *testing.T) { + t.Parallel() + + anError := errors.New("an error occurred") + + for _, tc := range []struct { + name string + testFunc func(iptables.Iptables) error + execError error + expectedCommands []string + expectedError error + }{ + { + name: "Adds rule", + testFunc: func(i iptables.Iptables) error { + return i.Add(iptables.Rule{ + Table: "some", + Chain: "ECHO", + Args: "foo -t bar -w xx", + }) + }, + expectedCommands: []string{ + "iptables -t some -A ECHO foo -t bar -w xx", + }, + }, + { + name: "Removes rule", + testFunc: func(i iptables.Iptables) error { + return i.Remove(iptables.Rule{ + Table: "some", + Chain: "ECHO", + Args: "foo -t bar -w xx", + }) + }, + expectedCommands: []string{ + "iptables -t some -D ECHO foo -t bar -w xx", + }, + }, + { + name: "Propagates error", + testFunc: func(i iptables.Iptables) error { + return i.Remove(iptables.Rule{ + Table: "some", + Chain: "ECHO", + Args: "foo -t bar -w xx", + }) + }, + execError: anError, + expectedCommands: []string{ + "iptables -t some -D ECHO foo -t bar -w xx", + }, + expectedError: anError, + }, + } { + tc := tc + + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + fakeExec := runtime.NewFakeExecutor(nil, tc.execError) + ipt := iptables.New(fakeExec) + err := tc.testFunc(ipt) + if !errors.Is(err, tc.expectedError) { + t.Fatalf("Expected error to be %v, got %v", tc.expectedError, err) + } + + commands := fakeExec.CmdHistory() + if diff := cmp.Diff(commands, tc.expectedCommands); diff != "" { + t.Fatalf("Ran commands do not match expected:\n%s", diff) + } + }) + } +} + func Test_RulesetAddsRemovesRules(t *testing.T) { t.Parallel() From 43dea2efb37ecbbe6ee18846dadc8f87e080ef05 Mon Sep 17 00:00:00 2001 From: Roberto Santalla Date: Fri, 10 Nov 2023 10:47:52 +0100 Subject: [PATCH 5/5] iptables: put tests in the same package as the code --- pkg/iptables/iptables_test.go | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/pkg/iptables/iptables_test.go b/pkg/iptables/iptables_test.go index f3c4f5e3..2b825f36 100644 --- a/pkg/iptables/iptables_test.go +++ b/pkg/iptables/iptables_test.go @@ -1,11 +1,10 @@ -package iptables_test +package iptables import ( "errors" "testing" "github.com/google/go-cmp/cmp" - "github.com/grafana/xk6-disruptor/pkg/iptables" "github.com/grafana/xk6-disruptor/pkg/runtime" ) @@ -16,15 +15,15 @@ func Test_Iptables(t *testing.T) { for _, tc := range []struct { name string - testFunc func(iptables.Iptables) error + testFunc func(Iptables) error execError error expectedCommands []string expectedError error }{ { name: "Adds rule", - testFunc: func(i iptables.Iptables) error { - return i.Add(iptables.Rule{ + testFunc: func(i Iptables) error { + return i.Add(Rule{ Table: "some", Chain: "ECHO", Args: "foo -t bar -w xx", @@ -36,8 +35,8 @@ func Test_Iptables(t *testing.T) { }, { name: "Removes rule", - testFunc: func(i iptables.Iptables) error { - return i.Remove(iptables.Rule{ + testFunc: func(i Iptables) error { + return i.Remove(Rule{ Table: "some", Chain: "ECHO", Args: "foo -t bar -w xx", @@ -49,8 +48,8 @@ func Test_Iptables(t *testing.T) { }, { name: "Propagates error", - testFunc: func(i iptables.Iptables) error { - return i.Remove(iptables.Rule{ + testFunc: func(i Iptables) error { + return i.Remove(Rule{ Table: "some", Chain: "ECHO", Args: "foo -t bar -w xx", @@ -69,7 +68,7 @@ func Test_Iptables(t *testing.T) { t.Parallel() fakeExec := runtime.NewFakeExecutor(nil, tc.execError) - ipt := iptables.New(fakeExec) + ipt := New(fakeExec) err := tc.testFunc(ipt) if !errors.Is(err, tc.expectedError) { t.Fatalf("Expected error to be %v, got %v", tc.expectedError, err) @@ -87,17 +86,17 @@ func Test_RulesetAddsRemovesRules(t *testing.T) { t.Parallel() exec := runtime.NewFakeExecutor(nil, nil) - ruleset := iptables.NewRuleSet(iptables.New(exec)) + ruleset := NewRuleSet(New(exec)) // Add two rules - err := ruleset.Add(iptables.Rule{ + err := ruleset.Add(Rule{ Table: "table1", Chain: "CHAIN1", Args: "--foo foo --bar bar", }) if err != nil { t.Fatalf("error adding rule: %v", err) } - err = ruleset.Add(iptables.Rule{ + err = ruleset.Add(Rule{ Table: "table2", Chain: "CHAIN2", Args: "--boo boo --baz baz", }) if err != nil { @@ -135,7 +134,7 @@ func Test_RulesetAddsRemovesRules(t *testing.T) { exec.Reset() // After removing the rules, add a new one. - err = ruleset.Add(iptables.Rule{ + err = ruleset.Add(Rule{ Table: "table3", Chain: "CHAIN3", Args: "--zoo zoo --zap zap", }) if err != nil {