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.

Sign up for GitHub

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

Jump to bottom

Regression in Pulumi v3.149.0: NullReferenceException in TestDotnet_Guestbook #456

Closed
rquitales opened this issue Feb 11, 2025 · 4 comments · Fixed by #459
Closed

Regression in Pulumi v3.149.0: NullReferenceException in TestDotnet_Guestbook #456

rquitales opened this issue Feb 11, 2025 · 4 comments · Fixed by #459
Assignees
@Frassle
Labels
impact/regression Something that used to work, but is now broken kind/bug Some behavior is incorrect or out of spec language/dotnet p1 A bug severe enough to be the next item assigned to an engineer resolution/fixed This issue was fixed
Milestone
0.116

Comments

@rquitales
Copy link
Member

rquitales commented Feb 11, 2025 •
edited
Loading

What happened?

When testing TestDotnet_Guestbook in pulumi/pulumi-kubernetes using Pulumi versions v3.149.0 and v3.150.0, previously working tests began failing with a NullReferenceException.

Error Output:

The following error occurs during pulumi preview:

pulumi preview
Previewing update (fix-k8s)

     Type                              Name                      Plan       Info
 +   pulumi:pulumi:Stack               guestbook_dotnet-fix-k8s  create     1 error
 +   └─ kubernetes:apps/v1:Deployment  redis-master              create

Diagnostics:
  pulumi:pulumi:Stack (guestbook_dotnet-fix-k8s):
    error: Running program '/Users/rquitales/code/providers/kubernetes/tests/sdk/dotnet/guestbook/bin/Debug/net7.0/Guestbook.dll' failed with an unhandled exception:
    System.NullReferenceException: Object reference not set to an instance of an object.
       at ImmutableDictionary<string, Input<V>> Pulumi.InputMap<V>+<>c.<op_Implicit>b__17_0(?)+(ImmutableDictionary<string, V> values) => { }
       at Output<ValueTuple> Pulumi.Output<T>.Apply(Func<T, Task> func)+(T t) => { }
       at async Task<OutputData<U>> Pulumi.Output<T>.ApplyHelperAsync<U>(Task<OutputData<T>> dataTask, Func<T, Output<U>> func) x 2
       at async Task<OutputData<object>> Pulumi.Output<T>.Pulumi.IOutput.GetDataAsync()
       at async Task<object> Pulumi.Serialization.Serializer.SerializeAsync(string ctx, object prop, bool keepResources, bool keepOutputValues) x 3
       at async Task<ImmutableDictionary<string, object>> Pulumi.Serialization.Serializer.SerializeDictionaryAsync(string ctx, IDictionary dictionary, bool keepResources, bool keepOutputValues)
       at async Task<ImmutableDictionary<string, object>> Pulumi.Serialization.Serializer.SerializeInputArgsAsync(string ctx, InputArgs args, bool keepResources, bool keepOutputValues)
       at async Task<object> Pulumi.Serialization.Serializer.SerializeAsync(string ctx, object prop, bool keepResources, bool keepOutputValues) x 3
       at async Task<RawSerializationResult> Pulumi.Deployment.SerializeFilteredPropertiesRawAsync(string label, IDictionary<string, object> args, Predicate<string> acceptKey, bool keepResources, bool keepOutputValues)
       at async Task<SerializationResult> Pulumi.Deployment.SerializeFilteredPropertiesAsync(string label, IDictionary<string, object> args, Predicate<string> acceptKey, bool keepResources, bool keepOutputValues)
       at async Task<PrepareResult> Pulumi.Deployment.PrepareResourceAsync(string label, Resource res, bool custom, bool remote, ResourceArgs args, ResourceOptions options, RegisterPackageRequest registerPackageRequest)
       at async Task<(string urn, string id, Struct data, ImmutableDictionary<string, ImmutableHashSet<Resource>> dependencies, Result result)> Pulumi.Deployment.RegisterResourceAsync(Resource resource, bool remote, Func<string, Resource> newDependency, ResourceArgs args, ResourceOptions options, RegisterPackageRequest registerPackageRequest)
       at async Task<(string urn, string id, Struct data, ImmutableDictionary<string, ImmutableHashSet<Resource>> dependencies, Result result)> Pulumi.Deployment.ReadOrRegisterResourceAsync(Resource resource, bool remote, Func<string, Resource> newDependency, ResourceArgs args, ResourceOptions options, RegisterPackageRequest registerPackageRequest)
       at async Task Pulumi.Deployment.CompleteResourceAsync(Resource resource, bool remote, Func<string, Resource> newDependency, ResourceArgs args, ResourceOptions options, ImmutableDictionary<string, IOutputCompletionSource> completionSources, RegisterPackageRequest registerPackageRequest)
       at async Task<T> Pulumi.Output<T>.GetValueAsync(T whenUnknown)
       at async Task<string> Pulumi.Deployment+EngineLogger.TryGetResourceUrnAsync(Resource resource)

Resources:
    + 2 to create

The issue appears to originate from the following line in the Pulumi program:
Guestbook.cs#L73.

Switching from Labels =redisLeaderDeployment.Metadata.Apply(metadata => metadata.Labels) to using a concrete object, Labels = redisMasterLabels, allows the Pulumi program to successfully run.

It appears to be potentially a bug in the way inputs/outputs are being handled.

Example

Use the sample program in: https://github.com/pulumi/examples/blob/master/kubernetes-cs-guestbook/simple/Guestbook.cs
(Note, the example here, and the test program use in pulumi-kubernetes' TestDotnet_Guestbook are identical)

Example failing test run: https://github.com/pulumi/pulumi-kubernetes/actions/runs/13210581683/job/36883489034

This is the exact line that triggers the error: https://github.com/pulumi/examples/blob/15391fd17ef276219d810a4308c96a78b5dbd29e/kubernetes-cs-guestbook/simple/Guestbook.cs#L73

Output of pulumi about

running 'dotnet build -nologo'
  Determining projects to restore...

  All projects are up-to-date for restore.

  Guestbook -> /Users/rquitales/code/providers/kubernetes/tests/sdk/dotnet/guestbook/bin/Debug/net7.0/Guestbook.dll



Build succeeded.
    0 Warning(s)
    0 Error(s)

Time Elapsed 00:00:00.85

'dotnet build -nologo' completed successfully
CLI
Version      3.150.0
Go Version   go1.23.6
Go Compiler  gc

Plugins
KIND      NAME        VERSION
language  dotnet      3.73.0
resource  kubernetes  4.21.1

Host
OS       darwin
Version  13.3.1
Arch     arm64

This project is written in dotnet: executable='/opt/homebrew/bin/dotnet' version='7.0.401'

Current Stack: ***

Found no resources associated with fix-k8s

Found no pending operations associated with fix-k8s

Backend
Name           pulumi.com
URL            ***
User           ***
Organizations ***
Token type     ***

Dependencies:
NAME               VERSION
Pulumi             3.73.0
Pulumi.Kubernetes  4.21.1

Additional context

No response

Contributing

Vote on this issue by adding a 👍 reaction.
To contribute a fix for this issue, leave a comment (and link to your pull request, if you've opened one already).
@rquitales rquitales added kind/bug Some behavior is incorrect or out of spec needs-triage Needs attention from the triage team labels Feb 11, 2025
@rquitales rquitales mentioned this issue Feb 11, 2025
Closed
@Frassle
Copy link
Member

Frassle commented Feb 12, 2025

OK I think this might actually be a bug in deserialisation that the InputMap/List changes have just flagged up, gonna see if I can repro it quickly.

@Frassle Frassle transferred this issue from pulumi/pulumi Feb 12, 2025
@lunaris lunaris added impact/regression Something that used to work, but is now broken p1 A bug severe enough to be the next item assigned to an engineer and removed needs-triage Needs attention from the triage team labels Feb 12, 2025
@lunaris lunaris added this to the 0.116 milestone Feb 12, 2025
@Frassle
Copy link
Member

Frassle commented Feb 12, 2025

Ack this is a deserialiser bug
#457

If labels is sent back as null or missing from the provider then we set Metadata.Labels to null but it doesn't have nullability annotations on it so it should just be filled in empty (or be marked as nullable).

It happens that this used to flow through the system ok because we didn't look at the map, but now because we have to explicitly flatten maps we hit a NRE in this case when something tries to use it.

@Frassle
Copy link
Member

Frassle commented Feb 12, 2025

Arguably there's three things we should fix here.

  1. Allow null to InputMap converts to succeed (they used to just result in null, despite the nullability annotations saying that wasn't possible).
  2. Update the nullability annotations that InputMap can be null
  3. Fix the deserialiser to either fill in empty for null maps, or mark all the output maps as nullable.

@Frassle Frassle mentioned this issue Feb 12, 2025
Merged
github-merge-queue bot pushed a commit that referenced this issue Feb 19, 2025
@Frassle
Handle null runtime values in InputMap/List cast (#459)
8f50081 
Fixes #456

This is a bit of a sad fix but `InputMap` used to tolerate the implicit
cast from dictionary being called with null by just passing that null
down. If you tried to call Add or enumerate it would NRE, but if it just
got assigned to another resource property the serialiser could handle
it.

#449 broke this because it
always tried to enumerate the incoming immutable dictionary to build the
new internal nested immutable dictionary. This fixes it to behave as
before were null flows through as null.
@pulumi-bot pulumi-bot added the resolution/fixed This issue was fixed label Feb 19, 2025
@pulumi-bot
Copy link

This issue has been addressed in PR #459 and shipped in release v3.74.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Frassle Frassle

Labels
impact/regression Something that used to work, but is now broken kind/bug Some behavior is incorrect or out of spec language/dotnet p1 A bug severe enough to be the next item assigned to an engineer resolution/fixed This issue was fixed
Projects
None yet
Milestone
0.116
Development

Successfully merging a pull request may close this issue.

Handle null runtime values in InputMap/List cast pulumi/pulumi-dotnet
4 participants
@lunaris @Frassle @rquitales @pulumi-bot