From 0f8e7b38ad0f8f544b238d9750afe1042647cda0 Mon Sep 17 00:00:00 2001 From: ssttehrani Date: Fri, 8 Dec 2023 20:04:19 +0330 Subject: [PATCH] refactor: enhance HttpVersion translation --- .../route-to-contour-httpproxy/Chart.yaml | 4 +- internal/controller/controller_test.go | 44 +++++++++++++++++++ internal/controller/route/handler.go | 8 +++- pkg/consts/consts.go | 1 + pkg/utils/meta.go | 10 +++++ 5 files changed, 64 insertions(+), 3 deletions(-) diff --git a/deploy/charts/route-to-contour-httpproxy/Chart.yaml b/deploy/charts/route-to-contour-httpproxy/Chart.yaml index 42643a6..c08f3b8 100644 --- a/deploy/charts/route-to-contour-httpproxy/Chart.yaml +++ b/deploy/charts/route-to-contour-httpproxy/Chart.yaml @@ -13,9 +13,9 @@ type: application # This is the chart version. This version number should be incremented each time you make changes # to the chart and its templates, including the app version. # Versions are expected to follow Semantic Versioning (https://semver.org/) -version: 0.1.5 +version: 0.1.6 # This is the version number of the application being deployed. This version number should be # incremented each time you make changes to the application. Versions are not expected to # follow Semantic Versioning. They should reflect the version the application is using. # It is recommended to use it with quotes. -appVersion: "1.3.6" +appVersion: "1.3.7" diff --git a/internal/controller/controller_test.go b/internal/controller/controller_test.go index 2b764f0..750364a 100644 --- a/internal/controller/controller_test.go +++ b/internal/controller/controller_test.go @@ -685,5 +685,49 @@ var _ = Describe("Testing Route to HTTPProxy Controller", func() { cleanUpRoute(route) }) + + It("Should set http versions to [http/1.1] for routes that do not use tls", func() { + route := getSampleRoute() + route.Spec.TLS = nil + + Expect(k8sClient.Create(context.Background(), route)).To(Succeed()) + + admitRoute(route) + + Eventually(func(g Gomega) { + httpProxyList := contourv1.HTTPProxyList{} + g.Expect(k8sClient.List(context.Background(), &httpProxyList, client.InNamespace(DefaultNamespace))).To(Succeed()) + g.Expect(len(httpProxyList.Items)).To(Equal(1)) + g.Expect(len(httpProxyList.Items[0].Spec.HttpVersions)).To(Equal(1)) + g.Expect(httpProxyList.Items[0].Spec.HttpVersions[0]).To(Equal(contourv1.HttpVersion("http/1.1"))) + }).Should(Succeed()) + + cleanUpRoute(route) + }) + + It("Should set http versions to [http/1.1] for routes that enforce http/1.1", func() { + route := getSampleRoute() + route.Annotations = map[string]string{ + consts.AnnotationKeyHttp1Enforced: "", + } + // Routes labeled as 'inter-dc' will use h2 in addition to http/1.1, independent of the certificate wildcard status. + route.Labels[consts.RouteShardLabel] = consts.IngressClassInterDc + route.Spec.TLS = &routev1.TLSConfig{ + Termination: routev1.TLSTerminationEdge, + } + Expect(k8sClient.Create(context.Background(), route)).To(Succeed()) + + admitRoute(route) + + Eventually(func(g Gomega) { + httpProxyList := contourv1.HTTPProxyList{} + g.Expect(k8sClient.List(context.Background(), &httpProxyList, client.InNamespace(DefaultNamespace))).To(Succeed()) + g.Expect(len(httpProxyList.Items)).To(Equal(1)) + g.Expect(len(httpProxyList.Items[0].Spec.HttpVersions)).To(Equal(1)) + g.Expect(httpProxyList.Items[0].Spec.HttpVersions[0]).To(Equal(contourv1.HttpVersion("http/1.1"))) + }).Should(Succeed()) + + cleanUpRoute(route) + }) }) }) diff --git a/internal/controller/route/handler.go b/internal/controller/route/handler.go index 073af3f..bdfab6a 100644 --- a/internal/controller/route/handler.go +++ b/internal/controller/route/handler.go @@ -292,7 +292,7 @@ func (r *Reconciler) assembleHttpproxy(ctx context.Context, owner *routev1.Route httpproxy.Spec.IngressClassName = utils.GetIngressClass(owner) // Enable h2 and http/1.1 by default. - // Later we disable h2 for a specific set of routes + // Later we disable h2 for a specific set of routes. httpproxy.Spec.HttpVersions = []contourv1.HttpVersion{"h2", "http/1.1"} if owner.Spec.TLS != nil { @@ -342,6 +342,12 @@ func (r *Reconciler) assembleHttpproxy(ctx context.Context, owner *routev1.Route default: return nil, fmt.Errorf("invalid termination mode specified on route") } + } else { + httpproxy.Spec.HttpVersions = []contourv1.HttpVersion{"http/1.1"} + } + + if utils.IsHttp1Enforced(r.route) { + httpproxy.Spec.HttpVersions = []contourv1.HttpVersion{"http/1.1"} } // use `tcpproxy` for passthrough mode and `routes` for other termination modes diff --git a/pkg/consts/consts.go b/pkg/consts/consts.go index 01b82de..039ac81 100644 --- a/pkg/consts/consts.go +++ b/pkg/consts/consts.go @@ -18,6 +18,7 @@ const ( AnnotationKeyPrefix = "snappcloud.io/" AnnotationKeyReconciliationPaused = AnnotationKeyPrefix + "paused" + AnnotationKeyHttp1Enforced = AnnotationKeyPrefix + "force-h1" TLSSecretNS = "openshift-ingress" TLSSecretName = "letsencrypt" diff --git a/pkg/utils/meta.go b/pkg/utils/meta.go index 96473bd..d0ec45a 100644 --- a/pkg/utils/meta.go +++ b/pkg/utils/meta.go @@ -20,6 +20,16 @@ func IsDeleted(obj metav1.Object) bool { return !obj.GetDeletionTimestamp().IsZero() } +// IsHttp1Enforced returns true if the object has the AnnotationKeyHttp1Enforced +func IsHttp1Enforced(obj metav1.Object) bool { + annotations := obj.GetAnnotations() + if annotations == nil { + return false + } + _, ok := annotations[consts.AnnotationKeyHttp1Enforced] + return ok +} + // IsPaused returns true if the object has the AnnotationKeyReconciliationPaused func IsPaused(obj metav1.Object) bool { annotations := obj.GetAnnotations()