Add image map wiring #624
Add image map wiring #624
Conversation
This PR adds image map wiring / interpolation into Porter's bag of tricks. Fixes: getporter#501
docs/content/wiring.md
Outdated
| images: | ||
| ALIAS: | ||
| description: A very useful image | ||
| imageType: docker # porter.yaml can default this to docker if we aren't already |
There was a problem hiding this comment.
Should the comment change to say porter is defaulting it to docker (if that's the case)?
docs/content/wiring.md
Outdated
| description: A very useful image | ||
| imageType: docker # porter.yaml can default this to docker if we aren't already | ||
| repository: gcr.io/mcguffin-co/mcguffin | ||
| digest: sha256:85b1a9 # this is what bundle.json allows |
There was a problem hiding this comment.
Should we reference the cnab spec around the bundle.json file here?
docs/content/wiring.md
Outdated
| imageType: docker # porter.yaml can default this to docker if we aren't already | ||
| repository: gcr.io/mcguffin-co/mcguffin | ||
| digest: sha256:85b1a9 # this is what bundle.json allows | ||
| tag: v1.1.0 # we can collect this and make it available but it won't land into bundle.json |
There was a problem hiding this comment.
It won't land into bundle.json b/c the digest sha is provided? Also, is the digest key/value mandatory or optional? (Or more generally, is it worth explicitly mentioning which fields are mandatory?)
There was a problem hiding this comment.
I think this is probably better in a separate doc about images? What say you @carolynvs. We added that but never really documented it (there was a comment that we did nothing with it before). I was going to open a more general "document how to use images" issue that we could give best practice info in and explain how to use for Helm charts, etc.
pkg/config/manifest.go
Outdated
| // plan on manually using this data in your own scripts. | ||
| ImageMap map[string]MappedImage `yaml:"imageMap,omitempty"` | ||
| // ImageMap is a map of images referenced in the bundle. If image relocation mapping occurs, that | ||
| // file will be mounted at as a file at runtime to /cnab/app/relocation-mapping.json. |
There was a problem hiding this comment.
Extra at in ...file will be mounted at as a file...
docs/content/wiring.md
Outdated
| @@ -3,9 +3,9 @@ title: Using Parameters, Credentials and Outputs | |||
| description: How to wire parameters, credentials and outputs into steps | |||
| --- | |||
|
|
|||
| # Parameters, Credentials and Outputs in Porter | |||
| # Parameters, Credentials, Outputs, and Images in Porter | |||
There was a problem hiding this comment.
Can you update this to be the title of the article instead in the front matter and remove this duplicate heading?
There was a problem hiding this comment.
yes indeed. it will require a change. thanks for catching that!
I'll update the headline in wiring.md too. Good suggestion.
|
Updated the docs and the workshop, however, I couldn't verify the bundle since there is an |
|
@carolynvs @vdice I ran this locally again and it works. LMK if it's good to merge or if you want to take another look! |
What does this change
This PR adds image map wiring / interpolation into Porter's bag of tricks. It changes the
imageMappart of the porter.yaml toimagesto match the issue comments and does some various cleanup.What issue does it fix
Closes #501
Notes
Since mustache template interpolation is case sensitive, I decided to use
reflectto iterate the MappedImage struct andstrings.ToLowerthe name of the field, that way it matches up against the yaml. Otherwise, we ended up with references like{{bundle.images.thing.Repository}}, which bugged me.Checklist