Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Generate and resolve parameter sources from templating #1158

Merged

Conversation

carolynvs
Copy link
Member

@carolynvs carolynvs commented Jul 23, 2020

What does this change

When the manifest uses an output from a previous action, either through templating or by directly injecting the ourput int a parameter, generate a parameter source. Then for templated outputs resolve them to the injected parameter source value.

What issue does it fix

Closes #1067

Notes for the reviewer

I'm going to need follow-up PR to really dig into how we want to support or gracefully handle how applyTo could impact whether or not an output is available.

Checklist

  • Unit Tests
  • Documentation
  • Schema (porter.yaml)

@carolynvs carolynvs added this to In Progress in Porter and Mixins [OLD] via automation Jul 23, 2020
@carolynvs carolynvs changed the base branch from main to release/v0.28.0 July 23, 2020 19:45
@carolynvs carolynvs force-pushed the template-to-param-source branch 2 times, most recently from 308da0a to 6398799 Compare July 24, 2020 13:17
* Determine required extensions based on what has been populated in the
bundle.json, not by what is in the manifest. We know it is required
because we have already populated data for that extension.
* Some additional bundle.bundle pointer to value cleanup.
* Added extension.HasDependencies and extension.HasParameterSources
functions so we can check if we should attempt to read extension data or
not from a bundle.
Consolidate logic for the wiring name or identifying outputs in the
manifest so that we can reuse it between the bundle adapter and the
runtime.
Test out the change in our exec-outputs example
@carolynvs carolynvs marked this pull request as ready for review July 24, 2020 14:59
Copy link
Member

@vdice vdice left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All looking good! A few minor comments.

@@ -224,6 +223,7 @@ func (c *ManifestConverter) buildDefaultPorterParameters() []manifest.ParameterD
EnvironmentVariable: "PORTER_DEBUG",
},
Schema: definition.Schema{
ID: "https://porter.sh/schema/bundle.json#porter-debug",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this URL intended to point to a valid resource? I'm seeing a 404.

Copy link
Member Author

@carolynvs carolynvs Jul 24, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, it isn't meant to be a resolvable URI. Just a way for us to say "porter made this" without relying on just the name porter-*. In the future we could publish something and explain our bundle.json though.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The wording in the docs is a bit ambiguous on this detail ("This provides a unique identifier for the schema, as well as, in most cases, indicating where it may be downloaded."). I know for the CNAB schemas we've gotten in the habit of populating id with a resolvable URI. That being said, I'd be fine revisiting this detail at a later date.

pkg/cnab/config-adapter/adapter.go Show resolved Hide resolved
pkg/cnab/config-adapter/adapter.go Outdated Show resolved Hide resolved
pkg/cnab/extensions/parameter_sources.go Outdated Show resolved Hide resolved
pkg/runtime/runtime-manifest.go Outdated Show resolved Hide resolved
Have the exec-outputs example consume an ouput as a parameter then
modify it, similar to how terraform will do so with tfstate.
@carolynvs
Copy link
Member Author

After speaking with Vaughn, I am going to follow-up and create a page at https://porter.sh/generated-bundle to make the URIs resolve and explain those generated parts of the bundle further.

@carolynvs carolynvs merged commit 514b830 into getporter:release/v0.28.0 Jul 24, 2020
Porter and Mixins [OLD] automation moved this from In Progress to Done Jul 24, 2020
@carolynvs carolynvs deleted the template-to-param-source branch July 24, 2020 21:09
carolynvs added a commit that referenced this pull request Jul 27, 2020
* List all pages in a section

The template was assuming a paginator, but our template
doesn't use one, so print all pages

* Put release branches in a "directory" structure

We are using release/VERSION instead of release-VERSION, to take
advantage of tooling that collapses releases.

Allow for triggering tests against other gitflow named branches as well.

* Rename instances to installations (#1102)

* fixes to install scripts for path set up

* fixes to install scripts for path set up

* instances to installations change

* updates

* Update show.go

Accidentally changed showInstallations to showInstances but now changed it back

* claim to installation fixes

* claim to installation change in invoke_test.go and parameters_test.go

* Use latest claims code in cnab-go (#1065)

* Use latest claims code in cnab-go

This gets porter using the latest claims spec that is now supported in
cnab-go. It doesn't take advantage yet of new features that we could do
with the additional data, just ensures that we still build and work.

* Apply suggestions from code review

Co-authored-by: Vaughn Dice <vadice@microsoft.com>

* Fix test-cli

* Set PORTER_HOME in integration tests on plugins

The integration tests don't use the PORTER_HOME env var, they
programmatically set the porter home directory on the porter app. So
when Porter creates a plugin, we need to explicitly set the home dir on
the plugin when it's instantiated since it won't get PORTER_HOME
automatically.

* Fix show instance status

Bump cnab-go to get fix for displaying instance status. We need the
entire installation history so that we have the installation time (the
first claim), and ReadInstallation needs to read claims AND results.

* Update slide reference to claim file

* Resolve merge conflicts with paramset feature

* Incorporate review feedback

* Change error handling when we can't find the install time
* Move definition of common runtime functions
* Remove accidental early return

* Consolidate logic for cnab provider

ONE FUNCTION TO RULE THEM ALL!

* Move UseFilesystem onto TestContext

* bump cnab-go

* Final cnab-go bump v0.13.0-beta1

Co-authored-by: Vaughn Dice <vadice@microsoft.com>

* Fix TestDependencySolver_ResolveDependencies Flake

We were relying on an array order that was generated from iterating
upon a map. So it was unreliable in our tests. This ensures that
we are looking at the correct elements when evaluating the test results.

* Claim data migration (#1090)

* Migrate to new claim layout on read/list

When we request claim data, detect if it's the old layout and
automatically migrate all the claim data to the new layout. At the same
time detect older schema (e.g. when claim.Installation was claim.Name)
and handle that migration too.

When we cut to v1.0 we will stop supporting older claim data stores.

* Use a schema file at the root of home

Place a schema file at the root of home, schema.json, that says the
layout of the files used so we can detect if we should migrate.

{
  "claims": "CLAIM SCHEMA"
}

* Fill in the credentials spec version

* Param source (#1129)

* Refactor before parameter sources feature

* Add action to ActionArguments
* Replace cnab Install/Upgrade/Invoke/Uninstall with Execute
* Use claim.Action* from cnab-go instead of our own definition
  with manifest.Action*
* Fix loadParameters to apply string -> type conversions for all
parameters, not just those supplied on the command-line.

* Apply parameter sources when executing a bundle

When executing a bundle, look for the parameter sources extension and
apply any defined (we only support outputs as sources for now) and use
them as parameter values. The precedence order is:

0. user defined overrides on command-line
1. parameter sets
2. parameter sources
3. bundle defaults

* Use CreateClaim helper to shorten tests

* Incorporate review feedback

* Show installation history

Print out the history of what has happened to a bundle installation,
instead of just the last action when you run `porter show`.

I've added a new printer method that prints out a table section and I'll
follow up and redo other sections so that they use it too.

* Hold open plugin connection during schema migration (#1155)

* Hold open plugin connection during schema migration

* Make sure we don't open/close the plugin connection during the schema
migration check.
* Bubble through the Connect call to the wrapped backing store.

* Add failing test for panic during migration

* Reuse backing store during claims migration

Use the same backing store for manual access and for the claim store
itself so that we can better manage the connection, i.e. connecting one
connects the other.

* Trigger build all branches (#1157)

* Fix running integration tests for any branch

The * only matches a single directory deep. Removing the pr
configuration entirely triggers builds for all branches.

* Fix failing test

The test setup should ignore removing a directory that doesn't exist on
the CI server.

* Generate and resolve parameter sources from templating (#1158)

* Refactor before adding parameter sources

* Determine required extensions based on what has been populated in the
bundle.json, not by what is in the manifest. We know it is required
because we have already populated data for that extension.
* Some additional bundle.bundle pointer to value cleanup.
* Added extension.HasDependencies and extension.HasParameterSources
functions so we can check if we should attempt to read extension data or
not from a bundle.

* Generate parameter sources from porter templating

* Directly connect outputs to parameters in the manifest

* Consolidate logic in manifest

Consolidate logic for the wiring name or identifying outputs in the
manifest so that we can reuse it between the bundle adapter and the
runtime.

* Document parameter source in manifest

* Handle file type parameters in parameter sources

Test out the change in our exec-outputs example

* Simplify tests

* Update exec-outputs example to modify parameter-source

Have the exec-outputs example consume an ouput as a parameter then
modify it, similar to how terraform will do so with tfstate.

* Incorporate review feedback

Co-authored-by: gaurimadhok <gmadhok@usc.edu>
Co-authored-by: Vaughn Dice <vadice@microsoft.com>
@vdice vdice removed this from Done in Porter and Mixins [OLD] Sep 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Generate parameter sources for outputs from the bundle templating
2 participants