Allow overriding PATCH vs PUT for updates of select resources

examples/examples_nodejs_test.go

@@ -12,6 +12,7 @@ import (
 	"testing"
 
 	"github.com/pulumi/pulumi/pkg/v3/testing/integration"
+	"github.com/pulumi/pulumi/sdk/v3/go/common/resource"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
@@ -76,11 +77,45 @@ func TestImportTs(t *testing.T) {
 }
 
 func TestPostgresTs(t *testing.T) {
-	// t.Skip("takes longer than 10 minutes and can fail with 'unexpected error', issue #898")
 	skipIfShort(t)
+	var initialServerId resource.ID
 	test := getJSBaseOptions(t).
 		With(integration.ProgramTestOptions{
 			Dir: filepath.Join(getCwd(t), "postgres"),
+			ExtraRuntimeValidation: func(t *testing.T, stackInfo integration.RuntimeValidationStackInfo) {
+				require.NotNil(t, stackInfo.Deployment)
+				require.NotNil(t, stackInfo.Deployment.Resources)
+
+				found := false
+				for _, resource := range stackInfo.Deployment.Resources {
+					if resource.Type == "azure-native:dbforpostgresql:Server" {
+						found = true
+						initialServerId = resource.ID
+						break
+					}
+				}
+				assert.True(t, found, "no storage account found in deployed resources")
+			},
+			EditDirs: []integration.EditDir{
+				{
+					Dir:      filepath.Join("postgres", "step2-update-pw"),
+					Additive: true,
+					ExtraRuntimeValidation: func(t *testing.T, stackInfo integration.RuntimeValidationStackInfo) {
+						require.NotNil(t, stackInfo.Deployment)
+						require.NotNil(t, stackInfo.Deployment.Resources)
+
+						found := false
+						for _, resource := range stackInfo.Deployment.Resources {
+							if resource.Type == "azure-native:dbforpostgresql:Server" {
+								found = true
+								assert.Equal(t, initialServerId, resource.ID)
+								break
+							}
+						}
+						assert.True(t, found, "no storage account found in deployed resources")
+					},
+				},
+			},
 		})
 
 	integration.ProgramTest(t, &test)

examples/postgres/index.ts

@@ -6,7 +6,7 @@ import * as resources from "@pulumi/azure-native/resources";
 const resourceGroup = new resources.ResourceGroup("rg", {
     // westus2 was not available for Postgres Flexible Server at the time of writing, see
     // https://learn.microsoft.com/en-us/azure/postgresql/flexible-server/overview#azure-regions
-    location: "westus",
+    location: "eastus2",
 });
 
 const flexibleServer = new postgresql.Server("server", {
@@ -24,10 +24,10 @@ const flexibleServer = new postgresql.Server("server", {
     version: "14",
     storage: {
         storageSizeGB: 64,
-    },
+    }
 });
 
-// Blocked tu to #898
+// Blocked due to #898
 // new postgresql.Configuration("ssl", {
 //     resourceGroupName: resourceGroup.name,
 //     serverName: flexibleServer.name,

examples/postgres/step2-update-pw/index.ts

@@ -0,0 +1,40 @@
+// Copyright 2023, Pulumi Corporation.  All rights reserved.
+
+import * as postgresql from "@pulumi/azure-native/dbforpostgresql";
+import * as resources from "@pulumi/azure-native/resources";
+
+const resourceGroup = new resources.ResourceGroup("rg", {
+    // westus2 was not available for Postgres Flexible Server at the time of writing, see
+    // https://learn.microsoft.com/en-us/azure/postgresql/flexible-server/overview#azure-regions
+    location: "eastus2",
+});
+
+const flexibleServer = new postgresql.Server("server", {
+    location: resourceGroup.location,
+    resourceGroupName: resourceGroup.name,
+    sku: {
+        tier: "GeneralPurpose",
+        name: "Standard_D2s_v3",
+    },
+    highAvailability: {
+        mode: postgresql.HighAvailabilityMode.SameZone,
+    },
+    administratorLogin: "cloudsa",
+    administratorLoginPassword: `NEW pa$$w0rd`,
+    version: "14",
+    storage: {
+        storageSizeGB: 64,
+    }
+});
+
+// Blocked due to #898
+// new postgresql.Configuration("ssl", {
+//     resourceGroupName: resourceGroup.name,
+//     serverName: flexibleServer.name,
+//     configurationName: "ssl",
+//     value: "on",
+//     source: "user-override",
+// });
+
+export const resourceGroupName = resourceGroup.name;
+export const serverName = flexibleServer.name;

provider/pkg/gen/schema.go

@@ -858,6 +858,27 @@ func (g *packageGenerator) mergeResourcePathsInto(resourcePaths map[openapi.Reso
 	}
 }
 
+// determineCreateAndUpdateOperation returns the operation to be used for resource creation (first return value), and
+// the HTTP method (PUT or PATCH) to be used for updates. The create operation can be PATCH in rare cases, when the
+// resource always exists and is only brought into Pulumi state by creating it.
+func determineCreateAndUpdateOperation(tok string, resource *resourceVariant) (*spec.Operation, string, error) {
+	path := resource.PathItem
+	module, _, name, err := resources.ParseToken(tok)
+	if err != nil {
+		return nil, "", err
+	}
+
+	// #3961
+	if module == "dbforpostgresql" && name == "Server" && path.Patch != nil {
+		return path.Put, "PATCH", nil
+	}
+
+	if path.Put == nil {
+		return path.Patch, "PATCH", nil
+	}
+	return path.Put, "", nil
+}
+
 func (g *packageGenerator) genResourceVariant(apiSpec *openapi.ResourceSpec, resource *resourceVariant, nestedResourceBodyRefs []string, typeNameAliases ...string) error {
 	module := g.moduleWithVersion()
 	swagger := resource.Swagger
@@ -892,14 +913,12 @@ func (g *packageGenerator) genResourceVariant(apiSpec *openapi.ResourceSpec, res
 		flattenedPropertyConflicts: map[string]any{},
 	}
 
-	updateOp := path.Put
-	updateMethod := ""
-	if path.Put == nil {
-		updateOp = path.Patch
-		updateMethod = "PATCH"
+	createOp, updateMethod, err := determineCreateAndUpdateOperation(resourceTok, resource)
+	if err != nil {
+		return errors.Wrapf(err, "failed to determine update operation for '%s'", resourceTok)
 	}
 
-	resourceResponse, err := gen.genResponse(updateOp.Responses.StatusCodeResponses, swagger.ReferenceContext, resource.response)
+	resourceResponse, err := gen.genResponse(createOp.Responses.StatusCodeResponses, swagger.ReferenceContext, resource.response)
 	if err != nil {
 		return errors.Wrapf(err, "failed to generate '%s': response type", resourceTok)
 	}
@@ -909,7 +928,7 @@ func (g *packageGenerator) genResourceVariant(apiSpec *openapi.ResourceSpec, res
 		return nil
 	}
 
-	parameters := resource.Swagger.MergeParameters(updateOp.Parameters, path.Parameters)
+	parameters := resource.Swagger.MergeParameters(createOp.Parameters, path.Parameters)
 	autoNamer := resources.NewAutoNamer(resource.Path)
 
 	resourceRequest, err := gen.genMethodParameters(parameters, swagger.ReferenceContext, &autoNamer, resource.body)
@@ -1030,7 +1049,7 @@ func (g *packageGenerator) genResourceVariant(apiSpec *openapi.ResourceSpec, res
 		Response:             resourceResponse.properties,
 		DefaultBody:          resource.DefaultBody,
 		Singleton:            isSingleton(resource),
-		PutAsyncStyle:        g.getAsyncStyle(updateOp),
+		PutAsyncStyle:        g.getAsyncStyle(createOp),
 		DeleteAsyncStyle:     g.getAsyncStyle(resource.PathItem.Delete),
 		ReadMethod:           readMethod,
 		ReadPath:             readPath,

provider/pkg/gen/schema_test.go

@@ -404,3 +404,83 @@ func TestResourcePathTracker(t *testing.T) {
 		assert.True(t, tracker.hasConflicts())
 	})
 }
+
+func TestDetermineUpdateOperation(t *testing.T) {
+	t.Run("overrides to use PATCH", func(t *testing.T) {
+		op, method, err := determineCreateAndUpdateOperation("azure-native:dbforpostgresql:Server", &resourceVariant{
+			ResourceSpec: &openapi.ResourceSpec{
+				PathItem: &spec.PathItem{
+					PathItemProps: spec.PathItemProps{
+						Put:   &spec.Operation{},
+						Patch: &spec.Operation{},
+					},
+				},
+			},
+		})
+		require.NoError(t, err)
+		assert.NotNil(t, op)
+		assert.Equal(t, "PATCH", method)
+	})
+
+	t.Run("overrides to use PATCH, versioned", func(t *testing.T) {
+		op, method, err := determineCreateAndUpdateOperation("azure-native:dbforpostgresql/v20240801:Server", &resourceVariant{
+			ResourceSpec: &openapi.ResourceSpec{
+				PathItem: &spec.PathItem{
+					PathItemProps: spec.PathItemProps{
+						Put:   &spec.Operation{},
+						Patch: &spec.Operation{},
+					},
+				},
+			},
+		})
+		require.NoError(t, err)
+		assert.NotNil(t, op)
+		assert.Equal(t, "PATCH", method)
+	})
+
+	t.Run("prefers PUT over PATCH", func(t *testing.T) {
+		op, method, err := determineCreateAndUpdateOperation("azure-native:foo:bar", &resourceVariant{
+			ResourceSpec: &openapi.ResourceSpec{
+				PathItem: &spec.PathItem{
+					PathItemProps: spec.PathItemProps{
+						Put:   &spec.Operation{},
+						Patch: &spec.Operation{},
+					},
+				},
+			},
+		})
+		require.NoError(t, err)
+		assert.NotNil(t, op)
+		assert.Equal(t, "", method)
+	})
+
+	t.Run("only PUT", func(t *testing.T) {
+		op, method, err := determineCreateAndUpdateOperation("azure-native:foo:bar", &resourceVariant{
+			ResourceSpec: &openapi.ResourceSpec{
+				PathItem: &spec.PathItem{
+					PathItemProps: spec.PathItemProps{
+						Put: &spec.Operation{},
+					},
+				},
+			},
+		})
+		require.NoError(t, err)
+		assert.NotNil(t, op)
+		assert.Equal(t, "", method)
+	})
+
+	t.Run("only PATCH", func(t *testing.T) {
+		op, method, err := determineCreateAndUpdateOperation("azure-native:foo:bar", &resourceVariant{
+			ResourceSpec: &openapi.ResourceSpec{
+				PathItem: &spec.PathItem{
+					PathItemProps: spec.PathItemProps{
+						Patch: &spec.Operation{},
+					},
+				},
+			},
+		})
+		require.NoError(t, err)
+		assert.NotNil(t, op)
+		assert.Equal(t, "PATCH", method)
+	})
+}

provider/pkg/provider/crud/crud.go

@@ -42,7 +42,8 @@ type AzureRESTConverter interface {
 // It operates in the context of a specific kind of Azure resource of type resources.AzureAPIResource.
 type ResourceCrudOperations interface {
 	CanCreate(ctx context.Context, id string) error
-	CreateOrUpdate(ctx context.Context, id string, bodyParams, queryParams map[string]any) (map[string]any, bool, error)
+	Create(ctx context.Context, id string, bodyParams, queryParams map[string]any) (map[string]any, bool, error)
+	Update(ctx context.Context, id string, bodyParams, queryParams map[string]any) (map[string]any, bool, error)
 	Read(ctx context.Context, id string) (map[string]any, error)
 	HandleErrorWithCheckpoint(ctx context.Context, err error, id string, inputs resource.PropertyMap, properties *structpb.Struct) error
 }
@@ -377,8 +378,13 @@ func (r *resourceCrudClient) CanCreate(ctx context.Context, id string) error {
 	})
 }
 
-func (r *resourceCrudClient) CreateOrUpdate(ctx context.Context, id string, bodyParams, queryParams map[string]any) (map[string]any, bool, error) {
-	// Submit the `PUT` or `PATCH` against the ARM endpoint
+func (r *resourceCrudClient) Create(ctx context.Context, id string, bodyParams, queryParams map[string]any) (map[string]any, bool, error) {
+	// Submit the `PUT` request against the ARM endpoint
+	return r.azureClient.Put(ctx, id, bodyParams, queryParams, r.res.PutAsyncStyle)
+}
+
+func (r *resourceCrudClient) Update(ctx context.Context, id string, bodyParams, queryParams map[string]any) (map[string]any, bool, error) {
+	// Submit the `PUT` or `PATCH` request against the ARM endpoint
 	op := r.azureClient.Put
 	if r.res.UpdateMethod == "PATCH" {
 		op = r.azureClient.Patch

provider/pkg/provider/provider.go

@@ -1033,7 +1033,7 @@ func (k *azureNativeProvider) defaultCreate(ctx context.Context, req *rpc.Create
 
 	crudClient.SetUnsetSubresourcePropertiesToDefaults(bodyParams, bodyParams, true)
 
-	response, created, err := crudClient.CreateOrUpdate(ctx, id, bodyParams, queryParams)
+	response, created, err := crudClient.Create(ctx, id, bodyParams, queryParams)
 	if err != nil {
 		if created {
 			return id, nil, crudClient.HandleErrorWithCheckpoint(ctx, err, id, inputs, req.GetProperties())
@@ -1467,7 +1467,7 @@ func (k *azureNativeProvider) defaultUpdate(ctx context.Context, req *rpc.Update
 		return nil, fmt.Errorf("failed maintaining unset sub-resource properties: %w", err)
 	}
 
-	response, updated, err := crudClient.CreateOrUpdate(ctx, id, bodyParams, queryParams)
+	response, updated, err := crudClient.Update(ctx, id, bodyParams, queryParams)
 	if err != nil {
 		if updated {
 			return nil, crudClient.HandleErrorWithCheckpoint(ctx, err, id, inputs, req.GetNews())

provider/pkg/resources/customresources/custom_pim.go

@@ -40,7 +40,7 @@ func (c *roleManagementPolicyClient) create(ctx context.Context, id string, inpu
 
 	// We could skip this if bodyParams = originalState, i.e., the user adds a policy
 	// in its default configuration to their program, but we don't have a diff function.
-	resp, _, err := c.client.CreateOrUpdate(ctx, id, bodyParams, queryParams)
+	resp, _, err := c.client.Update(ctx, id, bodyParams, queryParams)
 	if err != nil {
 		return nil, err
 	}
@@ -63,7 +63,7 @@ func (c *roleManagementPolicyClient) update(ctx context.Context, id string, news
 	}
 	queryParams := map[string]any{"api-version": c.client.ApiVersion()}
 
-	resp, _, err := c.client.CreateOrUpdate(ctx, id, bodyParams, queryParams)
+	resp, _, err := c.client.Update(ctx, id, bodyParams, queryParams)
 	if err != nil {
 		return nil, err
 	}
@@ -93,7 +93,7 @@ func (c *roleManagementPolicyClient) delete(ctx context.Context, id string, prev
 	}
 
 	queryParams := map[string]any{"api-version": c.client.ApiVersion()}
-	_, _, err = c.client.CreateOrUpdate(ctx, id, origRequest, queryParams)
+	_, _, err = c.client.Update(ctx, id, origRequest, queryParams)
 	return err
 }
 

provider/pkg/resources/customresources/custom_postgres_config.go

@@ -54,7 +54,7 @@ func postgresFlexibleServerConfiguration(crudClientFactory crud.ResourceCrudClie
 			if err != nil {
 				return err
 			}
-			result, _, err := crudClient.CreateOrUpdate(ctx, id, map[string]any{
+			result, _, err := crudClient.Update(ctx, id, map[string]any{
 				"properties": map[string]any{
 					"value":  defaultValue,
 					"source": source,