use mustache to wire up params, creds, outputs #264

jeremyrickard · 2019-04-12T16:27:51Z

This PR changes how we wire params, credentials and outputs between steps. Instead of the previous "source:" bundle.x.y.z, now we use Mustache templates.

Introduce
Update docs
Fix e2e tests

Closes #161

jeremyrickard · 2019-04-15T15:08:02Z

@carolynvs @vdice take a look.

The only thing I'm kind of bummed about in this is that the mustache templates need to be quoted, otherwise the go marshalling isn't happy. Maybe we need to change how the marshalling works and only marshall the steps after they've been fully resolved? This seems like a big enough change to review on it's own though, tackle that in another refactor.

carolynvs · 2019-04-15T15:26:48Z

I'm not sure I see how to resolve before we've parsed the file, but you are closer than I am to it for sure. Yeah let's go with this for now and iterate.

jeremyrickard · 2019-04-15T17:35:42Z

I'm not sure I see how to resolve before we've parsed the file, but you are closer than I am to it for sure. Yeah let's go with this for now and iterate.

I was thinking more that we wouldn't fully try and rehydrate a struct with the yaml marsheller, but I don't know if that's possible. just thinking out loud

carolynvs · 2019-04-15T18:13:45Z

build/testdata/bundles/mysql/porter.yaml

-        source: bundle.parameters.database-name
-      mysqlUser:
-        source: bundle.parameters.mysql-user
+      mysqlDatabase: "{{ bundle.parameters.database-name}}"


carolynvs · 2019-04-15T18:14:21Z

docs/content/authoring-bundles.md

-        source: bundle.parameters.database-name
-      mysqlUser:
-        source: bundle.parameters.mysql-user
+      mysqlDatabase: {{ bundle.parameters.database-name }}


should these have quotes?

carolynvs · 2019-04-15T18:14:43Z

docs/content/mixin-architecture.md

-      source: bundle.parameters.database-name
-    mysqlUser:
-      source: bundle.parameters.mysql-user
+    mysqlDatabase: {{ bundle.parameters.database-name }}


should these have quotes?

carolynvs · 2019-04-15T18:15:03Z

docs/content/porter-or-duffle.md

@@ -95,23 +95,18 @@ install:
          key: mysql-password
  - helm:
      description: "Install Wordpress"
-      name:
-        source: bundle.parameters.wordpress-name
+      name: {{ bundle.parameters.wordpress-name }}


Seems like all the markdown files are missing quotes around the moustash template

Yeah, I forgot to update them! Thanks for pointing that out.

carolynvs · 2019-04-15T18:15:57Z

docs/content/wiring.md

@@ -29,7 +29,7 @@ You can also provide any other attributes, as specified by the CNAB [parameters]

 ## Wiring Parameters

-Once a parameter has been declared in the `porter.yaml`, Porter provides a simple mechanism for specifying how the parameter value should be wired into the mixin. To reference a parameter in the bundle, you use a notation of the form `source: bundle.parameters.PARAM_NAME`. This can be used anywhere within step definition. For example, to use the `database_name` parameter defined above in the `set` block of the helm mixin:
+Once a parameter has been declared in the `porter.yaml`, Porter provides a simple mechanism for specifying how the parameter value should be wired into the mixin. To reference a parameter in the bundle, you use a notation of the form `{{ bundle.parameters.PARAM_NAME }}`. This can be used anywhere within step definition. For example, to use the `database_name` parameter defined above in the `set` block of the helm mixin:


we should mention the surrounding quotes

carolynvs · 2019-04-15T18:28:13Z

pkg/config/manifest.go

+	data := make(map[string]interface{})
+	m.sensitiveValues = []string{}
+	bundle := make(map[string]interface{})
+	data["bundle"] = bundle


This is a question about mustache. Looks like we are building up a map that has keys for parameters, outputs, etc inside of it, then that's what the template is operating upon.

If we had a struct called "TemplateMagicPants" (or whatever) and it had fields for parameters, outputs, credentials, and everything else we needed, would that work too as input data to mustache? Or would that result in different syntax in the template?

This is just a question for me to understand mustache, not asking you to change anything. 😀

We'd need to have a struct called TemplateMagicPants that had those, that lived in a map with the key bundle. Or another struct. It's walking what ever the template syntax is. I was trying to keep it mostly the same, and I think the comment from our Amazon friends used similar syntax.

I did find myself wondering what the point of the "bundle" is. everything is rooted off that....soooooo maybe just {{ parameters.x }} or {{ outputs.y }} or {{ credentials.z }} or {{ dependencies.x.y.z }}

I was thinking along these lines for the general refactor of "the manifest" from the running state of things, but here I didn't want to change the syntax of the template since we hadn't chatted about that. Happy to go either way, but I think my opinion is the "bundle" key is not really needed?

Let's not remove the bundle. prefix for now. I believe that it is important for both giving people a starting point in their heads for what to type, and also for the autocomplete to have something to go off of.

carolynvs · 2019-04-15T18:31:19Z

pkg/config/manifest.go

+			return nil, err
+		}
+		if param.Sensitive {
+			m.sensitiveValues = append(m.sensitiveValues, val)


This isn't on you, but the sensitive values stuff is in desperate need of a refactor so that it's not tied to the manifest like this. I can't wait for your work to split out the runtime from the manifest as I believe that's something we identified as being helpful in order to do the masking in a less obtrusive way going forward. 🤞

carolynvs · 2019-04-15T18:33:26Z

pkg/config/manifest.go

+	creds := make(map[string]interface{})
+	bundle["credentials"] = creds
+	for _, cred := range m.Credentials {
+		//pe := strings.ToUpper(cred.Name)


can we nuke this comment?

carolynvs · 2019-04-15T18:38:09Z

pkg/config/manifest.go

-		return nil, errors.New(fmt.Sprintf("no value found for source specification: %s", key))
+	payload, err := yaml.Marshal(step)
+	if err != nil {
+		return errors.Wrap(err, "unable to resolve step")


I'm not sure wrapping all these various errors with the same error message is going to be helpful? Having a bit more context about what was going on may be useful when dealing with errors like "unable to resolve step: unexpected EOF", you know?

carolynvs · 2019-04-15T18:38:44Z

pkg/config/manifest.go

 	}
-	return replacement, nil
+	return yaml.Unmarshal([]byte(rendered), step)


this last error isn't being wrapped

carolynvs

such quotes, much wow 🐶

carolynvs · 2019-04-16T16:25:04Z

Merge at will

ghost assigned jeremyrickard Apr 12, 2019

ghost added the review label Apr 12, 2019

jeremyrickard added 3 commits April 12, 2019 15:36

Use mustache to wire up params, creds, outputs

77cbd12

Updated YAML files for e2e stuff

67ceb9d

Doc fixup

d7e4ddf

jeremyrickard force-pushed the redo-sourcing branch from 62bfe10 to d7e4ddf Compare April 12, 2019 21:37

jeremyrickard added 3 commits April 12, 2019 15:47

Are you happy now, Dep?!

7b99d94

Quotes

dd2df81

That will teach me to rebase and not look at the updated test data

5d64385

jeremyrickard changed the title ~~WIP: use mustache to wire up params, creds, outputs~~ use mustache to wire up params, creds, outputs Apr 15, 2019

carolynvs reviewed Apr 15, 2019

View reviewed changes

Fixup for code review comments

a0ca8d3

carolynvs approved these changes Apr 16, 2019

View reviewed changes

vdice approved these changes Apr 16, 2019

View reviewed changes

jeremyrickard merged commit 3db86bc into getporter:master Apr 16, 2019

ghost removed the review label Apr 16, 2019

vdice mentioned this pull request Apr 16, 2019

fix(*): remove last of the source references #275

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

use mustache to wire up params, creds, outputs #264

use mustache to wire up params, creds, outputs #264

jeremyrickard commented Apr 12, 2019 •

edited

jeremyrickard commented Apr 15, 2019

carolynvs commented Apr 15, 2019

jeremyrickard commented Apr 15, 2019

carolynvs Apr 15, 2019

carolynvs Apr 15, 2019

carolynvs Apr 15, 2019

carolynvs Apr 15, 2019

jeremyrickard Apr 16, 2019

carolynvs Apr 15, 2019

carolynvs Apr 15, 2019

jeremyrickard Apr 16, 2019

carolynvs Apr 16, 2019

carolynvs Apr 15, 2019

carolynvs Apr 15, 2019

carolynvs Apr 15, 2019

carolynvs Apr 15, 2019

carolynvs left a comment

carolynvs commented Apr 16, 2019

use mustache to wire up params, creds, outputs #264

use mustache to wire up params, creds, outputs #264

Conversation

jeremyrickard commented Apr 12, 2019 • edited

jeremyrickard commented Apr 15, 2019

carolynvs commented Apr 15, 2019

jeremyrickard commented Apr 15, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

carolynvs left a comment

Choose a reason for hiding this comment

carolynvs commented Apr 16, 2019

jeremyrickard commented Apr 12, 2019 •

edited