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

Don't flatten outputs to top level for InputList and InputMap #449

Merged
merged 7 commits into from
Jan 31, 2025
Merged

Don't flatten outputs to top level for InputList and InputMap #449

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
ebb94d1
Add InputList tests
Frassle Jan 29, 2025
c5e6278
Fix InputList to not flatten
Frassle Jan 29, 2025
de4c3b4
Add InputMap tests
Frassle Jan 29, 2025
d2476b5
Fix InputMap to not flatten
Frassle Jan 29, 2025
4fe12ab
Add changelog
Frassle Jan 29, 2025
fe2d32f
format
Frassle Jan 29, 2025
e82f9b7
Add comments
Frassle Jan 31, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .changes/unreleased/Improvements-449.yaml
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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>>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reckon it's worth extending the doc comment to include some of the awesome context you put in the PR description? Or perhaps using that as a comment for the Value property/_inputValue instance variable if you think it's better kept private/internal?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On Value for sure, not sure it can be sensibly added to the user level description.

{
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;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm guessing Output.Concat does an Apply internally, so this use of Apply doesn't change "knownness" or anything like that?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeh, if we just Concat'd the whole thing we'd end up lifting all the sub-element secret/known bits to the top level.

}

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
View file
Open in desktop
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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comments as for list around doc comments/use of Apply

{
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
View file
Open in desktop
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
View file
Open in desktop
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
Loading