diff --git a/internal/cmd/local/local/cmd.go b/internal/cmd/local/local/cmd.go index 4a3adb0..259db2f 100644 --- a/internal/cmd/local/local/cmd.go +++ b/internal/cmd/local/local/cmd.go @@ -7,6 +7,7 @@ import ( "github.com/airbytehq/abctl/internal/cmd/local/helm" "github.com/airbytehq/abctl/internal/cmd/local/migrate" "github.com/airbytehq/abctl/internal/cmd/local/paths" + "github.com/airbytehq/abctl/internal/maps" "helm.sh/helm/v3/pkg/chart" "helm.sh/helm/v3/pkg/release" corev1 "k8s.io/api/core/v1" @@ -306,15 +307,6 @@ func (c *Command) persistentVolumeClaim(ctx context.Context, namespace, name, vo // Install handles the installation of Airbyte func (c *Command) Install(ctx context.Context, opts InstallOpts) error { - var vals string - if opts.ValuesFile != "" { - raw, err := os.ReadFile(opts.ValuesFile) - if err != nil { - return fmt.Errorf("unable to read values file '%s': %w", opts.ValuesFile, err) - } - vals = string(raw) - } - go c.watchEvents(ctx) if !c.k8s.NamespaceExists(ctx, airbyteNamespace) { @@ -358,9 +350,9 @@ func (c *Command) Install(ctx context.Context, opts InstallOpts) error { } airbyteValues := []string{ - fmt.Sprintf("global.env_vars.AIRBYTE_INSTALLATION_ID=%s", telUser), - fmt.Sprintf("global.jobs.resources.limits.cpu=3"), - fmt.Sprintf("global.jobs.resources.limits.memory=4Gi"), + "global.env_vars.AIRBYTE_INSTALLATION_ID=" + telUser, + "global.jobs.resources.limits.cpu=3", + "global.jobs.resources.limits.memory=4Gi", } if opts.dockerAuth() { @@ -396,6 +388,11 @@ 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) + if err != nil { + return fmt.Errorf("unable to merge values with values file '%s': %w", opts.ValuesFile, err) + } + if err := c.handleChart(ctx, chartRequest{ name: "airbyte", repoName: airbyteRepoName, @@ -404,8 +401,7 @@ func (c *Command) Install(ctx context.Context, opts InstallOpts) error { chartRelease: airbyteChartRelease, chartVersion: opts.HelmChartVersion, namespace: airbyteNamespace, - values: airbyteValues, - valuesYAML: vals, + valuesYAML: valuesYAML, }); err != nil { return fmt.Errorf("unable to install airbyte chart: %w", err) } @@ -871,3 +867,26 @@ func checkHelmReleaseShouldInstall(helm helm.Client, chart *chart.Chart, release return false } + +// mergeValuesWithValuesYAML ensures that the values defined within this code have a lower +// priority than any values defined in a values.yaml file. +// By default, the helm-client we're using reversed this priority, putting the values +// 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) { + 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) + + res, err := maps.ToYAML(a) + if err != nil { + return "", fmt.Errorf("unable to merge values from yaml file '%s': %w", valuesYAML, err) + } + + return res, nil + +} diff --git a/internal/cmd/local/local/cmd_test.go b/internal/cmd/local/local/cmd_test.go index 9122bc7..976623f 100644 --- a/internal/cmd/local/local/cmd_test.go +++ b/internal/cmd/local/local/cmd_test.go @@ -52,11 +52,15 @@ func TestCommand_Install(t *testing.T) { CreateNamespace: true, Wait: true, Timeout: 30 * time.Minute, - ValuesOptions: values.Options{Values: []string{ - "global.env_vars.AIRBYTE_INSTALLATION_ID=" + userID.String(), - "global.jobs.resources.limits.cpu=3", - "global.jobs.resources.limits.memory=4Gi", - }}, + ValuesYaml: `global: + env_vars: + AIRBYTE_INSTALLATION_ID: ` + userID.String() + ` + jobs: + resources: + limits: + cpu: "3" + memory: 4Gi +`, }, release: release.Release{ Chart: &chart.Chart{Metadata: &chart.Metadata{Version: "1.2.3.4"}}, @@ -205,12 +209,16 @@ func TestCommand_Install_ValuesFile(t *testing.T) { CreateNamespace: true, Wait: true, Timeout: 30 * time.Minute, - ValuesOptions: values.Options{Values: []string{ - "global.env_vars.AIRBYTE_INSTALLATION_ID=" + userID.String(), - "global.jobs.resources.limits.cpu=3", - "global.jobs.resources.limits.memory=4Gi", - }}, - ValuesYaml: "global:\n edition: \"test\"\n", + ValuesYaml: `global: + edition: test + env_vars: + AIRBYTE_INSTALLATION_ID: ` + userID.String() + ` + jobs: + resources: + limits: + cpu: "3" + memory: 4Gi +`, }, release: release.Release{ Chart: &chart.Chart{Metadata: &chart.Metadata{Version: "1.2.3.4"}}, @@ -356,7 +364,7 @@ func TestCommand_Install_InvalidValuesFile(t *testing.T) { if err == nil { t.Fatal("expecting an error, received none") } - if !strings.Contains(err.Error(), fmt.Sprintf("unable to read values file '%s'", valuesFile)) { + if !strings.Contains(err.Error(), fmt.Sprintf("unable to read values from yaml file '%s'", valuesFile)) { t.Error("unexpected error:", err) } @@ -560,6 +568,9 @@ func (m *mockTelemetryClient) Attr(key, val string) { } func (m *mockTelemetryClient) User() uuid.UUID { + if m.user == nil { + return uuid.Nil + } return m.user() } diff --git a/internal/maps/maps.go b/internal/maps/maps.go new file mode 100644 index 0000000..24f449e --- /dev/null +++ b/internal/maps/maps.go @@ -0,0 +1,103 @@ +package maps + +import ( + "fmt" + "gopkg.in/yaml.v3" + "os" + "strings" +) + +// FromSlice converts a slice of dot-delimited string values into a map[string]any. +// For example: +// "a.b.c=123","a.b.d=124" would return { "a": { "b": { "c": 123, "d": 124 } } } +func FromSlice(slice []string) map[string]any { + m := map[string]any{} + + for _, s := range slice { + // s has the format of: + // a.b.c=xyz + parts := strings.SplitN(s, "=", 2) + // a.b.c becomes [a, b, c] + keys := strings.Split(parts[0], ".") + // xyz + value := parts[1] + + // pointer to the root of the map, + // as this map will contain nested maps, this pointer will be + // updated to point to the root of the nested maps as it iterates + // through the for loop + p := m + for i, k := range keys { + // last key, put the value into the map + if i == len(keys)-1 { + p[k] = value + continue + } + // if the nested map doesn't exist, create it + if _, ok := p[k]; !ok { + p[k] = map[string]any{} + } + // cast the key to a map[string]any + // and update the pointer to point to it + p = p[k].(map[string]any) + } + } + + return m +} + +// FromYAMLFile converts a yaml file into a map[string]any. +func FromYAMLFile(path string) (map[string]any, error) { + if path == "" { + return map[string]any{}, nil + } + + raw, err := os.ReadFile(path) + if err != nil { + return nil, fmt.Errorf("failed to read file %s: %w", path, err) + } + var m map[string]any + if err := yaml.Unmarshal(raw, &m); err != nil { + return nil, fmt.Errorf("failed to unmarshal file %s: %w", path, err) + } + // ensure we don't return `nil, nil` + if m == nil { + return map[string]any{}, nil + } + return m, nil +} + +// ToYAML converts the m map into a yaml string. +// E.g. map[string]any{"a" : 1, "b", 2} becomes +// a: 1 +// b: 2 +func ToYAML(m map[string]any) (string, error) { + raw, err := yaml.Marshal(m) + if err != nil { + return "", fmt.Errorf("failed to marshal map: %w", err) + } + return string(raw), nil +} + +// Merge merges the override map into the base map. +// Modifying the base map in place. +func Merge(base, override map[string]any) { + for k, overrideVal := range override { + if baseVal, ok := base[k]; ok { + // both maps have this key + baseChild, baseChildIsMap := baseVal.(map[string]any) + overrideChild, overrideChildIsMap := overrideVal.(map[string]any) + + if baseChildIsMap && overrideChildIsMap { + // both values are maps, recurse + Merge(baseChild, overrideChild) + } else { + // override base with override + base[k] = overrideVal + } + } else { + // only override has this key + base[k] = overrideVal + } + } +} diff --git a/internal/maps/maps_test.go b/internal/maps/maps_test.go new file mode 100644 index 0000000..f6d28e4 --- /dev/null +++ b/internal/maps/maps_test.go @@ -0,0 +1,312 @@ +package maps + +import ( + "github.com/google/go-cmp/cmp" + "os" + "testing" +) + +func TestFromSlice(t *testing.T) { + tests := []struct { + name string + input []string + want map[string]any + }{ + { + name: "empty string", + input: []string{}, + want: map[string]any{}, + }, + { + name: "single element", + input: []string{"a=1"}, + want: map[string]any{ + "a": "1", + }, + }, + { + name: "multiple elements", + input: []string{"a=1", "b=2", "c=3"}, + want: map[string]any{ + "a": "1", + "b": "2", + "c": "3", + }, + }, + { + name: "single nested element", + input: []string{"a.b.c=123"}, + want: map[string]any{ + "a": map[string]any{ + "b": map[string]any{ + "c": "123", + }, + }, + }, + }, + { + name: "multiple nested elements", + input: []string{ + "a.b.c=123", + "a.b.g=127", + "d.e.f=456", + "z=26", + }, + want: map[string]any{ + "a": map[string]any{ + "b": map[string]any{ + "c": "123", + "g": "127", + }, + }, + "d": map[string]any{ + "e": map[string]any{ + "f": "456", + }, + }, + "z": "26", + }, + }, + { + name: "value contains equals", + input: []string{"a.b.c=1=2=3"}, + want: map[string]any{ + "a": map[string]any{ + "b": map[string]any{ + "c": "1=2=3", + }, + }, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if d := cmp.Diff(tt.want, FromSlice(tt.input)); d != "" { + t.Error("mismatch (-want, +got) = ", d) + } + }) + } +} + +func TestFromYMLFile(t *testing.T) { + tests := []struct { + name string + yaml string + want map[string]any + }{ + { + name: "empty string", + want: map[string]any{}, + }, + { + name: "single element", + yaml: "a: true", + want: map[string]any{ + "a": true, + }, + }, + { + name: "multiple elements", + yaml: `a: true +b: + c: "three" + d: 4 + e: + f: 6`, + want: map[string]any{ + "a": true, + "b": map[string]any{ + "c": "three", + "d": 4, + "e": map[string]any{ + "f": 6, + }, + }, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + f, err := os.CreateTemp(t.TempDir(), "*.yml") + if err != nil { + t.Fatal("could not create temp file", err) + } + if _, err := f.WriteString(tt.yaml); err != nil { + t.Fatal("could not write to temp file", err) + } + _ = f.Close() + + m, err := FromYAMLFile(f.Name()) + if err != nil { + t.Fatal("could not read from maps", err) + } + if d := cmp.Diff(tt.want, m); d != "" { + t.Error("mismatch (-want, +got) = ", d) + } + }) + } + + t.Run("no file provided", func(t *testing.T) { + m, err := FromYAMLFile("") + if err != nil { + t.Fatal("could not read from maps", err) + } + // if no file is provided, any empty map is returned + if d := cmp.Diff(map[string]any{}, m); d != "" { + t.Error("mismatch (-want, +got) = ", d) + } + }) +} + +func TestToYAML(t *testing.T) { + tests := []struct { + name string + m map[string]any + want string + }{ + { + name: "empty map", + m: map[string]any{}, + want: "{}\n", + }, + { + name: "single element", + m: map[string]any{ + "a": 1, + }, + want: "a: 1\n", + }, + { + name: "multiple elements", + m: map[string]any{ + "a": 1, + "b": "2", + "c": "three", + }, + want: `a: 1 +b: "2" +c: three +`, + }, + { + name: "nested elements", + m: map[string]any{ + "a": map[string]any{ + "b": map[string]any{ + "c": 123, + "d": "124", + }, + }, + "z": "26", + }, + want: `a: + b: + c: 123 + d: "124" +z: "26" +`, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + have, err := ToYAML(tt.m) + if err != nil { + t.Fatal("could not convert maps to YAML", err) + } + if d := cmp.Diff(tt.want, have); d != "" { + t.Error("mismatch (-want, +got) = ", d) + } + }) + } +} + +func TestMerge(t *testing.T) { + tests := []struct { + name string + base map[string]any + over map[string]any + want map[string]any + }{ + { + name: "empty", + base: map[string]any{}, + over: map[string]any{}, + want: map[string]any{}, + }, + { + name: "single element base only", + base: map[string]any{"a": "1"}, + over: map[string]any{}, + want: map[string]any{"a": "1"}, + }, + { + name: "single element over only", + base: map[string]any{}, + over: map[string]any{"a": "1"}, + want: map[string]any{"a": "1"}, + }, + { + name: "single element same in base and over", + base: map[string]any{"a": "1"}, + over: map[string]any{"a": "26"}, + want: map[string]any{"a": "26"}, + }, + { + name: "single element, different type in base", + base: map[string]any{"a": 1}, + over: map[string]any{"a": "26"}, + want: map[string]any{"a": "26"}, + }, + { + name: "single element, different type in over", + base: map[string]any{"a": "1"}, + over: map[string]any{"a": 26}, + want: map[string]any{"a": 26}, + }, + { + name: "single element diff in base and over", + base: map[string]any{"a": "1"}, + over: map[string]any{"z": "26"}, + want: map[string]any{ + "a": "1", + "z": "26", + }, + }, + { + name: "singled element diff in nested base", + base: map[string]any{ + "a": "1", + "b": map[string]any{ + "c": true, + }, + }, + over: map[string]any{ + "b": map[string]any{ + "c": false, + "d": "100", + }, + "z": "26", + }, + want: map[string]any{ + "a": "1", + "b": map[string]any{ + "c": false, + "d": "100", + }, + "z": "26", + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + Merge(tt.base, tt.over) + if d := cmp.Diff(tt.want, tt.base); d != "" { + t.Error("mismatch (-want, +got) = ", d) + } + }) + } +}