Mark token attribute in resources and datasources as deprecated (#321)

Having the token attribute in the resources causes issues: when a user updates it the previous value is used when calling the `Read()` method. This is expected based on the protocol Terraform Core uses and there is no way to have the correct behavior here. To fix this I mark the "token" attribute as deprecated so that users only set the one in the "provider" block. I also mark all "datacenter", "namespace" and "partition" attribute as ForceNew to avoid the same issue with those. Closes #298
hashicorp · Dec 12, 2022 · 61a64cb · 61a64cb
1 parent 50e7bc2
commit 61a64cb
Show file tree

Hide file tree

Showing 13 changed files with 93 additions and 30 deletions.
diff --git a/consul/data_source_consul_key_prefix.go b/consul/data_source_consul_key_prefix.go
@@ -16,9 +16,10 @@ func dataSourceConsulKeyPrefix() *schema.Resource {
 			},
 
 			"token": {
-				Type:      schema.TypeString,
-				Optional:  true,
-				Sensitive: true,
+				Type:       schema.TypeString,
+				Optional:   true,
+				Sensitive:  true,
+				Deprecated: tokenDeprecationMessage,
 			},
 
 			"path_prefix": {

diff --git a/consul/data_source_consul_keys.go b/consul/data_source_consul_keys.go
@@ -16,9 +16,10 @@ func dataSourceConsulKeys() *schema.Resource {
 			},
 
 			"token": {
-				Type:      schema.TypeString,
-				Optional:  true,
-				Sensitive: true,
+				Type:       schema.TypeString,
+				Optional:   true,
+				Sensitive:  true,
+				Deprecated: tokenDeprecationMessage,
 			},
 
 			"key": {

diff --git a/consul/data_source_consul_network_area_members.go b/consul/data_source_consul_network_area_members.go
@@ -19,9 +19,10 @@ func dataSourceConsulNetworkAreaMembers() *schema.Resource {
 			},
 
 			"token": {
-				Type:      schema.TypeString,
-				Optional:  true,
-				Sensitive: true,
+				Type:       schema.TypeString,
+				Optional:   true,
+				Sensitive:  true,
+				Deprecated: tokenDeprecationMessage,
 			},
 
 			"uuid": {

diff --git a/consul/data_source_consul_network_segments.go b/consul/data_source_consul_network_segments.go
@@ -19,9 +19,10 @@ func dataSourceConsulNetworkSegments() *schema.Resource {
 			},
 
 			"token": {
-				Type:      schema.TypeString,
-				Optional:  true,
-				Sensitive: true,
+				Type:       schema.TypeString,
+				Optional:   true,
+				Sensitive:  true,
+				Deprecated: tokenDeprecationMessage,
 			},
 
 			// Outputs

diff --git a/consul/resource_consul_autopilot_config.go b/consul/resource_consul_autopilot_config.go
@@ -19,6 +19,7 @@ func resourceConsulAutopilotConfig() *schema.Resource {
 			"datacenter": {
 				Type:     schema.TypeString,
 				Optional: true,
+				ForceNew: true,
 			},
 			"cleanup_dead_servers": {
 				Type:     schema.TypeBool,

diff --git a/consul/resource_consul_catalog_entry.go b/consul/resource_consul_catalog_entry.go
@@ -83,9 +83,10 @@ func resourceConsulCatalogEntry() *schema.Resource {
 			},
 
 			"token": {
-				Type:      schema.TypeString,
-				Optional:  true,
-				Sensitive: true,
+				Type:       schema.TypeString,
+				Optional:   true,
+				Sensitive:  true,
+				Deprecated: tokenDeprecationMessage,
 			},
 		},
 	}

diff --git a/consul/resource_consul_key_prefix.go b/consul/resource_consul_key_prefix.go
@@ -30,9 +30,10 @@ func resourceConsulKeyPrefix() *schema.Resource {
 			},
 
 			"token": {
-				Type:      schema.TypeString,
-				Optional:  true,
-				Sensitive: true,
+				Type:       schema.TypeString,
+				Optional:   true,
+				Sensitive:  true,
+				Deprecated: tokenDeprecationMessage,
 			},
 
 			"path_prefix": {

diff --git a/consul/resource_consul_keys.go b/consul/resource_consul_keys.go
@@ -33,9 +33,10 @@ func resourceConsulKeys() *schema.Resource {
 			},
 
 			"token": {
-				Type:      schema.TypeString,
-				Optional:  true,
-				Sensitive: true,
+				Type:       schema.TypeString,
+				Optional:   true,
+				Sensitive:  true,
+				Deprecated: tokenDeprecationMessage,
 			},
 
 			"key": {

diff --git a/consul/resource_consul_network_area.go b/consul/resource_consul_network_area.go
@@ -23,9 +23,10 @@ func resourceConsulNetworkArea() *schema.Resource {
 			},
 
 			"token": {
-				Type:      schema.TypeString,
-				Optional:  true,
-				Sensitive: true,
+				Type:       schema.TypeString,
+				Optional:   true,
+				Sensitive:  true,
+				Deprecated: tokenDeprecationMessage,
 			},
 
 			"peer_datacenter": {

diff --git a/consul/resource_consul_node.go b/consul/resource_consul_node.go
@@ -53,9 +53,10 @@ func resourceConsulNode() *schema.Resource {
 			},
 
 			"token": {
-				Type:      schema.TypeString,
-				Optional:  true,
-				Sensitive: true,
+				Type:       schema.TypeString,
+				Optional:   true,
+				Sensitive:  true,
+				Deprecated: tokenDeprecationMessage,
 			},
 
 			"partition": {

diff --git a/consul/resource_consul_prepared_query.go b/consul/resource_consul_prepared_query.go
@@ -23,6 +23,7 @@ func resourceConsulPreparedQuery() *schema.Resource {
 			"datacenter": {
 				Type:     schema.TypeString,
 				Optional: true,
+				ForceNew: true,
 			},
 
 			"name": {
@@ -36,9 +37,10 @@ func resourceConsulPreparedQuery() *schema.Resource {
 			},
 
 			"token": {
-				Type:      schema.TypeString,
-				Optional:  true,
-				Sensitive: true,
+				Type:       schema.TypeString,
+				Optional:   true,
+				Sensitive:  true,
+				Deprecated: tokenDeprecationMessage,
 			},
 
 			"stored_token": {

diff --git a/consul/resource_provider.go b/consul/resource_provider.go
@@ -13,6 +13,11 @@ import (
 	"github.com/mitchellh/mapstructure"
 )
 
+var (
+	tokenDeprecationMessage = `The token argument has been deprecated and will be removed in a future release.
+Please use the token argument in the provider configuration`
+)
+
 func deprecated(name string, resource *schema.Resource) *schema.Resource {
 	resource.DeprecationMessage = fmt.Sprintf("%s is deprecated and will be removed in a future version.", name)
 	return resource

diff --git a/consul/resource_provider_test.go b/consul/resource_provider_test.go
@@ -136,6 +136,52 @@ func TestResourceProvider_ConfigureTLSInsecureHttpsMismatch(t *testing.T) {
 	}
 }
 
+// Some resources have some attributes that allows for overriding the configuration
+// of the provider like "token", "datacenter", "namespace" or "partition". This
+// causes some issues when reading resources after a user changed it as the new
+// value is not available to the provider when refreshing the state. This is a
+// limitation of the gRPC protocol between Terraform Core and the Plugins so
+// there is not much that we can do. To avoid having confusing behavior there
+// (see https://github.com/hashicorp/terraform-provider-consul/issues/298 for
+// example) we will deprecated the "token" attribute and mark the others as
+// ForceNew. This way we will not attempt to read a resource in the wrong
+// datacenter, partition or namespace. This test just makes sure that I did not
+// forget one of those parameters in a schema.
+func TestProviderAttributesInResources(t *testing.T) {
+	rp := Provider().(*schema.Provider)
+
+	forceNewAttributes := []string{"datacenter", "namespace", "partition"}
+
+	for name, resource := range rp.ResourcesMap {
+		attr, found := resource.Schema["token"]
+
+		if found && attr.Deprecated == "" {
+			t.Logf(`"token" attribute need to be marked as deprecated in resource %q`, name)
+			t.Fail()
+		}
+
+		for _, fna := range forceNewAttributes {
+			attr, found := resource.Schema[fna]
+			if !found {
+				continue
+			}
+			if !attr.ForceNew {
+				t.Logf(`%q attribute need to be marked as ForceNew in resource %q`, fna, name)
+				t.Fail()
+			}
+		}
+	}
+
+	for name, resource := range rp.DataSourcesMap {
+		attr, found := resource.Schema["token"]
+
+		if found && attr.Deprecated == "" {
+			t.Logf(`"token" attribute need to be marked as deprecated in datasource %q`, name)
+			t.Fail()
+		}
+	}
+}
+
 // token is sometime nested inside the object
 // func checkToken(name string, resource *configschema.Block) error {
 // 	for key, value := range resource.BlockTypes {