Merge pull request #51 from /issues/5

Add trust-zone helm override command

cmd/cofidectl/cmd/trustzone/helm/helm.go

@@ -9,6 +9,7 @@ import (
 	"os"
 
 	"github.com/spf13/cobra"
+	"google.golang.org/protobuf/types/known/structpb"
 	"gopkg.in/yaml.v3"
 
 	cmdcontext "github.com/cofide/cofidectl/cmd/cofidectl/cmd/context"
@@ -32,19 +33,96 @@ This command consists of multiple sub-commands to administer Cofide trust zone H
 
 func (c *HelmCommand) GetRootCommand() *cobra.Command {
 	cmd := &cobra.Command{
-		Use:   "helm values [ARGS]",
+		Use:   "helm override|values [ARGS]",
 		Short: "Manage trust zone Helm configuration",
 		Long:  helmRootCmdDesc,
 		Args:  cobra.NoArgs,
 	}
 
 	cmd.AddCommand(
+		c.GetOverrideCommand(),
 		c.GetValuesCommand(),
 	)
 
 	return cmd
 }
 
+var helmOverrideCmdDesc = `
+This command will override Helm values for a trust zone in the Cofide configuration state.
+`
+
+type overrideOpts struct {
+	inputPath string
+}
+
+func (c *HelmCommand) GetOverrideCommand() *cobra.Command {
+	opts := overrideOpts{}
+	cmd := &cobra.Command{
+		Use:   "override [ARGS]",
+		Short: "Override Helm values for a trust zone",
+		Long:  helmOverrideCmdDesc,
+		Args:  cobra.ExactArgs(1),
+		RunE: func(cmd *cobra.Command, args []string) error {
+			ds, err := c.cmdCtx.PluginManager.GetDataSource()
+			if err != nil {
+				return err
+			}
+
+			var reader io.Reader
+			if opts.inputPath == "-" {
+				reader = os.Stdin
+			} else {
+				f, err := os.Open(opts.inputPath)
+				if err != nil {
+					return err
+				}
+				defer f.Close()
+				reader = f
+			}
+			values, err := readValues(reader)
+			if err != nil {
+				return err
+			}
+
+			return c.overrideValues(ds, args[0], values)
+		},
+	}
+
+	f := cmd.Flags()
+	f.StringVar(&opts.inputPath, "input-file", "values.yaml", "Path of a file to read YAML values from, or - for stdin")
+
+	return cmd
+}
+
+// overrideValues overrides Helm values for a trust zone.
+func (c *HelmCommand) overrideValues(ds plugin.DataSource, tzName string, values map[string]interface{}) error {
+	trustZone, err := ds.GetTrustZone(tzName)
+	if err != nil {
+		return err
+	}
+
+	trustZone.ExtraHelmValues, err = structpb.NewStruct(values)
+	if err != nil {
+		return err
+	}
+
+	// Check that the values are acceptable.
+	generator := helm.NewHelmValuesGenerator(trustZone, ds)
+	if _, err = generator.GenerateValues(); err != nil {
+		return err
+	}
+
+	return ds.UpdateTrustZone(trustZone)
+}
+
+// readValues reads values in YAML format from the specified reader.
+func readValues(reader io.Reader) (map[string]interface{}, error) {
+	decoder := yaml.NewDecoder(reader)
+	var values map[string]interface{}
+	err := decoder.Decode(&values)
+	return values, err
+}
+
 var helmValuesCmdDesc = `
 This command will generate Helm values for a trust zone in the Cofide configuration state.
 `

go.mod

@@ -5,7 +5,7 @@ go 1.22.7
 require (
 	buf.build/go/protoyaml v0.2.0
 	cuelang.org/go v0.10.1
-	github.com/cofide/cofide-api-sdk v0.3.0
+	github.com/cofide/cofide-api-sdk v0.3.1-0.20241125162004-bf7d02600ea6
 	github.com/fatih/color v1.18.0
 	github.com/gofrs/flock v0.12.1
 	github.com/google/go-cmp v0.6.0

go.sum

@@ -84,8 +84,8 @@ github.com/cncf/xds/go v0.0.0-20211001041855-01bcc9b48dfe/go.mod h1:eXthEFrGJvWH
 github.com/cncf/xds/go v0.0.0-20211011173535-cb28da3451f1/go.mod h1:eXthEFrGJvWHgFFCl3hGmgk+/aYT6PnTQLykKQRLhEs=
 github.com/cockroachdb/apd/v3 v3.2.1 h1:U+8j7t0axsIgvQUqthuNm82HIrYXodOV2iWLWtEaIwg=
 github.com/cockroachdb/apd/v3 v3.2.1/go.mod h1:klXJcjp+FffLTHlhIG69tezTDvdP065naDsHzKhYSqc=
-github.com/cofide/cofide-api-sdk v0.3.0 h1:EhnCMc0YMNE4T+cHzlLa8Y+fRppVSTCrB7Wbhh+X8p0=
-github.com/cofide/cofide-api-sdk v0.3.0/go.mod h1:Gh9QD4ZsH/1N+u1O90QFsYcODXCO7ZshqBGB2bL1gqw=
+github.com/cofide/cofide-api-sdk v0.3.1-0.20241125162004-bf7d02600ea6 h1:Ja1FpDhRccZ+RLjDQljwoKvF0cL/YQdjPSvrvwllcU4=
+github.com/cofide/cofide-api-sdk v0.3.1-0.20241125162004-bf7d02600ea6/go.mod h1:yKMfhL3qCIVJcKvgZsPZC1o60/8co6/0NsCaJtrUoFY=
 github.com/containerd/cgroups v1.1.0 h1:v8rEWFl6EoqHB+swVNjVoCJE8o3jX7e8nqBGPLaDFBM=
 github.com/containerd/cgroups v1.1.0/go.mod h1:6ppBcbh/NOOUU+dMKrykgaBnK9lCIBxHqJDGwsa1mIw=
 github.com/containerd/containerd v1.7.23 h1:H2CClyUkmpKAGlhQp95g2WXHfLYc7whAuvZGBNYOOwQ=

internal/pkg/config/schema.cue

@@ -11,6 +11,7 @@
 	federations: [...#Federation]
 	attestation_policies: [...#APBinding]
 	jwt_issuer?: string
+	extra_helm_values?: #HelmValues
 }
 
 #TrustProvider: {
@@ -53,6 +54,10 @@
 	to!: string
 }
 
+#HelmValues: {
+	[string]: _
+}
+
 #Config: {
 	data_source!: #DataSource
 	trust_zones: [...#TrustZone]

internal/pkg/config/testdata/config/full.yaml

@@ -16,6 +16,13 @@ trust_zones:
           federates_with:
             - tz2
       jwt_issuer: https://tz1.example.com
+      extra_helm_values:
+        global:
+            spire:
+                namespaces:
+                    create: true
+        spire-server:
+            logLevel: INFO
     - name: tz2
       trust_domain: td2
       kubernetes_cluster: local2

internal/pkg/provider/helm/values.go

@@ -128,27 +128,46 @@ func (g *HelmValuesGenerator) GenerateValues() (map[string]interface{}, error) {
 	ctx := cuecontext.New()
 	combinedValuesCUE := ctx.CompileBytes([]byte{})
 
-	for _, valuesMap := range valuesMaps {
-		valuesCUE := ctx.CompileBytes([]byte{})
+	if g.trustZone.ExtraHelmValues != nil {
+		// Extra values are in a structured map format rather than dotted-paths.
+		// These take precedence over the values in cofidectl.
+		extraValues := g.trustZone.ExtraHelmValues.AsMap()
+		extraValuesCUE := ctx.Encode(extraValues)
+		combinedValuesCUE = combinedValuesCUE.Unify(extraValuesCUE)
+		if err := combinedValuesCUE.Err(); err != nil {
+			return nil, err
+		}
+	}
 
+	for _, valuesMap := range valuesMaps {
 		for path, value := range valuesMap {
-			valuesCUE = valuesCUE.FillPath(cue.ParsePath(path), value)
+			// We need to merge the values generated by this function with the user-specified extra
+			// helm values.
+			// CUE does not easily handle merging values with multiple levels of precedence, since
+			// it sees different concrete values for the same path as a conflict. It supports
+			// disjunctions with defaults, but that would only work for a 2-level merge.
+			// For now we can start with the higher precedence user values, then merge in the
+			// defaults to paths that do not exist.
+			// TODO: Revisit the use of CUE here, consider reworking to use native Go types with a
+			// merge function.
+			if !combinedValuesCUE.LookupPath(cue.ParsePath(path)).Exists() {
+				combinedValuesCUE = combinedValuesCUE.FillPath(cue.ParsePath(path), value)
+				if err := combinedValuesCUE.Err(); err != nil {
+					return nil, err
+				}
+			}
 		}
-
-		combinedValuesCUE = combinedValuesCUE.Unify(valuesCUE)
 	}
 
 	combinedValuesJSON, err := combinedValuesCUE.MarshalJSON()
 	if err != nil {
-		// TODO: Improve error messaging.
-		return nil, err
+		return nil, fmt.Errorf("failed marshalling Helm values to JSON: %w", err)
 	}
 
 	var values map[string]interface{}
 	err = json.Unmarshal([]byte(combinedValuesJSON), &values)
 	if err != nil {
-		// TODO: Improve error messaging.
-		return nil, err
+		return nil, fmt.Errorf("failed unmarshalling Helm values from JSON: %w", err)
 	}
 
 	return values, nil

internal/pkg/provider/helm/values_test.go

@@ -14,6 +14,7 @@ import (
 	"github.com/cofide/cofidectl/pkg/plugin/local"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"google.golang.org/protobuf/types/known/structpb"
 )
 
 type Values = map[string]interface{}
@@ -34,6 +35,7 @@ func TestHelmValuesGenerator_GenerateValues_success(t *testing.T) {
 				tz.BundleEndpointUrl = nil
 				tz.Federations = nil
 				tz.JwtIssuer = nil
+				tz.ExtraHelmValues = nil
 				return tz
 			}(),
 			want: Values{
@@ -127,6 +129,9 @@ func TestHelmValuesGenerator_GenerateValues_success(t *testing.T) {
 					"spire": Values{
 						"clusterName": "local1",
 						"jwtIssuer":   "https://tz1.example.com",
+						"namespaces": Values{
+							"create": true,
+						},
 						"recommendations": Values{
 							"create": true,
 						},
@@ -199,7 +204,7 @@ func TestHelmValuesGenerator_GenerateValues_success(t *testing.T) {
 						"enabled": true,
 					},
 					"fullnameOverride": "spire-server",
-					"logLevel":         "DEBUG",
+					"logLevel":         "INFO",
 					"nodeAttestor": Values{
 						"k8sPsat": Values{
 							"allowedNodeLabelKeys": ValueList{},
@@ -229,7 +234,7 @@ func TestHelmValuesGenerator_GenerateValues_success(t *testing.T) {
 				trustZone: tt.trustZone,
 			}
 			got, err := g.GenerateValues()
-			require.Nil(t, err)
+			require.Nil(t, err, err)
 			assert.Equal(t, tt.want, got)
 		})
 	}
@@ -277,6 +282,21 @@ func TestHelmValuesGenerator_GenerateValues_failure(t *testing.T) {
 			}(),
 			wantErrString: "failed to find trust zone invalid-tz in local config",
 		},
+		{
+			name: "invalid extra helm values",
+			trustZone: func() *trust_zone_proto.TrustZone {
+				tz := fixtures.TrustZone("tz1")
+				s, err := structpb.NewStruct(map[string]any{
+					"global": "not-a-map",
+				})
+				if err != nil {
+					require.Nil(t, err, err)
+				}
+				tz.ExtraHelmValues = s
+				return tz
+			}(),
+			wantErrString: "global: conflicting values \"not-a-map\"",
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {

internal/pkg/test/fixtures/fixtures.go

@@ -12,6 +12,7 @@ import (
 	trust_provider_proto "github.com/cofide/cofide-api-sdk/gen/go/proto/trust_provider/v1alpha1"
 	trust_zone_proto "github.com/cofide/cofide-api-sdk/gen/go/proto/trust_zone/v1alpha1"
 	"github.com/cofide/cofidectl/internal/pkg/proto"
+	"google.golang.org/protobuf/types/known/structpb"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
@@ -39,6 +40,25 @@ var trustZoneFixtures map[string]*trust_zone_proto.TrustZone = map[string]*trust
 			},
 		},
 		JwtIssuer: StringPtr("https://tz1.example.com"),
+		ExtraHelmValues: func() *structpb.Struct {
+			ev := map[string]interface{}{
+				"global": map[string]interface{}{
+					"spire": map[string]interface{}{
+						"namespaces": map[string]interface{}{
+							"create": true,
+						},
+					},
+				},
+				"spire-server": map[string]interface{}{
+					"logLevel": "INFO",
+				},
+			}
+			value, err := structpb.NewStruct(ev)
+			if err != nil {
+				panic(err)
+			}
+			return value
+		}(),
 	},
 	"tz2": {
 		Name:              "tz2",