Fix SSA dry-run when an Apply is called on outputs #2615
Conversation
Does the PR have any schema changes?Looking good! No breaking changes found. |
Codecov Report
@@ Coverage Diff @@
## master #2615 +/- ##
==========================================
- Coverage 18.60% 18.59% -0.01%
==========================================
Files 47 47
Lines 9664 9667 +3
==========================================
Hits 1798 1798
- Misses 7757 7760 +3
Partials 109 109
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
| // We need to delete the empty status field returned from the API server if we are in | ||
| // preview mode. Having the status field set will cause a panic during preview if the Pulumi | ||
| // program attempts to read the status field. | ||
| if req.GetPreview() { | ||
| unstructured.RemoveNestedField(initialized.Object, "status") | ||
| } |
There was a problem hiding this comment.
What is the marshaled value of status? I ask because plugin.MarshalProperties is called with SkipNulls: true, which implies to me that this shouldn't be necessary. Since it is necessary, does SkipNulls: true not work?
There was a problem hiding this comment.
The status field is typically not nullable in Kubernetes objects, so I would expect that the status is empty, not null, here.
@rquitales could you elaborate on where the panic comes from? Is it in the application code or in the engine?
const ip = svc.status.apply((s) => s.loadBalancer.ingress[0].ip);
There was a problem hiding this comment.
I'm almost certain that the panic comes from s being null (or undefined) in the lambda that is run during preview, instead of svc.status being unknown and the apply not running.
There was a problem hiding this comment.
Yes, the status field returned from a SSA dry run is empty. The panic comes from the application code: const ip = svc.status.apply((s) => s.loadBalancer.ingress[0].ip) since it's attempting to grab the IP address.
Without this fix, the status field is populated with an empty, but initialized object. So attempting to access ingress[0] will cause a panic since the slice will be of length 0. If we nullify the status field, then the panic is averted.
rquitales
force-pushed
the
rquitales/fix-ssa-dry-run
branch
from
f9adfbb
to
75a4723
Compare
rquitales
force-pushed
the
rquitales/fix-ssa-dry-run
branch
from
75a4723
to
a28b9d9
Compare
Proposed changes
This PR unsets the empty initialized status subresource from a SSA dry-run to prevent panics during
pulumi previewwhen the Pulumi program contains an Apply on a status field. By removing the field completely, the Pulumi engine will be able to defer returning until after the resource is created, which is when the status subresource is populated.Related issues (optional)
Fixes: #2577
Fixes: #2315