Skip to content

Conversation

jbardin
Copy link
Member

@jbardin jbardin commented Sep 13, 2024

This add ephemeral resources to the configuration language. Terraform can now parse ephemeral resource configuration and references.

Copy link
Member

@radeksimko radeksimko left a comment

Choose a reason for hiding this comment

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

LGTM, I left just a few minor questions inline.

var ephemeralBlockSchema = &hcl.BodySchema{
Attributes: commonResourceAttributes,
Blocks: []hcl.BlockHeaderSchema{
{Type: "lifecycle"},
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
{Type: "lifecycle"},
{Type: "lifecycle"}, // reserved for future use

Comment on lines +475 to +487
case "precondition", "postcondition":
cr, moreDiags := decodeCheckRuleBlock(block, override)
diags = append(diags, moreDiags...)

moreDiags = cr.validateSelfReferences(block.Type, r.Addr())
diags = append(diags, moreDiags...)

switch block.Type {
case "precondition":
r.Preconditions = append(r.Preconditions, cr)
case "postcondition":
r.Postconditions = append(r.Postconditions, cr)
}
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure how much complexity/effort will be required to make conditions work for the new resource type but we could also just reserve these in the language initially. You are more likely to know the answer though, so I'm assuming if you are proposing to keep it like this, it's not too complex.

Copy link
Member Author

Choose a reason for hiding this comment

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

Aside from optimistically hoping that the implementation isn't too difficult (these are still just a resource type after all), they also seem useful when the values are not easily extracted by the user for other debugging purposes. If there is any problem implementing later steps we can always reserve the block names to prevent their use. These first commits are all preliminary anyway, there will probably be some changes as all the pieces are integrated.

@@ -18,6 +18,8 @@ import (
"github.com/hashicorp/terraform/internal/addrs"
"github.com/hashicorp/terraform/internal/depsfile"
"github.com/hashicorp/terraform/internal/getproviders/providerreqs"

_ "github.com/hashicorp/terraform/internal/logging"
Copy link
Member

Choose a reason for hiding this comment

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

Was this intentional or was it just to temporarily aid debugging in test environment?

Copy link
Member Author

Choose a reason for hiding this comment

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

You have to import the internal logging package somewhere to initialize the default logger, so any package tested in isolation needs this to not print all logs during tests.

Base automatically changed from jbardin/ephemeral-resources to main September 18, 2024 15:01
@jbardin jbardin merged commit 505b255 into main Sep 18, 2024
6 checks passed
@jbardin jbardin deleted the jbardin/ephemeral-config branch September 18, 2024 15:02
Copy link
Contributor

Reminder for the merging maintainer: if this is a user-visible change, please update the changelog on the appropriate release branch.

Copy link
Contributor

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 19, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants