ship default vals platform behavior (#205)

* ship default vals platform behavior

* build

* Update integration tests to use app command (#204)

* ship default vals platform behavior

* build

integration/base/basic-stateless/expected/.ship/state.json

@@ -1 +1 @@
-{"v1":{"config":{"test_option":""}}}
+{"v1":{"config":{}}}

integration/base/default-values/expected/.ship/release.yml

@@ -0,0 +1,47 @@
+
+---
+assets:
+  v1:
+  - inline:
+      contents: |
+        #!/bin/sh
+        echo namespace is {{repl ConfigOption "namespace"}}
+        echo cluster is {{repl ConfigOption "cluster"}}
+        echo ingress controller is {{repl ConfigOption "ingress"}}
+        echo scheduler is {{repl ConfigOption "scheduler"}}
+        echo environment is {{repl ConfigOption "environment"}}
+        echo pod is {{repl ConfigOption "pod"}}
+      dest: ./scripts/echo.sh
+      mode: 0777
+
+config:
+  v1:
+    - name: default-values-bug
+      description: information about your kubernetes cluster
+      items:
+        # Customer-value provided, no vendor-default: should use customer-provided value and write namespace:customer-value to `state.json`
+      - name: namespace
+        type: text
+        # Customer-value provided, vendor-default provided: should use customer-provided value and write cluster:customer-value to `state.json`
+      - name: cluster
+        type: text
+        default: Bravo
+        # No customer-value, vendor-default provided: should use vendor-default and NOT render ingress to `state.json`
+      - name: ingress
+        type: text
+        default: Charlie
+        # Empty value in state.json, vendor-default: should use empty value and write scheduler:"" to state.json
+      - name: scheduler
+        type: text
+        default: Delta
+        # Vendor-supplied default == customer-supplied value: should use customer-supplied value and write environment:customer-value to `state.json`
+      - name: environment
+        type: text
+        default: Epsilon
+        # No customer-value, no vendor-default: should use <> and write pod:<> to `state.json`
+      - name: pod
+        type: text
+
+lifecycle:
+  v1:
+  - render: {}

integration/base/default-values/expected/.ship/state.json

@@ -0,0 +1 @@
+{"v1":{"config":{"cluster":"Ovarb","environment":"Epsilon","namespace":"Alpha","scheduler":""}}}

integration/base/default-values/expected/installer/scripts/echo.sh

@@ -0,0 +1,7 @@
+#!/bin/sh
+echo namespace is Alpha
+echo cluster is Ovarb
+echo ingress controller is Charlie
+echo scheduler is 
+echo environment is Epsilon
+echo pod is 

integration/base/default-values/input/.ship/release.yml

@@ -0,0 +1,47 @@
+
+---
+assets:
+  v1:
+  - inline:
+      contents: |
+        #!/bin/sh
+        echo namespace is {{repl ConfigOption "namespace"}}
+        echo cluster is {{repl ConfigOption "cluster"}}
+        echo ingress controller is {{repl ConfigOption "ingress"}}
+        echo scheduler is {{repl ConfigOption "scheduler"}}
+        echo environment is {{repl ConfigOption "environment"}}
+        echo pod is {{repl ConfigOption "pod"}}
+      dest: ./scripts/echo.sh
+      mode: 0777
+
+config:
+  v1:
+    - name: default-values-bug
+      description: information about your kubernetes cluster
+      items:
+        # Customer-value provided, no vendor-default: should use customer-provided value and write namespace:customer-value to `state.json`
+      - name: namespace
+        type: text
+        # Customer-value provided, vendor-default provided: should use customer-provided value and write cluster:customer-value to `state.json`
+      - name: cluster
+        type: text
+        default: Bravo
+        # No customer-value, vendor-default provided: should use vendor-default and NOT render ingress to `state.json`
+      - name: ingress
+        type: text
+        default: Charlie
+        # Empty value in state.json, vendor-default: should use empty value and write scheduler:"" to state.json
+      - name: scheduler
+        type: text
+        default: Delta
+        # Vendor-supplied default == customer-supplied value: should use customer-supplied value and write environment:customer-value to `state.json`
+      - name: environment
+        type: text
+        default: Epsilon
+        # No customer-value, no vendor-default: should use <> and write pod:<> to `state.json`
+      - name: pod
+        type: text
+
+lifecycle:
+  v1:
+  - render: {}

integration/base/default-values/input/.ship/state.json

@@ -0,0 +1 @@
+{"v1":{"config":{"cluster":"Ovarb","environment":"Epsilon","namespace":"Alpha","scheduler":""}}}

integration/base/default-values/metadata.yaml

@@ -0,0 +1,3 @@
+customer_id: "-Am-_6i5pw0u4AbspOwKN4lZUCn49u_G"
+installation_id: "PYRBRKHcTSbMPFJqz3a_82p_kR00DPdz"
+release_version: "0.1.4"

pkg/lifecycle/daemon/daemon.go

@@ -550,7 +550,7 @@ func (d *ShipDaemon) postAppConfigLive(release *api.Release) gin.HandlerFunc {
 		}
 
 		debug.Log("event", "resolveConfig")
-		resolvedConfig, err := d.ConfigRenderer.ResolveConfig(c, release, savedSate.CurrentConfig(), liveValues)
+		resolvedConfig, err := d.ConfigRenderer.ResolveConfig(c, release, savedSate.CurrentConfig(), liveValues, true)
 		if err != nil {
 			level.Error(d.Logger).Log("event", "resolveconfig failed", "err", err)
 			c.AbortWithStatus(500)
@@ -620,7 +620,7 @@ func (d *ShipDaemon) putAppConfig(release *api.Release) gin.HandlerFunc {
 		}
 
 		debug.Log("event", "resolveConfig")
-		resolvedConfig, err := d.ConfigRenderer.ResolveConfig(c, release, savedState.CurrentConfig(), liveValues)
+		resolvedConfig, err := d.ConfigRenderer.ResolveConfig(c, release, savedState.CurrentConfig(), liveValues, false)
 		if err != nil {
 			level.Error(d.Logger).Log("event", "resolveconfig failed", "err", err)
 			c.AbortWithStatus(500)

pkg/lifecycle/daemon/headless_daemon.go

@@ -96,7 +96,7 @@ func (d *HeadlessDaemon) HeadlessResolve(ctx context.Context, release *api.Relea
 	warn := level.Warn(log.With(d.Logger, "struct", "fakeDaemon", "method", "HeadlessResolve"))
 	currentConfig := d.GetCurrentConfig()
 
-	resolved, err := d.ConfigRenderer.ResolveConfig(ctx, release, currentConfig, make(map[string]interface{}))
+	resolved, err := d.ConfigRenderer.ResolveConfig(ctx, release, currentConfig, make(map[string]interface{}), false)
 	if err != nil {
 		warn.Log("event", "resolveconfig failed", "err", err)
 		return err

pkg/lifecycle/render/config/resolve/api.go

@@ -67,8 +67,17 @@ func isReadOnly(item *libyaml.ConfigItem) bool {
 	return !editable
 }
 
-func shouldOverrideValueWithDefault(item *libyaml.ConfigItem) bool {
-	return item.Hidden && item.Value == "" && item.Default != ""
+func (r *APIConfigRenderer) shouldOverrideValueWithDefault(item *libyaml.ConfigItem, savedState map[string]interface{}, firstPass bool) bool {
+	// resolve config runs before values are saved in interactive mode.
+	// this first pass should override any hidden, empty values with
+	// non-empty defaults
+	if firstPass {
+		return item.Hidden && item.Value == "" && item.Default != ""
+	} else {
+		// vendor can't override a default with "" in interactive mode
+		_, ok := savedState[item.Name]
+		return !ok && item.Value == "" && item.Default != ""
+	}
 }
 
 func isRequired(item *libyaml.ConfigItem) bool {
@@ -195,6 +204,7 @@ func (r *APIConfigRenderer) ResolveConfig(
 	release *api.Release,
 	savedState map[string]interface{},
 	liveValues map[string]interface{},
+	firstPass bool,
 ) ([]libyaml.ConfigGroup, error) {
 	resolvedConfig := make([]libyaml.ConfigGroup, 0, 0)
 	configCopy, err := r.deepCopyConfig(release.Spec.Config.V1)
@@ -237,7 +247,7 @@ func (r *APIConfigRenderer) ResolveConfig(
 				return resolvedConfig, errors.Wrapf(err, "resolve item %s", configItem.Name)
 			}
 
-			if shouldOverrideValueWithDefault(configItem) {
+			if r.shouldOverrideValueWithDefault(configItem, savedState, firstPass) {
 				configItem.Value = configItem.Default
 			}
 

pkg/lifecycle/render/config/resolve/api_test.go

@@ -112,11 +112,11 @@ func TestAPIResolver(t *testing.T) {
 				var err error
 
 				if len(test.LiveValues) == 0 {
-					resolvedConfig, err = resolver.ResolveConfig(ctx, release, test.State, make(map[string]interface{}))
+					resolvedConfig, err = resolver.ResolveConfig(ctx, release, test.State, make(map[string]interface{}), true)
 				} else {
 					// simulate multiple inputs
 					for _, liveValues := range test.LiveValues {
-						resolvedConfig, err = resolver.ResolveConfig(ctx, release, test.State, liveValues)
+						resolvedConfig, err = resolver.ResolveConfig(ctx, release, test.State, liveValues, false)
 					}
 				}
 

pkg/lifecycle/render/config/resolve/test-cases/api/defaults.yml

@@ -34,7 +34,7 @@
                         "title": "ChoiceOne",
                         "type": "text",
                         "default": "vendor supplied value",
-                        "value": "",
+                        "value": "vendor supplied value",
                         "affix": "",
                         "data_cmd": null,
                         "default_cmd": null,
@@ -92,7 +92,7 @@
                         "title": "ChoiceThree",
                         "type": "text",
                         "default": "3",
-                        "value": "",
+                        "value": "3",
                         "affix": "",
                         "data_cmd": null,
                         "default_cmd": null,

pkg/lifecycle/render/render.go

@@ -10,6 +10,7 @@ import (
 	"github.com/go-kit/kit/log/level"
 	"github.com/mitchellh/cli"
 	"github.com/pkg/errors"
+	"github.com/replicatedhq/libyaml"
 	"github.com/replicatedhq/ship/pkg/api"
 	"github.com/replicatedhq/ship/pkg/constants"
 	"github.com/replicatedhq/ship/pkg/lifecycle/daemon"
@@ -107,8 +108,23 @@ func (r *Renderer) Execute(ctx context.Context, release *api.Release, step *api.
 		return errors.Wrap(err, "execute plan")
 	}
 
+	stateTemplateContext := make(map[string]interface{})
+	for _, configGroup := range release.Spec.Config.V1 {
+		for _, configItem := range configGroup.Items {
+			if valueNotOverridenByDefault(configItem, templateContext, previousState.CurrentConfig()) {
+				stateTemplateContext[configItem.Name] = templateContext[configItem.Name]
+			}
+		}
+	}
+
+	// edge case: empty config section of app yaml,
+	// persist data from previous state.json
+	if len(release.Spec.Config.V1) == 0 {
+		stateTemplateContext = templateContext
+	}
+
 	r.Daemon.SetProgress(ProgressCommit)
-	if err := r.StateManager.Serialize(release.Spec.Assets.V1, release.Metadata, templateContext); err != nil {
+	if err := r.StateManager.Serialize(release.Spec.Assets.V1, release.Metadata, stateTemplateContext); err != nil {
 		return errors.Wrap(err, "serialize state")
 	}
 
@@ -135,3 +151,23 @@ func (r *Renderer) backupIfPresent(basePath string) error {
 
 	return nil
 }
+
+func valueNotOverridenByDefault(item *libyaml.ConfigItem, templateContext map[string]interface{}, savedState map[string]interface{}) bool {
+	_, inSavedState := savedState[item.Name] // all values in savedState are non-default values
+
+	if templateContext[item.Name] == "" {
+		if inSavedState && savedState[item.Name] == "" {
+			// manually set value: "" in state.json
+			return true
+		} else {
+			// value overriden by default == ""?
+			return item.Default != ""
+		}
+	} else if templateContext[item.Name] == item.Default {
+		// the provided value is manually set to the default value
+		return inSavedState
+	} else {
+		// non-empty value != default. cannot have been overriden by default
+		return true
+	}
+}