Make IsSecret threadsafe

[iwahbe]

This PR ensures that pulumi.IsSecret works as expected.

IsSecret presented 3 problems.

1. It wasn't thread safe, which triggered go's -race flag. (brought up by Prevent race when resolving a property map #11186)
2. It returned the currently known status of secret, meaning that []bool{IsSecret(a), IsSecret(a)} could evaluate to []bool{false, true} if the underlying value resolved between calls.
3. IsSecret may return the wrong value for unknown or erred Outputs.

This PR fixes (1) and (2) by awaiting the underlying value during the IsSecret call. (3) is unfixable, and I explain why in a comment.

Blocking on calls to IsSecret is necessary for correctness in the general case. We can add optimizations later if necessary. The two I have in mind are

1. Adding a concept of "known secret". pulumi.Secret(output) is always secret, so we don't need to block on output to resolve this.
2. Distinguishing between functions that return an Output (which might be secret) and functions that return raw values (string, int, ...) which never change the "secretes" of an Output. We don't need to block on functions that return raw values.

I believe we should merge this PR without optimizations, and ensure that IsSecret is as correct as we can make it. We can add optimizations as they become necessary. I don't think we have customers who rely heavily on IsSecret evaluating instantly on not yet resolved outputs, since that is precisely the scenario where IsSecret may return the wrong answer.

[pulumi-bot]

Changelog

[uncommitted] (2022-10-28)

Bug Fixes

• [sdk/{go,yaml}] Block IsSecret until secretness is known
  #11189

[iwahbe]

bors r+

[bors]

Build succeeded:

• bors-ok

[t0yv0]

It's unfixable because the signature does not return (bool, error) correct? Should it be marked deprecated perhaps to point users to the mroe correct signature?

[Frassle]

It's unfixable because the signature does not return (bool, error) correct?

It's unfixable because the secretness of an Output is its self an output. E.g. if you have

x = y.apply(v => output.secret(v + 1))

then x should say it's a secret, but it can't know that until y has resolved and the apply has evaluated, and if y never resolves (because preview) than the secretness of x is actually unknown (neither strictly true or false).

We've added these functions to all the SDKs and they're all "broken" in this way, we should probably remove them but breaking back compat is tricky, so for now we can at least make them best effort.

[t0yv0]

That's an excellent point about unknown secretness! So then this signature does not quite cut it unless unknown secretness is an error:

func IsSecret(Output<T>) (bool, error) 

And this signature is probaly too cumbersome:

func IsSecret(Output<T>) (bool, bool, error) 

Yes, I appreciate the points on bw-compat and prioritizing resources. Just thought a revamp of these could be worth tracking to fold into 4.0 or else the generics-based Go SDK refresh or else some new unit of work down the line whenever it's worth it. LMK if you want me to file, for now looks like the idea is just to let this be as is. Thanks.

[Frassle]

worth tracking to fold into 4.0

Well my initial thinking was removing them as part of 4.0 but we need to understand what people are using them for now so we have something as a replacement so we might want to start deprecating them now so people can see and come tell us what they're doing with these functions, and we can put in new safe versions before 4.0 so we've got a migration path when we remove them in 4.