Merge pull request #33807 from hashicorp/jbardin/cached-schema

move `GetProviderSchemaOptional` check to correct block
hashicorp · Sep 6, 2023 · a6789a7 · a6789a7
2 parents 35baa30 + f971348
commit a6789a7
Show file tree

Hide file tree

Showing 4 changed files with 62 additions and 6 deletions.
diff --git a/internal/plugin/grpc_provider.go b/internal/plugin/grpc_provider.go
@@ -73,14 +73,15 @@ type GRPCProvider struct {
 	schema providers.GetProviderSchemaResponse
 }
 
-func (p *GRPCProvider) GetProviderSchema() (resp providers.GetProviderSchemaResponse) {
+func (p *GRPCProvider) GetProviderSchema() providers.GetProviderSchemaResponse {
 	logger.Trace("GRPCProvider: GetProviderSchema")
 	p.mu.Lock()
 	defer p.mu.Unlock()
 
 	// check the global cache if we can
-	if !p.Addr.IsZero() && resp.ServerCapabilities.GetProviderSchemaOptional {
-		if resp, ok := providers.SchemaCache.Get(p.Addr); ok {
+	if !p.Addr.IsZero() {
+		if resp, ok := providers.SchemaCache.Get(p.Addr); ok && resp.ServerCapabilities.GetProviderSchemaOptional {
+			logger.Trace("GRPCProvider: returning cached schema", p.Addr.String())
 			return resp
 		}
 	}
@@ -91,6 +92,8 @@ func (p *GRPCProvider) GetProviderSchema() (resp providers.GetProviderSchemaResp
 		return p.schema
 	}
 
+	var resp providers.GetProviderSchemaResponse
+
 	resp.ResourceTypes = make(map[string]providers.Schema)
 	resp.DataSources = make(map[string]providers.Schema)
 

diff --git a/internal/plugin/grpc_provider_test.go b/internal/plugin/grpc_provider_test.go
@@ -10,6 +10,7 @@ import (
 
 	"github.com/golang/mock/gomock"
 	"github.com/google/go-cmp/cmp"
+	"github.com/hashicorp/terraform/internal/addrs"
 	"github.com/hashicorp/terraform/internal/configs/hcl2shim"
 	"github.com/hashicorp/terraform/internal/providers"
 	"github.com/hashicorp/terraform/internal/tfdiags"
@@ -92,6 +93,9 @@ func providerProtoSchema() *proto.GetProviderSchema_Response {
 				},
 			},
 		},
+		ServerCapabilities: &proto.ServerCapabilities{
+			GetProviderSchemaOptional: true,
+		},
 	}
 }
 
@@ -104,6 +108,27 @@ func TestGRPCProvider_GetSchema(t *testing.T) {
 	checkDiags(t, resp.Diagnostics)
 }
 
+// ensure that the global schema cache is used when the provider supports
+// GetProviderSchemaOptional
+func TestGRPCProvider_GetSchema_globalCache(t *testing.T) {
+	p := &GRPCProvider{
+		Addr:   addrs.ImpliedProviderForUnqualifiedType("test"),
+		client: mockProviderClient(t),
+	}
+
+	// first call primes the cache
+	resp := p.GetProviderSchema()
+
+	// create a new provider instance which does not expect a GetProviderSchemaCall
+	p = &GRPCProvider{
+		Addr:   addrs.ImpliedProviderForUnqualifiedType("test"),
+		client: mockproto.NewMockProviderClient(gomock.NewController(t)),
+	}
+
+	resp = p.GetProviderSchema()
+	checkDiags(t, resp.Diagnostics)
+}
+
 // Ensure that gRPC errors are returned early.
 // Reference: https://github.com/hashicorp/terraform/issues/31047
 func TestGRPCProvider_GetSchema_GRPCError(t *testing.T) {

diff --git a/internal/plugin6/grpc_provider.go b/internal/plugin6/grpc_provider.go
@@ -73,14 +73,15 @@ type GRPCProvider struct {
 	schema providers.GetProviderSchemaResponse
 }
 
-func (p *GRPCProvider) GetProviderSchema() (resp providers.GetProviderSchemaResponse) {
+func (p *GRPCProvider) GetProviderSchema() providers.GetProviderSchemaResponse {
 	logger.Trace("GRPCProvider.v6: GetProviderSchema")
 	p.mu.Lock()
 	defer p.mu.Unlock()
 
 	// check the global cache if we can
-	if !p.Addr.IsZero() && resp.ServerCapabilities.GetProviderSchemaOptional {
-		if resp, ok := providers.SchemaCache.Get(p.Addr); ok {
+	if !p.Addr.IsZero() {
+		if resp, ok := providers.SchemaCache.Get(p.Addr); ok && resp.ServerCapabilities.GetProviderSchemaOptional {
+			logger.Trace("GRPCProvider.v6: returning cached schema", p.Addr.String())
 			return resp
 		}
 	}
@@ -91,6 +92,8 @@ func (p *GRPCProvider) GetProviderSchema() (resp providers.GetProviderSchemaResp
 		return p.schema
 	}
 
+	var resp providers.GetProviderSchemaResponse
+
 	resp.ResourceTypes = make(map[string]providers.Schema)
 	resp.DataSources = make(map[string]providers.Schema)
 

diff --git a/internal/plugin6/grpc_provider_test.go b/internal/plugin6/grpc_provider_test.go
@@ -11,6 +11,7 @@ import (
 	"github.com/golang/mock/gomock"
 	"github.com/google/go-cmp/cmp"
 	"github.com/google/go-cmp/cmp/cmpopts"
+	"github.com/hashicorp/terraform/internal/addrs"
 	"github.com/hashicorp/terraform/internal/configs/hcl2shim"
 	"github.com/hashicorp/terraform/internal/providers"
 	"github.com/hashicorp/terraform/internal/tfdiags"
@@ -99,6 +100,9 @@ func providerProtoSchema() *proto.GetProviderSchema_Response {
 				},
 			},
 		},
+		ServerCapabilities: &proto.ServerCapabilities{
+			GetProviderSchemaOptional: true,
+		},
 	}
 }
 
@@ -111,6 +115,27 @@ func TestGRPCProvider_GetSchema(t *testing.T) {
 	checkDiags(t, resp.Diagnostics)
 }
 
+// ensure that the global schema cache is used when the provider supports
+// GetProviderSchemaOptional
+func TestGRPCProvider_GetSchema_globalCache(t *testing.T) {
+	p := &GRPCProvider{
+		Addr:   addrs.ImpliedProviderForUnqualifiedType("test"),
+		client: mockProviderClient(t),
+	}
+
+	// first call primes the cache
+	resp := p.GetProviderSchema()
+
+	// create a new provider instance which does not expect a GetProviderSchemaCall
+	p = &GRPCProvider{
+		Addr:   addrs.ImpliedProviderForUnqualifiedType("test"),
+		client: mockproto.NewMockProviderClient(gomock.NewController(t)),
+	}
+
+	resp = p.GetProviderSchema()
+	checkDiags(t, resp.Diagnostics)
+}
+
 // Ensure that gRPC errors are returned early.
 // Reference: https://github.com/hashicorp/terraform/issues/31047
 func TestGRPCProvider_GetSchema_GRPCError(t *testing.T) {