From 954942e9b9776977b4297218ce5feb4b1dc62088 Mon Sep 17 00:00:00 2001 From: Houston Putman Date: Thu, 5 Oct 2023 17:38:59 -0400 Subject: [PATCH] Fix appProtocol issue for non-TLS SolrClouds --- .../solrcloud_controller_externaldns_test.go | 6 ++--- .../solrcloud_controller_ingress_test.go | 12 +++------ controllers/solrcloud_controller_test.go | 6 ++--- controllers/solrcloud_controller_tls_test.go | 26 +++++++++++++------ controllers/util/solr_util.go | 8 +++--- helm/solr-operator/Chart.yaml | 7 +++++ 6 files changed, 38 insertions(+), 27 deletions(-) diff --git a/controllers/solrcloud_controller_externaldns_test.go b/controllers/solrcloud_controller_externaldns_test.go index 92d7e7ea..0cd0bcc5 100644 --- a/controllers/solrcloud_controller_externaldns_test.go +++ b/controllers/solrcloud_controller_externaldns_test.go @@ -117,8 +117,7 @@ var _ = FDescribe("SolrCloud controller - External DNS", func() { Expect(commonService.Spec.Ports[0].Port).To(Equal(int32(4000)), "Wrong port on common Service") Expect(commonService.Spec.Ports[0].TargetPort.StrVal).To(Equal("solr-client"), "Wrong podPort name on common Service") Expect(commonService.Spec.Ports[0].Protocol).To(Equal(corev1.ProtocolTCP), "Wrong protocol on common Service") - Expect(commonService.Spec.Ports[0].AppProtocol).ToNot(BeNil(), "AppProtocol on common Service should not be nil") - Expect(*commonService.Spec.Ports[0].AppProtocol).To(Equal("http"), "Wrong appProtocol on common Service") + Expect(commonService.Spec.Ports[0].AppProtocol).To(BeNil(), "AppProtocol on common Service should be nil when not running with TLS") By("testing the Solr Headless Service") headlessService := expectService(ctx, solrCloud, solrCloud.HeadlessServiceName(), statefulSet.Spec.Selector.MatchLabels, true) @@ -130,8 +129,7 @@ var _ = FDescribe("SolrCloud controller - External DNS", func() { Expect(headlessService.Spec.Ports[0].Port).To(Equal(int32(3000)), "Wrong port on headless Service") Expect(headlessService.Spec.Ports[0].TargetPort.StrVal).To(Equal("solr-client"), "Wrong podPort name on headless Service") Expect(headlessService.Spec.Ports[0].Protocol).To(Equal(corev1.ProtocolTCP), "Wrong protocol on headless Service") - Expect(headlessService.Spec.Ports[0].AppProtocol).ToNot(BeNil(), "AppProtocol on headless Service should not be nil") - Expect(*headlessService.Spec.Ports[0].AppProtocol).To(Equal("http"), "Wrong appProtocol on headless Service") + Expect(headlessService.Spec.Ports[0].AppProtocol).To(BeNil(), "AppProtocol on headless Service should be nil when not running with TLS") By("making sure no individual Solr Node Services exist") expectNoServices(ctx, solrCloud, "Node service shouldn't exist, but it does.", solrCloud.GetAllSolrPodNames()) diff --git a/controllers/solrcloud_controller_ingress_test.go b/controllers/solrcloud_controller_ingress_test.go index d9ad9bf0..8a616a9a 100644 --- a/controllers/solrcloud_controller_ingress_test.go +++ b/controllers/solrcloud_controller_ingress_test.go @@ -231,8 +231,7 @@ var _ = FDescribe("SolrCloud controller - Ingress", func() { Expect(commonService.Spec.Ports[0].Port).To(Equal(int32(4000)), "Wrong port on common Service") Expect(commonService.Spec.Ports[0].TargetPort.StrVal).To(Equal("solr-client"), "Wrong podPort name on common Service") Expect(commonService.Spec.Ports[0].Protocol).To(Equal(corev1.ProtocolTCP), "Wrong protocol on common Service") - Expect(commonService.Spec.Ports[0].AppProtocol).ToNot(BeNil(), "AppProtocol on common Service should not be nil") - Expect(*commonService.Spec.Ports[0].AppProtocol).To(Equal("http"), "Wrong appProtocol on common Service") + Expect(commonService.Spec.Ports[0].AppProtocol).To(BeNil(), "AppProtocol on common Service should be nil when not running with TLS") By("testing the Solr Headless Service") headlessService := expectService(ctx, solrCloud, solrCloud.HeadlessServiceName(), statefulSet.Spec.Selector.MatchLabels, true) @@ -241,8 +240,7 @@ var _ = FDescribe("SolrCloud controller - Ingress", func() { Expect(headlessService.Spec.Ports[0].Port).To(Equal(int32(3000)), "Wrong port on headless Service") Expect(headlessService.Spec.Ports[0].TargetPort.StrVal).To(Equal("solr-client"), "Wrong podPort name on headless Service") Expect(headlessService.Spec.Ports[0].Protocol).To(Equal(corev1.ProtocolTCP), "Wrong protocol on headless Service") - Expect(headlessService.Spec.Ports[0].AppProtocol).ToNot(BeNil(), "AppProtocol on headless Service should not be nil") - Expect(*headlessService.Spec.Ports[0].AppProtocol).To(Equal("http"), "Wrong appProtocol on headless Service") + Expect(headlessService.Spec.Ports[0].AppProtocol).To(BeNil(), "AppProtocol on headless Service should be nil when not running with TLS") By("making sure no individual Solr Node Services exist") expectNoServices(ctx, solrCloud, "Node service shouldn't exist, but it does.", solrCloud.GetAllSolrPodNames()) @@ -311,8 +309,7 @@ var _ = FDescribe("SolrCloud controller - Ingress", func() { Expect(commonService.Spec.Ports[0].Port).To(Equal(int32(4000)), "Wrong port on common Service") Expect(commonService.Spec.Ports[0].TargetPort.StrVal).To(Equal("solr-client"), "Wrong podPort name on common Service") Expect(commonService.Spec.Ports[0].Protocol).To(Equal(corev1.ProtocolTCP), "Wrong protocol on common Service") - Expect(commonService.Spec.Ports[0].AppProtocol).ToNot(BeNil(), "AppProtocol on common Service should not be nil") - Expect(*commonService.Spec.Ports[0].AppProtocol).To(Equal("http"), "Wrong appProtocol on common Service") + Expect(commonService.Spec.Ports[0].AppProtocol).To(BeNil(), "AppProtocol on common Service should be nil when not running with TLS") By("ensuring the Solr Headless Service does not exist") expectNoService(ctx, solrCloud, solrCloud.HeadlessServiceName(), "Headless service shouldn't exist, but it does.") @@ -329,8 +326,7 @@ var _ = FDescribe("SolrCloud controller - Ingress", func() { Expect(service.Spec.Ports[0].Port).To(Equal(int32(100)), "Wrong port on node Service") Expect(service.Spec.Ports[0].TargetPort.StrVal).To(Equal("solr-client"), "Wrong podPort name on node Service") Expect(service.Spec.Ports[0].Protocol).To(Equal(corev1.ProtocolTCP), "Wrong protocol on node Service") - Expect(service.Spec.Ports[0].AppProtocol).ToNot(BeNil(), "AppProtocol on node Service should not be nil") - Expect(*service.Spec.Ports[0].AppProtocol).To(Equal("http"), "Wrong appProtocol on node Service") + Expect(service.Spec.Ports[0].AppProtocol).To(BeNil(), "AppProtocol on node Service should be nil when not running with TLS") } By("making sure Ingress was created correctly") diff --git a/controllers/solrcloud_controller_test.go b/controllers/solrcloud_controller_test.go index 2c568585..8d81397f 100644 --- a/controllers/solrcloud_controller_test.go +++ b/controllers/solrcloud_controller_test.go @@ -298,8 +298,7 @@ var _ = FDescribe("SolrCloud controller - General", func() { Expect(commonService.Labels).To(Equal(util.MergeLabelsOrAnnotations(expectedCommonServiceLabels, testCommonServiceLabels)), "Incorrect common service labels") Expect(commonService.Annotations).To(Equal(testCommonServiceAnnotations), "Incorrect common service annotations") Expect(commonService.Spec.Ports[0].Protocol).To(Equal(corev1.ProtocolTCP), "Wrong protocol on common Service") - Expect(commonService.Spec.Ports[0].AppProtocol).ToNot(BeNil(), "AppProtocol on common Service should not be nil") - Expect(*commonService.Spec.Ports[0].AppProtocol).To(Equal("http"), "Wrong appProtocol on common Service") + Expect(commonService.Spec.Ports[0].AppProtocol).To(BeNil(), "AppProtocol on common Service should be nil when not running with TLS") By("testing the Solr Headless Service") headlessService := expectService(ctx, solrCloud, solrCloud.HeadlessServiceName(), statefulSet.Spec.Selector.MatchLabels, true) @@ -307,8 +306,7 @@ var _ = FDescribe("SolrCloud controller - General", func() { Expect(headlessService.Labels).To(Equal(util.MergeLabelsOrAnnotations(expectedHeadlessServiceLabels, testHeadlessServiceLabels)), "Incorrect headless service labels") Expect(headlessService.Annotations).To(Equal(testHeadlessServiceAnnotations), "Incorrect headless service annotations") Expect(headlessService.Spec.Ports[0].Protocol).To(Equal(corev1.ProtocolTCP), "Wrong protocol on headless Service") - Expect(headlessService.Spec.Ports[0].AppProtocol).ToNot(BeNil(), "AppProtocol on headless Service should not be nil") - Expect(*headlessService.Spec.Ports[0].AppProtocol).To(Equal("http"), "Wrong appProtocol on headless Service") + Expect(headlessService.Spec.Ports[0].AppProtocol).To(BeNil(), "AppProtocol on headless Service should be nil when not running with TLS") By("testing the PodDisruptionBudget") expectNoPodDisruptionBudget(ctx, solrCloud, solrCloud.StatefulSetName()) diff --git a/controllers/solrcloud_controller_tls_test.go b/controllers/solrcloud_controller_tls_test.go index c24013f0..6dff3d70 100644 --- a/controllers/solrcloud_controller_tls_test.go +++ b/controllers/solrcloud_controller_tls_test.go @@ -946,11 +946,9 @@ func expectZkSetupInitContainerForTLSWithGomega(g Gomega, solrCloud *solrv1beta1 } func expectTLSService(ctx context.Context, solrCloud *solrv1beta1.SolrCloud, selectorLables map[string]string, podsHaveTLSEnabled bool) { - appProtocol := "http" podPort := 8983 servicePort := 80 if podsHaveTLSEnabled { - appProtocol = "https" servicePort = 443 } By("testing the Solr Common Service") @@ -959,8 +957,12 @@ func expectTLSService(ctx context.Context, solrCloud *solrv1beta1.SolrCloud, sel Expect(commonService.Spec.Ports[0].Port).To(Equal(int32(servicePort)), "Wrong port on common Service") Expect(commonService.Spec.Ports[0].TargetPort.StrVal).To(Equal("solr-client"), "Wrong podPort name on common Service") Expect(commonService.Spec.Ports[0].Protocol).To(Equal(corev1.ProtocolTCP), "Wrong protocol on common Service") - Expect(commonService.Spec.Ports[0].AppProtocol).ToNot(BeNil(), "AppProtocol on common Service should not be nil") - Expect(*commonService.Spec.Ports[0].AppProtocol).To(Equal(appProtocol), "Wrong appProtocol on common Service") + if podsHaveTLSEnabled { + Expect(commonService.Spec.Ports[0].AppProtocol).ToNot(BeNil(), "AppProtocol on common Service should not be nil") + Expect(*commonService.Spec.Ports[0].AppProtocol).To(Equal("https"), "Wrong appProtocol on common Service") + } else { + Expect(commonService.Spec.Ports[0].AppProtocol).To(BeNil(), "AppProtocol on common Service should be nil when not running with TLS") + } if solrCloud.Spec.SolrAddressability.External.UsesIndividualNodeServices() { nodeNames := solrCloud.GetAllSolrPodNames() @@ -971,8 +973,12 @@ func expectTLSService(ctx context.Context, solrCloud *solrv1beta1.SolrCloud, sel Expect(service.Spec.Ports[0].Port).To(Equal(int32(servicePort)), "Wrong port on node Service") Expect(service.Spec.Ports[0].TargetPort.StrVal).To(Equal("solr-client"), "Wrong podPort name on node Service") Expect(service.Spec.Ports[0].Protocol).To(Equal(corev1.ProtocolTCP), "Wrong protocol on node Service") - Expect(service.Spec.Ports[0].AppProtocol).ToNot(BeNil(), "AppProtocol on node Service should not be nil") - Expect(*service.Spec.Ports[0].AppProtocol).To(Equal(appProtocol), "Wrong appProtocol on node Service") + if podsHaveTLSEnabled { + Expect(service.Spec.Ports[0].AppProtocol).ToNot(BeNil(), "AppProtocol on node Service should not be nil") + Expect(*service.Spec.Ports[0].AppProtocol).To(Equal("https"), "Wrong appProtocol on node Service") + } else { + Expect(service.Spec.Ports[0].AppProtocol).To(BeNil(), "AppProtocol on node Service should be nil when not running with TLS") + } } } else { By("testing the Solr Headless Service") @@ -981,8 +987,12 @@ func expectTLSService(ctx context.Context, solrCloud *solrv1beta1.SolrCloud, sel Expect(headlessService.Spec.Ports[0].Port).To(Equal(int32(podPort)), "Wrong port on headless Service") Expect(headlessService.Spec.Ports[0].TargetPort.StrVal).To(Equal("solr-client"), "Wrong podPort name on headless Service") Expect(headlessService.Spec.Ports[0].Protocol).To(Equal(corev1.ProtocolTCP), "Wrong protocol on headless Service") - Expect(headlessService.Spec.Ports[0].AppProtocol).ToNot(BeNil(), "AppProtocol on headless Service should not be nil") - Expect(*headlessService.Spec.Ports[0].AppProtocol).To(Equal(appProtocol), "Wrong appProtocol on headless Service") + if podsHaveTLSEnabled { + Expect(headlessService.Spec.Ports[0].AppProtocol).ToNot(BeNil(), "AppProtocol on headless Service should not be nil") + Expect(*headlessService.Spec.Ports[0].AppProtocol).To(Equal("https"), "Wrong appProtocol on headless Service") + } else { + Expect(headlessService.Spec.Ports[0].AppProtocol).To(BeNil(), "AppProtocol on headless Service should be nil when not running with TLS") + } } } diff --git a/controllers/util/solr_util.go b/controllers/util/solr_util.go index c6ede52d..0c7f0980 100644 --- a/controllers/util/solr_util.go +++ b/controllers/util/solr_util.go @@ -26,6 +26,7 @@ import ( "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/utils/pointer" "sort" "strconv" "strings" @@ -880,11 +881,12 @@ func GenerateAdditionalLibXMLPart(solrModules []string, additionalLibs []string) } func getAppProtocol(solrCloud *solr.SolrCloud) *string { - appProtocol := "http" + // Only use https, because for non-tls we need to support both http & http2 for Solr if solrCloud.Spec.SolrTLS != nil { - appProtocol = "https" + return pointer.String("https") + } else { + return nil } - return &appProtocol } // GenerateCommonService returns a new corev1.Service pointer generated for the entire SolrCloud instance diff --git a/helm/solr-operator/Chart.yaml b/helm/solr-operator/Chart.yaml index 9524b43b..849b501a 100644 --- a/helm/solr-operator/Chart.yaml +++ b/helm/solr-operator/Chart.yaml @@ -128,6 +128,13 @@ annotations: url: https://github.com/apache/solr-operator/issues/603 - name: Github PR url: https://github.com/apache/solr-operator/pull/608 + - kind: changed + description: Remove Service AppProtocol for non-TLS SolrClouds, in order to support http and http2 + links: + - name: Github Issue + url: https://github.com/apache/solr-operator/issues/640 + - name: Github PR + url: https://github.com/apache/solr-operator/pull/641 artifacthub.io/images: | - name: solr-operator image: apache/solr-operator:v0.8.0-prerelease