Skip to content

Commit b8445ee

Browse files
ansgarmansgarm
authored
error if there's no identity schema (or an empty one) defined when it should be (#1452)
* error if there's no identity schema (or an empty one) defined when it should be * add test for new diagnostics returned by GetResourceIdentitySchemas * add test for new diagnostics returned by ReadResource, PlanResourceChange, and ApplyResourceChange also adds identity test for ApplyResourceChange that didn't exist yet
1 parent fe25e82 commit b8445ee

File tree

3 files changed

+604
-21
lines changed

3 files changed

+604
-21
lines changed

‎helper/schema/core_schema.go‎

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -374,21 +374,26 @@ func (r *Resource) coreConfigSchema() *configschema.Block {
374374
return schemaMap(r.SchemaMap()).CoreConfigSchema()
375375
}
376376

377-
func (r *Resource) CoreIdentitySchema() *configschema.Block {
378-
block := r.coreIdentitySchema()
377+
func (r *Resource) CoreIdentitySchema() (*configschema.Block, error) {
378+
block, err := r.coreIdentitySchema()
379+
380+
if err != nil {
381+
return nil, err
382+
}
379383

380384
if block.Attributes == nil {
381-
// TODO: we should error instead and callers should handle the error appropriately
382-
// and error would hint at an invalid provider implementation
383-
block.Attributes = map[string]*configschema.Attribute{}
385+
return nil, fmt.Errorf("identity schema must have at least one attribute")
384386
}
385387

386-
return block
388+
return block, nil
387389
}
388390

389-
func (r *Resource) coreIdentitySchema() *configschema.Block {
391+
func (r *Resource) coreIdentitySchema() (*configschema.Block, error) {
392+
if r.Identity == nil || r.Identity.Schema == nil {
393+
return nil, fmt.Errorf("resource does not have an identity schema")
394+
}
390395
// while there is schemaMapWithIdentity, we don't need to use it here
391396
// as we're only interested in the existing CoreConfigSchema() method
392397
// to convert our schema
393-
return schemaMap(r.Identity.Schema).CoreConfigSchema()
398+
return schemaMap(r.Identity.Schema).CoreConfigSchema(), nil
394399
}

‎helper/schema/grpc_provider.go‎

Lines changed: 48 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -90,9 +90,16 @@ func (s *GRPCProviderServer) GetResourceIdentitySchemas(ctx context.Context, req
9090
logging.HelperSchemaTrace(ctx, "Found resource identity type", map[string]interface{}{logging.KeyResourceType: typ})
9191

9292
if res.Identity != nil {
93+
idschema, err := res.CoreIdentitySchema()
94+
95+
if err != nil {
96+
resp.Diagnostics = convert.AppendProtoDiag(ctx, resp.Diagnostics, fmt.Errorf("getting identity schema failed for resource '%s': %w", typ, err))
97+
return resp, nil
98+
}
99+
93100
resp.IdentitySchemas[typ] = &tfprotov5.ResourceIdentitySchema{
94101
Version: res.Identity.Version,
95-
IdentityAttributes: convert.ConfigIdentitySchemaToProto(ctx, res.CoreIdentitySchema()),
102+
IdentityAttributes: convert.ConfigIdentitySchemaToProto(ctx, idschema),
96103
}
97104
}
98105
}
@@ -110,12 +117,16 @@ func (s *GRPCProviderServer) UpgradeResourceIdentity(ctx context.Context, req *t
110117
resp.Diagnostics = convert.AppendProtoDiag(ctx, resp.Diagnostics, fmt.Errorf("unknown resource type: %s", req.TypeName))
111118
return resp, nil
112119
}
113-
schemaBlock := s.getResourceIdentitySchemaBlock(req.TypeName)
120+
121+
schemaBlock, err := s.getResourceIdentitySchemaBlock(req.TypeName)
122+
if err != nil {
123+
resp.Diagnostics = convert.AppendProtoDiag(ctx, resp.Diagnostics, err)
124+
return resp, nil
125+
}
114126

115127
version := req.Version
116128

117129
jsonMap := map[string]interface{}{}
118-
var err error
119130

120131
switch {
121132
// if there's a JSON state, we need to decode it.
@@ -258,7 +269,7 @@ func (s *GRPCProviderServer) getResourceSchemaBlock(name string) *configschema.B
258269
return res.CoreConfigSchema()
259270
}
260271

261-
func (s *GRPCProviderServer) getResourceIdentitySchemaBlock(name string) *configschema.Block {
272+
func (s *GRPCProviderServer) getResourceIdentitySchemaBlock(name string) (*configschema.Block, error) {
262273
res := s.provider.ResourcesMap[name]
263274
return res.CoreIdentitySchema()
264275
}
@@ -826,7 +837,12 @@ func (s *GRPCProviderServer) ReadResource(ctx context.Context, req *tfprotov5.Re
826837

827838
// convert req.CurrentIdentity to flat map identity structure
828839
// Step 1: Turn JSON into cty.Value based on schema
829-
identityBlock := s.getResourceIdentitySchemaBlock(req.TypeName) // TODO: handle error if no schema exists (and after we add the error)
840+
identityBlock, err := s.getResourceIdentitySchemaBlock(req.TypeName)
841+
if err != nil {
842+
resp.Diagnostics = convert.AppendProtoDiag(ctx, resp.Diagnostics, fmt.Errorf("getting identity schema failed for resource '%s': %w", req.TypeName, err))
843+
return resp, nil
844+
}
845+
830846
identityVal, err := msgpack.Unmarshal(req.CurrentIdentity.IdentityData.MsgPack, identityBlock.ImpliedType())
831847
if err != nil {
832848
resp.Diagnostics = convert.AppendProtoDiag(ctx, resp.Diagnostics, err)
@@ -901,7 +917,11 @@ func (s *GRPCProviderServer) ReadResource(ctx context.Context, req *tfprotov5.Re
901917
}
902918

903919
if newInstanceState.Identity != nil {
904-
identityBlock := s.getResourceIdentitySchemaBlock(req.TypeName) // TODO: handle error if no schema exists (and after we add the error)
920+
identityBlock, err := s.getResourceIdentitySchemaBlock(req.TypeName)
921+
if err != nil {
922+
resp.Diagnostics = convert.AppendProtoDiag(ctx, resp.Diagnostics, fmt.Errorf("getting identity schema failed for resource '%s': %w", req.TypeName, err))
923+
return resp, nil
924+
}
905925

906926
newIdentityVal, err := hcl2shim.HCL2ValueFromFlatmap(newInstanceState.Identity, identityBlock.ImpliedType())
907927
if err != nil {
@@ -1036,7 +1056,12 @@ func (s *GRPCProviderServer) PlanResourceChange(ctx context.Context, req *tfprot
10361056
if req.PriorIdentity != nil && req.PriorIdentity.IdentityData != nil {
10371057
// convert req.PriorIdentity to flat map identity structure
10381058
// Step 1: Turn JSON into cty.Value based on schema
1039-
identityBlock := s.getResourceIdentitySchemaBlock(req.TypeName) // TODO: handle error if no schema exists (and after we add the error)
1059+
identityBlock, err := s.getResourceIdentitySchemaBlock(req.TypeName)
1060+
if err != nil {
1061+
resp.Diagnostics = convert.AppendProtoDiag(ctx, resp.Diagnostics, fmt.Errorf("getting identity schema failed for resource '%s': %w", req.TypeName, err))
1062+
return resp, nil
1063+
}
1064+
10401065
identityVal, err := msgpack.Unmarshal(req.PriorIdentity.IdentityData.MsgPack, identityBlock.ImpliedType())
10411066
if err != nil {
10421067
resp.Diagnostics = convert.AppendProtoDiag(ctx, resp.Diagnostics, err)
@@ -1226,7 +1251,11 @@ func (s *GRPCProviderServer) PlanResourceChange(ctx context.Context, req *tfprot
12261251

12271252
// TODO: if schema defines identity, we should error if there's none written to newInstanceState
12281253
if res.Identity != nil {
1229-
identityBlock := s.getResourceIdentitySchemaBlock(req.TypeName) // TODO: handle error if no schema exists (and after we add the error)
1254+
identityBlock, err := s.getResourceIdentitySchemaBlock(req.TypeName)
1255+
if err != nil {
1256+
resp.Diagnostics = convert.AppendProtoDiag(ctx, resp.Diagnostics, fmt.Errorf("getting identity schema failed for resource '%s': %w", req.TypeName, err))
1257+
return resp, nil
1258+
}
12301259

12311260
newIdentityVal, err := hcl2shim.HCL2ValueFromFlatmap(diff.Identity, identityBlock.ImpliedType())
12321261
if err != nil {
@@ -1300,7 +1329,12 @@ func (s *GRPCProviderServer) ApplyResourceChange(ctx context.Context, req *tfpro
13001329
if req.PlannedIdentity != nil && req.PlannedIdentity.IdentityData != nil {
13011330
// convert req.PriorIdentity to flat map identity structure
13021331
// Step 1: Turn JSON into cty.Value based on schema
1303-
identityBlock := s.getResourceIdentitySchemaBlock(req.TypeName) // TODO: handle error if no schema exists (and after we add the error)
1332+
identityBlock, err := s.getResourceIdentitySchemaBlock(req.TypeName)
1333+
if err != nil {
1334+
resp.Diagnostics = convert.AppendProtoDiag(ctx, resp.Diagnostics, fmt.Errorf("getting identity schema failed for resource '%s': %w", req.TypeName, err))
1335+
return resp, nil
1336+
}
1337+
13041338
identityVal, err := msgpack.Unmarshal(req.PlannedIdentity.IdentityData.MsgPack, identityBlock.ImpliedType())
13051339
if err != nil {
13061340
resp.Diagnostics = convert.AppendProtoDiag(ctx, resp.Diagnostics, err)
@@ -1443,7 +1477,11 @@ func (s *GRPCProviderServer) ApplyResourceChange(ctx context.Context, req *tfpro
14431477

14441478
// TODO: if schema defines identity, we should error if there's none written to newInstanceState
14451479
if res.Identity != nil {
1446-
identityBlock := s.getResourceIdentitySchemaBlock(req.TypeName) // TODO: handle error if no schema exists (and after we add the error)
1480+
identityBlock, err := s.getResourceIdentitySchemaBlock(req.TypeName)
1481+
if err != nil {
1482+
resp.Diagnostics = convert.AppendProtoDiag(ctx, resp.Diagnostics, fmt.Errorf("getting identity schema failed for resource '%s': %w", req.TypeName, err))
1483+
return resp, nil
1484+
}
14471485

14481486
newIdentityVal, err := hcl2shim.HCL2ValueFromFlatmap(newInstanceState.Identity, identityBlock.ImpliedType())
14491487
if err != nil {

0 commit comments

Comments
 (0)