Skip to content

Commit 45388c0

Browse files
dprotasodprotaso
authored
fix cluster-local routes being stalled when external-domain-tls is enabled (#15234)
1 parent 69c0589 commit 45388c0

File tree

4 files changed

+97
-9
lines changed

4 files changed

+97
-9
lines changed

‎pkg/apis/serving/v1/route_lifecycle.go

+5
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,11 @@ const (
202202
// RouteConditionCertificateProvisioned condition when it is set to True
203203
// because external-domain-tls was not enabled.
204204
ExternalDomainTLSNotEnabledMessage = "external-domain-tls is not enabled"
205+
206+
// TLSNotEnabledForClusterLocalMessage is the message which is set on the
207+
// RouteConditionCertificateProvisioned condition when it is set to True
208+
// because the domain is cluster-local.
209+
TLSNotEnabledForClusterLocalMessage = "TLS is not enabled for cluster-local"
205210
)
206211

207212
// MarkTLSNotEnabled sets RouteConditionCertificateProvisioned to true when

‎pkg/reconciler/route/route.go

+19-9
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,7 @@ func (c *Reconciler) ReconcileKind(ctx context.Context, r *v1.Route) pkgreconcil
143143
return err
144144
}
145145

146-
tls, acmeChallenges, err := c.externalDomainTLS(ctx, r.Status.URL.Host, r, traffic)
146+
tls, acmeChallenges, desiredCerts, err := c.externalDomainTLS(ctx, r.Status.URL.Host, r, traffic)
147147
if err != nil {
148148
return err
149149
}
@@ -154,6 +154,10 @@ func (c *Reconciler) ReconcileKind(ctx context.Context, r *v1.Route) pkgreconcil
154154
return err
155155
}
156156
tls = append(tls, internalTLS...)
157+
} else if externalDomainTLSEnabled(ctx, r) && len(desiredCerts) == 0 {
158+
// If external TLS is enabled but we have no desired certs then the route
159+
// must have only cluster local hosts
160+
r.Status.MarkTLSNotEnabled(v1.TLSNotEnabledForClusterLocalMessage)
157161
}
158162

159163
// Reconcile ingress and its children resources.
@@ -195,18 +199,24 @@ func (c *Reconciler) ReconcileKind(ctx context.Context, r *v1.Route) pkgreconcil
195199
return nil
196200
}
197201

198-
func (c *Reconciler) externalDomainTLS(ctx context.Context, host string, r *v1.Route, traffic *traffic.Config) ([]netv1alpha1.IngressTLS, []netv1alpha1.HTTP01Challenge, error) {
202+
func (c *Reconciler) externalDomainTLS(ctx context.Context, host string, r *v1.Route, traffic *traffic.Config) (
203+
[]netv1alpha1.IngressTLS,
204+
[]netv1alpha1.HTTP01Challenge,
205+
[]*netv1alpha1.Certificate,
206+
error,
207+
) {
208+
var desiredCerts []*netv1alpha1.Certificate
199209
logger := logging.FromContext(ctx)
200210

201211
tls := []netv1alpha1.IngressTLS{}
202212
if !externalDomainTLSEnabled(ctx, r) {
203213
r.Status.MarkTLSNotEnabled(v1.ExternalDomainTLSNotEnabledMessage)
204-
return tls, nil, nil
214+
return tls, nil, desiredCerts, nil
205215
}
206216

207217
domainToTagMap, err := domains.GetAllDomainsAndTags(ctx, r, getTrafficNames(traffic.Targets), traffic.Visibility)
208218
if err != nil {
209-
return nil, nil, err
219+
return nil, nil, desiredCerts, err
210220
}
211221

212222
for domain := range domainToTagMap {
@@ -223,15 +233,15 @@ func (c *Reconciler) externalDomainTLS(ctx context.Context, host string, r *v1.R
223233

224234
allWildcardCerts, err := c.certificateLister.Certificates(r.Namespace).List(labelSelector)
225235
if err != nil {
226-
return nil, nil, err
236+
return nil, nil, desiredCerts, err
227237
}
228238

229239
domainConfig := config.FromContext(ctx).Domain
230240
rLabels := r.Labels
231241
domain := domainConfig.LookupDomainForLabels(rLabels)
232242

233243
acmeChallenges := []netv1alpha1.HTTP01Challenge{}
234-
desiredCerts := resources.MakeCertificates(r, domainToTagMap, certClass(ctx, r), domain)
244+
desiredCerts = resources.MakeCertificates(r, domainToTagMap, certClass(ctx, r), domain)
235245
for _, desiredCert := range desiredCerts {
236246
dnsNames := sets.New(desiredCert.Spec.DNSNames...)
237247
// Look for a matching wildcard cert before provisioning a new one. This saves the
@@ -247,7 +257,7 @@ func (c *Reconciler) externalDomainTLS(ctx context.Context, host string, r *v1.R
247257
} else {
248258
r.Status.MarkCertificateProvisionFailed(desiredCert)
249259
}
250-
return nil, nil, err
260+
return nil, nil, desiredCerts, err
251261
}
252262
dnsNames = sets.New(cert.Spec.DNSNames...)
253263
}
@@ -306,12 +316,12 @@ func (c *Reconciler) externalDomainTLS(ctx context.Context, host string, r *v1.R
306316

307317
orphanCerts, err := c.getOrphanRouteCerts(r, domainToTagMap, netcfg.CertificateExternalDomain)
308318
if err != nil {
309-
return nil, nil, err
319+
return nil, nil, desiredCerts, err
310320
}
311321

312322
c.deleteOrphanedCerts(ctx, orphanCerts)
313323

314-
return tls, acmeChallenges, nil
324+
return tls, acmeChallenges, desiredCerts, nil
315325
}
316326

317327
func (c *Reconciler) clusterLocalDomainTLS(ctx context.Context, r *v1.Route, tc *traffic.Config) ([]netv1alpha1.IngressTLS, error) {

‎pkg/reconciler/route/table_test.go

+65
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,7 @@ const (
7575
rolloutDurationKey key = iota
7676
externalSchemeKey
7777
enableExternalDomainTLSKey
78+
domainConfigKey
7879
)
7980

8081
// This is heavily based on the way the OpenShift Ingress controller tests its reconciliation method.
@@ -3276,6 +3277,7 @@ func TestReconcileEnableExternalDomainTLS(t *testing.T) {
32763277
WithRouteUID("65-23"),
32773278
WithRouteGeneration(1), WithRouteObservedGeneration,
32783279
MarkTrafficAssigned, MarkIngressNotConfigured,
3280+
WithRouteConditionsTLSNotEnabledForClusterLocalMessage,
32793281
WithLocalDomain, WithAddress, WithInitRouteConditions,
32803282
WithRouteLabel(map[string]string{netapi.VisibilityLabelKey: serving.VisibilityClusterLocal}),
32813283
WithStatusTraffic(
@@ -3286,6 +3288,66 @@ func TestReconcileEnableExternalDomainTLS(t *testing.T) {
32863288
})),
32873289
}},
32883290
Key: "default/becomes-local",
3291+
}, {
3292+
Name: "local domain route should mark certificate provisioned TLS disabled",
3293+
Key: "default/local-domain",
3294+
Ctx: context.WithValue(context.Background(), domainConfigKey, &config.Domain{
3295+
Domains: map[string]config.DomainConfig{
3296+
"svc.cluster.local": {},
3297+
},
3298+
}),
3299+
Objects: []runtime.Object{
3300+
Route("default", "local-domain", WithConfigTarget("config"), WithRouteGeneration(1),
3301+
WithRouteObservedGeneration,
3302+
WithRouteUID("65-23")),
3303+
cfg("default", "config",
3304+
WithConfigGeneration(1), WithLatestCreated("config-00001"), WithLatestReady("config-00001")),
3305+
rev("default", "config", 1, MarkRevisionReady, WithRevName("config-00001")),
3306+
simpleIngress(
3307+
Route("default", "local-domain", WithConfigTarget("config"), WithRouteUID("65-23")),
3308+
&traffic.Config{
3309+
Targets: map[string]traffic.RevisionTargets{
3310+
traffic.DefaultTarget: {{
3311+
TrafficTarget: v1.TrafficTarget{
3312+
ConfigurationName: "config",
3313+
LatestRevision: ptr.Bool(true),
3314+
RevisionName: "config-00001",
3315+
Percent: ptr.Int64(100),
3316+
},
3317+
}},
3318+
},
3319+
},
3320+
withReadyIngress,
3321+
// simpleIngress and the test use different 'configs' (limit of reading config from the context)
3322+
// so we need to delete the external visible host rules
3323+
func(i *netv1alpha1.Ingress) {
3324+
localRules := i.Spec.Rules[:0]
3325+
for _, rule := range i.Spec.Rules {
3326+
if rule.Visibility == netv1alpha1.IngressVisibilityClusterLocal {
3327+
localRules = append(localRules, rule)
3328+
}
3329+
}
3330+
i.Spec.Rules = localRules
3331+
},
3332+
),
3333+
simpleK8sService(Route("default", "local-domain", WithConfigTarget("config"),
3334+
WithRouteUID("65-23"))),
3335+
},
3336+
WantStatusUpdates: []clientgotesting.UpdateActionImpl{{
3337+
Object: Route("default", "local-domain", WithConfigTarget("config"),
3338+
WithRouteUID("65-23"),
3339+
WithRouteGeneration(1), WithRouteObservedGeneration,
3340+
MarkTrafficAssigned,
3341+
MarkIngressReady,
3342+
WithRouteConditionsTLSNotEnabledForClusterLocalMessage,
3343+
WithLocalDomain, WithAddress, WithInitRouteConditions,
3344+
WithStatusTraffic(
3345+
v1.TrafficTarget{
3346+
RevisionName: "config-00001",
3347+
Percent: ptr.Int64(100),
3348+
LatestRevision: ptr.Bool(true),
3349+
})),
3350+
}},
32893351
}}
32903352

32913353
for i, row := range table {
@@ -3323,6 +3385,9 @@ func NewTestReconciler(ctx context.Context, listers *Listers, cmw configmap.Watc
33233385
if v := ctx.Value(externalSchemeKey); v != nil {
33243386
cfg.Network.DefaultExternalScheme = v.(string)
33253387
}
3388+
if v := ctx.Value(domainConfigKey); v != nil {
3389+
cfg.Domain = v.(*config.Domain)
3390+
}
33263391

33273392
return routereconciler.NewReconciler(ctx,
33283393
logging.FromContext(ctx),

‎pkg/testing/v1/route.go

+8
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,14 @@ func WithRouteConditionsExternalDomainTLSDisabled(rt *v1.Route) {
180180
rt.Status.MarkTLSNotEnabled(v1.ExternalDomainTLSNotEnabledMessage)
181181
}
182182

183+
// WithRouteConditionsTLSNotEnabledForClusterLocalMessage calls
184+
// MarkTLSNotEnabled with TLSNotEnabledForClusterLocalMessage after initialized
185+
// the Service's conditions.
186+
func WithRouteConditionsTLSNotEnabledForClusterLocalMessage(rt *v1.Route) {
187+
rt.Status.InitializeConditions()
188+
rt.Status.MarkTLSNotEnabled(v1.TLSNotEnabledForClusterLocalMessage)
189+
}
190+
183191
// WithRouteConditionsHTTPDowngrade calls MarkHTTPDowngrade after initialized the Service's conditions.
184192
func WithRouteConditionsHTTPDowngrade(rt *v1.Route) {
185193
rt.Status.InitializeConditions()

0 commit comments

Comments
 (0)