From 1bd61e4d64862cc0a7a2e54dae73fb073a9f5bd1 Mon Sep 17 00:00:00 2001 From: Guinevere Saenger Date: Tue, 14 Feb 2023 17:31:49 -0800 Subject: [PATCH] Ignore input changes to username and password (#498) * Intercept registry diff and handle its fields as a first pass at better diff * Return NONE for diffResponse if the only updates are to usernae or password * Separate out detailed diff functionality in separate function for unit test * Add unit test for diffUpdates * Add documentation * Typos in docs line Co-authored-by: Aaron Friel * Regenerate schema and SDKs --------- Co-authored-by: Aaron Friel --- .../cmd/pulumi-resource-docker/schema.json | 4 +- provider/provider.go | 37 +++++- provider/provider_test.go | 108 ++++++++++++++++++ provider/resources.go | 12 +- sdk/dotnet/Inputs/RegistryArgs.cs | 4 +- sdk/go/docker/pulumiTypes.go | 16 +-- .../pulumi/docker/inputs/RegistryArgs.java | 16 +-- sdk/nodejs/types/input.ts | 4 +- sdk/python/pulumi_docker/_inputs.py | 8 +- 9 files changed, 176 insertions(+), 33 deletions(-) create mode 100644 provider/provider_test.go diff --git a/provider/cmd/pulumi-resource-docker/schema.json b/provider/cmd/pulumi-resource-docker/schema.json index 513dec63..e47ad55a 100644 --- a/provider/cmd/pulumi-resource-docker/schema.json +++ b/provider/cmd/pulumi-resource-docker/schema.json @@ -3130,7 +3130,7 @@ "properties": { "password": { "type": "string", - "description": "The password to authenticate to the registry", + "description": "The password to authenticate to the registry. Does not cause image rebuild when changed.", "secret": true }, "server": { @@ -3139,7 +3139,7 @@ }, "username": { "type": "string", - "description": "The username to authenticate to the registry" + "description": "The username to authenticate to the registry. Does not cause image rebuild when changed." } }, "type": "object" diff --git a/provider/provider.go b/provider/provider.go index dc4b9b74..2ded7900 100644 --- a/provider/provider.go +++ b/provider/provider.go @@ -214,8 +214,19 @@ func (p *dockerNativeProvider) Diff(ctx context.Context, req *rpc.DiffRequest) ( for key := range d.Deletes { diff[string(key)] = &rpc.PropertyDiff{Kind: rpc.PropertyDiff_DELETE} } - for key := range d.Updates { - diff[string(key)] = &rpc.PropertyDiff{Kind: rpc.PropertyDiff_UPDATE} + + detailedUpdates := diffUpdates(d.Updates) + + // merge detailedUpdates into diff + for k, v := range detailedUpdates { + diff[k] = v + } + + // if diff is empty, it means we skipped any changes to username and password + if len(diff) == 0 { + return &rpc.DiffResponse{ + Changes: rpc.DiffResponse_DIFF_NONE, + }, nil } return &rpc.DiffResponse{ Changes: rpc.DiffResponse_DIFF_SOME, @@ -224,6 +235,28 @@ func (p *dockerNativeProvider) Diff(ctx context.Context, req *rpc.DiffRequest) ( }, nil } +func diffUpdates(updates map[resource.PropertyKey]resource.ValueDiff) map[string]*rpc.PropertyDiff { + updateDiff := map[string]*rpc.PropertyDiff{} + for key, valueDiff := range updates { + if string(key) != "registry" { + updateDiff[string(key)] = &rpc.PropertyDiff{ + Kind: rpc.PropertyDiff_UPDATE, + } + } else { + // only register a diff on "server" field, but not on "username" or "password", + // as they can change frequently and should not trigger a rebuild. + serverDiff := valueDiff.Object.Updates["server"] + // if serverDiff is not empty, we register a property diff update + if serverDiff != (resource.ValueDiff{}) { + updateDiff[string(key)] = &rpc.PropertyDiff{ + Kind: rpc.PropertyDiff_UPDATE, + } + } + } + } + return updateDiff +} + // Create allocates a new instance of the provided resource and returns its unique ID afterwards. func (p *dockerNativeProvider) Create(ctx context.Context, req *rpc.CreateRequest) (*rpc.CreateResponse, error) { urn := resource.URN(req.GetUrn()) diff --git a/provider/provider_test.go b/provider/provider_test.go new file mode 100644 index 00000000..79904378 --- /dev/null +++ b/provider/provider_test.go @@ -0,0 +1,108 @@ +package provider + +import ( + "github.com/pulumi/pulumi/sdk/v3/go/common/resource" + rpc "github.com/pulumi/pulumi/sdk/v3/proto/go" + "github.com/stretchr/testify/assert" + "testing" +) + +func TestDiffUpdates(t *testing.T) { + + t.Run("No diff happens on changed password", func(t *testing.T) { + expected := map[string]*rpc.PropertyDiff{} + input := map[resource.PropertyKey]resource.ValueDiff{ + "registry": { + Object: &resource.ObjectDiff{ + Updates: map[resource.PropertyKey]resource.ValueDiff{"password": { + Old: resource.PropertyValue{ + V: "FancyToken", + }, + New: resource.PropertyValue{ + V: "PedestrianPassword", + }, + Array: (*resource.ArrayDiff)(nil), + Object: (*resource.ObjectDiff)(nil), + }}, + }, + }, + } + actual := diffUpdates(input) + assert.Equal(t, expected, actual) + }) + + t.Run("No diff happens on changed username", func(t *testing.T) { + expected := map[string]*rpc.PropertyDiff{} + input := map[resource.PropertyKey]resource.ValueDiff{ + "registry": { + Object: &resource.ObjectDiff{ + Updates: map[resource.PropertyKey]resource.ValueDiff{"username": { + Old: resource.PropertyValue{ + V: "platypus", + }, + New: resource.PropertyValue{ + V: "Schnabeltier", + }, + Array: (*resource.ArrayDiff)(nil), + Object: (*resource.ObjectDiff)(nil), + }}, + }, + }, + } + actual := diffUpdates(input) + assert.Equal(t, expected, actual) + }) + + t.Run("Diff happens on changed server name", func(t *testing.T) { + expected := map[string]*rpc.PropertyDiff{ + "registry": { + Kind: rpc.PropertyDiff_UPDATE, + }, + } + input := map[resource.PropertyKey]resource.ValueDiff{ + "registry": { + Object: &resource.ObjectDiff{ + Updates: map[resource.PropertyKey]resource.ValueDiff{"server": { + Old: resource.PropertyValue{ + V: "dockerhub", + }, + New: resource.PropertyValue{ + V: "ShinyPrivateGHCR", + }, + Array: (*resource.ArrayDiff)(nil), + Object: (*resource.ObjectDiff)(nil), + }}, + }, + }, + } + actual := diffUpdates(input) + assert.Equal(t, expected, actual) + }) + + t.Run("Diff happens on changed build context", func(t *testing.T) { + expected := map[string]*rpc.PropertyDiff{ + "build": { + Kind: rpc.PropertyDiff_UPDATE, + }, + } + input := map[resource.PropertyKey]resource.ValueDiff{ + "build": { + Object: &resource.ObjectDiff{ + Updates: map[resource.PropertyKey]resource.ValueDiff{"contextDigest": { + Old: resource.PropertyValue{ + V: "12345", + }, + New: resource.PropertyValue{ + V: "54321", + }, + Array: (*resource.ArrayDiff)(nil), + Object: (*resource.ObjectDiff)(nil), + }}, + }, + }, + } + actual := diffUpdates(input) + assert.Equal(t, expected, actual) + }) + +} diff --git a/provider/resources.go b/provider/resources.go index 593fc692..bb31106d 100644 --- a/provider/resources.go +++ b/provider/resources.go @@ -125,14 +125,16 @@ func Provider() tfbridge.ProviderInfo { TypeSpec: schema.TypeSpec{Type: "string"}, }, "username": { - Description: "The username to authenticate to the registry", - TypeSpec: schema.TypeSpec{Type: "string"}, + Description: "The username to authenticate to the registry. " + + "Does not cause image rebuild when changed.", + TypeSpec: schema.TypeSpec{Type: "string"}, }, "password": { - Description: "The password to authenticate to the registry", - TypeSpec: schema.TypeSpec{Type: "string"}, - Secret: true, + Description: "The password to authenticate to the registry. " + + "Does not cause image rebuild when changed.", + TypeSpec: schema.TypeSpec{Type: "string"}, + Secret: true, }, }, }, diff --git a/sdk/dotnet/Inputs/RegistryArgs.cs b/sdk/dotnet/Inputs/RegistryArgs.cs index df7ab4c0..05360cc7 100644 --- a/sdk/dotnet/Inputs/RegistryArgs.cs +++ b/sdk/dotnet/Inputs/RegistryArgs.cs @@ -19,7 +19,7 @@ public sealed class RegistryArgs : global::Pulumi.ResourceArgs private Input? _password; /// - /// The password to authenticate to the registry + /// The password to authenticate to the registry. Does not cause image rebuild when changed. /// public Input? Password { @@ -38,7 +38,7 @@ public Input? Password public Input? Server { get; set; } /// - /// The username to authenticate to the registry + /// The username to authenticate to the registry. Does not cause image rebuild when changed. /// [Input("username")] public Input? Username { get; set; } diff --git a/sdk/go/docker/pulumiTypes.go b/sdk/go/docker/pulumiTypes.go index 477ebffb..b81e8eae 100644 --- a/sdk/go/docker/pulumiTypes.go +++ b/sdk/go/docker/pulumiTypes.go @@ -9935,11 +9935,11 @@ func (o GetNetworkIpamConfigArrayOutput) Index(i pulumi.IntInput) GetNetworkIpam // Describes a Docker container registry type Registry struct { - // The password to authenticate to the registry + // The password to authenticate to the registry. Does not cause image rebuild when changed. Password *string `pulumi:"password"` // The URL of the Docker registry server Server *string `pulumi:"server"` - // The username to authenticate to the registry + // The username to authenticate to the registry. Does not cause image rebuild when changed. Username *string `pulumi:"username"` } @@ -9956,11 +9956,11 @@ type RegistryInput interface { // Describes a Docker container registry type RegistryArgs struct { - // The password to authenticate to the registry + // The password to authenticate to the registry. Does not cause image rebuild when changed. Password pulumi.StringPtrInput `pulumi:"password"` // The URL of the Docker registry server Server pulumi.StringPtrInput `pulumi:"server"` - // The username to authenticate to the registry + // The username to authenticate to the registry. Does not cause image rebuild when changed. Username pulumi.StringPtrInput `pulumi:"username"` } @@ -10042,7 +10042,7 @@ func (o RegistryOutput) ToRegistryPtrOutputWithContext(ctx context.Context) Regi }).(RegistryPtrOutput) } -// The password to authenticate to the registry +// The password to authenticate to the registry. Does not cause image rebuild when changed. func (o RegistryOutput) Password() pulumi.StringPtrOutput { return o.ApplyT(func(v Registry) *string { return v.Password }).(pulumi.StringPtrOutput) } @@ -10052,7 +10052,7 @@ func (o RegistryOutput) Server() pulumi.StringPtrOutput { return o.ApplyT(func(v Registry) *string { return v.Server }).(pulumi.StringPtrOutput) } -// The username to authenticate to the registry +// The username to authenticate to the registry. Does not cause image rebuild when changed. func (o RegistryOutput) Username() pulumi.StringPtrOutput { return o.ApplyT(func(v Registry) *string { return v.Username }).(pulumi.StringPtrOutput) } @@ -10081,7 +10081,7 @@ func (o RegistryPtrOutput) Elem() RegistryOutput { }).(RegistryOutput) } -// The password to authenticate to the registry +// The password to authenticate to the registry. Does not cause image rebuild when changed. func (o RegistryPtrOutput) Password() pulumi.StringPtrOutput { return o.ApplyT(func(v *Registry) *string { if v == nil { @@ -10101,7 +10101,7 @@ func (o RegistryPtrOutput) Server() pulumi.StringPtrOutput { }).(pulumi.StringPtrOutput) } -// The username to authenticate to the registry +// The username to authenticate to the registry. Does not cause image rebuild when changed. func (o RegistryPtrOutput) Username() pulumi.StringPtrOutput { return o.ApplyT(func(v *Registry) *string { if v == nil { diff --git a/sdk/java/src/main/java/com/pulumi/docker/inputs/RegistryArgs.java b/sdk/java/src/main/java/com/pulumi/docker/inputs/RegistryArgs.java index bdc5c02c..33a3daaa 100644 --- a/sdk/java/src/main/java/com/pulumi/docker/inputs/RegistryArgs.java +++ b/sdk/java/src/main/java/com/pulumi/docker/inputs/RegistryArgs.java @@ -20,14 +20,14 @@ public final class RegistryArgs extends com.pulumi.resources.ResourceArgs { public static final RegistryArgs Empty = new RegistryArgs(); /** - * The password to authenticate to the registry + * The password to authenticate to the registry. Does not cause image rebuild when changed. * */ @Import(name="password") private @Nullable Output password; /** - * @return The password to authenticate to the registry + * @return The password to authenticate to the registry. Does not cause image rebuild when changed. * */ public Optional> password() { @@ -50,14 +50,14 @@ public Optional> server() { } /** - * The username to authenticate to the registry + * The username to authenticate to the registry. Does not cause image rebuild when changed. * */ @Import(name="username") private @Nullable Output username; /** - * @return The username to authenticate to the registry + * @return The username to authenticate to the registry. Does not cause image rebuild when changed. * */ public Optional> username() { @@ -91,7 +91,7 @@ public Builder(RegistryArgs defaults) { } /** - * @param password The password to authenticate to the registry + * @param password The password to authenticate to the registry. Does not cause image rebuild when changed. * * @return builder * @@ -102,7 +102,7 @@ public Builder password(@Nullable Output password) { } /** - * @param password The password to authenticate to the registry + * @param password The password to authenticate to the registry. Does not cause image rebuild when changed. * * @return builder * @@ -133,7 +133,7 @@ public Builder server(String server) { } /** - * @param username The username to authenticate to the registry + * @param username The username to authenticate to the registry. Does not cause image rebuild when changed. * * @return builder * @@ -144,7 +144,7 @@ public Builder username(@Nullable Output username) { } /** - * @param username The username to authenticate to the registry + * @param username The username to authenticate to the registry. Does not cause image rebuild when changed. * * @return builder * diff --git a/sdk/nodejs/types/input.ts b/sdk/nodejs/types/input.ts index e47bd81d..0bd6336b 100644 --- a/sdk/nodejs/types/input.ts +++ b/sdk/nodejs/types/input.ts @@ -374,7 +374,7 @@ export interface ProviderRegistryAuth { */ export interface Registry { /** - * The password to authenticate to the registry + * The password to authenticate to the registry. Does not cause image rebuild when changed. */ password?: pulumi.Input; /** @@ -382,7 +382,7 @@ export interface Registry { */ server?: pulumi.Input; /** - * The username to authenticate to the registry + * The username to authenticate to the registry. Does not cause image rebuild when changed. */ username?: pulumi.Input; } diff --git a/sdk/python/pulumi_docker/_inputs.py b/sdk/python/pulumi_docker/_inputs.py index a16b4c4f..8c6b79db 100644 --- a/sdk/python/pulumi_docker/_inputs.py +++ b/sdk/python/pulumi_docker/_inputs.py @@ -4185,9 +4185,9 @@ def __init__(__self__, *, username: Optional[pulumi.Input[str]] = None): """ Describes a Docker container registry - :param pulumi.Input[str] password: The password to authenticate to the registry + :param pulumi.Input[str] password: The password to authenticate to the registry. Does not cause image rebuild when changed. :param pulumi.Input[str] server: The URL of the Docker registry server - :param pulumi.Input[str] username: The username to authenticate to the registry + :param pulumi.Input[str] username: The username to authenticate to the registry. Does not cause image rebuild when changed. """ if password is not None: pulumi.set(__self__, "password", password) @@ -4200,7 +4200,7 @@ def __init__(__self__, *, @pulumi.getter def password(self) -> Optional[pulumi.Input[str]]: """ - The password to authenticate to the registry + The password to authenticate to the registry. Does not cause image rebuild when changed. """ return pulumi.get(self, "password") @@ -4224,7 +4224,7 @@ def server(self, value: Optional[pulumi.Input[str]]): @pulumi.getter def username(self) -> Optional[pulumi.Input[str]]: """ - The username to authenticate to the registry + The username to authenticate to the registry. Does not cause image rebuild when changed. """ return pulumi.get(self, "username")