core: NewInstanceInfo should take ResourceInstance, not Resource

On the initial pass here I reached a faulty conclusion about what from
the new world should shim into a NewInstanceInfo, based on a poor read
of existing code.

It actually _should've_ been based on an absolute instance after all,
as evidenced by the expected result of TestContext2Refresh_targetedCount.
Therefore the signature is changed here, and all of the callers (which,
in retrospect, were all holding a full instance address anyway!) are
updated to that new signature.
This commit is contained in:
Martin Atkins 2018-05-25 10:28:23 -07:00
parent 8d062fc577
commit 1ed56f9903
6 changed files with 28 additions and 18 deletions

View File

@ -32,7 +32,7 @@ func (n *EvalApply) Eval(ctx EvalContext) (interface{}, error) {
state := *n.State state := *n.State
// The provider API still expects our legacy InstanceInfo type, so we must shim it. // The provider API still expects our legacy InstanceInfo type, so we must shim it.
legacyInfo := NewInstanceInfo(n.Addr.Absolute(ctx.Path()).ContainingResource()) legacyInfo := NewInstanceInfo(n.Addr.Absolute(ctx.Path()))
if diff.Empty() { if diff.Empty() {
log.Printf("[DEBUG] apply %s: diff is empty, so skipping.", n.Addr) log.Printf("[DEBUG] apply %s: diff is empty, so skipping.", n.Addr)
@ -113,7 +113,7 @@ func (n *EvalApplyPre) Eval(ctx EvalContext) (interface{}, error) {
// The hook API still uses our legacy InstanceInfo type, so we must // The hook API still uses our legacy InstanceInfo type, so we must
// shim it. // shim it.
legacyInfo := NewInstanceInfo(n.Addr.ContainingResource().Absolute(ctx.Path())) legacyInfo := NewInstanceInfo(n.Addr.Absolute(ctx.Path()))
// If the state is nil, make it non-nil // If the state is nil, make it non-nil
if state == nil { if state == nil {
@ -147,7 +147,7 @@ func (n *EvalApplyPost) Eval(ctx EvalContext) (interface{}, error) {
// The hook API still uses our legacy InstanceInfo type, so we must // The hook API still uses our legacy InstanceInfo type, so we must
// shim it. // shim it.
legacyInfo := NewInstanceInfo(n.Addr.ContainingResource().Absolute(ctx.Path())) legacyInfo := NewInstanceInfo(n.Addr.Absolute(ctx.Path()))
if resourceHasUserVisibleApply(legacyInfo) { if resourceHasUserVisibleApply(legacyInfo) {
// Call post-apply hook // Call post-apply hook
@ -199,7 +199,7 @@ func (n *EvalApplyProvisioners) Eval(ctx EvalContext) (interface{}, error) {
state := *n.State state := *n.State
// The hook API still uses the legacy InstanceInfo type, so we need to shim it. // The hook API still uses the legacy InstanceInfo type, so we need to shim it.
legacyInfo := NewInstanceInfo(n.Addr.Resource.Absolute(ctx.Path())) legacyInfo := NewInstanceInfo(n.Addr.Absolute(ctx.Path()))
if n.CreateNew != nil && !*n.CreateNew { if n.CreateNew != nil && !*n.CreateNew {
// If we're not creating a new resource, then don't run provisioners // If we're not creating a new resource, then don't run provisioners
@ -286,7 +286,7 @@ func (n *EvalApplyProvisioners) apply(ctx EvalContext, provs []*configs.Provisio
state := *n.State state := *n.State
// The hook API still uses the legacy InstanceInfo type, so we need to shim it. // The hook API still uses the legacy InstanceInfo type, so we need to shim it.
legacyInfo := NewInstanceInfo(n.Addr.Resource.Absolute(ctx.Path())) legacyInfo := NewInstanceInfo(n.Addr.Absolute(ctx.Path()))
// Store the original connection info, restore later // Store the original connection info, restore later
origConnInfo := state.Ephemeral.ConnInfo origConnInfo := state.Ephemeral.ConnInfo

View File

@ -104,7 +104,7 @@ func (n *EvalDiff) Eval(ctx EvalContext) (interface{}, error) {
var diags tfdiags.Diagnostics var diags tfdiags.Diagnostics
// The provider and hook APIs still expect our legacy InstanceInfo type. // The provider and hook APIs still expect our legacy InstanceInfo type.
legacyInfo := NewInstanceInfo(n.Addr.Absolute(ctx.Path()).ContainingResource()) legacyInfo := NewInstanceInfo(n.Addr.Absolute(ctx.Path()))
// State still uses legacy-style internal ids, so we need to shim to get // State still uses legacy-style internal ids, so we need to shim to get
// a suitable key to use. // a suitable key to use.
@ -439,7 +439,7 @@ func (n *EvalDiffDestroy) Eval(ctx EvalContext) (interface{}, error) {
} }
// The provider and hook APIs still expect our legacy InstanceInfo type. // The provider and hook APIs still expect our legacy InstanceInfo type.
legacyInfo := NewInstanceInfo(n.Addr.Absolute(ctx.Path()).ContainingResource()) legacyInfo := NewInstanceInfo(n.Addr.Absolute(ctx.Path()))
// Call pre-diff hook // Call pre-diff hook
err := ctx.Hook(func(h Hook) (HookAction, error) { err := ctx.Hook(func(h Hook) (HookAction, error) {

View File

@ -37,7 +37,7 @@ func (n *EvalReadDataDiff) Eval(ctx EvalContext) (interface{}, error) {
var diags tfdiags.Diagnostics var diags tfdiags.Diagnostics
// The provider and hook APIs still expect our legacy InstanceInfo type. // The provider and hook APIs still expect our legacy InstanceInfo type.
legacyInfo := NewInstanceInfo(n.Addr.Absolute(ctx.Path()).ContainingResource()) legacyInfo := NewInstanceInfo(n.Addr.Absolute(ctx.Path()))
err := ctx.Hook(func(h Hook) (HookAction, error) { err := ctx.Hook(func(h Hook) (HookAction, error) {
return h.PreDiff(legacyInfo, nil) return h.PreDiff(legacyInfo, nil)
@ -138,7 +138,7 @@ func (n *EvalReadDataApply) Eval(ctx EvalContext) (interface{}, error) {
diff := *n.Diff diff := *n.Diff
// The provider and hook APIs still expect our legacy InstanceInfo type. // The provider and hook APIs still expect our legacy InstanceInfo type.
legacyInfo := NewInstanceInfo(n.Addr.Absolute(ctx.Path()).ContainingResource()) legacyInfo := NewInstanceInfo(n.Addr.Absolute(ctx.Path()))
// If the diff is for *destroying* this resource then we'll // If the diff is for *destroying* this resource then we'll
// just drop its state and move on, since data resources don't // just drop its state and move on, since data resources don't

View File

@ -22,7 +22,7 @@ func (n *EvalRefresh) Eval(ctx EvalContext) (interface{}, error) {
state := *n.State state := *n.State
// The provider and hook APIs still expect our legacy InstanceInfo type. // The provider and hook APIs still expect our legacy InstanceInfo type.
legacyInfo := NewInstanceInfo(n.Addr.Absolute(ctx.Path()).ContainingResource()) legacyInfo := NewInstanceInfo(n.Addr.Absolute(ctx.Path()))
// If we have no state, we don't do any refreshing // If we have no state, we don't do any refreshing
if state == nil { if state == nil {

View File

@ -105,10 +105,8 @@ type InstanceInfo struct {
uniqueExtra string uniqueExtra string
} }
// NewInstanceInfo constructs an InstanceInfo from an addrs.AbsResource. // NewInstanceInfo constructs an InstanceInfo from an addrs.AbsResourceInstance.
// //
// In spite of the confusing name, an InstanceInfo actually identifies a
// particular resource rather than a particular resource instance.
// InstanceInfo is a legacy type, and uses of it should be gradually replaced // InstanceInfo is a legacy type, and uses of it should be gradually replaced
// by direct use of addrs.AbsResource or addrs.AbsResourceInstance as // by direct use of addrs.AbsResource or addrs.AbsResourceInstance as
// appropriate. // appropriate.
@ -117,7 +115,11 @@ type InstanceInfo struct {
// keys, so this function will panic if given such a path. Uses of this type // keys, so this function will panic if given such a path. Uses of this type
// should all be removed or replaced before implementing "count" and "for_each" // should all be removed or replaced before implementing "count" and "for_each"
// arguments on modules in order to avoid such panics. // arguments on modules in order to avoid such panics.
func NewInstanceInfo(addr addrs.AbsResource) *InstanceInfo { //
// This legacy type also cannot represent resource instances with string
// instance keys. It will panic if the given key is not either NoKey or an
// IntKey.
func NewInstanceInfo(addr addrs.AbsResourceInstance) *InstanceInfo {
// We need an old-style []string module path for InstanceInfo. // We need an old-style []string module path for InstanceInfo.
path := make([]string, len(addr.Module)) path := make([]string, len(addr.Module))
for i, step := range addr.Module { for i, step := range addr.Module {
@ -132,15 +134,23 @@ func NewInstanceInfo(addr addrs.AbsResource) *InstanceInfo {
// a representation of the resource mode, and so it's impossible to // a representation of the resource mode, and so it's impossible to
// determine from an InstanceInfo alone whether it is a managed or data // determine from an InstanceInfo alone whether it is a managed or data
// resource that is being referred to. // resource that is being referred to.
id := fmt.Sprintf("%s.%s", addr.Resource.Type, addr.Resource.Name) id := fmt.Sprintf("%s.%s", addr.Resource.Resource.Type, addr.Resource.Resource.Name)
if addr.Resource.Mode == addrs.DataResourceMode { if addr.Resource.Resource.Mode == addrs.DataResourceMode {
id = "data." + id id = "data." + id
} }
if addr.Resource.Key != addrs.NoKey {
switch k := addr.Resource.Key.(type) {
case addrs.IntKey:
id = id + fmt.Sprintf(".%d", int(k))
default:
panic(fmt.Sprintf("NewInstanceInfo cannot convert resource instance with %T instance key", addr.Resource.Key))
}
}
return &InstanceInfo{ return &InstanceInfo{
Id: id, Id: id,
ModulePath: path, ModulePath: path,
Type: addr.Resource.Type, Type: addr.Resource.Resource.Type,
} }
} }

View File

@ -67,7 +67,7 @@ func (n *graphNodeImportState) Path() addrs.ModuleInstance {
// GraphNodeEvalable impl. // GraphNodeEvalable impl.
func (n *graphNodeImportState) EvalTree() EvalNode { func (n *graphNodeImportState) EvalTree() EvalNode {
var provider ResourceProvider var provider ResourceProvider
info := NewInstanceInfo(n.Addr.ContainingResource()) info := NewInstanceInfo(n.Addr)
// Reset our states // Reset our states
n.states = nil n.states = nil