From c682831bf1d622173603f9d3b20d5a1bbff7bbeb Mon Sep 17 00:00:00 2001 From: Kenjiro Nakayama Date: Fri, 6 Mar 2020 10:45:14 +0900 Subject: [PATCH 01/13] Propagate status from KCert to Route When autoTLS is enabled, KCert is one of the critical resources. This patch changes changes to propagate status status from KCert to Route. --- pkg/apis/serving/v1/route_lifecycle.go | 45 ++++++++------------ pkg/apis/serving/v1alpha1/route_lifecycle.go | 45 ++++++++------------ pkg/reconciler/route/route.go | 1 + pkg/reconciler/route/table_test.go | 10 ++--- 4 files changed, 40 insertions(+), 61 deletions(-) diff --git a/pkg/apis/serving/v1/route_lifecycle.go b/pkg/apis/serving/v1/route_lifecycle.go index 4b67a2b9efcf..226d36b1f1d9 100644 --- a/pkg/apis/serving/v1/route_lifecycle.go +++ b/pkg/apis/serving/v1/route_lifecycle.go @@ -29,6 +29,7 @@ import ( var routeCondSet = apis.NewLivingConditionSet( RouteConditionAllTrafficAssigned, RouteConditionIngressReady, + RouteConditionCertificateProvisioned, ) // GetGroupVersionKind returns the GroupVersionKind. @@ -44,6 +45,8 @@ func (rs *RouteStatus) IsReady() bool { // InitializeConditions sets the initial values to the conditions. func (rs *RouteStatus) InitializeConditions() { routeCondSet.Manage(rs).InitializeConditions() + // Since Certificate is optional, initialize the status with Ready. + routeCondSet.Manage(rs).MarkTrue(RouteConditionCertificateProvisioned) } // MarkServiceNotOwned changes the IngressReady status to be false with the reason being that @@ -99,43 +102,29 @@ func (rs *RouteStatus) MarkMissingTrafficTarget(kind, name string) { } func (rs *RouteStatus) MarkCertificateProvisionFailed(name string) { - routeCondSet.Manage(rs).SetCondition(apis.Condition{ - Type: RouteConditionCertificateProvisioned, - Status: corev1.ConditionFalse, - Severity: apis.ConditionSeverityWarning, - Reason: "CertificateProvisionFailed", - Message: fmt.Sprintf("Certificate %s fails to be provisioned.", name), - }) + routeCondSet.Manage(rs).MarkFalse(RouteConditionCertificateProvisioned, + "CertificateProvisionFailed", + "Certificate %s fails to be provisioned.", name) } func (rs *RouteStatus) MarkCertificateReady(name string) { - routeCondSet.Manage(rs).SetCondition(apis.Condition{ - Type: RouteConditionCertificateProvisioned, - Status: corev1.ConditionTrue, - Severity: apis.ConditionSeverityWarning, - Reason: "CertificateReady", - Message: fmt.Sprintf("Certificate %s is successfully provisioned", name), - }) + routeCondSet.Manage(rs).MarkTrue(RouteConditionCertificateProvisioned) } func (rs *RouteStatus) MarkCertificateNotReady(name string) { - routeCondSet.Manage(rs).SetCondition(apis.Condition{ - Type: RouteConditionCertificateProvisioned, - Status: corev1.ConditionUnknown, - Severity: apis.ConditionSeverityWarning, - Reason: "CertificateNotReady", - Message: fmt.Sprintf("Certificate %s is not ready.", name), - }) + routeCondSet.Manage(rs).MarkUnknown(RouteConditionCertificateProvisioned, + "CertificateNotReady", + "Certificate %s is not ready.", name) } func (rs *RouteStatus) MarkCertificateNotOwned(name string) { - routeCondSet.Manage(rs).SetCondition(apis.Condition{ - Type: RouteConditionCertificateProvisioned, - Status: corev1.ConditionFalse, - Severity: apis.ConditionSeverityWarning, - Reason: "CertificateNotOwned", - Message: fmt.Sprintf("There is an existing certificate %s that we don't own.", name), - }) + routeCondSet.Manage(rs).MarkFalse(RouteConditionCertificateProvisioned, + "CertificateNotOwned", + "There is an existing certificate %s that we don't own.", name) +} + +func (rs *RouteStatus) MarkCertificateNotEnabled() { + routeCondSet.Manage(rs).MarkTrue(RouteConditionCertificateProvisioned) } // PropagateIngressStatus update RouteConditionIngressReady condition diff --git a/pkg/apis/serving/v1alpha1/route_lifecycle.go b/pkg/apis/serving/v1alpha1/route_lifecycle.go index 017e6dc42995..c6a06dbd7d9f 100644 --- a/pkg/apis/serving/v1alpha1/route_lifecycle.go +++ b/pkg/apis/serving/v1alpha1/route_lifecycle.go @@ -29,6 +29,7 @@ import ( var routeCondSet = apis.NewLivingConditionSet( RouteConditionAllTrafficAssigned, RouteConditionIngressReady, + RouteConditionCertificateProvisioned, ) func (r *Route) GetGroupVersionKind() schema.GroupVersionKind { @@ -45,6 +46,8 @@ func (rs *RouteStatus) GetCondition(t apis.ConditionType) *apis.Condition { func (rs *RouteStatus) InitializeConditions() { routeCondSet.Manage(rs).InitializeConditions() + // Since Certificate is optional, initialize the status with Ready. + routeCondSet.Manage(rs).MarkTrue(RouteConditionCertificateProvisioned) } // // MarkResourceNotConvertible adds a Warning-severity condition to the resource noting that @@ -112,43 +115,29 @@ func (rs *RouteStatus) MarkMissingTrafficTarget(kind, name string) { } func (rs *RouteStatus) MarkCertificateProvisionFailed(name string) { - routeCondSet.Manage(rs).SetCondition(apis.Condition{ - Type: RouteConditionCertificateProvisioned, - Status: corev1.ConditionFalse, - Severity: apis.ConditionSeverityWarning, - Reason: "CertificateProvisionFailed", - Message: fmt.Sprintf("Certificate %s fails to be provisioned.", name), - }) + routeCondSet.Manage(rs).MarkFalse(RouteConditionCertificateProvisioned, + "CertificateProvisionFailed", + "Certificate %s fails to be provisioned.", name) } func (rs *RouteStatus) MarkCertificateReady(name string) { - routeCondSet.Manage(rs).SetCondition(apis.Condition{ - Type: RouteConditionCertificateProvisioned, - Status: corev1.ConditionTrue, - Severity: apis.ConditionSeverityWarning, - Reason: "CertificateReady", - Message: fmt.Sprintf("Certificate %s is successfully provisioned", name), - }) + routeCondSet.Manage(rs).MarkTrue(RouteConditionCertificateProvisioned) } func (rs *RouteStatus) MarkCertificateNotReady(name string) { - routeCondSet.Manage(rs).SetCondition(apis.Condition{ - Type: RouteConditionCertificateProvisioned, - Status: corev1.ConditionUnknown, - Severity: apis.ConditionSeverityWarning, - Reason: "CertificateNotReady", - Message: fmt.Sprintf("Certificate %s is not ready.", name), - }) + routeCondSet.Manage(rs).MarkUnknown(RouteConditionCertificateProvisioned, + "CertificateNotReady", + "Certificate %s is not ready.", name) } func (rs *RouteStatus) MarkCertificateNotOwned(name string) { - routeCondSet.Manage(rs).SetCondition(apis.Condition{ - Type: RouteConditionCertificateProvisioned, - Status: corev1.ConditionFalse, - Severity: apis.ConditionSeverityWarning, - Reason: "CertificateNotOwned", - Message: fmt.Sprintf("There is an existing certificate %s that we don't own.", name), - }) + routeCondSet.Manage(rs).MarkFalse(RouteConditionCertificateProvisioned, + "CertificateNotOwned", + "There is an existing certificate %s that we don't own.", name) +} + +func (rs *RouteStatus) MarkCertificateNotEnabled() { + routeCondSet.Manage(rs).MarkTrue(RouteConditionCertificateProvisioned) } // PropagateIngressStatus update RouteConditionIngressReady condition diff --git a/pkg/reconciler/route/route.go b/pkg/reconciler/route/route.go index 0de403c2ba70..afcae7c00dbc 100644 --- a/pkg/reconciler/route/route.go +++ b/pkg/reconciler/route/route.go @@ -189,6 +189,7 @@ func (c *Reconciler) reconcileIngressResources(ctx context.Context, r *v1.Route, func (c *Reconciler) tls(ctx context.Context, host string, r *v1.Route, traffic *traffic.Config) ([]netv1alpha1.IngressTLS, []netv1alpha1.HTTP01Challenge, error) { tls := []netv1alpha1.IngressTLS{} if !config.FromContext(ctx).Network.AutoTLS { + r.Status.MarkCertificateNotEnabled() return tls, nil, nil } domainToTagMap, err := domains.GetAllDomainsAndTags(ctx, r, getTrafficNames(traffic.Targets), traffic.Visibility) diff --git a/pkg/reconciler/route/table_test.go b/pkg/reconciler/route/table_test.go index 7d366145431a..7948d1b9dfbf 100644 --- a/pkg/reconciler/route/table_test.go +++ b/pkg/reconciler/route/table_test.go @@ -1923,13 +1923,13 @@ func TestReconcile_EnableAutoTLS(t *testing.T) { Object: Route("default", "becomes-ready", WithConfigTarget("config"), WithRouteUID("12-34"), // Populated by reconciliation when all traffic has been assigned. - WithURL, WithAddress, WithInitRouteConditions, + WithURL, WithAddress, WithInitRouteConditions, MarkCertificateNotReady, MarkTrafficAssigned, MarkIngressNotConfigured, WithStatusTraffic( v1.TrafficTarget{ RevisionName: "config-00001", Percent: ptr.Int64(100), LatestRevision: ptr.Bool(true), - }), MarkCertificateNotReady), + })), }}, WantEvents: []string{ Eventf(corev1.EventTypeNormal, "Created", "Created placeholder service %q", "becomes-ready"), @@ -2157,15 +2157,15 @@ func TestReconcile_EnableAutoTLS(t *testing.T) { WithRouteUID("12-34"), // Populated by reconciliation when all traffic has been assigned. WithAddress, WithInitRouteConditions, + // The certificate has to be created in the not ready state for the ACME challenge + // ingress rules to be added. + MarkCertificateNotReady, MarkTrafficAssigned, MarkIngressNotConfigured, WithStatusTraffic( v1.TrafficTarget{ RevisionName: "config-00001", Percent: ptr.Int64(100), LatestRevision: ptr.Bool(true), }), - // The certificate has to be created in the not ready state for the ACME challenge - // ingress rules to be added. - MarkCertificateNotReady, // Which also means no HTTPS URL WithURL, ), From d315d8fc3389dfedfe736a609e102f60c312e318 Mon Sep 17 00:00:00 2001 From: Kenjiro Nakayama Date: Fri, 6 Mar 2020 18:39:58 +0900 Subject: [PATCH 02/13] Fix lint error --- pkg/apis/serving/v1/route_lifecycle.go | 2 ++ pkg/apis/serving/v1alpha1/route_lifecycle.go | 2 ++ 2 files changed, 4 insertions(+) diff --git a/pkg/apis/serving/v1/route_lifecycle.go b/pkg/apis/serving/v1/route_lifecycle.go index 226d36b1f1d9..bc0abd849f93 100644 --- a/pkg/apis/serving/v1/route_lifecycle.go +++ b/pkg/apis/serving/v1/route_lifecycle.go @@ -123,6 +123,8 @@ func (rs *RouteStatus) MarkCertificateNotOwned(name string) { "There is an existing certificate %s that we don't own.", name) } +// MarkCertificateNotEnabled sets RouteConditionCertificateProvisioned to true when +// certificate config such as autoTLS is not enabled. func (rs *RouteStatus) MarkCertificateNotEnabled() { routeCondSet.Manage(rs).MarkTrue(RouteConditionCertificateProvisioned) } diff --git a/pkg/apis/serving/v1alpha1/route_lifecycle.go b/pkg/apis/serving/v1alpha1/route_lifecycle.go index c6a06dbd7d9f..fac49dbc0032 100644 --- a/pkg/apis/serving/v1alpha1/route_lifecycle.go +++ b/pkg/apis/serving/v1alpha1/route_lifecycle.go @@ -136,6 +136,8 @@ func (rs *RouteStatus) MarkCertificateNotOwned(name string) { "There is an existing certificate %s that we don't own.", name) } +// MarkCertificateNotEnabled sets RouteConditionCertificateProvisioned to true when +// certificate config such as autoTLS is not enabled. func (rs *RouteStatus) MarkCertificateNotEnabled() { routeCondSet.Manage(rs).MarkTrue(RouteConditionCertificateProvisioned) } From 68ba4456782bee4d24403c4f4fdea58235060f5b Mon Sep 17 00:00:00 2001 From: Kenjiro Nakayama Date: Sat, 7 Mar 2020 16:08:12 +0900 Subject: [PATCH 03/13] Fix review comment (Not work correctly yet) --- pkg/apis/serving/v1/route_lifecycle.go | 12 +++++++++--- pkg/reconciler/route/route.go | 2 +- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/pkg/apis/serving/v1/route_lifecycle.go b/pkg/apis/serving/v1/route_lifecycle.go index bc0abd849f93..7d8f1f50a915 100644 --- a/pkg/apis/serving/v1/route_lifecycle.go +++ b/pkg/apis/serving/v1/route_lifecycle.go @@ -123,10 +123,16 @@ func (rs *RouteStatus) MarkCertificateNotOwned(name string) { "There is an existing certificate %s that we don't own.", name) } -// MarkCertificateNotEnabled sets RouteConditionCertificateProvisioned to true when +// MarkAutoTLSNotEnabled sets RouteConditionCertificateProvisioned to true when // certificate config such as autoTLS is not enabled. -func (rs *RouteStatus) MarkCertificateNotEnabled() { - routeCondSet.Manage(rs).MarkTrue(RouteConditionCertificateProvisioned) +func (rs *RouteStatus) MarkAutoTLSNotEnabled() { + routeCondSet.Manage(rs).SetCondition(apis.Condition{ + Type: RouteConditionCertificateProvisioned, + Status: corev1.ConditionTrue, + Severity: apis.ConditionSeverityWarning, + Reason: "CertificateReady", + Message: "autoTLS is not enabled", + }) } // PropagateIngressStatus update RouteConditionIngressReady condition diff --git a/pkg/reconciler/route/route.go b/pkg/reconciler/route/route.go index afcae7c00dbc..e0f9918f54bc 100644 --- a/pkg/reconciler/route/route.go +++ b/pkg/reconciler/route/route.go @@ -189,7 +189,7 @@ func (c *Reconciler) reconcileIngressResources(ctx context.Context, r *v1.Route, func (c *Reconciler) tls(ctx context.Context, host string, r *v1.Route, traffic *traffic.Config) ([]netv1alpha1.IngressTLS, []netv1alpha1.HTTP01Challenge, error) { tls := []netv1alpha1.IngressTLS{} if !config.FromContext(ctx).Network.AutoTLS { - r.Status.MarkCertificateNotEnabled() + r.Status.MarkAutoTLSNotEnabled() return tls, nil, nil } domainToTagMap, err := domains.GetAllDomainsAndTags(ctx, r, getTrafficNames(traffic.Targets), traffic.Visibility) From 172be648fee446683d848b5522d7ea304820b58e Mon Sep 17 00:00:00 2001 From: Kenjiro Nakayama Date: Sat, 7 Mar 2020 17:40:56 +0900 Subject: [PATCH 04/13] Use MarkTrueWithReason --- pkg/apis/serving/v1/route_lifecycle.go | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/pkg/apis/serving/v1/route_lifecycle.go b/pkg/apis/serving/v1/route_lifecycle.go index 7d8f1f50a915..e22f3c572c17 100644 --- a/pkg/apis/serving/v1/route_lifecycle.go +++ b/pkg/apis/serving/v1/route_lifecycle.go @@ -126,13 +126,9 @@ func (rs *RouteStatus) MarkCertificateNotOwned(name string) { // MarkAutoTLSNotEnabled sets RouteConditionCertificateProvisioned to true when // certificate config such as autoTLS is not enabled. func (rs *RouteStatus) MarkAutoTLSNotEnabled() { - routeCondSet.Manage(rs).SetCondition(apis.Condition{ - Type: RouteConditionCertificateProvisioned, - Status: corev1.ConditionTrue, - Severity: apis.ConditionSeverityWarning, - Reason: "CertificateReady", - Message: "autoTLS is not enabled", - }) + routeCondSet.Manage(rs).MarkTrueWithReason(RouteConditionCertificateProvisioned, + "AutoTLSNotEnabled", + "autoTLS is not enabled") } // PropagateIngressStatus update RouteConditionIngressReady condition From 077afe5cba120d8c1089115b76eb83e8753b0821 Mon Sep 17 00:00:00 2001 From: Kenjiro Nakayama Date: Fri, 20 Mar 2020 11:01:27 +0900 Subject: [PATCH 05/13] Fix unit test --- pkg/reconciler/route/table_test.go | 62 +++++++++++++++--------------- pkg/testing/v1/route.go | 6 +++ 2 files changed, 37 insertions(+), 31 deletions(-) diff --git a/pkg/reconciler/route/table_test.go b/pkg/reconciler/route/table_test.go index 7948d1b9dfbf..c531274ff5a7 100644 --- a/pkg/reconciler/route/table_test.go +++ b/pkg/reconciler/route/table_test.go @@ -157,7 +157,7 @@ func TestReconcile(t *testing.T) { Object: Route("default", "becomes-ready", WithConfigTarget("config"), WithRouteUID("12-34"), // Populated by reconciliation when all traffic has been assigned. - WithURL, WithAddress, WithInitRouteConditions, + WithURL, WithAddress, WithRouteConditionsAutoTLSDisabled, MarkTrafficAssigned, MarkIngressNotConfigured, WithStatusTraffic( v1.TrafficTarget{ RevisionName: "config-00001", @@ -255,7 +255,7 @@ func TestReconcile(t *testing.T) { Object: Route("default", "becomes-ready", WithConfigTarget("config"), WithRouteUID("12-34"), WithIngressClass("custom-ingress-class"), // Populated by reconciliation when all traffic has been assigned. - WithURL, WithAddress, WithInitRouteConditions, + WithURL, WithAddress, WithRouteConditionsAutoTLSDisabled, MarkTrafficAssigned, MarkIngressNotConfigured, WithStatusTraffic( v1.TrafficTarget{ RevisionName: "config-00001", @@ -312,7 +312,7 @@ func TestReconcile(t *testing.T) { Object: Route("default", "becomes-ready", WithConfigTarget("config"), WithRouteUID("65-23"), // Populated by reconciliation when all traffic has been assigned. - WithLocalDomain, WithAddress, WithInitRouteConditions, + WithLocalDomain, WithAddress, WithRouteConditionsAutoTLSDisabled, WithRouteLabel(map[string]string{"serving.knative.dev/visibility": "cluster-local"}), MarkTrafficAssigned, MarkIngressNotConfigured, WithStatusTraffic( v1.TrafficTarget{ @@ -358,7 +358,7 @@ func TestReconcile(t *testing.T) { WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ Object: Route("default", "becomes-ready", WithConfigTarget("config"), // Populated by reconciliation when the route becomes ready. - WithURL, WithAddress, WithInitRouteConditions, + WithURL, WithAddress, WithRouteConditionsAutoTLSDisabled, MarkTrafficAssigned, MarkIngressReady, WithStatusTraffic( v1.TrafficTarget{ RevisionName: "config-00001", @@ -448,7 +448,7 @@ func TestReconcile(t *testing.T) { Object: Route("default", "ingress-create-failure", WithConfigTarget("config"), WithRouteFinalizer, // Populated by reconciliation when we fail to create the ingress. - WithURL, WithAddress, WithInitRouteConditions, + WithURL, WithAddress, WithRouteConditionsAutoTLSDisabled, MarkTrafficAssigned, WithStatusTraffic( v1.TrafficTarget{ RevisionName: "config-00001", @@ -466,7 +466,7 @@ func TestReconcile(t *testing.T) { Name: "steady state", Objects: []runtime.Object{ Route("default", "steady-state", WithConfigTarget("config"), - WithURL, WithAddress, WithInitRouteConditions, + WithURL, WithAddress, WithRouteConditionsAutoTLSDisabled, MarkTrafficAssigned, MarkIngressReady, WithRouteFinalizer, WithStatusTraffic( v1.TrafficTarget{ @@ -503,7 +503,7 @@ func TestReconcile(t *testing.T) { WantErr: true, Objects: []runtime.Object{ Route("default", "unhappy-owner", WithConfigTarget("config"), - WithURL, WithAddress, WithInitRouteConditions, + WithURL, WithAddress, WithRouteConditionsAutoTLSDisabled, MarkTrafficAssigned, MarkIngressReady, WithRouteFinalizer, WithStatusTraffic( v1.TrafficTarget{ RevisionName: "config-00001", @@ -542,7 +542,7 @@ func TestReconcile(t *testing.T) { Objects: []runtime.Object{ Route("default", "different-domain", WithConfigTarget("config"), WithAnotherDomain, WithAddress, - WithInitRouteConditions, MarkTrafficAssigned, MarkIngressReady, + WithRouteConditionsAutoTLSDisabled, MarkTrafficAssigned, MarkIngressReady, WithRouteFinalizer, WithStatusTraffic( v1.TrafficTarget{ RevisionName: "config-00001", @@ -604,7 +604,7 @@ func TestReconcile(t *testing.T) { Name: "new latest created revision", Objects: []runtime.Object{ Route("default", "new-latest-created", WithConfigTarget("config"), - WithURL, WithAddress, WithInitRouteConditions, + WithURL, WithAddress, WithRouteConditionsAutoTLSDisabled, MarkTrafficAssigned, MarkIngressReady, WithRouteFinalizer, WithStatusTraffic( v1.TrafficTarget{ RevisionName: "config-00001", @@ -642,7 +642,7 @@ func TestReconcile(t *testing.T) { Name: "new latest ready revision", Objects: []runtime.Object{ Route("default", "new-latest-ready", WithConfigTarget("config"), - WithURL, WithAddress, WithInitRouteConditions, + WithURL, WithAddress, WithRouteConditionsAutoTLSDisabled, MarkTrafficAssigned, MarkIngressReady, WithRouteFinalizer, WithStatusTraffic( v1.TrafficTarget{ RevisionName: "config-00001", @@ -695,7 +695,7 @@ func TestReconcile(t *testing.T) { }}, WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ Object: Route("default", "new-latest-ready", WithConfigTarget("config"), - WithURL, WithAddress, WithInitRouteConditions, + WithURL, WithAddress, WithRouteConditionsAutoTLSDisabled, MarkTrafficAssigned, MarkIngressReady, WithRouteFinalizer, WithStatusTraffic( v1.TrafficTarget{ RevisionName: "config-00002", @@ -760,7 +760,7 @@ func TestReconcile(t *testing.T) { Object: Route("default", "becomes-local", WithConfigTarget("config"), WithRouteUID("65-23"), MarkTrafficAssigned, MarkIngressNotConfigured, - WithLocalDomain, WithAddress, WithInitRouteConditions, + WithLocalDomain, WithAddress, WithRouteConditionsAutoTLSDisabled, WithRouteLabel(map[string]string{"serving.knative.dev/visibility": "cluster-local"}), WithStatusTraffic( v1.TrafficTarget{ @@ -824,7 +824,7 @@ func TestReconcile(t *testing.T) { Object: Route("default", "becomes-public", WithConfigTarget("config"), WithRouteUID("65-23"), MarkTrafficAssigned, MarkIngressNotConfigured, - WithAddress, WithInitRouteConditions, WithURL, + WithAddress, WithRouteConditionsAutoTLSDisabled, WithURL, WithStatusTraffic( v1.TrafficTarget{ RevisionName: "config-00001", @@ -842,7 +842,7 @@ func TestReconcile(t *testing.T) { }, Objects: []runtime.Object{ Route("default", "update-ci-failure", WithConfigTarget("config"), - WithURL, WithAddress, WithInitRouteConditions, + WithURL, WithAddress, WithRouteConditionsAutoTLSDisabled, MarkTrafficAssigned, MarkIngressReady, WithRouteFinalizer, WithStatusTraffic( v1.TrafficTarget{ RevisionName: "config-00001", @@ -894,7 +894,7 @@ func TestReconcile(t *testing.T) { }}, WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ Object: Route("default", "update-ci-failure", WithConfigTarget("config"), - WithURL, WithAddress, WithInitRouteConditions, + WithURL, WithAddress, WithRouteConditionsAutoTLSDisabled, MarkTrafficAssigned, MarkIngressReady, WithRouteFinalizer, WithStatusTraffic( v1.TrafficTarget{ RevisionName: "config-00002", @@ -910,7 +910,7 @@ func TestReconcile(t *testing.T) { Name: "reconcile service mutation", Objects: []runtime.Object{ Route("default", "svc-mutation", WithConfigTarget("config"), - WithURL, WithAddress, WithInitRouteConditions, + WithURL, WithAddress, WithRouteConditionsAutoTLSDisabled, MarkTrafficAssigned, MarkIngressReady, WithRouteFinalizer, WithStatusTraffic( v1.TrafficTarget{ RevisionName: "config-00001", @@ -954,7 +954,7 @@ func TestReconcile(t *testing.T) { }, Objects: []runtime.Object{ Route("default", "svc-mutation", WithConfigTarget("config"), WithRouteFinalizer, - WithURL, WithAddress, WithInitRouteConditions, + WithURL, WithAddress, WithRouteConditionsAutoTLSDisabled, MarkTrafficAssigned, MarkIngressReady, WithStatusTraffic( v1.TrafficTarget{ RevisionName: "config-00001", @@ -999,7 +999,7 @@ func TestReconcile(t *testing.T) { Name: "drop cluster ip", Objects: []runtime.Object{ Route("default", "cluster-ip", WithConfigTarget("config"), WithRouteFinalizer, - WithURL, WithAddress, WithInitRouteConditions, + WithURL, WithAddress, WithRouteConditionsAutoTLSDisabled, MarkTrafficAssigned, MarkIngressReady, WithStatusTraffic( v1.TrafficTarget{ RevisionName: "config-00001", @@ -1039,7 +1039,7 @@ func TestReconcile(t *testing.T) { Name: "fix external name", Objects: []runtime.Object{ Route("default", "external-name", WithConfigTarget("config"), WithRouteFinalizer, - WithURL, WithAddress, WithInitRouteConditions, + WithURL, WithAddress, WithRouteConditionsAutoTLSDisabled, MarkTrafficAssigned, MarkIngressReady, WithStatusTraffic( v1.TrafficTarget{ RevisionName: "config-00001", @@ -1078,7 +1078,7 @@ func TestReconcile(t *testing.T) { Name: "reconcile ingress mutation", Objects: []runtime.Object{ Route("default", "ingress-mutation", WithConfigTarget("config"), WithRouteFinalizer, - WithURL, WithAddress, WithInitRouteConditions, + WithURL, WithAddress, WithRouteConditionsAutoTLSDisabled, MarkTrafficAssigned, MarkIngressReady, WithStatusTraffic( v1.TrafficTarget{ RevisionName: "config-00001", @@ -1133,7 +1133,7 @@ func TestReconcile(t *testing.T) { Objects: []runtime.Object{ // The status reflects "oldconfig", but the spec "newconfig". Route("default", "change-configs", WithConfigTarget("newconfig"), WithRouteFinalizer, - WithURL, WithAddress, WithInitRouteConditions, + WithURL, WithAddress, WithRouteConditionsAutoTLSDisabled, MarkTrafficAssigned, MarkIngressReady, WithStatusTraffic( v1.TrafficTarget{ RevisionName: "oldconfig-00001", @@ -1189,7 +1189,7 @@ func TestReconcile(t *testing.T) { WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ // Status updated to "newconfig" Object: Route("default", "change-configs", WithConfigTarget("newconfig"), - WithURL, WithAddress, WithInitRouteConditions, + WithURL, WithAddress, WithRouteConditionsAutoTLSDisabled, MarkTrafficAssigned, MarkIngressReady, WithRouteFinalizer, WithStatusTraffic( v1.TrafficTarget{ RevisionName: "newconfig-00001", @@ -1270,7 +1270,7 @@ func TestReconcile(t *testing.T) { Object: Route("default", "pinned-becomes-ready", // Use the Revision name from the config WithRevTarget("config-00001"), WithRouteFinalizer, - WithURL, WithAddress, WithInitRouteConditions, + WithURL, WithAddress, WithRouteConditionsAutoTLSDisabled, MarkTrafficAssigned, MarkIngressReady, WithStatusTraffic( v1.TrafficTarget{ RevisionName: "config-00001", @@ -1352,7 +1352,7 @@ func TestReconcile(t *testing.T) { ConfigurationName: "green", Percent: ptr.Int64(50), }), WithRouteUID("34-78"), - WithURL, WithAddress, WithInitRouteConditions, + WithURL, WithAddress, WithRouteConditionsAutoTLSDisabled, MarkTrafficAssigned, MarkIngressNotConfigured, WithStatusTraffic( v1.TrafficTarget{ RevisionName: "blue-00001", @@ -1489,7 +1489,7 @@ func TestReconcile(t *testing.T) { RevisionName: "gray-00001", Percent: ptr.Int64(50), }), WithRouteUID("1-2"), WithRouteFinalizer, - WithURL, WithAddress, WithInitRouteConditions, + WithURL, WithAddress, WithRouteConditionsAutoTLSDisabled, MarkTrafficAssigned, MarkIngressNotConfigured, WithStatusTraffic( v1.TrafficTarget{ Tag: "gray", @@ -1524,7 +1524,7 @@ func TestReconcile(t *testing.T) { // Start from a steady state referencing "blue", and modify the route spec to point to "green" instead. Objects: []runtime.Object{ Route("default", "switch-configs", WithConfigTarget("green"), - WithURL, WithAddress, WithInitRouteConditions, + WithURL, WithAddress, WithRouteConditionsAutoTLSDisabled, MarkTrafficAssigned, MarkIngressReady, WithStatusTraffic( v1.TrafficTarget{ Tag: "blue", @@ -1579,7 +1579,7 @@ func TestReconcile(t *testing.T) { }}, WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ Object: Route("default", "switch-configs", WithConfigTarget("green"), - WithURL, WithAddress, WithInitRouteConditions, + WithURL, WithAddress, WithRouteConditionsAutoTLSDisabled, MarkTrafficAssigned, MarkIngressReady, WithStatusTraffic( v1.TrafficTarget{ RevisionName: "green-00001", @@ -1642,7 +1642,7 @@ func TestReconcile(t *testing.T) { Name: "Update stale lastPinned", Objects: []runtime.Object{ Route("default", "stale-lastpinned", WithConfigTarget("config"), - WithURL, WithAddress, WithInitRouteConditions, + WithURL, WithAddress, WithRouteConditionsAutoTLSDisabled, MarkTrafficAssigned, MarkIngressReady, WithRouteFinalizer, WithStatusTraffic( v1.TrafficTarget{ RevisionName: "config-00001", @@ -1682,7 +1682,7 @@ func TestReconcile(t *testing.T) { Name: "check that we can find the ingress with old naming", Objects: []runtime.Object{ Route("default", "old-naming", WithConfigTarget("config"), WithRouteFinalizer, - WithURL, WithAddress, WithInitRouteConditions, + WithURL, WithAddress, WithRouteConditionsAutoTLSDisabled, MarkTrafficAssigned, MarkIngressReady, WithStatusTraffic( v1.TrafficTarget{ RevisionName: "config-00001", @@ -1718,7 +1718,7 @@ func TestReconcile(t *testing.T) { Name: "deletes service when route no longer references service", Objects: []runtime.Object{ Route("default", "my-route", WithConfigTarget("config"), - WithURL, WithAddress, WithInitRouteConditions, + WithURL, WithAddress, WithRouteConditionsAutoTLSDisabled, MarkTrafficAssigned, MarkIngressReady, WithRouteFinalizer, WithStatusTraffic( v1.TrafficTarget{ @@ -2279,7 +2279,7 @@ func TestReconcile_EnableAutoTLS(t *testing.T) { Object: Route("default", "becomes-ready", WithConfigTarget("config"), WithRouteUID("12-34"), // Populated by reconciliation when all traffic has been assigned. - WithAddress, WithInitRouteConditions, + WithAddress, WithRouteConditionsAutoTLSDisabled, MarkTrafficAssigned, MarkIngressNotConfigured, WithStatusTraffic( v1.TrafficTarget{ RevisionName: "config-00001", diff --git a/pkg/testing/v1/route.go b/pkg/testing/v1/route.go index 186f6d8804ae..c8969c331993 100644 --- a/pkg/testing/v1/route.go +++ b/pkg/testing/v1/route.go @@ -159,6 +159,12 @@ func WithInitRouteConditions(rt *v1.Route) { rt.Status.InitializeConditions() } +// WithRouteConditionsAutoTLSDisabled calls MarkAutoTLSNotEnabled after initialized the Service's conditions. +func WithRouteConditionsAutoTLSDisabled(rt *v1.Route) { + rt.Status.InitializeConditions() + rt.Status.MarkAutoTLSNotEnabled() +} + // MarkTrafficAssigned calls the method of the same name on .Status func MarkTrafficAssigned(r *v1.Route) { r.Status.MarkTrafficAssigned() From b14d17624e9b9c844bb5918a6b1355559255f0f0 Mon Sep 17 00:00:00 2001 From: Kenjiro Nakayama Date: Fri, 20 Mar 2020 11:17:39 +0900 Subject: [PATCH 06/13] Add unit test for MarkAutoTLSNotEnabled --- pkg/apis/serving/v1/route_lifecycle_test.go | 8 ++++++++ pkg/apis/serving/v1alpha1/route_lifecycle_test.go | 8 ++++++++ 2 files changed, 16 insertions(+) diff --git a/pkg/apis/serving/v1/route_lifecycle_test.go b/pkg/apis/serving/v1/route_lifecycle_test.go index 3b16f5559a00..aabb9c39fb9a 100644 --- a/pkg/apis/serving/v1/route_lifecycle_test.go +++ b/pkg/apis/serving/v1/route_lifecycle_test.go @@ -372,6 +372,14 @@ func TestRouteNotOwnCertificate(t *testing.T) { apistest.CheckConditionFailed(r, RouteConditionCertificateProvisioned, t) } +func TestRouteAutoTLSNotEnabled(t *testing.T) { + r := &RouteStatus{} + r.InitializeConditions() + r.MarkAutoTLSNotEnabled() + + apistest.CheckConditionSucceeded(r, RouteConditionCertificateProvisioned, t) +} + func TestIngressNotConfigured(t *testing.T) { r := &RouteStatus{} r.InitializeConditions() diff --git a/pkg/apis/serving/v1alpha1/route_lifecycle_test.go b/pkg/apis/serving/v1alpha1/route_lifecycle_test.go index 04dec0bf6ac4..25c48282be78 100644 --- a/pkg/apis/serving/v1alpha1/route_lifecycle_test.go +++ b/pkg/apis/serving/v1alpha1/route_lifecycle_test.go @@ -376,6 +376,14 @@ func TestRouteNotOwnCertificate(t *testing.T) { apistest.CheckConditionFailed(r, RouteConditionCertificateProvisioned, t) } +func TestRouteAutoTLSNotEnabled(t *testing.T) { + r := &RouteStatus{} + r.InitializeConditions() + r.MarkAutoTLSNotEnabled() + + apistest.CheckConditionSucceeded(r, RouteConditionCertificateProvisioned, t) +} + func TestIngressNotConfigured(t *testing.T) { r := &RouteStatus{} r.InitializeConditions() From 6f71b22d29861835d584ba8fcbc49fbce9da7f06 Mon Sep 17 00:00:00 2001 From: Kenjiro Nakayama Date: Fri, 20 Mar 2020 12:58:16 +0900 Subject: [PATCH 07/13] Fix unit test --- pkg/apis/serving/v1alpha1/route_lifecycle.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/pkg/apis/serving/v1alpha1/route_lifecycle.go b/pkg/apis/serving/v1alpha1/route_lifecycle.go index fac49dbc0032..4235231ddded 100644 --- a/pkg/apis/serving/v1alpha1/route_lifecycle.go +++ b/pkg/apis/serving/v1alpha1/route_lifecycle.go @@ -136,6 +136,14 @@ func (rs *RouteStatus) MarkCertificateNotOwned(name string) { "There is an existing certificate %s that we don't own.", name) } +// MarkAutoTLSNotEnabled sets RouteConditionCertificateProvisioned to true when +// certificate config such as autoTLS is not enabled. +func (rs *RouteStatus) MarkAutoTLSNotEnabled() { + routeCondSet.Manage(rs).MarkTrueWithReason(RouteConditionCertificateProvisioned, + "AutoTLSNotEnabled", + "autoTLS is not enabled") +} + // MarkCertificateNotEnabled sets RouteConditionCertificateProvisioned to true when // certificate config such as autoTLS is not enabled. func (rs *RouteStatus) MarkCertificateNotEnabled() { From 4e3f3e265cbec4e91b6ccfc6089c164daa32d7bd Mon Sep 17 00:00:00 2001 From: Kenjiro Nakayama Date: Fri, 20 Mar 2020 16:01:48 +0900 Subject: [PATCH 08/13] Do not add RouteConditionCertificateProvisioned to NewLivingConditionSet If NewLivingConditionSet is added new condition, it fails when downgrade test. Add RouteConditionCertificateProvisioned in the next release --- pkg/apis/serving/v1/route_lifecycle.go | 6 +++--- pkg/apis/serving/v1alpha1/route_lifecycle.go | 6 +++--- pkg/reconciler/route/table_test.go | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/pkg/apis/serving/v1/route_lifecycle.go b/pkg/apis/serving/v1/route_lifecycle.go index e22f3c572c17..e3a2eabdcd87 100644 --- a/pkg/apis/serving/v1/route_lifecycle.go +++ b/pkg/apis/serving/v1/route_lifecycle.go @@ -29,7 +29,9 @@ import ( var routeCondSet = apis.NewLivingConditionSet( RouteConditionAllTrafficAssigned, RouteConditionIngressReady, - RouteConditionCertificateProvisioned, + +// TODO(nak3): Add RouteConditionCertificateProvisioned in the next release. +// RouteConditionCertificateProvisioned, ) // GetGroupVersionKind returns the GroupVersionKind. @@ -45,8 +47,6 @@ func (rs *RouteStatus) IsReady() bool { // InitializeConditions sets the initial values to the conditions. func (rs *RouteStatus) InitializeConditions() { routeCondSet.Manage(rs).InitializeConditions() - // Since Certificate is optional, initialize the status with Ready. - routeCondSet.Manage(rs).MarkTrue(RouteConditionCertificateProvisioned) } // MarkServiceNotOwned changes the IngressReady status to be false with the reason being that diff --git a/pkg/apis/serving/v1alpha1/route_lifecycle.go b/pkg/apis/serving/v1alpha1/route_lifecycle.go index 4235231ddded..4bae71d11e0a 100644 --- a/pkg/apis/serving/v1alpha1/route_lifecycle.go +++ b/pkg/apis/serving/v1alpha1/route_lifecycle.go @@ -29,7 +29,9 @@ import ( var routeCondSet = apis.NewLivingConditionSet( RouteConditionAllTrafficAssigned, RouteConditionIngressReady, - RouteConditionCertificateProvisioned, + +// TODO(nak3): Add RouteConditionCertificateProvisioned in the next release. +// RouteConditionCertificateProvisioned, ) func (r *Route) GetGroupVersionKind() schema.GroupVersionKind { @@ -46,8 +48,6 @@ func (rs *RouteStatus) GetCondition(t apis.ConditionType) *apis.Condition { func (rs *RouteStatus) InitializeConditions() { routeCondSet.Manage(rs).InitializeConditions() - // Since Certificate is optional, initialize the status with Ready. - routeCondSet.Manage(rs).MarkTrue(RouteConditionCertificateProvisioned) } // // MarkResourceNotConvertible adds a Warning-severity condition to the resource noting that diff --git a/pkg/reconciler/route/table_test.go b/pkg/reconciler/route/table_test.go index c531274ff5a7..78cd377c7a4d 100644 --- a/pkg/reconciler/route/table_test.go +++ b/pkg/reconciler/route/table_test.go @@ -520,7 +520,7 @@ func TestReconcile(t *testing.T) { }, WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ Object: Route("default", "unhappy-owner", WithConfigTarget("config"), - WithURL, WithAddress, WithInitRouteConditions, + WithURL, WithAddress, WithRouteConditionsAutoTLSDisabled, MarkTrafficAssigned, MarkIngressReady, WithRouteFinalizer, WithStatusTraffic( v1.TrafficTarget{ RevisionName: "config-00001", From b4210e98bb852db112479c945db2048035d17b08 Mon Sep 17 00:00:00 2001 From: Kenjiro Nakayama Date: Fri, 17 Apr 2020 11:50:00 +0900 Subject: [PATCH 09/13] Remove TODO --- pkg/apis/serving/v1/route_lifecycle.go | 4 +--- pkg/apis/serving/v1alpha1/route_lifecycle.go | 4 +--- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/pkg/apis/serving/v1/route_lifecycle.go b/pkg/apis/serving/v1/route_lifecycle.go index e3a2eabdcd87..eed07ea89a2d 100644 --- a/pkg/apis/serving/v1/route_lifecycle.go +++ b/pkg/apis/serving/v1/route_lifecycle.go @@ -29,9 +29,7 @@ import ( var routeCondSet = apis.NewLivingConditionSet( RouteConditionAllTrafficAssigned, RouteConditionIngressReady, - -// TODO(nak3): Add RouteConditionCertificateProvisioned in the next release. -// RouteConditionCertificateProvisioned, + RouteConditionCertificateProvisioned, ) // GetGroupVersionKind returns the GroupVersionKind. diff --git a/pkg/apis/serving/v1alpha1/route_lifecycle.go b/pkg/apis/serving/v1alpha1/route_lifecycle.go index 4bae71d11e0a..08f6cb548fdd 100644 --- a/pkg/apis/serving/v1alpha1/route_lifecycle.go +++ b/pkg/apis/serving/v1alpha1/route_lifecycle.go @@ -29,9 +29,7 @@ import ( var routeCondSet = apis.NewLivingConditionSet( RouteConditionAllTrafficAssigned, RouteConditionIngressReady, - -// TODO(nak3): Add RouteConditionCertificateProvisioned in the next release. -// RouteConditionCertificateProvisioned, + RouteConditionCertificateProvisioned, ) func (r *Route) GetGroupVersionKind() schema.GroupVersionKind { From e591fa7d9b2933c3aeeabb6917e5de7553b74e8c Mon Sep 17 00:00:00 2001 From: Kenjiro Nakayama Date: Fri, 17 Apr 2020 11:59:25 +0900 Subject: [PATCH 10/13] Update unit test --- pkg/apis/serving/v1/route_lifecycle_test.go | 2 ++ pkg/apis/serving/v1alpha1/route_lifecycle_test.go | 2 ++ pkg/reconciler/route/table_test.go | 2 +- 3 files changed, 5 insertions(+), 1 deletion(-) diff --git a/pkg/apis/serving/v1/route_lifecycle_test.go b/pkg/apis/serving/v1/route_lifecycle_test.go index aabb9c39fb9a..ff095b256dd7 100644 --- a/pkg/apis/serving/v1/route_lifecycle_test.go +++ b/pkg/apis/serving/v1/route_lifecycle_test.go @@ -168,6 +168,7 @@ func TestTypicalRouteFlow(t *testing.T) { apistest.CheckConditionOngoing(r, RouteConditionReady, t) r.MarkTrafficAssigned() + r.MarkAutoTLSNotEnabled() apistest.CheckConditionSucceeded(r, RouteConditionAllTrafficAssigned, t) apistest.CheckConditionOngoing(r, RouteConditionIngressReady, t) apistest.CheckConditionOngoing(r, RouteConditionReady, t) @@ -281,6 +282,7 @@ func TestIngressFailureRecovery(t *testing.T) { apistest.CheckConditionOngoing(r, RouteConditionReady, t) r.MarkTrafficAssigned() + r.MarkAutoTLSNotEnabled() r.PropagateIngressStatus(netv1alpha1.IngressStatus{ Status: duckv1.Status{ Conditions: duckv1.Conditions{{ diff --git a/pkg/apis/serving/v1alpha1/route_lifecycle_test.go b/pkg/apis/serving/v1alpha1/route_lifecycle_test.go index 25c48282be78..df55bd586983 100644 --- a/pkg/apis/serving/v1alpha1/route_lifecycle_test.go +++ b/pkg/apis/serving/v1alpha1/route_lifecycle_test.go @@ -160,6 +160,7 @@ func TestTypicalRouteFlow(t *testing.T) { apistest.CheckConditionOngoing(r, RouteConditionReady, t) r.MarkTrafficAssigned() + r.MarkAutoTLSNotEnabled() apistest.CheckConditionSucceeded(r, RouteConditionAllTrafficAssigned, t) apistest.CheckConditionOngoing(r, RouteConditionIngressReady, t) apistest.CheckConditionOngoing(r, RouteConditionReady, t) @@ -273,6 +274,7 @@ func TestIngressFailureRecovery(t *testing.T) { apistest.CheckConditionOngoing(r, RouteConditionReady, t) r.MarkTrafficAssigned() + r.MarkAutoTLSNotEnabled() r.PropagateIngressStatus(netv1alpha1.IngressStatus{ Status: duckv1.Status{ Conditions: duckv1.Conditions{{ diff --git a/pkg/reconciler/route/table_test.go b/pkg/reconciler/route/table_test.go index 78cd377c7a4d..70445aba0483 100644 --- a/pkg/reconciler/route/table_test.go +++ b/pkg/reconciler/route/table_test.go @@ -204,7 +204,7 @@ func TestReconcile(t *testing.T) { Object: Route("default", "ingress-failed", WithConfigTarget("config"), WithRouteUID("12-34"), // Populated by reconciliation when all traffic has been assigned. - WithURL, WithAddress, WithInitRouteConditions, + WithURL, WithAddress, WithRouteConditionsAutoTLSDisabled, WithInitRouteConditions, MarkTrafficAssigned, WithStatusTraffic( v1.TrafficTarget{ From 410aaf6bcd0f2fb2c54645379e617bc441344a4d Mon Sep 17 00:00:00 2001 From: Kenjiro Nakayama Date: Fri, 17 Apr 2020 12:13:15 +0900 Subject: [PATCH 11/13] Remove unused func --- pkg/apis/serving/v1alpha1/route_lifecycle.go | 6 ------ 1 file changed, 6 deletions(-) diff --git a/pkg/apis/serving/v1alpha1/route_lifecycle.go b/pkg/apis/serving/v1alpha1/route_lifecycle.go index 08f6cb548fdd..18bc2ed29b2f 100644 --- a/pkg/apis/serving/v1alpha1/route_lifecycle.go +++ b/pkg/apis/serving/v1alpha1/route_lifecycle.go @@ -142,12 +142,6 @@ func (rs *RouteStatus) MarkAutoTLSNotEnabled() { "autoTLS is not enabled") } -// MarkCertificateNotEnabled sets RouteConditionCertificateProvisioned to true when -// certificate config such as autoTLS is not enabled. -func (rs *RouteStatus) MarkCertificateNotEnabled() { - routeCondSet.Manage(rs).MarkTrue(RouteConditionCertificateProvisioned) -} - // PropagateIngressStatus update RouteConditionIngressReady condition // in RouteStatus according to IngressStatus. func (rs *RouteStatus) PropagateIngressStatus(cs v1alpha1.IngressStatus) { From 6a2d8fb34ccbbe56a370f7290fd0d97772df7cb5 Mon Sep 17 00:00:00 2001 From: Kenjiro Nakayama Date: Tue, 21 Apr 2020 10:28:33 +0900 Subject: [PATCH 12/13] Add MarkHTTPDownward --- pkg/apis/serving/v1/route_lifecycle.go | 8 ++++++++ pkg/apis/serving/v1/route_lifecycle_test.go | 8 ++++++++ pkg/apis/serving/v1alpha1/route_lifecycle.go | 8 ++++++++ pkg/apis/serving/v1alpha1/route_lifecycle_test.go | 8 ++++++++ pkg/reconciler/route/route.go | 1 + pkg/reconciler/route/table_test.go | 9 ++++----- pkg/testing/v1/route.go | 6 ++++++ 7 files changed, 43 insertions(+), 5 deletions(-) diff --git a/pkg/apis/serving/v1/route_lifecycle.go b/pkg/apis/serving/v1/route_lifecycle.go index eed07ea89a2d..0d60a4ac6d6a 100644 --- a/pkg/apis/serving/v1/route_lifecycle.go +++ b/pkg/apis/serving/v1/route_lifecycle.go @@ -129,6 +129,14 @@ func (rs *RouteStatus) MarkAutoTLSNotEnabled() { "autoTLS is not enabled") } +// MarkHTTPDownward sets RouteConditionCertificateProvisioned to true when plain +// HTTP is enabled even when Certificated is not ready. +func (rs *RouteStatus) MarkHTTPDownward(name string) { + routeCondSet.Manage(rs).MarkTrueWithReason(RouteConditionCertificateProvisioned, + "HTTPDownward", + "Certificate %s is not ready downard HTTP.", name) +} + // PropagateIngressStatus update RouteConditionIngressReady condition // in RouteStatus according to IngressStatus. func (rs *RouteStatus) PropagateIngressStatus(cs v1alpha1.IngressStatus) { diff --git a/pkg/apis/serving/v1/route_lifecycle_test.go b/pkg/apis/serving/v1/route_lifecycle_test.go index ff095b256dd7..edb6c1c47083 100644 --- a/pkg/apis/serving/v1/route_lifecycle_test.go +++ b/pkg/apis/serving/v1/route_lifecycle_test.go @@ -382,6 +382,14 @@ func TestRouteAutoTLSNotEnabled(t *testing.T) { apistest.CheckConditionSucceeded(r, RouteConditionCertificateProvisioned, t) } +func TestRouteHTTPDownward(t *testing.T) { + r := &RouteStatus{} + r.InitializeConditions() + r.MarkHTTPDownward("cert") + + apistest.CheckConditionSucceeded(r, RouteConditionCertificateProvisioned, t) +} + func TestIngressNotConfigured(t *testing.T) { r := &RouteStatus{} r.InitializeConditions() diff --git a/pkg/apis/serving/v1alpha1/route_lifecycle.go b/pkg/apis/serving/v1alpha1/route_lifecycle.go index 18bc2ed29b2f..7cf0effef9b2 100644 --- a/pkg/apis/serving/v1alpha1/route_lifecycle.go +++ b/pkg/apis/serving/v1alpha1/route_lifecycle.go @@ -142,6 +142,14 @@ func (rs *RouteStatus) MarkAutoTLSNotEnabled() { "autoTLS is not enabled") } +// MarkHTTPDownward sets RouteConditionCertificateProvisioned to true when plain +// HTTP is enabled even when Certificated is not ready. +func (rs *RouteStatus) MarkHTTPDownward(name string) { + routeCondSet.Manage(rs).MarkTrueWithReason(RouteConditionCertificateProvisioned, + "HTTPDownward", + "Certificate %s is not ready downard HTTP.", name) +} + // PropagateIngressStatus update RouteConditionIngressReady condition // in RouteStatus according to IngressStatus. func (rs *RouteStatus) PropagateIngressStatus(cs v1alpha1.IngressStatus) { diff --git a/pkg/apis/serving/v1alpha1/route_lifecycle_test.go b/pkg/apis/serving/v1alpha1/route_lifecycle_test.go index df55bd586983..209c58f92f0e 100644 --- a/pkg/apis/serving/v1alpha1/route_lifecycle_test.go +++ b/pkg/apis/serving/v1alpha1/route_lifecycle_test.go @@ -386,6 +386,14 @@ func TestRouteAutoTLSNotEnabled(t *testing.T) { apistest.CheckConditionSucceeded(r, RouteConditionCertificateProvisioned, t) } +func TestRouteHTTPDownward(t *testing.T) { + r := &RouteStatus{} + r.InitializeConditions() + r.MarkHTTPDownward("cert") + + apistest.CheckConditionSucceeded(r, RouteConditionCertificateProvisioned, t) +} + func TestIngressNotConfigured(t *testing.T) { r := &RouteStatus{} r.InitializeConditions() diff --git a/pkg/reconciler/route/route.go b/pkg/reconciler/route/route.go index e0f9918f54bc..16a46051e7e7 100644 --- a/pkg/reconciler/route/route.go +++ b/pkg/reconciler/route/route.go @@ -258,6 +258,7 @@ func (c *Reconciler) tls(ctx context.Context, host string, r *v1.Route, traffic } } setTargetsScheme(&r.Status, dnsNames.List(), "http") + r.Status.MarkHTTPDownward(cert.Name) } } } diff --git a/pkg/reconciler/route/table_test.go b/pkg/reconciler/route/table_test.go index 70445aba0483..479b0ab36e1f 100644 --- a/pkg/reconciler/route/table_test.go +++ b/pkg/reconciler/route/table_test.go @@ -1923,7 +1923,7 @@ func TestReconcile_EnableAutoTLS(t *testing.T) { Object: Route("default", "becomes-ready", WithConfigTarget("config"), WithRouteUID("12-34"), // Populated by reconciliation when all traffic has been assigned. - WithURL, WithAddress, WithInitRouteConditions, MarkCertificateNotReady, + WithURL, WithAddress, WithInitRouteConditions, WithRouteConditionsHTTPDownward, MarkTrafficAssigned, MarkIngressNotConfigured, WithStatusTraffic( v1.TrafficTarget{ RevisionName: "config-00001", @@ -2159,7 +2159,6 @@ func TestReconcile_EnableAutoTLS(t *testing.T) { WithAddress, WithInitRouteConditions, // The certificate has to be created in the not ready state for the ACME challenge // ingress rules to be added. - MarkCertificateNotReady, MarkTrafficAssigned, MarkIngressNotConfigured, WithStatusTraffic( v1.TrafficTarget{ RevisionName: "config-00001", @@ -2167,7 +2166,7 @@ func TestReconcile_EnableAutoTLS(t *testing.T) { LatestRevision: ptr.Bool(true), }), // Which also means no HTTPS URL - WithURL, + WithURL, WithRouteConditionsHTTPDownward, ), }}, Key: "default/becomes-ready", @@ -2279,13 +2278,13 @@ func TestReconcile_EnableAutoTLS(t *testing.T) { Object: Route("default", "becomes-ready", WithConfigTarget("config"), WithRouteUID("12-34"), // Populated by reconciliation when all traffic has been assigned. - WithAddress, WithRouteConditionsAutoTLSDisabled, + WithAddress, WithRouteConditionsHTTPDownward, MarkTrafficAssigned, MarkIngressNotConfigured, WithStatusTraffic( v1.TrafficTarget{ RevisionName: "config-00001", Percent: ptr.Int64(100), LatestRevision: ptr.Bool(true), - }), MarkCertificateNotReady, MarkIngressNotConfigured, + }), MarkIngressNotConfigured, // The certificate is not ready. So we want to have HTTP URL. WithURL), }}, diff --git a/pkg/testing/v1/route.go b/pkg/testing/v1/route.go index c8969c331993..fe4c364d3b0f 100644 --- a/pkg/testing/v1/route.go +++ b/pkg/testing/v1/route.go @@ -165,6 +165,12 @@ func WithRouteConditionsAutoTLSDisabled(rt *v1.Route) { rt.Status.MarkAutoTLSNotEnabled() } +// WithRouteConditionsHTTPDownward calls MarkHTTPDownward( after initialized the Service's conditions. +func WithRouteConditionsHTTPDownward(rt *v1.Route) { + rt.Status.InitializeConditions() + rt.Status.MarkHTTPDownward(routenames.Certificate(rt)) +} + // MarkTrafficAssigned calls the method of the same name on .Status func MarkTrafficAssigned(r *v1.Route) { r.Status.MarkTrafficAssigned() From 8e05e2c08fc863ff4896858b26728ae0923f3c7b Mon Sep 17 00:00:00 2001 From: Kenjiro Nakayama Date: Wed, 22 Apr 2020 15:10:32 +0900 Subject: [PATCH 13/13] Fix downward to downgrade --- pkg/apis/serving/v1/route_lifecycle.go | 8 ++++---- pkg/apis/serving/v1/route_lifecycle_test.go | 4 ++-- pkg/apis/serving/v1alpha1/route_lifecycle.go | 8 ++++---- pkg/apis/serving/v1alpha1/route_lifecycle_test.go | 4 ++-- pkg/reconciler/route/route.go | 4 ++-- pkg/reconciler/route/table_test.go | 6 +++--- pkg/testing/v1/route.go | 6 +++--- 7 files changed, 20 insertions(+), 20 deletions(-) diff --git a/pkg/apis/serving/v1/route_lifecycle.go b/pkg/apis/serving/v1/route_lifecycle.go index 0d60a4ac6d6a..29f611a33dac 100644 --- a/pkg/apis/serving/v1/route_lifecycle.go +++ b/pkg/apis/serving/v1/route_lifecycle.go @@ -129,12 +129,12 @@ func (rs *RouteStatus) MarkAutoTLSNotEnabled() { "autoTLS is not enabled") } -// MarkHTTPDownward sets RouteConditionCertificateProvisioned to true when plain +// MarkHTTPDowngrade sets RouteConditionCertificateProvisioned to true when plain // HTTP is enabled even when Certificated is not ready. -func (rs *RouteStatus) MarkHTTPDownward(name string) { +func (rs *RouteStatus) MarkHTTPDowngrade(name string) { routeCondSet.Manage(rs).MarkTrueWithReason(RouteConditionCertificateProvisioned, - "HTTPDownward", - "Certificate %s is not ready downard HTTP.", name) + "HTTPDowngrade", + "Certificate %s is not ready downgrade HTTP.", name) } // PropagateIngressStatus update RouteConditionIngressReady condition diff --git a/pkg/apis/serving/v1/route_lifecycle_test.go b/pkg/apis/serving/v1/route_lifecycle_test.go index edb6c1c47083..ef2710a67102 100644 --- a/pkg/apis/serving/v1/route_lifecycle_test.go +++ b/pkg/apis/serving/v1/route_lifecycle_test.go @@ -382,10 +382,10 @@ func TestRouteAutoTLSNotEnabled(t *testing.T) { apistest.CheckConditionSucceeded(r, RouteConditionCertificateProvisioned, t) } -func TestRouteHTTPDownward(t *testing.T) { +func TestRouteHTTPDowngrade(t *testing.T) { r := &RouteStatus{} r.InitializeConditions() - r.MarkHTTPDownward("cert") + r.MarkHTTPDowngrade("cert") apistest.CheckConditionSucceeded(r, RouteConditionCertificateProvisioned, t) } diff --git a/pkg/apis/serving/v1alpha1/route_lifecycle.go b/pkg/apis/serving/v1alpha1/route_lifecycle.go index 7cf0effef9b2..523187d49dd4 100644 --- a/pkg/apis/serving/v1alpha1/route_lifecycle.go +++ b/pkg/apis/serving/v1alpha1/route_lifecycle.go @@ -142,12 +142,12 @@ func (rs *RouteStatus) MarkAutoTLSNotEnabled() { "autoTLS is not enabled") } -// MarkHTTPDownward sets RouteConditionCertificateProvisioned to true when plain +// MarkHTTPDowngrade sets RouteConditionCertificateProvisioned to true when plain // HTTP is enabled even when Certificated is not ready. -func (rs *RouteStatus) MarkHTTPDownward(name string) { +func (rs *RouteStatus) MarkHTTPDowngrade(name string) { routeCondSet.Manage(rs).MarkTrueWithReason(RouteConditionCertificateProvisioned, - "HTTPDownward", - "Certificate %s is not ready downard HTTP.", name) + "HTTPDowngrade", + "Certificate %s is not ready downgrade HTTP.", name) } // PropagateIngressStatus update RouteConditionIngressReady condition diff --git a/pkg/apis/serving/v1alpha1/route_lifecycle_test.go b/pkg/apis/serving/v1alpha1/route_lifecycle_test.go index 209c58f92f0e..b1685869efe6 100644 --- a/pkg/apis/serving/v1alpha1/route_lifecycle_test.go +++ b/pkg/apis/serving/v1alpha1/route_lifecycle_test.go @@ -386,10 +386,10 @@ func TestRouteAutoTLSNotEnabled(t *testing.T) { apistest.CheckConditionSucceeded(r, RouteConditionCertificateProvisioned, t) } -func TestRouteHTTPDownward(t *testing.T) { +func TestRouteHTTPDowngrade(t *testing.T) { r := &RouteStatus{} r.InitializeConditions() - r.MarkHTTPDownward("cert") + r.MarkHTTPDowngrade("cert") apistest.CheckConditionSucceeded(r, RouteConditionCertificateProvisioned, t) } diff --git a/pkg/reconciler/route/route.go b/pkg/reconciler/route/route.go index 16a46051e7e7..145677f1d096 100644 --- a/pkg/reconciler/route/route.go +++ b/pkg/reconciler/route/route.go @@ -249,7 +249,7 @@ func (c *Reconciler) tls(ctx context.Context, host string, r *v1.Route, traffic } else { acmeChallenges = append(acmeChallenges, cert.Status.HTTP01Challenges...) r.Status.MarkCertificateNotReady(cert.Name) - // When httpProtocol is enabled, downward http scheme. + // When httpProtocol is enabled, downgrade http scheme. if config.FromContext(ctx).Network.HTTPProtocol == network.HTTPEnabled { if dnsNames.Has(host) { r.Status.URL = &apis.URL{ @@ -258,7 +258,7 @@ func (c *Reconciler) tls(ctx context.Context, host string, r *v1.Route, traffic } } setTargetsScheme(&r.Status, dnsNames.List(), "http") - r.Status.MarkHTTPDownward(cert.Name) + r.Status.MarkHTTPDowngrade(cert.Name) } } } diff --git a/pkg/reconciler/route/table_test.go b/pkg/reconciler/route/table_test.go index 479b0ab36e1f..dd7e21152f39 100644 --- a/pkg/reconciler/route/table_test.go +++ b/pkg/reconciler/route/table_test.go @@ -1923,7 +1923,7 @@ func TestReconcile_EnableAutoTLS(t *testing.T) { Object: Route("default", "becomes-ready", WithConfigTarget("config"), WithRouteUID("12-34"), // Populated by reconciliation when all traffic has been assigned. - WithURL, WithAddress, WithInitRouteConditions, WithRouteConditionsHTTPDownward, + WithURL, WithAddress, WithInitRouteConditions, WithRouteConditionsHTTPDowngrade, MarkTrafficAssigned, MarkIngressNotConfigured, WithStatusTraffic( v1.TrafficTarget{ RevisionName: "config-00001", @@ -2166,7 +2166,7 @@ func TestReconcile_EnableAutoTLS(t *testing.T) { LatestRevision: ptr.Bool(true), }), // Which also means no HTTPS URL - WithURL, WithRouteConditionsHTTPDownward, + WithURL, WithRouteConditionsHTTPDowngrade, ), }}, Key: "default/becomes-ready", @@ -2278,7 +2278,7 @@ func TestReconcile_EnableAutoTLS(t *testing.T) { Object: Route("default", "becomes-ready", WithConfigTarget("config"), WithRouteUID("12-34"), // Populated by reconciliation when all traffic has been assigned. - WithAddress, WithRouteConditionsHTTPDownward, + WithAddress, WithRouteConditionsHTTPDowngrade, MarkTrafficAssigned, MarkIngressNotConfigured, WithStatusTraffic( v1.TrafficTarget{ RevisionName: "config-00001", diff --git a/pkg/testing/v1/route.go b/pkg/testing/v1/route.go index fe4c364d3b0f..c990d3d60ab3 100644 --- a/pkg/testing/v1/route.go +++ b/pkg/testing/v1/route.go @@ -165,10 +165,10 @@ func WithRouteConditionsAutoTLSDisabled(rt *v1.Route) { rt.Status.MarkAutoTLSNotEnabled() } -// WithRouteConditionsHTTPDownward calls MarkHTTPDownward( after initialized the Service's conditions. -func WithRouteConditionsHTTPDownward(rt *v1.Route) { +// WithRouteConditionsHTTPDowngrade calls MarkHTTPDowngrade after initialized the Service's conditions. +func WithRouteConditionsHTTPDowngrade(rt *v1.Route) { rt.Status.InitializeConditions() - rt.Status.MarkHTTPDownward(routenames.Certificate(rt)) + rt.Status.MarkHTTPDowngrade(routenames.Certificate(rt)) } // MarkTrafficAssigned calls the method of the same name on .Status