Merge pull request #51500 from m1093782566/fix-kube-proxy-panic

Automatic merge from submit-queue (batch tested with PRs 51819, 51706, 51761, 51818, 51500)

fix kube-proxy panic because of nil sessionAffinityConfig

**What this PR does / why we need it**:

fix kube-proxy panic because of nil sessionAffinityConfig

**Which issue this PR fixes**: closes #51499 

**Special notes for your reviewer**:

I apology that this bug is introduced by #49850 :(

@thockin @smarterclayton @gnufied 

**Release note**:

```release-note
NONE
```

pkg/api/fuzzer/fuzzer.go

@@ -439,6 +439,19 @@ var Funcs = func(codecs runtimeserializer.CodecFactory) []interface{} {
 					ss.Ports[i].TargetPort.StrVal = "x" + ss.Ports[i].TargetPort.StrVal // non-empty
 				}
 			}
+			types := []api.ServiceAffinity{api.ServiceAffinityNone, api.ServiceAffinityClientIP}
+			ss.SessionAffinity = types[c.Rand.Intn(len(types))]
+			switch ss.SessionAffinity {
+			case api.ServiceAffinityClientIP:
+				timeoutSeconds := int32(c.Rand.Intn(int(api.MaxClientIPServiceAffinitySeconds)))
+				ss.SessionAffinityConfig = &api.SessionAffinityConfig{
+					ClientIP: &api.ClientIPConfig{
+						TimeoutSeconds: &timeoutSeconds,
+					},
+				}
+			case api.ServiceAffinityNone:
+				ss.SessionAffinityConfig = nil
+			}
 		},
 		func(n *api.Node, c fuzz.Continue) {
 			c.FuzzNoCustom(n)

pkg/api/v1/defaults.go

@@ -101,6 +101,19 @@ func SetDefaults_Service(obj *v1.Service) {
 	if obj.Spec.SessionAffinity == "" {
 		obj.Spec.SessionAffinity = v1.ServiceAffinityNone
 	}
+	if obj.Spec.SessionAffinity == v1.ServiceAffinityNone {
+		obj.Spec.SessionAffinityConfig = nil
+	}
+	if obj.Spec.SessionAffinity == v1.ServiceAffinityClientIP {
+		if obj.Spec.SessionAffinityConfig == nil || obj.Spec.SessionAffinityConfig.ClientIP == nil || obj.Spec.SessionAffinityConfig.ClientIP.TimeoutSeconds == nil {
+			timeoutSeconds := v1.DefaultClientIPServiceAffinitySeconds
+			obj.Spec.SessionAffinityConfig = &v1.SessionAffinityConfig{
+				ClientIP: &v1.ClientIPConfig{
+					TimeoutSeconds: &timeoutSeconds,
+				},
+			}
+		}
+	}
 	if obj.Spec.Type == "" {
 		obj.Spec.Type = v1.ServiceTypeClusterIP
 	}

pkg/api/v1/defaults_test.go

@@ -667,11 +667,53 @@ func TestSetDefaultService(t *testing.T) {
 	if svc2.Spec.SessionAffinity != v1.ServiceAffinityNone {
 		t.Errorf("Expected default session affinity type:%s, got: %s", v1.ServiceAffinityNone, svc2.Spec.SessionAffinity)
 	}
+	if svc2.Spec.SessionAffinityConfig != nil {
+		t.Errorf("Expected empty session affinity config when session affinity type: %s, got: %v", v1.ServiceAffinityNone, svc2.Spec.SessionAffinityConfig)
+	}
 	if svc2.Spec.Type != v1.ServiceTypeClusterIP {
 		t.Errorf("Expected default type:%s, got: %s", v1.ServiceTypeClusterIP, svc2.Spec.Type)
 	}
 }
 
+func TestSetDefaultServiceSessionAffinityConfig(t *testing.T) {
+	testCases := map[string]v1.Service{
+		"SessionAffinityConfig is empty": {
+			Spec: v1.ServiceSpec{
+				SessionAffinity:       v1.ServiceAffinityClientIP,
+				SessionAffinityConfig: nil,
+			},
+		},
+		"ClientIP is empty": {
+			Spec: v1.ServiceSpec{
+				SessionAffinity: v1.ServiceAffinityClientIP,
+				SessionAffinityConfig: &v1.SessionAffinityConfig{
+					ClientIP: nil,
+				},
+			},
+		},
+		"TimeoutSeconds is empty": {
+			Spec: v1.ServiceSpec{
+				SessionAffinity: v1.ServiceAffinityClientIP,
+				SessionAffinityConfig: &v1.SessionAffinityConfig{
+					ClientIP: &v1.ClientIPConfig{
+						TimeoutSeconds: nil,
+					},
+				},
+			},
+		},
+	}
+	for name, test := range testCases {
+		obj2 := roundTrip(t, runtime.Object(&test))
+		svc2 := obj2.(*v1.Service)
+		if svc2.Spec.SessionAffinityConfig == nil || svc2.Spec.SessionAffinityConfig.ClientIP == nil || svc2.Spec.SessionAffinityConfig.ClientIP.TimeoutSeconds == nil {
+			t.Fatalf("Case: %s, unexpected empty SessionAffinityConfig/ClientIP/TimeoutSeconds when session affinity type: %s, got: %v", name, v1.ServiceAffinityClientIP, svc2.Spec.SessionAffinityConfig)
+		}
+		if *svc2.Spec.SessionAffinityConfig.ClientIP.TimeoutSeconds != v1.DefaultClientIPServiceAffinitySeconds {
+			t.Errorf("Case: %s, default TimeoutSeconds should be %d when session affinity type: %s, got: %d", name, v1.DefaultClientIPServiceAffinitySeconds, v1.ServiceAffinityClientIP, *svc2.Spec.SessionAffinityConfig.ClientIP.TimeoutSeconds)
+		}
+	}
+}
+
 func TestSetDefaultSecretVolumeSource(t *testing.T) {
 	s := v1.PodSpec{}
 	s.Volumes = []v1.Volume{

pkg/master/controller.go

@@ -250,7 +250,6 @@ func (c *Controller) CreateOrUpdateMasterServiceIfNeeded(serviceName string, ser
 		}
 		return nil
 	}
-	timeoutSeconds := api.DefaultClientIPServiceAffinitySeconds
 	svc := &api.Service{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      serviceName,
@@ -264,11 +263,6 @@ func (c *Controller) CreateOrUpdateMasterServiceIfNeeded(serviceName string, ser
 			ClusterIP:       serviceIP.String(),
 			SessionAffinity: api.ServiceAffinityClientIP,
 			Type:            serviceType,
-			SessionAffinityConfig: &api.SessionAffinityConfig{
-				ClientIP: &api.ClientIPConfig{
-					TimeoutSeconds: &timeoutSeconds,
-				},
-			},
 		},
 	}
 

pkg/master/controller_test.go

@@ -546,7 +546,6 @@ func TestCreateOrUpdateMasterService(t *testing.T) {
 	om := func(name string) metav1.ObjectMeta {
 		return metav1.ObjectMeta{Namespace: ns, Name: name}
 	}
-	timeoutSeconds := api.DefaultClientIPServiceAffinitySeconds
 
 	create_tests := []struct {
 		testName     string
@@ -571,12 +570,7 @@ func TestCreateOrUpdateMasterService(t *testing.T) {
 					Selector:        nil,
 					ClusterIP:       "1.2.3.4",
 					SessionAffinity: api.ServiceAffinityClientIP,
-					SessionAffinityConfig: &api.SessionAffinityConfig{
+					Type:            api.ServiceTypeClusterIP,
-						ClientIP: &api.ClientIPConfig{
-							TimeoutSeconds: &timeoutSeconds,
-						},
-					},
-					Type: api.ServiceTypeClusterIP,
 				},
 			},
 		},
@@ -631,12 +625,7 @@ func TestCreateOrUpdateMasterService(t *testing.T) {
 					Selector:        nil,
 					ClusterIP:       "1.2.3.4",
 					SessionAffinity: api.ServiceAffinityClientIP,
-					SessionAffinityConfig: &api.SessionAffinityConfig{
+					Type:            api.ServiceTypeClusterIP,
-						ClientIP: &api.ClientIPConfig{
-							TimeoutSeconds: &timeoutSeconds,
-						},
-					},
-					Type: api.ServiceTypeClusterIP,
 				},
 			},
 			expectUpdate: &api.Service{
@@ -648,12 +637,7 @@ func TestCreateOrUpdateMasterService(t *testing.T) {
 					Selector:        nil,
 					ClusterIP:       "1.2.3.4",
 					SessionAffinity: api.ServiceAffinityClientIP,
-					SessionAffinityConfig: &api.SessionAffinityConfig{
+					Type:            api.ServiceTypeClusterIP,
-						ClientIP: &api.ClientIPConfig{
-							TimeoutSeconds: &timeoutSeconds,
-						},
-					},
-					Type: api.ServiceTypeClusterIP,
 				},
 			},
 		},
@@ -674,12 +658,7 @@ func TestCreateOrUpdateMasterService(t *testing.T) {
 					Selector:        nil,
 					ClusterIP:       "1.2.3.4",
 					SessionAffinity: api.ServiceAffinityClientIP,
-					SessionAffinityConfig: &api.SessionAffinityConfig{
+					Type:            api.ServiceTypeClusterIP,
-						ClientIP: &api.ClientIPConfig{
-							TimeoutSeconds: &timeoutSeconds,
-						},
-					},
-					Type: api.ServiceTypeClusterIP,
 				},
 			},
 			expectUpdate: &api.Service{
@@ -692,12 +671,7 @@ func TestCreateOrUpdateMasterService(t *testing.T) {
 					Selector:        nil,
 					ClusterIP:       "1.2.3.4",
 					SessionAffinity: api.ServiceAffinityClientIP,
-					SessionAffinityConfig: &api.SessionAffinityConfig{
+					Type:            api.ServiceTypeClusterIP,
-						ClientIP: &api.ClientIPConfig{
-							TimeoutSeconds: &timeoutSeconds,
-						},
-					},
-					Type: api.ServiceTypeClusterIP,
 				},
 			},
 		},
@@ -717,12 +691,7 @@ func TestCreateOrUpdateMasterService(t *testing.T) {
 					Selector:        nil,
 					ClusterIP:       "1.2.3.4",
 					SessionAffinity: api.ServiceAffinityClientIP,
-					SessionAffinityConfig: &api.SessionAffinityConfig{
+					Type:            api.ServiceTypeClusterIP,
-						ClientIP: &api.ClientIPConfig{
-							TimeoutSeconds: &timeoutSeconds,
-						},
-					},
-					Type: api.ServiceTypeClusterIP,
 				},
 			},
 			expectUpdate: &api.Service{
@@ -734,12 +703,7 @@ func TestCreateOrUpdateMasterService(t *testing.T) {
 					Selector:        nil,
 					ClusterIP:       "1.2.3.4",
 					SessionAffinity: api.ServiceAffinityClientIP,
-					SessionAffinityConfig: &api.SessionAffinityConfig{
+					Type:            api.ServiceTypeClusterIP,
-						ClientIP: &api.ClientIPConfig{
-							TimeoutSeconds: &timeoutSeconds,
-						},
-					},
-					Type: api.ServiceTypeClusterIP,
 				},
 			},
 		},
@@ -759,12 +723,7 @@ func TestCreateOrUpdateMasterService(t *testing.T) {
 					Selector:        nil,
 					ClusterIP:       "1.2.3.4",
 					SessionAffinity: api.ServiceAffinityClientIP,
-					SessionAffinityConfig: &api.SessionAffinityConfig{
+					Type:            api.ServiceTypeClusterIP,
-						ClientIP: &api.ClientIPConfig{
-							TimeoutSeconds: &timeoutSeconds,
-						},
-					},
-					Type: api.ServiceTypeClusterIP,
 				},
 			},
 			expectUpdate: &api.Service{
@@ -776,12 +735,7 @@ func TestCreateOrUpdateMasterService(t *testing.T) {
 					Selector:        nil,
 					ClusterIP:       "1.2.3.4",
 					SessionAffinity: api.ServiceAffinityClientIP,
-					SessionAffinityConfig: &api.SessionAffinityConfig{
+					Type:            api.ServiceTypeClusterIP,
-						ClientIP: &api.ClientIPConfig{
-							TimeoutSeconds: &timeoutSeconds,
-						},
-					},
-					Type: api.ServiceTypeClusterIP,
 				},
 			},
 		},
@@ -801,12 +755,7 @@ func TestCreateOrUpdateMasterService(t *testing.T) {
 					Selector:        nil,
 					ClusterIP:       "1.2.3.4",
 					SessionAffinity: api.ServiceAffinityClientIP,
-					SessionAffinityConfig: &api.SessionAffinityConfig{
+					Type:            api.ServiceTypeClusterIP,
-						ClientIP: &api.ClientIPConfig{
-							TimeoutSeconds: &timeoutSeconds,
-						},
-					},
-					Type: api.ServiceTypeClusterIP,
 				},
 			},
 			expectUpdate: &api.Service{
@@ -818,12 +767,7 @@ func TestCreateOrUpdateMasterService(t *testing.T) {
 					Selector:        nil,
 					ClusterIP:       "1.2.3.4",
 					SessionAffinity: api.ServiceAffinityClientIP,
-					SessionAffinityConfig: &api.SessionAffinityConfig{
+					Type:            api.ServiceTypeClusterIP,
-						ClientIP: &api.ClientIPConfig{
-							TimeoutSeconds: &timeoutSeconds,
-						},
-					},
-					Type: api.ServiceTypeClusterIP,
 				},
 			},
 		},
@@ -843,12 +787,7 @@ func TestCreateOrUpdateMasterService(t *testing.T) {
 					Selector:        nil,
 					ClusterIP:       "1.2.3.4",
 					SessionAffinity: api.ServiceAffinityClientIP,
-					SessionAffinityConfig: &api.SessionAffinityConfig{
+					Type:            api.ServiceTypeClusterIP,
-						ClientIP: &api.ClientIPConfig{
-							TimeoutSeconds: &timeoutSeconds,
-						},
-					},
-					Type: api.ServiceTypeClusterIP,
 				},
 			},
 			expectUpdate: &api.Service{
@@ -860,12 +799,7 @@ func TestCreateOrUpdateMasterService(t *testing.T) {
 					Selector:        nil,
 					ClusterIP:       "1.2.3.4",
 					SessionAffinity: api.ServiceAffinityClientIP,
-					SessionAffinityConfig: &api.SessionAffinityConfig{
+					Type:            api.ServiceTypeClusterIP,
-						ClientIP: &api.ClientIPConfig{
-							TimeoutSeconds: &timeoutSeconds,
-						},
-					},
-					Type: api.ServiceTypeClusterIP,
 				},
 			},
 		},
@@ -885,12 +819,7 @@ func TestCreateOrUpdateMasterService(t *testing.T) {
 					Selector:        nil,
 					ClusterIP:       "1.2.3.4",
 					SessionAffinity: api.ServiceAffinityClientIP,
-					SessionAffinityConfig: &api.SessionAffinityConfig{
+					Type:            api.ServiceTypeNodePort,
-						ClientIP: &api.ClientIPConfig{
-							TimeoutSeconds: &timeoutSeconds,
-						},
-					},
-					Type: api.ServiceTypeNodePort,
 				},
 			},
 			expectUpdate: &api.Service{
@@ -902,12 +831,7 @@ func TestCreateOrUpdateMasterService(t *testing.T) {
 					Selector:        nil,
 					ClusterIP:       "1.2.3.4",
 					SessionAffinity: api.ServiceAffinityClientIP,
-					SessionAffinityConfig: &api.SessionAffinityConfig{
+					Type:            api.ServiceTypeClusterIP,
-						ClientIP: &api.ClientIPConfig{
-							TimeoutSeconds: &timeoutSeconds,
-						},
-					},
-					Type: api.ServiceTypeClusterIP,
 				},
 			},
 		},
@@ -927,12 +851,7 @@ func TestCreateOrUpdateMasterService(t *testing.T) {
 					Selector:        nil,
 					ClusterIP:       "1.2.3.4",
 					SessionAffinity: api.ServiceAffinityClientIP,
-					SessionAffinityConfig: &api.SessionAffinityConfig{
+					Type:            api.ServiceTypeClusterIP,
-						ClientIP: &api.ClientIPConfig{
-							TimeoutSeconds: &timeoutSeconds,
-						},
-					},
-					Type: api.ServiceTypeClusterIP,
 				},
 			},
 			expectUpdate: nil,
@@ -991,12 +910,7 @@ func TestCreateOrUpdateMasterService(t *testing.T) {
 					Selector:        nil,
 					ClusterIP:       "1.2.3.4",
 					SessionAffinity: api.ServiceAffinityClientIP,
-					SessionAffinityConfig: &api.SessionAffinityConfig{
+					Type:            api.ServiceTypeClusterIP,
-						ClientIP: &api.ClientIPConfig{
-							TimeoutSeconds: &timeoutSeconds,
-						},
-					},
-					Type: api.ServiceTypeClusterIP,
 				},
 			},
 			expectUpdate: nil,

pkg/proxy/iptables/proxier.go

@@ -196,6 +196,7 @@ func newServiceInfo(svcPortName proxy.ServicePortName, port *api.ServicePort, se
 	}
 	var stickyMaxAgeSeconds int
 	if service.Spec.SessionAffinity == api.ServiceAffinityClientIP {
+		// Kube-apiserver side guarantees SessionAffinityConfig won't be nil when session affinity type is ClientIP
 		stickyMaxAgeSeconds = int(*service.Spec.SessionAffinityConfig.ClientIP.TimeoutSeconds)
 	}
 	info := &serviceInfo{

pkg/proxy/userspace/proxier.go

@@ -448,7 +448,7 @@ func (proxier *Proxier) mergeService(service *api.Service) sets.String {
 		info.loadBalancerStatus = *helper.LoadBalancerStatusDeepCopy(&service.Status.LoadBalancer)
 		info.nodePort = int(servicePort.NodePort)
 		info.sessionAffinityType = service.Spec.SessionAffinity
-		// Set session affinity timeout value when sessionAffinity==ClientIP
+		// Kube-apiserver side guarantees SessionAffinityConfig won't be nil when session affinity type is ClientIP
 		if service.Spec.SessionAffinity == api.ServiceAffinityClientIP {
 			info.stickyMaxAgeSeconds = int(*service.Spec.SessionAffinityConfig.ClientIP.TimeoutSeconds)
 		}

staging/src/k8s.io/api/core/v1/types.go

@@ -3031,6 +3031,8 @@ const (
 	ServiceAffinityNone ServiceAffinity = "None"
 )
 
+const DefaultClientIPServiceAffinitySeconds int32 = 10800
+
 // SessionAffinityConfig represents the configurations of session affinity.
 type SessionAffinityConfig struct {
 	// clientIP contains the configurations of Client IP based session affinity.