From 54e625033d4aa554da4e3255a701baab3a4c7e1f Mon Sep 17 00:00:00 2001 From: Ashing Zheng Date: Mon, 13 Oct 2025 17:07:52 +0800 Subject: [PATCH 1/3] fix: generate unique SSL IDs to prevent certificate conflicts across different hosts (#2592) Signed-off-by: Ashing Zheng --- api/adc/types.go | 13 ++++ internal/adc/translator/apisixtls.go | 3 +- internal/adc/translator/gateway.go | 51 +------------ internal/adc/translator/ingress.go | 8 +-- test/e2e/crds/v2/tls.go | 103 +++++++++++++++++++++++++++ test/e2e/gatewayapi/gateway.go | 6 +- test/e2e/gatewayapi/tcproute.go | 9 +-- test/e2e/scaffold/assertion.go | 53 ++++++++++---- 8 files changed, 170 insertions(+), 76 deletions(-) diff --git a/api/adc/types.go b/api/adc/types.go index fd59546fb..3d7d35dd1 100644 --- a/api/adc/types.go +++ b/api/adc/types.go @@ -472,6 +472,19 @@ func (n Upstream) MarshalJSON() ([]byte, error) { return json.Marshal((Alias)(n)) } +func ComposeSSLName(kind, namespace, name string) string { + p := make([]byte, 0, len(kind)+len(namespace)+len(name)+2) + buf := bytes.NewBuffer(p) + + buf.WriteString(kind) + buf.WriteByte('_') + buf.WriteString(namespace) + buf.WriteByte('_') + buf.WriteString(name) + + return buf.String() +} + // ComposeRouteName uses namespace, name and rule name to compose // the route name. func ComposeRouteName(namespace, name string, rule string) string { diff --git a/internal/adc/translator/apisixtls.go b/internal/adc/translator/apisixtls.go index 2f05facf0..bd67893ea 100644 --- a/internal/adc/translator/apisixtls.go +++ b/internal/adc/translator/apisixtls.go @@ -27,6 +27,7 @@ import ( "github.com/apache/apisix-ingress-controller/internal/controller/label" "github.com/apache/apisix-ingress-controller/internal/id" "github.com/apache/apisix-ingress-controller/internal/provider" + internaltypes "github.com/apache/apisix-ingress-controller/internal/types" ) func (t *Translator) TranslateApisixTls(tctx *provider.TranslateContext, tls *apiv2.ApisixTls) (*TranslateResult, error) { @@ -57,7 +58,7 @@ func (t *Translator) TranslateApisixTls(tctx *provider.TranslateContext, tls *ap // Create SSL object ssl := &adctypes.SSL{ Metadata: adctypes.Metadata{ - ID: id.GenID(tls.Namespace + "_" + tls.Name), + ID: id.GenID(adctypes.ComposeSSLName(internaltypes.KindApisixTls, tls.Namespace, tls.Name)), Labels: label.GenLabel(tls), }, Certificates: []adctypes.Certificate{ diff --git a/internal/adc/translator/gateway.go b/internal/adc/translator/gateway.go index db2848452..2ee2454cb 100644 --- a/internal/adc/translator/gateway.go +++ b/internal/adc/translator/gateway.go @@ -22,7 +22,6 @@ import ( "encoding/json" "encoding/pem" "fmt" - "slices" "github.com/pkg/errors" corev1 "k8s.io/api/core/v1" @@ -50,7 +49,6 @@ func (t *Translator) TranslateGateway(tctx *provider.TranslateContext, obj *gate result.SSL = append(result.SSL, ssl...) } } - result.SSL = mergeSSLWithSameID(result.SSL) rk := utils.NamespacedNameKind(obj) gatewayProxy, ok := tctx.GatewayProxies[rk] @@ -80,7 +78,7 @@ func (t *Translator) translateSecret(tctx *provider.TranslateContext, listener g sslObjs := make([]*adctypes.SSL, 0) switch *listener.TLS.Mode { case gatewayv1.TLSModeTerminate: - for _, ref := range listener.TLS.CertificateRefs { + for refIndex, ref := range listener.TLS.CertificateRefs { ns := obj.GetNamespace() if ref.Namespace != nil { ns = string(*ref.Namespace) @@ -122,8 +120,7 @@ func (t *Translator) translateSecret(tctx *provider.TranslateContext, listener g } sslObj.Snis = append(sslObj.Snis, hosts...) } - // Note: use cert as id to avoid duplicate certificate across ssl objects - sslObj.ID = id.GenID(string(cert)) + sslObj.ID = id.GenID(fmt.Sprintf("%s_%s_%d", adctypes.ComposeSSLName(internaltypes.KindGateway, obj.Namespace, obj.Name), listener.Name, refIndex)) t.Log.V(1).Info("generated ssl id", "ssl id", sslObj.ID, "secret", secretNN.String()) sslObj.Labels = label.GenLabel(obj) sslObjs = append(sslObjs, sslObj) @@ -241,47 +238,3 @@ func (t *Translator) fillPluginMetadataFromGatewayProxy(pluginMetadata adctypes. pluginMetadata[pluginName] = pluginConfig } } - -// mergeSSLWithSameID merge ssl with same id -func mergeSSLWithSameID(sslList []*adctypes.SSL) []*adctypes.SSL { - if len(sslList) <= 1 { - return sslList - } - - // create a map to store ssl with same id - sslMap := make(map[string]*adctypes.SSL) - for _, ssl := range sslList { - if existing, exists := sslMap[ssl.ID]; exists { - // if ssl with same id exists, merge their snis - // use map to deduplicate - sniMap := make(map[string]struct{}) - // add existing snis - for _, sni := range existing.Snis { - sniMap[sni] = struct{}{} - } - // add new snis - for _, sni := range ssl.Snis { - sniMap[sni] = struct{}{} - } - // rebuild deduplicated snis list - newSnis := make([]string, 0, len(sniMap)) - for sni := range sniMap { - newSnis = append(newSnis, sni) - } - - slices.Sort(newSnis) - // update existing ssl object - existing.Snis = newSnis - } else { - slices.Sort(ssl.Snis) - // if new ssl id, add to map - sslMap[ssl.ID] = ssl - } - } - - mergedSSL := make([]*adctypes.SSL, 0, len(sslMap)) - for _, ssl := range sslMap { - mergedSSL = append(mergedSSL, ssl) - } - return mergedSSL -} diff --git a/internal/adc/translator/ingress.go b/internal/adc/translator/ingress.go index f17b159fa..98cb4c3e3 100644 --- a/internal/adc/translator/ingress.go +++ b/internal/adc/translator/ingress.go @@ -33,7 +33,7 @@ import ( internaltypes "github.com/apache/apisix-ingress-controller/internal/types" ) -func (t *Translator) translateIngressTLS(ingressTLS *networkingv1.IngressTLS, secret *corev1.Secret, labels map[string]string) (*adctypes.SSL, error) { +func (t *Translator) translateIngressTLS(namespace, name string, tlsIndex int, ingressTLS *networkingv1.IngressTLS, secret *corev1.Secret, labels map[string]string) (*adctypes.SSL, error) { // extract the key pair from the secret cert, key, err := extractKeyPair(secret, true) if err != nil { @@ -64,7 +64,7 @@ func (t *Translator) translateIngressTLS(ingressTLS *networkingv1.IngressTLS, se }, Snis: hosts, } - ssl.ID = id.GenID(string(cert)) + ssl.ID = id.GenID(fmt.Sprintf("%s_%d", adctypes.ComposeSSLName(internaltypes.KindIngress, namespace, name), tlsIndex)) return ssl, nil } @@ -75,7 +75,7 @@ func (t *Translator) TranslateIngress(tctx *provider.TranslateContext, obj *netw labels := label.GenLabel(obj) // handle TLS configuration, convert to SSL objects - for _, tls := range obj.Spec.TLS { + for tlsIndex, tls := range obj.Spec.TLS { if tls.SecretName == "" { continue } @@ -86,7 +86,7 @@ func (t *Translator) TranslateIngress(tctx *provider.TranslateContext, obj *netw if secret == nil { continue } - ssl, err := t.translateIngressTLS(&tls, secret, labels) + ssl, err := t.translateIngressTLS(obj.Namespace, obj.Name, tlsIndex, &tls, secret, labels) if err != nil { return nil, err } diff --git a/test/e2e/crds/v2/tls.go b/test/e2e/crds/v2/tls.go index ce3748bbf..6f1c8df4c 100644 --- a/test/e2e/crds/v2/tls.go +++ b/test/e2e/crds/v2/tls.go @@ -310,5 +310,108 @@ spec: assert.Equal(GinkgoT(), int64(10), *tls[0].Client.Depth, "client depth should be 10") assert.Contains(GinkgoT(), tls[0].Client.SkipMtlsURIRegex, skipMtlsUriRegex, "skip_mtls_uri_regex should be set") }) + + It("ApisixTls and Ingress with same certificate but different hosts", func() { + By("create shared TLS secret") + err := s.NewKubeTlsSecret("shared-tls-secret", Cert, Key) + Expect(err).NotTo(HaveOccurred(), "creating shared TLS secret") + + const apisixTlsSpec = ` +apiVersion: apisix.apache.org/v2 +kind: ApisixTls +metadata: + name: test-apisixtls-shared +spec: + ingressClassName: %s + hosts: + - api6.com + secret: + name: shared-tls-secret + namespace: %s +` + + By("apply ApisixTls with api6.com") + var apisixTls apiv2.ApisixTls + tlsSpec := fmt.Sprintf(apisixTlsSpec, s.Namespace(), s.Namespace()) + applier.MustApplyAPIv2(types.NamespacedName{Namespace: s.Namespace(), Name: "test-apisixtls-shared"}, &apisixTls, tlsSpec) + + const ingressYamlWithTLS = ` +apiVersion: networking.k8s.io/v1 +kind: Ingress +metadata: + name: test-ingress-tls-shared +spec: + ingressClassName: %s + tls: + - hosts: + - api7.com + secretName: shared-tls-secret + rules: + - host: api7.com + http: + paths: + - path: / + pathType: Prefix + backend: + service: + name: httpbin-service-e2e-test + port: + number: 80 +` + + By("apply Ingress with api7.com using same certificate") + err = s.CreateResourceFromString(fmt.Sprintf(ingressYamlWithTLS, s.Namespace())) + Expect(err).NotTo(HaveOccurred(), "creating Ingress") + + By("verify two SSL objects exist in control plane") + Eventually(func() bool { + tls, err := s.DefaultDataplaneResource().SSL().List(context.Background()) + if err != nil { + return false + } + return len(tls) == 2 + }).WithTimeout(30 * time.Second).ProbeEvery(1 * time.Second).Should(BeTrue()) + + tls, err := s.DefaultDataplaneResource().SSL().List(context.Background()) + assert.Nil(GinkgoT(), err, "list tls error") + assert.Len(GinkgoT(), tls, 2, "should have exactly 2 SSL objects") + + By("verify SSL objects have different IDs and SNIs") + sniFound := make(map[string]bool) + + for i := range tls { + // Check certificate content is the same + assert.Len(GinkgoT(), tls[i].Certificates, 1, "each SSL should have 1 certificate") + assert.Equal(GinkgoT(), Cert, tls[i].Certificates[0].Certificate, "certificate should match") + + // Track SNIs + for _, sni := range tls[i].Snis { + sniFound[sni] = true + } + } + + By("verify both hosts are covered") + assert.True(GinkgoT(), sniFound["api6.com"], "api6.com should be in SNIs") + assert.True(GinkgoT(), sniFound["api7.com"], "api7.com should be in SNIs") + + By("test HTTPS request to api6.com") + Eventually(func() int { + return s.NewAPISIXHttpsClient("api6.com"). + GET("/get"). + WithHost("api6.com"). + Expect(). + Raw().StatusCode + }).WithTimeout(30 * time.Second).ProbeEvery(1 * time.Second).Should(Equal(http.StatusOK)) + + By("test HTTPS request to api7.com") + Eventually(func() int { + return s.NewAPISIXHttpsClient("api7.com"). + GET("/get"). + WithHost("api7.com"). + Expect(). + Raw().StatusCode + }).WithTimeout(30 * time.Second).ProbeEvery(1 * time.Second).Should(Equal(http.StatusOK)) + }) + }) }) diff --git a/test/e2e/gatewayapi/gateway.go b/test/e2e/gatewayapi/gateway.go index c161b4990..f1e8ca3a0 100644 --- a/test/e2e/gatewayapi/gateway.go +++ b/test/e2e/gatewayapi/gateway.go @@ -284,8 +284,8 @@ spec: Eventually(func() error { tls, err := s.DefaultDataplaneResource().SSL().List(context.Background()) Expect(err).NotTo(HaveOccurred(), "list ssl") - if len(tls) != 1 { - return fmt.Errorf("expect 1 ssl, got %d", len(tls)) + if len(tls) != 2 { + return fmt.Errorf("expect 2 ssl, got %d", len(tls)) } if len(tls[0].Certificates) != 1 { return fmt.Errorf("expect 1 certificate, got %d", len(tls[0].Certificates)) @@ -305,7 +305,7 @@ spec: Eventually(func() string { tls, err := s.DefaultDataplaneResource().SSL().List(context.Background()) Expect(err).NotTo(HaveOccurred(), "list ssl") - if len(tls) < 1 { + if len(tls) != 2 { return "" } if len(tls[0].Certificates) < 1 { diff --git a/test/e2e/gatewayapi/tcproute.go b/test/e2e/gatewayapi/tcproute.go index 7f9e73602..7ed014492 100644 --- a/test/e2e/gatewayapi/tcproute.go +++ b/test/e2e/gatewayapi/tcproute.go @@ -72,6 +72,8 @@ spec: // Create GatewayClass Expect(s.CreateResourceFromString(s.GetGatewayClassYaml())).NotTo(HaveOccurred(), "creating GatewayClass") + Expect(s.CreateResourceFromString(s.GetGatewayClassYaml())). + NotTo(HaveOccurred(), "creating GatewayClass") // Create Gateway with TCP listener Expect(s.CreateResourceFromString(fmt.Sprintf(tcpGateway, s.Namespace(), s.Namespace()))). @@ -79,14 +81,13 @@ spec: }) It("should route TCP traffic to backend service", func() { - gatewayName := s.Namespace() By("creating TCPRoute") - Expect(s.CreateResourceFromString(fmt.Sprintf(tcpRoute, gatewayName))). + Expect(s.CreateResourceFromString(fmt.Sprintf(tcpRoute, s.Namespace()))). NotTo(HaveOccurred(), "creating TCPRoute") time.Sleep(2 * time.Second) By("verifying TCPRoute is functional") - s.HTTPOverTCPConnectAssert(true, time.Minute*5) // should be able to connect + s.HTTPOverTCPConnectAssert(true, time.Minute*3) // should be able to connect By("sending TCP traffic to verify routing") s.RequestAssert(&scaffold.RequestAssert{ Client: s.NewAPISIXClientOnTCPPort(), @@ -101,7 +102,7 @@ spec: Expect(s.DeleteResource("TCPRoute", "tcp-app-1")). NotTo(HaveOccurred(), "deleting TCPRoute") - s.HTTPOverTCPConnectAssert(false, time.Minute*5) + s.HTTPOverTCPConnectAssert(false, time.Minute*3) }) }) }) diff --git a/test/e2e/scaffold/assertion.go b/test/e2e/scaffold/assertion.go index 612e2c817..a7a18246a 100644 --- a/test/e2e/scaffold/assertion.go +++ b/test/e2e/scaffold/assertion.go @@ -200,35 +200,58 @@ func (s *Scaffold) HTTPOverTCPConnectAssert(shouldRespond bool, timeout time.Dur defer func() { _ = conn.Close() }() - _, _ = fmt.Fprintf(conn, "GET /get HTTP/1.1\r\nHost: localhost\r\n\r\n") + _, _ = fmt.Fprintf(conn, "GET /get HTTP/1.1\r\nHost: localhost\r\nConnection: close\r\n\r\n") + + // Read response - loop until EOF or error + // Set deadline for total read time (not per-read) + _ = conn.SetReadDeadline(time.Now().Add(10 * time.Second)) + var responseBuilder strings.Builder + buf := make([]byte, 4096) + + for { + n, err := conn.Read(buf) + if n > 0 { + responseBuilder.Write(buf[:n]) + } - // Read response - _ = conn.SetReadDeadline(time.Now().Add(2 * time.Second)) - buf := make([]byte, 1024) - n, err := conn.Read(buf) + // EOF is expected with Connection: close + if err == io.EOF { + break + } + + // Other errors (including timeout) + if err != nil { + // If we haven't received any data yet, this is an error + if responseBuilder.Len() == 0 { + return fmt.Errorf("read error before receiving data: %v", err) + } + // If we have partial data, treat timeout as potential issue + if strings.Contains(err.Error(), "timeout") { + break // Use whatever we've received so far + } + return fmt.Errorf("read error: %v", err) + } + } + + response := responseBuilder.String() if shouldRespond { // Should get a response (HTTP 200 from httpbin) - if err != nil || n == 0 { - return fmt.Errorf("expected response but got error: %v or empty response", err) + if len(response) == 0 { + return fmt.Errorf("expected response but got empty response") } // Check if we got a valid HTTP response - response := string(buf[:n]) if !strings.Contains(response, "HTTP/1.1") { return fmt.Errorf("expected HTTP response but got: %s", response) } } else { // Should get no response or connection reset - if err == nil && n > 0 { - return fmt.Errorf("expected no response but got: %s", string(buf[:n])) - } - // EOF or timeout is expected when no route is configured - if err != io.EOF && !strings.Contains(err.Error(), "timeout") { - return fmt.Errorf("expected EOF or timeout but got: %v", err) + if len(response) > 0 { + return fmt.Errorf("expected no response but got: %s", response) } } return nil - }).WithTimeout(timeout).WithPolling(2 * time.Second).Should(Succeed()) + }).WithTimeout(timeout).WithPolling(10 * time.Second).Should(Succeed()) } func (s *Scaffold) RequestAssert(r *RequestAssert) bool { From be41e0917b2c41be9d15a0f6be05fcf239306905 Mon Sep 17 00:00:00 2001 From: Ashing Zheng Date: Wed, 5 Nov 2025 18:19:01 +0800 Subject: [PATCH 2/3] fix: r Signed-off-by: Ashing Zheng --- test/e2e/gatewayapi/gateway.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/e2e/gatewayapi/gateway.go b/test/e2e/gatewayapi/gateway.go index f1e8ca3a0..97eee6ed2 100644 --- a/test/e2e/gatewayapi/gateway.go +++ b/test/e2e/gatewayapi/gateway.go @@ -256,7 +256,7 @@ spec: name: %s - name: https-with-hostname port: 443 - hostname: api6.com + hostname: test.com protocol: HTTPS allowedRoutes: namespaces: From 0dd76e06dc6129325c1025947c81bf51efbb8aca Mon Sep 17 00:00:00 2001 From: Ashing Zheng Date: Thu, 6 Nov 2025 01:04:31 +0800 Subject: [PATCH 3/3] fix: r Signed-off-by: Ashing Zheng --- test/e2e/crds/v2/tls.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/e2e/crds/v2/tls.go b/test/e2e/crds/v2/tls.go index 6f1c8df4c..ef4acba02 100644 --- a/test/e2e/crds/v2/tls.go +++ b/test/e2e/crds/v2/tls.go @@ -312,6 +312,9 @@ spec: }) It("ApisixTls and Ingress with same certificate but different hosts", func() { + if framework.IngressVersion != "v1" { + Skip("skipping test in non-v1 ingress version") + } By("create shared TLS secret") err := s.NewKubeTlsSecret("shared-tls-secret", Cert, Key) Expect(err).NotTo(HaveOccurred(), "creating shared TLS secret")