From e7ff4c40447d4be673fc46c056a259186bd31b4f Mon Sep 17 00:00:00 2001 From: Paul Tyng Date: Wed, 6 May 2020 15:33:12 -0400 Subject: [PATCH] Update diag convenience methods --- diag/diagnostic.go | 8 ----- diag/helpers.go | 36 +++++++++++++++++++++++ helper/schema/provider.go | 16 ++++------ helper/schema/provider_test.go | 2 +- helper/schema/resource.go | 54 +++++++++++++--------------------- 5 files changed, 63 insertions(+), 53 deletions(-) create mode 100644 diag/helpers.go diff --git a/diag/diagnostic.go b/diag/diagnostic.go index d6abbd1acef..3b1ced344f5 100644 --- a/diag/diagnostic.go +++ b/diag/diagnostic.go @@ -93,14 +93,6 @@ func (d Diagnostic) Validate() error { return nil } -// FromErr will convert an error into a Diagnostic -func FromErr(err error) Diagnostic { - return Diagnostic{ - Severity: Error, - Summary: err.Error(), - } -} - // Severity is an enum type marking the severity level of a Diagnostic type Severity int diff --git a/diag/helpers.go b/diag/helpers.go new file mode 100644 index 00000000000..8037b1301e5 --- /dev/null +++ b/diag/helpers.go @@ -0,0 +1,36 @@ +package diag + +import "fmt" + +// FromErr will convert an error into a Diagnostics. This returns Diagnostics +// as the most common use case in Go will be handling a single error +// returned from a function. +// +// if err != nil { +// return diag.FromErr(err) +// } +func FromErr(err error) Diagnostics { + return Diagnostics{ + Diagnostic{ + Severity: Error, + Summary: err.Error(), + }, + } +} + +// Errorf creates a Diagnostics with a single Error level Diagnostic entry. +// The summary is populated by performing a fmt.Sprintf with the supplied +// values. This returns a single error in a Diagnostics as errors typically +// do not occur in multiples as warnings may. +// +// if unexpectedCondition { +// return diag.Errorf("unexpected: %s", someValue) +// } +func Errorf(format string, a ...interface{}) Diagnostics { + return Diagnostics{ + Diagnostic{ + Severity: Error, + Summary: fmt.Sprintf(format, a...), + }, + } +} diff --git a/helper/schema/provider.go b/helper/schema/provider.go index 83d40aa26ee..642218276d8 100644 --- a/helper/schema/provider.go +++ b/helper/schema/provider.go @@ -243,12 +243,9 @@ func (p *Provider) ValidateResource( // // This won't be called at all if no provider configuration is given. func (p *Provider) Configure(ctx context.Context, c *terraform.ResourceConfig) diag.Diagnostics { - - var diags diag.Diagnostics - // No configuration if p.ConfigureFunc == nil && p.ConfigureContextFunc == nil { - return diags + return nil } sm := schemaMap(p.Schema) @@ -257,31 +254,30 @@ func (p *Provider) Configure(ctx context.Context, c *terraform.ResourceConfig) d // generate an intermediary "diff" although that is never exposed. diff, err := sm.Diff(ctx, nil, c, nil, p.meta, true) if err != nil { - return append(diags, diag.FromErr(err)) + return diag.FromErr(err) } data, err := sm.Data(nil, diff) if err != nil { - return append(diags, diag.FromErr(err)) + return diag.FromErr(err) } if p.ConfigureFunc != nil { meta, err := p.ConfigureFunc(data) if err != nil { - return append(diags, diag.FromErr(err)) + return diag.FromErr(err) } p.meta = meta } if p.ConfigureContextFunc != nil { - meta, ds := p.ConfigureContextFunc(ctx, data) - diags = append(diags, ds...) + meta, diags := p.ConfigureContextFunc(ctx, data) if diags.HasError() { return diags } p.meta = meta } - return diags + return nil } // Resources returns all the available resource types that this provider diff --git a/helper/schema/provider_test.go b/helper/schema/provider_test.go index a5a67480473..0811df835bc 100644 --- a/helper/schema/provider_test.go +++ b/helper/schema/provider_test.go @@ -144,7 +144,7 @@ func TestProviderConfigure(t *testing.T) { return nil, nil } - return nil, diag.Diagnostics{diag.FromErr(fmt.Errorf("nope"))} + return nil, diag.Errorf("nope") }, }, Config: map[string]interface{}{ diff --git a/helper/schema/resource.go b/helper/schema/resource.go index dfeaba3a360..3e03f00985d 100644 --- a/helper/schema/resource.go +++ b/helper/schema/resource.go @@ -266,11 +266,10 @@ type CustomizeDiffFunc func(context.Context, *ResourceDiff, interface{}) error func (r *Resource) create(ctx context.Context, d *ResourceData, meta interface{}) diag.Diagnostics { if r.Create != nil { - var diags diag.Diagnostics if err := r.Create(d, meta); err != nil { - diags = append(diags, diag.FromErr(err)) + return diag.FromErr(err) } - return diags + return nil } ctx, cancel := context.WithTimeout(ctx, d.Timeout(TimeoutCreate)) defer cancel() @@ -279,11 +278,10 @@ func (r *Resource) create(ctx context.Context, d *ResourceData, meta interface{} func (r *Resource) read(ctx context.Context, d *ResourceData, meta interface{}) diag.Diagnostics { if r.Read != nil { - var diags diag.Diagnostics if err := r.Read(d, meta); err != nil { - diags = append(diags, diag.FromErr(err)) + return diag.FromErr(err) } - return diags + return nil } ctx, cancel := context.WithTimeout(ctx, d.Timeout(TimeoutRead)) defer cancel() @@ -292,11 +290,10 @@ func (r *Resource) read(ctx context.Context, d *ResourceData, meta interface{}) func (r *Resource) update(ctx context.Context, d *ResourceData, meta interface{}) diag.Diagnostics { if r.Update != nil { - var diags diag.Diagnostics if err := r.Update(d, meta); err != nil { - diags = append(diags, diag.FromErr(err)) + return diag.FromErr(err) } - return diags + return nil } ctx, cancel := context.WithTimeout(ctx, d.Timeout(TimeoutUpdate)) defer cancel() @@ -305,11 +302,10 @@ func (r *Resource) update(ctx context.Context, d *ResourceData, meta interface{} func (r *Resource) delete(ctx context.Context, d *ResourceData, meta interface{}) diag.Diagnostics { if r.Delete != nil { - var diags diag.Diagnostics if err := r.Delete(d, meta); err != nil { - diags = append(diags, diag.FromErr(err)) + return diag.FromErr(err) } - return diags + return nil } ctx, cancel := context.WithTimeout(ctx, d.Timeout(TimeoutDelete)) defer cancel() @@ -322,12 +318,9 @@ func (r *Resource) Apply( s *terraform.InstanceState, d *terraform.InstanceDiff, meta interface{}) (*terraform.InstanceState, diag.Diagnostics) { - - var diags diag.Diagnostics - data, err := schemaMap(r.Schema).Data(s, d) if err != nil { - return s, append(diags, diag.FromErr(err)) + return s, diag.FromErr(err) } if s != nil && data != nil { @@ -358,11 +351,12 @@ func (r *Resource) Apply( s = new(terraform.InstanceState) } + var diags diag.Diagnostics + if d.Destroy || d.RequiresNew() { if s.ID != "" { // Destroy the resource since it is created diags = append(diags, r.delete(ctx, data, meta)...) - if diags.HasError() { return r.recordCurrentSchemaVersion(data.State()), diags } @@ -380,7 +374,7 @@ func (r *Resource) Apply( // Reset the data to be stateless since we just destroyed data, err = schemaMap(r.Schema).Data(nil, d) if err != nil { - return nil, append(diags, diag.FromErr(err)) + return nil, append(diags, diag.FromErr(err)...) } // data was reset, need to re-apply the parsed timeouts @@ -485,18 +479,14 @@ func (r *Resource) ReadDataApply( d *terraform.InstanceDiff, meta interface{}, ) (*terraform.InstanceState, diag.Diagnostics) { - - var diags diag.Diagnostics - // Data sources are always built completely from scratch // on each read, so the source state is always nil. data, err := schemaMap(r.Schema).Data(nil, d) if err != nil { - return nil, append(diags, diag.FromErr(err)) + return nil, diag.FromErr(err) } - diags = append(diags, r.read(ctx, data, meta)...) - + diags := r.read(ctx, data, meta) state := data.State() if state != nil && state.ID == "" { // Data sources can set an ID if they want, but they aren't @@ -517,12 +507,9 @@ func (r *Resource) RefreshWithoutUpgrade( ctx context.Context, s *terraform.InstanceState, meta interface{}) (*terraform.InstanceState, diag.Diagnostics) { - - var diags diag.Diagnostics - // If the ID is already somehow blank, it doesn't exist if s.ID == "" { - return nil, diags + return nil, nil } rt := ResourceTimeout{} @@ -537,7 +524,7 @@ func (r *Resource) RefreshWithoutUpgrade( // affect our Read later. data, err := schemaMap(r.Schema).Data(s, nil) if err != nil { - return s, append(diags, diag.FromErr(err)) + return s, diag.FromErr(err) } data.timeouts = &rt @@ -547,17 +534,17 @@ func (r *Resource) RefreshWithoutUpgrade( exists, err := r.Exists(data, meta) if err != nil { - return s, append(diags, diag.FromErr(err)) + return s, diag.FromErr(err) } if !exists { - return nil, diags + return nil, nil } } data, err := schemaMap(r.Schema).Data(s, nil) if err != nil { - return s, append(diags, diag.FromErr(err)) + return s, diag.FromErr(err) } data.timeouts = &rt @@ -565,8 +552,7 @@ func (r *Resource) RefreshWithoutUpgrade( data.providerMeta = s.ProviderMeta } - diags = append(diags, r.read(ctx, data, meta)...) - + diags := r.read(ctx, data, meta) state := data.State() if state != nil && state.ID == "" { state = nil