Data source secret outputs #2887
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2887 +/- ##
==========================================
+ Coverage 67.75% 67.77% +0.02%
==========================================
Files 328 329 +1
Lines 42132 42179 +47
==========================================
+ Hits 28545 28586 +41
- Misses 11997 12000 +3
- Partials 1590 1593 +3 ☔ View full report in Codecov by Sentry. |
|
Can we include tests for Plugin Framework data sources? |
| @@ -1884,7 +1884,10 @@ func (p *Provider) Invoke(ctx context.Context, req *pulumirpc.InvokeRequest) (*p | |||
|
|
|||
| ret, err = plugin.MarshalProperties( | |||
| props, | |||
| plugin.MarshalOptions{Label: fmt.Sprintf("%s.returns", label)}) | |||
| plugin.MarshalOptions{ | |||
There was a problem hiding this comment.
Do the tests still pass without this change?
There was a problem hiding this comment.
no, this is the change which fixes the issue
|
Did the schema has already marked these as secret, or is this coming with this PR (and we have a test gap)? We should have schema mark these as secret. |
|
Edit: There is a schema option for secretness: https://www.pulumi.com/docs/iac/using-pulumi/pulumi-packages/schema/#property EDIT2: AWS uses it: https://github.com/pulumi/pulumi-aws/blob/c18a42565a455738d11a189512203d1bcd245a7e/provider/cmd/pulumi-resource-aws/schema.json#L172510 Cloudflare doesn't: https://github.com/pulumi/pulumi-cloudflare/blob/abcb69c6a01bc5656bc3f42924133658693138cf/provider/cmd/pulumi-resource-cloudflare/schema.json#L2 GCP uses that property too, I'll add a test for the schema. We seem to have been doing it correctly before though. |
|
You can just link an existing test from here if there is one that shows the PacakgeSpec has a secret flag as we expect, cool thanks. |
There was a problem hiding this comment.
Blocking per convo with @Zaid-Ajaj as to need to double-check breaking/non-breaking concerns.
Anton
11 minutes ago
as a user I have a program that calls listStorageAccountKeys() and suppose it also calls listStorageAccountKeysOutput()
Anton
[11 minutes ago](https://pulumi.slack.com/archives/C9SGX9QA1/p1738943378695449?thread_ts=1738867572.305319&cid=C9SGX9QA1)
THis program used to work, but the provider didn't mark the results as secret.
Anton
10 minutes ago
Now I'm upgrading to v-next of the provider, and it will now mark some of the results as secret.
Anton
10 minutes ago
What happens to my program? Does error out? Does it warn? Specifically the listStorageAccountKeys() bit is suspect I think because it returns a plan promise.
Anton
9 minutes ago
I need to know what happens accross every language :upside_down_face:
We have a suspicion that for languages like Go this will start breaking previously working programs. We need to have a conversation on whether this is a change we want to take directly or in a provider's next major. Yes it does implicate security but note also there may be users who worked around this by wrapping the output values in explicit secret flags, that will now break.
There's a few rollout options to consider.
This PR allows bridged providers to return secrets from data sources. This is a follow up from pulumi/pulumi#12710 to support this in the engine.
fixes #1051