Merge pull request #20224 from hashicorp/jbardin/sdk

SDK set fixes

builtin/providers/test/provider.go

@@ -30,7 +30,9 @@ func Provider() terraform.ResourceProvider {
 			"test_resource_deprecated":       testResourceDeprecated(),
 			"test_resource_defaults":         testResourceDefaults(),
 			"test_resource_list":             testResourceList(),
+			"test_resource_list_set":         testResourceListSet(),
 			"test_resource_map":              testResourceMap(),
+			"test_resource_computed_set":     testResourceComputedSet(),
 		},
 		DataSourcesMap: map[string]*schema.Resource{
 			"test_data_source":    testDataSource(),

builtin/providers/test/resource_computed_set.go

@@ -0,0 +1,74 @@
+package test
+
+import (
+	"fmt"
+	"math/rand"
+
+	"github.com/hashicorp/terraform/helper/schema"
+)
+
+func testResourceComputedSet() *schema.Resource {
+	return &schema.Resource{
+		Create: testResourceComputedSetCreate,
+		Read:   testResourceComputedSetRead,
+		Delete: testResourceComputedSetDelete,
+		Update: testResourceComputedSetUpdate,
+
+		CustomizeDiff: func(d *schema.ResourceDiff, _ interface{}) error {
+			o, n := d.GetChange("set_count")
+			if o != n {
+				d.SetNewComputed("string_set")
+			}
+			return nil
+		},
+
+		Importer: &schema.ResourceImporter{
+			State: schema.ImportStatePassthrough,
+		},
+
+		Schema: map[string]*schema.Schema{
+			"set_count": {
+				Type:     schema.TypeInt,
+				Optional: true,
+			},
+			"string_set": {
+				Type:     schema.TypeSet,
+				Computed: true,
+				Elem: &schema.Schema{
+					Type: schema.TypeString,
+				},
+				Set: schema.HashString,
+			},
+		},
+	}
+}
+
+func testResourceComputedSetCreate(d *schema.ResourceData, meta interface{}) error {
+	d.SetId(fmt.Sprintf("%x", rand.Int63()))
+	return testResourceComputedSetRead(d, meta)
+}
+
+func testResourceComputedSetRead(d *schema.ResourceData, meta interface{}) error {
+	count := 3
+	v, ok := d.GetOk("set_count")
+	if ok {
+		count = v.(int)
+	}
+
+	var set []interface{}
+	for i := 0; i < count; i++ {
+		set = append(set, fmt.Sprintf("%d", i))
+	}
+
+	d.Set("string_set", schema.NewSet(schema.HashString, set))
+	return nil
+}
+
+func testResourceComputedSetUpdate(d *schema.ResourceData, meta interface{}) error {
+	return testResourceComputedSetRead(d, meta)
+}
+
+func testResourceComputedSetDelete(d *schema.ResourceData, meta interface{}) error {
+	d.SetId("")
+	return nil
+}

builtin/providers/test/resource_computed_set_test.go

@@ -0,0 +1,52 @@
+package test
+
+import (
+	"strings"
+	"testing"
+
+	"github.com/hashicorp/terraform/helper/resource"
+)
+
+func TestResourceComputedSet_update(t *testing.T) {
+	resource.UnitTest(t, resource.TestCase{
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckResourceDestroy,
+		Steps: []resource.TestStep{
+			resource.TestStep{
+				Config: strings.TrimSpace(`
+resource "test_resource_computed_set" "foo" {
+}
+				`),
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr(
+						"test_resource_computed_set.foo", "string_set.#", "3",
+					),
+				),
+			},
+			resource.TestStep{
+				Config: strings.TrimSpace(`
+resource "test_resource_computed_set" "foo" {
+	set_count = 5
+}
+				`),
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr(
+						"test_resource_computed_set.foo", "string_set.#", "5",
+					),
+				),
+			},
+			resource.TestStep{
+				Config: strings.TrimSpace(`
+resource "test_resource_computed_set" "foo" {
+	set_count = 2
+}
+				`),
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr(
+						"test_resource_computed_set.foo", "string_set.#", "2",
+					),
+				),
+			},
+		},
+	})
+}

builtin/providers/test/resource_list_set.go

@@ -0,0 +1,73 @@
+package test
+
+import (
+	"fmt"
+	"math/rand"
+
+	"github.com/hashicorp/terraform/helper/schema"
+)
+
+func testResourceListSet() *schema.Resource {
+	return &schema.Resource{
+		Create: testResourceListSetCreate,
+		Read:   testResourceListSetRead,
+		Delete: testResourceListSetDelete,
+		Update: testResourceListSetUpdate,
+
+		Importer: &schema.ResourceImporter{
+			State: schema.ImportStatePassthrough,
+		},
+
+		Schema: map[string]*schema.Schema{
+			"list": {
+				Type:     schema.TypeList,
+				Optional: true,
+				MaxItems: 1,
+				Elem: &schema.Resource{
+					Schema: map[string]*schema.Schema{
+						"set": {
+							Type:     schema.TypeSet,
+							Optional: true,
+							Elem: &schema.Resource{
+								Schema: map[string]*schema.Schema{
+									"elem": {
+										Type:     schema.TypeString,
+										Optional: true,
+										DiffSuppressFunc: func(_, o, n string, _ *schema.ResourceData) bool {
+											return o == n
+										},
+									},
+								},
+							},
+							Set: func(v interface{}) int {
+								raw := v.(map[string]interface{})
+								if el, ok := raw["elem"]; ok {
+									return schema.HashString(el)
+								}
+								return 42
+							},
+						},
+					},
+				},
+			},
+		},
+	}
+}
+
+func testResourceListSetCreate(d *schema.ResourceData, meta interface{}) error {
+	d.SetId(fmt.Sprintf("%x", rand.Int63()))
+	return testResourceListSetRead(d, meta)
+}
+
+func testResourceListSetUpdate(d *schema.ResourceData, meta interface{}) error {
+	return testResourceListSetRead(d, meta)
+}
+
+func testResourceListSetRead(d *schema.ResourceData, meta interface{}) error {
+	return nil
+}
+
+func testResourceListSetDelete(d *schema.ResourceData, meta interface{}) error {
+	d.SetId("")
+	return nil
+}

builtin/providers/test/resource_list_set_test.go

@@ -0,0 +1,55 @@
+package test
+
+import (
+	"strings"
+	"testing"
+
+	"github.com/hashicorp/terraform/helper/resource"
+)
+
+func TestResourceListSet_basic(t *testing.T) {
+	resource.UnitTest(t, resource.TestCase{
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckResourceDestroy,
+		Steps: []resource.TestStep{
+			resource.TestStep{
+				Config: strings.TrimSpace(`
+resource "test_resource_list_set" "foo" {
+  list {
+    set {
+      elem = "A"
+    }
+    set {
+      elem = "B"
+    }
+  }
+}
+				`),
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr("test_resource_list_set.foo", "list.0.set.1255198513.elem", "B"),
+					resource.TestCheckResourceAttr("test_resource_list_set.foo", "list.0.set.3554254475.elem", "A"),
+					resource.TestCheckResourceAttr("test_resource_list_set.foo", "list.0.set.#", "2"),
+				),
+			},
+			resource.TestStep{
+				Config: strings.TrimSpace(`
+resource "test_resource_list_set" "foo" {
+  list {
+    set {
+      elem = "B"
+    }
+    set {
+      elem = "C"
+    }
+  }
+}
+				`),
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr("test_resource_list_set.foo", "list.0.set.1255198513.elem", "B"),
+					resource.TestCheckResourceAttr("test_resource_list_set.foo", "list.0.set.1037565863.elem", "C"),
+					resource.TestCheckResourceAttr("test_resource_list_set.foo", "list.0.set.#", "2"),
+				),
+			},
+		},
+	})
+}

builtin/providers/test/resource_nested_set_test.go

@@ -496,3 +496,34 @@ resource "test_resource_nested_set" "foo" {
 		},
 	})
 }
+
+func TestResourceNestedSet_multipleUnknownSetElements(t *testing.T) {
+	checkFunc := func(s *terraform.State) error {
+		return nil
+	}
+	resource.UnitTest(t, resource.TestCase{
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckResourceDestroy,
+		Steps: []resource.TestStep{
+			resource.TestStep{
+				Config: strings.TrimSpace(`
+resource "test_resource_nested_set" "a" {
+}
+
+resource "test_resource_nested_set" "b" {
+}
+
+resource "test_resource_nested_set" "c" {
+	multi {
+		optional = test_resource_nested_set.a.id
+	}
+	multi {
+		optional = test_resource_nested_set.b.id
+	}
+}
+				`),
+				Check: checkFunc,
+			},
+		},
+	})
+}

helper/plugin/grpc_provider.go

@@ -44,6 +44,10 @@ type GRPCProviderServer struct {
 }
 
 func (s *GRPCProviderServer) GetSchema(_ context.Context, req *proto.GetProviderSchema_Request) (*proto.GetProviderSchema_Response, error) {
+	// Here we are certain that the provider is being called through grpc, so
+	// make sure the feature flag for helper/schema is set
+	schema.SetProto5()
+
 	resp := &proto.GetProviderSchema_Response{
 		ResourceSchemas:   make(map[string]*proto.Schema),
 		DataSourceSchemas: make(map[string]*proto.Schema),
@@ -454,7 +458,6 @@ func (s *GRPCProviderServer) ReadResource(_ context.Context, req *proto.ReadReso
 	}
 
 	newStateVal = copyTimeoutValues(newStateVal, stateVal)
-	newStateVal = normalizeNullValues(newStateVal, stateVal, false)
 
 	newStateMP, err := msgpack.Marshal(newStateVal, block.ImpliedType())
 	if err != nil {

helper/schema/schema.go

@@ -19,6 +19,7 @@ import (
 	"sort"
 	"strconv"
 	"strings"
+	"sync"
 
 	"github.com/hashicorp/terraform/config"
 	"github.com/hashicorp/terraform/terraform"
@@ -32,6 +33,27 @@ const PanicOnErr = "TF_SCHEMA_PANIC_ON_ERROR"
 // type used for schema package context keys
 type contextKey string
 
+var (
+	protoVersionMu sync.Mutex
+	protoVersion5  = false
+)
+
+func isProto5() bool {
+	protoVersionMu.Lock()
+	defer protoVersionMu.Unlock()
+	return protoVersion5
+
+}
+
+// SetProto5 enables a feature flag for any internal changes required required
+// to work with the new plugin protocol.  This should not be called by
+// provider.
+func SetProto5() {
+	protoVersionMu.Lock()
+	defer protoVersionMu.Unlock()
+	protoVersion5 = true
+}
+
 // Schema is used to describe the structure of a value.
 //
 // Read the documentation of the struct elements for important details.
@@ -799,11 +821,20 @@ func (m schemaMap) diff(
 	for attrK, attrV := range unsupressedDiff.Attributes {
 		switch rd := d.(type) {
 		case *ResourceData:
-			if schema.DiffSuppressFunc != nil &&
-				attrV != nil &&
+			if schema.DiffSuppressFunc != nil && attrV != nil &&
 				schema.DiffSuppressFunc(attrK, attrV.Old, attrV.New, rd) {
+				// If this attr diff is suppressed, we may still need it in the
+				// overall diff if it's contained within a set. Rather than
+				// dropping the diff, make it a NOOP.
+				if !all {
 					continue
 				}
+
+				attrV = &terraform.ResourceAttrDiff{
+					Old: attrV.Old,
+					New: attrV.Old,
+				}
+			}
 		}
 		diff.Attributes[attrK] = attrV
 	}

helper/schema/set.go

@@ -198,6 +198,16 @@ func (s *Set) add(item interface{}, computed bool) string {
 	code := s.hash(item)
 	if computed {
 		code = "~" + code
+
+		if isProto5() {
+			tmpCode := code
+			count := 0
+			for _, exists := s.m[tmpCode]; exists; _, exists = s.m[tmpCode] {
+				count++
+				tmpCode = fmt.Sprintf("%s%d", code, count)
+			}
+			code = tmpCode
+		}
 	}
 
 	if _, ok := s.m[code]; !ok {