Skip to content
This repository has been archived by the owner on Jan 8, 2024. It is now read-only.

Allow complex non-string types for dynamic default variables #4357

Merged
merged 21 commits into from
Jan 20, 2023

Conversation

izaaklauer
Copy link
Contributor

@izaaklauer izaaklauer commented Dec 22, 2022

Depends on hashicorp/waypoint-plugin-sdk#79
Demonstrated in hashicorp/waypoint-examples#106

Internal video describing the motivation (thanks @andrew-hashicorp): https://drive.google.com/file/d/1aCDfFioVELb7Q9MZhN0XiRegUlNJ4LdM/view?usp=share_link

Internal video code walkthrough: https://drive.google.com/file/d/1-58UzSXTJFWnlBoE4LpoIQsg1hExubxV/view?usp=share_link

What does this do?

Prior to this, waypoint only supported string types in variables with dynamic defaults. Configsourcer plugins could only emit strings, and if a user tried to use any type other than string for a variable with a dynamic default, waypoint would throw a validation error.

This updates configsourcer plugins to optionally return json, which is then interpreted internally as an hcl snippet and can be used as a complex type anywhere else in the hcl. It also allows users to specify non-string types, or use the any type to opt out of early type checking.

This also updates the terraform configsourcer to output json if a user picks a variable with a non-string json-representable type. It also allows the user to omit the output field, in which case the plugin returns all of the workspace's outputs as a map.

Motivation

The primary motivation here is to make it easier to pull more values from terraform cloud.

See this example of using terraform to manage all the infrastructure associated with an ECS deployment in two environments: https://github.com/izaaklauer/acmeapp1/blob/main/waypoint.hcl. You'll notice that it involves 27 variables across three workspaces. It also forces users to take the list of subnets output and turn it into many individual string outputs, as waypoint can't natively consume the list.

This change brings us down to three variable stanzas, one per workspace: https://github.com/izaaklauer/acmeapp1/blob/443ab9e64734bcf834a9309d1035d98e505263d0/waypoint.varmulti.hcl#L74-L107

It also allows native use of the subnets list variable: https://github.com/izaaklauer/acmeapp1/blob/443ab9e64734bcf834a9309d1035d98e505263d0/waypoint.varmulti.hcl#L36

After this is merged, we intend to write a full public use-case.

How do I test this?

I build a sample project in waypoint-examples: hashicorp/waypoint-examples#106. You can use that to test the feature, and we can merge the example once this is released in core waypoint.

This is a bit hacky, mostly because it continues to use existing hacks. The variable system is requesting it's dynamic default vars as "files". The appconfig system then reads any json-typed variables, dumps the raw json into the "file" contents, and returns it to the variable system.

The variable system then looks at the type the user specified. If it's a string, it'll assume the plugin returned a string value. If it's a bool or number, it'll error. If it's anything else, it'll try reading the plugin output from the "file" as json, and if that works, surface it up the chain as an hcl snippet input variable, which to my great surprise, _just works_.

It's not great that we're guessing at what kind of data the plugins returned based on the type that the user specified, but I'm not up for a broader overhaul of the system at this point to wire more type information through.
Honestly not sure why all this embedJson changed, but it seems benign
@@ -54,7 +55,7 @@ type ConfigSourcer struct {
}

type cachedSecret struct {
Outputs map[string]string // The most recent outputs for a workspace
Outputs map[string]StateIncludeAttributes // The most recent outputs for a workspace
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Caching the full TFC attributes response now, because it's useful to look at the value and the type later on, and we're now allowing types other than string.

@@ -12,6 +12,7 @@
"Default": "",
"EnvVar": "",
"Category": false,
"Example": "",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not really sure why anything in embedJson changed when I regenerated website mdx, but it seems benign.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, I think this is because of the waypoint-plugin-sdk changes. It added a new Example field, and so the re-gen is adding these for every doc given that SDK change.

@izaaklauer izaaklauer marked this pull request as ready for review December 23, 2022 23:49
@izaaklauer izaaklauer requested a review from a team December 23, 2022 23:49
@izaaklauer izaaklauer changed the title Non-string configvar variable types json configvar variable values Dec 23, 2022
@izaaklauer izaaklauer changed the title json configvar variable values json configsourcer variable values Dec 23, 2022
@izaaklauer izaaklauer changed the title json configsourcer variable values Allow complex non-string types for dynamic default variables Dec 23, 2022
@ashleemboyer
Copy link
Contributor

Hello @izaaklauer! 😊 This is a quick comment to let you know I updated this PR with main so it's up to date with the changes made in #4277.

Copy link
Member

@briancain briancain left a comment

Choose a reason for hiding this comment

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

Haven't had a chance to test, but have a few comments! Overall looks pretty good though 👍🏻

builtin/tfc/config_sourcer.go Outdated Show resolved Hide resolved
builtin/tfc/config_sourcer_test.go Outdated Show resolved Hide resolved
internal/appconfig/watcher.go Show resolved Hide resolved
izaaklauer and others added 2 commits January 17, 2023 15:40
Co-authored-by: Cassie Coyle <25733780+cicoyle@users.noreply.github.com>
Co-authored-by: Cassie Coyle <25733780+cicoyle@users.noreply.github.com>
Copy link
Contributor

@cicoyle cicoyle left a comment

Choose a reason for hiding this comment

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

This looks great! It looks like more changes due to the embedJson/ changes, but yeah those look benign.

builtin/tfc/config_sourcer.go Outdated Show resolved Hide resolved
builtin/tfc/config_sourcer.go Outdated Show resolved Hide resolved
"are currently supported.",
"If unspecified, all outputs from the workspace will be read",
"and returned as a map[string]object, and can be referenced as such",
"in the waypoint.hcl. See <docs> for more details.",
Copy link
Member

Choose a reason for hiding this comment

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

Was <docs> intended to be a URL?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed! Not sure which docs I meant there, but I linked to waypoint-examples.

@@ -12,6 +12,7 @@
"Default": "",
"EnvVar": "",
"Category": false,
"Example": "",
Copy link
Member

Choose a reason for hiding this comment

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

Ah, I think this is because of the waypoint-plugin-sdk changes. It added a new Example field, and so the re-gen is adding these for every doc given that SDK change.

internal/config/variables/variables.go Outdated Show resolved Hide resolved
Co-authored-by: Brian Cain <bcain@hashicorp.com>
Copy link
Contributor

@paladin-devops paladin-devops left a comment

Choose a reason for hiding this comment

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

Just read through, this looks great! I haven't tested, but had a few minor comments. Can't wait to see this in a use case! 😄

internal/appconfig/watcher.go Show resolved Hide resolved
internal/appconfig/watcher_test.go Outdated Show resolved Hide resolved
internal/config/variables/variables.go Outdated Show resolved Hide resolved
izaaklauer and others added 2 commits January 18, 2023 14:12
Co-authored-by: Joe <83741749+paladin-devops@users.noreply.github.com>
Co-authored-by: Joe <83741749+paladin-devops@users.noreply.github.com>
@voycey
Copy link

voycey commented Feb 27, 2023

@izaaklauer tagging the various issues that this solves is recommended - there are open issues that request this specifically

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants