Skip to content

Commit

Permalink
Don't flatten outputs to top level for InputList and InputMap (#449)
Browse files Browse the repository at this point in the history
Fixes #22

InputList and InputMap are types used to make it easier to work with
lists/maps where both the overall structure or individual elements can
be `Input`s.

The current implementation just flattens everything to a top-level
input. That is `InputList<T>` is just `Input<ImmutableArray<T>>`. So if
you start with a known empty list then add an unknown input element to
it you end up with an unknown list structure. This is not ideal for
previews (see #22) and doesn't align with the other SDKs where if you
add an unknown element to a list the overall list shape is still known
and sent to the engine for preview correctly.

This PR fixes the above by keeping track of non-flattened data inside
InputMap/List. i.e. the internal shape of `InputList<T>` is now
`Input<ImmutableArray<Input<T>>>`. This allows us to take a known list
and add an unknown element to it and leave the overall shape of the list
still known.

We do this without changing the public surface of InputList/Map, so they
still inherit from `Input<ImmutableArray<T>>`. We add another field for
the nested structure and ensure that we just always update both fields
(the one in InputList/Map and the one in the base Input class) together.
In the property serializer we then use reflection to pull out the fully
nested value (e.g. `Input<ImmutableArray<Input<T>>>`) and then
recursively serialize that, rather than hitting the standard `IInput`
case and getting the flattened value (e.g. `Input<ImmutableArray<T>>`).
  • Loading branch information
Frassle authored Jan 31, 2025
1 parent 36c214b commit f7020ec
Show file tree
Hide file tree
Showing 6 changed files with 197 additions and 24 deletions.
7 changes: 7 additions & 0 deletions .changes/unreleased/Improvements-449.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
component: sdk
kind: Improvements
body: InputMap and InputList no longer flatten nested unknowns/secrets to apply to
the whole object.
time: 2025-01-29T14:14:44.986465684Z
custom:
PR: "449"
49 changes: 49 additions & 0 deletions sdk/Pulumi.Tests/Serialization/MarshalOutputTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using System.Linq;
using System.Threading.Tasks;
using Pulumi.Serialization;
using Pulumi.Utilities;
using Xunit;

namespace Pulumi.Tests.Serialization
Expand Down Expand Up @@ -90,6 +91,26 @@ public sealed class BarArgs : ResourceArgs
ImmutableArray<object>.Empty.Add(CreateOutputValue("hello", isSecret: true))
},
new object[]
{
new InputList<string> { "hello" },
ImmutableArray<object>.Empty.Add("hello")
},
new object[]
{
new InputList<string> { Output.Create("hello") },
ImmutableArray<object>.Empty.Add("hello")
},
new object[]
{
new InputList<string> { Output.CreateSecret("hello") },
ImmutableArray<object>.Empty.Add(CreateSecretValue("hello"))
},
new object[]
{
new InputList<string> { OutputUtilities.CreateUnknown("") },
ImmutableArray<object>.Empty.Add(Constants.UnknownValue)
},
new object[]
{
new Dictionary<string, Input<string>> { { "foo", "hello" } },
ImmutableDictionary<string, object>.Empty.Add("foo", "hello")
Expand All @@ -105,6 +126,26 @@ public sealed class BarArgs : ResourceArgs
ImmutableDictionary<string, object>.Empty.Add("foo", CreateOutputValue("hello", isSecret: true))
},
new object[]
{
new InputMap<string> { { "foo", "hello" } },
ImmutableDictionary<string, object>.Empty.Add("foo", "hello")
},
new object[]
{
new InputMap<string> { { "foo", Output.Create("hello") } },
ImmutableDictionary<string, object>.Empty.Add("foo", "hello")
},
new object[]
{
new InputMap<string> { { "foo", Output.CreateSecret("hello") } },
ImmutableDictionary<string, object>.Empty.Add("foo", CreateSecretValue("hello"))
},
new object[]
{
new InputMap<string> { { "foo", OutputUtilities.CreateUnknown("") } },
ImmutableDictionary<string, object>.Empty.Add("foo", Constants.UnknownValue)
},
new object[]
{
new BarArgs { Foo = new FooArgs { Foo = "hello" } },
ImmutableDictionary<string, object>.Empty.Add("foo",
Expand Down Expand Up @@ -156,5 +197,13 @@ public static Task TestSerialize(object input, object expected) => RunInNormal(a
if (deps.Length > 0) b.Add(Constants.DependenciesName, deps.ToImmutableArray());
return b.ToImmutableDictionary();
}

private static ImmutableDictionary<string, object?> CreateSecretValue(object? value)
{
var b = ImmutableDictionary.CreateBuilder<string, object?>();
b.Add(Constants.SpecialSigKey, Constants.SpecialSecretSig);
b.Add(Constants.ValueName, value);
return b.ToImmutableDictionary();
}
}
}
54 changes: 45 additions & 9 deletions sdk/Pulumi/Core/InputList.cs
Original file line number Diff line number Diff line change
Expand Up @@ -46,18 +46,38 @@ namespace Pulumi
/// </summary>
public sealed class InputList<T> : Input<ImmutableArray<T>>, IEnumerable, IAsyncEnumerable<Input<T>>
{
public InputList() : this(Output.Create(ImmutableArray<T>.Empty))
Input<ImmutableArray<Input<T>>> _inputValue;
/// <summary>
/// InputList externally has to behave as an <c>Input{ImmutableArray{T}}</c>, but we actually want to
/// keep nested Input/Output values separate, so that we can serialise the overall list shape even if one of the
/// inner elements is an unknown value.
///
/// To do that we keep a separate value of the form <c>Input{ImmutableArray{Input{T}}}</c>/> which each
/// time we set syncs the flattened value to the base <c>Input{ImmutableArray{T}}</c>.
/// </summary>
Input<ImmutableArray<Input<T>>> Value
{
get => _inputValue;
set
{
_inputValue = value;
_outputValue = _inputValue.Apply(inputs => Output.All(inputs));
}
}

public InputList() : this(ImmutableArray<Input<T>>.Empty)
{
}

private InputList(Output<ImmutableArray<T>> values)
: base(values)
private InputList(Input<ImmutableArray<Input<T>>> values)
: base(values.Apply(values => Output.All(values)))
{
_inputValue = values;
}

public void Add(params Input<T>[] inputs)
{
_outputValue = Concat(inputs);
Value = Concat(inputs).Value;
}

/// <summary>
Expand All @@ -70,18 +90,26 @@ public void Add(InputList<T> inputs)

public void AddRange(InputList<T> inputs)
{
_outputValue = Concat(inputs);
Value = Concat(inputs).Value;
}

/// <summary>
/// Concatenates the values in this list with the values in <paramref name="other"/>,
/// returning the concatenated sequence in a new <see cref="InputList{T}"/>.
/// </summary>
public InputList<T> Concat(InputList<T> other)
=> Output.Concat(_outputValue, other._outputValue);
{
var list = new InputList<T>();
list.Value = Output.Tuple(Value, other.Value).Apply(t =>
{
var (first, second) = t;
return first.AddRange(second);
});
return list;
}

internal InputList<T> Clone()
=> new InputList<T>(_outputValue);
=> new InputList<T>(Value);

#region construct from unary

Expand Down Expand Up @@ -131,7 +159,7 @@ public static implicit operator InputList<T>(ImmutableArray<Output<T>> values)
=> values.SelectAsArray(v => (Input<T>)v);

public static implicit operator InputList<T>(ImmutableArray<Input<T>> values)
=> Output.All(values);
=> new InputList<T>(values);

#endregion

Expand All @@ -147,7 +175,15 @@ public static implicit operator InputList<T>(Output<IEnumerable<T>> values)
=> values.Apply(ImmutableArray.CreateRange);

public static implicit operator InputList<T>(Output<ImmutableArray<T>> values)
=> new InputList<T>(values);
=> new InputList<T>(values.Apply(values =>
{
var builder = ImmutableArray.CreateBuilder<Input<T>>(values.Length);
foreach (var value in values)
{
builder.Add(value);
}
return builder.MoveToImmutable();
}));

#endregion

Expand Down
73 changes: 59 additions & 14 deletions sdk/Pulumi/Core/InputMap.cs
Original file line number Diff line number Diff line change
Expand Up @@ -42,20 +42,56 @@ namespace Pulumi
/// </summary>
public sealed class InputMap<V> : Input<ImmutableDictionary<string, V>>, IEnumerable, IAsyncEnumerable<Input<KeyValuePair<string, V>>>
{
public InputMap() : this(Output.Create(ImmutableDictionary<string, V>.Empty))
private static Input<ImmutableDictionary<string, V>> Flatten(Input<ImmutableDictionary<string, Input<V>>> inputs)
{
return inputs.Apply(inputs =>
{
var list = inputs.Select(kv => kv.Value.Apply(value => KeyValuePair.Create(kv.Key, value)));
return Output.All(list).Apply(kvs =>
{
var result = ImmutableDictionary.CreateBuilder<string, V>();
foreach (var (k, v) in kvs)
{
result[k] = v;
}
return result.ToImmutable();
});
});
}

Input<ImmutableDictionary<string, Input<V>>> _inputValue;
/// <summary>
/// InputMap externally has to behave as an <c>Input{ImmutableDictionary{string, T}}</c>, but we actually
/// want to keep nested Input/Output values separate, so that we can serialise the overall map shape even if
/// one of the inner elements is an unknown value.
///
/// To do that we keep a separate value of the form <c>Input{ImmutableDictionary{string, Input{T}}}</c>
/// which each time we set syncs the flattened value to the base <c>Input{ImmutableDictionary{string,
/// T}}</c>.
/// </summary>
Input<ImmutableDictionary<string, Input<V>>> Value
{
get => _inputValue;
set
{
_inputValue = value;
_outputValue = Flatten(_inputValue);
}
}

public InputMap() : this(ImmutableDictionary<string, Input<V>>.Empty)
{
}

private InputMap(Output<ImmutableDictionary<string, V>> values)
: base(values)
private InputMap(Input<ImmutableDictionary<string, Input<V>>> values)
: base(Flatten(values))
{
_inputValue = values;
}

public void Add(string key, Input<V> value)
{
var inputDictionary = (Input<ImmutableDictionary<string, V>>)_outputValue;
_outputValue = Output.Tuple(inputDictionary, value)
.Apply(x => x.Item1.Add(key, x.Item2));
Value = Value.Apply(self => self.Add(key, value));
}

/// <summary>
Expand All @@ -68,8 +104,7 @@ public void Add(InputMap<V> values)

public void AddRange(InputMap<V> values)
{
var inputDictionary = (Input<ImmutableDictionary<string, V>>)_outputValue;
_outputValue = Output.Tuple(inputDictionary, values)
Value = Output.Tuple(Value, values.Value)
.Apply(x => x.Item1.AddRange(x.Item2));
}

Expand All @@ -91,16 +126,18 @@ public Input<V> this[string key]
/// both input maps.</returns>
public static InputMap<V> Merge(InputMap<V> first, InputMap<V> second)
{
var output = Output.Tuple(first._outputValue, second._outputValue)
var output = Output.Tuple(first.Value, second.Value)
.Apply(dicts =>
{
var result = new Dictionary<string, V>(dicts.Item1);
var builder = ImmutableDictionary.CreateBuilder<string, Input<V>>();
foreach (var (k, v) in dicts.Item1)
builder[k] = v;
// Overwrite keys if duplicates are found
foreach (var (k, v) in dicts.Item2)
result[k] = v;
return result;
builder[k] = v;
return builder.ToImmutable();
});
return output;
return new InputMap<V>(output);
}

#region construct from dictionary types
Expand All @@ -118,7 +155,15 @@ public static implicit operator InputMap<V>(Output<IDictionary<string, V>> value
=> values.Apply(ImmutableDictionary.CreateRange);

public static implicit operator InputMap<V>(Output<ImmutableDictionary<string, V>> values)
=> new InputMap<V>(values);
=> new InputMap<V>(values.Apply(values =>
{
var builder = ImmutableDictionary.CreateBuilder<string, Input<V>>();
foreach (var value in values)
{
builder.Add(value.Key, value.Value);
}
return builder.ToImmutable();
}));

#endregion

Expand Down
21 changes: 21 additions & 0 deletions sdk/Pulumi/Pulumi.xml
Original file line number Diff line number Diff line change
Expand Up @@ -513,6 +513,16 @@
</code>
</summary>
</member>
<member name="P:Pulumi.InputList`1.Value">
<summary>
InputList externally has to behave as an <c>Input{ImmutableArray{T}}</c>, but we actually want to
keep nested Input/Output values separate, so that we can serialise the overall list shape even if one of the
inner elements is an unknown value.

To do that we keep a separate value of the form <c>Input{ImmutableArray{Input{T}}}</c>/> which each
time we set syncs the flattened value to the base <c>Input{ImmutableArray{T}}</c>.
</summary>
</member>
<member name="M:Pulumi.InputList`1.Add(Pulumi.InputList{`0})">
<summary>
Note: this is non-standard convenience for use with collection initializers.
Expand Down Expand Up @@ -557,6 +567,17 @@
</code>
</summary>
</member>
<member name="P:Pulumi.InputMap`1.Value">
<summary>
InputMap externally has to behave as an <c>Input{ImmutableDictionary{string, T}}</c>, but we actually
want to keep nested Input/Output values separate, so that we can serialise the overall map shape even if
one of the inner elements is an unknown value.

To do that we keep a separate value of the form <c>Input{ImmutableDictionary{string, Input{T}}}</c>
which each time we set syncs the flattened value to the base <c>Input{ImmutableDictionary{string,
T}}</c>.
</summary>
</member>
<member name="M:Pulumi.InputMap`1.Add(Pulumi.InputMap{`0})">
<summary>
Note: this is non-standard convenience for use with collection initializers.
Expand Down
17 changes: 16 additions & 1 deletion sdk/Pulumi/Serialization/Serializer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,22 @@ prop is double ||
$"Tasks are not allowed inside ResourceArgs. Please wrap your Task in an Output:\n\t{ctx}");
}

var propType = prop.GetType();
// if prop is an InputList<T>
if (propType.IsGenericType && propType.GetGenericTypeDefinition() == typeof(InputList<>))
{
// pull off the Value property from the InputList<T>
var inputList = propType.GetProperty("Value", BindingFlags.NonPublic | BindingFlags.Instance)?.GetValue(prop);
return await SerializeAsync(ctx, inputList, keepResources, keepOutputValues).ConfigureAwait(false);
}
// if prop is an InputMap<T>
if (propType.IsGenericType && propType.GetGenericTypeDefinition() == typeof(InputMap<>))
{
// pull off the Value property from the InputMap<T>
var inputList = propType.GetProperty("Value", BindingFlags.NonPublic | BindingFlags.Instance)?.GetValue(prop);
return await SerializeAsync(ctx, inputList, keepResources, keepOutputValues).ConfigureAwait(false);
}

if (prop is IInput input)
{
if (_excessiveDebugOutput)
Expand Down Expand Up @@ -289,7 +305,6 @@ prop is double ||
return null;
}

var propType = prop.GetType();
if (propType.IsValueType && propType.GetCustomAttribute<EnumTypeAttribute>() != null)
{
var mi = propType.GetMethod("op_Explicit", BindingFlags.Public | BindingFlags.Static, null, new[] { propType }, null);
Expand Down

0 comments on commit f7020ec

Please sign in to comment.