Skip to content

Commit

Permalink
Fix appProtocol issue for non-TLS SolrClouds
Browse files Browse the repository at this point in the history
  • Loading branch information
@HoustonPutman
HoustonPutman committed Oct 5, 2023
1 parent 1887fdd commit 954942e
Show file tree
Hide file tree
Showing 6 changed files with 38 additions and 27 deletions.
6 changes: 2 additions & 4 deletions controllers/solrcloud_controller_externaldns_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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())
Expand Down
12 changes: 4 additions & 8 deletions controllers/solrcloud_controller_ingress_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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())
Expand Down Expand Up @@ -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.")
Expand All @@ -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")
Expand Down
6 changes: 2 additions & 4 deletions controllers/solrcloud_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -298,17 +298,15 @@ 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)
expectedHeadlessServiceLabels := util.MergeLabelsOrAnnotations(solrCloud.SharedLabelsWith(solrCloud.Labels), map[string]string{"service-type": "headless"})
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())
Expand Down
26 changes: 18 additions & 8 deletions controllers/solrcloud_controller_tls_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand All @@ -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()
Expand All @@ -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")
Expand All @@ -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")
}
}
}

Expand Down
8 changes: 5 additions & 3 deletions controllers/util/solr_util.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down
7 changes: 7 additions & 0 deletions helm/solr-operator/Chart.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 954942e

Please sign in to comment.