fix: validate values file yaml early (#125)

internal/cmd/local/local/install.go

@@ -43,7 +43,7 @@ const (
 
 type InstallOpts struct {
 	HelmChartVersion string
-	ValuesFile       string
+	HelmValues       map[string]any
 	Secrets          []string
 	Migrate          bool
 	Hosts            []string
@@ -241,9 +241,9 @@ func (c *Command) Install(ctx context.Context, opts InstallOpts) error {
 		pterm.Success.Println(fmt.Sprintf("Secret from '%s' created or updated", secretFile))
 	}
 
-	valuesYAML, err := mergeValuesWithValuesYAML(airbyteValues, opts.ValuesFile)
+	valuesYAML, err := mergeValuesWithValuesYAML(airbyteValues, opts.HelmValues)
 	if err != nil {
-		return fmt.Errorf("unable to merge values with values file '%s': %w", opts.ValuesFile, err)
+		return err
 	}
 
 	if err := c.handleChart(ctx, chartRequest{
@@ -744,19 +744,15 @@ func determineHelmChartAction(helm helm.Client, chart *chart.Chart, releaseName
 // defined in this code at a higher priority than the values defined in the values.yaml file.
 // This function returns a string representation of the value.yaml file after all
 // values provided were potentially overridden by the valuesYML file.
-func mergeValuesWithValuesYAML(values []string, valuesYAML string) (string, error) {
+func mergeValuesWithValuesYAML(values []string, userValues map[string]any) (string, error) {
 	a := maps.FromSlice(values)
-	b, err := maps.FromYAMLFile(valuesYAML)
-	if err != nil {
-		return "", fmt.Errorf("unable to read values from yaml file '%s': %w", valuesYAML, err)
-	}
-	maps.Merge(a, b)
+
+	maps.Merge(a, userValues)
 
 	res, err := maps.ToYAML(a)
 	if err != nil {
-		return "", fmt.Errorf("unable to merge values from yaml file '%s': %w", valuesYAML, err)
+		return "", fmt.Errorf("unable to merge values: %w", err)
 	}
 
 	return res, nil
-
 }

internal/cmd/local/local/install_test.go

@@ -5,7 +5,6 @@ import (
 	"errors"
 	"fmt"
 	"net/http"
-	"strings"
 	"testing"
 	"time"
 
@@ -201,7 +200,7 @@ func TestCommand_Install(t *testing.T) {
 	}
 }
 
-func TestCommand_Install_ValuesFile(t *testing.T) {
+func TestCommand_Install_HelmValues(t *testing.T) {
 	expChartRepoCnt := 0
 	expChartRepo := []struct {
 		name string
@@ -365,36 +364,12 @@ func TestCommand_Install_ValuesFile(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	if err := c.Install(context.Background(), InstallOpts{ValuesFile: "testdata/values.yml"}); err != nil {
-		t.Fatal(err)
+	helmValues := map[string]any{
+		"global": map[string]any{
+			"edition": "test",
+		},
 	}
-}
-
-func TestCommand_Install_InvalidValuesFile(t *testing.T) {
-	c, err := New(
-		k8s.TestProvider,
-		WithPortHTTP(portTest),
-		WithHelmClient(&mockHelmClient{}),
-		WithK8sClient(&k8stest.MockClient{}),
-		WithTelemetryClient(&mockTelemetryClient{}),
-		WithHTTPClient(&mockHTTP{}),
-		WithBrowserLauncher(func(url string) error {
-			return nil
-		}),
-	)
-
-	if err != nil {
+	if err := c.Install(context.Background(), InstallOpts{HelmValues: helmValues}); err != nil {
 		t.Fatal(err)
 	}
-
-	valuesFile := "testdata/dne.yml"
-
-	err = c.Install(context.Background(), InstallOpts{ValuesFile: valuesFile})
-	if err == nil {
-		t.Fatal("expecting an error, received none")
-	}
-	if !strings.Contains(err.Error(), fmt.Sprintf("unable to read values from yaml file '%s'", valuesFile)) {
-		t.Error("unexpected error:", err)
-	}
-
 }

internal/cmd/local/local_install.go

@@ -7,6 +7,7 @@ import (
 
 	"github.com/airbytehq/abctl/internal/cmd/local/k8s"
 	"github.com/airbytehq/abctl/internal/cmd/local/local"
+	"github.com/airbytehq/abctl/internal/maps"
 	"github.com/airbytehq/abctl/internal/telemetry"
 	"github.com/pterm/pterm"
 )
@@ -39,6 +40,11 @@ func (i *InstallCmd) Run(ctx context.Context, provider k8s.Provider, telClient t
 		return fmt.Errorf("unable to determine docker installation status: %w", err)
 	}
 
+	helmValues, err := maps.FromYAMLFile(i.Values)
+	if err != nil {
+		return err
+	}
+
 	return telClient.Wrap(ctx, telemetry.Install, func() error {
 		spinner.UpdateText(fmt.Sprintf("Checking for existing Kubernetes cluster '%s'", provider.ClusterName))
 
@@ -102,7 +108,7 @@ func (i *InstallCmd) Run(ctx context.Context, provider k8s.Provider, telClient t
 
 		opts := local.InstallOpts{
 			HelmChartVersion: i.ChartVersion,
-			ValuesFile:       i.Values,
+			HelmValues:       helmValues,
 			Secrets:          i.Secret,
 			Migrate:          i.Migrate,
 			Docker:           dockerClient,

internal/cmd/local/local_test.go

@@ -1,11 +1,15 @@
 package local
 
 import (
+	"context"
 	"os"
 	"path/filepath"
+	"strings"
 	"testing"
 
+	"github.com/airbytehq/abctl/internal/cmd/local/k8s"
 	"github.com/airbytehq/abctl/internal/cmd/local/paths"
+	"github.com/airbytehq/abctl/internal/telemetry"
 	"github.com/google/go-cmp/cmp"
 )
 
@@ -89,3 +93,39 @@ func TestCheckAirbyteDir(t *testing.T) {
 		}
 	})
 }
+
+func TestValues_FileDoesntExist(t *testing.T) {
+	cmd := InstallCmd{Values: "thisfiledoesnotexist"}
+	err := cmd.Run(context.Background(), k8s.TestProvider, telemetry.NoopClient{})
+	if err == nil {
+		t.Fatal("expected error")
+	}
+	expect := "failed to read file thisfiledoesnotexist: open thisfiledoesnotexist: no such file or directory"
+	if err.Error() != expect {
+		t.Errorf("expected %q but got %q", expect, err)
+	}
+}
+
+func TestValues_BadYaml(t *testing.T) {
+	tmpdir := t.TempDir()
+	p := filepath.Join(tmpdir, "values.yaml")
+	content := `
+foo:
+  - bar: baz
+    - foo
+`
+
+	if err := os.WriteFile(p, []byte(content), 0644); err != nil {
+		t.Fatal(err)
+	}
+
+	cmd := InstallCmd{Values: p}
+	err := cmd.Run(context.Background(), k8s.TestProvider, telemetry.NoopClient{})
+	if err == nil {
+		t.Fatal("expected error")
+	}
+
+	if !strings.HasPrefix(err.Error(), "failed to unmarshal file") {
+		t.Errorf("unexpected error: %v", err)
+	}
+}