legacy/helper/schema: ResourceData no longer supports prior state

This package is now here only to provide some support code for older
backends to wrangle their configuration, so there's never any situation
where we have a prior state; that's a managed resource concept.

We'll therefore now panic if something tries to create a ResourceData with
a non-nil state, which means that in all of the other remaining code for
ResourceData we can assume that code that runs when state is not nil is
dead code.

Signed-off-by: Martin Atkins <mart@degeneration.co.uk>

internal/legacy/helper/schema/resource.go

@@ -243,85 +243,6 @@ type StateUpgradeFunc func(rawState map[string]interface{}, meta interface{}) (m
 // See Resource documentation.
 type CustomizeDiffFunc func(*ResourceDiff, interface{}) error
 
-// Apply creates, updates, and/or deletes a resource.
-func (r *Resource) Apply(
-	s *tofu.InstanceState,
-	d *tofu.InstanceDiff,
-	meta interface{}) (*tofu.InstanceState, error) {
-	data, err := schemaMap(r.Schema).Data(s, d)
-	if err != nil {
-		return s, err
-	}
-	if s != nil && data != nil {
-		data.providerMeta = s.ProviderMeta
-	}
-
-	// Instance Diff should have the timeout info, need to copy it over to the
-	// ResourceData meta
-	rt := ResourceTimeout{}
-	if _, ok := d.Meta[TimeoutKey]; ok {
-		if err := rt.DiffDecode(d); err != nil {
-			log.Printf("[ERR] Error decoding ResourceTimeout: %s", err)
-		}
-	} else if s != nil {
-		if _, ok := s.Meta[TimeoutKey]; ok {
-			if err := rt.StateDecode(s); err != nil {
-				log.Printf("[ERR] Error decoding ResourceTimeout: %s", err)
-			}
-		}
-	} else {
-		log.Printf("[DEBUG] No meta timeoutkey found in Apply()")
-	}
-	data.timeouts = &rt
-
-	if s == nil {
-		// The OpenTofu API dictates that this should never happen, but
-		// it doesn't hurt to be safe in this case.
-		s = new(tofu.InstanceState)
-	}
-
-	if d.Destroy || d.RequiresNew() {
-		if s.ID != "" {
-			// Destroy the resource since it is created
-			if err := r.Delete(data, meta); err != nil {
-				return r.recordCurrentSchemaVersion(data.State()), err
-			}
-
-			// Make sure the ID is gone.
-			data.SetId("")
-		}
-
-		// If we're only destroying, and not creating, then return
-		// now since we're done!
-		if !d.RequiresNew() {
-			return nil, nil
-		}
-
-		// Reset the data to be stateless since we just destroyed
-		data, err = schemaMap(r.Schema).Data(nil, d)
-		// data was reset, need to re-apply the parsed timeouts
-		data.timeouts = &rt
-		if err != nil {
-			return nil, err
-		}
-	}
-
-	err = nil
-	if data.Id() == "" {
-		// We're creating, it is a new resource.
-		data.MarkNewResource()
-		err = r.Create(data, meta)
-	} else {
-		if r.Update == nil {
-			return s, fmt.Errorf("doesn't support update")
-		}
-
-		err = r.Update(data, meta)
-	}
-
-	return r.recordCurrentSchemaVersion(data.State()), err
-}
-
 // Diff returns a diff of this resource.
 func (r *Resource) Diff(
 	s *tofu.InstanceState,
@@ -351,34 +272,6 @@ func (r *Resource) Diff(
 	return instanceDiff, err
 }
 
-func (r *Resource) simpleDiff(
-	s *tofu.InstanceState,
-	c *tofu.ResourceConfig,
-	meta interface{}) (*tofu.InstanceDiff, error) {
-
-	instanceDiff, err := schemaMap(r.Schema).Diff(s, c, r.CustomizeDiff, meta, false)
-	if err != nil {
-		return instanceDiff, err
-	}
-
-	if instanceDiff == nil {
-		instanceDiff = tofu.NewInstanceDiff()
-	}
-
-	// Make sure the old value is set in each of the instance diffs.
-	// This was done by the RequiresNew logic in the full legacy Diff.
-	for k, attr := range instanceDiff.Attributes {
-		if attr == nil {
-			continue
-		}
-		if s != nil {
-			attr.Old = s.Attributes[k]
-		}
-	}
-
-	return instanceDiff, nil
-}
-
 // Validate validates the resource configuration against the schema.
 func (r *Resource) Validate(c *tofu.ResourceConfig) ([]string, []error) {
 	warns, errs := schemaMap(r.Schema).Validate(c)
@@ -390,214 +283,6 @@ func (r *Resource) Validate(c *tofu.ResourceConfig) ([]string, []error) {
 	return warns, errs
 }
 
-// ReadDataApply loads the data for a data source, given a diff that
-// describes the configuration arguments and desired computed attributes.
-func (r *Resource) ReadDataApply(
-	d *tofu.InstanceDiff,
-	meta interface{},
-) (*tofu.InstanceState, error) {
-	// Data sources are always built completely from scratch
-	// on each read, so the source state is always nil.
-	data, err := schemaMap(r.Schema).Data(nil, d)
-	if err != nil {
-		return nil, err
-	}
-
-	err = r.Read(data, meta)
-	state := data.State()
-	if state != nil && state.ID == "" {
-		// Data sources can set an ID if they want, but they aren't
-		// required to; we'll provide a placeholder if they don't,
-		// to preserve the invariant that all resources have non-empty
-		// ids.
-		state.ID = "-"
-	}
-
-	return r.recordCurrentSchemaVersion(state), err
-}
-
-// RefreshWithoutUpgrade reads the instance state, but does not call
-// MigrateState or the StateUpgraders, since those are now invoked in a
-// separate API call.
-// RefreshWithoutUpgrade is part of the new plugin shims.
-func (r *Resource) RefreshWithoutUpgrade(
-	s *tofu.InstanceState,
-	meta interface{}) (*tofu.InstanceState, error) {
-	// If the ID is already somehow blank, it doesn't exist
-	if s.ID == "" {
-		return nil, nil
-	}
-
-	rt := ResourceTimeout{}
-	if _, ok := s.Meta[TimeoutKey]; ok {
-		if err := rt.StateDecode(s); err != nil {
-			log.Printf("[ERR] Error decoding ResourceTimeout: %s", err)
-		}
-	}
-
-	if r.Exists != nil {
-		// Make a copy of data so that if it is modified it doesn't
-		// affect our Read later.
-		data, err := schemaMap(r.Schema).Data(s, nil)
-		data.timeouts = &rt
-
-		if err != nil {
-			return s, err
-		}
-
-		if s != nil {
-			data.providerMeta = s.ProviderMeta
-		}
-
-		exists, err := r.Exists(data, meta)
-		if err != nil {
-			return s, err
-		}
-		if !exists {
-			return nil, nil
-		}
-	}
-
-	data, err := schemaMap(r.Schema).Data(s, nil)
-	data.timeouts = &rt
-	if err != nil {
-		return s, err
-	}
-
-	if s != nil {
-		data.providerMeta = s.ProviderMeta
-	}
-
-	err = r.Read(data, meta)
-	state := data.State()
-	if state != nil && state.ID == "" {
-		state = nil
-	}
-
-	return r.recordCurrentSchemaVersion(state), err
-}
-
-// Refresh refreshes the state of the resource.
-func (r *Resource) Refresh(
-	s *tofu.InstanceState,
-	meta interface{}) (*tofu.InstanceState, error) {
-	// If the ID is already somehow blank, it doesn't exist
-	if s.ID == "" {
-		return nil, nil
-	}
-
-	rt := ResourceTimeout{}
-	if _, ok := s.Meta[TimeoutKey]; ok {
-		if err := rt.StateDecode(s); err != nil {
-			log.Printf("[ERR] Error decoding ResourceTimeout: %s", err)
-		}
-	}
-
-	if r.Exists != nil {
-		// Make a copy of data so that if it is modified it doesn't
-		// affect our Read later.
-		data, err := schemaMap(r.Schema).Data(s, nil)
-		data.timeouts = &rt
-
-		if err != nil {
-			return s, err
-		}
-
-		exists, err := r.Exists(data, meta)
-		if err != nil {
-			return s, err
-		}
-		if !exists {
-			return nil, nil
-		}
-	}
-
-	// there may be new StateUpgraders that need to be run
-	s, err := r.upgradeState(s, meta)
-	if err != nil {
-		return s, err
-	}
-
-	data, err := schemaMap(r.Schema).Data(s, nil)
-	data.timeouts = &rt
-	if err != nil {
-		return s, err
-	}
-
-	err = r.Read(data, meta)
-	state := data.State()
-	if state != nil && state.ID == "" {
-		state = nil
-	}
-
-	return r.recordCurrentSchemaVersion(state), err
-}
-
-func (r *Resource) upgradeState(s *tofu.InstanceState, meta interface{}) (*tofu.InstanceState, error) {
-	var err error
-
-	needsMigration, stateSchemaVersion := r.checkSchemaVersion(s)
-	migrate := needsMigration && r.MigrateState != nil
-
-	if migrate {
-		s, err = r.MigrateState(stateSchemaVersion, s, meta)
-		if err != nil {
-			return s, err
-		}
-	}
-
-	if len(r.StateUpgraders) == 0 {
-		return s, nil
-	}
-
-	// If we ran MigrateState, then the stateSchemaVersion value is no longer
-	// correct. We can expect the first upgrade function to be the correct
-	// schema type version.
-	if migrate {
-		stateSchemaVersion = r.StateUpgraders[0].Version
-	}
-
-	schemaType := r.CoreConfigSchema().ImpliedType()
-	// find the expected type to convert the state
-	for _, upgrader := range r.StateUpgraders {
-		if stateSchemaVersion == upgrader.Version {
-			schemaType = upgrader.Type
-		}
-	}
-
-	// StateUpgraders only operate on the new JSON format state, so the state
-	// need to be converted.
-	stateVal, err := StateValueFromInstanceState(s, schemaType)
-	if err != nil {
-		return nil, err
-	}
-
-	jsonState, err := StateValueToJSONMap(stateVal, schemaType)
-	if err != nil {
-		return nil, err
-	}
-
-	for _, upgrader := range r.StateUpgraders {
-		if stateSchemaVersion != upgrader.Version {
-			continue
-		}
-
-		jsonState, err = upgrader.Upgrade(jsonState, meta)
-		if err != nil {
-			return nil, err
-		}
-		stateSchemaVersion++
-	}
-
-	// now we need to re-flatmap the new state
-	stateVal, err = JSONMapToStateValue(jsonState, r.CoreConfigSchema())
-	if err != nil {
-		return nil, err
-	}
-
-	return r.ShimInstanceStateFromValue(stateVal)
-}
-
 // InternalValidate should be called to validate the structure
 // of the resource.
 //
@@ -739,36 +424,6 @@ func isReservedResourceFieldName(name string, s *Schema) bool {
 	return false
 }
 
-// Data returns a ResourceData struct for this Resource. Each return value
-// is a separate copy and can be safely modified differently.
-//
-// The data returned from this function has no actual affect on the Resource
-// itself (including the state given to this function).
-//
-// This function is useful for unit tests and ResourceImporter functions.
-func (r *Resource) Data(s *tofu.InstanceState) *ResourceData {
-	result, err := schemaMap(r.Schema).Data(s, nil)
-	if err != nil {
-		// At the time of writing, this isn't possible (Data never returns
-		// non-nil errors). We panic to find this in the future if we have to.
-		// I don't see a reason for Data to ever return an error.
-		panic(err)
-	}
-
-	// load the Resource timeouts
-	result.timeouts = r.Timeouts
-	if result.timeouts == nil {
-		result.timeouts = &ResourceTimeout{}
-	}
-
-	// Set the schema version to latest by default
-	result.meta = map[string]interface{}{
-		"schema_version": strconv.Itoa(r.SchemaVersion),
-	}
-
-	return result
-}
-
 // TestResourceData Yields a ResourceData filled with this resource's schema for use in unit testing
 //
 // TODO: May be able to be removed with the above ResourceData function.

internal/legacy/helper/schema/resource_data.go

@@ -29,7 +29,7 @@ type ResourceData struct {
 	// Settable (internally)
 	schema       map[string]*Schema
 	config       *tofu.ResourceConfig
-	state        *tofu.InstanceState
+	state        *tofu.InstanceState // legacy field; now always nil
 	diff         *tofu.InstanceDiff
 	meta         map[string]interface{}
 	timeouts     *ResourceTimeout
@@ -426,12 +426,16 @@ func (d *ResourceData) Timeout(key string) time.Duration {
 }
 
 func (d *ResourceData) init() {
-	// Initialize the field that will store our new state
-	var copyState tofu.InstanceState
 	if d.state != nil {
-		copyState = *d.state.DeepCopy()
+		// We no longer support ResourceData with prior state, because we
+		// maintain this package only for use by [Backend] and that never
+		// has prior state.
+		panic("ResourceData with state is no longer allowed")
 	}
-	d.newState = &copyState
+	// We place an uninitialized state here only because some existing code
+	// expects to be able to write into here, even though we don't actually
+	// make any use of the result now that we only care about [Backend].
+	d.newState = &tofu.InstanceState{}
 
 	// Initialize the map for storing set data
 	d.setWriter = &MapFieldWriter{Schema: d.schema}
@@ -439,14 +443,6 @@ func (d *ResourceData) init() {
 	// Initialize the reader for getting data from the
 	// underlying sources (config, diff, etc.)
 	readers := make(map[string]FieldReader)
-	var stateAttributes map[string]string
-	if d.state != nil {
-		stateAttributes = d.state.Attributes
-		readers["state"] = &MapFieldReader{
-			Schema: d.schema,
-			Map:    BasicMapReader(stateAttributes),
-		}
-	}
 	if d.config != nil {
 		readers["config"] = &ConfigFieldReader{
 			Schema: d.schema,
@@ -458,7 +454,10 @@ func (d *ResourceData) init() {
 			Schema: d.schema,
 			Diff:   d.diff,
 			Source: &MultiLevelFieldReader{
-				Levels:  []string{"state", "config"},
+				Levels: []string{
+					"state", // NOTE: this is vestigial; there is no longer ever any state reader in practice
+					"config",
+				},
 				Readers: readers,
 			},
 		}
@@ -469,7 +468,7 @@ func (d *ResourceData) init() {
 	}
 	d.multiReader = &MultiLevelFieldReader{
 		Levels: []string{
-			"state",
+			"state", // NOTE: this is vestigial; there is no longer ever any state reader in practice
 			"config",
 			"diff",
 			"set",

internal/legacy/helper/schema/schema.go

@@ -445,15 +445,22 @@ func (m schemaMap) panicOnError() bool {
 	return false
 }
 
-// Data returns a ResourceData for the given schema, state, and diff.
+// Data returns a ResourceData for the given schema and diff.
 //
-// The diff is optional.
+// The state argument is vestigial; this function will panic if it's set to
+// anything other than nil.
 func (m schemaMap) Data(
 	s *tofu.InstanceState,
 	d *tofu.InstanceDiff) (*ResourceData, error) {
+	if s != nil {
+		// This package is now focused only on supporting the functionality
+		// of the [Backend] type, which never has prior state and therefore
+		// we guard this explicitly here so we can know that any remaining
+		// code that runs only when state != nil is definitely dead code.
+		panic("schemaMap.Data no longer accepts prior state")
+	}
 	return &ResourceData{
 		schema:       m,
-		state:        s,
 		diff:         d,
 		panicOnError: m.panicOnError(),
 	}, nil

internal/legacy/helper/schema/shims_test.go

@@ -8,7 +8,6 @@ package schema
 import (
 	"reflect"
 	"testing"
-	"time"
 
 	"github.com/google/go-cmp/cmp"
 	"github.com/google/go-cmp/cmp/cmpopts"
@@ -118,284 +117,6 @@ func TestShimResourcePlan_destroyCreate(t *testing.T) {
 	testApplyDiff(t, r, state, expected, d)
 }
 
-func TestShimResourceApply_create(t *testing.T) {
-	r := &Resource{
-		SchemaVersion: 2,
-		Schema: map[string]*Schema{
-			"foo": &Schema{
-				Type:     TypeInt,
-				Optional: true,
-			},
-		},
-	}
-
-	called := false
-	r.Create = func(d *ResourceData, m interface{}) error {
-		called = true
-		d.SetId("foo")
-		return nil
-	}
-
-	var s *tofu.InstanceState = nil
-
-	d := &tofu.InstanceDiff{
-		Attributes: map[string]*tofu.ResourceAttrDiff{
-			"foo": &tofu.ResourceAttrDiff{
-				New: "42",
-			},
-		},
-	}
-
-	actual, err := r.Apply(s, d, nil)
-	if err != nil {
-		t.Fatalf("err: %s", err)
-	}
-
-	if !called {
-		t.Fatal("not called")
-	}
-
-	expected := &tofu.InstanceState{
-		ID: "foo",
-		Attributes: map[string]string{
-			"id":  "foo",
-			"foo": "42",
-		},
-		Meta: map[string]interface{}{
-			"schema_version": "2",
-		},
-	}
-
-	if !reflect.DeepEqual(actual, expected) {
-		t.Fatalf("bad: %#v", actual)
-	}
-
-	// Shim
-	// now that we have our diff and desired state, see if we can reproduce
-	// that with the shim
-	// we're not testing Resource.Create, so we need to start with the "created" state
-	createdState := &tofu.InstanceState{
-		ID:         "foo",
-		Attributes: map[string]string{"id": "foo"},
-	}
-
-	testApplyDiff(t, r, createdState, expected, d)
-}
-
-func TestShimResourceApply_Timeout_state(t *testing.T) {
-	r := &Resource{
-		SchemaVersion: 2,
-		Schema: map[string]*Schema{
-			"foo": &Schema{
-				Type:     TypeInt,
-				Optional: true,
-			},
-		},
-		Timeouts: &ResourceTimeout{
-			Create: DefaultTimeout(40 * time.Minute),
-			Update: DefaultTimeout(80 * time.Minute),
-			Delete: DefaultTimeout(40 * time.Minute),
-		},
-	}
-
-	called := false
-	r.Create = func(d *ResourceData, m interface{}) error {
-		called = true
-		d.SetId("foo")
-		return nil
-	}
-
-	var s *tofu.InstanceState = nil
-
-	d := &tofu.InstanceDiff{
-		Attributes: map[string]*tofu.ResourceAttrDiff{
-			"foo": &tofu.ResourceAttrDiff{
-				New: "42",
-			},
-		},
-	}
-
-	diffTimeout := &ResourceTimeout{
-		Create: DefaultTimeout(40 * time.Minute),
-		Update: DefaultTimeout(80 * time.Minute),
-		Delete: DefaultTimeout(40 * time.Minute),
-	}
-
-	if err := diffTimeout.DiffEncode(d); err != nil {
-		t.Fatalf("Error encoding timeout to diff: %s", err)
-	}
-
-	actual, err := r.Apply(s, d, nil)
-	if err != nil {
-		t.Fatalf("err: %s", err)
-	}
-
-	if !called {
-		t.Fatal("not called")
-	}
-
-	expected := &tofu.InstanceState{
-		ID: "foo",
-		Attributes: map[string]string{
-			"id":  "foo",
-			"foo": "42",
-		},
-		Meta: map[string]interface{}{
-			"schema_version": "2",
-			TimeoutKey:       expectedForValues(40, 0, 80, 40, 0),
-		},
-	}
-
-	if !reflect.DeepEqual(actual, expected) {
-		t.Fatalf("Not equal in Timeout State:\n\texpected: %#v\n\tactual: %#v", expected.Meta, actual.Meta)
-	}
-
-	// Shim
-	// we're not testing Resource.Create, so we need to start with the "created" state
-	createdState := &tofu.InstanceState{
-		ID:         "foo",
-		Attributes: map[string]string{"id": "foo"},
-	}
-
-	testApplyDiff(t, r, createdState, expected, d)
-}
-
-func TestShimResourceApply_destroy(t *testing.T) {
-	r := &Resource{
-		Schema: map[string]*Schema{
-			"foo": &Schema{
-				Type:     TypeInt,
-				Optional: true,
-			},
-		},
-	}
-
-	called := false
-	r.Delete = func(d *ResourceData, m interface{}) error {
-		called = true
-		return nil
-	}
-
-	s := &tofu.InstanceState{
-		ID: "bar",
-	}
-
-	d := &tofu.InstanceDiff{
-		Destroy: true,
-	}
-
-	actual, err := r.Apply(s, d, nil)
-	if err != nil {
-		t.Fatalf("err: %s", err)
-	}
-
-	if !called {
-		t.Fatal("delete not called")
-	}
-
-	if actual != nil {
-		t.Fatalf("bad: %#v", actual)
-	}
-
-	// Shim
-	// now that we have our diff and desired state, see if we can reproduce
-	// that with the shim
-	testApplyDiff(t, r, s, actual, d)
-}
-
-func TestShimResourceApply_destroyCreate(t *testing.T) {
-	r := &Resource{
-		Schema: map[string]*Schema{
-			"foo": &Schema{
-				Type:     TypeInt,
-				Optional: true,
-				ForceNew: true,
-			},
-
-			"tags": &Schema{
-				Type:     TypeMap,
-				Optional: true,
-				Computed: true,
-			},
-		},
-	}
-
-	change := false
-	r.Create = func(d *ResourceData, m interface{}) error {
-		change = d.HasChange("tags")
-		d.SetId("foo")
-		return nil
-	}
-	r.Delete = func(d *ResourceData, m interface{}) error {
-		return nil
-	}
-
-	var s *tofu.InstanceState = &tofu.InstanceState{
-		ID: "bar",
-		Attributes: map[string]string{
-			"foo":       "7",
-			"tags.Name": "foo",
-		},
-	}
-
-	d := &tofu.InstanceDiff{
-		Attributes: map[string]*tofu.ResourceAttrDiff{
-			"id": &tofu.ResourceAttrDiff{
-				New: "foo",
-			},
-			"foo": &tofu.ResourceAttrDiff{
-				Old:         "7",
-				New:         "42",
-				RequiresNew: true,
-			},
-			"tags.Name": &tofu.ResourceAttrDiff{
-				Old:         "foo",
-				New:         "foo",
-				RequiresNew: true,
-			},
-		},
-	}
-
-	actual, err := r.Apply(s, d, nil)
-	if err != nil {
-		t.Fatalf("err: %s", err)
-	}
-
-	if !change {
-		t.Fatal("should have change")
-	}
-
-	expected := &tofu.InstanceState{
-		ID: "foo",
-		Attributes: map[string]string{
-			"id":        "foo",
-			"foo":       "42",
-			"tags.%":    "1",
-			"tags.Name": "foo",
-		},
-	}
-
-	if !reflect.DeepEqual(actual, expected) {
-		cmp.Diff(actual, expected)
-	}
-
-	// Shim
-	// now that we have our diff and desired state, see if we can reproduce
-	// that with the shim
-	// we're not testing Resource.Create, so we need to start with the "created" state
-	createdState := &tofu.InstanceState{
-		ID: "foo",
-		Attributes: map[string]string{
-			"id":        "foo",
-			"foo":       "7",
-			"tags.%":    "1",
-			"tags.Name": "foo",
-		},
-	}
-
-	testApplyDiff(t, r, createdState, expected, d)
-}
-
 func resourceSchemaToBlock(s map[string]*Schema) *configschema.Block {
 	return (&Resource{Schema: s}).CoreConfigSchema()
 }