diff --git a/api/adc/types.go b/api/adc/types.go index 472a12a10..d66886e38 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 43fc765f2..202c93fd2 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/api7/gopkg/pkg/log" "github.com/pkg/errors" @@ -52,7 +51,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] @@ -82,7 +80,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) @@ -123,9 +121,14 @@ func (t *Translator) translateSecret(tctx *provider.TranslateContext, listener g } sslObj.Snis = append(sslObj.Snis, hosts...) } +<<<<<<< HEAD // Note: use cert as id to avoid duplicate certificate across ssl objects sslObj.ID = id.GenID(string(cert)) log.Debugw("generated ssl id", zap.String("ssl id", sslObj.ID), zap.String("secret", secret.Namespace+"/"+secret.Name)) +======= + 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()) +>>>>>>> 5f0d1af1 (fix: generate unique SSL IDs to prevent certificate conflicts across different hosts (#2592)) sslObj.Labels = label.GenLabel(obj) sslObjs = append(sslObjs, sslObj) } @@ -242,47 +245,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/internal/provider/init/init.go b/internal/provider/init/init.go index efd5b43d1..032eaeb3a 100644 --- a/internal/provider/init/init.go +++ b/internal/provider/init/init.go @@ -18,6 +18,8 @@ package init import ( + "github.com/go-logr/logr" + "github.com/apache/apisix-ingress-controller/internal/controller/status" "github.com/apache/apisix-ingress-controller/internal/manager/readiness" "github.com/apache/apisix-ingress-controller/internal/provider" diff --git a/internal/provider/register.go b/internal/provider/register.go index 25cc670dc..021df18c3 100644 --- a/internal/provider/register.go +++ b/internal/provider/register.go @@ -21,6 +21,8 @@ import ( "fmt" "net/http" + "github.com/go-logr/logr" + "github.com/apache/apisix-ingress-controller/internal/controller/status" "github.com/apache/apisix-ingress-controller/internal/manager/readiness" ) diff --git a/test/e2e/crds/v2/tls.go b/test/e2e/crds/v2/tls.go index ce3748bbf..75589ce78 100644 --- a/test/e2e/crds/v2/tls.go +++ b/test/e2e/crds/v2/tls.go @@ -310,5 +310,111 @@ 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") }) +<<<<<<< HEAD +======= + + 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)) + }) + +>>>>>>> 5f0d1af1 (fix: generate unique SSL IDs to prevent certificate conflicts across different hosts (#2592)) }) }) 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 new file mode 100644 index 000000000..b6de0a42c --- /dev/null +++ b/test/e2e/gatewayapi/tcproute.go @@ -0,0 +1,111 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you 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 gatewayapi + +import ( + "fmt" + "time" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + "github.com/apache/apisix-ingress-controller/test/e2e/scaffold" +) + +var _ = Describe("TCPRoute E2E Test", Label("networking.k8s.io", "tcproute"), func() { + s := scaffold.NewDefaultScaffold() + Context("TCPRoute Base", func() { + var tcpGateway = ` +apiVersion: gateway.networking.k8s.io/v1 +kind: Gateway +metadata: + name: %s +spec: + gatewayClassName: %s + listeners: + - name: tcp + protocol: TCP + port: 80 + allowedRoutes: + kinds: + - kind: TCPRoute + infrastructure: + parametersRef: + group: apisix.apache.org + kind: GatewayProxy + name: apisix-proxy-config +` + + var tcpRoute = ` +apiVersion: gateway.networking.k8s.io/v1alpha2 +kind: TCPRoute +metadata: + name: tcp-app-1 +spec: + parentRefs: + - name: %s + sectionName: tcp + rules: + - backendRefs: + - name: httpbin-service-e2e-test + port: 80 +` + + BeforeEach(func() { + // Create GatewayProxy + Expect(s.CreateResourceFromString(s.GetGatewayProxySpec())). + NotTo(HaveOccurred(), "creating GatewayProxy") + + // Create 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()))). + NotTo(HaveOccurred(), "creating Gateway") + }) + + It("should route TCP traffic to backend service", func() { + By("creating TCPRoute") + Expect(s.CreateResourceFromString(fmt.Sprintf(tcpRoute, s.Namespace()))). + NotTo(HaveOccurred(), "creating TCPRoute") + + // Verify TCPRoute status becomes programmed + routeYaml, _ := s.GetResourceYaml("TCPRoute", "tcp-app-1") + s.ResourceApplied("TCPRoute", "tcp-app-1", routeYaml, 1) + + By("verifying TCPRoute is functional") + 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(), + Method: "GET", + Path: "/get", + Check: scaffold.WithExpectedStatus(200), + Timeout: time.Second * 60, + Interval: time.Second * 2, + }) + + By("deleting TCPRoute") + Expect(s.DeleteResource("TCPRoute", "tcp-app-1")). + NotTo(HaveOccurred(), "deleting TCPRoute") + + s.HTTPOverTCPConnectAssert(false, time.Minute*3) + }) + }) +}) diff --git a/test/e2e/scaffold/assertion.go b/test/e2e/scaffold/assertion.go index ec0d2f210..a8dc70c7c 100644 --- a/test/e2e/scaffold/assertion.go +++ b/test/e2e/scaffold/assertion.go @@ -189,6 +189,72 @@ func WithExpectedNotHeaders(unexpectedHeaders []string) ResponseCheckFunc { } } +<<<<<<< HEAD +======= +func (s *Scaffold) HTTPOverTCPConnectAssert(shouldRespond bool, timeout time.Duration) { + EventuallyWithOffset(1, func() error { + conn, err := net.DialTimeout("tcp", s.GetAPISIXTCPEndpoint(), 3*time.Second) + if err != nil { + return fmt.Errorf("failed to connect: %v", err) + } + defer func() { + _ = conn.Close() + }() + _, _ = 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]) + } + + // 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 len(response) == 0 { + return fmt.Errorf("expected response but got empty response") + } + // Check if we got a valid HTTP response + 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 len(response) > 0 { + return fmt.Errorf("expected no response but got: %s", response) + } + } + return nil + }).WithTimeout(timeout).WithPolling(10 * time.Second).Should(Succeed()) +} + +>>>>>>> 5f0d1af1 (fix: generate unique SSL IDs to prevent certificate conflicts across different hosts (#2592)) func (s *Scaffold) RequestAssert(r *RequestAssert) bool { if r.Client == nil { r.Client = s.NewAPISIXClient()