Merge pull request #33587 from bprashanth/esipp-nodeport-2

pkg/proxy/iptables/proxier.go

@@ -177,6 +177,7 @@ type Proxier struct {
 	clusterCIDR    string
 	hostname       string
 	nodeIP         net.IP
+	portMapper     portOpener
 }
 
 type localPort struct {
@@ -194,6 +195,20 @@ type closeable interface {
 	Close() error
 }
 
+// portOpener is an interface around port opening/closing.
+// Abstracted out for testing.
+type portOpener interface {
+	OpenLocalPort(lp *localPort) (closeable, error)
+}
+
+// listenPortOpener opens ports by calling bind() and listen().
+type listenPortOpener struct{}
+
+// OpenLocalPort holds the given local port open.
+func (l *listenPortOpener) OpenLocalPort(lp *localPort) (closeable, error) {
+	return openLocalPort(lp)
+}
+
 // Proxier implements ProxyProvider
 var _ proxy.ProxyProvider = &Proxier{}
 
@@ -241,6 +256,7 @@ func NewProxier(ipt utiliptables.Interface, sysctl utilsysctl.Interface, exec ut
 		clusterCIDR:    clusterCIDR,
 		hostname:       hostname,
 		nodeIP:         nodeIP,
+		portMapper:     &listenPortOpener{},
 	}, nil
 }
 
@@ -941,7 +957,7 @@ func (proxier *Proxier) syncProxyRules() {
 					glog.V(4).Infof("Port %s was open before and is still needed", lp.String())
 					replacementPortsMap[lp] = proxier.portsMap[lp]
 				} else {
-					socket, err := openLocalPort(&lp)
+					socket, err := proxier.portMapper.OpenLocalPort(&lp)
 					if err != nil {
 						glog.Errorf("can't open %s, skipping this externalIP: %v", lp.String(), err)
 						continue
@@ -1056,7 +1072,7 @@ func (proxier *Proxier) syncProxyRules() {
 				glog.V(4).Infof("Port %s was open before and is still needed", lp.String())
 				replacementPortsMap[lp] = proxier.portsMap[lp]
 			} else {
-				socket, err := openLocalPort(&lp)
+				socket, err := proxier.portMapper.OpenLocalPort(&lp)
 				if err != nil {
 					glog.Errorf("can't open %s, skipping this nodePort: %v", lp.String(), err)
 					continue
@@ -1070,10 +1086,16 @@ func (proxier *Proxier) syncProxyRules() {
 				"-m", protocol, "-p", protocol,
 				"--dport", fmt.Sprintf("%d", svcInfo.nodePort),
 			}
-			// Nodeports need SNAT.
-			writeLine(natRules, append(args, "-j", string(KubeMarkMasqChain))...)
-			// Jump to the service chain.
-			writeLine(natRules, append(args, "-j", string(svcChain))...)
+			if !svcInfo.onlyNodeLocalEndpoints {
+				// Nodeports need SNAT, unless they're local.
+				writeLine(natRules, append(args, "-j", string(KubeMarkMasqChain))...)
+				// Jump to the service chain.
+				writeLine(natRules, append(args, "-j", string(svcChain))...)
+			} else {
+				// TODO: Make all nodePorts jump to the firewall chain.
+				// Currently we only create it for loadbalancers (#33586).
+				writeLine(natRules, append(args, "-j", string(svcXlbChain))...)
+			}
 		}
 
 		// If the service has no endpoints then reject packets.
@@ -1173,6 +1195,16 @@ func (proxier *Proxier) syncProxyRules() {
 				localEndpointChains = append(localEndpointChains, endpointChains[i])
 			}
 		}
+		// First rule in the chain redirects all pod -> external vip traffic to the
+		// Service's ClusterIP instead. This happens whether or not we have local
+		// endpoints.
+		args = []string{
+			"-A", string(svcXlbChain),
+			"-m", "comment", "--comment",
+			fmt.Sprintf(`"Redirect pods trying to reach external loadbalancer VIP to clusterIP"`),
+		}
+		writeLine(natRules, append(args, "-s", proxier.clusterCIDR, "-j", string(svcChain))...)
+
 		numLocalEndpoints := len(localEndpointChains)
 		if numLocalEndpoints == 0 {
 			// Blackhole all traffic since there are no local endpoints

pkg/util/iptables/testing/fake.go

@@ -16,60 +16,107 @@ limitations under the License.
 
 package testing
 
-import "k8s.io/kubernetes/pkg/util/iptables"
+import (
+	"fmt"
+	"strings"
+
+	"k8s.io/kubernetes/pkg/util/iptables"
+)
+
+const (
+	Destination = "-d "
+	Source      = "-s "
+	DPort       = "--dport "
+	Protocol    = "-p "
+	Jump        = "-j "
+	Reject      = "REJECT"
+	ToDest      = "--to-destination "
+)
+
+type Rule map[string]string
 
 // no-op implementation of iptables Interface
-type fake struct{}
+type FakeIPTables struct {
+	Lines []byte
+}
 
-func NewFake() *fake {
-	return &fake{}
+func NewFake() *FakeIPTables {
+	return &FakeIPTables{}
 }
 
-func (*fake) GetVersion() (string, error) {
+func (*FakeIPTables) GetVersion() (string, error) {
 	return "0.0.0", nil
 }
 
-func (*fake) EnsureChain(table iptables.Table, chain iptables.Chain) (bool, error) {
+func (*FakeIPTables) EnsureChain(table iptables.Table, chain iptables.Chain) (bool, error) {
 	return true, nil
 }
 
-func (*fake) FlushChain(table iptables.Table, chain iptables.Chain) error {
+func (*FakeIPTables) FlushChain(table iptables.Table, chain iptables.Chain) error {
 	return nil
 }
 
-func (*fake) DeleteChain(table iptables.Table, chain iptables.Chain) error {
+func (*FakeIPTables) DeleteChain(table iptables.Table, chain iptables.Chain) error {
 	return nil
 }
 
-func (*fake) EnsureRule(position iptables.RulePosition, table iptables.Table, chain iptables.Chain, args ...string) (bool, error) {
+func (*FakeIPTables) EnsureRule(position iptables.RulePosition, table iptables.Table, chain iptables.Chain, args ...string) (bool, error) {
 	return true, nil
 }
 
-func (*fake) DeleteRule(table iptables.Table, chain iptables.Chain, args ...string) error {
+func (*FakeIPTables) DeleteRule(table iptables.Table, chain iptables.Chain, args ...string) error {
 	return nil
 }
 
-func (*fake) IsIpv6() bool {
+func (*FakeIPTables) IsIpv6() bool {
 	return false
 }
 
-func (*fake) Save(table iptables.Table) ([]byte, error) {
+func (*FakeIPTables) Save(table iptables.Table) ([]byte, error) {
 	return make([]byte, 0), nil
 }
 
-func (*fake) SaveAll() ([]byte, error) {
+func (*FakeIPTables) SaveAll() ([]byte, error) {
 	return make([]byte, 0), nil
 }
 
-func (*fake) Restore(table iptables.Table, data []byte, flush iptables.FlushFlag, counters iptables.RestoreCountersFlag) error {
+func (*FakeIPTables) Restore(table iptables.Table, data []byte, flush iptables.FlushFlag, counters iptables.RestoreCountersFlag) error {
 	return nil
 }
 
-func (*fake) RestoreAll(data []byte, flush iptables.FlushFlag, counters iptables.RestoreCountersFlag) error {
+func (f *FakeIPTables) RestoreAll(data []byte, flush iptables.FlushFlag, counters iptables.RestoreCountersFlag) error {
+	f.Lines = data
 	return nil
 }
-func (*fake) AddReloadFunc(reloadFunc func()) {}
+func (*FakeIPTables) AddReloadFunc(reloadFunc func()) {}
 
-func (*fake) Destroy() {}
+func (*FakeIPTables) Destroy() {}
+
+func getToken(line, seperator string) string {
+	tokens := strings.Split(line, seperator)
+	if len(tokens) == 2 {
+		return strings.Split(tokens[1], " ")[0]
+	}
+	return ""
+}
+
+// GetChain returns a list of rules for the givne chain.
+// The chain name must match exactly.
+// The matching is pretty dumb, don't rely on it for anything but testing.
+func (f *FakeIPTables) GetRules(chainName string) (rules []Rule) {
+	for _, l := range strings.Split(string(f.Lines), "\n") {
+		if strings.Contains(l, fmt.Sprintf("-A %v", chainName)) {
+			newRule := Rule(map[string]string{})
+			for _, arg := range []string{Destination, Source, DPort, Protocol, Jump, ToDest} {
+				tok := getToken(l, arg)
+				if tok != "" {
+					newRule[arg] = tok
+				}
+			}
+			rules = append(rules, newRule)
+		}
+	}
+	return
+}
 
-var _ = iptables.Interface(&fake{})
+var _ = iptables.Interface(&FakeIPTables{})