Automatically infer an id for pf resources (#3193)

This applies the logic from `dynamic` to `pf` based resources. As more
and more providers are moved to creating new resources with the plugin
framework, more and more of our upgrades are failing due to the missing
id error. This requires manual work for the team to create a `ComputeID`
mapping.

Since this `id` is not actually used for anything in Pulumi, we have
decided to fallback to hardcoding a "missing ID" value. This will reduce
the number of provider upgrades that require manual intervention by the
team and decrease the delay in releasing upgrades.

closes #3182

Author: corymhall

docs/guides/resource-ids.md

@@ -60,8 +60,14 @@ If the type of the `"id"` attribute is not coercible to a string, you must set `
 error: Resource test_res has a problem: no "id" attribute. To map this resource consider specifying ResourceInfo.ComputeID
 ```
 
-If the resource simply doesn't have an `"id"` attribute, you will need to set `ResourceInfo.ComputeID`.
-If you want to delegate the ID field in Pulumi to another attribute, you should use `tfbridge.DelegateIDField` to produce a `ResourceInfo.ComputeID` compatible function.
+This error should not occur anymore. For dynamic providers and Plugin Framework resources the bridge falls back to emitting a placeholder
+value (`"missing ID"`, exported as `tfbridge.MissingIDPlaceholder`) so schema generation and provider execution no longer fail.
+Providing an explicit `ComputeID` keeps the generated provider more aligned with the upstream import story. If you want to delegate the ID field
+in Pulumi to another attribute, you should use `tfbridge.DelegateIDField` to produce a `ResourceInfo.ComputeID` compatible function.
+
+When using `ResourceInfo.ComputeID` we typically map it to the "import id" of the resource, but there is nothing that requires it to be set to a
+specific value. The resource id and the import id do not need to match. Pulumi requires that each resource has an id (Terraform does not), but
+there is not requirement that the id mean anything. This is why we fall back to adding the `"missing ID"` value.
 
 ```go
 "test_res": {ComputeID: tfbridge.DelegateIDField("id", "testprovider", "https://github.com/pulumi/pulumi-testprovider")}

dynamic/internal/fixup/properties.go

@@ -20,7 +20,6 @@
 package fixup
 
 import (
-	"context"
 	"errors"
 	"fmt"
 	"reflect"
@@ -100,16 +99,12 @@ func fixMissingIDs(fixCtx fixCtx, p *info.Provider) error {
 			(id.Type() == shim.TypeString || getIDType(r.Schema) == "string") &&
 			id.Computed()
 		if !ok {
-			r.Schema.ComputeID = missingID
+			r.Schema.ComputeID = tfbridge.MissingIDComputeID()
 		}
 		return nil
 	})
 }
 
-func missingID(context.Context, resource.PropertyMap) (resource.ID, error) {
-	return "missing ID", nil
-}
-
 func fixPropertyConflict(fixCtx fixCtx, p *info.Provider) error {
 	if p.Name == "" {
 		return fmt.Errorf("must set p.Name")

pkg/pf/internal/check/check_test.go

@@ -101,10 +101,8 @@ func TestMissingIDProperty(t *testing.T) {
 		P: pfbridge.ShimProvider(testProvider{missingID: true}),
 	})
 
-	assert.Equal(t, "error: Resource test_res has a problem: no \"id\" attribute. "+
-		"To map this resource consider specifying ResourceInfo.ComputeID to a valid field on the upstream resource\n", stderr)
-
-	assert.ErrorContains(t, err, "There were 1 unresolved ID mapping errors")
+	assert.Equal(t, "", stderr)
+	assert.NoError(t, err)
 }
 
 func TestMissingIDWithOverride(t *testing.T) {

pkg/pf/internal/check/checks.go

@@ -46,7 +46,7 @@ func Provider(sink diag.Sink, info tfbridge.ProviderInfo) error {
 }
 
 func checkIDProperties(sink diag.Sink, info tfbridge.ProviderInfo, isPFResource func(tfToken string) bool) error {
-	errors := 0
+	numErrors := 0
 
 	info.P.ResourcesMap().Range(func(rname string, resource shim.Resource) bool {
 		// Unmapped resources are not available, so they don't need to be correct.
@@ -67,14 +67,32 @@ func checkIDProperties(sink diag.Sink, info tfbridge.ProviderInfo, isPFResource
 			return true
 		}
 
-		errors++
+		// if the resource does not have an id, and the provider author has not
+		// added a ComputeID override, then fallback to using MissingID.
+		var missing errMissingIDAttribute
+		if errors.As(err, &missing) {
+			if info.Resources == nil {
+				info.Resources = map[string]*tfbridge.ResourceInfo{}
+			}
+			resInfo := info.Resources[rname]
+			if resInfo == nil {
+				resInfo = &tfbridge.ResourceInfo{}
+				info.Resources[rname] = resInfo
+			}
+			if resInfo.ComputeID == nil {
+				resInfo.ComputeID = tfbridge.MissingIDComputeID()
+			}
+			return true
+		}
+
+		numErrors++
 		sink.Errorf(&diag.Diag{Message: resourceError{rname, err}.Error()})
 
 		return true
 	})
 
-	if errors > 0 {
-		return fmt.Errorf("There were %d unresolved ID mapping errors", errors)
+	if numErrors > 0 {
+		return fmt.Errorf("There were %d unresolved ID mapping errors", numErrors)
 	}
 
 	return nil

pkg/tfbridge/info.go

@@ -15,6 +15,7 @@
 package tfbridge
 
 import (
+	"context"
 	"fmt"
 	"os"
 	"strings"
@@ -24,7 +25,6 @@ import (
 	"github.com/pulumi/pulumi/sdk/v3/go/common/resource"
 	"github.com/pulumi/pulumi/sdk/v3/go/common/tokens"
 	"github.com/pulumi/pulumi/sdk/v3/go/common/util/contract"
-	"golang.org/x/net/context"
 
 	"github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfbridge/info"
 	"github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfbridge/log"
@@ -35,6 +35,16 @@ type urnCtxKeyType struct{}
 
 var urnCtxKey = urnCtxKeyType{}
 
+// MissingIDPlaceholder is the fallback identifier used when a Terraform resource does not expose an "id" field.
+const MissingIDPlaceholder = "missing ID"
+
+// MissingIDComputeID returns a ComputeID implementation that always supplies MissingIDPlaceholder.
+func MissingIDComputeID() ComputeID {
+	return func(ctx context.Context, state resource.PropertyMap) (resource.ID, error) {
+		return resource.ID(MissingIDPlaceholder), nil
+	}
+}
+
 // XWithUrn returns a copy of ctx with the resource URN value.
 //
 // XWithUrn is unstable and may be changed or removed in any release.