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 <[email protected]> * Regenerate schema and SDKs --------- Co-authored-by: Aaron Friel <[email protected]>
pulumi · Feb 15, 2023 · 1bd61e4 · 1bd61e4
1 parent 4705df3
commit 1bd61e4
Show file tree

Hide file tree

Showing 9 changed files with 176 additions and 33 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -19,7 +19,7 @@ public sealed class RegistryArgs : global::Pulumi.ResourceArgs
         private Input<string>? _password;
 
         /// <summary>
-        /// The password to authenticate to the registry
+        /// The password to authenticate to the registry. Does not cause image rebuild when changed.
         /// </summary>
         public Input<string>? Password
         {
@@ -38,7 +38,7 @@ public Input<string>? Password
         public Input<string>? Server { get; set; }
 
         /// <summary>
-        /// The username to authenticate to the registry
+        /// The username to authenticate to the registry. Does not cause image rebuild when changed.
         /// </summary>
         [Input("username")]
         public Input<string>? Username { get; set; }

diff --git a/sdk/go/docker/pulumiTypes.go b/sdk/go/docker/pulumiTypes.go
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
@@ -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<String> 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<Output<String>> password() {
@@ -50,14 +50,14 @@ public Optional<Output<String>> 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<String> 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<Output<String>> 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<String> 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<String> 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
@@ -374,15 +374,15 @@ 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<string>;
     /**
      * The URL of the Docker registry server
      */
     server?: pulumi.Input<string>;
     /**
-     * The username to authenticate to the registry
+     * The username to authenticate to the registry. Does not cause image rebuild when changed.
      */
     username?: pulumi.Input<string>;
 }

diff --git 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")