From f99807ece82d05e3dc4ce567d57df6d5ebec0214 Mon Sep 17 00:00:00 2001 From: yunmaoQu <2643354262@qq.com> Date: Fri, 13 Dec 2024 10:51:16 +0000 Subject: [PATCH 1/5] feat: add TLS version annotation support for per-rule configuration --- pkg/ingress/config/ingress_config.go | 2 + pkg/ingress/kube/annotations/annotations.go | 6 + .../kube/annotations/annotations_test.go | 40 ++ pkg/ingress/kube/annotations/downstreamtls.go | 53 +- .../kube/annotations/downstreamtls_test.go | 641 ++++++++++++------ pkg/ingress/kube/annotations/tls.go | 81 +++ 6 files changed, 619 insertions(+), 204 deletions(-) create mode 100644 pkg/ingress/kube/annotations/tls.go diff --git a/pkg/ingress/config/ingress_config.go b/pkg/ingress/config/ingress_config.go index bb4b261ac1..fbf28d2eb5 100644 --- a/pkg/ingress/config/ingress_config.go +++ b/pkg/ingress/config/ingress_config.go @@ -151,6 +151,8 @@ type IngressConfig struct { clusterId cluster.ID httpsConfigMgr *cert.ConfigMgr + + UpstreamTLS *TLSConfig } func NewIngressConfig(localKubeClient kube.Client, xdsUpdater istiomodel.XDSUpdater, namespace string, clusterId cluster.ID) *IngressConfig { diff --git a/pkg/ingress/kube/annotations/annotations.go b/pkg/ingress/kube/annotations/annotations.go index 36e4a6dea0..4d48231654 100644 --- a/pkg/ingress/kube/annotations/annotations.go +++ b/pkg/ingress/kube/annotations/annotations.go @@ -138,6 +138,10 @@ type AnnotationHandler interface { RouteHandler TrafficPolicyHandler } +type TLSConfig struct { + MinVersion string + MaxVersion string +} type AnnotationHandlerManager struct { parsers []Parser @@ -169,6 +173,7 @@ func NewAnnotationHandlerManager() AnnotationHandler { match{}, headerControl{}, http2rpc{}, + tls{}, }, gatewayHandlers: []GatewayHandler{ downstreamTLS{}, @@ -193,6 +198,7 @@ func NewAnnotationHandlerManager() AnnotationHandler { trafficPolicyHandlers: []TrafficPolicyHandler{ upstreamTLS{}, loadBalance{}, + tls{}, }, } } diff --git a/pkg/ingress/kube/annotations/annotations_test.go b/pkg/ingress/kube/annotations/annotations_test.go index bfad684d20..c8ed7f703c 100644 --- a/pkg/ingress/kube/annotations/annotations_test.go +++ b/pkg/ingress/kube/annotations/annotations_test.go @@ -219,3 +219,43 @@ func TestNeedTrafficPolicy(t *testing.T) { t.Fatal("should be true") } } + +func TestTLSConfig(t *testing.T) { + testCases := []struct { + annotations map[string]string + expectTLS *TLSConfig + }{ + { + annotations: map[string]string{ + "tls-min-version": "TLSv1_2", + "tls-max-version": "TLSv1_3", + }, + expectTLS: &TLSConfig{ + MinVersion: "TLSv1_2", + MaxVersion: "TLSv1_3", + }, + }, + { + annotations: map[string]string{ + "tls-min-version": "TLSv1_1", + }, + expectTLS: &TLSConfig{ + MinVersion: "TLSv1_1", + }, + }, + } + + for _, testCase := range testCases { + ingress := &Ingress{} + annotations := Annotations(testCase.annotations) + handler := tls{} + err := handler.Parse(annotations, ingress, nil) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if ingress.UpstreamTLS.MinVersion != testCase.expectTLS.MinVersion || ingress.UpstreamTLS.MaxVersion != testCase.expectTLS.MaxVersion { + t.Fatalf("expected %+v, got %+v", testCase.expectTLS, ingress.UpstreamTLS) + } + } +} \ No newline at end of file diff --git a/pkg/ingress/kube/annotations/downstreamtls.go b/pkg/ingress/kube/annotations/downstreamtls.go index 32a66897cc..a19927c695 100644 --- a/pkg/ingress/kube/annotations/downstreamtls.go +++ b/pkg/ingress/kube/annotations/downstreamtls.go @@ -27,9 +27,11 @@ import ( ) const ( - authTLSSecret = "auth-tls-secret" - sslCipher = "ssl-cipher" - gatewaySdsCaSuffix = "-cacert" + authTLSSecret = "auth-tls-secret" + sslCipher = "ssl-cipher" + gatewaySdsCaSuffix = "-cacert" + annotationMinTLSVersion = "tls-min-protocol-version" + annotationMaxTLSVersion = "tls-max-protocol-version" ) var ( @@ -41,6 +43,8 @@ type DownstreamTLSConfig struct { CipherSuites []string Mode networking.ServerTLSSettings_TLSmode CASecretName types.NamespacedName + MinVersion string + MaxVersion string } type downstreamTLS struct{} @@ -81,6 +85,14 @@ func (d downstreamTLS) Parse(annotations Annotations, config *Ingress, _ *Global downstreamTLSConfig.CipherSuites = validCipherSuite } + + if minVersion, err := annotations.ParseStringASAP(annotationMinTLSVersion); err == nil { + downstreamTLSConfig.MinVersion = minVersion + } + + if maxVersion, err := annotations.ParseStringASAP(annotationMaxTLSVersion); err == nil { + downstreamTLSConfig.MaxVersion = maxVersion + } return nil } @@ -107,11 +119,44 @@ func (d downstreamTLS) ApplyGateway(gateway *networking.Gateway, config *Ingress if len(downstreamTLSConfig.CipherSuites) != 0 { server.Tls.CipherSuites = downstreamTLSConfig.CipherSuites } + + if downstreamTLSConfig.MinVersion != "" { + if version, err := convertTLSVersion(downstreamTLSConfig.MinVersion); err != nil { + IngressLog.Errorf("Invalid minimum TLS version: %v", err) + } else { + server.Tls.MinProtocolVersion = version + } + } + + if downstreamTLSConfig.MaxVersion != "" { + if version, err := convertTLSVersion(downstreamTLSConfig.MaxVersion); err != nil { + IngressLog.Errorf("Invalid maximum TLS version: %v", err) + } else { + server.Tls.MaxProtocolVersion = version + } + } + } } } func needDownstreamTLS(annotations Annotations) bool { return annotations.HasASAP(sslCipher) || - annotations.HasASAP(authTLSSecret) + annotations.HasASAP(authTLSSecret)|| + annotations.HasASAP(annotationMinTLSVersion) || + annotations.HasASAP(annotationMaxTLSVersion) } + +func convertTLSVersion(version string) networking.ServerTLSSettings_TLSProtocol { + switch version { + case "TLSv1.0": + return networking.ServerTLSSettings_TLSV1_0 + case "TLSv1.1": + return networking.ServerTLSSettings_TLSV1_1 + case "TLSv1.2": + return networking.ServerTLSSettings_TLSV1_2 + case "TLSv1.3": + default: + return networking.ServerTLSSettings_TLS_AUTO, fmt.Errorf("invalid TLS version: %s. Valid values are: TLSv1.0, TLSv1.1, TLSv1.2, TLSv1.3", version) + } +} \ No newline at end of file diff --git a/pkg/ingress/kube/annotations/downstreamtls_test.go b/pkg/ingress/kube/annotations/downstreamtls_test.go index 5aeba194a0..030276a48a 100644 --- a/pkg/ingress/kube/annotations/downstreamtls_test.go +++ b/pkg/ingress/kube/annotations/downstreamtls_test.go @@ -15,238 +15,479 @@ package annotations import ( - "reflect" - "testing" + "reflect" + "testing" - networking "istio.io/api/networking/v1alpha3" - "k8s.io/apimachinery/pkg/types" + networking "istio.io/api/networking/v1alpha3" + "k8s.io/apimachinery/pkg/types" ) var parser = downstreamTLS{} func TestParse(t *testing.T) { - testCases := []struct { - input map[string]string - expect *DownstreamTLSConfig - }{ - {}, - { - input: map[string]string{ - buildNginxAnnotationKey(sslCipher): "ECDHE-RSA-AES256-GCM-SHA384:AES128-SHA", - }, - expect: &DownstreamTLSConfig{ - Mode: networking.ServerTLSSettings_SIMPLE, - CipherSuites: []string{"ECDHE-RSA-AES256-GCM-SHA384", "AES128-SHA"}, - }, - }, - { - input: map[string]string{ - buildNginxAnnotationKey(authTLSSecret): "test", - buildNginxAnnotationKey(sslCipher): "ECDHE-RSA-AES256-GCM-SHA384:AES128-SHA", - }, - expect: &DownstreamTLSConfig{ - CASecretName: types.NamespacedName{ - Namespace: "foo", - Name: "test", + testCases := []struct { + name string + input map[string]string + expect *DownstreamTLSConfig + }{ + { + name: "empty annotations", }, - Mode: networking.ServerTLSSettings_MUTUAL, - CipherSuites: []string{"ECDHE-RSA-AES256-GCM-SHA384", "AES128-SHA"}, - }, - }, - { - input: map[string]string{ - buildHigressAnnotationKey(authTLSSecret): "test/foo", - DefaultAnnotationsPrefix + "/" + sslCipher: "ECDHE-RSA-AES256-GCM-SHA384:AES128-SHA", - }, - expect: &DownstreamTLSConfig{ - CASecretName: types.NamespacedName{ - Namespace: "test", - Name: "foo", - }, - Mode: networking.ServerTLSSettings_MUTUAL, - CipherSuites: []string{"ECDHE-RSA-AES256-GCM-SHA384", "AES128-SHA"}, - }, - }, - } - - for _, testCase := range testCases { - t.Run("", func(t *testing.T) { - config := &Ingress{ - Meta: Meta{ - Namespace: "foo", - }, - } - _ = parser.Parse(testCase.input, config, nil) - if !reflect.DeepEqual(testCase.expect, config.DownstreamTLS) { - t.Fatalf("Should be equal") - } - }) - } -} - -func TestApplyGateway(t *testing.T) { - testCases := []struct { - input *networking.Gateway - config *Ingress - expect *networking.Gateway - }{ - { - input: &networking.Gateway{ - Servers: []*networking.Server{ - { - Port: &networking.Port{ - Protocol: "HTTPS", + { + name: "cipher suite only", + input: map[string]string{ + buildNginxAnnotationKey(sslCipher): "ECDHE-RSA-AES256-GCM-SHA384:AES128-SHA", }, - Tls: &networking.ServerTLSSettings{ - Mode: networking.ServerTLSSettings_SIMPLE, + expect: &DownstreamTLSConfig{ + Mode: networking.ServerTLSSettings_SIMPLE, + CipherSuites: []string{"ECDHE-RSA-AES256-GCM-SHA384", "AES128-SHA"}, + RuleMinVersion: make(map[string]string), + RuleMaxVersion: make(map[string]string), }, - }, }, - }, - config: &Ingress{ - DownstreamTLS: &DownstreamTLSConfig{ - CipherSuites: []string{"ECDHE-RSA-AES256-GCM-SHA384"}, + { + name: "global TLS version only", + input: map[string]string{ + buildNginxAnnotationKey(annotationMinTLSVersion): "TLSv1_2", + buildNginxAnnotationKey(annotationMaxTLSVersion): "TLSv1_3", + }, + expect: &DownstreamTLSConfig{ + Mode: networking.ServerTLSSettings_SIMPLE, + MinVersion: "TLSv1_2", + MaxVersion: "TLSv1_3", + RuleMinVersion: make(map[string]string), + RuleMaxVersion: make(map[string]string), + }, }, - }, - expect: &networking.Gateway{ - Servers: []*networking.Server{ - { - Port: &networking.Port{ - Protocol: "HTTPS", + { + name: "rule specific TLS version", + input: map[string]string{ + buildNginxAnnotationKey(annotationMinTLSVersion + ".rule1"): "TLSv1_1", + buildNginxAnnotationKey(annotationMaxTLSVersion + ".rule1"): "TLSv1_2", }, - Tls: &networking.ServerTLSSettings{ - Mode: networking.ServerTLSSettings_SIMPLE, - CipherSuites: []string{"ECDHE-RSA-AES256-GCM-SHA384"}, + expect: &DownstreamTLSConfig{ + Mode: networking.ServerTLSSettings_SIMPLE, + RuleMinVersion: map[string]string{"rule1": "TLSv1_1"}, + RuleMaxVersion: map[string]string{"rule1": "TLSv1_2"}, }, - }, }, - }, - }, - { - input: &networking.Gateway{ - Servers: []*networking.Server{ - { - Port: &networking.Port{ - Protocol: "HTTPS", - }, - Tls: &networking.ServerTLSSettings{ - Mode: networking.ServerTLSSettings_SIMPLE, - CredentialName: "kubernetes-ingress://cluster/foo/bar", + { + name: "global and rule specific TLS version", + input: map[string]string{ + buildNginxAnnotationKey(annotationMinTLSVersion): "TLSv1_2", + buildNginxAnnotationKey(annotationMaxTLSVersion): "TLSv1_3", + buildNginxAnnotationKey(annotationMinTLSVersion + ".rule1"): "TLSv1_1", + buildNginxAnnotationKey(annotationMaxTLSVersion + ".rule1"): "TLSv1_2", + }, + expect: &DownstreamTLSConfig{ + Mode: networking.ServerTLSSettings_SIMPLE, + MinVersion: "TLSv1_2", + MaxVersion: "TLSv1_3", + RuleMinVersion: map[string]string{"rule1": "TLSv1_1"}, + RuleMaxVersion: map[string]string{"rule1": "TLSv1_2"}, }, - }, }, - }, - config: &Ingress{ - DownstreamTLS: &DownstreamTLSConfig{ - CASecretName: types.NamespacedName{ - Namespace: "foo", - Name: "bar", - }, - Mode: networking.ServerTLSSettings_MUTUAL, - CipherSuites: []string{"ECDHE-RSA-AES256-GCM-SHA384"}, + { + name: "multiple rules TLS version", + input: map[string]string{ + buildNginxAnnotationKey(annotationMinTLSVersion + ".rule1"): "TLSv1_1", + buildNginxAnnotationKey(annotationMaxTLSVersion + ".rule1"): "TLSv1_2", + buildNginxAnnotationKey(annotationMinTLSVersion + ".rule2"): "TLSv1_2", + buildNginxAnnotationKey(annotationMaxTLSVersion + ".rule2"): "TLSv1_3", + }, + expect: &DownstreamTLSConfig{ + Mode: networking.ServerTLSSettings_SIMPLE, + RuleMinVersion: map[string]string{ + "rule1": "TLSv1_1", + "rule2": "TLSv1_2", + }, + RuleMaxVersion: map[string]string{ + "rule1": "TLSv1_2", + "rule2": "TLSv1_3", + }, + }, }, - }, - expect: &networking.Gateway{ - Servers: []*networking.Server{ - { - Port: &networking.Port{ - Protocol: "HTTPS", + { + name: "complete configuration", + input: map[string]string{ + buildHigressAnnotationKey(authTLSSecret): "test/foo", + buildHigressAnnotationKey(annotationMinTLSVersion): "TLSv1_2", + buildHigressAnnotationKey(annotationMaxTLSVersion): "TLSv1_3", + buildHigressAnnotationKey(annotationMinTLSVersion + ".rule1"): "TLSv1_1", + buildHigressAnnotationKey(annotationMaxTLSVersion + ".rule1"): "TLSv1_2", + DefaultAnnotationsPrefix + "/" + sslCipher: "ECDHE-RSA-AES256-GCM-SHA384", }, - Tls: &networking.ServerTLSSettings{ - CredentialName: "kubernetes-ingress://cluster/foo/bar", - Mode: networking.ServerTLSSettings_MUTUAL, - CipherSuites: []string{"ECDHE-RSA-AES256-GCM-SHA384"}, + expect: &DownstreamTLSConfig{ + CASecretName: types.NamespacedName{ + Namespace: "test", + Name: "foo", + }, + Mode: networking.ServerTLSSettings_MUTUAL, + MinVersion: "TLSv1_2", + MaxVersion: "TLSv1_3", + CipherSuites: []string{"ECDHE-RSA-AES256-GCM-SHA384"}, + RuleMinVersion: map[string]string{"rule1": "TLSv1_1"}, + RuleMaxVersion: map[string]string{"rule1": "TLSv1_2"}, }, - }, }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + config := &Ingress{ + Meta: Meta{ + Namespace: "foo", + }, + } + err := parser.Parse(tc.input, config, nil) + if err != nil { + t.Fatalf("Parse failed: %v", err) + } + + if !reflect.DeepEqual(tc.expect, config.DownstreamTLS) { + t.Fatalf("Parse result mismatch:\nExpect: %+v\nGot: %+v", tc.expect, config.DownstreamTLS) + } + }) + } +} + +func TestApplyGateway(t *testing.T) { + testCases := []struct { + name string + input *networking.Gateway + config *Ingress + expect *networking.Gateway + }{ + { + name: "global TLS version only", + input: &networking.Gateway{ + Servers: []*networking.Server{ + { + Name: "rule1", + Port: &networking.Port{ + Protocol: "HTTPS", + }, + Tls: &networking.ServerTLSSettings{ + Mode: networking.ServerTLSSettings_SIMPLE, + }, + }, + }, + }, + config: &Ingress{ + DownstreamTLS: &DownstreamTLSConfig{ + Mode: networking.ServerTLSSettings_SIMPLE, + MinVersion: "TLSv1_2", + MaxVersion: "TLSv1_3", + RuleMinVersion: make(map[string]string), + RuleMaxVersion: make(map[string]string), + }, + }, + expect: &networking.Gateway{ + Servers: []*networking.Server{ + { + Name: "rule1", + Port: &networking.Port{ + Protocol: "HTTPS", + }, + Tls: &networking.ServerTLSSettings{ + Mode: networking.ServerTLSSettings_SIMPLE, + MinProtocolVersion: networking.ServerTLSSettings_TLSV1_2, + MaxProtocolVersion: networking.ServerTLSSettings_TLSV1_3, + }, + }, + }, + }, }, - }, - { - input: &networking.Gateway{ - Servers: []*networking.Server{ - { - Port: &networking.Port{ - Protocol: "HTTPS", - }, - Tls: &networking.ServerTLSSettings{ - Mode: networking.ServerTLSSettings_SIMPLE, - CredentialName: "kubernetes-ingress://cluster/foo/bar", - }, + { + name: "rule specific TLS version", + input: &networking.Gateway{ + Servers: []*networking.Server{ + { + Name: "rule1", + Port: &networking.Port{ + Protocol: "HTTPS", + }, + Tls: &networking.ServerTLSSettings{ + Mode: networking.ServerTLSSettings_SIMPLE, + }, + }, + }, + }, + config: &Ingress{ + DownstreamTLS: &DownstreamTLSConfig{ + Mode: networking.ServerTLSSettings_SIMPLE, + RuleMinVersion: map[string]string{ + "rule1": "TLSv1_1", + }, + RuleMaxVersion: map[string]string{ + "rule1": "TLSv1_2", + }, + }, + }, + expect: &networking.Gateway{ + Servers: []*networking.Server{ + { + Name: "rule1", + Port: &networking.Port{ + Protocol: "HTTPS", + }, + Tls: &networking.ServerTLSSettings{ + Mode: networking.ServerTLSSettings_SIMPLE, + MinProtocolVersion: networking.ServerTLSSettings_TLSV1_1, + MaxProtocolVersion: networking.ServerTLSSettings_TLSV1_2, + }, + }, + }, }, - }, }, - config: &Ingress{ - DownstreamTLS: &DownstreamTLSConfig{ - CASecretName: types.NamespacedName{ - Namespace: "foo", - Name: "bar-cacert", + { + name: "rule override global TLS version", + input: &networking.Gateway{ + Servers: []*networking.Server{ + { + Name: "rule1", + Port: &networking.Port{ + Protocol: "HTTPS", + }, + Tls: &networking.ServerTLSSettings{ + Mode: networking.ServerTLSSettings_SIMPLE, + }, + }, + }, + }, + config: &Ingress{ + DownstreamTLS: &DownstreamTLSConfig{ + Mode: networking.ServerTLSSettings_SIMPLE, + MinVersion: "TLSv1_2", + MaxVersion: "TLSv1_3", + RuleMinVersion: map[string]string{ + "rule1": "TLSv1_1", + }, + RuleMaxVersion: map[string]string{ + "rule1": "TLSv1_2", + }, + }, + }, + expect: &networking.Gateway{ + Servers: []*networking.Server{ + { + Name: "rule1", + Port: &networking.Port{ + Protocol: "HTTPS", + }, + Tls: &networking.ServerTLSSettings{ + Mode: networking.ServerTLSSettings_SIMPLE, + MinProtocolVersion: networking.ServerTLSSettings_TLSV1_1, + MaxProtocolVersion: networking.ServerTLSSettings_TLSV1_2, + }, + }, + }, }, - Mode: networking.ServerTLSSettings_MUTUAL, - CipherSuites: []string{"ECDHE-RSA-AES256-GCM-SHA384"}, - }, }, - expect: &networking.Gateway{ - Servers: []*networking.Server{ - { - Port: &networking.Port{ - Protocol: "HTTPS", - }, - Tls: &networking.ServerTLSSettings{ - CredentialName: "kubernetes-ingress://cluster/foo/bar", - Mode: networking.ServerTLSSettings_MUTUAL, - CipherSuites: []string{"ECDHE-RSA-AES256-GCM-SHA384"}, - }, + { + name: "complete configuration with cipher suites", + input: &networking.Gateway{ + Servers: []*networking.Server{ + { + Name: "rule1", + Port: &networking.Port{ + Protocol: "HTTPS", + }, + Tls: &networking.ServerTLSSettings{ + Mode: networking.ServerTLSSettings_SIMPLE, + CredentialName: "kubernetes-ingress://cluster/foo/bar", + }, + }, + }, + }, + config: &Ingress{ + DownstreamTLS: &DownstreamTLSConfig{ + Mode: networking.ServerTLSSettings_MUTUAL, + MinVersion: "TLSv1_2", + MaxVersion: "TLSv1_3", + RuleMinVersion: map[string]string{ + "rule1": "TLSv1_1", + }, + RuleMaxVersion: map[string]string{ + "rule1": "TLSv1_2", + }, + CipherSuites: []string{"ECDHE-RSA-AES256-GCM-SHA384"}, + CASecretName: types.NamespacedName{ + Namespace: "foo", + Name: "bar", + }, + }, + }, + expect: &networking.Gateway{ + Servers: []*networking.Server{ + { + Name: "rule1", + Port: &networking.Port{ + Protocol: "HTTPS", + }, + Tls: &networking.ServerTLSSettings{ + Mode: networking.ServerTLSSettings_MUTUAL, + CredentialName: "kubernetes-ingress://cluster/foo/bar", + MinProtocolVersion: networking.ServerTLSSettings_TLSV1_1, + MaxProtocolVersion: networking.ServerTLSSettings_TLSV1_2, + CipherSuites: []string{"ECDHE-RSA-AES256-GCM-SHA384"}, + }, + }, + }, }, - }, }, - }, - { - input: &networking.Gateway{ - Servers: []*networking.Server{ - { - Port: &networking.Port{ - Protocol: "HTTPS", - }, - Tls: &networking.ServerTLSSettings{ - Mode: networking.ServerTLSSettings_SIMPLE, - CredentialName: "kubernetes-ingress://cluster/foo/bar", + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + parser.ApplyGateway(tc.input, tc.config) + if !reflect.DeepEqual(tc.input, tc.expect) { + t.Fatalf("ApplyGateway result mismatch:\nExpect: %+v\nGot: %+v", tc.expect, tc.input) + } + }) + } +} + + func TestConvertTLSVersion(t *testing.T) { + testCases := []struct { + name string + version string + expect networking.ServerTLSSettings_TLSProtocol + }{ + { + name: "TLS 1.0", + version: "TLSv1_0", + expect: networking.ServerTLSSettings_TLSV1_0, + }, + { + name: "TLS 1.1", + version: "TLSv1_1", + expect: networking.ServerTLSSettings_TLSV1_1, + }, + { + name: "TLS 1.2", + version: "TLSv1_2", + expect: networking.ServerTLSSettings_TLSV1_2, + }, + { + name: "TLS 1.3", + version: "TLSv1_3", + expect: networking.ServerTLSSettings_TLSV1_3, + }, + { + name: "invalid version", + version: "invalid", + expect: networking.ServerTLSSettings_TLS_AUTO, + }, + { + name: "empty version", + version: "", + expect: networking.ServerTLSSettings_TLS_AUTO, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + result := convertTLSVersion(tc.version) + if result != tc.expect { + t.Errorf("convertTLSVersion(%s) = %v, want %v", tc.version, result, tc.expect) + } + }) + } + } + + func TestNeedDownstreamTLS(t *testing.T) { + testCases := []struct { + name string + annotations map[string]string + expect bool + }{ + { + name: "empty annotations", + annotations: map[string]string{}, + expect: false, + }, + { + name: "ssl cipher only", + annotations: map[string]string{ + buildNginxAnnotationKey(sslCipher): "ECDHE-RSA-AES256-GCM-SHA384", }, - }, + expect: true, }, - }, - config: &Ingress{ - DownstreamTLS: &DownstreamTLSConfig{ - CASecretName: types.NamespacedName{ - Namespace: "bar", - Name: "foo", - }, - Mode: networking.ServerTLSSettings_MUTUAL, - CipherSuites: []string{"ECDHE-RSA-AES256-GCM-SHA384"}, + { + name: "auth TLS secret only", + annotations: map[string]string{ + buildNginxAnnotationKey(authTLSSecret): "test/foo", + }, + expect: true, }, - }, - expect: &networking.Gateway{ - Servers: []*networking.Server{ - { - Port: &networking.Port{ - Protocol: "HTTPS", + { + name: "global TLS version only", + annotations: map[string]string{ + buildNginxAnnotationKey(annotationMinTLSVersion): "TLSv1_2", }, - Tls: &networking.ServerTLSSettings{ - CredentialName: "kubernetes-ingress://cluster/foo/bar", - Mode: networking.ServerTLSSettings_SIMPLE, - CipherSuites: []string{"ECDHE-RSA-AES256-GCM-SHA384"}, + expect: true, + }, + { + name: "rule specific TLS version only", + annotations: map[string]string{ + buildNginxAnnotationKey(annotationMinTLSVersion + ".rule1"): "TLSv1_2", }, - }, + expect: true, }, - }, - }, - } + { + name: "multiple TLS configurations", + annotations: map[string]string{ + buildNginxAnnotationKey(sslCipher): "ECDHE-RSA-AES256-GCM-SHA384", + buildNginxAnnotationKey(annotationMinTLSVersion): "TLSv1_2", + buildNginxAnnotationKey(annotationMinTLSVersion + ".rule1"): "TLSv1_1", + }, + expect: true, + }, + } - for _, testCase := range testCases { - t.Run("", func(t *testing.T) { - parser.ApplyGateway(testCase.input, testCase.config) - if !reflect.DeepEqual(testCase.input, testCase.expect) { - t.Fatalf("Should be equal") - } - }) + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + result := needDownstreamTLS(Annotations(tc.annotations)) + if result != tc.expect { + t.Errorf("needDownstreamTLS() = %v, want %v", result, tc.expect) + } + }) + } } -} + + func TestExtraSecret(t *testing.T) { + testCases := []struct { + name string + credentialName string + expect types.NamespacedName + }{ + { + name: "valid credential name", + credentialName: "kubernetes-ingress://cluster/foo/bar", + expect: types.NamespacedName{ + Namespace: "foo", + Name: "bar", + }, + }, + { + name: "invalid credential name", + credentialName: "invalid-format", + expect: types.NamespacedName{}, + }, + { + name: "empty credential name", + credentialName: "", + expect: types.NamespacedName{}, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + result := extraSecret(tc.credentialName) + if !reflect.DeepEqual(result, tc.expect) { + t.Errorf("extraSecret(%s) = %v, want %v", tc.credentialName, result, tc.expect) + } + }) + } + } \ No newline at end of file diff --git a/pkg/ingress/kube/annotations/tls.go b/pkg/ingress/kube/annotations/tls.go new file mode 100644 index 0000000000..ddfe227e76 --- /dev/null +++ b/pkg/ingress/kube/annotations/tls.go @@ -0,0 +1,81 @@ +package annotations + +import ( + networking "istio.io/api/networking/v1alpha3" +) + +const ( + tlsMinVersion = "tls-min-version" + tlsMaxVersion = "tls-max-version" +) + +var ( + _ Parser = tls{} + _ TrafficPolicyHandler = tls{} +) + +type tls struct{} + +func (t tls) Parse(annotations Annotations, config *Ingress, _ *GlobalContext) error { + if !needTLSConfig(annotations) { + return nil + } + + tlsConfig := &TLSConfig{} + defer func() { + config.UpstreamTLS = tlsConfig + }() + + // Parse minimum TLS version + if minVersion, err := annotations.ParseStringASAP(tlsMinVersion); err == nil { + tlsConfig.MinVersion = minVersion + } + + // Parse maximum TLS version + if maxVersion, err := annotations.ParseStringASAP(tlsMaxVersion); err == nil { + tlsConfig.MaxVersion = maxVersion + } + + return nil +} + +func (t tls) ApplyTrafficPolicy(trafficPolicy *networking.TrafficPolicy, _ *networking.TrafficPolicy_PortTrafficPolicy, config *Ingress) { + tlsConfig := config.UpstreamTLS + if tlsConfig == nil { + return + } + + if trafficPolicy.Tls == nil { + trafficPolicy.Tls = &networking.ClientTLSSettings{} + } + + // Apply min version + if tlsConfig.MinVersion != "" { + trafficPolicy.Tls.MinProtocolVersion = convertTLSVersion(tlsConfig.MinVersion) + } + + // Apply max version + if tlsConfig.MaxVersion != "" { + trafficPolicy.Tls.MaxProtocolVersion = convertTLSVersion(tlsConfig.MaxVersion) + } +} + +func needTLSConfig(annotations Annotations) bool { + return annotations.HasASAP(tlsMinVersion) || annotations.HasASAP(tlsMaxVersion) +} + +// Helper to convert TLS version string to istio enum +func convertTLSVersion(version string) networking.ClientTLSSettings_TLSProtocol { + switch version { + case "TLSv1_0": + return networking.ClientTLSSettings_TLSV1_0 + case "TLSv1_1": + return networking.ClientTLSSettings_TLSV1_1 + case "TLSv1_2": + return networking.ClientTLSSettings_TLSV1_2 + case "TLSv1_3": + return networking.ClientTLSSettings_TLSV1_3 + default: + return networking.ClientTLSSettings_TLS_AUTO + } +} From eb716ee0491544dd48cea2819060ff6c70b424a7 Mon Sep 17 00:00:00 2001 From: yunmaoQu <2643354262@qq.com> Date: Mon, 6 Jan 2025 07:52:29 +0000 Subject: [PATCH 2/5] fix rebase error --- pkg/ingress/config/ingress_config.go | 1 - pkg/ingress/kube/annotations/annotations.go | 6 -- pkg/ingress/kube/annotations/tls.go | 81 --------------------- 3 files changed, 88 deletions(-) delete mode 100644 pkg/ingress/kube/annotations/tls.go diff --git a/pkg/ingress/config/ingress_config.go b/pkg/ingress/config/ingress_config.go index fbf28d2eb5..eb6393d1ea 100644 --- a/pkg/ingress/config/ingress_config.go +++ b/pkg/ingress/config/ingress_config.go @@ -152,7 +152,6 @@ type IngressConfig struct { httpsConfigMgr *cert.ConfigMgr - UpstreamTLS *TLSConfig } func NewIngressConfig(localKubeClient kube.Client, xdsUpdater istiomodel.XDSUpdater, namespace string, clusterId cluster.ID) *IngressConfig { diff --git a/pkg/ingress/kube/annotations/annotations.go b/pkg/ingress/kube/annotations/annotations.go index 4d48231654..36e4a6dea0 100644 --- a/pkg/ingress/kube/annotations/annotations.go +++ b/pkg/ingress/kube/annotations/annotations.go @@ -138,10 +138,6 @@ type AnnotationHandler interface { RouteHandler TrafficPolicyHandler } -type TLSConfig struct { - MinVersion string - MaxVersion string -} type AnnotationHandlerManager struct { parsers []Parser @@ -173,7 +169,6 @@ func NewAnnotationHandlerManager() AnnotationHandler { match{}, headerControl{}, http2rpc{}, - tls{}, }, gatewayHandlers: []GatewayHandler{ downstreamTLS{}, @@ -198,7 +193,6 @@ func NewAnnotationHandlerManager() AnnotationHandler { trafficPolicyHandlers: []TrafficPolicyHandler{ upstreamTLS{}, loadBalance{}, - tls{}, }, } } diff --git a/pkg/ingress/kube/annotations/tls.go b/pkg/ingress/kube/annotations/tls.go deleted file mode 100644 index ddfe227e76..0000000000 --- a/pkg/ingress/kube/annotations/tls.go +++ /dev/null @@ -1,81 +0,0 @@ -package annotations - -import ( - networking "istio.io/api/networking/v1alpha3" -) - -const ( - tlsMinVersion = "tls-min-version" - tlsMaxVersion = "tls-max-version" -) - -var ( - _ Parser = tls{} - _ TrafficPolicyHandler = tls{} -) - -type tls struct{} - -func (t tls) Parse(annotations Annotations, config *Ingress, _ *GlobalContext) error { - if !needTLSConfig(annotations) { - return nil - } - - tlsConfig := &TLSConfig{} - defer func() { - config.UpstreamTLS = tlsConfig - }() - - // Parse minimum TLS version - if minVersion, err := annotations.ParseStringASAP(tlsMinVersion); err == nil { - tlsConfig.MinVersion = minVersion - } - - // Parse maximum TLS version - if maxVersion, err := annotations.ParseStringASAP(tlsMaxVersion); err == nil { - tlsConfig.MaxVersion = maxVersion - } - - return nil -} - -func (t tls) ApplyTrafficPolicy(trafficPolicy *networking.TrafficPolicy, _ *networking.TrafficPolicy_PortTrafficPolicy, config *Ingress) { - tlsConfig := config.UpstreamTLS - if tlsConfig == nil { - return - } - - if trafficPolicy.Tls == nil { - trafficPolicy.Tls = &networking.ClientTLSSettings{} - } - - // Apply min version - if tlsConfig.MinVersion != "" { - trafficPolicy.Tls.MinProtocolVersion = convertTLSVersion(tlsConfig.MinVersion) - } - - // Apply max version - if tlsConfig.MaxVersion != "" { - trafficPolicy.Tls.MaxProtocolVersion = convertTLSVersion(tlsConfig.MaxVersion) - } -} - -func needTLSConfig(annotations Annotations) bool { - return annotations.HasASAP(tlsMinVersion) || annotations.HasASAP(tlsMaxVersion) -} - -// Helper to convert TLS version string to istio enum -func convertTLSVersion(version string) networking.ClientTLSSettings_TLSProtocol { - switch version { - case "TLSv1_0": - return networking.ClientTLSSettings_TLSV1_0 - case "TLSv1_1": - return networking.ClientTLSSettings_TLSV1_1 - case "TLSv1_2": - return networking.ClientTLSSettings_TLSV1_2 - case "TLSv1_3": - return networking.ClientTLSSettings_TLSV1_3 - default: - return networking.ClientTLSSettings_TLS_AUTO - } -} From 471d3148b501616c2dcd7263e0f60b01d3e7a5b6 Mon Sep 17 00:00:00 2001 From: yunmaoQu <2643354262@qq.com> Date: Mon, 6 Jan 2025 08:31:17 +0000 Subject: [PATCH 3/5] fix rebase error --- .../kube/annotations/annotations_test.go | 40 - pkg/ingress/kube/annotations/downstreamtls.go | 10 +- .../kube/annotations/downstreamtls_test.go | 715 +++++++----------- 3 files changed, 281 insertions(+), 484 deletions(-) diff --git a/pkg/ingress/kube/annotations/annotations_test.go b/pkg/ingress/kube/annotations/annotations_test.go index c8ed7f703c..bfad684d20 100644 --- a/pkg/ingress/kube/annotations/annotations_test.go +++ b/pkg/ingress/kube/annotations/annotations_test.go @@ -219,43 +219,3 @@ func TestNeedTrafficPolicy(t *testing.T) { t.Fatal("should be true") } } - -func TestTLSConfig(t *testing.T) { - testCases := []struct { - annotations map[string]string - expectTLS *TLSConfig - }{ - { - annotations: map[string]string{ - "tls-min-version": "TLSv1_2", - "tls-max-version": "TLSv1_3", - }, - expectTLS: &TLSConfig{ - MinVersion: "TLSv1_2", - MaxVersion: "TLSv1_3", - }, - }, - { - annotations: map[string]string{ - "tls-min-version": "TLSv1_1", - }, - expectTLS: &TLSConfig{ - MinVersion: "TLSv1_1", - }, - }, - } - - for _, testCase := range testCases { - ingress := &Ingress{} - annotations := Annotations(testCase.annotations) - handler := tls{} - err := handler.Parse(annotations, ingress, nil) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - - if ingress.UpstreamTLS.MinVersion != testCase.expectTLS.MinVersion || ingress.UpstreamTLS.MaxVersion != testCase.expectTLS.MaxVersion { - t.Fatalf("expected %+v, got %+v", testCase.expectTLS, ingress.UpstreamTLS) - } - } -} \ No newline at end of file diff --git a/pkg/ingress/kube/annotations/downstreamtls.go b/pkg/ingress/kube/annotations/downstreamtls.go index a19927c695..bc944d7df4 100644 --- a/pkg/ingress/kube/annotations/downstreamtls.go +++ b/pkg/ingress/kube/annotations/downstreamtls.go @@ -147,16 +147,16 @@ func needDownstreamTLS(annotations Annotations) bool { annotations.HasASAP(annotationMaxTLSVersion) } -func convertTLSVersion(version string) networking.ServerTLSSettings_TLSProtocol { +func convertTLSVersion(version string) (networking.ServerTLSSettings_TLSProtocol, error) { switch version { case "TLSv1.0": - return networking.ServerTLSSettings_TLSV1_0 + return networking.ServerTLSSettings_TLSV1_0 , nil case "TLSv1.1": - return networking.ServerTLSSettings_TLSV1_1 + return networking.ServerTLSSettings_TLSV1_1 , nil case "TLSv1.2": - return networking.ServerTLSSettings_TLSV1_2 + return networking.ServerTLSSettings_TLSV1_2 , nil case "TLSv1.3": default: return networking.ServerTLSSettings_TLS_AUTO, fmt.Errorf("invalid TLS version: %s. Valid values are: TLSv1.0, TLSv1.1, TLSv1.2, TLSv1.3", version) } -} \ No newline at end of file +} diff --git a/pkg/ingress/kube/annotations/downstreamtls_test.go b/pkg/ingress/kube/annotations/downstreamtls_test.go index 030276a48a..1bf36517ae 100644 --- a/pkg/ingress/kube/annotations/downstreamtls_test.go +++ b/pkg/ingress/kube/annotations/downstreamtls_test.go @@ -15,479 +15,316 @@ package annotations import ( - "reflect" - "testing" + "reflect" + "testing" - networking "istio.io/api/networking/v1alpha3" - "k8s.io/apimachinery/pkg/types" + networking "istio.io/api/networking/v1alpha3" + "k8s.io/apimachinery/pkg/types" ) var parser = downstreamTLS{} func TestParse(t *testing.T) { - testCases := []struct { - name string - input map[string]string - expect *DownstreamTLSConfig - }{ - { - name: "empty annotations", + testCases := []struct { + name string + input map[string]string + expect *DownstreamTLSConfig + }{ + { + name: "empty config", + }, + { + name: "ssl cipher only", + input: map[string]string{ + buildNginxAnnotationKey(sslCipher): "ECDHE-RSA-AES256-GCM-SHA384:AES128-SHA", + }, + expect: &DownstreamTLSConfig{ + Mode: networking.ServerTLSSettings_SIMPLE, + CipherSuites: []string{"ECDHE-RSA-AES256-GCM-SHA384", "AES128-SHA"}, + }, + }, + { + name: "with TLS version config", + input: map[string]string{ + buildNginxAnnotationKey(annotationMinTLSVersion): "TLSv1.2", + buildNginxAnnotationKey(annotationMaxTLSVersion): "TLSv1.3", + }, + expect: &DownstreamTLSConfig{ + Mode: networking.ServerTLSSettings_SIMPLE, + MinVersion: "TLSv1.2", + MaxVersion: "TLSv1.3", + }, + }, + { + name: "complete config", + input: map[string]string{ + buildNginxAnnotationKey(authTLSSecret): "test", + buildNginxAnnotationKey(sslCipher): "ECDHE-RSA-AES256-GCM-SHA384:AES128-SHA", + buildNginxAnnotationKey(annotationMinTLSVersion): "TLSv1.2", + buildNginxAnnotationKey(annotationMaxTLSVersion): "TLSv1.3", + }, + expect: &DownstreamTLSConfig{ + CASecretName: types.NamespacedName{ + Namespace: "foo", + Name: "test", + }, + Mode: networking.ServerTLSSettings_MUTUAL, + CipherSuites: []string{"ECDHE-RSA-AES256-GCM-SHA384", "AES128-SHA"}, + MinVersion: "TLSv1.2", + MaxVersion: "TLSv1.3", + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + config := &Ingress{ + Meta: Meta{ + Namespace: "foo", }, - { - name: "cipher suite only", - input: map[string]string{ - buildNginxAnnotationKey(sslCipher): "ECDHE-RSA-AES256-GCM-SHA384:AES128-SHA", + } + err := parser.Parse(tc.input, config, nil) + if err != nil { + t.Fatalf("Parse failed: %v", err) + } + if !reflect.DeepEqual(tc.expect, config.DownstreamTLS) { + t.Fatalf("Parse result mismatch:\nExpect: %+v\nGot: %+v", tc.expect, config.DownstreamTLS) + } + }) + } +} + +func TestConvertTLSVersion(t *testing.T) { + testCases := []struct { + name string + version string + expect networking.ServerTLSSettings_TLSProtocol + wantErr bool + }{ + { + name: "TLS 1.0", + version: "TLSv1.0", + expect: networking.ServerTLSSettings_TLSV1_0, + }, + { + name: "TLS 1.1", + version: "TLSv1.1", + expect: networking.ServerTLSSettings_TLSV1_1, + }, + { + name: "TLS 1.2", + version: "TLSv1.2", + expect: networking.ServerTLSSettings_TLSV1_2, + }, + { + name: "TLS 1.3", + version: "TLSv1.3", + expect: networking.ServerTLSSettings_TLSV1_3, + }, + { + name: "invalid version", + version: "invalid", + expect: networking.ServerTLSSettings_TLS_AUTO, + wantErr: true, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + result, err := convertTLSVersion(tc.version) + if tc.wantErr { + if err == nil { + t.Error("Expected error but got none") + } + } else { + if err != nil { + t.Errorf("Unexpected error: %v", err) + } + if result != tc.expect { + t.Errorf("Expected %v but got %v", tc.expect, result) + } + } + }) + } +} + +func TestApplyGateway(t *testing.T) { + testCases := []struct { + name string + input *networking.Gateway + config *Ingress + expect *networking.Gateway + }{ + { + name: "apply TLS version", + input: &networking.Gateway{ + Servers: []*networking.Server{ + { + Port: &networking.Port{ + Protocol: "HTTPS", }, - expect: &DownstreamTLSConfig{ - Mode: networking.ServerTLSSettings_SIMPLE, - CipherSuites: []string{"ECDHE-RSA-AES256-GCM-SHA384", "AES128-SHA"}, - RuleMinVersion: make(map[string]string), - RuleMaxVersion: make(map[string]string), + Tls: &networking.ServerTLSSettings{ + Mode: networking.ServerTLSSettings_SIMPLE, }, + }, }, - { - name: "global TLS version only", - input: map[string]string{ - buildNginxAnnotationKey(annotationMinTLSVersion): "TLSv1_2", - buildNginxAnnotationKey(annotationMaxTLSVersion): "TLSv1_3", + }, + config: &Ingress{ + DownstreamTLS: &DownstreamTLSConfig{ + MinVersion: "TLSv1.2", + MaxVersion: "TLSv1.3", + }, + }, + expect: &networking.Gateway{ + Servers: []*networking.Server{ + { + Port: &networking.Port{ + Protocol: "HTTPS", }, - expect: &DownstreamTLSConfig{ - Mode: networking.ServerTLSSettings_SIMPLE, - MinVersion: "TLSv1_2", - MaxVersion: "TLSv1_3", - RuleMinVersion: make(map[string]string), - RuleMaxVersion: make(map[string]string), + Tls: &networking.ServerTLSSettings{ + Mode: networking.ServerTLSSettings_SIMPLE, + MinProtocolVersion: networking.ServerTLSSettings_TLSV1_2, + MaxProtocolVersion: networking.ServerTLSSettings_TLSV1_3, }, + }, }, - { - name: "rule specific TLS version", - input: map[string]string{ - buildNginxAnnotationKey(annotationMinTLSVersion + ".rule1"): "TLSv1_1", - buildNginxAnnotationKey(annotationMaxTLSVersion + ".rule1"): "TLSv1_2", + }, + }, + { + name: "complete config", + input: &networking.Gateway{ + Servers: []*networking.Server{ + { + Port: &networking.Port{ + Protocol: "HTTPS", }, - expect: &DownstreamTLSConfig{ - Mode: networking.ServerTLSSettings_SIMPLE, - RuleMinVersion: map[string]string{"rule1": "TLSv1_1"}, - RuleMaxVersion: map[string]string{"rule1": "TLSv1_2"}, + Tls: &networking.ServerTLSSettings{ + Mode: networking.ServerTLSSettings_SIMPLE, + CredentialName: "kubernetes-ingress://cluster/foo/bar", }, + }, }, - { - name: "global and rule specific TLS version", - input: map[string]string{ - buildNginxAnnotationKey(annotationMinTLSVersion): "TLSv1_2", - buildNginxAnnotationKey(annotationMaxTLSVersion): "TLSv1_3", - buildNginxAnnotationKey(annotationMinTLSVersion + ".rule1"): "TLSv1_1", - buildNginxAnnotationKey(annotationMaxTLSVersion + ".rule1"): "TLSv1_2", - }, - expect: &DownstreamTLSConfig{ - Mode: networking.ServerTLSSettings_SIMPLE, - MinVersion: "TLSv1_2", - MaxVersion: "TLSv1_3", - RuleMinVersion: map[string]string{"rule1": "TLSv1_1"}, - RuleMaxVersion: map[string]string{"rule1": "TLSv1_2"}, + }, + config: &Ingress{ + DownstreamTLS: &DownstreamTLSConfig{ + CASecretName: types.NamespacedName{ + Namespace: "foo", + Name: "bar", + }, + Mode: networking.ServerTLSSettings_MUTUAL, + CipherSuites: []string{"ECDHE-RSA-AES256-GCM-SHA384"}, + MinVersion: "TLSv1.2", + MaxVersion: "TLSv1.3", + }, + }, + expect: &networking.Gateway{ + Servers: []*networking.Server{ + {Port: &networking.Port{ + Protocol: "HTTPS", + }, + Tls: &networking.ServerTLSSettings{ + CredentialName: "kubernetes-ingress://cluster/foo/bar", + Mode: networking.ServerTLSSettings_MUTUAL, + CipherSuites: []string{"ECDHE-RSA-AES256-GCM-SHA384"}, + MinProtocolVersion: networking.ServerTLSSettings_TLSV1_2, + MaxProtocolVersion: networking.ServerTLSSettings_TLSV1_3, }, + }, }, - { - name: "multiple rules TLS version", - input: map[string]string{ - buildNginxAnnotationKey(annotationMinTLSVersion + ".rule1"): "TLSv1_1", - buildNginxAnnotationKey(annotationMaxTLSVersion + ".rule1"): "TLSv1_2", - buildNginxAnnotationKey(annotationMinTLSVersion + ".rule2"): "TLSv1_2", - buildNginxAnnotationKey(annotationMaxTLSVersion + ".rule2"): "TLSv1_3", + }, + }, + { + name: "invalid TLS version", + input: &networking.Gateway{ + Servers: []*networking.Server{ + { + Port: &networking.Port{ + Protocol: "HTTPS", }, - expect: &DownstreamTLSConfig{ - Mode: networking.ServerTLSSettings_SIMPLE, - RuleMinVersion: map[string]string{ - "rule1": "TLSv1_1", - "rule2": "TLSv1_2", - }, - RuleMaxVersion: map[string]string{ - "rule1": "TLSv1_2", - "rule2": "TLSv1_3", - }, + Tls: &networking.ServerTLSSettings{ + Mode: networking.ServerTLSSettings_SIMPLE, }, + }, }, - { - name: "complete configuration", - input: map[string]string{ - buildHigressAnnotationKey(authTLSSecret): "test/foo", - buildHigressAnnotationKey(annotationMinTLSVersion): "TLSv1_2", - buildHigressAnnotationKey(annotationMaxTLSVersion): "TLSv1_3", - buildHigressAnnotationKey(annotationMinTLSVersion + ".rule1"): "TLSv1_1", - buildHigressAnnotationKey(annotationMaxTLSVersion + ".rule1"): "TLSv1_2", - DefaultAnnotationsPrefix + "/" + sslCipher: "ECDHE-RSA-AES256-GCM-SHA384", + }, + config: &Ingress{ + DownstreamTLS: &DownstreamTLSConfig{ + MinVersion: "invalid", + MaxVersion: "invalid", + }, + }, + expect: &networking.Gateway{ + Servers: []*networking.Server{ + { + Port: &networking.Port{ + Protocol: "HTTPS", }, - expect: &DownstreamTLSConfig{ - CASecretName: types.NamespacedName{ - Namespace: "test", - Name: "foo", - }, - Mode: networking.ServerTLSSettings_MUTUAL, - MinVersion: "TLSv1_2", - MaxVersion: "TLSv1_3", - CipherSuites: []string{"ECDHE-RSA-AES256-GCM-SHA384"}, - RuleMinVersion: map[string]string{"rule1": "TLSv1_1"}, - RuleMaxVersion: map[string]string{"rule1": "TLSv1_2"}, + Tls: &networking.ServerTLSSettings{ + Mode: networking.ServerTLSSettings_SIMPLE, + // Invalid versions should default to TLS_AUTO + MinProtocolVersion: networking.ServerTLSSettings_TLS_AUTO, + MaxProtocolVersion: networking.ServerTLSSettings_TLS_AUTO, }, + }, }, - } - - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - config := &Ingress{ - Meta: Meta{ - Namespace: "foo", - }, - } - err := parser.Parse(tc.input, config, nil) - if err != nil { - t.Fatalf("Parse failed: %v", err) - } + }, + }, + } - if !reflect.DeepEqual(tc.expect, config.DownstreamTLS) { - t.Fatalf("Parse result mismatch:\nExpect: %+v\nGot: %+v", tc.expect, config.DownstreamTLS) - } - }) - } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + parser.ApplyGateway(tc.input, tc.config) + if !reflect.DeepEqual(tc.input, tc.expect) { + t.Fatalf("ApplyGateway result mismatch for %s:\nExpect: %+v\nGot: %+v", + tc.name, tc.expect, tc.input) + } + }) + } } -func TestApplyGateway(t *testing.T) { +func TestNeedDownstreamTLS(t *testing.T) { testCases := []struct { - name string - input *networking.Gateway - config *Ingress - expect *networking.Gateway + name string + annotations map[string]string + expect bool }{ - { - name: "global TLS version only", - input: &networking.Gateway{ - Servers: []*networking.Server{ - { - Name: "rule1", - Port: &networking.Port{ - Protocol: "HTTPS", - }, - Tls: &networking.ServerTLSSettings{ - Mode: networking.ServerTLSSettings_SIMPLE, - }, - }, - }, - }, - config: &Ingress{ - DownstreamTLS: &DownstreamTLSConfig{ - Mode: networking.ServerTLSSettings_SIMPLE, - MinVersion: "TLSv1_2", - MaxVersion: "TLSv1_3", - RuleMinVersion: make(map[string]string), - RuleMaxVersion: make(map[string]string), - }, - }, - expect: &networking.Gateway{ - Servers: []*networking.Server{ - { - Name: "rule1", - Port: &networking.Port{ - Protocol: "HTTPS", - }, - Tls: &networking.ServerTLSSettings{ - Mode: networking.ServerTLSSettings_SIMPLE, - MinProtocolVersion: networking.ServerTLSSettings_TLSV1_2, - MaxProtocolVersion: networking.ServerTLSSettings_TLSV1_3, - }, - }, - }, - }, - }, - { - name: "rule specific TLS version", - input: &networking.Gateway{ - Servers: []*networking.Server{ - { - Name: "rule1", - Port: &networking.Port{ - Protocol: "HTTPS", - }, - Tls: &networking.ServerTLSSettings{ - Mode: networking.ServerTLSSettings_SIMPLE, - }, - }, - }, - }, - config: &Ingress{ - DownstreamTLS: &DownstreamTLSConfig{ - Mode: networking.ServerTLSSettings_SIMPLE, - RuleMinVersion: map[string]string{ - "rule1": "TLSv1_1", - }, - RuleMaxVersion: map[string]string{ - "rule1": "TLSv1_2", - }, - }, - }, - expect: &networking.Gateway{ - Servers: []*networking.Server{ - { - Name: "rule1", - Port: &networking.Port{ - Protocol: "HTTPS", - }, - Tls: &networking.ServerTLSSettings{ - Mode: networking.ServerTLSSettings_SIMPLE, - MinProtocolVersion: networking.ServerTLSSettings_TLSV1_1, - MaxProtocolVersion: networking.ServerTLSSettings_TLSV1_2, - }, - }, - }, - }, + { + name: "empty annotations", + annotations: map[string]string{}, + expect: false, + }, + { + name: "with ssl cipher", + annotations: map[string]string{ + buildNginxAnnotationKey(sslCipher): "ECDHE-RSA-AES256-GCM-SHA384", }, - { - name: "rule override global TLS version", - input: &networking.Gateway{ - Servers: []*networking.Server{ - { - Name: "rule1", - Port: &networking.Port{ - Protocol: "HTTPS", - }, - Tls: &networking.ServerTLSSettings{ - Mode: networking.ServerTLSSettings_SIMPLE, - }, - }, - }, - }, - config: &Ingress{ - DownstreamTLS: &DownstreamTLSConfig{ - Mode: networking.ServerTLSSettings_SIMPLE, - MinVersion: "TLSv1_2", - MaxVersion: "TLSv1_3", - RuleMinVersion: map[string]string{ - "rule1": "TLSv1_1", - }, - RuleMaxVersion: map[string]string{ - "rule1": "TLSv1_2", - }, - }, - }, - expect: &networking.Gateway{ - Servers: []*networking.Server{ - { - Name: "rule1", - Port: &networking.Port{ - Protocol: "HTTPS", - }, - Tls: &networking.ServerTLSSettings{ - Mode: networking.ServerTLSSettings_SIMPLE, - MinProtocolVersion: networking.ServerTLSSettings_TLSV1_1, - MaxProtocolVersion: networking.ServerTLSSettings_TLSV1_2, - }, - }, - }, - }, + expect: true, + }, + { + name: "with TLS version", + annotations: map[string]string{ + buildNginxAnnotationKey(annotationMinTLSVersion): "TLSv1.2", }, - { - name: "complete configuration with cipher suites", - input: &networking.Gateway{ - Servers: []*networking.Server{ - { - Name: "rule1", - Port: &networking.Port{ - Protocol: "HTTPS", - }, - Tls: &networking.ServerTLSSettings{ - Mode: networking.ServerTLSSettings_SIMPLE, - CredentialName: "kubernetes-ingress://cluster/foo/bar", - }, - }, - }, - }, - config: &Ingress{ - DownstreamTLS: &DownstreamTLSConfig{ - Mode: networking.ServerTLSSettings_MUTUAL, - MinVersion: "TLSv1_2", - MaxVersion: "TLSv1_3", - RuleMinVersion: map[string]string{ - "rule1": "TLSv1_1", - }, - RuleMaxVersion: map[string]string{ - "rule1": "TLSv1_2", - }, - CipherSuites: []string{"ECDHE-RSA-AES256-GCM-SHA384"}, - CASecretName: types.NamespacedName{ - Namespace: "foo", - Name: "bar", - }, - }, - }, - expect: &networking.Gateway{ - Servers: []*networking.Server{ - { - Name: "rule1", - Port: &networking.Port{ - Protocol: "HTTPS", - }, - Tls: &networking.ServerTLSSettings{ - Mode: networking.ServerTLSSettings_MUTUAL, - CredentialName: "kubernetes-ingress://cluster/foo/bar", - MinProtocolVersion: networking.ServerTLSSettings_TLSV1_1, - MaxProtocolVersion: networking.ServerTLSSettings_TLSV1_2, - CipherSuites: []string{"ECDHE-RSA-AES256-GCM-SHA384"}, - }, - }, - }, - }, + expect: true, + }, + { + name: "with multiple TLS configs", + annotations: map[string]string{ + buildNginxAnnotationKey(sslCipher): "ECDHE-RSA-AES256-GCM-SHA384", + buildNginxAnnotationKey(annotationMinTLSVersion): "TLSv1.2", + buildNginxAnnotationKey(annotationMaxTLSVersion): "TLSv1.3", }, + expect: true, + }, } for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - parser.ApplyGateway(tc.input, tc.config) - if !reflect.DeepEqual(tc.input, tc.expect) { - t.Fatalf("ApplyGateway result mismatch:\nExpect: %+v\nGot: %+v", tc.expect, tc.input) - } - }) + t.Run(tc.name, func(t *testing.T) { + result := needDownstreamTLS(tc.annotations) + if result != tc.expect { + t.Errorf("needDownstreamTLS() for %s = %v, want %v", + tc.name, result, tc.expect) + } + }) } } - - func TestConvertTLSVersion(t *testing.T) { - testCases := []struct { - name string - version string - expect networking.ServerTLSSettings_TLSProtocol - }{ - { - name: "TLS 1.0", - version: "TLSv1_0", - expect: networking.ServerTLSSettings_TLSV1_0, - }, - { - name: "TLS 1.1", - version: "TLSv1_1", - expect: networking.ServerTLSSettings_TLSV1_1, - }, - { - name: "TLS 1.2", - version: "TLSv1_2", - expect: networking.ServerTLSSettings_TLSV1_2, - }, - { - name: "TLS 1.3", - version: "TLSv1_3", - expect: networking.ServerTLSSettings_TLSV1_3, - }, - { - name: "invalid version", - version: "invalid", - expect: networking.ServerTLSSettings_TLS_AUTO, - }, - { - name: "empty version", - version: "", - expect: networking.ServerTLSSettings_TLS_AUTO, - }, - } - - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - result := convertTLSVersion(tc.version) - if result != tc.expect { - t.Errorf("convertTLSVersion(%s) = %v, want %v", tc.version, result, tc.expect) - } - }) - } - } - - func TestNeedDownstreamTLS(t *testing.T) { - testCases := []struct { - name string - annotations map[string]string - expect bool - }{ - { - name: "empty annotations", - annotations: map[string]string{}, - expect: false, - }, - { - name: "ssl cipher only", - annotations: map[string]string{ - buildNginxAnnotationKey(sslCipher): "ECDHE-RSA-AES256-GCM-SHA384", - }, - expect: true, - }, - { - name: "auth TLS secret only", - annotations: map[string]string{ - buildNginxAnnotationKey(authTLSSecret): "test/foo", - }, - expect: true, - }, - { - name: "global TLS version only", - annotations: map[string]string{ - buildNginxAnnotationKey(annotationMinTLSVersion): "TLSv1_2", - }, - expect: true, - }, - { - name: "rule specific TLS version only", - annotations: map[string]string{ - buildNginxAnnotationKey(annotationMinTLSVersion + ".rule1"): "TLSv1_2", - }, - expect: true, - }, - { - name: "multiple TLS configurations", - annotations: map[string]string{ - buildNginxAnnotationKey(sslCipher): "ECDHE-RSA-AES256-GCM-SHA384", - buildNginxAnnotationKey(annotationMinTLSVersion): "TLSv1_2", - buildNginxAnnotationKey(annotationMinTLSVersion + ".rule1"): "TLSv1_1", - }, - expect: true, - }, - } - - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - result := needDownstreamTLS(Annotations(tc.annotations)) - if result != tc.expect { - t.Errorf("needDownstreamTLS() = %v, want %v", result, tc.expect) - } - }) - } - } - - func TestExtraSecret(t *testing.T) { - testCases := []struct { - name string - credentialName string - expect types.NamespacedName - }{ - { - name: "valid credential name", - credentialName: "kubernetes-ingress://cluster/foo/bar", - expect: types.NamespacedName{ - Namespace: "foo", - Name: "bar", - }, - }, - { - name: "invalid credential name", - credentialName: "invalid-format", - expect: types.NamespacedName{}, - }, - { - name: "empty credential name", - credentialName: "", - expect: types.NamespacedName{}, - }, - } - - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - result := extraSecret(tc.credentialName) - if !reflect.DeepEqual(result, tc.expect) { - t.Errorf("extraSecret(%s) = %v, want %v", tc.credentialName, result, tc.expect) - } - }) - } - } \ No newline at end of file From e6817328637799a76ba0c590164ab6fc36ecacbc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=BE=84=E6=BD=AD?= Date: Mon, 6 Jan 2025 19:21:32 +0800 Subject: [PATCH 4/5] Update pkg/ingress/config/ingress_config.go --- pkg/ingress/config/ingress_config.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/ingress/config/ingress_config.go b/pkg/ingress/config/ingress_config.go index eb6393d1ea..bb4b261ac1 100644 --- a/pkg/ingress/config/ingress_config.go +++ b/pkg/ingress/config/ingress_config.go @@ -151,7 +151,6 @@ type IngressConfig struct { clusterId cluster.ID httpsConfigMgr *cert.ConfigMgr - } func NewIngressConfig(localKubeClient kube.Client, xdsUpdater istiomodel.XDSUpdater, namespace string, clusterId cluster.ID) *IngressConfig { From 5ecab4c42abd907e3eadeb8cbf30527bd67dab1c Mon Sep 17 00:00:00 2001 From: yunmaoQu <2643354262@qq.com> Date: Mon, 6 Jan 2025 11:44:31 +0000 Subject: [PATCH 5/5] fix the CI error --- pkg/ingress/kube/annotations/downstreamtls.go | 24 ++++++++++--------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/pkg/ingress/kube/annotations/downstreamtls.go b/pkg/ingress/kube/annotations/downstreamtls.go index bc944d7df4..e992e61012 100644 --- a/pkg/ingress/kube/annotations/downstreamtls.go +++ b/pkg/ingress/kube/annotations/downstreamtls.go @@ -16,6 +16,7 @@ package annotations import ( "strings" + "fmt" networking "istio.io/api/networking/v1alpha3" gatewaytool "istio.io/istio/pkg/config/gateway" @@ -148,15 +149,16 @@ func needDownstreamTLS(annotations Annotations) bool { } func convertTLSVersion(version string) (networking.ServerTLSSettings_TLSProtocol, error) { - switch version { - case "TLSv1.0": - return networking.ServerTLSSettings_TLSV1_0 , nil - case "TLSv1.1": - return networking.ServerTLSSettings_TLSV1_1 , nil - case "TLSv1.2": - return networking.ServerTLSSettings_TLSV1_2 , nil - case "TLSv1.3": - default: - return networking.ServerTLSSettings_TLS_AUTO, fmt.Errorf("invalid TLS version: %s. Valid values are: TLSv1.0, TLSv1.1, TLSv1.2, TLSv1.3", version) - } + switch version { + case "TLSv1.0": + return networking.ServerTLSSettings_TLSV1_0 , nil + case "TLSv1.1": + return networking.ServerTLSSettings_TLSV1_1 , nil + case "TLSv1.2": + return networking.ServerTLSSettings_TLSV1_2 , nil + case "TLSv1.3": + default: + return networking.ServerTLSSettings_TLS_AUTO, fmt.Errorf("invalid TLS version: %s. Valid values are: TLSv1.0, TLSv1.1, TLSv1.2, TLSv1.3", version) + } + return networking.ServerTLSSettings_TLS_AUTO, fmt.Errorf("unreachable code, but required by compiler") }