This is an automated email from the ASF dual-hosted git repository.
ronething pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/apisix-ingress-controller.git
The following commit(s) were added to refs/heads/master by this push:
new 5f0d1af1 fix: generate unique SSL IDs to prevent certificate conflicts
across different hosts (#2592)
5f0d1af1 is described below
commit 5f0d1af1dea737f8c509ee6f52b34f55bccd5b2d
Author: Ashing Zheng <[email protected]>
AuthorDate: Mon Oct 13 17:07:52 2025 +0800
fix: generate unique SSL IDs to prevent certificate conflicts across
different hosts (#2592)
Signed-off-by: Ashing Zheng <[email protected]>
---
api/adc/types.go | 13 +++++
internal/adc/translator/apisixtls.go | 3 +-
internal/adc/translator/gateway.go | 51 +-----------------
internal/adc/translator/ingress.go | 8 +--
internal/provider/init/init.go | 3 +-
internal/provider/register.go | 3 +-
test/e2e/crds/v2/tls.go | 102 +++++++++++++++++++++++++++++++++++
test/e2e/gatewayapi/gateway.go | 6 +--
test/e2e/gatewayapi/tcproute.go | 16 ++----
test/e2e/scaffold/assertion.go | 53 ++++++++++++------
10 files changed, 172 insertions(+), 86 deletions(-)
diff --git a/api/adc/types.go b/api/adc/types.go
index 48abd10f..8c2a9506 100644
--- a/api/adc/types.go
+++ b/api/adc/types.go
@@ -473,6 +473,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 2f05facf..bd67893e 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 db284845..2ee2454c 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 f17b159f..98cb4c3e 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 b6ed9e99..be21c07d 100644
--- a/internal/provider/init/init.go
+++ b/internal/provider/init/init.go
@@ -18,11 +18,12 @@
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"
"github.com/apache/apisix-ingress-controller/internal/provider/apisix"
- "github.com/go-logr/logr"
)
func init() {
diff --git a/internal/provider/register.go b/internal/provider/register.go
index fddb1af5..a9feb032 100644
--- a/internal/provider/register.go
+++ b/internal/provider/register.go
@@ -21,9 +21,10 @@ 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"
- "github.com/go-logr/logr"
)
type RegisterHandler interface {
diff --git a/test/e2e/crds/v2/tls.go b/test/e2e/crds/v2/tls.go
index 1564fb15..a7edff3d 100644
--- a/test/e2e/crds/v2/tls.go
+++ b/test/e2e/crds/v2/tls.go
@@ -336,5 +336,107 @@ spec:
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 c161b499..f1e8ca3a 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 d9c30375..b6de0a42 100644
--- a/test/e2e/gatewayapi/tcproute.go
+++ b/test/e2e/gatewayapi/tcproute.go
@@ -72,25 +72,17 @@ spec:
NotTo(HaveOccurred(), "creating GatewayProxy")
// Create GatewayClass
- gatewayClassName := s.Namespace()
Expect(s.CreateResourceFromString(s.GetGatewayClassYaml())).
NotTo(HaveOccurred(), "creating GatewayClass")
- gcyaml, _ := s.GetResourceYaml("GatewayClass",
gatewayClassName)
- s.ResourceApplied("GatewayClass", gatewayClassName,
gcyaml, 1)
// Create Gateway with TCP listener
- gatewayName := s.Namespace()
-
Expect(s.CreateResourceFromString(fmt.Sprintf(tcpGateway, gatewayName,
gatewayClassName))).
+
Expect(s.CreateResourceFromString(fmt.Sprintf(tcpGateway, s.Namespace(),
s.Namespace()))).
NotTo(HaveOccurred(), "creating Gateway")
-
- gwyaml, _ := s.GetResourceYaml("Gateway", gatewayName)
- s.ResourceApplied("Gateway", gatewayName, gwyaml, 1)
})
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")
// Verify TCPRoute status becomes programmed
@@ -98,7 +90,7 @@ spec:
s.ResourceApplied("TCPRoute", "tcp-app-1", routeYaml, 1)
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(),
@@ -113,7 +105,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 612e2c81..a7a18246 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 {
