From 389308b8e3a1fd3fc13e17076c09b36604c1811e Mon Sep 17 00:00:00 2001 From: Zhen Tang Date: Wed, 20 Sep 2023 10:31:49 +0800 Subject: [PATCH] iptables-helper: Refactoring. Remove usage of go-iptables in other modules. Signed-off-by: Zhen Tang --- pkg/agent/config.go | 13 ------ pkg/agent/iptables.go | 34 +++++++++------ pkg/agent/manager.go | 7 +--- pkg/cloud-agent/cloud_agent.go | 5 +-- pkg/cloud-agent/iptables.go | 41 +++++++++---------- pkg/connector/iptables.go | 13 +++--- .../{rule => iptables}/iptables_helper.go | 23 +++++++++-- 7 files changed, 71 insertions(+), 65 deletions(-) rename pkg/util/{rule => iptables}/iptables_helper.go (93%) diff --git a/pkg/agent/config.go b/pkg/agent/config.go index de80dd7..cc70925 100644 --- a/pkg/agent/config.go +++ b/pkg/agent/config.go @@ -21,7 +21,6 @@ import ( "time" debpkg "github.com/bep/debounce" - "github.com/coreos/go-iptables/iptables" "github.com/spf13/pflag" "k8s.io/klog/v2/klogr" "k8s.io/utils/exec" @@ -174,21 +173,9 @@ func (cfg Config) Manager() (*Manager, error) { return nil, err } - ipt, err := iptables.New() - if err != nil { - return nil, err - } - - ipt6, err := iptables.NewWithProtocol(iptables.ProtocolIPv6) - if err != nil { - return nil, err - } - m := &Manager{ Config: cfg, tm: tm, - ipt4: ipt, - ipt6: ipt6, log: klogr.New().WithName("manager"), events: make(chan struct{}), diff --git a/pkg/agent/iptables.go b/pkg/agent/iptables.go index e6a9c7b..75e4e9b 100644 --- a/pkg/agent/iptables.go +++ b/pkg/agent/iptables.go @@ -20,7 +20,7 @@ import ( "k8s.io/apimachinery/pkg/util/sets" "github.com/fabedge/fabedge/pkg/util/ipset" - "github.com/fabedge/fabedge/pkg/util/rule" + "github.com/fabedge/fabedge/pkg/util/iptables" ) type IPSet struct { @@ -34,11 +34,21 @@ func (m *Manager) ensureIPTablesRules() error { peerIPSet4, peerIPSet6 := m.getAllPeerCIDRs() subnetsIP4, subnetsIP6 := classifySubnets(current.Subnets) + ipt, err := iptables.NewIPTablesHelper() + if err != nil { + return err + } + + ipt6, err := iptables.NewIP6TablesHelper() + if err != nil { + return err + } + configs := []struct { peerIPSet IPSet loopbackIPSet IPSet subnets []string - helper *rule.IPTablesHelper + helper *iptables.IPTablesHelper }{ { peerIPSet: IPSet{ @@ -50,7 +60,7 @@ func (m *Manager) ensureIPTablesRules() error { EntrySet: peerIPSet4, }, subnets: subnetsIP4, - helper: rule.NewIPTablesHelper(m.ipt4), + helper: ipt, }, { peerIPSet: IPSet{ @@ -62,7 +72,7 @@ func (m *Manager) ensureIPTablesRules() error { EntrySet: peerIPSet6, }, subnets: subnetsIP6, - helper: rule.NewIPTablesHelper(m.ipt6), + helper: ipt6, }, } @@ -89,21 +99,21 @@ func (m *Manager) ensureIPTablesRules() error { return nil } -func (m *Manager) ensureIPForwardRules(helper *rule.IPTablesHelper, subnets []string) error { +func (m *Manager) ensureIPForwardRules(helper *iptables.IPTablesHelper, subnets []string) error { if err := helper.CheckOrCreateFabEdgeForwardChain(); err != nil { - m.log.Error(err, "failed to check or create iptables chain", "table", rule.TableFilter, "chain", rule.ChainFabEdgeForward) + m.log.Error(err, "failed to check or create iptables chain", "table", iptables.TableFilter, "chain", iptables.ChainFabEdgeForward) return err } if err := helper.PrepareForwardChain(); err != nil { - m.log.Error(err, "failed to check or add rule", "table", rule.TableFilter, "chain", rule.ChainForward, "rule", "-j FABEDGE-FORWARD") + m.log.Error(err, "failed to check or add rule", "table", iptables.TableFilter, "chain", iptables.ChainForward, "rule", "-j FABEDGE-FORWARD") return err } // subnets won't change most of the time, and is append-only, so for now we don't need // to handle removing old subnet if err, errRule := helper.MaintainForwardRulesForSubnets(subnets); err != nil { - m.log.Error(err, "failed to check or add rule", "table", rule.TableFilter, "chain", rule.ChainFabEdgeForward, "rule", errRule) + m.log.Error(err, "failed to check or add rule", "table", iptables.TableFilter, "chain", iptables.ChainFabEdgeForward, "rule", errRule) return err } @@ -111,16 +121,16 @@ func (m *Manager) ensureIPForwardRules(helper *rule.IPTablesHelper, subnets []st } // outbound NAT from pods to outside the cluster -func (m *Manager) configureOutboundRules(helper *rule.IPTablesHelper, peerIPSet IPSet, subnets []string, clearFabEdgeNatOutgoingChain bool) error { +func (m *Manager) configureOutboundRules(helper *iptables.IPTablesHelper, peerIPSet IPSet, subnets []string, clearFabEdgeNatOutgoingChain bool) error { if clearFabEdgeNatOutgoingChain { m.log.V(3).Info("Subnets are changed, clear iptables chain FABEDGE-NAT-OUTGOING") if err := helper.ClearOrCreateFabEdgeNatOutgoingChain(); err != nil { - m.log.Error(err, "failed to check or add rule", "table", rule.TableNat, "chain", rule.ChainFabEdgeNatOutgoing) + m.log.Error(err, "failed to check or add rule", "table", iptables.TableNat, "chain", iptables.ChainFabEdgeNatOutgoing) return err } } else { if err := helper.CheckOrCreateFabEdgeNatOutgoingChain(); err != nil { - m.log.Error(err, "failed to check or add rule", "table", rule.TableNat, "chain", rule.ChainFabEdgeNatOutgoing) + m.log.Error(err, "failed to check or add rule", "table", iptables.TableNat, "chain", iptables.ChainFabEdgeNatOutgoing) return err } } @@ -132,7 +142,7 @@ func (m *Manager) configureOutboundRules(helper *rule.IPTablesHelper, peerIPSet m.log.V(3).Info("configure outgoing NAT iptables rules") if err, errRule := helper.MaintainNatOutgoingRulesForSubnets(subnets, peerIPSet.IPSet.Name); err != nil { - m.log.Error(err, "failed to append rule", "table", rule.TableNat, "chain", rule.ChainFabEdgeNatOutgoing, "rule", errRule) + m.log.Error(err, "failed to append rule", "table", iptables.TableNat, "chain", iptables.ChainFabEdgeNatOutgoing, "rule", errRule) return err } diff --git a/pkg/agent/manager.go b/pkg/agent/manager.go index cae61d3..1352756 100644 --- a/pkg/agent/manager.go +++ b/pkg/agent/manager.go @@ -24,7 +24,6 @@ import ( "sync" "time" - "github.com/coreos/go-iptables/iptables" "github.com/go-logr/logr" "k8s.io/apimachinery/pkg/util/sets" @@ -47,10 +46,8 @@ type Manager struct { ipvs ipvs.Interface ipset ipset.Interface - tm tunnel.Manager - ipt4 *iptables.IPTables - ipt6 *iptables.IPTables - log logr.Logger + tm tunnel.Manager + log logr.Logger currentEndpoint Endpoint mediatorEndpoint *Endpoint diff --git a/pkg/cloud-agent/cloud_agent.go b/pkg/cloud-agent/cloud_agent.go index 49c92aa..6ac10e2 100644 --- a/pkg/cloud-agent/cloud_agent.go +++ b/pkg/cloud-agent/cloud_agent.go @@ -23,7 +23,6 @@ import ( "time" "github.com/bep/debounce" - "github.com/coreos/go-iptables/iptables" flag "github.com/spf13/pflag" "github.com/vishvananda/netlink" utilerrors "k8s.io/apimachinery/pkg/util/errors" @@ -108,12 +107,12 @@ func Execute() { } func NewCloudAgent() (*CloudAgent, error) { - iph, err := newIptableHandler(iptables.ProtocolIPv4) + iph, err := newIptableHandler() if err != nil { return nil, err } - iph6, err := newIptableHandler(iptables.ProtocolIPv6) + iph6, err := newIp6tableHandler() if err != nil { return nil, err } diff --git a/pkg/cloud-agent/iptables.go b/pkg/cloud-agent/iptables.go index 00c21b1..cf1a1f1 100644 --- a/pkg/cloud-agent/iptables.go +++ b/pkg/cloud-agent/iptables.go @@ -15,11 +15,8 @@ package cloud_agent import ( - "fmt" - - "github.com/coreos/go-iptables/iptables" ipsetutil "github.com/fabedge/fabedge/pkg/util/ipset" - "github.com/fabedge/fabedge/pkg/util/rule" + "github.com/fabedge/fabedge/pkg/util/iptables" "k8s.io/apimachinery/pkg/util/sets" ) @@ -32,34 +29,34 @@ type IptablesHandler struct { ipset ipsetutil.Interface ipsetName string hashFamily string - helper *rule.IPTablesHelper + helper *iptables.IPTablesHelper } -func newIptableHandler(version iptables.Protocol) (*IptablesHandler, error) { - var ( - ipsetName string - hashFamily string - ) - - switch version { - case iptables.ProtocolIPv4: - ipsetName, hashFamily = IPSetRemotePodCIDR, ipsetutil.ProtocolFamilyIPV4 - case iptables.ProtocolIPv6: - ipsetName, hashFamily = IPSetRemotePodCIDR6, ipsetutil.ProtocolFamilyIPV6 - default: - return nil, fmt.Errorf("unknown version") +func newIptableHandler() (*IptablesHandler, error) { + iptHelper, err := iptables.NewIPTablesHelper() + if err != nil { + return nil, err } - ipt, err := iptables.NewWithProtocol(version) + return &IptablesHandler{ + ipset: ipsetutil.New(), + ipsetName: IPSetRemotePodCIDR, + hashFamily: ipsetutil.ProtocolFamilyIPV4, + helper: iptHelper, + }, nil +} + +func newIp6tableHandler() (*IptablesHandler, error) { + iptHelper, err := iptables.NewIP6TablesHelper() if err != nil { return nil, err } return &IptablesHandler{ ipset: ipsetutil.New(), - ipsetName: ipsetName, - hashFamily: hashFamily, - helper: rule.NewIPTablesHelper(ipt), + ipsetName: IPSetRemotePodCIDR6, + hashFamily: ipsetutil.ProtocolFamilyIPV6, + helper: iptHelper, }, nil } diff --git a/pkg/connector/iptables.go b/pkg/connector/iptables.go index bbefffa..f00c9d2 100644 --- a/pkg/connector/iptables.go +++ b/pkg/connector/iptables.go @@ -17,9 +17,8 @@ package connector import ( "sync" - "github.com/coreos/go-iptables/iptables" "github.com/fabedge/fabedge/pkg/util/ipset" - "github.com/fabedge/fabedge/pkg/util/rule" + "github.com/fabedge/fabedge/pkg/util/iptables" "github.com/go-logr/logr" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/klog/v2/klogr" @@ -58,11 +57,11 @@ type IPTablesHandler struct { specs []IPSetSpec lock sync.RWMutex - helper *rule.IPTablesHelper + helper *iptables.IPTablesHelper } func newIP4TablesHandler() (*IPTablesHandler, error) { - ipt, err := iptables.New() + iptHelper, err := iptables.NewIPTablesHelper() if err != nil { return nil, err } @@ -77,12 +76,12 @@ func newIP4TablesHandler() (*IPTablesHandler, error) { CloudPodCIDR: IPSetCloudPodCIDR, CloudNodeCIDR: IPSetCloudNodeCIDR, }, - helper: rule.NewIPTablesHelper(ipt), + helper: iptHelper, }, nil } func newIP6TablesHandler() (*IPTablesHandler, error) { - ipt, err := iptables.New(iptables.IPFamily(iptables.ProtocolIPv6)) + iptHelper, err := iptables.NewIP6TablesHelper() if err != nil { return nil, err } @@ -97,7 +96,7 @@ func newIP6TablesHandler() (*IPTablesHandler, error) { CloudPodCIDR: IPSetCloudPodCIDR6, CloudNodeCIDR: IPSetCloudNodeCIDR6, }, - helper: rule.NewIPTablesHelper(ipt), + helper: iptHelper, }, nil } diff --git a/pkg/util/rule/iptables_helper.go b/pkg/util/iptables/iptables_helper.go similarity index 93% rename from pkg/util/rule/iptables_helper.go rename to pkg/util/iptables/iptables_helper.go index 08b8992..f2d6df7 100644 --- a/pkg/util/rule/iptables_helper.go +++ b/pkg/util/iptables/iptables_helper.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package rule +package iptables import ( "fmt" @@ -36,15 +36,32 @@ const ( ChainFabEdgeNatOutgoing = "FABEDGE-NAT-OUTGOING" ) +const ( + IPTablesRestoreCommand = "iptables-restore" + IP6TablesRestoreCommand = "ip6tables-restore" +) + type IPTablesHelper struct { ipt *iptables.IPTables Mutex sync.RWMutex } -func NewIPTablesHelper(t *iptables.IPTables) *IPTablesHelper { +func NewIPTablesHelper() (*IPTablesHelper, error) { + return doCreateIPTablesHelper(iptables.ProtocolIPv4) +} + +func NewIP6TablesHelper() (*IPTablesHelper, error) { + return doCreateIPTablesHelper(iptables.ProtocolIPv6) +} + +func doCreateIPTablesHelper(protocol iptables.Protocol) (*IPTablesHelper, error) { + t, err := iptables.NewWithProtocol(protocol) + if err != nil { + return nil, err + } return &IPTablesHelper{ ipt: t, - } + }, err } func (h *IPTablesHelper) ClearOrCreateFabEdgePostRoutingChain() (err error) {