resilien
/
terraform
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Merge pull request #20081 from hashicorp/jbardin/list-block 

New Diff.Apply method
This commit is contained in:
James Bardin 2019-01-22 19:20:53 -05:00 committed by GitHub
parent f65b7c5372 273f20ec8b
commit 46a4628782
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 429 additions and 150 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
builtin/providers/test/provider.go
View File

@ -28,6 +28,7 @@ func Provider() terraform.ResourceProvider {
"test_resource_state_func": testResourceStateFunc(),
"test_resource_deprecated": testResourceDeprecated(),
"test_resource_defaults": testResourceDefaults(),
"test_resource_list": testResourceList(),
},
DataSourcesMap: map[string]*schema.Resource{
"test_data_source": testDataSource(),

3
builtin/providers/test/resource_defaults_test.go
View File

@ -66,7 +66,8 @@ resource "test_resource_defaults" "foo" {
}
func TestResourceDefaults_import(t *testing.T) {
// FIXME: this test fails
// FIXME: The ReadResource after ImportResourceState sin't returning the
// complete state, yet the later refresh does.
return
resource.UnitTest(t, resource.TestCase{

69
builtin/providers/test/resource_list.go Normal file
View File

@ -0,0 +1,69 @@
package test
import (
"github.com/hashicorp/terraform/helper/schema"
)
func testResourceList() *schema.Resource {
return &schema.Resource{
Create: testResourceListCreate,
Read: testResourceListRead,
Update: testResourceListUpdate,
Delete: testResourceListDelete,
Schema: map[string]*schema.Schema{
"list_block": {
Type: schema.TypeList,
Optional: true,
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
"string": {
Type: schema.TypeString,
Optional: true,
},
"int": {
Type: schema.TypeInt,
Optional: true,
},
"sublist_block": {
Type: schema.TypeList,
Optional: true,
Computed: true,
ForceNew: true,
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
"string": {
Type: schema.TypeString,
Required: true,
},
"int": {
Type: schema.TypeInt,
Required: true,
},
},
},
},
},
},
},
},
}
}
func testResourceListCreate(d *schema.ResourceData, meta interface{}) error {
d.SetId("testId")
return nil
}
func testResourceListRead(d *schema.ResourceData, meta interface{}) error {
return nil
}
func testResourceListUpdate(d *schema.ResourceData, meta interface{}) error {
return nil
}
func testResourceListDelete(d *schema.ResourceData, meta interface{}) error {
d.SetId("")
return nil
}

114
builtin/providers/test/resource_list_test.go Normal file
View File

@ -0,0 +1,114 @@
package test
import (
"strings"
"testing"
"github.com/hashicorp/terraform/helper/resource"
)
// an empty config should be ok, because no deprecated/removed fields are set.
func TestResourceList_changed(t *testing.T) {
resource.UnitTest(t, resource.TestCase{
Providers: testAccProviders,
CheckDestroy: testAccCheckResourceDestroy,
Steps: []resource.TestStep{
resource.TestStep{
Config: strings.TrimSpace(`
resource "test_resource_list" "foo" {
list_block {
string = "a"
int = 1
}
list_block {
string = "b"
int = 2
}
}
`),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr(
"test_resource_list.foo", "list_block.#", "2",
),
resource.TestCheckResourceAttr(
"test_resource_list.foo", "list_block.0.string", "a",
),
resource.TestCheckResourceAttr(
"test_resource_list.foo", "list_block.0.int", "1",
),
resource.TestCheckResourceAttr(
"test_resource_list.foo", "list_block.1.string", "b",
),
resource.TestCheckResourceAttr(
"test_resource_list.foo", "list_block.1.int", "2",
),
),
},
resource.TestStep{
Config: strings.TrimSpace(`
resource "test_resource_list" "foo" {
list_block {
string = "a"
int = 1
}
list_block {
string = "c"
int = 2
}
}
`),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr(
"test_resource_list.foo", "list_block.#", "2",
),
resource.TestCheckResourceAttr(
"test_resource_list.foo", "list_block.0.string", "a",
),
resource.TestCheckResourceAttr(
"test_resource_list.foo", "list_block.0.int", "1",
),
resource.TestCheckResourceAttr(
"test_resource_list.foo", "list_block.1.string", "c",
),
resource.TestCheckResourceAttr(
"test_resource_list.foo", "list_block.1.int", "2",
),
),
},
},
})
}
func TestResourceList_sublist(t *testing.T) {
resource.UnitTest(t, resource.TestCase{
Providers: testAccProviders,
CheckDestroy: testAccCheckResourceDestroy,
Steps: []resource.TestStep{
resource.TestStep{
Config: strings.TrimSpace(`
resource "test_resource_list" "foo" {
list_block {
sublist_block {
string = "a"
int = 1
}
}
}
`),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr(
"test_resource_list.foo", "list_block.0.sublist_block.#", "1",
),
resource.TestCheckResourceAttr(
"test_resource_list.foo", "list_block.0.sublist_block.0.string", "a",
),
resource.TestCheckResourceAttr(
"test_resource_list.foo", "list_block.0.sublist_block.0.int", "1",
),
),
},
},
})
}

50
builtin/providers/test/resource_nested_set_test.go
View File

@ -91,21 +91,11 @@ func TestResourceNestedSet_emptyNestedListBlock(t *testing.T) {
root := s.ModuleByPath(addrs.RootModuleInstance)
res := root.Resources["test_resource_nested_set.foo"]
found := false
for k, v := range res.Primary.Attributes {
for k := range res.Primary.Attributes {
if !regexp.MustCompile(`^with_list\.\d+\.list_block\.`).MatchString(k) {
continue
}
found = true
if strings.HasSuffix(k, ".#") {
if v != "1" {
return fmt.Errorf("expected block with no objects: got %s:%s", k, v)
}
continue
}
// there should be no other attribute values for an empty block
return fmt.Errorf("unexpected attribute: %s:%s", k, v)
}
if !found {
return fmt.Errorf("with_list.X.list_block not found")
@ -199,14 +189,27 @@ resource "test_resource_nested_set" "foo" {
}
}
`),
Check: checkFunc,
Check: resource.ComposeTestCheckFunc(
checkFunc,
resource.TestCheckResourceAttr(
"test_resource_nested_set.foo", "single.#", "1",
),
// the hash of single seems to change here, so we're not
// going to test for "value" directly
// FIXME: figure out why the set hash changes
),
},
resource.TestStep{
Config: strings.TrimSpace(`
resource "test_resource_nested_set" "foo" {
}
`),
Check: checkFunc,
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr(
"test_resource_nested_set.foo", "single.#", "0",
),
checkFunc,
),
},
resource.TestStep{
Config: strings.TrimSpace(`
@ -456,9 +459,6 @@ resource "test_resource_nested_set" "foo" {
// This is the same as forceNewEmptyString, but we start with the empty value,
// instead of changing it.
func TestResourceNestedSet_nestedSetEmptyString(t *testing.T) {
checkFunc := func(s *terraform.State) error {
return nil
}
resource.UnitTest(t, resource.TestCase{
Providers: testAccProviders,
CheckDestroy: testAccCheckResourceDestroy,
@ -473,19 +473,17 @@ resource "test_resource_nested_set" "foo" {
}
}
`),
Check: checkFunc,
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr(
"test_resource_nested_set.foo", "multi.529860700.set.4196279896.required", "",
),
),
},
},
})
}
func TestResourceNestedSet_emptySet(t *testing.T) {
// FIXME: this test fails
return
checkFunc := func(s *terraform.State) error {
return nil
}
resource.UnitTest(t, resource.TestCase{
Providers: testAccProviders,
CheckDestroy: testAccCheckResourceDestroy,
@ -497,7 +495,11 @@ resource "test_resource_nested_set" "foo" {
}
}
`),
Check: checkFunc,
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr(
"test_resource_nested_set.foo", "multi.#", "1",
),
),
},
},
})

12
helper/plugin/grpc_provider.go
View File

@ -513,16 +513,6 @@ func (s *GRPCProviderServer) PlanResourceChange(_ context.Context, req *proto.Pl
return resp, nil
}
if diff != nil {
// strip out non-diffs
for k, v := range diff.Attributes {
if v.New == v.Old && !v.NewComputed {
delete(diff.Attributes, k)
}
}
}
if diff == nil || len(diff.Attributes) == 0 {
// schema.Provider.Diff returns nil if it ends up making a diff with no
// changes, but our new interface wants us to return an actual change
@ -966,6 +956,8 @@ func normalizeFlatmapContainers(prior map[string]string, attrs map[string]string
// schema.
if isCount(k) && v == "1" {
ones[k] = true
// make sure we don't have the same key under both categories.
delete(zeros, k)
}
}

4
terraform/context_plan_test.go
View File

@ -2267,12 +2267,12 @@ func TestContext2Plan_computedMultiIndex(t *testing.T) {
case "aws_instance.foo[0]":
checkVals(t, objectVal(t, schema, map[string]cty.Value{
"ip": cty.UnknownVal(cty.List(cty.String)),
"foo": cty.ListValEmpty(cty.String),
"foo": cty.NullVal(cty.List(cty.String)),
}), ric.After)
case "aws_instance.foo[1]":
checkVals(t, objectVal(t, schema, map[string]cty.Value{
"ip": cty.UnknownVal(cty.List(cty.String)),
"foo": cty.ListValEmpty(cty.String),
"foo": cty.NullVal(cty.List(cty.String)),
}), ric.After)
case "aws_instance.bar[0]":
checkVals(t, objectVal(t, schema, map[string]cty.Value{

322
terraform/diff.go
View File

@ -416,7 +416,6 @@ func (d *InstanceDiff) Unlock() { d.mu.Unlock() }
// This method is intended for shimming old subsystems that still use this
// legacy diff type to work with the new-style types.
func (d *InstanceDiff) ApplyToValue(base cty.Value, schema *configschema.Block) (cty.Value, error) {
// Create an InstanceState attributes from our existing state.
// We can use this to more easily apply the diff changes.
attrs := hcl2shim.FlatmapValueFromHCL2(base)
@ -447,10 +446,6 @@ func (d *InstanceDiff) Apply(attrs map[string]string, schema *configschema.Block
attrs = map[string]string{}
}
return d.applyDiff(attrs, schema)
}
func (d *InstanceDiff) applyDiff(attrs map[string]string, schema *configschema.Block) (map[string]string, error) {
// Rather applying the diff to mutate the attrs, we'll copy new values into
// here to avoid the possibility of leaving stale values.
result := map[string]string{}
@ -459,61 +454,196 @@ func (d *InstanceDiff) applyDiff(attrs map[string]string, schema *configschema.B
return result, nil
}
// iterate over the schema rather than the attributes, so we can handle
// blocks separately from plain attributes
for name, attrSchema := range schema.Attributes {
var err error
var newAttrs map[string]string
return d.blockDiff(nil, attrs, schema)
}
// handle non-block collections
switch ty := attrSchema.Type; {
case ty.IsListType() || ty.IsTupleType() || ty.IsMapType():
newAttrs, err = d.applyCollectionDiff(name, attrs, attrSchema)
case ty.IsSetType():
newAttrs, err = d.applySetDiff(name, attrs, schema)
default:
newAttrs, err = d.applyAttrDiff(name, attrs, attrSchema)
}
func (d *InstanceDiff) blockDiff(path []string, attrs map[string]string, schema *configschema.Block) (map[string]string, error) {
result := map[string]string{}
name := ""
if len(path) > 0 {
name = path[len(path)-1]
}
// localPrefix is used to build the local result map
localPrefix := ""
if name != "" {
localPrefix = name + "."
}
// iterate over the schema rather than the attributes, so we can handle
// different block types separately from plain attributes
for n, attrSchema := range schema.Attributes {
var err error
newAttrs, err := d.applyAttrDiff(append(path, n), attrs, attrSchema)
if err != nil {
return result, err
}
for k, v := range newAttrs {
result[k] = v
result[localPrefix+k] = v
}
}
for name, block := range schema.BlockTypes {
newAttrs, err := d.applySetDiff(name, attrs, &block.Block)
if err != nil {
return result, err
blockPrefix := strings.Join(path, ".")
if blockPrefix != "" {
blockPrefix += "."
}
for n, block := range schema.BlockTypes {
// we need to find the set of all keys that traverse this block
candidateKeys := map[string]bool{}
blockKey := blockPrefix + n + "."
// we can only trust the diff for sets, since the path changes, so don't
// count existing values as candidate keys. If it turns out we're
// keeping the attributes, we will check catch it down below with
// "keepBlock" after we check the set count.
if block.Nesting != configschema.NestingSet {
for k := range attrs {
if strings.HasPrefix(k, blockKey) {
nextDot := strings.Index(k[len(blockKey):], ".")
if nextDot < 0 {
continue
}
nextDot += len(blockKey)
candidateKeys[k[len(blockKey):nextDot]] = true
}
}
}
for k, v := range newAttrs {
result[k] = v
for k, diff := range d.Attributes {
if strings.HasPrefix(k, blockKey) {
nextDot := strings.Index(k[len(blockKey):], ".")
if nextDot < 0 {
continue
}
if diff.NewRemoved {
continue
}
nextDot += len(blockKey)
candidateKeys[k[len(blockKey):nextDot]] = true
}
}
// check each set candidate to see if it was removed.
// we need to do this, because when entire sets are removed, they may
// have the wrong key, and ony show diffs going to ""
if block.Nesting == configschema.NestingSet {
for k := range candidateKeys {
indexPrefix := strings.Join(append(path, n, k), ".") + "."
keep := false
// now check each set element to see if it's a new diff, or one
// that we're dropping. Since we're only applying the "New"
// portion of the set, we can ignore diffs that only contain "Old"
for attr, diff := range d.Attributes {
if !strings.HasPrefix(attr, indexPrefix) {
continue
}
// check for empty "count" keys
if strings.HasSuffix(attr, ".#") && diff.New == "0" {
continue
}
// removed items don't count either
if diff.NewRemoved {
continue
}
// this must be a diff to keep
keep = true
break
}
if !keep {
delete(candidateKeys, k)
}
}
}
for k := range candidateKeys {
newAttrs, err := d.blockDiff(append(path, n, k), attrs, &block.Block)
if err != nil {
return result, err
}
for attr, v := range newAttrs {
result[localPrefix+n+"."+attr] = v
}
}
keepBlock := true
// check this block's count diff directly first, since we may not
// have candidates because it was removed and only set to "0"
if diff, ok := d.Attributes[blockKey+"#"]; ok {
if diff.New == "0" || diff.NewRemoved {
keepBlock = false
}
}
// if there was no diff at all, then we need to keep the block attributes
if len(candidateKeys) == 0 && keepBlock {
for k, v := range attrs {
if strings.HasPrefix(k, blockKey) {
// we need the key relative to this block, so remove the
// entire prefix, then re-insert the block name.
localKey := n + "." + k[len(blockKey):]
result[localKey] = v
}
}
}
if countDiff, ok := d.Attributes[strings.Join(append(path, n, ".#"), ".")]; ok {
if countDiff.NewComputed {
result[localPrefix+n+".#"] = hcl2shim.UnknownVariableValue
} else {
result[localPrefix+n+".#"] = countDiff.New
}
} else {
result[localPrefix+n+".#"] = countFlatmapContainerValues(localPrefix+n+".#", result)
}
}
return result, nil
}
func (d *InstanceDiff) applyAttrDiff(attrName string, oldAttrs map[string]string, attrSchema *configschema.Attribute) (map[string]string, error) {
result := map[string]string{}
func (d *InstanceDiff) applyAttrDiff(path []string, attrs map[string]string, attrSchema *configschema.Attribute) (map[string]string, error) {
ty := attrSchema.Type
switch {
case ty.IsListType(), ty.IsTupleType(), ty.IsMapType(), ty.IsSetType():
return d.applyCollectionDiff(path, attrs, attrSchema)
default:
return d.applySingleAttrDiff(path, attrs, attrSchema)
}
}
diff := d.Attributes[attrName]
old, exists := oldAttrs[attrName]
func (d *InstanceDiff) applySingleAttrDiff(path []string, attrs map[string]string, attrSchema *configschema.Attribute) (map[string]string, error) {
currentKey := strings.Join(path, ".")
attr := path[len(path)-1]
result := map[string]string{}
diff := d.Attributes[currentKey]
old, exists := attrs[currentKey]
if diff != nil && diff.NewComputed {
result[attrName] = config.UnknownVariableValue
result[attr] = config.UnknownVariableValue
return result, nil
}
if attrName == "id" {
// "id" must exist and not be an empty string, or it must be unknown
if attr == "id" {
if old == "" {
result["id"] = config.UnknownVariableValue
result[attr] = config.UnknownVariableValue
} else {
result["id"] = old
result[attr] = old
}
return result, nil
}
@ -522,29 +652,41 @@ func (d *InstanceDiff) applyAttrDiff(attrName string, oldAttrs map[string]string
// old value
if diff == nil {
if exists {
result[attrName] = old
result[attr] = old
} else {
// We need required values, so set those with an empty value. It
// must be set in the config, since if it were missing it would have
// failed validation.
if attrSchema.Required {
result[attrName] = ""
// we only set a missing string here, since bool or number types
// would have distinct zero value which shouldn't have been
// lost.
if attrSchema.Type == cty.String {
result[attr] = ""
}
}
}
return result, nil
}
if diff.Old == diff.New && diff.New == "" {
// this can only be a valid empty string
if attrSchema.Type == cty.String {
result[attr] = ""
}
return result, nil
}
// check for missmatched diff values
if exists &&
old != diff.Old &&
old != config.UnknownVariableValue &&
diff.Old != config.UnknownVariableValue {
return result, fmt.Errorf("diff apply conflict for %s: diff expects %q, but prior value has %q", attrName, diff.Old, old)
return result, fmt.Errorf("diff apply conflict for %s: diff expects %q, but prior value has %q", attr, diff.Old, old)
}
if attrSchema.Computed && diff.NewComputed {
result[attrName] = config.UnknownVariableValue
result[attr] = config.UnknownVariableValue
return result, nil
}
@ -553,29 +695,42 @@ func (d *InstanceDiff) applyAttrDiff(attrName string, oldAttrs map[string]string
return result, nil
}
result[attrName] = diff.New
result[attr] = diff.New
return result, nil
}
func (d *InstanceDiff) applyCollectionDiff(attrName string, oldAttrs map[string]string, attrSchema *configschema.Attribute) (map[string]string, error) {
func (d *InstanceDiff) applyCollectionDiff(path []string, attrs map[string]string, attrSchema *configschema.Attribute) (map[string]string, error) {
result := map[string]string{}
prefix := ""
if len(path) > 1 {
prefix = strings.Join(path[:len(path)-1], ".") + "."
}
name := ""
if len(path) > 0 {
name = path[len(path)-1]
}
currentKey := prefix + name
// check the index first for special handling
for k, diff := range d.Attributes {
// check the index value, which can be set, and 0
if k == attrName+".#" || k == attrName+".%" || k == attrName {
if k == currentKey+".#" || k == currentKey+".%" || k == currentKey {
if diff.NewRemoved {
return result, nil
}
if diff.NewComputed {
result[k] = config.UnknownVariableValue
result[k[len(prefix):]] = config.UnknownVariableValue
return result, nil
}
// do what the diff tells us to here, so that it's consistent with applies
if diff.New == "0" {
result[k] = "0"
result[k[len(prefix):]] = "0"
return result, nil
}
}
@ -586,93 +741,38 @@ func (d *InstanceDiff) applyCollectionDiff(attrName string, oldAttrs map[string]
for k := range d.Attributes {
keys[k] = true
}
for k := range oldAttrs {
for k := range attrs {
keys[k] = true
}
idx := attrName + ".#"
idx := "#"
if attrSchema.Type.IsMapType() {
idx = attrName + ".%"
idx = "%"
}
for k := range keys {
if !strings.HasPrefix(k, attrName+".") {
if !strings.HasPrefix(k, currentKey+".") {
continue
}
res, err := d.applyAttrDiff(k, oldAttrs, attrSchema)
// generate an schema placeholder for the values
elSchema := &configschema.Attribute{
Type: attrSchema.Type.ElementType(),
}
res, err := d.applySingleAttrDiff(append(path, k[len(currentKey)+1:]), attrs, elSchema)
if err != nil {
return result, err
}
for k, v := range res {
result[k] = v
result[name+"."+k] = v
}
}
// Don't trust helper/schema to return a valid count, or even have one at
// all.
result[idx] = countFlatmapContainerValues(idx, result)
return result, nil
}
func (d *InstanceDiff) applySetDiff(attrName string, oldAttrs map[string]string, block *configschema.Block) (map[string]string, error) {
result := map[string]string{}
idx := attrName + ".#"
// first find the index diff
for k, diff := range d.Attributes {
if k != idx {
continue
}
if diff.NewComputed {
result[k] = config.UnknownVariableValue
return result, nil
}
}
// Flag if there was a diff used in the set at all.
// If not, take the pre-existing set values
setDiff := false
// here we're trusting the diff to supply all the known items
for k, diff := range d.Attributes {
if !strings.HasPrefix(k, attrName+".") {
continue
}
setDiff = true
if diff.NewRemoved {
// no longer in the set
continue
}
if diff.NewComputed {
result[k] = config.UnknownVariableValue
continue
}
// helper/schema doesn't list old removed values, but since the set
// exists NewRemoved may not be true.
if diff.New == "" && diff.Old == "" {
continue
}
result[k] = diff.New
}
// use the existing values if there was no set diff at all
if !setDiff {
for k, v := range oldAttrs {
if strings.HasPrefix(k, attrName+".") {
result[k] = v
}
}
}
result[idx] = countFlatmapContainerValues(idx, result)
result[idx] = countFlatmapContainerValues(name+"."+idx, result)
return result, nil
}

4
terraform/resource.go
View File

@ -297,14 +297,14 @@ func newResourceConfigShimmedComputedKeys(obj cty.Value, schema *configschema.Bl
i := 0
for it := blockVal.ElementIterator(); it.Next(); i++ {
_, subVal := it.Element()
subPrefix := fmt.Sprintf("%s%d.", prefix, i)
subPrefix := fmt.Sprintf("%s.%s%d.", typeName, prefix, i)
keys := newResourceConfigShimmedComputedKeys(subVal, &blockS.Block, subPrefix)
ret = append(ret, keys...)
}
case configschema.NestingMap:
for it := blockVal.ElementIterator(); it.Next(); {
subK, subVal := it.Element()
subPrefix := fmt.Sprintf("%s%s.", prefix, subK.AsString())
subPrefix := fmt.Sprintf("%s.%s%s.", typeName, prefix, subK.AsString())
keys := newResourceConfigShimmedComputedKeys(subVal, &blockS.Block, subPrefix)
ret = append(ret, keys...)
}