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

issensitive must return unknown for unknown args without sensitive #36012

Merged
merged 1 commit into from
Nov 15, 2024

Conversation

jbardin
Copy link
Member

@jbardin jbardin commented Nov 15, 2024

Terraform attempts to track marks as accurately as possible, but unknown values may not always have the same marks as they will when they become known. This is most easily seen with functions, which are allowed to return an unknown value when faced with any unknown arguments, while they are also allowed to manipulate the marks on the values as they see fit. This results in situations where the marks simply cannot be known.

Terraform generally takes the stance that if an unknown has a mark, it will remain in the final value, but the absence of a mark is not indicative of the absence of any marks in the final value. That appears to be something we can continue to maintain throughout the codebase, so given that axiom I'm going to codify it here by only changing the issensitive results for unknown, unmarked values, but allowing unknown+sensitive values to return true.

Terraform attempts to track marks as accurately as possible, but unknown
values may not always have the same marks as they will when they become
known. This is most easily seen with functions, which are allowed to
return an unknown value when faced with any unknown arguments, while
they are also allowed to manipulate the marks on the values as they see
fit. This results in situations where the marks simply cannot be known.

Terraform generally takes the stance that if an unknown has a mark, it
will remain in the final value, but the absence of a mark is not
indicative of the absence of any marks in the final value.
@jbardin jbardin requested a review from a team as a code owner November 15, 2024 15:50
Comment on lines +74 to +77
case v.HasMark(marks.Sensitive):
return cty.True, nil
case !v.IsKnown():
return cty.UnknownVal(cty.Bool), nil
Copy link
Member

@liamcervante liamcervante Nov 15, 2024

Choose a reason for hiding this comment

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

It can't lose marks going from unknown to known? Do we want to switch these case statements around?

Copy link
Member

Choose a reason for hiding this comment

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

Whoops, I didn't read the PR description 😅

Comment on lines +74 to +77
case v.HasMark(marks.Sensitive):
return cty.True, nil
case !v.IsKnown():
return cty.UnknownVal(cty.Bool), nil
Copy link
Member

Choose a reason for hiding this comment

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

Whoops, I didn't read the PR description 😅

@jbardin jbardin added the 1.10-backport If you add this label to a PR before merging, backport-assistant will open a new PR once merged label Nov 15, 2024
@jbardin jbardin merged commit bf64925 into main Nov 15, 2024
7 checks passed
@jbardin jbardin deleted the jbardin/issensitive-unknown branch November 15, 2024 16:19
Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.10-backport If you add this label to a PR before merging, backport-assistant will open a new PR once merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants