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

ephemeral: support write-only attributes #36031

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from
Draft

Conversation

DanielMSchmidt
Copy link
Collaborator

@DanielMSchmidt DanielMSchmidt commented Nov 18, 2024

TODO:

  • Test E2E workflow

This PR aims on building baseline support for write-only attributes, so attributes that can be set to ephemeral values and can not be read / referenced. These attributes are hidden from the state.

Fixes TF-18617

Target Release

1.11.x

Draft CHANGELOG entry

NEW FEATURES

  • Ephemeral Values: support for write-only attributes, so attributes that can be set to ephemeral values and can not be read / referenced. These attributes are hidden from the state and plan files.

@DanielMSchmidt DanielMSchmidt requested a review from a team as a code owner November 18, 2024 16:28
@DanielMSchmidt DanielMSchmidt marked this pull request as draft November 18, 2024 16:28
Copy link
Contributor

The equivalence tests will be updated. Please verify the changes here.

Copy link
Contributor

The equivalence tests will be updated. Please verify the changes here.

Copy link
Contributor

The equivalence tests will be updated. Please verify the changes here.

Copy link
Contributor

The equivalence tests will be updated. Please verify the changes here.

Copy link
Contributor

The equivalence tests will be updated. Please verify the changes here.

@@ -1180,6 +1181,10 @@ func (n *NodeAbstractResourceInstance) plan(
}
}

// We don't need ephemeral values anymore since the value has been planned
ephemeralValuePaths, _ := marks.PathsWithMark(unmarkedPaths, marks.Ephemeral)
plannedNewVal = ephemeral.RemoveEphemeralValues(plannedNewVal)
Copy link
Member

Choose a reason for hiding this comment

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

Provider cannot send back ephemeral values, so we should not just be blindly removing the values here. If they did somehow exist, that would be an error.

@@ -388,6 +394,10 @@ type ChangeSrc struct {
// the serialized change.
BeforeSensitivePaths, AfterSensitivePaths []cty.Path

// BeforeWriteOnlyPaths and AfterWriteOnlyPaths are paths for any values
// in Before or After (respectively) that are considered to be write-only.
BeforeWriteOnlyPaths, AfterWriteOnlyPaths []cty.Path
Copy link
Member

Choose a reason for hiding this comment

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

write-only paths are statically known in the schema, is this intended to track if one was set or not? If so there are no AfterWriteOnlyPaths, because the provider must not return any write-only values.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I was under the impression we need to track where ephemeral values have been set. If e.g. a provider changes the schema between two runs these paths might be different for the last state vs the next state, so I thought we would need to track both.

I'm also fine with not tracking them for now at all and taking a second look at it if we find we need them, currently I am also not really clear on the problem they are solving.

Copy link
Member

Choose a reason for hiding this comment

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

They can't change during a run, so before and after here would always match. I don't think we want to go down the path of tracking if something was set on a previous run though (at least for now), the semantics of that get really messy and don't really have any bearing on the API itself. From what I can see, the only reason to track this here is to help format the diff for the user.


// AttrWriteOnlyPaths is an array of paths to mark as ephemeral coming out of
// state, or to save as write_only paths when saving state
AttrWriteOnlyPaths []cty.Path
Copy link
Member

Choose a reason for hiding this comment

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

ephemeral can't go into or out of state, so there should be no reason to track the marked paths

"sensitive_attributes": []interface{}{},
"schema_version": 0.0,
"sensitive_attributes": []interface{}{},
"write_only_attributes": []interface{}{},
Copy link
Member

Choose a reason for hiding this comment

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

remote-state is all about outputs, so there can't be anything write-only

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.

2 participants