CASMHMS-6295: Initial set of changes

Dockerfile

@@ -26,7 +26,7 @@
 ARG LIBRDKAFKA_VER_MIN=1.1.0
 
 # Build base just has the packages installed we need.
-FROM artifactory.algol60.net/docker.io/library/golang:1.16-alpine AS build-base
+FROM artifactory.algol60.net/docker.io/library/golang:1.23-alpine AS build-base
 
 ARG LIBRDKAFKA_VER_MIN
 

Dockerfile.testing

@@ -26,7 +26,7 @@
 ARG LIBRDKAFKA_VER_MIN=1.1.0
 
 # Build base just has the packages installed we need.
-FROM artifactory.algol60.net/docker.io/library/golang:1.16-alpine AS build-base
+FROM artifactory.algol60.net/docker.io/library/golang:1.23-alpine AS build-base
 
 ARG LIBRDKAFKA_VER_MIN
 

cmd/hmcollector/hmcollector.go

@@ -25,6 +25,7 @@ package main
 import (
 	"context"
 	"fmt"
+	"io"
 	"net/http"
 	"os"
 	"os/signal"
@@ -206,6 +207,26 @@ func fillMap(m map[string]struct{}, values string) {
 
 }
 
+// While it is generally not a requirement to close request bodies in server
+// handlers, it is good practice.  If a body is only partially read, there can
+// be a resource leak.  Additionally, if the body is not read at all, the
+// server may not be able to reuse the connection.
+func DrainAndCloseRequestBody(req *http.Request) {
+       if req != nil && req.Body != nil {
+               _, _ = io.Copy(io.Discard, req.Body) // ok even if already drained
+               req.Body.Close()
+       }
+}
+
+// Response bodies on the other hand, should always be drained and closed,
+// else we leak resources
+func DrainAndCloseResponseBody(resp *http.Response) {
+       if resp != nil && resp.Body != nil {
+               _, _ = io.Copy(io.Discard, resp.Body) // ok even if already drained
+               resp.Body.Close()
+       }
+}
+
 func SetupLogging() {
 	logLevel := os.Getenv("LOG_LEVEL")
 	logLevel = strings.ToUpper(logLevel)

cmd/hmcollector/http.go

@@ -1,6 +1,6 @@
 // MIT License
 //
-// (C) Copyright [2020-2021] Hewlett Packard Enterprise Development LP
+// (C) Copyright [2020-2021,2024] Hewlett Packard Enterprise Development LP
 //
 // Permission is hereby granted, free of charge, to any person obtaining a
 // copy of this software and associated documentation files (the "Software"),
@@ -25,10 +25,11 @@ package main
 import (
 	"bytes"
 	"fmt"
-	"go.uber.org/zap"
 	"io/ioutil"
 	"net/http"
+
 	rf "github.com/Cray-HPE/hms-smd/pkg/redfish"
+	"go.uber.org/zap"
 )
 
 func doHTTPAction(endpoint *rf.RedfishEPDescription, method string,
@@ -59,10 +60,8 @@ func doHTTPAction(endpoint *rf.RedfishEPDescription, method string,
 
 	rfClientLock.RLock()
 	resp, doErr = rfClient.Do(request)
-	if resp != nil {
-		defer resp.Body.Close()
-	}
 	rfClientLock.RUnlock()
+	defer DrainAndCloseResponseBody(resp)
 	if doErr != nil {
 		endpointLogger.Error("Unable to do request!", 
 			zap.Error(doErr), zap.String("fullURL", fullURL))
@@ -76,11 +75,6 @@ func doHTTPAction(endpoint *rf.RedfishEPDescription, method string,
 	if resp.StatusCode == http.StatusUnauthorized || resp.StatusCode == http.StatusForbidden {
 		endpointLogger.Warn("Got Unauthorized response from endpoint, refreshing credentials...")
 
-		//Drain the unused response body.
-		if (resp.Body != nil) {
-			_,_ = ioutil.ReadAll(resp.Body)
-		}
-
 		// Keep track of the previous credentials to see if they change.
 		previousUsername := endpoint.User
 		previousPassword := endpoint.Password

cmd/hmcollector/rest.go

@@ -1,6 +1,6 @@
 // MIT License
 //
-// (C) Copyright [2020-2023] Hewlett Packard Enterprise Development LP
+// (C) Copyright [2020-2024] Hewlett Packard Enterprise Development LP
 //
 // Permission is hereby granted, free of charge, to any person obtaining a
 // copy of this software and associated documentation files (the "Software"),
@@ -116,6 +116,7 @@ func unmarshalEvents(bodyBytes []byte) (events hmcollector.Events, err error) {
 
 func parseRequest(w http.ResponseWriter, r *http.Request) {
 	bodyBytes, err := ioutil.ReadAll(r.Body)
+	DrainAndCloseRequestBody(r)
 	if err != nil {
 		logger.Error("Unable to ready body!", zap.Error(err))
 		return
@@ -237,13 +238,16 @@ func parseRequest(w http.ResponseWriter, r *http.Request) {
 // Kubernetes liveness probe - if this responds with anything other than success (code <400) it will cause the
 // pod to be restarted (eventually).
 func doLiveness(w http.ResponseWriter, r *http.Request) {
+	defer DrainAndCloseRequestBody(r)
 
 	w.WriteHeader(http.StatusNoContent)
 }
 
 // Kubernetes liveness probe - if this responds with anything other than success (code <400) multiple times in
 // a row it will cause the pod to be restarted.  Only fail this probe for issues that we expect a restart to fix.
 func doReadiness(w http.ResponseWriter, r *http.Request) {
+	defer DrainAndCloseRequestBody(r)
+
 	ready := true
 
 	// If the Kafka bus isn't good, then return not ready since any incoming data will be dropped.  A restart may not
@@ -281,6 +285,8 @@ type EndpointStatus struct {
 }
 
 func doHealth(w http.ResponseWriter, r *http.Request) {
+	defer DrainAndCloseRequestBody(r)
+
 	// Only allow 'GET' calls.
 	if r.Method != http.MethodGet {
 		w.Header().Set("Allow", "GET")

go.mod

@@ -1,6 +1,6 @@
 module github.com/Cray-HPE/hms-hmcollector
 
-go 1.16
+go 1.23
 
 require (
 	github.com/Cray-HPE/hms-base v1.15.0

internal/hmcollector/smd_api.go

@@ -27,8 +27,9 @@ import (
 	"fmt"
 	"io/ioutil"
 	"net/http"
-	"github.com/Cray-HPE/hms-smd/pkg/redfish"
+
 	"github.com/Cray-HPE/hms-certs/pkg/hms_certs"
+	rf "github.com/Cray-HPE/hms-smd/pkg/redfish"
 )
 
 func GetEndpointList(httpClient *hms_certs.HTTPClientPair, gatewayUrl string) ([]rf.RedfishEPDescription, error) {
@@ -39,6 +40,7 @@ func GetEndpointList(httpClient *hms_certs.HTTPClientPair, gatewayUrl string) ([
 		return nil,fmt.Errorf("Unable to create HTTP request: %v",qerr)
 	}
 	rsp,serr := httpClient.Do(request)
+	defer DrainAndCloseBody(rsp)
 	if (serr != nil) {
 		return nil, fmt.Errorf("Error in HTTP request: %v",serr)
 	}