Reduce string constant duplication

Newer versions of goconst detect more instances of string duplication, in particular ".svc." in Lighthouse. Address that by providing a framework function to construct a service's DNS name based on its components. A few string constants aren't actually shared, remove them to improve code clarity. Signed-off-by: Stephen Kitt <[email protected]>
submariner-io · Oct 24, 2023 · e23ee7d · e23ee7d
1 parent d4193c3
commit e23ee7d
Show file tree

Hide file tree

Showing 4 changed files with 20 additions and 23 deletions.
diff --git a/test/e2e/discovery/headless_services.go b/test/e2e/discovery/headless_services.go
@@ -30,11 +30,6 @@ import (
 	corev1 "k8s.io/api/core/v1"
 )
 
-const (
-	httpPortName = "http"
-	opAre        = "are"
-)
-
 var _ = Describe("Test Headless Service Discovery Across Clusters", Label(TestLabel), func() {
 	f := lhframework.NewFramework("discovery")
 
@@ -331,10 +326,10 @@ func verifyHeadlessSRVRecordsWithDig(f *framework.Framework, cluster framework.C
 		for j := range ports {
 			port := &ports[j]
 			cmd, domainName := createSRVQuery(f, port, service, domains[i], clusterName, withPort, withcluster)
-			op := opAre
+			op := "are"
 
 			if !shouldContain {
-				op += not
+				op += " not"
 			}
 
 			By(fmt.Sprintf("Executing %q to verify hostNames %v for service %q %q discoverable",
@@ -384,7 +379,7 @@ func createSRVQuery(f *framework.Framework, port *corev1.ServicePort, service *c
 ) (cmd []string, domainName string) {
 	cmd = []string{"dig", "+short", "SRV"}
 
-	domainName = service.Name + "." + f.Namespace + ".svc." + domain
+	domainName = lhframework.BuildServiceDNSName("", service.Name, f.Namespace, domain)
 	clusterDNSName := domainName
 
 	if withcluster {

diff --git a/test/e2e/discovery/service_discovery.go b/test/e2e/discovery/service_discovery.go
@@ -34,7 +34,6 @@ import (
 )
 
 const (
-	not       = " not"
 	TestLabel = "service-discovery"
 )
 
@@ -557,7 +556,7 @@ func RunServicesClusterAvailabilityMultiClusterTest(f *lhframework.Framework) {
 		checkedDomains, "", false)
 }
 
-//nolint:gocognit,unparam // This really isn't that complex and would be awkward to refactor.
+//nolint:unparam // Keep srcCluster as a parameter for consistency and possible future extensions
 func verifySRVWithDig(f *framework.Framework, srcCluster framework.ClusterIndex, service *corev1.Service, targetPod *corev1.PodList,
 	domains []string, clusterName string, withPort, shouldContain bool,
 ) {
@@ -566,11 +565,7 @@ func verifySRVWithDig(f *framework.Framework, srcCluster framework.ClusterIndex,
 		for _, port := range ports {
 			cmd := []string{"dig", "+short", "SRV"}
 
-			clusterDNSName := service.Name + "." + f.Namespace + ".svc." + domains[i]
-
-			if clusterName != "" {
-				clusterDNSName = clusterName + "." + clusterDNSName
-			}
+			clusterDNSName := lhframework.BuildServiceDNSName(clusterName, service.Name, f.Namespace, domains[i])
 
 			portName := clusterDNSName
 
@@ -582,7 +577,7 @@ func verifySRVWithDig(f *framework.Framework, srcCluster framework.ClusterIndex,
 
 			op := "is"
 			if !shouldContain {
-				op += not
+				op += " not"
 			}
 
 			By(fmt.Sprintf("Executing %q to verify SRV record for service %q %q discoverable", strings.Join(cmd, " "),
@@ -632,7 +627,7 @@ func verifyRoundRobinWithDig(f *framework.Framework, srcCluster framework.Cluste
 	cmd := []string{"dig", "+short"}
 
 	for i := range domains {
-		cmd = append(cmd, serviceName+"."+f.Namespace+".svc."+domains[i])
+		cmd = append(cmd, lhframework.BuildServiceDNSName("", serviceName, f.Namespace, domains[i]))
 	}
 
 	serviceIPMap := make(map[string]int)

diff --git a/test/e2e/discovery/statefulsets.go b/test/e2e/discovery/statefulsets.go
@@ -30,6 +30,8 @@ import (
 	discovery "k8s.io/api/discovery/v1"
 )
 
+const httpPortName = "http"
+
 var _ = Describe("Test Stateful Sets Discovery Across Clusters", Label(TestLabel), func() {
 	f := lhframework.NewFramework("discovery")
 

diff --git a/test/e2e/framework/framework.go b/test/e2e/framework/framework.go
@@ -629,18 +629,23 @@ func (f *Framework) VerifyServiceIPWithDig(srcCluster, targetCluster framework.C
 	f.VerifyIPWithDig(srcCluster, service, targetPod, domains, clusterName, serviceIP, shouldContain)
 }
 
+func BuildServiceDNSName(clusterName, serviceName, namespace, tld string) string {
+	name := ""
+
+	if clusterName != "" {
+		name = clusterName + "."
+	}
+
+	return name + serviceName + "." + namespace + ".svc." + tld
+}
+
 func (f *Framework) VerifyIPWithDig(srcCluster framework.ClusterIndex, service *v1.Service, targetPod *v1.PodList,
 	domains []string, clusterName, serviceIP string, shouldContain bool,
 ) {
 	cmd := []string{"dig", "+short"}
 
-	var clusterDNSName string
-	if clusterName != "" {
-		clusterDNSName = clusterName + "."
-	}
-
 	for i := range domains {
-		cmd = append(cmd, clusterDNSName+service.Name+"."+f.Namespace+".svc."+domains[i])
+		cmd = append(cmd, BuildServiceDNSName(clusterName, service.Name, f.Namespace, domains[i]))
 	}
 
 	op := "is"