Copilot commented on code in PR #2603: URL: https://github.com/apache/apisix-ingress-controller/pull/2603#discussion_r2431161679
########## internal/webhook/v1/ssl/conflict_detector.go: ########## @@ -0,0 +1,474 @@ +// 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 ssl + +import ( + "context" + "fmt" + "strings" + + corev1 "k8s.io/api/core/v1" + networkingv1 "k8s.io/api/networking/v1" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/log" + gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" + + v1alpha1 "github.com/apache/apisix-ingress-controller/api/v1alpha1" + apiv2 "github.com/apache/apisix-ingress-controller/api/v2" + "github.com/apache/apisix-ingress-controller/internal/controller" + "github.com/apache/apisix-ingress-controller/internal/controller/config" + "github.com/apache/apisix-ingress-controller/internal/controller/indexer" + sslutil "github.com/apache/apisix-ingress-controller/internal/ssl" + internaltypes "github.com/apache/apisix-ingress-controller/internal/types" +) + +var logger = log.Log.WithName("ssl-conflict-detector") + +// HostCertMapping represents the relationship between a host and its certificate hash. +type HostCertMapping struct { + Host string + CertificateHash string + ResourceRef string +} + +// SSLConflict exposes the conflict details to the admission webhook for reporting. +type SSLConflict struct { + Host string + ConflictingResource string + CertificateHash string +} + +// ConflictDetector detects SSL conflicts among Gateway, Ingress, and ApisixTls resources. +type ConflictDetector struct { + client client.Client + secretCache map[types.NamespacedName]*secretInfo +} + +type secretInfo struct { + hash string + hosts []string +} + +// NewConflictDetector creates a detector backed by the provided client. +func NewConflictDetector(c client.Client) *ConflictDetector { + return &ConflictDetector{ + client: c, + secretCache: make(map[types.NamespacedName]*secretInfo), + } +} + +// DetectConflicts returns the list of conflicts between the provided mappings and +// existing resources that are associated with the same GatewayProxy. Best-effort: +// failures while enumerating existing resources or reading Secrets will be logged +// and result in no conflicts instead of blocking the admission. +func (d *ConflictDetector) DetectConflicts(ctx context.Context, obj client.Object, newMappings []HostCertMapping) ([]SSLConflict, error) { + gatewayProxy, err := d.resolveGatewayProxy(ctx, obj) + if err != nil { + logger.Error(err, "failed to resolve GatewayProxy", "object", objectKey(obj)) + return nil, nil + } + if gatewayProxy == nil { + return nil, nil + } + + existingMappings, err := d.collectExistingMappings(ctx, gatewayProxy, obj.GetUID()) + if err != nil { + logger.Error(err, "failed to collect existing SSL mappings", "gatewayProxy", objectKey(gatewayProxy)) + return nil, nil + } + + conflicts := make([]SSLConflict, 0) + byHost := make(map[string]HostCertMapping, len(existingMappings)) + for _, mapping := range existingMappings { + if mapping.Host == "" || mapping.CertificateHash == "" { + continue + } + if existing, ok := byHost[mapping.Host]; ok { + if existing.CertificateHash == mapping.CertificateHash { + continue + } + // keep the first encountered mapping, this will not happen in normal cases Review Comment: [nitpick] The comment 'this will not happen in normal cases' suggests unexpected behavior. Consider adding error logging or a more explicit comment explaining when this scenario might occur and why it's being silently ignored. ```suggestion // keep the first encountered mapping, this should not happen in normal cases. logger.Info("duplicate host with different certificate hash found in existingMappings; ignoring subsequent mapping", "host", mapping.Host, "firstCertificateHash", existing.CertificateHash, "secondCertificateHash", mapping.CertificateHash, "firstResourceRef", existing.ResourceRef, "secondResourceRef", mapping.ResourceRef) ``` ########## test/e2e/webhook/ssl_conflict.go: ########## @@ -0,0 +1,933 @@ +// 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 webhook + +import ( + "crypto/rand" + "crypto/rsa" + "crypto/x509" + "crypto/x509/pkix" + "encoding/base64" + "encoding/pem" + "fmt" + "math/big" + "time" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + "github.com/apache/apisix-ingress-controller/test/e2e/scaffold" +) + +var _ = Describe("Test SSL/TLS Conflict Detection", Label("webhook"), func() { + s := scaffold.NewScaffold(scaffold.Options{ + Name: "ssl-conflict-test", + EnableWebhook: true, + }) + + BeforeEach(func() { + By("creating GatewayProxy") + err := s.CreateResourceFromString(s.GetGatewayProxySpec()) + Expect(err).NotTo(HaveOccurred(), "creating GatewayProxy") + time.Sleep(5 * time.Second) + + By("creating GatewayClass") + err = s.CreateResourceFromString(s.GetGatewayClassYaml()) + Expect(err).NotTo(HaveOccurred(), "creating GatewayClass") + time.Sleep(2 * time.Second) + + By("creating IngressClass") + err = s.CreateResourceFromStringWithNamespace(s.GetIngressClassYaml(), "") + Expect(err).NotTo(HaveOccurred(), "creating IngressClass") + time.Sleep(2 * time.Second) + }) + + Context("ApisixTls conflict detection", func() { + It("should reject ApisixTls with conflicting certificate for same host", func() { + host := "conflict.example.com" + secretA := "tls-cert-a" + secretB := "tls-cert-b" + + By("creating two different TLS secrets") + certA, keyA := generateTLSCertificate([]string{host}) + secretAYAML := fmt.Sprintf(` +apiVersion: v1 +kind: Secret +metadata: + name: %s + namespace: %s +type: kubernetes.io/tls +data: + tls.crt: %s + tls.key: %s +`, secretA, s.Namespace(), certA, keyA) + err := s.CreateResourceFromString(secretAYAML) + Expect(err).NotTo(HaveOccurred(), "creating secret A") + + certB, keyB := generateTLSCertificate([]string{host}) + secretBYAML := fmt.Sprintf(` +apiVersion: v1 +kind: Secret +metadata: + name: %s + namespace: %s +type: kubernetes.io/tls +data: + tls.crt: %s + tls.key: %s +`, secretB, s.Namespace(), certB, keyB) + err = s.CreateResourceFromString(secretBYAML) + Expect(err).NotTo(HaveOccurred(), "creating secret B") + + time.Sleep(2 * time.Second) Review Comment: Multiple hard-coded sleep statements throughout the test indicate a pattern that should be replaced with proper synchronization mechanisms or resource readiness checks. ########## internal/webhook/v1/ssl/conflict_detector.go: ########## @@ -0,0 +1,474 @@ +// 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 ssl + +import ( + "context" + "fmt" + "strings" + + corev1 "k8s.io/api/core/v1" + networkingv1 "k8s.io/api/networking/v1" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/log" + gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" + + v1alpha1 "github.com/apache/apisix-ingress-controller/api/v1alpha1" + apiv2 "github.com/apache/apisix-ingress-controller/api/v2" + "github.com/apache/apisix-ingress-controller/internal/controller" + "github.com/apache/apisix-ingress-controller/internal/controller/config" + "github.com/apache/apisix-ingress-controller/internal/controller/indexer" + sslutil "github.com/apache/apisix-ingress-controller/internal/ssl" + internaltypes "github.com/apache/apisix-ingress-controller/internal/types" +) + +var logger = log.Log.WithName("ssl-conflict-detector") + +// HostCertMapping represents the relationship between a host and its certificate hash. +type HostCertMapping struct { + Host string + CertificateHash string + ResourceRef string +} + +// SSLConflict exposes the conflict details to the admission webhook for reporting. +type SSLConflict struct { + Host string + ConflictingResource string + CertificateHash string +} + +// ConflictDetector detects SSL conflicts among Gateway, Ingress, and ApisixTls resources. +type ConflictDetector struct { + client client.Client + secretCache map[types.NamespacedName]*secretInfo +} + +type secretInfo struct { + hash string + hosts []string +} + +// NewConflictDetector creates a detector backed by the provided client. +func NewConflictDetector(c client.Client) *ConflictDetector { + return &ConflictDetector{ + client: c, + secretCache: make(map[types.NamespacedName]*secretInfo), + } +} + +// DetectConflicts returns the list of conflicts between the provided mappings and +// existing resources that are associated with the same GatewayProxy. Best-effort: +// failures while enumerating existing resources or reading Secrets will be logged +// and result in no conflicts instead of blocking the admission. +func (d *ConflictDetector) DetectConflicts(ctx context.Context, obj client.Object, newMappings []HostCertMapping) ([]SSLConflict, error) { + gatewayProxy, err := d.resolveGatewayProxy(ctx, obj) + if err != nil { + logger.Error(err, "failed to resolve GatewayProxy", "object", objectKey(obj)) + return nil, nil + } + if gatewayProxy == nil { + return nil, nil + } + + existingMappings, err := d.collectExistingMappings(ctx, gatewayProxy, obj.GetUID()) + if err != nil { + logger.Error(err, "failed to collect existing SSL mappings", "gatewayProxy", objectKey(gatewayProxy)) + return nil, nil + } + + conflicts := make([]SSLConflict, 0) + byHost := make(map[string]HostCertMapping, len(existingMappings)) + for _, mapping := range existingMappings { + if mapping.Host == "" || mapping.CertificateHash == "" { + continue + } + if existing, ok := byHost[mapping.Host]; ok { + if existing.CertificateHash == mapping.CertificateHash { + continue + } + // keep the first encountered mapping, this will not happen in normal cases + continue + } + byHost[mapping.Host] = mapping + } + + // First, check for conflicts within the new resource itself + seen := make(map[string]string, len(newMappings)) + for _, mapping := range newMappings { + if mapping.Host == "" || mapping.CertificateHash == "" { + continue + } + if prev, ok := seen[mapping.Host]; ok { + // If same host appears with different certificate in the same resource, report conflict + if prev != mapping.CertificateHash { + conflicts = append(conflicts, SSLConflict{ + Host: mapping.Host, + ConflictingResource: mapping.ResourceRef, + CertificateHash: prev, + }) + } + continue + } + seen[mapping.Host] = mapping.CertificateHash + } + + for host, hash := range seen { + existing, ok := byHost[host] + if !ok { + continue + } + if existing.CertificateHash == hash { + continue + } + conflicts = append(conflicts, SSLConflict{ + Host: host, + ConflictingResource: existing.ResourceRef, + CertificateHash: existing.CertificateHash, + }) + } + + return conflicts, nil +} + +// FormatConflicts renders a human-readable error message for multiple conflicts. +func FormatConflicts(conflicts []SSLConflict) string { + if len(conflicts) == 0 { + return "" + } + var sb strings.Builder + sb.WriteString("SSL configuration conflicts detected:") + for _, conflict := range conflicts { + sb.WriteString(fmt.Sprintf("\n- Host '%s' is already configured with a different certificate in %s", conflict.Host, conflict.ConflictingResource)) + } + return sb.String() +} + +// BuildGatewayMappings calculates host-to-certificate mappings for a Gateway. +func (d *ConflictDetector) BuildGatewayMappings(ctx context.Context, gateway *gatewayv1.Gateway) []HostCertMapping { + mappings := make([]HostCertMapping, 0) + + if gateway == nil { + return mappings + } + + for _, listener := range gateway.Spec.Listeners { + if listener.TLS == nil || listener.TLS.CertificateRefs == nil { + continue + } + for _, ref := range listener.TLS.CertificateRefs { + if ref.Kind != nil && *ref.Kind != internaltypes.KindSecret { + continue + } + if ref.Group != nil && string(*ref.Group) != corev1.GroupName { + continue + } + secretNN := types.NamespacedName{ + Namespace: gateway.Namespace, + Name: string(ref.Name), + } + if ref.Namespace != nil && *ref.Namespace != "" { Review Comment: [nitpick] The condition checks for both nil and empty string, but the empty string check is redundant since an empty namespace reference would typically be represented as nil rather than an empty string pointer. ```suggestion if ref.Namespace != nil { ``` ########## test/e2e/webhook/ssl_conflict.go: ########## @@ -0,0 +1,933 @@ +// 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 webhook + +import ( + "crypto/rand" + "crypto/rsa" + "crypto/x509" + "crypto/x509/pkix" + "encoding/base64" + "encoding/pem" + "fmt" + "math/big" + "time" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + "github.com/apache/apisix-ingress-controller/test/e2e/scaffold" +) + +var _ = Describe("Test SSL/TLS Conflict Detection", Label("webhook"), func() { + s := scaffold.NewScaffold(scaffold.Options{ + Name: "ssl-conflict-test", + EnableWebhook: true, + }) + + BeforeEach(func() { + By("creating GatewayProxy") + err := s.CreateResourceFromString(s.GetGatewayProxySpec()) + Expect(err).NotTo(HaveOccurred(), "creating GatewayProxy") + time.Sleep(5 * time.Second) + + By("creating GatewayClass") + err = s.CreateResourceFromString(s.GetGatewayClassYaml()) + Expect(err).NotTo(HaveOccurred(), "creating GatewayClass") + time.Sleep(2 * time.Second) + + By("creating IngressClass") + err = s.CreateResourceFromStringWithNamespace(s.GetIngressClassYaml(), "") + Expect(err).NotTo(HaveOccurred(), "creating IngressClass") + time.Sleep(2 * time.Second) Review Comment: Hard-coded sleep durations in tests can make them flaky and slow. Consider using polling with timeouts or resource condition checks instead of fixed sleeps. ```suggestion Eventually(func() bool { ready, _ := s.IsGatewayProxyReady() return ready }, 30*time.Second, 1*time.Second).Should(BeTrue(), "GatewayProxy should be ready") By("creating GatewayClass") err = s.CreateResourceFromString(s.GetGatewayClassYaml()) Expect(err).NotTo(HaveOccurred(), "creating GatewayClass") Eventually(func() bool { ready, _ := s.IsGatewayClassReady() return ready }, 30*time.Second, 1*time.Second).Should(BeTrue(), "GatewayClass should be ready") By("creating IngressClass") err = s.CreateResourceFromStringWithNamespace(s.GetIngressClassYaml(), "") Expect(err).NotTo(HaveOccurred(), "creating IngressClass") Eventually(func() bool { ready, _ := s.IsIngressClassReady() return ready }, 30*time.Second, 1*time.Second).Should(BeTrue(), "IngressClass should be ready") ``` ########## internal/webhook/v1/ssl/conflict_detector.go: ########## @@ -0,0 +1,474 @@ +// 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 ssl + +import ( + "context" + "fmt" + "strings" + + corev1 "k8s.io/api/core/v1" + networkingv1 "k8s.io/api/networking/v1" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/log" + gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" + + v1alpha1 "github.com/apache/apisix-ingress-controller/api/v1alpha1" + apiv2 "github.com/apache/apisix-ingress-controller/api/v2" + "github.com/apache/apisix-ingress-controller/internal/controller" + "github.com/apache/apisix-ingress-controller/internal/controller/config" + "github.com/apache/apisix-ingress-controller/internal/controller/indexer" + sslutil "github.com/apache/apisix-ingress-controller/internal/ssl" + internaltypes "github.com/apache/apisix-ingress-controller/internal/types" +) + +var logger = log.Log.WithName("ssl-conflict-detector") + +// HostCertMapping represents the relationship between a host and its certificate hash. +type HostCertMapping struct { + Host string + CertificateHash string + ResourceRef string +} + +// SSLConflict exposes the conflict details to the admission webhook for reporting. +type SSLConflict struct { + Host string + ConflictingResource string + CertificateHash string +} + +// ConflictDetector detects SSL conflicts among Gateway, Ingress, and ApisixTls resources. +type ConflictDetector struct { + client client.Client + secretCache map[types.NamespacedName]*secretInfo +} + +type secretInfo struct { + hash string + hosts []string +} + +// NewConflictDetector creates a detector backed by the provided client. +func NewConflictDetector(c client.Client) *ConflictDetector { + return &ConflictDetector{ + client: c, + secretCache: make(map[types.NamespacedName]*secretInfo), + } +} + +// DetectConflicts returns the list of conflicts between the provided mappings and +// existing resources that are associated with the same GatewayProxy. Best-effort: +// failures while enumerating existing resources or reading Secrets will be logged +// and result in no conflicts instead of blocking the admission. +func (d *ConflictDetector) DetectConflicts(ctx context.Context, obj client.Object, newMappings []HostCertMapping) ([]SSLConflict, error) { + gatewayProxy, err := d.resolveGatewayProxy(ctx, obj) + if err != nil { + logger.Error(err, "failed to resolve GatewayProxy", "object", objectKey(obj)) + return nil, nil + } + if gatewayProxy == nil { + return nil, nil + } + + existingMappings, err := d.collectExistingMappings(ctx, gatewayProxy, obj.GetUID()) + if err != nil { + logger.Error(err, "failed to collect existing SSL mappings", "gatewayProxy", objectKey(gatewayProxy)) + return nil, nil + } + + conflicts := make([]SSLConflict, 0) + byHost := make(map[string]HostCertMapping, len(existingMappings)) + for _, mapping := range existingMappings { + if mapping.Host == "" || mapping.CertificateHash == "" { + continue + } + if existing, ok := byHost[mapping.Host]; ok { + if existing.CertificateHash == mapping.CertificateHash { + continue + } + // keep the first encountered mapping, this will not happen in normal cases + continue + } + byHost[mapping.Host] = mapping + } + + // First, check for conflicts within the new resource itself + seen := make(map[string]string, len(newMappings)) + for _, mapping := range newMappings { + if mapping.Host == "" || mapping.CertificateHash == "" { + continue + } + if prev, ok := seen[mapping.Host]; ok { + // If same host appears with different certificate in the same resource, report conflict + if prev != mapping.CertificateHash { + conflicts = append(conflicts, SSLConflict{ + Host: mapping.Host, + ConflictingResource: mapping.ResourceRef, + CertificateHash: prev, + }) + } + continue + } + seen[mapping.Host] = mapping.CertificateHash + } + + for host, hash := range seen { + existing, ok := byHost[host] + if !ok { + continue + } + if existing.CertificateHash == hash { + continue + } + conflicts = append(conflicts, SSLConflict{ + Host: host, + ConflictingResource: existing.ResourceRef, + CertificateHash: existing.CertificateHash, + }) + } + + return conflicts, nil +} + +// FormatConflicts renders a human-readable error message for multiple conflicts. +func FormatConflicts(conflicts []SSLConflict) string { + if len(conflicts) == 0 { + return "" + } + var sb strings.Builder + sb.WriteString("SSL configuration conflicts detected:") + for _, conflict := range conflicts { + sb.WriteString(fmt.Sprintf("\n- Host '%s' is already configured with a different certificate in %s", conflict.Host, conflict.ConflictingResource)) + } + return sb.String() +} + +// BuildGatewayMappings calculates host-to-certificate mappings for a Gateway. +func (d *ConflictDetector) BuildGatewayMappings(ctx context.Context, gateway *gatewayv1.Gateway) []HostCertMapping { + mappings := make([]HostCertMapping, 0) + + if gateway == nil { + return mappings + } + + for _, listener := range gateway.Spec.Listeners { + if listener.TLS == nil || listener.TLS.CertificateRefs == nil { + continue + } + for _, ref := range listener.TLS.CertificateRefs { + if ref.Kind != nil && *ref.Kind != internaltypes.KindSecret { + continue + } + if ref.Group != nil && string(*ref.Group) != corev1.GroupName { + continue + } + secretNN := types.NamespacedName{ + Namespace: gateway.Namespace, + Name: string(ref.Name), + } + if ref.Namespace != nil && *ref.Namespace != "" { + secretNN.Namespace = string(*ref.Namespace) + } + + info, err := d.getSecretInfo(ctx, secretNN) + if err != nil { + logger.Error(err, "failed to read secret for Gateway", "gateway", objectKey(gateway), "secret", secretNN) + continue + } + + hosts := make([]string, 0, 1) + if listener.Hostname != nil && *listener.Hostname != "" { + hosts = append(hosts, string(*listener.Hostname)) + } + hosts = sslutil.NormalizeHosts(hosts) + if len(hosts) == 0 { + hosts = info.hosts + } + for _, host := range hosts { + mappings = append(mappings, HostCertMapping{ + Host: host, + CertificateHash: info.hash, + ResourceRef: fmt.Sprintf("%s/%s/%s", internaltypes.KindGateway, gateway.Namespace, gateway.Name), + }) + } + } + } + + return mappings +} + +// BuildIngressMappings calculates host-to-certificate mappings for an Ingress. +func (d *ConflictDetector) BuildIngressMappings(ctx context.Context, ingress *networkingv1.Ingress) []HostCertMapping { + mappings := make([]HostCertMapping, 0) + if ingress == nil { + return mappings + } + + for _, tls := range ingress.Spec.TLS { + if tls.SecretName == "" { + continue + } + secretNN := types.NamespacedName{Namespace: ingress.Namespace, Name: tls.SecretName} + info, err := d.getSecretInfo(ctx, secretNN) + if err != nil { + logger.Error(err, "failed to read secret for Ingress", "ingress", objectKey(ingress), "secret", secretNN) + continue + } + + hosts := sslutil.NormalizeHosts(tls.Hosts) + if len(hosts) == 0 { + hosts = info.hosts + } + for _, host := range hosts { + mappings = append(mappings, HostCertMapping{ + Host: host, + CertificateHash: info.hash, + ResourceRef: fmt.Sprintf("%s/%s/%s", internaltypes.KindIngress, ingress.Namespace, ingress.Name), + }) + } + } + + return mappings +} + +// BuildApisixTlsMappings calculates host-to-certificate mappings for an ApisixTls resource. +func (d *ConflictDetector) BuildApisixTlsMappings(ctx context.Context, tls *apiv2.ApisixTls) []HostCertMapping { + mappings := make([]HostCertMapping, 0) + if tls == nil { + return mappings + } + + secretNN := types.NamespacedName{ + Namespace: tls.Spec.Secret.Namespace, + Name: tls.Spec.Secret.Name, + } + info, err := d.getSecretInfo(ctx, secretNN) + if err != nil { + logger.Error(err, "failed to read secret for ApisixTls", "apisixtls", objectKey(tls), "secret", secretNN) + return mappings + } + + hosts := make([]string, 0, len(tls.Spec.Hosts)) + for _, host := range tls.Spec.Hosts { + hosts = append(hosts, string(host)) + } + hosts = sslutil.NormalizeHosts(hosts) + // NOTICE: hosts is required by the CRD, so this should never happen + // if len(hosts) == 0 { + // hosts = info.hosts + // } Review Comment: These commented lines contain outdated logic that should be removed rather than left as commented code. If this fallback is truly unnecessary due to CRD requirements, the dead code should be deleted to improve maintainability. ```suggestion ``` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
