Skip to content

Add input variables support to Waypoint add-ons #864

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

Merged
merged 10 commits into from
Jun 28, 2024
Merged

Conversation

paladin-devops
Copy link
Contributor

🛠️ Description

This PR enables config authors to configure their Waypoint add-ons with input variables.

@paladin-devops paladin-devops marked this pull request as ready for review June 12, 2024 04:03
@paladin-devops paladin-devops requested review from a team as code owners June 12, 2024 04:03
Copy link

@teresamychu teresamychu left a comment

Choose a reason for hiding this comment

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

LGTM

@paladin-devops paladin-devops force-pushed the rename-waypoint-templates-resource branch from 6b82845 to e73e73f Compare June 25, 2024 13:52
Base automatically changed from rename-waypoint-templates-resource to main June 25, 2024 13:56
Copy link
Contributor

@HenryEstberg HenryEstberg left a comment

Choose a reason for hiding this comment

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

Looks good to me! I have one question that I have included but it is nonblocking.

state.OutputValues, diags = types.ListValueFrom(ctx, types.ObjectType{AttrTypes: outputValue{}.attrTypes()}, ol)
} else {
state.OutputValues = types.ListNull(types.ObjectType{AttrTypes: outputValue{}.attrTypes()})
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you think you could explain the change here a little bit? It looks like this is to make this more readable but I want to better understand the why of this change.

Copy link
Contributor

Choose a reason for hiding this comment

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

It simplifies the function to only operate on the common thing, the outputs, so we can reuse the method for both resources and data sources.

Copy link
Contributor

Choose a reason for hiding this comment

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

I like how thorough the test configs that you have added are!

@paladin-devops paladin-devops merged commit 75ac135 into main Jun 28, 2024
6 checks passed
@paladin-devops paladin-devops deleted the add-on-input-vars branch June 28, 2024 16:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants