helper/schema: Implementation of the AsSingle mechanism

The previous commit added a new flag to schema.Schema which is documented
to make a list with MaxItems: 1 be presented to Terraform Core as a single
value instead, giving a way to switch to non-list nested resources without
it being a breaking change for Terraform v0.11 users as long as it's done
prior to a provider's first v0.12-compatible release.

This is the implementation of that mechanism. It's intentionally
implemented as a suite of extra fixups rather than direct modifications to
existing shim code because we want to ensure that this has no effect
whatsoever on the result of a resource type that _isn't_ using AsSingle.

Although there is some small unit test coverage of the fixup steps here,
the primary testing for this is in the test provider since the integration
of all of these fixup steps in the correct order is the more important
result than any of the intermediate fixup steps.
This commit is contained in:
Martin Atkins 2019-03-14 14:39:38 -07:00
parent 1c8150428f
commit 1987a92386
10 changed files with 916 additions and 25 deletions

View File

@ -18,6 +18,7 @@ func Provider() terraform.ResourceProvider {
},
ResourcesMap: map[string]*schema.Resource{
"test_resource": testResource(),
"test_resource_as_single": testResourceAsSingle(),
"test_resource_gh12183": testResourceGH12183(),
"test_resource_import_other": testResourceImportOther(),
"test_resource_with_custom_diff": testResourceCustomDiff(),

View File

@ -0,0 +1,158 @@
package test
import (
"fmt"
"github.com/hashicorp/terraform/helper/schema"
)
func testResourceAsSingle() *schema.Resource {
return &schema.Resource{
Create: testResourceAsSingleCreate,
Read: testResourceAsSingleRead,
Delete: testResourceAsSingleDelete,
Update: testResourceAsSingleUpdate,
Schema: map[string]*schema.Schema{
"list_resource_as_block": {
Type: schema.TypeList,
Optional: true,
MaxItems: 1,
AsSingle: true,
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
"foo": {
Type: schema.TypeString,
Optional: true,
},
},
},
},
"list_resource_as_attr": {
Type: schema.TypeList,
ConfigMode: schema.SchemaConfigModeAttr,
Optional: true,
MaxItems: 1,
AsSingle: true,
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
"foo": {
Type: schema.TypeString,
Optional: true,
},
},
},
},
"list_primitive": {
Type: schema.TypeList,
Optional: true,
MaxItems: 1,
AsSingle: true,
Elem: &schema.Schema{
Type: schema.TypeString,
},
},
"set_resource_as_block": {
Type: schema.TypeSet,
Optional: true,
MaxItems: 1,
AsSingle: true,
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
"foo": {
Type: schema.TypeString,
Optional: true,
},
},
},
},
"set_resource_as_attr": {
Type: schema.TypeSet,
ConfigMode: schema.SchemaConfigModeAttr,
Optional: true,
MaxItems: 1,
AsSingle: true,
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
"foo": {
Type: schema.TypeString,
Optional: true,
},
},
},
},
"set_primitive": {
Type: schema.TypeSet,
Optional: true,
MaxItems: 1,
AsSingle: true,
Elem: &schema.Schema{
Type: schema.TypeString,
},
},
},
}
}
func testResourceAsSingleCreate(d *schema.ResourceData, meta interface{}) error {
d.SetId("placeholder")
return testResourceAsSingleRead(d, meta)
}
func testResourceAsSingleRead(d *schema.ResourceData, meta interface{}) error {
for _, k := range []string{"list_resource_as_block", "list_resource_as_attr", "list_primitive"} {
v := d.Get(k)
if v == nil {
continue
}
if l, ok := v.([]interface{}); !ok {
return fmt.Errorf("%s should appear as []interface {}, not %T", k, v)
} else {
for i, item := range l {
switch k {
case "list_primitive":
if _, ok := item.(string); item != nil && !ok {
return fmt.Errorf("%s[%d] should appear as string, not %T", k, i, item)
}
default:
if _, ok := item.(map[string]interface{}); item != nil && !ok {
return fmt.Errorf("%s[%d] should appear as map[string]interface {}, not %T", k, i, item)
}
}
}
}
}
for _, k := range []string{"set_resource_as_block", "set_resource_as_attr", "set_primitive"} {
v := d.Get(k)
if v == nil {
continue
}
if s, ok := v.(*schema.Set); !ok {
return fmt.Errorf("%s should appear as *schema.Set, not %T", k, v)
} else {
for i, item := range s.List() {
switch k {
case "set_primitive":
if _, ok := item.(string); item != nil && !ok {
return fmt.Errorf("%s[%d] should appear as string, not %T", k, i, item)
}
default:
if _, ok := item.(map[string]interface{}); item != nil && !ok {
return fmt.Errorf("%s[%d] should appear as map[string]interface {}, not %T", k, i, item)
}
}
}
}
}
return nil
}
func testResourceAsSingleUpdate(d *schema.ResourceData, meta interface{}) error {
return testResourceAsSingleRead(d, meta)
}
func testResourceAsSingleDelete(d *schema.ResourceData, meta interface{}) error {
d.SetId("")
return nil
}

View File

@ -0,0 +1,114 @@
package test
import (
"strings"
"testing"
"github.com/hashicorp/terraform/helper/resource"
"github.com/hashicorp/terraform/terraform"
)
func TestResourceAsSingle(t *testing.T) {
resource.UnitTest(t, resource.TestCase{
Providers: testAccProviders,
CheckDestroy: testAccCheckResourceDestroy,
Steps: []resource.TestStep{
resource.TestStep{
Config: strings.TrimSpace(`
resource "test_resource_as_single" "foo" {
list_resource_as_block {
foo = "as block a"
}
list_resource_as_attr = {
foo = "as attr a"
}
list_primitive = "primitive a"
set_resource_as_block {
foo = "as block a"
}
set_resource_as_attr = {
foo = "as attr a"
}
set_primitive = "primitive a"
}
`),
Check: resource.ComposeTestCheckFunc(
func(s *terraform.State) error {
t.Log("state after initial create:\n", s.String())
return nil
},
resource.TestCheckResourceAttr("test_resource_as_single.foo", "list_resource_as_block.#", "1"),
resource.TestCheckResourceAttr("test_resource_as_single.foo", "list_resource_as_block.0.foo", "as block a"),
resource.TestCheckResourceAttr("test_resource_as_single.foo", "list_resource_as_attr.#", "1"),
resource.TestCheckResourceAttr("test_resource_as_single.foo", "list_resource_as_attr.0.foo", "as attr a"),
resource.TestCheckResourceAttr("test_resource_as_single.foo", "list_primitive.#", "1"),
resource.TestCheckResourceAttr("test_resource_as_single.foo", "list_primitive.0", "primitive a"),
resource.TestCheckResourceAttr("test_resource_as_single.foo", "set_resource_as_block.#", "1"),
resource.TestCheckResourceAttr("test_resource_as_single.foo", "set_resource_as_block.1417230722.foo", "as block a"),
resource.TestCheckResourceAttr("test_resource_as_single.foo", "set_resource_as_attr.#", "1"),
resource.TestCheckResourceAttr("test_resource_as_single.foo", "set_resource_as_attr.2549052262.foo", "as attr a"),
resource.TestCheckResourceAttr("test_resource_as_single.foo", "set_primitive.#", "1"),
resource.TestCheckResourceAttr("test_resource_as_single.foo", "set_primitive.247272358", "primitive a"),
),
},
resource.TestStep{
Config: strings.TrimSpace(`
resource "test_resource_as_single" "foo" {
list_resource_as_block {
foo = "as block b"
}
list_resource_as_attr = {
foo = "as attr b"
}
list_primitive = "primitive b"
set_resource_as_block {
foo = "as block b"
}
set_resource_as_attr = {
foo = "as attr b"
}
set_primitive = "primitive b"
}
`),
Check: resource.ComposeTestCheckFunc(
func(s *terraform.State) error {
t.Log("state after update:\n", s.String())
return nil
},
resource.TestCheckResourceAttr("test_resource_as_single.foo", "list_resource_as_block.#", "1"),
resource.TestCheckResourceAttr("test_resource_as_single.foo", "list_resource_as_block.0.foo", "as block b"),
resource.TestCheckResourceAttr("test_resource_as_single.foo", "list_resource_as_attr.#", "1"),
resource.TestCheckResourceAttr("test_resource_as_single.foo", "list_resource_as_attr.0.foo", "as attr b"),
resource.TestCheckResourceAttr("test_resource_as_single.foo", "list_primitive.#", "1"),
resource.TestCheckResourceAttr("test_resource_as_single.foo", "list_primitive.0", "primitive b"),
resource.TestCheckResourceAttr("test_resource_as_single.foo", "set_resource_as_block.#", "1"),
resource.TestCheckResourceAttr("test_resource_as_single.foo", "set_resource_as_block.2136238657.foo", "as block b"),
resource.TestCheckResourceAttr("test_resource_as_single.foo", "set_resource_as_attr.#", "1"),
resource.TestCheckResourceAttr("test_resource_as_single.foo", "set_resource_as_attr.3166838949.foo", "as attr b"),
resource.TestCheckResourceAttr("test_resource_as_single.foo", "set_primitive.#", "1"),
resource.TestCheckResourceAttr("test_resource_as_single.foo", "set_primitive.630210661", "primitive b"),
),
},
resource.TestStep{
Config: strings.TrimSpace(`
resource "test_resource_as_single" "foo" {
}
`),
Check: resource.ComposeTestCheckFunc(
func(s *terraform.State) error {
t.Log("state after everything unset:\n", s.String())
return nil
},
resource.TestCheckResourceAttr("test_resource_as_single.foo", "list_resource_as_block.#", "0"),
resource.TestCheckResourceAttr("test_resource_as_single.foo", "list_resource_as_attr.#", "0"),
resource.TestCheckResourceAttr("test_resource_as_single.foo", "list_primitive.#", "0"),
resource.TestCheckResourceAttr("test_resource_as_single.foo", "set_resource_as_block.#", "0"),
resource.TestCheckResourceAttr("test_resource_as_single.foo", "set_resource_as_attr.#", "0"),
resource.TestCheckResourceAttr("test_resource_as_single.foo", "set_primitive.#", "0"),
),
},
},
})
}

View File

@ -169,6 +169,7 @@ func (s *GRPCProviderServer) PrepareProviderConfig(_ context.Context, req *proto
}
config := terraform.NewResourceConfigShimmed(configVal, block)
schema.FixupAsSingleResourceConfigIn(config, s.provider.Schema)
warns, errs := s.provider.Validate(config)
resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, convert.WarnsAndErrsToProto(warns, errs))
@ -196,6 +197,7 @@ func (s *GRPCProviderServer) ValidateResourceTypeConfig(_ context.Context, req *
}
config := terraform.NewResourceConfigShimmed(configVal, block)
schema.FixupAsSingleResourceConfigIn(config, s.provider.ResourcesMap[req.TypeName].Schema)
warns, errs := s.provider.ValidateResource(req.TypeName, config)
resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, convert.WarnsAndErrsToProto(warns, errs))
@ -215,6 +217,7 @@ func (s *GRPCProviderServer) ValidateDataSourceConfig(_ context.Context, req *pr
}
config := terraform.NewResourceConfigShimmed(configVal, block)
schema.FixupAsSingleResourceConfigIn(config, s.provider.DataSourcesMap[req.TypeName].Schema)
warns, errs := s.provider.ValidateDataSource(req.TypeName, config)
resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, convert.WarnsAndErrsToProto(warns, errs))
@ -242,6 +245,12 @@ func (s *GRPCProviderServer) UpgradeResourceState(_ context.Context, req *proto.
}
}
// NOTE WELL: The AsSingle mechanism cannot be automatically normalized here,
// so providers that use it must be ready to handle both normalized and
// unnormalized input in their upgrade codepaths. The _result_ of an upgrade
// should set a single-element list/set for any AsSingle element so that it
// can be normalized to a single value automatically on return.
// We first need to upgrade a flatmap state if it exists.
// There should never be both a JSON and Flatmap state in the request.
if req.RawState.Flatmap != nil {
@ -259,6 +268,8 @@ func (s *GRPCProviderServer) UpgradeResourceState(_ context.Context, req *proto.
return resp, nil
}
schema.FixupAsSingleConfigValueOut(jsonMap, s.provider.ResourcesMap[req.TypeName].Schema)
// now we need to turn the state into the default json representation, so
// that it can be re-decoded using the actual schema.
val, err := schema.JSONMapToStateValue(jsonMap, block)
@ -395,6 +406,7 @@ func (s *GRPCProviderServer) Configure(_ context.Context, req *proto.Configure_R
s.provider.TerraformVersion = req.TerraformVersion
config := terraform.NewResourceConfigShimmed(configVal, block)
schema.FixupAsSingleResourceConfigIn(config, s.provider.Schema)
err = s.provider.Configure(config)
resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, err)
@ -418,6 +430,7 @@ func (s *GRPCProviderServer) ReadResource(_ context.Context, req *proto.ReadReso
resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, err)
return resp, nil
}
// res.ShimInstanceStateFromValue result has already had FixupAsSingleInstanceStateIn applied
newInstanceState, err := res.RefreshWithoutUpgrade(instanceState, s.provider.Meta())
if err != nil {
@ -442,6 +455,7 @@ func (s *GRPCProviderServer) ReadResource(_ context.Context, req *proto.ReadReso
// helper/schema should always copy the ID over, but do it again just to be safe
newInstanceState.Attributes["id"] = newInstanceState.ID
schema.FixupAsSingleInstanceStateOut(newInstanceState, s.provider.ResourcesMap[req.TypeName])
newStateVal, err := hcl2shim.HCL2ValueFromFlatmap(newInstanceState.Attributes, block.ImpliedType())
if err != nil {
resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, err)
@ -507,6 +521,7 @@ func (s *GRPCProviderServer) PlanResourceChange(_ context.Context, req *proto.Pl
resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, err)
return resp, nil
}
// res.ShimInstanceStateFromValue result has already had FixupAsSingleInstanceStateIn applied
priorPrivate := make(map[string]interface{})
if len(req.PriorPrivate) > 0 {
if err := json.Unmarshal(req.PriorPrivate, &priorPrivate); err != nil {
@ -519,6 +534,7 @@ func (s *GRPCProviderServer) PlanResourceChange(_ context.Context, req *proto.Pl
// turn the proposed state into a legacy configuration
cfg := terraform.NewResourceConfigShimmed(proposedNewStateVal, block)
schema.FixupAsSingleResourceConfigIn(cfg, s.provider.ResourcesMap[req.TypeName].Schema)
diff, err := s.provider.SimpleDiff(info, priorState, cfg)
if err != nil {
@ -562,7 +578,17 @@ func (s *GRPCProviderServer) PlanResourceChange(_ context.Context, req *proto.Pl
}
// now we need to apply the diff to the prior state, so get the planned state
plannedAttrs, err := diff.Apply(priorState.Attributes, block)
plannedAttrs, err := diff.Apply(priorState.Attributes, res.CoreConfigSchemaWhenShimmed())
schema.FixupAsSingleInstanceStateOut(
&terraform.InstanceState{Attributes: plannedAttrs},
s.provider.ResourcesMap[req.TypeName],
)
// We also fix up the diff for AsSingle here, but note that we intentionally
// do it _after_ diff.Apply (so that the state can have its own fixup applied)
// but before we deal with requiresNew below so that fixing up the diff
// also fixes up the requiresNew keys to match.
schema.FixupAsSingleInstanceDiffOut(diff, s.provider.ResourcesMap[req.TypeName])
plannedStateVal, err := hcl2shim.HCL2ValueFromFlatmap(plannedAttrs, block.ImpliedType())
if err != nil {
@ -702,6 +728,7 @@ func (s *GRPCProviderServer) ApplyResourceChange(_ context.Context, req *proto.A
resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, err)
return resp, nil
}
// res.ShimInstanceStateFromValue result has already had FixupAsSingleInstanceStateIn applied
private := make(map[string]interface{})
if len(req.PlannedPrivate) > 0 {
@ -723,7 +750,10 @@ func (s *GRPCProviderServer) ApplyResourceChange(_ context.Context, req *proto.A
Destroy: true,
}
} else {
diff, err = schema.DiffFromValues(priorStateVal, plannedStateVal, stripResourceModifiers(res))
diff, err = schema.DiffFromValues(
priorStateVal, plannedStateVal,
stripResourceModifiers(res),
)
if err != nil {
resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, err)
return resp, nil
@ -737,6 +767,10 @@ func (s *GRPCProviderServer) ApplyResourceChange(_ context.Context, req *proto.A
}
}
// NOTE WELL: schema.DiffFromValues has already effectively applied
// schema.FixupAsSingleInstanceDiffIn to the diff, so we need not (and must not)
// repeat that here.
// We need to fix any sets that may be using the "~" index prefix to
// indicate partially computed. The special sigil isn't really used except
// as a clue to visually indicate that the set isn't wholly known.
@ -788,6 +822,7 @@ func (s *GRPCProviderServer) ApplyResourceChange(_ context.Context, req *proto.A
if err != nil {
resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, err)
}
schema.FixupAsSingleInstanceStateOut(newInstanceState, s.provider.ResourcesMap[req.TypeName])
newStateVal := cty.NullVal(block.ImpliedType())
// Always return a null value for destroy.
@ -864,6 +899,8 @@ func (s *GRPCProviderServer) ImportResourceState(_ context.Context, req *proto.I
// copy the ID again just to be sure it wasn't missed
is.Attributes["id"] = is.ID
schema.FixupAsSingleInstanceStateOut(is, s.provider.ResourcesMap[req.TypeName])
resourceType := is.Ephemeral.Type
if resourceType == "" {
resourceType = req.TypeName
@ -919,6 +956,7 @@ func (s *GRPCProviderServer) ReadDataSource(_ context.Context, req *proto.ReadDa
}
config := terraform.NewResourceConfigShimmed(configVal, block)
schema.FixupAsSingleResourceConfigIn(config, s.provider.DataSourcesMap[req.TypeName].Schema)
// we need to still build the diff separately with the Read method to match
// the old behavior
@ -934,6 +972,7 @@ func (s *GRPCProviderServer) ReadDataSource(_ context.Context, req *proto.ReadDa
resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, err)
return resp, nil
}
schema.FixupAsSingleInstanceStateOut(newInstanceState, s.provider.DataSourcesMap[req.TypeName])
newStateVal, err := schema.StateValueFromInstanceState(newInstanceState, block.ImpliedType())
if err != nil {

View File

@ -4,13 +4,11 @@ import (
"fmt"
"github.com/hashicorp/terraform/addrs"
"github.com/zclconf/go-cty/cty"
"github.com/hashicorp/terraform/config/hcl2shim"
"github.com/hashicorp/terraform/helper/schema"
"github.com/hashicorp/terraform/states"
"github.com/hashicorp/terraform/terraform"
"github.com/zclconf/go-cty/cty"
)
// shimState takes a new *states.State and reverts it to a legacy state for the provider ACC tests
@ -154,6 +152,7 @@ func shimmedAttributes(instance *states.ResourceInstanceObjectSrc, res *schema.R
}
instanceState, err := res.ShimInstanceStateFromValue(rio.Value)
// instanceState has already had the AsSingle fixup applied because ShimInstanceStateFromValue calls FixupAsSingleInstanceStateIn.
if err != nil {
return nil, err
}

View File

@ -0,0 +1,286 @@
package schema
import (
"sort"
"strings"
"github.com/hashicorp/terraform/terraform"
)
// FixupAsSingleResourceConfigIn modifies the given ResourceConfig in-place if
// any attributes in the schema have the AsSingle flag set, wrapping the given
// values for these in an extra level of slice so that they can be understood
// by legacy SDK code that'll be expecting to decode into a list/set.
func FixupAsSingleResourceConfigIn(rc *terraform.ResourceConfig, s map[string]*Schema) {
if rc == nil {
return
}
FixupAsSingleConfigValueIn(rc.Config, s)
}
// FixupAsSingleInstanceStateIn modifies the given InstanceState in-place if
// any attributes in the schema have the AsSingle flag set, adding additional
// index steps to the flatmap keys for these so that they can be understood
// by legacy SDK code that'll be expecting to decode into a list/set.
func FixupAsSingleInstanceStateIn(is *terraform.InstanceState, r *Resource) {
fixupAsSingleInstanceState(is, r.Schema, "", fixupAsSingleFlatmapKeysIn)
}
// FixupAsSingleInstanceStateOut modifies the given InstanceState in-place if
// any attributes in the schema have the AsSingle flag set, removing unneeded
// index steps from the flatmap keys for these so that they can be understood
// by the shim back to Terraform Core as a single nested value.
func FixupAsSingleInstanceStateOut(is *terraform.InstanceState, r *Resource) {
fixupAsSingleInstanceState(is, r.Schema, "", fixupAsSingleFlatmapKeysOut)
}
// FixupAsSingleInstanceDiffIn modifies the given InstanceDiff in-place if any
// attributes in the schema have the AsSingle flag set, adding additional index
// steps to the flatmap keys for these so that they can be understood by legacy
// SDK code that'll be expecting to decode into a list/set.
func FixupAsSingleInstanceDiffIn(id *terraform.InstanceDiff, r *Resource) {
fixupAsSingleInstanceDiff(id, r.Schema, "", fixupAsSingleAttrsMapKeysIn)
}
// FixupAsSingleInstanceDiffOut modifies the given InstanceDiff in-place if any
// attributes in the schema have the AsSingle flag set, removing unneeded index
// steps from the flatmap keys for these so that they can be understood by the
// shim back to Terraform Core as a single nested value.
func FixupAsSingleInstanceDiffOut(id *terraform.InstanceDiff, r *Resource) {
fixupAsSingleInstanceDiff(id, r.Schema, "", fixupAsSingleAttrsMapKeysOut)
}
// FixupAsSingleConfigValueIn modifies the given "config value" in-place if
// any attributes in the schema have the AsSingle flag set, wrapping the given
// values for these in an extra level of slice so that they can be understood
// by legacy SDK code that'll be expecting to decode into a list/set.
//
// "Config value" for the purpose of this function has the same meaning as for
// the hcl2shims: a map[string]interface{} using the same subset of Go value
// types that would be generated by HCL/HIL when decoding a configuration in
// Terraform v0.11.
func FixupAsSingleConfigValueIn(c map[string]interface{}, s map[string]*Schema) {
for k, as := range s {
if !as.AsSingle {
continue // Don't touch non-AsSingle values at all. This is explicitly opt-in.
}
v, ok := c[k]
if ok {
c[k] = []interface{}{v}
}
if nr, ok := as.Elem.(*Resource); ok {
// Recursively fixup nested attributes too
nm, ok := v.(map[string]interface{})
if !ok {
// Weird for a nested resource to not be a map, but we'll tolerate it rather than crashing
continue
}
FixupAsSingleConfigValueIn(nm, nr.Schema)
}
}
}
// FixupAsSingleConfigValueOut modifies the given "config value" in-place if
// any attributes in the schema have the AsSingle flag set, unwrapping the
// given values from their single-element slices so that they can be understood
// as a single object value by Terraform Core.
//
// This is the opposite of fixupAsSingleConfigValueIn.
func FixupAsSingleConfigValueOut(c map[string]interface{}, s map[string]*Schema) {
for k, as := range s {
if !as.AsSingle {
continue // Don't touch non-AsSingle values at all. This is explicitly opt-in.
}
sv, ok := c[k].([]interface{})
if ok && len(sv) != 0 { // Should always be a single-element slice, but if not we'll just leave it alone rather than crashing
c[k] = sv[0]
if nr, ok := as.Elem.(*Resource); ok {
// Recursively fixup nested attributes too
nm, ok := sv[0].(map[string]interface{})
if ok {
FixupAsSingleConfigValueOut(nm, nr.Schema)
}
}
}
}
}
func fixupAsSingleInstanceState(is *terraform.InstanceState, s map[string]*Schema, prefix string, fn func(map[string]string, string) string) {
if is == nil {
return
}
for k, as := range s {
if !as.AsSingle {
continue // Don't touch non-AsSingle values at all. This is explicitly opt-in.
}
nextPrefix := fn(is.Attributes, prefix+k+".")
if nr, ok := as.Elem.(*Resource); ok {
// Recursively fixup nested attributes too
fixupAsSingleInstanceState(is, nr.Schema, nextPrefix, fn)
}
}
}
func fixupAsSingleInstanceDiff(id *terraform.InstanceDiff, s map[string]*Schema, prefix string, fn func(map[string]*terraform.ResourceAttrDiff, string) string) {
if id == nil {
return
}
for k, as := range s {
if !as.AsSingle {
continue // Don't touch non-AsSingle values at all. This is explicitly opt-in.
}
nextPrefix := fn(id.Attributes, prefix+k+".")
if nr, ok := as.Elem.(*Resource); ok {
// Recursively fixup nested attributes too
fixupAsSingleInstanceDiff(id, nr.Schema, nextPrefix, fn)
}
}
}
// fixupAsSingleFlatmapKeysIn searches the given flatmap for all keys with
// the given prefix (which must end with a dot) and replaces them with keys
// where that prefix is followed by the dummy index "0." and, if any such
// keys are found, a ".#"-suffixed key is also added whose value is "1".
//
// This function will also replace an exact match of the given prefix with
// the trailing dot removed, to recognize values of primitive-typed attributes.
func fixupAsSingleFlatmapKeysIn(attrs map[string]string, prefix string) string {
ks := make([]string, 0, len(attrs))
for k := range attrs {
ks = append(ks, k)
}
sort.Strings(ks) // Makes no difference for valid input, but will ensure we handle invalid input deterministically
for _, k := range ks {
newK, countK := fixupAsSingleFlatmapKeyIn(k, prefix)
if _, exists := attrs[newK]; k != newK && !exists {
attrs[newK] = attrs[k]
delete(attrs, k)
}
if _, exists := attrs[countK]; countK != "" && !exists {
attrs[countK] = "1"
}
}
return prefix + "0."
}
// fixupAsSingleAttrsMapKeysIn searches the given AttrDiff map for all keys with
// the given prefix (which must end with a dot) and replaces them with keys
// where that prefix is followed by the dummy index "0." and, if any such
// keys are found, a ".#"-suffixed key is also added whose value is "1".
//
// This function will also replace an exact match of the given prefix with
// the trailing dot removed, to recognize values of primitive-typed attributes.
func fixupAsSingleAttrsMapKeysIn(attrs map[string]*terraform.ResourceAttrDiff, prefix string) string {
ks := make([]string, 0, len(attrs))
for k := range attrs {
ks = append(ks, k)
}
sort.Strings(ks) // Makes no difference for valid input, but will ensure we handle invalid input deterministically
for _, k := range ks {
newK, countK := fixupAsSingleFlatmapKeyIn(k, prefix)
if _, exists := attrs[newK]; k != newK && !exists {
attrs[newK] = attrs[k]
delete(attrs, k)
}
if _, exists := attrs[countK]; countK != "" && !exists {
attrs[countK] = &terraform.ResourceAttrDiff{
Old: "1", // One should _always_ be present, so this seems okay?
New: "1",
}
}
}
return prefix + "0."
}
func fixupAsSingleFlatmapKeyIn(k, prefix string) (string, string) {
exact := prefix[:len(prefix)-1]
switch {
case k == exact:
return exact + ".0", exact + ".#"
case strings.HasPrefix(k, prefix):
return prefix + "0." + k[len(prefix):], prefix + "#"
default:
return k, ""
}
}
// fixupAsSingleFlatmapKeysOut searches the given flatmap for all keys with
// the given prefix (which must end with a dot) and replaces them with keys
// where the following dot-separated label is removed, under the assumption that
// it's an index that is no longer needed and, if such a key is present, also
// remove the "count" key for the prefix, which is the prefix followed by "#".
func fixupAsSingleFlatmapKeysOut(attrs map[string]string, prefix string) string {
ks := make([]string, 0, len(attrs))
for k := range attrs {
ks = append(ks, k)
}
sort.Strings(ks) // Makes no difference for valid input, but will ensure we handle invalid input deterministically
for _, k := range ks {
newK := fixupAsSingleFlatmapKeyOut(k, prefix)
if newK != k && newK == "" {
delete(attrs, k)
} else if _, exists := attrs[newK]; newK != k && !exists {
attrs[newK] = attrs[k]
delete(attrs, k)
}
}
delete(attrs, prefix+"#") // drop the count key, if it's present
return prefix
}
// fixupAsSingleAttrsMapKeysOut searches the given AttrDiff map for all keys with
// the given prefix (which must end with a dot) and replaces them with keys
// where the following dot-separated label is removed, under the assumption that
// it's an index that is no longer needed and, if such a key is present, also
// remove the "count" key for the prefix, which is the prefix followed by "#".
func fixupAsSingleAttrsMapKeysOut(attrs map[string]*terraform.ResourceAttrDiff, prefix string) string {
ks := make([]string, 0, len(attrs))
for k := range attrs {
ks = append(ks, k)
}
sort.Strings(ks) // Makes no difference for valid input, but will ensure we handle invalid input deterministically
for _, k := range ks {
newK := fixupAsSingleFlatmapKeyOut(k, prefix)
if newK != k && newK == "" {
delete(attrs, k)
} else if _, exists := attrs[newK]; newK != k && !exists {
attrs[newK] = attrs[k]
delete(attrs, k)
}
}
delete(attrs, prefix+"#") // drop the count key, if it's present
return prefix
}
func fixupAsSingleFlatmapKeyOut(k, prefix string) string {
if strings.HasPrefix(k, prefix) {
remain := k[len(prefix):]
if remain == "#" {
// Don't need the count element anymore
return ""
}
dotIdx := strings.Index(remain, ".")
if dotIdx == -1 {
return prefix[:len(prefix)-1] // no follow-on attributes then
} else {
return prefix + remain[dotIdx+1:] // everything after the next dot
}
}
return k
}

View File

@ -0,0 +1,239 @@
package schema
import (
"testing"
"github.com/google/go-cmp/cmp"
"github.com/hashicorp/terraform/terraform"
)
func TestFixupAsSingleInstanceStateInOut(t *testing.T) {
tests := map[string]struct {
AttrsIn map[string]string
AttrsOut map[string]string
Schema map[string]*Schema
}{
"empty": {
nil,
nil,
nil,
},
"simple": {
map[string]string{
"a": "a value",
},
map[string]string{
"a": "a value",
},
map[string]*Schema{
"a": {Type: TypeString, Optional: true},
},
},
"normal list of primitive, empty": {
map[string]string{
"a.%": "0",
},
map[string]string{
"a.%": "0",
},
map[string]*Schema{
"a": {
Type: TypeList,
Optional: true,
Elem: &Schema{Type: TypeString},
},
},
},
"normal list of primitive, single": {
map[string]string{
"a.%": "1",
"a.0": "hello",
},
map[string]string{
"a.%": "1",
"a.0": "hello",
},
map[string]*Schema{
"a": {
Type: TypeList,
Optional: true,
Elem: &Schema{Type: TypeString},
},
},
},
"AsSingle list of primitive": {
map[string]string{
"a": "hello",
},
map[string]string{
"a.#": "1",
"a.0": "hello",
},
map[string]*Schema{
"a": {
Type: TypeList,
Optional: true,
MaxItems: 1,
AsSingle: true,
Elem: &Schema{Type: TypeString},
},
},
},
"AsSingle list of resource": {
map[string]string{
"a.b": "hello",
},
map[string]string{
"a.#": "1",
"a.0.b": "hello",
},
map[string]*Schema{
"a": {
Type: TypeList,
Optional: true,
MaxItems: 1,
AsSingle: true,
Elem: &Resource{
Schema: map[string]*Schema{
"b": {
Type: TypeString,
Optional: true,
},
},
},
},
},
},
"AsSingle list of resource with nested primitive list": {
map[string]string{
"a.b.#": "1",
"a.b.0": "hello",
},
map[string]string{
"a.#": "1",
"a.0.b.#": "1",
"a.0.b.0": "hello",
},
map[string]*Schema{
"a": {
Type: TypeList,
Optional: true,
MaxItems: 1,
AsSingle: true,
Elem: &Resource{
Schema: map[string]*Schema{
"b": {
Type: TypeList,
Optional: true,
Elem: &Schema{Type: TypeString},
},
},
},
},
},
},
"AsSingle list of resource with nested AsSingle primitive list": {
map[string]string{
"a.b": "hello",
},
map[string]string{
"a.#": "1",
"a.0.b.#": "1",
"a.0.b.0": "hello",
},
map[string]*Schema{
"a": {
Type: TypeList,
Optional: true,
MaxItems: 1,
AsSingle: true,
Elem: &Resource{
Schema: map[string]*Schema{
"b": {
Type: TypeList,
Optional: true,
MaxItems: 1,
AsSingle: true,
Elem: &Schema{Type: TypeString},
},
},
},
},
},
},
"AsSingle list of resource with nested AsSingle resource list": {
map[string]string{
"a.b.c": "hello",
},
map[string]string{
"a.#": "1",
"a.0.b.#": "1",
"a.0.b.0.c": "hello",
},
map[string]*Schema{
"a": {
Type: TypeList,
Optional: true,
MaxItems: 1,
AsSingle: true,
Elem: &Resource{
Schema: map[string]*Schema{
"b": {
Type: TypeList,
Optional: true,
MaxItems: 1,
AsSingle: true,
Elem: &Resource{
Schema: map[string]*Schema{
"c": {
Type: TypeString,
Optional: true,
},
},
},
},
},
},
},
},
},
}
copyMap := func(m map[string]string) map[string]string {
if m == nil {
return nil
}
ret := make(map[string]string, len(m))
for k, v := range m {
ret[k] = v
}
return ret
}
for name, test := range tests {
t.Run(name, func(t *testing.T) {
t.Run("In", func(t *testing.T) {
input := copyMap(test.AttrsIn)
is := &terraform.InstanceState{
Attributes: input,
}
r := &Resource{Schema: test.Schema}
FixupAsSingleInstanceStateIn(is, r)
if !cmp.Equal(is.Attributes, test.AttrsOut) {
t.Errorf("wrong result\ninput: %#v\ngot: %#v\nwant: %#v\n\n%s", input, is.Attributes, test.AttrsOut, cmp.Diff(test.AttrsOut, is.Attributes))
}
})
t.Run("Out", func(t *testing.T) {
input := copyMap(test.AttrsOut)
is := &terraform.InstanceState{
Attributes: input,
}
r := &Resource{Schema: test.Schema}
FixupAsSingleInstanceStateOut(is, r)
if !cmp.Equal(is.Attributes, test.AttrsIn) {
t.Errorf("wrong result\ninput: %#v\ngot: %#v\nwant: %#v\n\n%s", input, is.Attributes, test.AttrsIn, cmp.Diff(test.AttrsIn, is.Attributes))
}
})
})
}
}

View File

@ -22,6 +22,24 @@ import (
// This method presumes a schema that passes InternalValidate, and so may
// panic or produce an invalid result if given an invalid schemaMap.
func (m schemaMap) CoreConfigSchema() *configschema.Block {
return m.coreConfigSchema(true)
}
// CoreConfigSchemaWhenShimmed is a variant of CoreConfigSchema that returns
// the schema as it would appear when working with data structures that have
// already been shimmed to the legacy form.
//
// In particular, it ignores the AsSingle flag on any legacy schemas and behaves
// as if they were really lists/sets instead, thus giving a description of
// the shape of the data structure after the AsSingle fixup has been applied.
//
// This should be used with care only in unusual situations where we need to
// work with an already-shimmed value using a new-style schema.
func (m schemaMap) CoreConfigSchemaWhenShimmed() *configschema.Block {
return m.coreConfigSchema(false)
}
func (m schemaMap) coreConfigSchema(enableAsSingle bool) *configschema.Block {
if len(m) == 0 {
// We return an actual (empty) object here, rather than a nil,
// because a nil result would mean that we don't have a schema at
@ -36,7 +54,7 @@ func (m schemaMap) CoreConfigSchema() *configschema.Block {
for name, schema := range m {
if schema.Elem == nil {
ret.Attributes[name] = schema.coreConfigSchemaAttribute()
ret.Attributes[name] = schema.coreConfigSchemaAttribute(enableAsSingle)
continue
}
if schema.Type == TypeMap {
@ -50,27 +68,27 @@ func (m schemaMap) CoreConfigSchema() *configschema.Block {
sch.Elem = &Schema{
Type: TypeString,
}
ret.Attributes[name] = sch.coreConfigSchemaAttribute()
ret.Attributes[name] = sch.coreConfigSchemaAttribute(enableAsSingle)
continue
}
}
switch schema.ConfigMode {
case SchemaConfigModeAttr:
ret.Attributes[name] = schema.coreConfigSchemaAttribute()
ret.Attributes[name] = schema.coreConfigSchemaAttribute(enableAsSingle)
case SchemaConfigModeBlock:
ret.BlockTypes[name] = schema.coreConfigSchemaBlock()
ret.BlockTypes[name] = schema.coreConfigSchemaBlock(enableAsSingle)
default: // SchemaConfigModeAuto, or any other invalid value
if schema.Computed && !schema.Optional {
// Computed-only schemas are always handled as attributes,
// because they never appear in configuration.
ret.Attributes[name] = schema.coreConfigSchemaAttribute()
ret.Attributes[name] = schema.coreConfigSchemaAttribute(enableAsSingle)
continue
}
switch schema.Elem.(type) {
case *Schema, ValueType:
ret.Attributes[name] = schema.coreConfigSchemaAttribute()
ret.Attributes[name] = schema.coreConfigSchemaAttribute(enableAsSingle)
case *Resource:
ret.BlockTypes[name] = schema.coreConfigSchemaBlock()
ret.BlockTypes[name] = schema.coreConfigSchemaBlock(enableAsSingle)
default:
// Should never happen for a valid schema
panic(fmt.Errorf("invalid Schema.Elem %#v; need *Schema or *Resource", schema.Elem))
@ -85,7 +103,7 @@ func (m schemaMap) CoreConfigSchema() *configschema.Block {
// of a schema. This is appropriate only for primitives or collections whose
// Elem is an instance of Schema. Use coreConfigSchemaBlock for collections
// whose elem is a whole resource.
func (s *Schema) coreConfigSchemaAttribute() *configschema.Attribute {
func (s *Schema) coreConfigSchemaAttribute(enableAsSingle bool) *configschema.Attribute {
// The Schema.DefaultFunc capability adds some extra weirdness here since
// it can be combined with "Required: true" to create a sitution where
// required-ness is conditional. Terraform Core doesn't share this concept,
@ -116,7 +134,7 @@ func (s *Schema) coreConfigSchemaAttribute() *configschema.Attribute {
}
return &configschema.Attribute{
Type: s.coreConfigSchemaType(),
Type: s.coreConfigSchemaType(enableAsSingle),
Optional: opt,
Required: reqd,
Computed: s.Computed,
@ -128,9 +146,9 @@ func (s *Schema) coreConfigSchemaAttribute() *configschema.Attribute {
// coreConfigSchemaBlock prepares a configschema.NestedBlock representation of
// a schema. This is appropriate only for collections whose Elem is an instance
// of Resource, and will panic otherwise.
func (s *Schema) coreConfigSchemaBlock() *configschema.NestedBlock {
func (s *Schema) coreConfigSchemaBlock(enableAsSingle bool) *configschema.NestedBlock {
ret := &configschema.NestedBlock{}
if nested := s.Elem.(*Resource).coreConfigSchema(); nested != nil {
if nested := schemaMap(s.Elem.(*Resource).Schema).coreConfigSchema(enableAsSingle); nested != nil {
ret.Block = *nested
}
switch s.Type {
@ -148,6 +166,14 @@ func (s *Schema) coreConfigSchemaBlock() *configschema.NestedBlock {
ret.MinItems = s.MinItems
ret.MaxItems = s.MaxItems
if s.AsSingle && enableAsSingle {
// In AsSingle mode, we artifically force a TypeList or TypeSet
// attribute in the SDK to be treated as a single block by Terraform Core.
// This must then be fixed up in the shim code (in helper/plugin) so
// that the SDK still sees the lists or sets it's expecting.
ret.Nesting = configschema.NestingSingle
}
if s.Required && s.MinItems == 0 {
// configschema doesn't have a "required" representation for nested
// blocks, but we can fake it by requiring at least one item.
@ -173,7 +199,7 @@ func (s *Schema) coreConfigSchemaBlock() *configschema.NestedBlock {
// coreConfigSchemaType determines the core config schema type that corresponds
// to a particular schema's type.
func (s *Schema) coreConfigSchemaType() cty.Type {
func (s *Schema) coreConfigSchemaType(enableAsSingle bool) cty.Type {
switch s.Type {
case TypeString:
return cty.String
@ -188,17 +214,17 @@ func (s *Schema) coreConfigSchemaType() cty.Type {
var elemType cty.Type
switch set := s.Elem.(type) {
case *Schema:
elemType = set.coreConfigSchemaType()
elemType = set.coreConfigSchemaType(enableAsSingle)
case ValueType:
// This represents a mistake in the provider code, but it's a
// common one so we'll just shim it.
elemType = (&Schema{Type: set}).coreConfigSchemaType()
elemType = (&Schema{Type: set}).coreConfigSchemaType(enableAsSingle)
case *Resource:
// By default we construct a NestedBlock in this case, but this
// behavior is selected either for computed-only schemas or
// when ConfigMode is explicitly SchemaConfigModeBlock.
// See schemaMap.CoreConfigSchema for the exact rules.
elemType = set.coreConfigSchema().ImpliedType()
elemType = schemaMap(set.Schema).coreConfigSchema(enableAsSingle).ImpliedType()
default:
if set != nil {
// Should never happen for a valid schema
@ -208,6 +234,13 @@ func (s *Schema) coreConfigSchemaType() cty.Type {
// to be compatible with them.
elemType = cty.String
}
if s.AsSingle && enableAsSingle {
// In AsSingle mode, we artifically force a TypeList or TypeSet
// attribute in the SDK to be treated as a single value by Terraform Core.
// This must then be fixed up in the shim code (in helper/plugin) so
// that the SDK still sees the lists or sets it's expecting.
return elemType
}
switch s.Type {
case TypeList:
return cty.List(elemType)
@ -229,7 +262,25 @@ func (s *Schema) coreConfigSchemaType() cty.Type {
// the resource's schema. CoreConfigSchema adds the implicitly required "id"
// attribute for top level resources if it doesn't exist.
func (r *Resource) CoreConfigSchema() *configschema.Block {
block := r.coreConfigSchema()
return r.coreConfigSchema(true)
}
// CoreConfigSchemaWhenShimmed is a variant of CoreConfigSchema that returns
// the schema as it would appear when working with data structures that have
// already been shimmed to the legacy form.
//
// In particular, it ignores the AsSingle flag on any legacy schemas and behaves
// as if they were really lists/sets instead, thus giving a description of
// the shape of the data structure after the AsSingle fixup has been applied.
//
// This should be used with care only in unusual situations where we need to
// work with an already-shimmed value using a new-style schema.
func (r *Resource) CoreConfigSchemaWhenShimmed() *configschema.Block {
return r.coreConfigSchema(false)
}
func (r *Resource) coreConfigSchema(enableAsSingle bool) *configschema.Block {
block := schemaMap(r.Schema).coreConfigSchema(enableAsSingle)
if block.Attributes == nil {
block.Attributes = map[string]*configschema.Attribute{}
@ -298,10 +349,6 @@ func (r *Resource) CoreConfigSchema() *configschema.Block {
return block
}
func (r *Resource) coreConfigSchema() *configschema.Block {
return schemaMap(r.Schema).CoreConfigSchema()
}
// CoreConfigSchema is a convenient shortcut for calling CoreConfigSchema
// on the backends's schema.
func (r *Backend) CoreConfigSchema() *configschema.Block {

View File

@ -162,6 +162,8 @@ func (r *Resource) ShimInstanceStateFromValue(state cty.Value) (*terraform.Insta
// match those from the Schema.
s := terraform.NewInstanceStateShimmedFromValue(state, r.SchemaVersion)
FixupAsSingleInstanceStateIn(s, r)
// We now rebuild the state through the ResourceData, so that the set indexes
// match what helper/schema expects.
data, err := schemaMap(r.Schema).Data(s, nil)

View File

@ -14,6 +14,10 @@ import (
// derives a terraform.InstanceDiff to give to the legacy providers. This is
// used to take the states provided by the new ApplyResourceChange method and
// convert them to a state+diff required for the legacy Apply method.
//
// If the fixup function is non-nil, it will be called with the constructed
// shimmed InstanceState and ResourceConfig values to do any necessary in-place
// mutations before producing the diff.
func DiffFromValues(prior, planned cty.Value, res *Resource) (*terraform.InstanceDiff, error) {
return diffFromValues(prior, planned, res, nil)
}
@ -24,6 +28,7 @@ func DiffFromValues(prior, planned cty.Value, res *Resource) (*terraform.Instanc
// have already been done.
func diffFromValues(prior, planned cty.Value, res *Resource, cust CustomizeDiffFunc) (*terraform.InstanceDiff, error) {
instanceState, err := res.ShimInstanceStateFromValue(prior)
// The result of ShimInstanceStateFromValue already has FixupAsSingleInstanceStateIn applied
if err != nil {
return nil, err
}
@ -31,6 +36,7 @@ func diffFromValues(prior, planned cty.Value, res *Resource, cust CustomizeDiffF
configSchema := res.CoreConfigSchema()
cfg := terraform.NewResourceConfigShimmed(planned, configSchema)
FixupAsSingleResourceConfigIn(cfg, schemaMap(res.Schema))
diff, err := schemaMap(res.Schema).Diff(instanceState, cfg, cust, nil, false)
if err != nil {