diff --git a/Gopkg.lock b/Gopkg.lock index 3e657aa..28b405a 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -10,7 +10,7 @@ version = "v0.34.0" [[projects]] - digest = "1:8645fbb3be6573f110455b93bb7c2dd29297a438229e4ae83669f7579f1921e0" + digest = "1:5fab90311f251dd2813cb016ba64841c953fb7070bc028eedc1e7852d1fa7347" name = "github.com/DevFactory/go-tools" packages = [ "pkg/extensions/collections", @@ -25,8 +25,8 @@ "pkg/nettools/testhelpers", ] pruneopts = "T" - revision = "507ef6c9f6a3056dae91a307d2a7db8fabfb44c1" - version = "v0.1.1" + revision = "5438de9ccc074183d42dd5cf7cc9e9710aca3b1e" + version = "v0.2.2" [[projects]] branch = "master" diff --git a/Gopkg.toml b/Gopkg.toml index 1f7df9c..a0f007c 100644 --- a/Gopkg.toml +++ b/Gopkg.toml @@ -25,7 +25,7 @@ required = [ [[constraint]] name = "github.com/DevFactory/go-tools" - version = "0.1.1" + version = "0.2.0" [[constraint]] name = "github.com/stretchr/testify" diff --git a/doc/ops_guide.md b/doc/ops_guide.md index 12b71d5..101ea0d 100644 --- a/doc/ops_guide.md +++ b/doc/ops_guide.md @@ -130,6 +130,7 @@ Advanced options: * "SNCTRLR_IPTABLES_TIMEOUT_SEC" - timeout in seconds for the `iptables` system command that is frequently called by smartnat. Default should be OK, might need to be increased on heavily loaded smartnat instances. * "SNCTRLR_AUTOREFRESH_PERIOD_SEC" - smartnat automatically reloads information about available network interface and assigned IP addresses. This setting controls how frequently (in seconds) should this information be loaded from the operating system. The default is 60 s. * "SNCTRLR_MAX_FINALIZE_WAIT_MINUTES" - sets the absolute finalizer timeout in minutes. The finalizer timeout is a safety mechanism that needs to be triggered if a smartnat instance has configured a Mapping and then the instance had failed and never came back. In that case, when the Mapping is removed, kubernetes doesn't really delete the Mapping until all smartnat nodes that had been configured for it confirm that the configuration was removed. A failed instance will never do that, so we use this timeout to forcibly remove the Mapping after SNCTRLR_AUTOREFRESH_PERIOD_SEC, even if it is indicated as being still configured on some smartnat nodes. +* "SNCTRLR_MAX_PORTS_PER_MAPPING" - limits the number of Ports a single Mapping can configure. The number of exposed ports has influence on traffic forwarding rate and thus might be needed to tune by the administrator. The default is 50. ### 3. Cluster configuration diff --git a/pkg/config/config.go b/pkg/config/config.go index a1a3bdf..442b62b 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -33,6 +33,7 @@ type Config struct { MaxFinalizeWaitMinutes int GwAddressOffset int32 MonPort int + MaxPortsPerMapping int } const ( @@ -79,6 +80,8 @@ const ( snctrlrGwAddressOffset = "SNCTRLR_GW_ADDRESS_OFFSET" // snctrlrMonitorPort configures TCP port for exposing HTTP healthcheck and performance metrics snctrlrMonitorPort = "SNCTRLR_MONITOR_PORT" + // snctrlrMaxPortsPerMapping configures a limit of ports used by a single mapping + snctrlrMaxPortsPerMapping = "SNCTRLR_MAX_PORTS_PER_MAPPING" ) // NewConfig returns new configuration settings for smart-nat-controller @@ -91,7 +94,7 @@ func NewConfig() (*Config, error) { // NewConfigFromArgs creates new config struct from arguments, after running validation func NewConfigFromArgs(interfaceNamePattern string, setupMasquerade, setupSNAT bool, defaultGWInterface string, ipTablesTimeoutSec int, enableDebug bool, interfaceAutorefreshPeriodSec, maxFinalizeWaitMinutes int, - gwAddressOffset int32, monitorPort int) (*Config, error) { + gwAddressOffset int32, monitorPort int, maxPortsPerMapping int) (*Config, error) { cfg := &Config{ DefaultGWInterface: defaultGWInterface, EnableDebug: enableDebug, @@ -103,6 +106,7 @@ func NewConfigFromArgs(interfaceNamePattern string, setupMasquerade, setupSNAT b MaxFinalizeWaitMinutes: maxFinalizeWaitMinutes, GwAddressOffset: gwAddressOffset, MonPort: monitorPort, + MaxPortsPerMapping: maxPortsPerMapping, } err := cfg.validate() return cfg, err @@ -121,6 +125,7 @@ func (cfg *Config) load() error { offset, _ := eos.GetIntFromEnvVarWithDefault(snctrlrGwAddressOffset, 1) cfg.GwAddressOffset = int32(offset) cfg.MonPort, _ = eos.GetIntFromEnvVarWithDefault(snctrlrMonitorPort, 8080) + cfg.MaxPortsPerMapping, _ = eos.GetIntFromEnvVarWithDefault(snctrlrMaxPortsPerMapping, 50) return cfg.validate() } @@ -143,6 +148,9 @@ func (cfg *Config) validate() error { if cfg.MaxFinalizeWaitMinutes <= 0 { return fmt.Errorf("Configuration env var %s can't be <= 0", snctrlrMaxFinalizeWaitMinutes) } + if cfg.MaxPortsPerMapping <= 0 { + return fmt.Errorf("Configuration env var %s can't be <= 0", snctrlrMaxPortsPerMapping) + } if cfg.MonPort <= 0 || cfg.MonPort > 65535 { return fmt.Errorf("Configuration env var %s must be a valid TCP port num (1-65535)", snctrlrMonitorPort) } diff --git a/pkg/controller/mapping/dnat_providers.go b/pkg/controller/mapping/dnat_providers.go index baa8b94..569ba0c 100644 --- a/pkg/controller/mapping/dnat_providers.go +++ b/pkg/controller/mapping/dnat_providers.go @@ -16,7 +16,6 @@ package mapping import ( "fmt" "net" - "sort" "strings" "github.com/DevFactory/go-tools/pkg/extensions/collections" @@ -27,6 +26,7 @@ import ( const ( chainNamePrefix = "MAP" + ipSetNamePrefix = "DNAT" markMasqComment = "mark for masquerade in " + masqPostroutingChain ) @@ -50,13 +50,16 @@ type DNATProvider interface { // supported configuration. type ThroughServiceDNAT struct { iptables nettools.IPTablesHelper + ipset nettools.IPSetHelper namer Namer } // NewThroughServiceDNATProvider returns new instance of the ThroughServiceDNAT -func NewThroughServiceDNATProvider(iptables nettools.IPTablesHelper, namer Namer) DNATProvider { +func NewThroughServiceDNATProvider(iptables nettools.IPTablesHelper, ipset nettools.IPSetHelper, + namer Namer) DNATProvider { return &ThroughServiceDNAT{ iptables: iptables, + ipset: ipset, namer: namer, } } @@ -79,15 +82,10 @@ func (p *ThroughServiceDNAT) SetupDNAT(externalIP net.IP, mapping *v1alpha1.Mapp return err } - // sort all external port numbers by protocol - tcpPorts, udpPorts := p.getPerProtocolPorts(mapping) - // jumps to chain - if err := p.synchronizeJumpPerProtocol(mapping, tcpPorts, externalIP, "tcp", preroutingChain, chainName); err != nil { - return err - } - if err := p.synchronizeJumpPerProtocol(mapping, udpPorts, externalIP, "udp", preroutingChain, chainName); err != nil { - return err - } + // create ipset that contains all "allowed_source_ip:proto,port" pairs + // and add iptables rules that checks against that set and jumps to + // per-service chain with per-port translation rules + p.synchronizeJumpAndIPSet(mapping, externalIP, preroutingChain, chainName) // setup Masquerade marks if enabled if setupMasquerade { @@ -124,17 +122,25 @@ func (p *ThroughServiceDNAT) DeleteDNAT(externalIP net.IP, mapping *v1alpha1.Map logrusWithMapping(mapping).Errorf("Error when flushing chain %s: %v", chainName, err) return err } - logrusWithMapping(mapping).Debugf("Removing jump(s) from chain %s to %s", preroutingChain, chainName) - if err := p.deleteJumpPerProtocol("tcp", preroutingChain, mapping); err != nil { - logrusWithMapping(mapping).Errorf("Error when deleting jump to chain %s from %s for "+ - "protocol TCP: %v", chainName, preroutingChain, err) + + // remove ipset + setName := p.getIPSetName(mapping) + if err := p.ipset.DeleteSet(setName); err != nil { + logrusWithMapping(mapping).Errorf("Error removing ipset %s: %v", setName, err) return err } - if err := p.deleteJumpPerProtocol("udp", preroutingChain, mapping); err != nil { - logrusWithMapping(mapping).Errorf("Error when deleting jump to chain %s from %s for "+ - "protocol UDP: %v", chainName, preroutingChain, err) + + // remove jump rule + comment := p.getJumpComment(mapping) + logrusWithMapping(mapping).Debugf("Deleting jump in chain %s with comment: %s", + chainName, comment) + if err := p.iptables.DeleteByComment(natTableName, preroutingChain, comment); err != nil { + logrusWithMapping(mapping).Errorf("Error deleting iptables jump from chain %s to %s: %v", + preroutingChain, chainName, err) return err } + + // delete custom chain logrusWithMapping(mapping).Debugf("Deleting chain %s", chainName) if err := p.iptables.DeleteChain(natTableName, chainName); err != nil { logrusWithMapping(mapping).Errorf("Error when deleting chain %s: %v", chainName, err) @@ -143,24 +149,59 @@ func (p *ThroughServiceDNAT) DeleteDNAT(externalIP net.IP, mapping *v1alpha1.Map return nil } -func (p *ThroughServiceDNAT) synchronizeJumpPerProtocol(mapping *v1alpha1.Mapping, ports []v1alpha1.MappingPort, - externalIP net.IP, protocol, fromChain, toChain string) error { - if len(ports) > 0 { - logrusWithMapping(mapping).Debugf("Setting up jump to chain %s from %s for protocol %s", toChain, - fromChain, protocol) - if err := p.setupJumpPerProtocol(externalIP, protocol, ports, toChain, mapping); err != nil { - logrusWithMapping(mapping).Errorf("Error when setting up jump to chain %s from %s "+ - "and protocol protocol: %v; %v", toChain, fromChain, protocol, err) +func (p *ThroughServiceDNAT) synchronizeJumpAndIPSet(mapping *v1alpha1.Mapping, externalIP net.IP, fromChain, + toChain string) error { + // ensure ipset for this services exists + setName := p.getIPSetName(mapping) + logrusWithMapping(mapping).Debugf("Ensuring ipset %s exists", setName) + if err := p.ipset.EnsureSetExists(setName, "hash:net,port"); err != nil { + logrusWithMapping(mapping).Errorf("Error creating ipset: %v", err) + return err + } + + // synchronize ipset content + logrusWithMapping(mapping).Debugf("Synchronizing entries in ipset %s", setName) + tcpPorts, udpPorts := p.getPerProtocolPorts(mapping) + netPorts := []nettools.NetPort{} + for _, allowedSrc := range mapping.Spec.AllowedSources { + _, srcNet, err := net.ParseCIDR(allowedSrc) + if err != nil { + logrusWithMapping(mapping).Errorf("Error parsing allowed source %s: %v", allowedSrc, err) return err } - } else { - if err := p.deleteJumpPerProtocol(protocol, fromChain, mapping); err != nil { - logrusWithMapping(mapping).Errorf("Error when deleting jump to chain %s from %s for "+ - "protocol %s: %v", toChain, fromChain, protocol, err) - return err + endpoints := []struct { + ports []v1alpha1.MappingPort + proto nettools.Protocol + }{ + {tcpPorts, nettools.TCP}, + {udpPorts, nettools.UDP}, + } + for _, ep := range endpoints { + for _, port := range ep.ports { + netPort := nettools.NetPort{ + Net: *srcNet, + Port: uint16(port.Port), + Protocol: ep.proto, + } + netPorts = append(netPorts, netPort) + } } } - return nil + if err := p.ipset.EnsureSetHasOnlyNetPort(setName, netPorts); err != nil { + logrusWithMapping(mapping).Errorf("Error synchronizing entries in ipset %s: %v", setName, err) + return err + } + + // ensure jump to toChain exists with match against the ipset + logrusWithMapping(mapping).Debugf("Adding jump from chain %s to chain %s using ipset %s", + fromChain, toChain, setName) + return p.iptables.EnsureExistsOnlyAppend(nettools.IPTablesRuleArgs{ + Table: natTableName, + ChainName: fromChain, + Selector: []string{"-d", fmt.Sprintf("%s/32", externalIP.String()), "-m", "set", "--match-set", setName, "src,dst"}, + Action: []string{toChain}, + Comment: p.getJumpComment(mapping), + }) } // synchronizePerPortRules needs to get current rules in the specified chain and the new set @@ -169,14 +210,15 @@ func (p *ThroughServiceDNAT) synchronizeJumpPerProtocol(mapping *v1alpha1.Mappin func (p *ThroughServiceDNAT) synchronizePerPortRules(mapping *v1alpha1.Mapping, svc *v1.Service, chainName string) error { logrusWithMapping(mapping).Debugf("Starting to synchronize per port rules in chain %s", chainName) - // load current rules from the chain - tentatively we will remove all rules from this list + // load current rules from the chain current, err := p.iptables.LoadRules(natTableName, chainName) if err != nil { logrusWithMapping(mapping).Errorf("Error getting iptables rules from the operating system for"+ " chain %s. Error: %v", chainName, err) return err } - current = p.skipMarkMasq(current) + // exclude the mark for masquerade rule, so it is always kept + current = p.excludeMarkMasqRule(current) new := p.rulesFromMappedPorts(mapping, svc.Spec.ClusterIP, chainName) // now find set difference "current\new" and "new\current" @@ -220,7 +262,7 @@ func (p *ThroughServiceDNAT) synchronizePerPortRules(mapping *v1alpha1.Mapping, return nil } -func (*ThroughServiceDNAT) skipMarkMasq(rules []*nettools.IPTablesRuleArgs) []*nettools.IPTablesRuleArgs { +func (*ThroughServiceDNAT) excludeMarkMasqRule(rules []*nettools.IPTablesRuleArgs) []*nettools.IPTablesRuleArgs { res := make([]*nettools.IPTablesRuleArgs, 0, len(rules)) for _, rule := range rules { if rule.Comment == markMasqComment { @@ -248,39 +290,8 @@ func (p *ThroughServiceDNAT) rulesFromMappedPorts(mapping *v1alpha1.Mapping, svc return new } -func (p *ThroughServiceDNAT) setupJumpPerProtocol(externalIP net.IP, protocol string, - ports []v1alpha1.MappingPort, chainName string, mapping *v1alpha1.Mapping) error { - list := make([]string, 0, len(ports)) - for _, port := range ports { - list = append(list, fmt.Sprintf("%d", port.Port)) - } - stringList := strings.Join(list, ",") - logrusWithMapping(mapping).Debugf("Setting jump from %s to %s for protocol %s", - preroutingChain, chainName, protocol) - src := make([]string, len(mapping.Spec.AllowedSources)) - copy(src, mapping.Spec.AllowedSources) - sort.Strings(src) - allowedSources := strings.Join(src, ",") - return p.iptables.EnsureExistsOnlyAppend(nettools.IPTablesRuleArgs{ - Table: natTableName, - ChainName: preroutingChain, - Selector: []string{"-d", fmt.Sprintf("%s/32", externalIP.String()), "-p", protocol, - "-m", "multiport", "--dports", stringList, "-s", allowedSources}, - Action: []string{chainName}, - Comment: p.getJumpComment(mapping, protocol), - }) -} - -func (p *ThroughServiceDNAT) deleteJumpPerProtocol(protocol string, chainName string, - mapping *v1alpha1.Mapping) error { - comment := p.getJumpComment(mapping, protocol) - logrusWithMapping(mapping).Debugf("Deleting jump in chain %s with comment: %s", - chainName, comment) - return p.iptables.DeleteByComment(natTableName, chainName, comment) -} - -func (p *ThroughServiceDNAT) getJumpComment(mapping *v1alpha1.Mapping, protocol string) string { - return fmt.Sprintf("for mapping %s/%s [%s]", mapping.Namespace, mapping.Name, protocol) +func (p *ThroughServiceDNAT) getJumpComment(mapping *v1alpha1.Mapping) string { + return fmt.Sprintf("for mapping %s/%s", mapping.Namespace, mapping.Name) } func (p *ThroughServiceDNAT) getPerProtocolPorts(mapping *v1alpha1.Mapping) ( @@ -301,3 +312,7 @@ func (p *ThroughServiceDNAT) getPerProtocolPorts(mapping *v1alpha1.Mapping) ( func (p *ThroughServiceDNAT) getChainName(mapping *v1alpha1.Mapping) string { return fmt.Sprintf("%s-%s", chainNamePrefix, p.namer.Name(mapping.ObjectMeta)) } + +func (p *ThroughServiceDNAT) getIPSetName(mapping *v1alpha1.Mapping) string { + return fmt.Sprintf("%s-%s", ipSetNamePrefix, p.namer.Name(mapping.ObjectMeta)) +} diff --git a/pkg/controller/mapping/dnat_providers_test.go b/pkg/controller/mapping/dnat_providers_test.go index 77b9731..41b439f 100644 --- a/pkg/controller/mapping/dnat_providers_test.go +++ b/pkg/controller/mapping/dnat_providers_test.go @@ -47,11 +47,12 @@ type testCase struct { externalIP net.IP dport, servicePort int32 allowed string + allowedNet *net.IPNet serviceIP string meta v1meta.ObjectMeta mapping *v1alpha1.Mapping svc *v1.Service - customChain string + customChain, ipset string } func getTestCase() testCase { @@ -61,6 +62,7 @@ func getTestCase() testCase { Namespace: "test", } allowed := "0.0.0.0/0" + _, allowedNet, _ := net.ParseCIDR(allowed) serviceIP := "172.16.1.1" return testCase{ extIP: extIP, @@ -68,6 +70,7 @@ func getTestCase() testCase { dport: 80, servicePort: 8080, allowed: allowed, + allowedNet: allowedNet, serviceIP: serviceIP, meta: meta, mapping: &v1alpha1.Mapping{ @@ -85,6 +88,7 @@ func getTestCase() testCase { }, }, customChain: "MAP-MDICIJE6TVWKE333NKHBEORO", + ipset: "DNAT-MDICIJE6TVWKE333NKHBEORO", } } @@ -102,7 +106,7 @@ func Test_throughServiceDNATProvider_SetupDNAT(t *testing.T) { tests := []struct { name string ports []v1alpha1.MappingPort - init func(ipt *ntmocks.IPTablesHelper) + init func(ipt *ntmocks.IPTablesHelper, ips *ntmocks.IPSetHelper) }{ { name: "tcp only, no existing rules", @@ -113,16 +117,23 @@ func Test_throughServiceDNATProvider_SetupDNAT(t *testing.T) { ServicePort: c.servicePort, }, }, - init: func(ipt *ntmocks.IPTablesHelper) { + init: func(ipt *ntmocks.IPTablesHelper, ips *ntmocks.IPSetHelper) { ipt. + On("EnsureChainExists", "nat", c.customChain).Return(nil). On("EnsureExistsOnlyAppend", - getIptablesProtoJumpRule(c.extIP, protoTcp, c.allowed, c.customChain, c.dport, c.meta)).Return(nil). - On("DeleteByComment", natTableName, preroutingChain, fmt.Sprintf("for mapping %s/%s [%s]", - c.mapping.Namespace, c.mapping.Name, protoUdp)).Return(nil). + getIptablesProtoJumpRule(c.extIP, c.customChain, c.ipset, c.meta)).Return(nil). On("EnsureExistsInsert", iptablesMarkRule).Return(nil). On("LoadRules", natTableName, c.customChain).Return([]*nt.IPTablesRuleArgs{}, nil). On("EnsureExistsAppend", getIptablesDNATRule(c.serviceIP, protoTcp, c.customChain, c.dport, c.servicePort, c.meta)).Return(nil) + ips.On("EnsureSetExists", c.ipset, "hash:net,port").Return(nil). + On("EnsureSetHasOnlyNetPort", c.ipset, []nettools.NetPort{ + nettools.NetPort{ + Net: *c.allowedNet, + Port: uint16(c.dport), + Protocol: nettools.TCP, + }, + }).Return(nil) }, }, { @@ -139,12 +150,11 @@ func Test_throughServiceDNATProvider_SetupDNAT(t *testing.T) { ServicePort: c.servicePort, }, }, - init: func(ipt *ntmocks.IPTablesHelper) { + init: func(ipt *ntmocks.IPTablesHelper, ips *ntmocks.IPSetHelper) { ipt. + On("EnsureChainExists", "nat", c.customChain).Return(nil). On("EnsureExistsOnlyAppend", - getIptablesProtoJumpRule(c.extIP, protoTcp, c.allowed, c.customChain, c.dport, c.meta)).Return(nil). - On("EnsureExistsOnlyAppend", - getIptablesProtoJumpRule(c.extIP, protoUdp, c.allowed, c.customChain, c.dport, c.meta)).Return(nil). + getIptablesProtoJumpRule(c.extIP, c.customChain, c.ipset, c.meta)).Return(nil). On("EnsureExistsInsert", iptablesMarkRule).Return(nil). On("LoadRules", natTableName, c.customChain).Return( []*nt.IPTablesRuleArgs{&iptablesBogusExistingRule}, nil). @@ -153,6 +163,19 @@ func Test_throughServiceDNATProvider_SetupDNAT(t *testing.T) { getIptablesDNATRule(c.serviceIP, protoTcp, c.customChain, c.dport, c.servicePort, c.meta)).Return(nil). On("EnsureExistsAppend", getIptablesDNATRule(c.serviceIP, protoUdp, c.customChain, c.dport, c.servicePort, c.meta)).Return(nil) + ips.On("EnsureSetExists", c.ipset, "hash:net,port").Return(nil). + On("EnsureSetHasOnlyNetPort", c.ipset, []nettools.NetPort{ + nettools.NetPort{ + Net: *c.allowedNet, + Port: uint16(c.dport), + Protocol: nettools.TCP, + }, + nettools.NetPort{ + Net: *c.allowedNet, + Port: uint16(c.dport), + Protocol: nettools.UDP, + }, + }).Return(nil) }, }, } @@ -160,13 +183,14 @@ func Test_throughServiceDNATProvider_SetupDNAT(t *testing.T) { t.Run(tt.name, func(t *testing.T) { c.mapping.Spec.Ports = tt.ports iptables := &ntmocks.IPTablesHelper{} - iptables.On("EnsureChainExists", "nat", c.customChain).Return(nil) - tt.init(iptables) - dnat := mapping.NewThroughServiceDNATProvider(iptables, mapping.NewNamer()) + ipset := &ntmocks.IPSetHelper{} + tt.init(iptables, ipset) + dnat := mapping.NewThroughServiceDNATProvider(iptables, ipset, mapping.NewNamer()) // call SetupDNAT err := dnat.SetupDNAT(c.externalIP, c.mapping, c.svc, nil, true) assert.Nil(t, err) iptables.AssertExpectations(t) + ipset.AssertExpectations(t) }) } } @@ -210,12 +234,12 @@ func Test_throughServiceDNATProvider_DeleteDNAT(t *testing.T) { iptables := &ntmocks.IPTablesHelper{} iptables. On("FlushChain", "nat", c.customChain).Return(nil). - On("DeleteByComment", natTableName, preroutingChain, fmt.Sprintf("for mapping %s/%s [%s]", - c.mapping.Namespace, c.mapping.Name, protoTcp)).Return(nil). - On("DeleteByComment", natTableName, preroutingChain, fmt.Sprintf("for mapping %s/%s [%s]", - c.mapping.Namespace, c.mapping.Name, protoUdp)).Return(nil). + On("DeleteByComment", natTableName, preroutingChain, fmt.Sprintf("for mapping %s/%s", + c.mapping.Namespace, c.mapping.Name)).Return(nil). On("DeleteChain", natTableName, c.customChain).Return(nil) - dnat := mapping.NewThroughServiceDNATProvider(iptables, mapping.NewNamer()) + ipset := &ntmocks.IPSetHelper{} + ipset.On("DeleteSet", c.ipset).Return(nil) + dnat := mapping.NewThroughServiceDNATProvider(iptables, ipset, mapping.NewNamer()) // call DeleteDNAT err := dnat.DeleteDNAT(c.externalIP, c.mapping) assert.Nil(t, err) @@ -224,15 +248,14 @@ func Test_throughServiceDNATProvider_DeleteDNAT(t *testing.T) { } } -func getIptablesProtoJumpRule(extIP, proto, allowed, customChain string, dport int32, +func getIptablesProtoJumpRule(extIP, customChain, ipset string, meta v1meta.ObjectMeta) nettools.IPTablesRuleArgs { return nettools.IPTablesRuleArgs{ Table: natTableName, ChainName: preroutingChain, - Selector: []string{"-d", fmt.Sprintf("%s/32", extIP), "-p", proto, - "-m", "multiport", "--dports", fmt.Sprintf("%d", dport), "-s", allowed}, - Action: []string{customChain}, - Comment: fmt.Sprintf("for mapping %s/%s [%s]", meta.Namespace, meta.Name, proto), + Selector: []string{"-d", fmt.Sprintf("%s/32", extIP), "-m", "set", "--match-set", ipset, "src,dst"}, + Action: []string{customChain}, + Comment: fmt.Sprintf("for mapping %s/%s", meta.Namespace, meta.Name), } } diff --git a/pkg/controller/mapping/mapping_controller.go b/pkg/controller/mapping/mapping_controller.go index 5da9878..2730645 100644 --- a/pkg/controller/mapping/mapping_controller.go +++ b/pkg/controller/mapping/mapping_controller.go @@ -67,7 +67,8 @@ func Add(mgr manager.Manager, heartbeatChan chan<- string) error { } iptablesHelper := nettools.NewExecIPTablesHelper(executor, time.Duration(cfg.IPTablesTimeoutSec)*time.Second) namer := NewNamer() - dnatProvider := NewThroughServiceDNATProvider(iptablesHelper, namer) + ipsetHelper := nettools.NewExecIPSetHelper(executor) + dnatProvider := NewThroughServiceDNATProvider(iptablesHelper, ipsetHelper, namer) ipTablesHelper, err := NewIPTablesHelper(dnatProvider, iptablesHelper, namer, interfaceProvider, cfg.SetupMasquerade, cfg.SetupSNAT) if err != nil { @@ -75,9 +76,8 @@ func Add(mgr manager.Manager, heartbeatChan chan<- string) error { } routeIoOp := nettools.NewIOSimpleFileOperator() ipRouteHelper := NewIPRouteSmartNatHelper(executor, routeIoOp, interfaceProvider, refreshInterval, cfg.GwAddressOffset) - scrubber := NewScrubber(interfaceProvider) + scrubber := NewScrubber(interfaceProvider, cfg) conntrackHelper := nettools.NewExecConntrackHelper(executor) - ipsetHelper := nettools.NewExecIPSetHelper(executor) syncer := NewSyncer(namer, interfaceProvider, ipRouteHelper, conntrackHelper, ipTablesHelper, ipsetHelper, cfg.SetupSNAT, cfg.SetupMasquerade) return add(mgr, newReconciler(mgr, cfg, interfaceProvider, ipRouteHelper, syncer, scrubber, heartbeatChan)) @@ -622,6 +622,7 @@ func (r *ReconcileMapping) checkSetupFinalizer(mapping *smartnatv1alpha1.Mapping return false, false, nil } +// GetHeartbeatString returns string printed by heartbeat http request func GetHeartbeatString() string { return fmt.Sprintf("OK %v", time.Now().UTC().Format(time.RFC3339)) } diff --git a/pkg/controller/mapping/scrubber.go b/pkg/controller/mapping/scrubber.go index 88f0ca2..e4c517c 100644 --- a/pkg/controller/mapping/scrubber.go +++ b/pkg/controller/mapping/scrubber.go @@ -22,14 +22,13 @@ import ( "github.com/DevFactory/go-tools/pkg/nettools" "github.com/DevFactory/smartnat/pkg/apis/smartnat/v1alpha1" smartnatv1alpha1 "github.com/DevFactory/smartnat/pkg/apis/smartnat/v1alpha1" + "github.com/DevFactory/smartnat/pkg/config" v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/util/validation" ) const ( - pfx = "SmartNat validation failed: " - // MaxPortsPerProtocol - iptables multiport extension supports max 15 ports as arguments - MaxPortsPerProtocol = 15 + pfx = "SmartNat validation failed: " defaultInvalidOKStatus = "no" ) @@ -49,12 +48,14 @@ type Scrubber interface { type scrubber struct { provider nettools.InterfaceProvider + config *config.Config } // NewScrubber returns a scrubber for SmartNat objects -func NewScrubber(interfaceProvider nettools.InterfaceProvider) Scrubber { +func NewScrubber(interfaceProvider nettools.InterfaceProvider, cfg *config.Config) Scrubber { return &scrubber{ provider: interfaceProvider, + config: cfg, } } @@ -219,15 +220,9 @@ func (s *scrubber) validatePorts(mapping *v1alpha1.Mapping) (bool, string) { } usedPorts[key] = true } - if tcpCount > MaxPortsPerProtocol { - msg := fmt.Sprintf("Too many TCP ports configured. Max is %d, found %d", - MaxPortsPerProtocol, tcpCount) - logrusWithMapping(mapping).Info(pfx + msg) - return false, msg - } - if udpCount > MaxPortsPerProtocol { - msg := fmt.Sprintf("Too many UDP ports configured. Max is %d, found %d", - MaxPortsPerProtocol, udpCount) + if tcpCount+udpCount > s.config.MaxPortsPerMapping { + msg := fmt.Sprintf("Too many ports configured. Max is %d, found %d", s.config.MaxPortsPerMapping, + tcpCount+udpCount) logrusWithMapping(mapping).Info(pfx + msg) return false, msg } diff --git a/pkg/controller/mapping/scrubber_test.go b/pkg/controller/mapping/scrubber_test.go index b6511fb..5b57566 100644 --- a/pkg/controller/mapping/scrubber_test.go +++ b/pkg/controller/mapping/scrubber_test.go @@ -19,12 +19,14 @@ import ( ntmocks "github.com/DevFactory/go-tools/pkg/nettools/mocks" "github.com/DevFactory/smartnat/pkg/apis/smartnat/v1alpha1" "github.com/DevFactory/smartnat/pkg/controller/mapping" + "github.com/DevFactory/smartnat/pkg/controller/mapping/testhelpers" "github.com/stretchr/testify/assert" v1 "k8s.io/api/core/v1" v1meta "k8s.io/apimachinery/pkg/apis/meta/v1" ) func Test_scrubber_ScrubMapping(t *testing.T) { + cfg := testhelpers.GetTestConfig(t) // OK uses all features valid1 := &v1alpha1.Mapping{ ObjectMeta: v1meta.ObjectMeta{ @@ -118,19 +120,17 @@ func Test_scrubber_ScrubMapping(t *testing.T) { }, } // wrong - uses to many ports per protocol - wrongTooManyTcpPorts := valid1.DeepCopy() - wrongTooManyUdpPorts := valid1.DeepCopy() - var i int32 - for i = 0; i < mapping.MaxPortsPerProtocol+1; i++ { - wrongTooManyTcpPorts.Spec.Ports = append(wrongTooManyTcpPorts.Spec.Ports, v1alpha1.MappingPort{ - Port: i + 2000, - Protocol: "tcp", - ServicePort: i + 2000, - }) - wrongTooManyUdpPorts.Spec.Ports = append(wrongTooManyUdpPorts.Spec.Ports, v1alpha1.MappingPort{ - Port: i + 2000, - Protocol: "udp", - ServicePort: i + 2000, + wrongTooManyPorts := valid1.DeepCopy() + for i := 0; i < cfg.MaxPortsPerMapping+1; i++ { + num := int32(i + 2000) + proto := "tcp" + if num%2 == 0 { + proto = "udp" + } + wrongTooManyPorts.Spec.Ports = append(wrongTooManyPorts.Spec.Ports, v1alpha1.MappingPort{ + Port: num, + Protocol: proto, + ServicePort: num, }) } @@ -227,13 +227,8 @@ func Test_scrubber_ScrubMapping(t *testing.T) { want: false, }, { - name: "Fails because uses too many TCP ports", - sn: wrongTooManyTcpPorts, - want: false, - }, - { - name: "Fails because uses too many UDP ports", - sn: wrongTooManyUdpPorts, + name: "Fails because uses too many ports", + sn: wrongTooManyPorts, want: false, }, } @@ -244,7 +239,7 @@ func Test_scrubber_ScrubMapping(t *testing.T) { t.Logf("Cannot create ifaceProvider object: %v", err) t.Fail() } - s := mapping.NewScrubber(ifaceProvider) + s := mapping.NewScrubber(ifaceProvider, cfg) valid, _, _, _ := s.ScrubMapping(tt.sn, tt.others) if valid != tt.want { t.Errorf("scrubber.ScrubMapping() got = %v, want %v", valid, tt.want) @@ -260,6 +255,7 @@ func Test_scrubber_ScrubMapping(t *testing.T) { } func Test_scrubber_ValidateEndpoints(t *testing.T) { + cfg := testhelpers.GetTestConfig(t) tests := []struct { name string endpoints *v1.Endpoints @@ -316,7 +312,7 @@ func Test_scrubber_ValidateEndpoints(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { provider, _, _ := ntmocks.NewMockInterfaceProvider(".*", false) - s := mapping.NewScrubber(provider) + s := mapping.NewScrubber(provider, cfg) if err := s.ValidateEndpoints(nil, tt.endpoints); (err != nil) != tt.wantErr { t.Errorf("scrubber.ValidateEndpoints() error = %v, wantErr %v", err, tt.wantErr) } diff --git a/pkg/controller/mapping/testhelper_reconciler_deps.go b/pkg/controller/mapping/testhelper_reconciler_deps.go index c3421f8..63deb59 100644 --- a/pkg/controller/mapping/testhelper_reconciler_deps.go +++ b/pkg/controller/mapping/testhelper_reconciler_deps.go @@ -67,7 +67,7 @@ func getReconcilerDependencies(t *testing.T) *reconcilerDependencies { ipRouteSmartNatHelperExecutor: execMock, ipRouteSmartNatHelperFileOperator: ioOp, ipRouteSmartNatHelperUpdateChan: routeUpdateChan, - scrubber: NewScrubber(ifaceProvider), + scrubber: NewScrubber(ifaceProvider, cfg), mappingSyncer: &mocks.Syncer{}, heartbeatChan: heartbeatChan, } diff --git a/pkg/controller/mapping/testhelpers/config.go b/pkg/controller/mapping/testhelpers/config.go index bcec7f1..c902ecf 100644 --- a/pkg/controller/mapping/testhelpers/config.go +++ b/pkg/controller/mapping/testhelpers/config.go @@ -20,7 +20,7 @@ import ( ) func GetTestConfig(t *testing.T) *config.Config { - cfg, err := config.NewConfigFromArgs("eth[0-9]+", true, false, "eth0", 10, true, 10, 60, 1, 8080) + cfg, err := config.NewConfigFromArgs("eth[0-9]+", true, false, "eth0", 10, true, 10, 60, 1, 8080, 50) if err != nil { t.Logf("Cannot create configuration object: %v", err) t.Fail()