Fix options migration, implement proxy options (#41)

* Replace - with _ for env mapping

* Move secrets-config proxy code to options package

* config.enabled toggle and add proxy options migration

* Implement processing new proxy options

* Add basic proxy option validation

* Remove implementation comments

* Remove proxy option migration

* Deprecate exported proxy functions

* Update tests

* Export log init to re-initialize debug mode in external tests

* Fix tests

* Conditional config-enable warning

* Decouple config and custom option processing

log/init.go

@@ -13,10 +13,10 @@ var (
 )
 
 func init() {
-	initialize()
+	Init()
 }
 
-func initialize() {
+func Init() {
 	value, err := exec.Command("snapctl", "get", "debug").CombinedOutput()
 	if err != nil {
 		stderr(err)

log/init_test.go

@@ -33,13 +33,13 @@ func TestInitialize(t *testing.T) {
 		// set it to true and check
 		output, err := exec.Command("snapctl", "set", "debug=true").CombinedOutput()
 		assert.NoError(t, err, "Error setting config value via snapctl: %s", output)
-		initialize()
+		Init()
 		require.True(t, debug)
 
 		// unset and re-check
 		output, err = exec.Command("snapctl", "unset", "debug").CombinedOutput()
 		assert.NoError(t, err, "Error setting config value via snapctl: %s", output)
-		initialize()
+		Init()
 		require.False(t, debug)
 	})
 

options/env.go

@@ -43,8 +43,10 @@ func getEnvVarFile(service string) *envVarOverrides {
 
 func (e *envVarOverrides) setEnvVariable(setting string, value string) error {
 	result := strings.ToUpper(setting)
-	result = strings.Replace(result, "-", "", -1)
-	result = strings.Replace(result, ".", "_", -1)
+	// replace - with _ for keys such as add-known-secrets and edgex-startup-duration
+	result = strings.ReplaceAll(result, "-", "_")
+	// replace . with _ for config file overrides such as service.port
+	result = strings.ReplaceAll(result, ".", "_")
 	log.Infof("Mapping %s to %s", setting, result)
 	_, err := fmt.Fprintf(e.buffer, "export %s=%s\n", result, value)
 	return err
@@ -74,7 +76,7 @@ func (e *envVarOverrides) writeEnvFile(append bool) error {
 	}
 	buf.Write(e.buffer.Bytes())
 
-	log.Infof("Writing settings to %s", e.filename)
+	log.Infof("Writing settings to %s: %s", e.filename, strings.ReplaceAll(e.buffer.String(), "\n", " "))
 
 	tmp := e.filename + ".tmp"
 	err := ioutil.WriteFile(tmp, buf.Bytes(), 0644)

options/options.go

@@ -22,13 +22,16 @@ import (
 	"encoding/json"
 	"fmt"
 
+	"github.com/canonical/edgex-snap-hooks/v2/env"
 	"github.com/canonical/edgex-snap-hooks/v2/log"
 	"github.com/canonical/edgex-snap-hooks/v2/snapctl"
 )
 
+type configOptions map[string]interface{}
+
 type snapOptions struct {
-	Apps   map[string]map[string]map[string]interface{} `json:"apps"`
-	Config map[string]interface{}                       `json:"config"`
+	Apps   map[string]map[string]configOptions `json:"apps"`
+	Config configOptions                       `json:"config"`
 }
 
 func getConfigMap(config map[string]interface{}) (map[string]string, error) {
@@ -58,7 +61,8 @@ func processGlobalConfigOptions(services []string) error {
 
 	if options.Config == nil {
 		log.Debugf("No global configuration settings")
-		return nil
+		// return nil
+		// continue to empty the files
 	}
 
 	configuration, err := getConfigMap(options.Config)
@@ -75,16 +79,14 @@ func processGlobalConfigOptions(services []string) error {
 	return nil
 }
 
-func migrateLegacyOptions() error {
-
-	clear := []string{"env.security-bootstrapper", "env.security-secret-store"}
+func migrateLegacyInternalOptions() error {
 
 	namespaceMap := map[string]string{
 		"env.security-secret-store.add-secretstore-tokens": "apps.security-secretstore-setup.config.add-secretstore-tokens",
 		"env.security-secret-store.add-known-secrets":      "apps.security-secretstore-setup.config.add-known-secrets",
-		"env.security-bootstrapper.add-registry-acl-roles": "apps.security-bootstrapper.config.add-registry-acl-roles"}
+		"env.security-bootstrapper.add-registry-acl-roles": "apps.security-bootstrapper.config.add-registry-acl-roles",
+	}
 
-	migrated := false
 	for k, v := range namespaceMap {
 		setting, err := snapctl.Get(k).Run()
 		if err != nil {
@@ -94,18 +96,51 @@ func migrateLegacyOptions() error {
 			if err := snapctl.Unset(k).Run(); err != nil {
 				return err
 			}
+
 			if err := snapctl.Set(v, setting).Run(); err != nil {
 				return err
 			}
 			log.Debugf("Migrated %s to %s", k, v)
-			migrated = true
 		}
 	}
 
-	if migrated {
-		for _, s := range clear {
-			if err := snapctl.Unset(s).Run(); err != nil {
-				return err
+	return nil
+}
+
+// Process the "apps.<app>.<my.option>" options, where <my.option> is not config
+func processAppCustomOptions(service, key string, value configOptions) error {
+	switch service {
+	case "secrets-config":
+		return processSecretsConfigOptions(key, value)
+	default:
+		return fmt.Errorf("Unknown custom option %s for service %s", key, service)
+	}
+}
+
+// Process the "apps.<app>.<custom.option>" where <custom.option> is not "config"
+func ProcessAppCustomOptions(service string) error {
+	var options snapOptions
+
+	// get the 'apps' json structure
+	jsonString, err := snapctl.Get("apps").Document().Run()
+	if err != nil {
+		return err
+	}
+	err = json.Unmarshal([]byte(jsonString), &options)
+	if err != nil {
+		return err
+	}
+
+	log.Debugf("Processing custom options for service: %s", service)
+
+	appOptions := options.Apps[service]
+	log.Debugf("Processing custom options: %v", appOptions)
+	if appOptions != nil {
+		for k, v := range appOptions {
+			if k != "config" {
+				if err := processAppCustomOptions(service, k, v); err != nil {
+					return err
+				}
 			}
 		}
 	}
@@ -127,32 +162,41 @@ func processAppConfigOptions(services []string) error {
 	if err != nil {
 		return err
 	}
+
+	// TODO
+	// reject unknown servies
+
 	// iterate through the known services in this snap
 	for _, service := range services {
-		log.Debugf("Processing service:%s", service)
+		log.Debugf("Processing service: %s", service)
 
 		// get the configuration specified for each service
 		// and create the environment override file
 		appConfig := options.Apps[service]
-		log.Debugf("Processing appConfig:%v", appConfig)
+		log.Debugf("Processing appConfig: %v", appConfig)
 		if appConfig != nil {
-			config := appConfig["config"]
-			log.Debugf("Processing config:%v", config)
-			if config != nil {
-				configuration, err := getConfigMap(config)
+			for k, v := range appConfig {
+				if k == "config" { // config overrides
+					config := v
+					log.Debugf("Processing config: %v", config)
+					if config != nil {
+						configuration, err := getConfigMap(config)
 
-				log.Debugf("Processing configuration:%v", configuration)
-				if err != nil {
-					return err
-				}
-				overrides := getEnvVarFile(service)
+						log.Debugf("Processing configuration: %v", configuration)
+						if err != nil {
+							return err
+						}
+						overrides := getEnvVarFile(service)
 
-				log.Debugf("Processing overrides:%v", overrides)
-				for env, value := range configuration {
-					log.Debugf("Processing overrides setEnvVariable:%v %v", env, value)
-					overrides.setEnvVariable(env, value)
+						log.Debugf("Processing overrides: %v", overrides)
+						for env, value := range configuration {
+
+							log.Debugf("Processing overrides setEnvVariable: %v=%v", env, value)
+							overrides.setEnvVariable(env, value)
+						}
+						overrides.writeEnvFile(true)
+					}
 				}
-				overrides.writeEnvFile(true)
 			}
 		}
 	}
@@ -174,38 +218,66 @@ func ProcessAppConfig(services ...string) error {
 		return fmt.Errorf("empty service list")
 	}
 
-	err := migrateLegacyOptions()
+	configEnabledStr, err := snapctl.Get("config-enabled").Run()
 	if err != nil {
 		return err
 	}
+	configEnabled := (configEnabledStr == "true")
+
+	log.Infof("Processing apps.* and config.* options: %t", configEnabled)
 
 	isSet := func(v string) bool {
 		return !(v == "" || v == "{}")
 	}
 
-	// reject mixed legacy options
-	appsOptions, err := snapctl.Get("apps").Run()
-	if err != nil {
-		return err
+	if !configEnabled {
+		appsOptions, err := snapctl.Get("apps").Run()
+		if err != nil {
+			return err
+		}
+		globalOptions, err := snapctl.Get("config").Run()
+		if err != nil {
+			return err
+		}
+		if isSet(appsOptions) || isSet(globalOptions) {
+			var migratable string
+			if env.SnapName == "edgexfoundry" {
+				migratable = `
+Exception: The following legacy 'env.' options are automatically converted:
+	- env.security-secret-store.add-secretstore-tokens
+	- env.security-secret-store.add-known-secrets
+	- env.security-bootstrapper.add-registry-acl-roles`
+			}
+			return fmt.Errorf("'config.' and 'apps.' options are allowed only when config-enabled is true.\n\n%s%s",
+				"WARNING: Setting config-enabled=true will unset existing 'env.' options and ignore future sets!!",
+				migratable)
+
+		} else {
+			log.Debug("No config options are set.")
+			// return and continue with legacy option handling.
+			return nil
+		}
 	}
-	globalOptions, err := snapctl.Get("config").Run()
+
+	err = migrateLegacyInternalOptions()
 	if err != nil {
 		return err
 	}
-	envOptions, err := snapctl.Get("env").Run()
-	if err != nil {
+
+	// It is important to unset any options to avoid conflicts in
+	// 	deprecated configure hook processing
+	if err := snapctl.Unset("env").Run(); err != nil {
 		return err
 	}
-	if isSet(envOptions) &&
-		(isSet(appsOptions) || isSet(globalOptions)) {
-		return fmt.Errorf("'config.' and 'app.' options must not be mixed with legacy 'env.' options: %s",
-			envOptions)
-	}
+	log.Info("Unset all 'env.' options.")
 
 	if err := processGlobalConfigOptions(services); err != nil {
 		return err
 	}
 
+	// The app-specific options have higher precedence.
+	// They should be processed last to end up at the bottom of the .env file
+	// 	and override global environment variables.
 	if err := processAppConfigOptions(services); err != nil {
 		return err
 	}