Adding secret support to webhooks (#440)

### Summary
- Added logic to handle ciphertext + plaintext way to handle secret
value, preventing drift on webhook secrets
- Moved shared logic into secret_util file for further reuse

### Testing
- Updated integ tests to use secret value

Note: do NOT merge until ciphertext change rolls out to prod

CHANGELOG_PENDING.md

@@ -2,4 +2,6 @@
 
 ### Bug Fixes
 
+- Fixed eternal drift in Webhook resource when `secret` field is supplied [#369](https://github.com/pulumi/pulumi-pulumiservice/issues/369)
+
 ### Miscellaneous

examples/ts-webhooks/index.ts

@@ -21,6 +21,7 @@ const webhookAllEvents = new service.Webhook("org-webhook-all", {
   displayName: "webhook-from-provider",
   organizationName: serviceOrg,
   payloadUrl: "https://google.com",
+  secret: config.require("digits"),
 });
 
 // Organization webhook only subscribed to environments and stacks groups
@@ -29,7 +30,8 @@ const webhook = new service.Webhook("org-webhook-groups", {
   displayName: "webhook-from-provider",
   organizationName: serviceOrg,
   payloadUrl: "https://google.com",
-  groups: [ WebhookGroup.Environments, WebhookGroup.Stacks ]
+  groups: [ WebhookGroup.Environments, WebhookGroup.Stacks ],
+  secret: config.require("digits"),
 });
 
 // Stack webhook subscribed to a group and specific filters
@@ -43,6 +45,7 @@ const stackWebhook = new service.Webhook("stack-webhook", {
   format: WebhookFormat.Slack,
   groups: [ WebhookGroup.Stacks ],
   filters: [WebhookFilters.DeploymentStarted, WebhookFilters.DeploymentSucceeded],
+  secret: config.require("digits"),
 })
 
 // Environment webhook subscribed to specific filters only
@@ -54,4 +57,5 @@ const environmentWebhook = new service.Webhook("env-webhook", {
   environmentName: environment.name,
   payloadUrl: "https://example.com",
   filters: [WebhookFilters.EnvironmentRevisionCreated, WebhookFilters.ImportedEnvironmentChanged],
+  secret: config.require("digits"),
 })

examples/yaml-webhooks/Pulumi.yaml

@@ -10,6 +10,7 @@ resources:
       displayName: yaml-webhook
       organizationName: service-provider-test-org
       payloadUrl: "https://google.com"
+      secret: super-secret
 
 outputs:
   # export the name of the webhook

provider/pkg/internal/pulumiapi/webhooks.go

@@ -25,19 +25,21 @@ type WebhookClient interface {
 	CreateWebhook(ctx context.Context, req WebhookRequest) (*Webhook, error)
 	ListWebhooks(ctx context.Context, orgName string, projectName, stackName, environmentName *string) ([]Webhook, error)
 	GetWebhook(ctx context.Context, orgName string, projectName, stackName, environmentName *string, webhookName string) (*Webhook, error)
-	UpdateWebhook(ctx context.Context, req UpdateWebhookRequest) error
+	UpdateWebhook(ctx context.Context, req UpdateWebhookRequest) (*Webhook, error)
 	DeleteWebhook(ctx context.Context, orgName string, projectName, stackName, environmentName *string, name string) error
 }
 
 type Webhook struct {
-	Active      bool
-	DisplayName string
-	PayloadUrl  string
-	Secret      *string
-	Name        string
-	Format      string
-	Filters     []string
-	Groups      []string
+	Active           bool
+	DisplayName      string
+	PayloadUrl       string
+	Secret           *string
+	Name             string
+	Format           string
+	Filters          []string
+	Groups           []string
+	HasSecret        bool
+	SecretCiphertext string
 }
 
 type WebhookRequest struct {
@@ -123,27 +125,28 @@ func (c *Client) GetWebhook(ctx context.Context,
 	return &webhook, nil
 }
 
-func (c *Client) UpdateWebhook(ctx context.Context, req UpdateWebhookRequest) error {
+func (c *Client) UpdateWebhook(ctx context.Context, req UpdateWebhookRequest) (*Webhook, error) {
 	if len(req.Name) == 0 {
-		return errors.New("name must not be empty")
+		return nil, errors.New("name must not be empty")
 	}
 	if len(req.OrganizationName) == 0 {
-		return errors.New("orgname must not be empty")
+		return nil, errors.New("orgname must not be empty")
 	}
 	if len(req.DisplayName) == 0 {
-		return errors.New("displayname must not be empty")
+		return nil, errors.New("displayname must not be empty")
 	}
 	if len(req.PayloadURL) == 0 {
-		return errors.New("payloadurl must not be empty")
+		return nil, errors.New("payloadurl must not be empty")
 	}
 
 	apiPath := constructApiPath(req.OrganizationName, req.ProjectName, req.StackName, req.EnvironmentName) + "/" + req.Name
 
-	_, err := c.do(ctx, http.MethodPatch, apiPath, req, nil)
+	var webhook Webhook
+	_, err := c.do(ctx, http.MethodPatch, apiPath, req, &webhook)
 	if err != nil {
-		return fmt.Errorf("failed to update webhook: %w", err)
+		return nil, fmt.Errorf("failed to update webhook: %w", err)
 	}
-	return nil
+	return &webhook, nil
 }
 
 func (c *Client) DeleteWebhook(ctx context.Context, orgName string, projectName, stackName, environmentName *string, name string) error {

provider/pkg/internal/pulumiapi/webhooks_test.go

@@ -191,8 +191,9 @@ func TestUpdateWebhook(t *testing.T) {
 			ResponseBody:      webhook,
 		})
 		defer cleanup()
-		err := c.UpdateWebhook(ctx, updateReq)
+		response, err := c.UpdateWebhook(ctx, updateReq)
 		assert.NoError(t, err)
+		assert.EqualValues(t, webhook, *response)
 	})
 
 	t.Run("error", func(t *testing.T) {
@@ -206,7 +207,7 @@ func TestUpdateWebhook(t *testing.T) {
 			},
 		})
 		defer cleanup()
-		err := c.UpdateWebhook(ctx, updateReq)
+		_, err := c.UpdateWebhook(ctx, updateReq)
 		assert.EqualError(t, err, "failed to update webhook: 401 API error: unauthorized")
 	})
 }

provider/pkg/provider/deployment_settings.go

@@ -268,50 +268,12 @@ func (ds *PulumiServiceDeploymentSettingsInput) ToPropertyMap(plaintextInputSett
 	if ds.CacheOptions != nil {
 		coMap := resource.PropertyMap{}
 		coMap["enable"] = resource.NewPropertyValue(ds.CacheOptions.Enable)
-		pm["cacheOptions"] = resource.PropertyValue{coMap}
+		pm["cacheOptions"] = resource.PropertyValue{V: coMap}
 	}
 
 	return pm
 }
 
-// All imported inputs will have a dummy value, asking to be replaced in real code
-// All imported properties are just set to ciphertext read from Pulumi Service
-func importSecretValue(propertyMap resource.PropertyMap, propertyName string, cipherValue pulumiapi.SecretValue, isInput bool) {
-	if isInput {
-		propertyMap[resource.PropertyKey(propertyName)] = resource.MakeSecret(resource.NewPropertyValue(replaceMe))
-	} else {
-		propertyMap[resource.PropertyKey(propertyName)] = resource.NewPropertyValue(cipherValue.Value)
-	}
-}
-
-// On Create or Update, inputs already have a plaintext value, just set it
-// Properties are just set to ciphertext returned from Pulumi Service
-func createSecretValue(propertyMap resource.PropertyMap, propertyName string, cipherValue pulumiapi.SecretValue, plaintextValue pulumiapi.SecretValue, isInput bool) {
-	if isInput {
-		propertyMap[resource.PropertyKey(propertyName)] = resource.MakeSecret(resource.NewPropertyValue(plaintextValue.Value))
-	} else {
-		propertyMap[resource.PropertyKey(propertyName)] = resource.NewPropertyValue(cipherValue.Value)
-	}
-}
-
-// Merge happens when existing resource is refreshed from Pulumi Service
-// Output properties are just replaced with ciphertext retrieved from Pulumi Service
-// Inputs are more complicated :
-// If ciphertext never changed, keep existing plaintext value
-// If ciphertext is different, set plaintext to empty string
-// If retrieved state has a value that current state does not have, pass in nil, which will fill plaintext with empty string
-func mergeSecretValue(propertyMap resource.PropertyMap, propertyName string, cipherValue pulumiapi.SecretValue, plaintextValue *pulumiapi.SecretValue, oldCipherValue *pulumiapi.SecretValue, isInput bool) {
-	if isInput {
-		if oldCipherValue != nil && cipherValue.Value == oldCipherValue.Value {
-			propertyMap[resource.PropertyKey(propertyName)] = resource.MakeSecret(resource.NewPropertyValue(plaintextValue.Value))
-		} else {
-			propertyMap[resource.PropertyKey(propertyName)] = resource.MakeSecret(resource.NewPropertyValue(""))
-		}
-	} else {
-		propertyMap[resource.PropertyKey(propertyName)] = resource.NewPropertyValue(cipherValue.Value)
-	}
-}
-
 type PulumiServiceDeploymentSettingsResource struct {
 	client pulumiapi.DeploymentSettingsClient
 }
@@ -614,17 +576,6 @@ func toCacheOptions(inputMap resource.PropertyMap) *pulumiapi.CacheOptions {
 	return &co
 }
 
-func getSecretOrStringValue(prop resource.PropertyValue) string {
-	switch prop.V.(type) {
-	case *resource.Secret:
-		return prop.SecretValue().Element.StringValue()
-	case nil:
-		return ""
-	default:
-		return prop.StringValue()
-	}
-}
-
 func (ds *PulumiServiceDeploymentSettingsResource) Diff(req *pulumirpc.DiffRequest) (*pulumirpc.DiffResponse, error) {
 	olds, err := plugin.UnmarshalProperties(req.GetOldInputs(), plugin.MarshalOptions{KeepUnknowns: true, SkipNulls: true})
 	if err != nil {

provider/pkg/provider/secret_util.go

@@ -0,0 +1,68 @@
+package provider
+
+import (
+	"github.com/pulumi/pulumi-pulumiservice/provider/pkg/internal/pulumiapi"
+	"github.com/pulumi/pulumi/sdk/v3/go/common/resource"
+)
+
+func getSecretOrStringValue(prop resource.PropertyValue) string {
+	switch prop.V.(type) {
+	case *resource.Secret:
+		return prop.SecretValue().Element.StringValue()
+	case nil:
+		return ""
+	default:
+		return prop.StringValue()
+	}
+}
+
+func getSecretOrStringNullableValue(prop resource.PropertyValue) *string {
+	var resultString string
+	switch prop.V.(type) {
+	case *resource.Secret:
+		resultString = prop.SecretValue().Element.StringValue()
+	case nil:
+		return nil
+	default:
+		resultString = prop.StringValue()
+	}
+	return &resultString
+}
+
+// All imported inputs will have a dummy value, asking to be replaced in real code
+// All imported properties are just set to ciphertext read from Pulumi Service
+func importSecretValue(propertyMap resource.PropertyMap, propertyName string, cipherValue pulumiapi.SecretValue, isInput bool) {
+	if isInput {
+		propertyMap[resource.PropertyKey(propertyName)] = resource.MakeSecret(resource.NewPropertyValue(replaceMe))
+	} else {
+		propertyMap[resource.PropertyKey(propertyName)] = resource.NewPropertyValue(cipherValue.Value)
+	}
+}
+
+// On Create or Update, inputs already have a plaintext value, just set it
+// Properties are just set to ciphertext returned from Pulumi Service
+func createSecretValue(propertyMap resource.PropertyMap, propertyName string, cipherValue pulumiapi.SecretValue, plaintextValue pulumiapi.SecretValue, isInput bool) {
+	if isInput {
+		propertyMap[resource.PropertyKey(propertyName)] = resource.MakeSecret(resource.NewPropertyValue(plaintextValue.Value))
+	} else {
+		propertyMap[resource.PropertyKey(propertyName)] = resource.NewPropertyValue(cipherValue.Value)
+	}
+}
+
+// Merge happens when existing resource is refreshed from Pulumi Service
+// Output properties are just replaced with ciphertext retrieved from Pulumi Service
+// Inputs are more complicated :
+// If ciphertext never changed, keep existing plaintext value
+// If ciphertext is different, set plaintext to empty string
+// If retrieved state has a value that current state does not have, pass in nil, which will fill plaintext with empty string
+func mergeSecretValue(propertyMap resource.PropertyMap, propertyName string, cipherValue pulumiapi.SecretValue, plaintextValue *pulumiapi.SecretValue, oldCipherValue *pulumiapi.SecretValue, isInput bool) {
+	if isInput {
+		if oldCipherValue != nil && cipherValue.Value == oldCipherValue.Value {
+			propertyMap[resource.PropertyKey(propertyName)] = resource.MakeSecret(resource.NewPropertyValue(plaintextValue.Value))
+		} else {
+			propertyMap[resource.PropertyKey(propertyName)] = resource.MakeSecret(resource.NewPropertyValue(""))
+		}
+	} else {
+		propertyMap[resource.PropertyKey(propertyName)] = resource.NewPropertyValue(cipherValue.Value)
+	}
+}