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

Include JSONStateOutputs #31466

Merged
merged 4 commits into from Jul 22, 2022
Merged

Include JSONStateOutputs #31466

merged 4 commits into from Jul 22, 2022

Conversation

annawinkler
Copy link
Contributor

@annawinkler annawinkler commented Jul 18, 2022

Description

This PR is part of the outputs project, and builds on the work of two previously-opened (not merged) PRs: 31284, and 31241.

The goal of the project is to show more accurate type information, and initially we tried including all of the state data as JSON and there were concerns that some of the terraform commands would not be performant with this amount of data.

This is attempt number three. With this attempt, we've reduced the amount of JSON data in the state - we want to include only the state outputs JSON in a new field, JSONStateOutputs.

This feature is in beta, and is optional, so if it is not included, Terraform Cloud continues merrily along as before without the additional type information.

  • Related go-tfe PR

Still to do:

  • Update relevant tests

Test Plan

  1. Pull down this branch, and build and install it via go install
  2. Pick your favorite terraform config (the kitchen sink is a good one), configure the cloud config. Then apply via ~/go/bin/terraform apply
  3. In the UI, note the state version ID.

@apparentlymart
Copy link
Member

Hi @annawinkler!

I don't think I have sufficient context to dig into the details of this PR right now, but I did just want to address your specific question about beta features.

There's not really any such thing as a "beta feature" in Terraform CLI, because once someone has started using a particular version of Terraform CLI we can't take that away from them: any future changes to the API must be in some sense backward-compatible with any client we ship as part of a stable Terraform CLI release.

We do have a couple options at our disposal, though:

  1. We can design the underlying API in way that allows the client (Terraform CLI) to unambiguously determine whether the API supports the feature or not. If we decide to change the feature in a way that is breaking for already-deployed clients, we can make the old version of the API signal that it's not available anymore, thereby causing the existing clients to revert to whatever behavior they would've used without this change.

    We tend to be forced to design our client-server interactions in this way anyway, since we also need to contend with the opposite direction where Terraform CLI is released but a feature isn't available in Terraform Enterprise yet. So this sort of "progressive enhancement" technique has worked well for us in the past, although admittedly I don't think we've ever used it specifically to retract a beta feature whose API design didn't work out. Our main uses in the past were situations like verifying that the remote API can support the API equivalent of the -target option before allowing users to specify it when using the remote backend / Terraform Cloud integration.

    (The server side does of course also need to behave reasonably when the client doesn't make use of this new feature, because all existing versions of Terraform CLI released to date will not use the new API features.)

  2. For a feature that requires affirmative user action to enable, we can in principle mark it as "experimental", in which case it will only be available in our alpha releases and not in any beta, release candidate, or final releases.

    This strategy doesn't work for anything that's just implicitly enabled without the user's awareness, because in that case they don't get to make the choice about whether to use the experimental feature or not and if it's not chosen by the user then it's effectively a non-experimental release.

    We are currently in a transitional state where new experiments won't even be visible in alpha releases, because we need to transition to a new release pipeline which knows how to detect when the release being generated is an alpha and flags the generated executable accordingly. So in practice this option isn't super useful right now unless you only intend to use the feature in custom builds of Terraform CLI you make yourself, where you can explicitly enable experimental features.

From what I see in the PR, this seems more like an option-1-flavored problem, since this is just an additional property in the API request that users have no direct control over once they've upgraded. If the remote API will quietly ignore a new object property it doesn't expect then this might well just solve itself, as long as we're prepared to rename the property in the event that we need to change the design in a breaking way, and have the API either continue to support the old shape for backward compatibility or accept that older CLI versions cannot generate this data in the way Terraform Cloud needs it.

I hope that's helpful in deciding how best to proceed here!

Copy link
Contributor

@brandonc brandonc left a comment

Choose a reason for hiding this comment

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

Just minor feedback. I smoke tested this and couldn't find any issues.

internal/command/jsonstate/state.go Show resolved Hide resolved
internal/cloud/backend_state.go Show resolved Hide resolved
@brandonc brandonc merged commit 74ee19b into main Jul 22, 2022
@brandonc brandonc deleted the aw/send-jsonstateoutputs branch July 22, 2022 20:49
@github-actions
Copy link

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

@github-actions
Copy link

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 Aug 22, 2022
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.

None yet

3 participants