Repository: skupperproject/skupper
Pull request #1583 (2024-08-09 14:21:07)

Fix various (potential) correctness issues
Nothing major, a few linter warnings that could hide bugs or indicate correctness issues. See individual commits for details.


Patch:
diff --git a/api/types/types.go b/api/types/types.go
index 36cc725c..e326aa66 100644
--- a/api/types/types.go
+++ b/api/types/types.go
@@ -320,9 +320,9 @@ type Platform string
 
 const (
 	PlatformKubernetes Platform = "kubernetes"
-	PlatformPodman              = "podman"
-	PlatformDocker              = "docker"
-	PlatformSystemd             = "systemd"
+	PlatformPodman     Platform = "podman"
+	PlatformDocker     Platform = "docker"
+	PlatformSystemd    Platform = "systemd"
 )
 
 func (p Platform) IsKubernetes() bool {
@@ -337,8 +337,8 @@ type ConsoleAuthMode string
 
 const (
 	ConsoleAuthModeOpenshift ConsoleAuthMode = "openshift"
-	ConsoleAuthModeInternal                  = "internal"
-	ConsoleAuthModeUnsecured                 = "unsecured"
+	ConsoleAuthModeInternal  ConsoleAuthMode = "internal"
+	ConsoleAuthModeUnsecured ConsoleAuthMode = "unsecured"
 )
 
 const (
@@ -351,8 +351,8 @@ type PrometheusAuthMode string
 
 const (
 	PrometheusAuthModeTls       PrometheusAuthMode = "tls"
-	PrometheusAuthModeBasic                        = "basic"
-	PrometheusAuthModeUnsecured                    = "unsecured"
+	PrometheusAuthModeBasic     PrometheusAuthMode = "basic"
+	PrometheusAuthModeUnsecured PrometheusAuthMode = "unsecured"
 )
 
 // Prometheus server constants (note: use console auth mode)
@@ -539,7 +539,7 @@ type ConnectorRole string
 
 const (
 	ConnectorRoleInterRouter ConnectorRole = "inter-router"
-	ConnectorRoleEdge                      = "edge"
+	ConnectorRoleEdge        ConnectorRole = "edge"
 )
 
 const (
@@ -839,7 +839,7 @@ func (a ByServiceInterfaceAddress) Len() int {
 }
 
 func (a ByServiceInterfaceAddress) Less(i, j int) bool {
-	return a[i].Address > a[i].Address
+	return a[i].Address < a[j].Address
 }
 
 func (a ByServiceInterfaceAddress) Swap(i, j int) {
diff --git a/internal/kube/client/client.go b/internal/kube/client/client.go
index 88a40ee8..ed279c38 100644
--- a/internal/kube/client/client.go
+++ b/internal/kube/client/client.go
@@ -94,6 +94,9 @@ func NewClient(namespace string, context string, kubeConfigPath string) (*KubeCl
 		return c, err
 	}
 	dc, err := discovery.NewDiscoveryClientForConfig(restconfig)
+	if err != nil {
+		return c, err
+	}
 	resources, err := dc.ServerResourcesForGroupVersion("route.openshift.io/v1")
 	if err == nil && len(resources.APIResources) > 0 {
 		c.Route, err = openshiftroute.NewForConfig(restconfig)
diff --git a/internal/non-kube/client/compat/container_mock.go b/internal/non-kube/client/compat/container_mock.go
index 6f1a7868..dade9337 100644
--- a/internal/non-kube/client/compat/container_mock.go
+++ b/internal/non-kube/client/compat/container_mock.go
@@ -100,7 +100,7 @@ func (r *RestClientMock) CleanupMockVolumeDir() error {
 
 func (r *RestClientMock) Submit(operation *runtime.ClientOperation) (interface{}, error) {
 	var res interface{}
-	var err error = nil
+	var err error
 
 	switch operation.ID {
 	case "SystemInfo":
diff --git a/pkg/flow/flow_mem_driver.go b/pkg/flow/flow_mem_driver.go
index 191d1907..f6d108f3 100644
--- a/pkg/flow/flow_mem_driver.go
+++ b/pkg/flow/flow_mem_driver.go
@@ -331,7 +331,7 @@ func (fc *FlowCollector) annotateFlowTrace(flow *FlowRecord) *string {
 }
 
 func (fc *FlowCollector) linkFlowPair(flow *FlowRecord) (*FlowPairRecord, bool) {
-	var sourceSiteId, destSiteId, sourceSiteName, destSiteName string = "", "", "sourceSite", "destSite"
+	var sourceSiteName, destSiteName string = "sourceSite", "destSite"
 	var sourceFlow, destFlow *FlowRecord = nil, nil
 	var ok bool
 
@@ -360,11 +360,11 @@ func (fc *FlowCollector) linkFlowPair(flow *FlowRecord) (*FlowPairRecord, bool)
 		return nil, false
 	}
 
-	sourceSiteId = fc.getRecordSiteId(*sourceFlow)
+	sourceSiteId := fc.getRecordSiteId(*sourceFlow)
 	if sourceSite, ok := fc.Sites[sourceSiteId]; ok {
 		sourceSiteName = *sourceSite.Name
 	}
-	destSiteId = fc.getRecordSiteId(*destFlow)
+	destSiteId := fc.getRecordSiteId(*destFlow)
 	if destSite, ok := fc.Sites[destSiteId]; ok {
 		destSiteName = *destSite.Name
 	}
diff --git a/pkg/kube/claims/redeem.go b/pkg/kube/claims/redeem.go
index 91e6d433..3a5913c4 100644
--- a/pkg/kube/claims/redeem.go
+++ b/pkg/kube/claims/redeem.go
@@ -32,6 +32,9 @@ func RedeemAccessToken(claim *skupperv1alpha1.AccessToken, site *skupperv1alpha1
 		Transport: transport,
 	}
 	request, err := http.NewRequest(http.MethodPost, claim.Spec.Url, bytes.NewReader([]byte(claim.Spec.Code)))
+	if err != nil {
+		return err
+	}
 	request.Header.Add("name", claim.Name)
 	request.Header.Add("subject", string(site.ObjectMeta.UID))
 	response, err := client.Do(request)
diff --git a/pkg/kube/contour.go b/pkg/kube/contour.go
index f5d57c09..4e735ff1 100644
--- a/pkg/kube/contour.go
+++ b/pkg/kube/contour.go
@@ -142,5 +142,5 @@ func UpdateContourProxyService(clients internalclient.Clients, namespace string,
 		return err
 	}
 	_, err = client.Resource(httpProxyResource).Namespace(namespace).Update(context.TODO(), obj, metav1.UpdateOptions{})
-	return nil
+	return err
 }
diff --git a/pkg/kube/ingress.go b/pkg/kube/ingress.go
index d1f4ee08..99654080 100644
--- a/pkg/kube/ingress.go
+++ b/pkg/kube/ingress.go
@@ -316,11 +316,17 @@ func updateIngressRuleServiceNameV1(client dynamic.Interface, namespace string,
 		return err
 	}
 	routes, err := readIngressRules(obj)
+	if err != nil {
+		return err
+	}
 	for index, route := range routes {
 		if strings.HasPrefix(route.Host, hostPrefix) {
 			route.ServiceName = serviceName
 			routes[index] = route
 			err = writeIngressRules(routes, obj)
+			if err != nil {
+				return err
+			}
 			_, err = client.Resource(ingressResource).Namespace(namespace).Update(context.TODO(), obj, metav1.UpdateOptions{})
 			if err != nil {
 				return err
@@ -396,7 +402,7 @@ func (r *IngressRoute) fromUntypedRule(u interface{}) error {
 	if len(paths) == 0 {
 		return fmt.Errorf("No paths in rule")
 	}
-	path, ok := paths[0].(map[string]interface{})
+	path := paths[0].(map[string]interface{})
 	serviceName, _, err := unstructured.NestedString(path, "backend", "service", "name")
 	if err != nil {
 		return err
diff --git a/pkg/non_kube/apis/site_state.go b/pkg/non_kube/apis/site_state.go
index ff473393..4b62874b 100644
--- a/pkg/non_kube/apis/site_state.go
+++ b/pkg/non_kube/apis/site_state.go
@@ -218,10 +218,10 @@ func (s *SiteState) linkMap(sslProfileBasePath string) site.LinkMap {
 func (s *SiteState) bindings() *site.Bindings {
 	b := site.NewBindings()
 	for name, connector := range s.Connectors {
-		_, _ = b.UpdateConnector(name, connector)
+		_ = b.UpdateConnector(name, connector)
 	}
 	for name, listener := range s.Listeners {
-		_, _ = b.UpdateListener(name, listener)
+		_ = b.UpdateListener(name, listener)
 	}
 	return b
 }
diff --git a/pkg/non_kube/common/claim_redeemer.go b/pkg/non_kube/common/claim_redeemer.go
index 2f600257..37826fbd 100644
--- a/pkg/non_kube/common/claim_redeemer.go
+++ b/pkg/non_kube/common/claim_redeemer.go
@@ -52,6 +52,9 @@ func redeemAccessToken(claim *skupperv1alpha1.AccessToken, siteState *apis.SiteS
 		Timeout:   10 * time.Second,
 	}
 	request, err := http.NewRequest(http.MethodPost, claim.Spec.Url, bytes.NewReader([]byte(claim.Spec.Code)))
+	if err != nil {
+		return err
+	}
 	request.Header.Add("name", claim.Name)
 	request.Header.Add("subject", string(siteState.Site.Name))
 	response, err := client.Do(request)
diff --git a/pkg/site/siteconfig.go b/pkg/site/siteconfig.go
index 92e6ff93..fa53e48e 100644
--- a/pkg/site/siteconfig.go
+++ b/pkg/site/siteconfig.go
@@ -128,7 +128,7 @@ func WriteSiteConfig(spec types.SiteConfigSpec, namespace string) (*corev1.Confi
 			SiteConfigClusterPermissionsKey:    "false",
 			SiteConfigRouterConsoleKey:         "false",
 			SiteConfigRouterLoggingKey:         "",
-			SiteConfigConsoleAuthenticationKey: types.ConsoleAuthModeInternal,
+			SiteConfigConsoleAuthenticationKey: string(types.ConsoleAuthModeInternal),
 			SiteConfigConsoleUserKey:           "",
 			SiteConfigConsolePasswordKey:       "",
 			SiteConfigIngressKey:               types.IngressLoadBalancerString,
@@ -551,7 +551,7 @@ func ReadSiteConfigFrom(siteConfig *metav1.ObjectMeta, typeMeta *metav1.TypeMeta
 	if authMode, ok := data[SiteConfigConsoleAuthenticationKey]; ok {
 		result.Spec.AuthMode = authMode
 	} else {
-		result.Spec.AuthMode = types.ConsoleAuthModeInternal
+		result.Spec.AuthMode = string(types.ConsoleAuthModeInternal)
 	}
 	if user, ok := data[SiteConfigConsoleUserKey]; ok {
 		result.Spec.User = user
diff --git a/pkg/utils/formatter/status.go b/pkg/utils/formatter/status.go
index 5264b33a..dd49e568 100644
--- a/pkg/utils/formatter/status.go
+++ b/pkg/utils/formatter/status.go
@@ -2,11 +2,12 @@ package formatter
 
 import (
 	"fmt"
-	"github.com/skupperproject/skupper/pkg/network"
 	"os"
 	"strconv"
 	"strings"
 	"text/tabwriter"
+
+	"github.com/skupperproject/skupper/pkg/network"
 )
 
 type PlatformSupport struct {
@@ -174,8 +175,8 @@ func PrintNetworkStatus(currentSite string, currentNetworkStatus *network.Networ
 
 					if len(mapSiteLink) > 0 {
 						siteLinks := siteLevel.NewChild("Linked sites:")
-						for key, value := range mapSiteLink {
-							siteLinks.NewChildWithDetail(fmt.Sprintln(key), map[string]string{"direction": value.Direction})
+						for key := range mapSiteLink {
+							siteLinks.NewChild(fmt.Sprintln(key))
 						}
 					}
 
@@ -197,7 +198,7 @@ func PrintNetworkStatus(currentSite string, currentNetworkStatus *network.Networ
 									links := routerLevel.NewChild("Links:")
 									for _, link := range printableLinks {
 										linkItem := fmt.Sprintf("name:  %s\n", link.Name)
-										detailsLink := map[string]string{"direction": link.Direction}
+										detailsLink := map[string]string{}
 										if link.LinkCost > 0 {
 											detailsLink["cost"] = strconv.FormatUint(link.LinkCost, 10)
 										}


Test patch:
diff --git a/pkg/non_kube/apis/environment_test.go b/pkg/non_kube/apis/environment_test.go
index 89bcb2b1..29583107 100644
--- a/pkg/non_kube/apis/environment_test.go
+++ b/pkg/non_kube/apis/environment_test.go
@@ -242,9 +242,8 @@ func TestPlatformInfo_Load(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			f, err := os.CreateTemp(os.TempDir(), "platform-*.yaml")
-			tempFile := f.Name()
 			assert.Assert(t, err, "error creating temporary platform config file")
-			tempFile = f.Name()
+			tempFile := f.Name()
 			defer func(name string) {
 				_ = os.Remove(name)
 			}(tempFile)