Skip to content

Commit 4cf51bb

Browse files
lunarwhitelunarwhite
committed
Add ACME identity label protection for solver extra labels
Prevent global solver extra labels from overwriting ACME identity labels (acme.cert-manager.io/http-domain, acme.cert-manager.io/http-token, acme.cert-manager.io/http01-solver) on dynamically-created HTTP01 solver resources. A filterACMEIdentityLabels helper strips these protected keys before merging. Without this guard, extra labels could silently break resource discovery. Signed-off-by: Yuedong Wu <dwcn22@outlook.com>
1 parent d50bc41 commit 4cf51bb

14 files changed

Lines changed: 148 additions & 28 deletions

File tree

‎cmd/controller/app/options/options.go‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,9 @@ func AddConfigFlags(fs *pflag.FlagSet, c *config.ControllerConfiguration) {
136136
"acme-http01-solver-extra-labels",
137137
"A set of key=value pairs for additional labels to apply to dynamically-created "+
138138
"ACME HTTP01 solver resources (pods, services, ingresses, or Gateway API HTTPRoutes). "+
139-
"These labels can be overridden by per-Issuer "+
139+
"The following ACME identity label keys are reserved and will be silently "+
140+
"ignored: acme.cert-manager.io/http-domain, acme.cert-manager.io/http-token, "+
141+
"acme.cert-manager.io/http01-solver. These labels can be overridden by per-Issuer "+
140142
"podTemplate/ingressTemplate/GatewayHTTPRoute.Labels.")
141143

142144
fs.BoolVar(&c.ClusterIssuerAmbientCredentials, "cluster-issuer-ambient-credentials", c.ClusterIssuerAmbientCredentials, ""+

‎deploy/charts/cert-manager/README.template.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ If a component-specific nodeSelector is also set, it will be merged and take pre
112112
Labels to apply to all resources.
113113
These labels are also applied to dynamically-created ACME HTTP01 solver resources
114114
(pods, services, ingresses, or Gateway API HTTPRoutes).
115-
For per-Issuer-specific labels, use the HTTP01 ingress solver podTemplate and ingressTemplate fields for pod/ingress resources, or the gatewayHTTPRoute solver labels field for Gateway API HTTPRoute resources.
115+
The following ACME identity label keys are reserved and will be silently ignored on dynamically-created resources: acme.cert-manager.io/http-domain, acme.cert-manager.io/http-token, acme.cert-manager.io/http01-solver. For per-Issuer-specific labels, use the HTTP01 ingress solver podTemplate and ingressTemplate fields for pod/ingress resources, or the gatewayHTTPRoute solver labels field for Gateway API HTTPRoute resources.
116116
#### **global.revisionHistoryLimit** ~ `number`
117117
118118
The number of old ReplicaSets to retain to allow rollback (if not set, the default Kubernetes value is set to 10).

‎deploy/charts/cert-manager/values.schema.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -831,7 +831,7 @@
831831
},
832832
"helm-values.global.commonLabels": {
833833
"default": {},
834-
"description": "Labels to apply to all resources.\nThese labels are also applied to dynamically-created ACME HTTP01 solver resources\n(pods, services, ingresses, or Gateway API HTTPRoutes).\nFor per-Issuer-specific labels, use the HTTP01 ingress solver podTemplate and ingressTemplate fields for pod/ingress resources, or the gatewayHTTPRoute solver labels field for Gateway API HTTPRoute resources.",
834+
"description": "Labels to apply to all resources.\nThese labels are also applied to dynamically-created ACME HTTP01 solver resources\n(pods, services, ingresses, or Gateway API HTTPRoutes).\nThe following ACME identity label keys are reserved and will be silently ignored on dynamically-created resources: acme.cert-manager.io/http-domain, acme.cert-manager.io/http-token, acme.cert-manager.io/http01-solver. For per-Issuer-specific labels, use the HTTP01 ingress solver podTemplate and ingressTemplate fields for pod/ingress resources, or the gatewayHTTPRoute solver labels field for Gateway API HTTPRoute resources.",
835835
"type": "object"
836836
},
837837
"helm-values.global.hostUsers": {

‎deploy/charts/cert-manager/values.yaml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,9 @@ global:
2525
# Labels to apply to all resources.
2626
# These labels are also applied to dynamically-created ACME HTTP01 solver resources
2727
# (pods, services, ingresses, or Gateway API HTTPRoutes).
28+
# The following ACME identity label keys are reserved and will be
29+
# silently ignored on dynamically-created resources: acme.cert-manager.io/http-domain,
30+
# acme.cert-manager.io/http-token, acme.cert-manager.io/http01-solver.
2831
# For per-Issuer-specific labels, use the HTTP01 ingress solver podTemplate and
2932
# ingressTemplate fields for pod/ingress resources, or the gatewayHTTPRoute
3033
# solver labels field for Gateway API HTTPRoute resources.

‎internal/apis/config/controller/types.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -222,6 +222,9 @@ type ACMEHTTP01Config struct {
222222
// Extra labels applied to all dynamically-created ACME HTTP01 solver
223223
// resources (pods, services, ingresses, or Gateway API HTTPRoutes). Applied
224224
// in addition to the standard ACME challenge identification labels.
225+
// The following ACME identity label keys are reserved and will be silently
226+
// ignored: acme.cert-manager.io/http-domain, acme.cert-manager.io/http-token,
227+
// acme.cert-manager.io/http01-solver.
225228
SolverExtraLabels map[string]string
226229
}
227230

‎pkg/apis/config/controller/v1alpha1/types.go‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -221,9 +221,12 @@ type ACMEHTTP01Config struct {
221221
// Allows specifying a list of custom nameservers to perform HTTP01 checks on.
222222
SolverNameservers []string `json:"solverNameservers,omitempty"`
223223

224-
// Additional labels applied to all dynamically-created ACME HTTP01 solver
225-
// resources (pods, services, ingresses, or gateway HTTPRoutes). Applied in
226-
// addition to the standard ACME challenge identification labels.
224+
// Extra labels applied to all dynamically-created ACME HTTP01 solver
225+
// resources (pods, services, ingresses, or Gateway API HTTPRoutes). Applied
226+
// in addition to the standard ACME challenge identification labels.
227+
// The following ACME identity label keys are reserved and will be silently
228+
// ignored: acme.cert-manager.io/http-domain, acme.cert-manager.io/http-token,
229+
// acme.cert-manager.io/http01-solver.
227230
SolverExtraLabels map[string]string `json:"solverExtraLabels,omitempty"`
228231
}
229232

‎pkg/issuer/acme/http/httproute.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ func (s *Solver) getGatewayHTTPRoute(ctx context.Context, ch *cmacme.Challenge)
9292

9393
func (s *Solver) createGatewayHTTPRoute(ctx context.Context, ch *cmacme.Challenge, svcName string) (*gwapi.HTTPRoute, error) {
9494
labels := podLabels(ch)
95-
maps.Copy(labels, s.ACMEOptions.HTTP01SolverExtraLabels)
95+
maps.Copy(labels, filterACMEIdentityLabels(s.ACMEOptions.HTTP01SolverExtraLabels))
9696
maps.Copy(labels, ch.Spec.Solver.HTTP01.GatewayHTTPRoute.Labels)
9797
httpRoute := &gwapi.HTTPRoute{
9898
ObjectMeta: metav1.ObjectMeta{
@@ -115,7 +115,7 @@ func (s *Solver) checkAndUpdateGatewayHTTPRoute(ctx context.Context, ch *cmacme.
115115
expectedSpec := generateHTTPRouteSpec(ch, svcName)
116116
actualSpec := httpRoute.Spec
117117
expectedLabels := podLabels(ch)
118-
maps.Copy(expectedLabels, s.ACMEOptions.HTTP01SolverExtraLabels)
118+
maps.Copy(expectedLabels, filterACMEIdentityLabels(s.ACMEOptions.HTTP01SolverExtraLabels))
119119
maps.Copy(expectedLabels, ch.Spec.Solver.HTTP01.GatewayHTTPRoute.Labels)
120120
actualLabels := httpRoute.Labels
121121
if reflect.DeepEqual(expectedSpec, actualSpec) && reflect.DeepEqual(expectedLabels, actualLabels) {

‎pkg/issuer/acme/http/httproute_test.go‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ func TestGetGatewayHTTPRouteForChallenge(t *testing.T) {
151151
}
152152
},
153153
},
154-
"should include extra labels from HTTP01SolverExtraLabels": {
154+
"should apply extra labels from HTTP01SolverExtraLabels and filter ACME identity labels": {
155155
Challenge: &cmacme.Challenge{
156156
Spec: cmacme.ChallengeSpec{
157157
DNSName: "example.com",
@@ -164,22 +164,28 @@ func TestGetGatewayHTTPRouteForChallenge(t *testing.T) {
164164
},
165165
PreFn: func(t *testing.T, s *solverFixture) {
166166
s.Solver.Context.ACMEOptions.HTTP01SolverExtraLabels = map[string]string{
167-
"custom-extra-label": "custom-extra-value",
167+
cmacme.DomainLabelKey: "badvalue",
168+
"custom-extra-label": "custom-extra-value",
168169
}
169170
httpRoute, err := s.Solver.createGatewayHTTPRoute(t.Context(), s.Challenge, "fakeservice")
170171
if err != nil {
171172
t.Errorf("error preparing test: %v", err)
172173
}
173-
// Verify extra labels are present on the created HTTPRoute
174-
if httpRoute.Labels["custom-extra-label"] != "custom-extra-value" {
175-
t.Errorf("expected HTTPRoute to have extra label 'custom-extra-label=custom-extra-value', but got %v", httpRoute.Labels)
176-
}
177174
s.testResources[createdHTTPRouteKey] = httpRoute
178175
s.Builder.Sync()
179176
},
180177
CheckFn: func(t *testing.T, s *solverFixture, args ...any) {
181178
createdHTTPRoute := s.testResources[createdHTTPRouteKey].(*gwapi.HTTPRoute)
182179
gotHttpRoute := args[0].(*gwapi.HTTPRoute)
180+
// ACME identity label should not be overridden by extra labels
181+
if gotHttpRoute.Labels[cmacme.DomainLabelKey] == "badvalue" {
182+
t.Errorf("ACME identity label %s should not be overridden by extra labels, got %q",
183+
cmacme.DomainLabelKey, gotHttpRoute.Labels[cmacme.DomainLabelKey])
184+
}
185+
// Non-ACME label should be present
186+
if gotHttpRoute.Labels["custom-extra-label"] != "custom-extra-value" {
187+
t.Errorf("expected HTTPRoute to have extra label 'custom-extra-label=custom-extra-value', but got %v", gotHttpRoute.Labels)
188+
}
183189
if !reflect.DeepEqual(gotHttpRoute, createdHTTPRoute) {
184190
t.Errorf("Expected %v to equal %v", gotHttpRoute, createdHTTPRoute)
185191
}

‎pkg/issuer/acme/http/ingress.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -203,7 +203,7 @@ func (s *Solver) buildIngressResource(ch *cmacme.Challenge, svcName string) (*ne
203203
},
204204
}
205205

206-
maps.Copy(ing.Labels, s.ACMEOptions.HTTP01SolverExtraLabels)
206+
maps.Copy(ing.Labels, filterACMEIdentityLabels(s.ACMEOptions.HTTP01SolverExtraLabels))
207207

208208
return ing, nil
209209
}

‎pkg/issuer/acme/http/ingress_test.go‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -670,7 +670,7 @@ func TestMergeIngressObjectMetaWithIngressResourceTemplate(t *testing.T) {
670670
}
671671
},
672672
},
673-
"should include extra labels from HTTP01SolverExtraLabels": {
673+
"should apply extra labels from HTTP01SolverExtraLabels and filter ACME identity labels": {
674674
Challenge: &cmacme.Challenge{
675675
Spec: cmacme.ChallengeSpec{
676676
DNSName: "example.com",
@@ -685,7 +685,8 @@ func TestMergeIngressObjectMetaWithIngressResourceTemplate(t *testing.T) {
685685
},
686686
PreFn: func(t *testing.T, s *solverFixture) {
687687
s.Solver.Context.ACMEOptions.HTTP01SolverExtraLabels = map[string]string{
688-
"custom-extra-label": "custom-extra-value",
688+
cmacme.DomainLabelKey: "badvalue",
689+
"custom-extra-label": "custom-extra-value",
689690
}
690691
expectedIngress, err := s.Solver.buildIngressResource(s.Challenge, "fakeservice")
691692
if err != nil {
@@ -702,6 +703,16 @@ func TestMergeIngressObjectMetaWithIngressResourceTemplate(t *testing.T) {
702703
t.Fail()
703704
return
704705
}
706+
// ACME identity label should not be overridden by extra labels
707+
if resp.Labels[cmacme.DomainLabelKey] == "badvalue" {
708+
t.Errorf("ACME identity label %s should not be overridden by extra labels, got %q",
709+
cmacme.DomainLabelKey, resp.Labels[cmacme.DomainLabelKey])
710+
}
711+
// Non-ACME label should be present
712+
if resp.Labels["custom-extra-label"] != "custom-extra-value" {
713+
t.Errorf("expected non-ACME extra label %s=%s, got %q",
714+
"custom-extra-label", "custom-extra-value", resp.Labels["custom-extra-label"])
715+
}
705716
expectedIngress.APIVersion = resp.APIVersion
706717
expectedIngress.Kind = resp.Kind
707718
expectedIngress.OwnerReferences = resp.OwnerReferences

0 commit comments

Comments
 (0)