Skip to content

Commit 5fbd94e

Browse files
linkvtlinkvt
authored
Fix duplicate ACME challenge paths across ingress rules (#16259)
* Fix duplicate ACME challenge paths across ingress rules When a Service has traffic tags, the Route controller was adding ALL ACME challenge paths to EVERY external ingress rule. This caused each challenge path to appear multiple times, leading to routing conflicts. For example, with tags "blue" and "green", the challenge for blue-app.example.com would incorrectly appear in both the blue and green ingress rules. Solution: Create a dedicated ingress rule for each ACME challenge domain. Each rule contains only that challenge's path, preventing duplicates while allowing the same domain to appear in both traffic and ACME rules. * Remove outdated linting exclusions
1 parent ce76741 commit 5fbd94e

File tree

6 files changed

+266
-64
lines changed

6 files changed

+266
-64
lines changed

‎pkg/http/trace.go‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,15 +24,13 @@ import (
2424
// ExtractTraceID extracts the trace ID from the request headers.
2525
// It supports both W3C Trace Context (traceparent) and B3 (X-B3-TraceId) formats.
2626
func ExtractTraceID(h http.Header) string {
27-
//nolint:canonicalheader
2827
if traceparent := h.Get("traceparent"); traceparent != "" {
2928
parts := strings.SplitN(traceparent, "-", 3)
3029
if len(parts) >= 2 {
3130
return parts[1]
3231
}
3332
}
3433

35-
//nolint:canonicalheader
3634
if b3TraceID := h.Get("X-B3-TraceId"); b3TraceID != "" {
3735
return b3TraceID
3836
}

‎pkg/http/trace_integration_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,7 @@ func TestRequestLogTemplateWithTraceID(t *testing.T) {
146146

147147
// Test with W3C Trace Context
148148
req := httptest.NewRequest(http.MethodPost, "http://example.com/api", nil)
149-
//nolint:canonicalheader
149+
150150
req.Header.Set("traceparent", "00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-01")
151151

152152
resp := httptest.NewRecorder()

‎pkg/reconciler/domainmapping/resources/ingress.go‎

Lines changed: 29 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ package resources
1919
import (
2020
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2121
"k8s.io/apimachinery/pkg/util/intstr"
22-
"k8s.io/apimachinery/pkg/util/sets"
2322

2423
netapi "knative.dev/networking/pkg/apis/networking"
2524
netv1alpha1 "knative.dev/networking/pkg/apis/networking/v1alpha1"
@@ -36,7 +35,34 @@ import (
3635
// KIngress). The created ingress will contain a RewriteHost rule to cause the
3736
// given hostName to be used as the host.
3837
func MakeIngress(dm *servingv1beta1.DomainMapping, backendServiceName, hostName, ingressClass string, httpOption netv1alpha1.HTTPOption, tls []netv1alpha1.IngressTLS, acmeChallenges ...netv1alpha1.HTTP01Challenge) *netv1alpha1.Ingress {
39-
paths, hosts := routeresources.MakeACMEIngressPaths(acmeChallenges, sets.New(dm.GetName()))
38+
// Traffic rule
39+
rules := []netv1alpha1.IngressRule{{
40+
Hosts: []string{dm.Name},
41+
Visibility: netv1alpha1.IngressVisibilityExternalIP,
42+
HTTP: &netv1alpha1.HTTPIngressRuleValue{
43+
Paths: []netv1alpha1.HTTPIngressPath{{
44+
RewriteHost: hostName,
45+
Splits: []netv1alpha1.IngressBackendSplit{{
46+
Percent: 100,
47+
AppendHeaders: map[string]string{
48+
netheader.OriginalHostKey: dm.Name,
49+
},
50+
IngressBackend: netv1alpha1.IngressBackend{
51+
ServiceNamespace: dm.Namespace,
52+
ServiceName: backendServiceName,
53+
ServicePort: intstr.FromInt(80),
54+
},
55+
}},
56+
}},
57+
},
58+
}}
59+
60+
// Add dedicated ACME challenge rules
61+
if len(acmeChallenges) > 0 {
62+
acmeRules := routeresources.MakeACMEChallengeRules(acmeChallenges)
63+
rules = append(rules, acmeRules...)
64+
}
65+
4066
return &netv1alpha1.Ingress{
4167
ObjectMeta: metav1.ObjectMeta{
4268
Name: kmeta.ChildName(dm.GetName(), ""),
@@ -53,28 +79,7 @@ func MakeIngress(dm *servingv1beta1.DomainMapping, backendServiceName, hostName,
5379
Spec: netv1alpha1.IngressSpec{
5480
HTTPOption: httpOption,
5581
TLS: tls,
56-
Rules: []netv1alpha1.IngressRule{{
57-
Hosts: append(hosts, dm.Name),
58-
Visibility: netv1alpha1.IngressVisibilityExternalIP,
59-
HTTP: &netv1alpha1.HTTPIngressRuleValue{
60-
// The order of the paths is sensitive, always put tls challenge first
61-
Paths: append(paths,
62-
[]netv1alpha1.HTTPIngressPath{{
63-
RewriteHost: hostName,
64-
Splits: []netv1alpha1.IngressBackendSplit{{
65-
Percent: 100,
66-
AppendHeaders: map[string]string{
67-
netheader.OriginalHostKey: dm.Name,
68-
},
69-
IngressBackend: netv1alpha1.IngressBackend{
70-
ServiceNamespace: dm.Namespace,
71-
ServiceName: backendServiceName,
72-
ServicePort: intstr.FromInt(80),
73-
},
74-
}},
75-
}}...),
76-
},
77-
}},
82+
Rules: rules,
7883
},
7984
}
8085
}

‎pkg/reconciler/domainmapping/resources/ingress_test.go‎

Lines changed: 80 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -194,6 +194,25 @@ func TestMakeIngress(t *testing.T) {
194194
Spec: netv1alpha1.IngressSpec{
195195
HTTPOption: netv1alpha1.HTTPOptionEnabled,
196196
Rules: []netv1alpha1.IngressRule{{
197+
Hosts: []string{"mapping.com"},
198+
Visibility: netv1alpha1.IngressVisibilityExternalIP,
199+
HTTP: &netv1alpha1.HTTPIngressRuleValue{
200+
Paths: []netv1alpha1.HTTPIngressPath{{
201+
RewriteHost: "the-rewrite-host",
202+
Splits: []netv1alpha1.IngressBackendSplit{{
203+
Percent: 100,
204+
AppendHeaders: map[string]string{
205+
netheader.OriginalHostKey: "mapping.com",
206+
},
207+
IngressBackend: netv1alpha1.IngressBackend{
208+
ServiceName: "the-target-svc",
209+
ServiceNamespace: "the-namespace",
210+
ServicePort: intstr.FromInt(80),
211+
},
212+
}},
213+
}},
214+
},
215+
}, {
197216
Hosts: []string{"mapping.com"},
198217
Visibility: netv1alpha1.IngressVisibilityExternalIP,
199218
HTTP: &netv1alpha1.HTTPIngressRuleValue{
@@ -207,7 +226,51 @@ func TestMakeIngress(t *testing.T) {
207226
},
208227
Percent: 100,
209228
}},
210-
}, {
229+
}},
230+
},
231+
}},
232+
},
233+
},
234+
}, {
235+
name: "challenge with non-matching host",
236+
dm: v1beta1.DomainMapping{
237+
ObjectMeta: metav1.ObjectMeta{
238+
Name: "mapping.com",
239+
Namespace: "the-namespace",
240+
UID: types.UID("the-uid"),
241+
},
242+
Spec: v1beta1.DomainMappingSpec{
243+
Ref: duckv1.KReference{
244+
Namespace: "the-namespace",
245+
Name: "the-name",
246+
},
247+
},
248+
},
249+
acmeChallenges: []netv1alpha1.HTTP01Challenge{{
250+
ServiceNamespace: "test-ns",
251+
ServiceName: "cm-solver",
252+
ServicePort: intstr.FromInt(8090),
253+
URL: &apis.URL{
254+
Scheme: "http",
255+
Path: "/.well-known/acme-challenge/token",
256+
Host: "truncated.example.com", // Different from mapping.com
257+
},
258+
}},
259+
want: netv1alpha1.Ingress{
260+
ObjectMeta: metav1.ObjectMeta{
261+
Name: "mapping.com",
262+
Namespace: "the-namespace",
263+
Annotations: map[string]string{
264+
netapi.IngressClassAnnotationKey: "the-ingress-class",
265+
},
266+
},
267+
Spec: netv1alpha1.IngressSpec{
268+
HTTPOption: netv1alpha1.HTTPOptionEnabled,
269+
Rules: []netv1alpha1.IngressRule{{
270+
Hosts: []string{"mapping.com"},
271+
Visibility: netv1alpha1.IngressVisibilityExternalIP,
272+
HTTP: &netv1alpha1.HTTPIngressRuleValue{
273+
Paths: []netv1alpha1.HTTPIngressPath{{
211274
RewriteHost: "the-rewrite-host",
212275
Splits: []netv1alpha1.IngressBackendSplit{{
213276
Percent: 100,
@@ -222,6 +285,22 @@ func TestMakeIngress(t *testing.T) {
222285
}},
223286
}},
224287
},
288+
}, {
289+
Hosts: []string{"truncated.example.com"},
290+
Visibility: netv1alpha1.IngressVisibilityExternalIP,
291+
HTTP: &netv1alpha1.HTTPIngressRuleValue{
292+
Paths: []netv1alpha1.HTTPIngressPath{{
293+
Path: "/.well-known/acme-challenge/token",
294+
Splits: []netv1alpha1.IngressBackendSplit{{
295+
IngressBackend: netv1alpha1.IngressBackend{
296+
ServiceNamespace: "test-ns",
297+
ServiceName: "cm-solver",
298+
ServicePort: intstr.FromInt(8090),
299+
},
300+
Percent: 100,
301+
}},
302+
}},
303+
},
225304
}},
226305
},
227306
},

‎pkg/reconciler/route/resources/ingress.go‎

Lines changed: 27 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -181,16 +181,16 @@ func makeIngressSpec(
181181
rule.HTTP.Paths[0].AppendHeaders[netheader.RouteTagKey] = name
182182
}
183183
}
184-
// If this is a public rule, we need to configure ACME challenge paths.
185-
if visibility == netv1alpha1.IngressVisibilityExternalIP {
186-
paths, hosts := MakeACMEIngressPaths(acmeChallenges, domains)
187-
rule.Hosts = append(hosts, rule.Hosts...)
188-
rule.HTTP.Paths = append(paths, rule.HTTP.Paths...)
189-
}
190184
rules = append(rules, rule)
191185
}
192186
}
193187

188+
// Create dedicated rules for ACME challenges
189+
if len(acmeChallenges) > 0 {
190+
acmeRules := MakeACMEChallengeRules(acmeChallenges)
191+
rules = append(rules, acmeRules...)
192+
}
193+
194194
httpOption, err := servingnetworking.GetHTTPOption(ctx, config.FromContext(ctx).Network, r.GetAnnotations())
195195
if err != nil {
196196
return netv1alpha1.IngressSpec{}, err
@@ -203,30 +203,31 @@ func makeIngressSpec(
203203
}, nil
204204
}
205205

206-
// MakeACMEIngressPaths returns a set of netv1alpha1.HTTPIngressPath
207-
// that can be used to perform ACME challenges.
208-
func MakeACMEIngressPaths(acmeChallenges []netv1alpha1.HTTP01Challenge, domains sets.Set[string]) ([]netv1alpha1.HTTPIngressPath, []string) {
209-
paths := make([]netv1alpha1.HTTPIngressPath, 0, len(acmeChallenges))
210-
var extraHosts []string
206+
// MakeACMEChallengeRules creates one dedicated rule per ACME challenge domain.
207+
func MakeACMEChallengeRules(acmeChallenges []netv1alpha1.HTTP01Challenge) []netv1alpha1.IngressRule {
208+
rules := make([]netv1alpha1.IngressRule, 0, len(acmeChallenges))
211209

212210
for _, challenge := range acmeChallenges {
213-
if !domains.Has(challenge.URL.Host) {
214-
extraHosts = append(extraHosts, challenge.URL.Host)
215-
}
216-
217-
paths = append(paths, netv1alpha1.HTTPIngressPath{
218-
Splits: []netv1alpha1.IngressBackendSplit{{
219-
IngressBackend: netv1alpha1.IngressBackend{
220-
ServiceNamespace: challenge.ServiceNamespace,
221-
ServiceName: challenge.ServiceName,
222-
ServicePort: challenge.ServicePort,
223-
},
224-
Percent: 100,
225-
}},
226-
Path: challenge.URL.Path,
211+
rules = append(rules, netv1alpha1.IngressRule{
212+
Hosts: []string{challenge.URL.Host},
213+
Visibility: netv1alpha1.IngressVisibilityExternalIP,
214+
HTTP: &netv1alpha1.HTTPIngressRuleValue{
215+
Paths: []netv1alpha1.HTTPIngressPath{{
216+
Splits: []netv1alpha1.IngressBackendSplit{{
217+
IngressBackend: netv1alpha1.IngressBackend{
218+
ServiceNamespace: challenge.ServiceNamespace,
219+
ServiceName: challenge.ServiceName,
220+
ServicePort: challenge.ServicePort,
221+
},
222+
Percent: 100,
223+
}},
224+
Path: challenge.URL.Path,
225+
}},
226+
},
227227
})
228228
}
229-
return paths, extraHosts
229+
230+
return rules
230231
}
231232

232233
func makeIngressRule(domains sets.Set[string], ns string,

0 commit comments

Comments
 (0)