Enable hairpin for kube-router by default

We agreed that the most logical decision was to enable hairpin by
default. The old configuration doesn't reflect our real needs, for this
reason we decided to deprecate the old configuration and add a new
configuration. This commit handles all these details.

Fixes #1953

Co-authored-by: Tom Wieczorek <twz123@users.noreply.github.com>
Signed-off-by: Juan-Luis de Sousa-Valadas Castaño <jvaladas@mirantis.com>

docs/configuration.md

@@ -65,7 +65,7 @@ spec:
       peerRouterIPs: ""
       peerRouterASNs: ""
       autoMTU: true
-      hairpinMode: false
+      hairpin: enabled 
     kubeProxy:
       disabled: false
       mode: iptables
@@ -209,14 +209,15 @@ CALICO_IPV6POOL_CIDR: "{{ spec.network.dualStack.IPv6podCIDR }}"
 
 #### `spec.network.kuberouter`
 
-| Element          | Description                                                                                                                                        |
-| ---------------- |----------------------------------------------------------------------------------------------------------------------------------------------------|
-| `autoMTU`        | Autodetection of used MTU (default: `true`).                                                                                                       |
-| `mtu`            | Override MTU setting, if `autoMTU` must be set to `false`).                                                                                        |
-| `metricsPort`    | Kube-router metrics server port. Set to 0 to disable metrics  (default: `8080`).                                                                   |
-| `peerRouterIPs`  | Comma-separated list of [global peer addresses](https://github.com/cloudnativelabs/kube-router/blob/master/docs/bgp.md#global-external-bgp-peers). |
-| `peerRouterASNs` | Comma-separated list of [global peer ASNs](https://github.com/cloudnativelabs/kube-router/blob/master/docs/bgp.md#global-external-bgp-peers).      |
-| `hairpinMode`    | Activate hairpinMode (default: `false`) (https://github.com/cloudnativelabs/kube-router/blob/master/docs/user-guide.md#hairpin-mode)               |
+| Element          | Description                                                                                                                                                                                                                                                                                   |
+| ---------------- |-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
+| `autoMTU`        | Autodetection of used MTU (default: `true`).                                                                                                                                                                                                                                                  |
+| `mtu`            | Override MTU setting, if `autoMTU` must be set to `false`).                                                                                                                                                                                                                                   |
+| `metricsPort`    | Kube-router metrics server port. Set to 0 to disable metrics  (default: `8080`).                                                                                                                                                                                                              |
+| `peerRouterIPs`  | Comma-separated list of [global peer addresses](https://github.com/cloudnativelabs/kube-router/blob/master/docs/bgp.md#global-external-bgp-peers).                                                                                                                                            |
+| `peerRouterASNs` | Comma-separated list of [global peer ASNs](https://github.com/cloudnativelabs/kube-router/blob/master/docs/bgp.md#global-external-bgp-peers).                                                                                                                                                 |
+| `hairpin`        | Hairpin mode, supported modes `enabled`: enabled cluster wide, `allowed`: must be allowed per service [using annotations](https://github.com/cloudnativelabs/kube-router/blob/master/docs/user-guide.md#hairpin-mode), `disabled`: doesn't work at all. If empty falls back to `hairpinMode`  |
+| `hairpinMode`    | **Deprecated** Use hairpin instead Activate hairpinMode (default: `false`) (https://github.com/cloudnativelabs/kube-router/blob/master/docs/user-guide.md#hairpin-mode)                                                                                                                       |
 
 **Note**: Kube-router allows many networking aspects to be configured per node, service, and pod (for more information, refer to the [Kube-router user guide](https://github.com/cloudnativelabs/kube-router/blob/master/docs/user-guide.md)).
 

inttest/kuberouter/kuberouter_hairpin_test.go

@@ -100,8 +100,6 @@ const k0sConfigWithHairpinning = `
 spec:
   network:
     provider: kuberouter
-    kuberouter:
-      hairpinMode: true
 `
 
 const podManifest = `

pkg/apis/k0s.k0sproject.io/v1beta1/kuberouter.go

@@ -16,6 +16,12 @@ limitations under the License.
 
 package v1beta1
 
+import (
+	"encoding/json"
+	"fmt"
+	"strings"
+)
+
 // KubeRouter defines the kube-router related config options
 type KubeRouter struct {
 	// Auto-detection of used MTU (default: true)
@@ -24,19 +30,82 @@ type KubeRouter struct {
 	MTU int `json:"mtu"`
 	// Kube-router metrics server port. Set to 0 to disable metrics  (default: 8080)
 	MetricsPort int `json:"metricsPort"`
-	// Activate Hairpin Mode (allow a Pod behind a Service to communicate to its own ClusterIP:Port)
-	HairpinMode bool `json:"hairpinMode"`
+	// Admits three values: "enabled" or "true" enables it globaly, "allowed" allows but services must be annotated explictly and "disabled" or "false"
+	HairpinMode HairpinMode `json:"hairpinMode"`
 	// Comma-separated list of global peer addresses
 	PeerRouterASNs string `json:"peerRouterASNs"`
 	// Comma-separated list of global peer ASNs
 	PeerRouterIPs string `json:"peerRouterIPs"`
 }
 
+type HairpinModeValues string
+
+const (
+	HairpinEnabled   HairpinModeValues = "Enabled"
+	HairpinAllowed   HairpinModeValues = "Allowed"
+	HairpinDisabled  HairpinModeValues = "Disabled"
+	HairpinUndefined HairpinModeValues = ""
+)
+
+// +kubebuilder:validation:Enum=Enabled;Allowed;Disabled;true;false
+type HairpinMode struct {
+	Value HairpinModeValues `json:"value"`
+}
+
+func (HairpinMode) OpenAPISchemaType() []string { return []string{"string"} }
+
+func (HairpinMode) OpenAPISchemaFormat() string { return "" }
+
+func (HairpinMode) OpenAPIV3OneOfTypes() []string { return []string{"bool", "string"} }
+
+// UnmarshalJSON implements the json.Unmarshaller interface.
+func (h *HairpinMode) UnmarshalJSON(value []byte) error {
+	if value[0] == '"' {
+		var stringVal *string
+		err := json.Unmarshal(value, &stringVal)
+		if err != nil {
+			return err
+		}
+
+		if strings.EqualFold(*stringVal, string(HairpinEnabled)) || len(strings.TrimSpace(*stringVal)) == 0 {
+			h.Value = HairpinEnabled
+		} else if strings.EqualFold(*stringVal, string(HairpinAllowed)) {
+			h.Value = HairpinAllowed
+		} else if strings.EqualFold(*stringVal, string(HairpinDisabled)) {
+			h.Value = HairpinDisabled
+		} else {
+			return fmt.Errorf("HairpinMode has an unexpected value: %s", string(value))
+		}
+
+		return nil
+	}
+
+	var boolVal *bool
+
+	err := json.Unmarshal(value, &boolVal)
+	if err != nil {
+		return err
+	}
+	if *boolVal {
+		h.Value = HairpinEnabled
+	} else {
+		h.Value = HairpinDisabled
+	}
+
+	return nil
+}
+
+// MarshalJSON implements the json.Marshaller interface.
+func (h HairpinMode) MarshalJSON() ([]byte, error) {
+	return json.Marshal(h.Value)
+}
+
 // DefaultKubeRouter returns the default config for kube-router
 func DefaultKubeRouter() *KubeRouter {
 	return &KubeRouter{
 		MTU:         0,
 		AutoMTU:     true,
 		MetricsPort: 8080,
+		HairpinMode: HairpinMode{Value: HairpinEnabled},
 	}
 }

pkg/apis/k0s.k0sproject.io/v1beta1/kuberouter_test.go

@@ -0,0 +1,82 @@
+/*
+Copyright 2022 k0s authors
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package v1beta1
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/suite"
+)
+
+type KubeRouterSuite struct {
+	suite.Suite
+}
+
+func TestUnmarshalJSON(t *testing.T) {
+
+	type unmarshalTestCases struct {
+		input          string
+		expectedOutput HairpinModeValues
+		expectsError   bool
+	}
+
+	tests := []unmarshalTestCases{
+		{
+			input:          "\"enabled\"",
+			expectedOutput: HairpinEnabled,
+			expectsError:   false,
+		},
+		{
+			input:          "\"ALLOWED\"",
+			expectedOutput: HairpinAllowed,
+			expectsError:   false,
+		},
+		{
+			input:          "\"DiSaBlEd\"",
+			expectedOutput: HairpinDisabled,
+			expectsError:   false,
+		},
+		{
+			input:        "\"true\"",
+			expectsError: true,
+		},
+		{
+			input:          "true",
+			expectedOutput: HairpinEnabled,
+			expectsError:   false,
+		},
+		{
+			input:          "false",
+			expectedOutput: HairpinDisabled,
+			expectsError:   false,
+		},
+	}
+
+	for _, tc := range tests {
+		var hm HairpinMode
+		err := hm.UnmarshalJSON([]byte(tc.input))
+		if tc.expectedOutput != hm.Value {
+			t.Fatalf("Testcase (%#v) expected %s and got %s", tc, tc.expectedOutput, hm.Value)
+		}
+
+		if tc.expectsError && err == nil {
+			t.Fatalf("Testcase (%#v) was expected to fail ", tc)
+		} else if !tc.expectsError && err != nil {
+			t.Fatalf("Testcase (%#v) was expected to fail with error: (%#v) ", tc, err)
+		}
+	}
+}

pkg/component/controller/kuberouter.go

@@ -48,7 +48,8 @@ type kubeRouterConfig struct {
 	MetricsPort       int
 	CNIInstallerImage string
 	CNIImage          string
-	HairpinMode       bool
+	GlobalHairpin     bool
+	CNIHairpin        bool
 	PeerRouterIPs     string
 	PeerRouterASNs    string
 	PullPolicy        string
@@ -70,6 +71,23 @@ func (k *KubeRouter) Init(_ context.Context) error { return nil }
 // Stop no-op as nothing running
 func (k *KubeRouter) Stop() error { return nil }
 
+func applyHairpinConfig(cfg *kubeRouterConfig, krc *v1beta1.KubeRouter) error {
+	switch krc.HairpinMode.Value {
+	case v1beta1.HairpinEnabled:
+		cfg.CNIHairpin = true
+		cfg.GlobalHairpin = true
+	case v1beta1.HairpinAllowed:
+		cfg.CNIHairpin = true
+		cfg.GlobalHairpin = false
+	case v1beta1.HairpinDisabled:
+		cfg.CNIHairpin = false
+		cfg.GlobalHairpin = false
+	default:
+		return fmt.Errorf("HairpinMode has an unexpected string value: '%s'", krc.HairpinMode.Value)
+	}
+	return nil
+}
+
 // Reconcile detects changes in configuration and applies them to the component
 func (k *KubeRouter) Reconcile(_ context.Context, clusterConfig *v1beta1.ClusterConfig) error {
 	logrus.Debug("reconcile method called for: KubeRouter")
@@ -88,11 +106,14 @@ func (k *KubeRouter) Reconcile(_ context.Context, clusterConfig *v1beta1.Cluster
 		MetricsPort:       clusterConfig.Spec.Network.KubeRouter.MetricsPort,
 		PeerRouterIPs:     clusterConfig.Spec.Network.KubeRouter.PeerRouterIPs,
 		PeerRouterASNs:    clusterConfig.Spec.Network.KubeRouter.PeerRouterASNs,
-		HairpinMode:       clusterConfig.Spec.Network.KubeRouter.HairpinMode,
 		CNIImage:          clusterConfig.Spec.Images.KubeRouter.CNI.URI(),
 		CNIInstallerImage: clusterConfig.Spec.Images.KubeRouter.CNIInstaller.URI(),
 		PullPolicy:        clusterConfig.Spec.Images.DefaultPullPolicy,
 	}
+	err := applyHairpinConfig(&cfg, clusterConfig.Spec.Network.KubeRouter)
+	if err != nil {
+		return err
+	}
 
 	if cfg == k.previousConfig {
 		k.log.Info("config matches with previous, not reconciling anything")
@@ -106,7 +127,7 @@ func (k *KubeRouter) Reconcile(_ context.Context, clusterConfig *v1beta1.Cluster
 		Data:     cfg,
 	}
 
-	err := tw.WriteToBuffer(output)
+	err = tw.WriteToBuffer(output)
 	if err != nil {
 		return fmt.Errorf("error writing kube-router manifests, will NOT retry: %w", err)
 	}
@@ -150,7 +171,7 @@ data:
              "auto-mtu": {{ .AutoMTU }},
              "bridge":"kube-bridge",
              "isDefaultGateway":true,
-             "hairpinMode": {{ .HairpinMode }},
+             "hairpinMode": {{ .CNIHairpin }},
              "ipam":{
                 "type":"host-local"
              }
@@ -259,7 +280,7 @@ spec:
         - "--run-service-proxy=false"
         - "--bgp-graceful-restart=true"
         - "--metrics-port={{ .MetricsPort }}"
-        - "--hairpin-mode={{ .HairpinMode }}"
+        - "--hairpin-mode={{ .GlobalHairpin }}"
         {{- if .PeerRouterIPs }}
         - "--peer-router-ips={{ .PeerRouterIPs }}"
         {{- end }}