allow yaml / json objects in config files for

encrypted_passwords
plan definitions
provision defaults and other overrides in services
currently csb expects

service.something.plans to be a STRING. The code will do some manual deserializing and would fail if that path in the yaml / json config didn't contain a string.

This adds support for using real json / yaml objects to configure the csb app.
In the current deployment process of csb via a Tile, multiple layers of config are rendered and ensuring that (e.g. plans) value stays a string is convoluted and error prone. We should just allow using actual objects instead of expecting string representations of objects..

add comments for helper functions to access viper config

[#187498188]

cmd/serve.go

@@ -17,6 +17,7 @@ package cmd
 import (
 	"context"
 	"database/sql"
+	"encoding/json"
 	"fmt"
 	"log/slog"
 	"net/http"
@@ -139,21 +140,33 @@ func serveDocs() {
 }
 
 func setupDBEncryption(db *gorm.DB, logger lager.Logger) storage.Encryptor {
-	config, err := encryption.ParseConfiguration(db, viper.GetBool(encryptionEnabled), viper.GetString(encryptionPasswords))
+	encryptionPasswordsPropertyValue := viper.Get(encryptionPasswords)
+	passwordsString := ""
+	switch v := encryptionPasswordsPropertyValue.(type) {
+	case string:
+		passwordsString = v
+	default:
+		bytes, err := json.Marshal(v)
+		if err != nil {
+			logger.Fatal("couldn't parse encryption.passwords", err)
+		}
+		passwordsString = string(bytes)
+	}
+	config, err := encryption.ParseConfiguration(db, viper.GetBool(encryptionEnabled), passwordsString)
 	if err != nil {
 		logger.Fatal("Error parsing encryption configuration", err)
 	}
 
 	if config.Changed {
-		if err := storage.New(db, config.RotationEncryptor).CheckAllRecords(); err != nil {
+		if err = storage.New(db, config.RotationEncryptor).CheckAllRecords(); err != nil {
 			logger.Fatal("refusing to encrypt the database as some fields cannot be successfully read", err)
 		}
 
 		logger.Info("rotating-database-encryption", lager.Data{"previous-primary": labelName(config.StoredPrimaryLabel), "new-primary": labelName(config.ConfiguredPrimaryLabel)})
-		if err := storage.New(db, config.RotationEncryptor).UpdateAllRecords(); err != nil {
+		if err = storage.New(db, config.RotationEncryptor).UpdateAllRecords(); err != nil {
 			logger.Fatal("Error rotating database encryption", err)
 		}
-		if err := encryption.UpdatePasswordMetadata(db, config.ConfiguredPrimaryLabel); err != nil {
+		if err = encryption.UpdatePasswordMetadata(db, config.ConfiguredPrimaryLabel); err != nil {
 			logger.Fatal("Error updating password metadata", err)
 		}
 	}

integrationtest/database_encryption_test.go

@@ -142,6 +142,17 @@ var _ = Describe("Database Encryption", func() {
 		return err
 	}
 
+	startBrokerWithConfig := func(config map[string]interface{}) {
+
+		broker = must(testdrive.StartBroker(
+			csb,
+			brokerpak,
+			database,
+			testdrive.WithOutputs(GinkgoWriter, GinkgoWriter),
+			testdrive.WithConfig(config),
+		))
+	}
+
 	startBroker := func(encryptionEnabled bool, encryptionPasswords string) {
 		broker = must(testdrive.StartBroker(
 			csb,
@@ -228,6 +239,61 @@ var _ = Describe("Database Encryption", func() {
 		})
 	})
 
+	When("encryption is turned on and config is via a config file", func() {
+		BeforeEach(func() {
+			startBrokerWithConfig(map[string]interface{}{
+				"db": map[string]interface{}{
+					"encryption": map[string]interface{}{
+						"enabled": true,
+						"passwords": []map[string]interface{}{
+							{
+								"primary": true,
+								"label":   "my-password",
+								"password": map[string]interface{}{
+									"secret": "supersecretcoolpassword",
+								},
+							},
+						},
+					},
+				},
+			})
+		})
+
+		It("encrypts sensitive fields", func() {
+			serviceInstance := must(broker.Provision(serviceOfferingGUID, servicePlanGUID, testdrive.WithProvisionParams(provisionParams)))
+			By("checking the provision fields")
+			Expect(persistedProvisionRequestDetails(serviceInstance.GUID)).NotTo(bePlaintextProvisionParams)
+			Expect(persistedServiceInstanceDetails(serviceInstance.GUID)).NotTo(bePlaintextProvisionOutput)
+			Expect(persistedServiceInstanceTerraformWorkspace(serviceInstance.GUID)).NotTo(haveAnyPlaintextServiceTerraformState)
+
+			By("checking the binding fields")
+			serviceBinding := must(broker.CreateBinding(serviceInstance, testdrive.WithBindingParams(bindParams)))
+			Expect(persistedBindRequestDetails(serviceBinding.GUID)).NotTo(bePlaintextBindParams)
+			Expect(persistedServiceBindingDetails(serviceInstance.GUID)).NotTo(bePlaintextBindingDetails)
+			Expect(persistedServiceBindingTerraformWorkspace(serviceInstance.GUID, serviceBinding.GUID)).NotTo(haveAnyPlaintextBindingTerraformState)
+
+			By("checking how update persists service instance fields")
+			Expect(broker.UpdateService(serviceInstance, testdrive.WithUpdateParams(updateParams))).To(Succeed())
+			Expect(persistedProvisionRequestDetails(serviceInstance.GUID)).NotTo(Equal(mergedParams))
+			Expect(persistedServiceInstanceDetails(serviceInstance.GUID)).NotTo(Equal(updateOutput))
+			Expect(persistedServiceInstanceTerraformWorkspace(serviceInstance.GUID)).NotTo(SatisfyAny(
+				ContainSubstring(provisionOutputStateValue),
+				ContainSubstring(updateOutputStateValue),
+				ContainSubstring(tfStateKey),
+			))
+
+			By("checking the binding fields after unbind")
+			Expect(broker.DeleteBinding(serviceInstance, serviceBinding.GUID)).To(Succeed())
+			expectServiceBindingDetailsToNotExist(serviceInstance.GUID)
+			expectBindRequestDetailsToNotExist(serviceBinding.GUID)
+			expectBindTerraformWorkspaceToNotExist(serviceInstance.GUID, serviceBinding.GUID)
+
+			By("checking the service instance fields after deprovision")
+			Expect(broker.Deprovision(serviceInstance)).To(Succeed())
+			expectServiceInstanceDetailsToNotExist(serviceInstance.GUID)
+			expectServiceInstanceTerraformWorkspaceToNotExist(serviceInstance.GUID)
+		})
+	})
 	When("encryption is turned on", func() {
 		BeforeEach(func() {
 			const encryptionPasswords = `[{"primary":true,"label":"my-password","password":{"secret":"supersecretcoolpassword"}}]`

internal/testdrive/broker_start.go

@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"fmt"
 	"io"
+	"log"
 	"net/http"
 	"os"
 	"os/exec"
@@ -13,12 +14,14 @@ import (
 	"github.com/cloudfoundry/cloud-service-broker/v2/pkg/client"
 	"github.com/cloudfoundry/cloud-service-broker/v2/utils/freeport"
 	"github.com/google/uuid"
+	"gopkg.in/yaml.v3"
 )
 
 type StartBrokerOption func(config *startBrokerConfig)
 
 type startBrokerConfig struct {
 	env    []string
+	args   []string
 	stdout io.Writer
 	stderr io.Writer
 }
@@ -50,6 +53,7 @@ func StartBroker(csbPath, bpk, db string, opts ...StartBrokerOption) (*Broker, e
 		fmt.Sprintf("SECURITY_USER_NAME=%s", username),
 		fmt.Sprintf("SECURITY_USER_PASSWORD=%s", password),
 	)
+	cmd.Args = append(cmd.Args, cfg.args...)
 
 	switch cfg.stdout {
 	case nil:
@@ -98,7 +102,21 @@ func StartBroker(csbPath, bpk, db string, opts ...StartBrokerOption) (*Broker, e
 		time.Sleep(100 * time.Millisecond)
 	}
 }
-
+func WithConfig(config map[string]interface{}) StartBrokerOption {
+	return func(cfg *startBrokerConfig) {
+		bytes, err := yaml.Marshal(config)
+		if err != nil {
+			log.Fatal(err)
+		}
+		file, err := os.CreateTemp("", `*.yml`)
+		if err != nil {
+			log.Fatal(err)
+		}
+		defer file.Close()
+		file.Write(bytes)
+		cfg.args = append(cfg.args, "--config", file.Name())
+	}
+}
 func WithEnv(extraEnv ...string) StartBrokerOption {
 	return func(cfg *startBrokerConfig) {
 		cfg.env = append(cfg.env, extraEnv...)

pkg/broker/service_definition.go

@@ -152,19 +152,52 @@ func (svc *ServiceDefinition) ProvisionDefaultOverrideProperty() string {
 // ProvisionDefaultOverrides returns the deserialized JSON object for the
 // operator-provided property overrides.
 func (svc *ServiceDefinition) ProvisionDefaultOverrides() (map[string]any, error) {
-	return unmarshalViper(svc.ProvisionDefaultOverrideProperty())
+	return unmarshalViperMap(svc.ProvisionDefaultOverrideProperty())
 }
 
 func ProvisionGlobalDefaults() (map[string]any, error) {
-	return unmarshalViper(GlobalProvisionDefaults)
+	return unmarshalViperMap(GlobalProvisionDefaults)
 }
 
-func unmarshalViper(key string) (map[string]any, error) {
+// Some config values were historically expected to be provided as a string.
+// That is because these values were sourced from ENV Vars. But these values can
+// be provided via a config file.
+// E.g. consider the value .service.x.plans which would need to be a string in the config,
+// file even though it would be more readable as a list of objects.
+func unmarshalViperList(key string) ([]map[string]any, error) {
+	vals := []map[string]any{}
+	if viper.IsSet(key) {
+		val := viper.Get(key)
+
+		switch v := val.(type) {
+		case string:
+			if err := json.Unmarshal([]byte(v), &vals); err != nil {
+				return nil, fmt.Errorf("failed unmarshaling config value %s: %w", key, err)
+			}
+		case []map[string]any:
+			vals = v
+		}
+	}
+	return vals, nil
+}
+
+// Some config values were historically expected to be provided as a string.
+// That is because these values were sourced from ENV Vars. But these values can
+// be provided via a config file.
+// E.g. consider the value .service.x.provision.defaults which would need to be a
+// string in the config file even though it would be more readable as a map object.
+func unmarshalViperMap(key string) (map[string]any, error) {
 	vals := make(map[string]any)
 	if viper.IsSet(key) {
-		val := viper.GetString(key)
-		if err := json.Unmarshal([]byte(val), &vals); err != nil {
-			return nil, fmt.Errorf("failed unmarshaling config value %s", key)
+
+		val := viper.Get(key)
+		switch v := val.(type) {
+		case string:
+			if err := json.Unmarshal([]byte(v), &vals); err != nil {
+				return nil, fmt.Errorf("failed unmarshaling config value %s", key)
+			}
+		case map[string]interface{}:
+			vals = v
 		}
 	}
 	return vals, nil
@@ -270,13 +303,19 @@ func (svc *ServiceDefinition) UserDefinedPlans(maintenanceInfo *domain.Maintenan
 	// but we need [{"id":"1234", "name":"foo", "service_properties":{"A": 1, "B": 2}}]
 	// Go doesn't support this natively so we do it manually here.
 	rawPlans := []json.RawMessage{}
-
 	// Unmarshal the plans from the viper configuration which is just a JSON list
 	// of plans
-	if userPlanJSON := viper.GetString(svc.UserDefinedPlansProperty()); userPlanJSON != "" {
-		if err := json.Unmarshal([]byte(userPlanJSON), &rawPlans); err != nil {
-			return []ServicePlan{}, err
-		}
+
+	userPlan, err := unmarshalViperList(svc.UserDefinedPlansProperty())
+	if err != nil {
+		return []ServicePlan{}, err
+	}
+	bytes, err := json.Marshal(userPlan)
+	if err != nil {
+		return []ServicePlan{}, err
+	}
+	if err := json.Unmarshal(bytes, &rawPlans); err != nil {
+		return []ServicePlan{}, err
 	}
 
 	// Unmarshal tile plans if they're included, which are a JSON object where
@@ -385,10 +424,7 @@ func (svc *ServiceDefinition) bindDefaults() []varcontext.DefaultVariable {
 // For example, to create a default database name based on a user-provided instance name.
 // Therefore, they get executed conditionally if a user-provided variable does not exist.
 // Computed variables get executed either unconditionally or conditionally for greater flexibility.
-func (svc *ServiceDefinition) variables(
-	constants map[string]any,
-	userProvidedParameters map[string]any,
-	plan ServicePlan) (*varcontext.VarContext, error) {
+func (svc *ServiceDefinition) variables(constants map[string]any, userProvidedParameters map[string]any, plan ServicePlan) (*varcontext.VarContext, error) {
 
 	globalDefaults, err := ProvisionGlobalDefaults()
 	if err != nil {

pkg/broker/service_definition_test.go

@@ -14,6 +14,7 @@ import (
 )
 
 var _ = Describe("ServiceDefinition", func() {
+
 	Describe("CatalogEntry", func() {
 		Context("Metadata", func() {
 			var serviceDefinition broker.ServiceDefinition
@@ -377,10 +378,41 @@ var _ = Describe("ServiceDefinition", func() {
 
 				It("should return an error", func() {
 					_, err := service.UserDefinedPlans(maintenanceInfo)
-					Expect(err).To(MatchError("invalid character 'i' looking for beginning of object key string"))
+					Expect(err).To(MatchError(ContainSubstring("invalid character 'i' looking for beginning of object key string")))
 
 				})
 			})
+			When("a plan is provided in configuration as an object", func() {
+				BeforeEach(func() {
+					// fakePlanName, fakePlanID, fakePlanDescription, fakePlanProperty
+					fakeServicePlanConfigObject := []map[string]interface{}{
+						{
+							"name":                fakePlanName,
+							"id":                  fakePlanID,
+							"description":         fakePlanDescription,
+							"additional_property": fakePlanProperty,
+						},
+						{
+							"name":                fmt.Sprintf("second-%s", fakePlanName),
+							"id":                  fmt.Sprintf("second-%s", fakePlanID),
+							"description":         fmt.Sprintf("second-%s", fakePlanDescription),
+							"additional_property": fmt.Sprintf("second-%s", fakePlanProperty),
+						},
+					}
+					viper.Set("service.fake-service.provision.defaults", map[string]interface{}{
+						"test": "value",
+					})
+					viper.Set("service.fake-service.plans", fakeServicePlanConfigObject)
+				})
+				It("should work", func() {
+					plan, err := service.UserDefinedPlans(maintenanceInfo)
+					Expect(err).To(Not(HaveOccurred()))
+					Expect(plan).To(Not(HaveLen(0)))
+					provisionOverrides, err := service.ProvisionDefaultOverrides()
+					Expect(err).To(Not(HaveOccurred()))
+					Expect(provisionOverrides).To(Equal(map[string]interface{}{"test": `value`}))
+				})
+			})
 
 		})