refactor pkg/kind (#482)

squidboylan · web-flow · commit 886c15fcc688 · 2025-02-24T08:51:20.000-08:00
Signed-off-by: Caleb Boylan &lt;calebboylan@gmail.com&gt;
diff --git a/pkg/kind/cluster.go b/pkg/kind/cluster.go
@@ -2,16 +2,12 @@ package kind
 
 import (
 	"context"
-	"embed"
 	"errors"
 	"fmt"
 	"github.com/cnoe-io/idpbuilder/pkg/util"
 	"github.com/cnoe-io/idpbuilder/pkg/util/files"
-	"io"
-	"io/fs"
-	"os"
+	"net/http"
 	"strconv"
-	"strings"
 
 	"github.com/cnoe-io/idpbuilder/api/v1alpha1"
 	"github.com/go-logr/logr"
@@ -32,8 +28,13 @@ var (
 	setupLog = log.Log.WithName("setup")
 )
 
+type HttpClient interface {
+	Get(url string) (resp *http.Response, err error)
+}
+
 type Cluster struct {
 	provider          IProvider
+	httpClient        HttpClient
 	name              string
 	kubeVersion       string
 	kubeConfigPath    string
@@ -43,11 +44,6 @@ type Cluster struct {
 	cfg               v1alpha1.BuildCustomizationSpec
 }
 
-type PortMapping struct {
-	HostPort      string
-	ContainerPort string
-}
-
 type IProvider interface {
 	List() ([]string, error)
 	ListNodes(string) ([]nodes.Node, error)
@@ -57,67 +53,12 @@ type IProvider interface {
 	ExportKubeConfig(string, string, bool) error
 }
 
-type TemplateConfig struct {
-	v1alpha1.BuildCustomizationSpec
-	KubernetesVersion string
-	ExtraPortsMapping []PortMapping
-	RegistryConfig    string
-}
-
-//go:embed resources/*
-var configFS embed.FS
-
 func (c *Cluster) getConfig() ([]byte, error) {
+	rawConfigTempl, err := loadConfig(c.kindConfigPath, c.httpClient)
 
-	var rawConfigTempl []byte
-	var err error
+	portMappingPairs := parsePortMappings(c.extraPortsMapping)
 
-	if c.kindConfigPath != "" {
-		if strings.HasPrefix(c.kindConfigPath, "https://") || strings.HasPrefix(c.kindConfigPath, "http://") {
-			httpClient := util.GetHttpClient()
-			resp, err := httpClient.Get(c.kindConfigPath)
-			if err != nil {
-				return nil, fmt.Errorf("fetching remote kind config: %w", err)
-			}
-			defer resp.Body.Close()
-			rawConfigTempl, err = io.ReadAll(resp.Body)
-			if err != nil {
-				return nil, fmt.Errorf("reading remote kind config body: %w", err)
-			}
-		} else {
-			rawConfigTempl, err = os.ReadFile(c.kindConfigPath)
-		}
-	} else {
-		rawConfigTempl, err = fs.ReadFile(configFS, "resources/kind.yaml.tmpl")
-	}
-
-	if err != nil {
-		return nil, fmt.Errorf("reading kind config: %w", err)
-	}
-
-	var portMappingPairs []PortMapping
-	if len(c.extraPortsMapping) > 0 {
-		// Split pairs of ports "11=1111","22=2222",etc
-		pairs := strings.Split(c.extraPortsMapping, ",")
-		// Create a slice to store PortMapping pairs.
-		portMappingPairs = make([]PortMapping, len(pairs))
-		// Parse each pair into PortPair objects.
-		for i, pair := range pairs {
-			parts := strings.Split(pair, ":")
-			if len(parts) == 2 {
-				portMappingPairs[i] = PortMapping{parts[0], parts[1]}
-			}
-		}
-	}
-
-	registryConfig := ""
-	for _, s := range c.registryConfig {
-		path := os.ExpandEnv(s)
-		if _, err := os.Stat(path); err == nil {
-			registryConfig = path
-			break
-		}
-	}
+	registryConfig := findRegistryConfig(c.registryConfig)
 
 	if len(c.registryConfig) > 0 && registryConfig == "" {
 		return nil, errors.New("--registry-config flag used but no registry config was found")
@@ -159,6 +100,7 @@ func NewCluster(name, kubeVersion, kubeConfigPath, kindConfigPath, extraPortsMap
 
 	return &Cluster{
 		provider:          provider,
+		httpClient:        util.GetHttpClient(),
 		name:              name,
 		kindConfigPath:    kindConfigPath,
 		kubeVersion:       kubeVersion,
diff --git a/pkg/kind/config.go b/pkg/kind/config.go
@@ -0,0 +1,85 @@
+package kind
+
+import (
+	"embed"
+	"fmt"
+	"io"
+	"io/fs"
+	"os"
+	"strings"
+
+	"github.com/cnoe-io/idpbuilder/api/v1alpha1"
+)
+
+type PortMapping struct {
+	HostPort      string
+	ContainerPort string
+}
+
+type TemplateConfig struct {
+	v1alpha1.BuildCustomizationSpec
+	KubernetesVersion string
+	ExtraPortsMapping []PortMapping
+	RegistryConfig    string
+}
+
+//go:embed resources/* testdata/custom-kind.yaml.tmpl
+var configFS embed.FS
+
+func loadConfig(path string, httpClient HttpClient) ([]byte, error) {
+	var rawConfigTempl []byte
+	var err error
+	if path != "" {
+		if strings.HasPrefix(path, "https://") || strings.HasPrefix(path, "http://") {
+			resp, err := httpClient.Get(path)
+			if err != nil {
+				return nil, fmt.Errorf("fetching remote kind config: %w", err)
+			}
+			defer resp.Body.Close()
+			if !(resp.StatusCode < 300 && resp.StatusCode >= 200) {
+				return nil, fmt.Errorf("got %d status code when fetching kind config", resp.StatusCode)
+			}
+			rawConfigTempl, err = io.ReadAll(resp.Body)
+			if err != nil {
+				return nil, fmt.Errorf("reading remote kind config body: %w", err)
+			}
+		} else {
+			rawConfigTempl, err = os.ReadFile(path)
+		}
+	} else {
+		rawConfigTempl, err = fs.ReadFile(configFS, "resources/kind.yaml.tmpl")
+	}
+
+	if err != nil {
+		return nil, fmt.Errorf("reading kind config: %w", err)
+	}
+	return rawConfigTempl, nil
+}
+
+func parsePortMappings(extraPortsMapping string) []PortMapping {
+	var portMappingPairs []PortMapping
+	if len(extraPortsMapping) > 0 {
+		// Split pairs of ports "11=1111","22=2222",etc
+		pairs := strings.Split(extraPortsMapping, ",")
+		// Create a slice to store PortMapping pairs.
+		portMappingPairs = make([]PortMapping, len(pairs))
+		// Parse each pair into PortPair objects.
+		for i, pair := range pairs {
+			parts := strings.Split(pair, ":")
+			if len(parts) == 2 {
+				portMappingPairs[i] = PortMapping{parts[0], parts[1]}
+			}
+		}
+	}
+	return portMappingPairs
+}
+
+func findRegistryConfig(registryConfigPaths []string) string {
+	for _, s := range registryConfigPaths {
+		path := os.ExpandEnv(s)
+		if _, err := os.Stat(path); err == nil {
+			return path
+		}
+	}
+	return ""
+}
diff --git a/pkg/kind/config_test.go b/pkg/kind/config_test.go
@@ -0,0 +1,186 @@
+package kind
+
+import (
+	"errors"
+	"io"
+	"io/fs"
+	"net/http"
+	"reflect"
+	"strings"
+	"testing"
+)
+
+type MockHttpClient struct{}
+
+func (o *MockHttpClient) Get(url string) (resp *http.Response, err error) {
+	if url == "https://doesnotexist" || url == "http://doesnotexist" {
+		return nil, errors.New("connection error")
+	} else if url == "https://404" {
+		body := io.NopCloser(strings.NewReader(""))
+		r := http.Response{
+			Status:     "404 NotFound",
+			StatusCode: 404,
+			Body:       body,
+		}
+		return &r, nil
+	}
+
+	body := io.NopCloser(strings.NewReader("foo: bar"))
+	r := http.Response{
+		Status:     "200 OK",
+		StatusCode: 200,
+		Body:       body,
+	}
+
+	return &r, nil
+}
+
+func TestLoadConfig(t *testing.T) {
+	httpClient := MockHttpClient{}
+	defaultTemplate, err := fs.ReadFile(configFS, "resources/kind.yaml.tmpl")
+	if err != nil {
+		t.Fatalf("failed to load default kind template: %v", err)
+	}
+
+	customTemplate, err := fs.ReadFile(configFS, "testdata/custom-kind.yaml.tmpl")
+	if err != nil {
+		t.Fatalf("failed to load custom kind template: %v", err)
+	}
+
+	httpsTemplate := []byte("foo: bar")
+
+	connectionErr := "fetching remote kind config: connection error"
+	notFoundErr := "got 404 status code when fetching kind config"
+
+	type test struct {
+		path     string
+		expected []byte
+		err      *string
+	}
+	tests := []test{
+		{
+			path:     "",
+			expected: defaultTemplate,
+			err:      nil,
+		},
+		{
+			path:     "testdata/custom-kind.yaml.tmpl",
+			expected: customTemplate,
+			err:      nil,
+		},
+		{
+			path:     "https://doesnotexist",
+			expected: defaultTemplate,
+			err:      &connectionErr,
+		},
+		{
+			path:     "http://doesnotexist",
+			expected: customTemplate,
+			err:      &connectionErr,
+		},
+		{
+			path:     "https://404",
+			expected: defaultTemplate,
+			err:      &notFoundErr,
+		},
+		{
+			path:     "https://anyurlworks",
+			expected: httpsTemplate,
+			err:      nil,
+		},
+	}
+
+	for _, tc := range tests {
+		out, err := loadConfig(tc.path, &httpClient)
+		if tc.err != nil {
+			if err != nil {
+				if err.Error() != *tc.err {
+					t.Errorf("expected error: %v\nfound error: %v", *tc.err, err.Error())
+				}
+			} else {
+				t.Errorf("expected error: %v\ndidnt find an error", *tc.err)
+			}
+		} else {
+			if err != nil {
+				t.Errorf("failed to load kind config: %v", err)
+			}
+			if !reflect.DeepEqual(tc.expected, out) {
+				t.Errorf("expected:\n%v\ngot:\n%v", string(tc.expected), string(out))
+			}
+		}
+	}
+}
+
+func TestExtraPortMappingsUtilFunc(t *testing.T) {
+	type test struct {
+		extraPortMappings string
+		expected          []PortMapping
+	}
+	tests := []test{
+		{
+			extraPortMappings: "",
+			expected:          []PortMapping(nil),
+		},
+		{
+			extraPortMappings: "22:32222",
+			expected: []PortMapping{
+				{
+					HostPort:      "22",
+					ContainerPort: "32222",
+				},
+			},
+		},
+		{
+			extraPortMappings: "11:1111,33:3333,4444:4444",
+			expected: []PortMapping{
+				{
+					HostPort:      "11",
+					ContainerPort: "1111",
+				},
+				{
+					HostPort:      "33",
+					ContainerPort: "3333",
+				},
+				{
+					HostPort:      "4444",
+					ContainerPort: "4444",
+				},
+			},
+		},
+	}
+
+	for _, tc := range tests {
+		pmOutput := parsePortMappings(tc.extraPortMappings)
+		if !reflect.DeepEqual(tc.expected, pmOutput) {
+			t.Errorf("expected: %v, got: %v", tc.expected, pmOutput)
+		}
+	}
+}
+
+func TestFindRegistryConfig(t *testing.T) {
+	type test struct {
+		paths    []string
+		expected string
+	}
+	tests := []test{
+		{
+			paths:    []string{"testdata/empty.json"},
+			expected: "testdata/empty.json",
+		},
+		{
+			paths:    []string{"doesntexist"},
+			expected: "",
+		},
+		{
+			paths:    []string{"doesntexist", "testdata/empty.json"},
+			expected: "testdata/empty.json",
+		},
+	}
+
+	for _, tc := range tests {
+		out := findRegistryConfig(tc.paths)
+		if !reflect.DeepEqual(tc.expected, out) {
+			t.Errorf("expected:\n%v\ngot:\n%v", tc.expected, out)
+		}
+	}
+}
diff --git a/pkg/kind/testdata/custom-kind.yaml.tmpl b/pkg/kind/testdata/custom-kind.yaml.tmpl
