Cleaning up a little of the implementation

Author: davidcollom

pkg/controller/kube_controller.go

@@ -4,7 +4,9 @@ import (
 	"context"
 	"fmt"
 	"io"
+	"net/http"
 	"net/url"
+	"slices"
 	"strings"
 	"time"
 
@@ -22,6 +24,7 @@ const channelURLSuffix = "https://dl.k8s.io/release/"
 
 type ClusterVersionScheduler struct {
 	client   kubernetes.Interface
+	http     http.Client
 	log      *logrus.Entry
 	metrics  *metrics.Metrics
 	interval time.Duration
@@ -40,10 +43,18 @@ func NewKubeReconciler(
 		log.Info("Kubernetes version checking disabled (no channel specified)")
 		return nil
 	}
+	log = log.WithField("controller", "channel")
+
+	httpClient := retryablehttp.NewClient()
+	httpClient.RetryMax = 3
+	httpClient.RetryWaitMin = 1 * time.Second
+	httpClient.RetryWaitMax = 30 * time.Second
+	httpClient.Logger = log
 
 	return &ClusterVersionScheduler{
 		log:      log.WithField("channel", channel),
 		client:   kubernetes.NewForConfigOrDie(config),
+		http:     *httpClient.StandardClient(),
 		interval: interval,
 		metrics:  metrics,
 		channel:  channel,
@@ -52,6 +63,7 @@ func NewKubeReconciler(
 
 func (s *ClusterVersionScheduler) Start(ctx context.Context) error {
 	go s.runScheduler(ctx)
+	// Run an initial check on startup
 	return s.reconcile()
 }
 
@@ -83,7 +95,7 @@ func (s *ClusterVersionScheduler) reconcile() error {
 	}
 
 	// Get latest version from specified channel
-	latest, err := getLatestVersion(s.channel)
+	latest, err := s.getLatestVersion(s.channel)
 	if err != nil {
 		return fmt.Errorf("fetching latest version from channel %s: %w", s.channel, err)
 	}
@@ -114,13 +126,13 @@ func (s *ClusterVersionScheduler) reconcile() error {
 	return nil
 }
 
-func getLatestVersion(channel string) (string, error) {
+func (s *ClusterVersionScheduler) getLatestVersion(channel string) (string, error) {
 	// Always use upstream Kubernetes channels - this is the authoritative source
 	// Platform detection is kept for logging purposes only
-	return getLatestVersionFromUpstream(channel)
+	return s.getLatestVersionFromUpstream(channel)
 }
 
-func getLatestVersionFromUpstream(channel string) (string, error) {
+func (s *ClusterVersionScheduler) getLatestVersionFromUpstream(channel string) (string, error) {
 	// Validate channel - only allow known Kubernetes channels
 	if !isValidKubernetesChannel(channel) {
 		return "", fmt.Errorf("unsupported channel: %s. Valid channels: stable, latest, latest-1.xx", channel)
@@ -135,21 +147,20 @@ func getLatestVersionFromUpstream(channel string) (string, error) {
 		return "", fmt.Errorf("failed to join channel URL: %w", err)
 	}
 
-	client := retryablehttp.NewClient()
-	client.RetryMax = 3
-	client.RetryWaitMin = 1 * time.Second
-	client.RetryWaitMax = 30 * time.Second
-	client.Logger = nil
-
-	resp, err := client.Get(channelURL)
+	resp, err := s.http.Get(channelURL)
 	if err != nil {
 		return "", fmt.Errorf("failed to fetch from channel URL %s: %w", channelURL, err)
 	}
-	defer resp.Body.Close()
+	defer func() {
+		_ = resp.Body.Close()
+	}()
 
-	if resp.StatusCode != 200 {
+	if resp.StatusCode != http.StatusOK {
 		return "", fmt.Errorf("unexpected status code %d when fetching channel %s", resp.StatusCode, channel)
 	}
+	if resp.Header.Get("content-type") != "text/plain" {
+		return "", fmt.Errorf("unexpected content-type %s when fetching channel %s", resp.Header.Get("content-type"), channel)
+	}
 
 	body, err := io.ReadAll(resp.Body)
 	if err != nil {
@@ -158,7 +169,7 @@ func getLatestVersionFromUpstream(channel string) (string, error) {
 
 	version := strings.TrimSpace(string(body))
 	if version == "" {
-		return "", fmt.Errorf("empty version returned from channel %s", channel)
+		return "", fmt.Errorf("empty version received from channel %s", channel)
 	}
 
 	return version, nil
@@ -168,15 +179,10 @@ func isValidKubernetesChannel(channel string) bool {
 	// Only allow official Kubernetes channels
 	validChannels := []string{"stable", "latest"}
 
-	// Allow latest-X.Y format
-	if strings.HasPrefix(channel, "latest-1.") {
+	// Allow latest-X.Y and stable-X.Y formats
+	if strings.HasPrefix(channel, "latest-1.") || strings.HasPrefix(channel, "stable-1.") {
 		return true
 	}
 
-	for _, valid := range validChannels {
-		if channel == valid {
-			return true
-		}
-	}
-	return false
+	return slices.Contains(validChannels, channel)
 }

pkg/controller/kube_controller_test.go

@@ -220,7 +220,10 @@ func TestGetLatestVersion(t *testing.T) {
 				if err != nil {
 					return "", err
 				}
-				defer resp.Body.Close()
+
+				defer func() {
+					require.NoError(t, resp.Body.Close())
+				}()
 
 				if resp.StatusCode != http.StatusOK {
 					return "", assert.AnError