From 6b3a3480d8e7014fc77b1482c8f9238c63a87b16 Mon Sep 17 00:00:00 2001 From: Sean Marciniak <30928402+MovieStoreGuy@users.noreply.github.com> Date: Fri, 11 Oct 2024 09:43:06 +1030 Subject: [PATCH 1/2] Feature: Adding helper functions around errors and logging (#516) * Improve support for multi error and convience logging * Fixing lint issues * Fixing typo --- internal/tfextension/diag.go | 35 +++++++++++++----- internal/tfextension/diag_test.go | 20 +++++++++++ internal/tfextension/logging.go | 42 ++++++++++++++++++++++ internal/tfextension/logging_test.go | 54 ++++++++++++++++++++++++++++ 4 files changed, 143 insertions(+), 8 deletions(-) create mode 100644 internal/tfextension/logging.go create mode 100644 internal/tfextension/logging_test.go diff --git a/internal/tfextension/diag.go b/internal/tfextension/diag.go index ab81069b..74288f12 100644 --- a/internal/tfextension/diag.go +++ b/internal/tfextension/diag.go @@ -8,6 +8,7 @@ import ( "github.com/hashicorp/go-cty/cty" "github.com/hashicorp/terraform-plugin-sdk/v2/diag" + "go.uber.org/multierr" ) // AppendDiagnostics is to be used similar to combining errors together @@ -18,21 +19,39 @@ func AppendDiagnostics(diags diag.Diagnostics, values ...diag.Diagnostic) diag.D // AsErrorDiagnostics is the same as `diag.FromErr`, however, it allow allows // adding the attribute values that are provided in CRUD operations. -func AsErrorDiagnostics(err error, path ...cty.Path) diag.Diagnostics { - return newDiagnostics(diag.Error, err, path...) +func AsErrorDiagnostics(err error, path ...cty.Path) (issues diag.Diagnostics) { + return newUnwrapErrors(diag.Error, err, path...) } // AsWarnDiagnostics is the same as `diag.FromErr`, however, it sets the severity as Warning // and allows for appending the attribute path as part of the values. -func AsWarnDiagnostics(err error, path ...cty.Path) diag.Diagnostics { - return newDiagnostics(diag.Warning, err, path...) +func AsWarnDiagnostics(err error, path ...cty.Path) (issues diag.Diagnostics) { + return newUnwrapErrors(diag.Warning, err, path...) } -func newDiagnostics(sev diag.Severity, summary error, path ...cty.Path) diag.Diagnostics { - if summary == nil { +func newUnwrapErrors(sev diag.Severity, err error, path ...cty.Path) (issues diag.Diagnostics) { + if err == nil { return nil } - return diag.Diagnostics{ - {Severity: sev, Summary: summary.Error(), AttributePath: slices.Concat(path...)}, + + // Checking to see if there is any joined errors + // so it can be unpacked into separate reported issues. + // This useses the unpublished errors' [interface{ Unwrap() []error }] + // and if that is unset it then checks the uber's implementation. + var errs []error + if v, ok := err.(interface{ Unwrap() []error }); ok { + errs = v.Unwrap() + } + + if len(errs) == 0 { + errs = multierr.Errors(err) } + + for _, err := range errs { + issues = AppendDiagnostics(issues, diag.Diagnostic{ + Severity: sev, Summary: err.Error(), AttributePath: slices.Concat(path...), + }) + } + + return issues } diff --git a/internal/tfextension/diag_test.go b/internal/tfextension/diag_test.go index 5e996792..427031da 100644 --- a/internal/tfextension/diag_test.go +++ b/internal/tfextension/diag_test.go @@ -101,6 +101,16 @@ func TestAsErrorDiagnostics(t *testing.T) { {Severity: diag.Error, Summary: "bad entry", AttributePath: cty.IndexStringPath("attr")}, }, }, + { + name: "multiple errors reported", + value: AsErrorDiagnostics( + errors.Join(errors.New("failed to validate entry #1"), errors.New("failed to validate entry #2")), + ), + expect: diag.Diagnostics{ + {Severity: diag.Error, Summary: "failed to validate entry #1"}, + {Severity: diag.Error, Summary: "failed to validate entry #2"}, + }, + }, } { t.Run(tc.name, func(t *testing.T) { assert.Equal(t, tc.expect, tc.value) @@ -135,6 +145,16 @@ func TestAsWarnDiagnostics(t *testing.T) { {Severity: diag.Warning, Summary: "bad entry", AttributePath: cty.IndexStringPath("attr")}, }, }, + { + name: "multiple errors reported", + value: AsWarnDiagnostics( + errors.Join(errors.New("failed to validate entry #1"), errors.New("failed to validate entry #2")), + ), + expect: diag.Diagnostics{ + {Severity: diag.Warning, Summary: "failed to validate entry #1"}, + {Severity: diag.Warning, Summary: "failed to validate entry #2"}, + }, + }, } { t.Run(tc.name, func(t *testing.T) { assert.Equal(t, tc.expect, tc.value) diff --git a/internal/tfextension/logging.go b/internal/tfextension/logging.go new file mode 100644 index 00000000..705aa2cc --- /dev/null +++ b/internal/tfextension/logging.go @@ -0,0 +1,42 @@ +// Copyright Splunk, Inc. +// SPDX-License-Identifier: MPL-2.0 + +package tfext + +import "time" + +// LogFields is an extension to logging fields parameter +// to help as a convience and provides some level of standards. +type LogFields map[string]any + +// NewLogFields creates an empty [LogFields] that be used +// as part of the `tflog` fields parameter. +func NewLogFields() LogFields { + return make(LogFields) +} + +// ErrorLogFields is a convience function that allows +// for a [LogFields] to be created with the error entry set. +func ErrorLogFields(err error) LogFields { + return NewLogFields().Error(err) +} + +// Error appends the field name `error` if the error value is not nil +func (lf LogFields) Error(err error) LogFields { + if err != nil { + lf["error"] = err.Error() + } + return lf +} + +// Duration is a convience function to ensure the key is correctly set. +func (lf LogFields) Duration(key string, val time.Duration) LogFields { + return lf.Field(key, val.String()) +} + +// Field appends any type to be set as the key's value. +// if the field already exists, it is overwritten. +func (lf LogFields) Field(key string, val any) LogFields { + lf[key] = val + return lf +} diff --git a/internal/tfextension/logging_test.go b/internal/tfextension/logging_test.go new file mode 100644 index 00000000..fb959b3a --- /dev/null +++ b/internal/tfextension/logging_test.go @@ -0,0 +1,54 @@ +// Copyright Splunk, Inc. +// SPDX-License-Identifier: MPL-2.0 + +package tfext + +import ( + "errors" + "testing" + "time" + + "github.com/stretchr/testify/assert" +) + +func TestLogFields(t *testing.T) { + t.Parallel() + + for _, tc := range []struct { + name string + lf LogFields + expect map[string]any + }{ + { + name: "empty", + lf: NewLogFields(), + expect: map[string]any{}, + }, + { + name: "contains error message", + lf: ErrorLogFields(errors.New("failed operation")), + expect: map[string]any{"error": "failed operation"}, + }, + { + name: "no error message", + lf: ErrorLogFields(nil), + expect: map[string]any{}, + }, + { + name: "duration set", + lf: NewLogFields().Duration("min-delay", 1*time.Minute+30*time.Second), + expect: map[string]any{"min-delay": "1m30s"}, + }, + { + name: "custom field", + lf: NewLogFields().Field("example", 1), + expect: map[string]any{"example": 1}, + }, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + assert.EqualValues(t, tc.expect, tc.lf, "Must match the expected field") + }) + } +} From 9f996c1f4e530fe0d5203460ac9ce2ee8415caf3 Mon Sep 17 00:00:00 2001 From: Sean Marciniak <30928402+MovieStoreGuy@users.noreply.github.com> Date: Fri, 11 Oct 2024 09:43:30 +1030 Subject: [PATCH 2/2] Feature: Adding Provider defintion (#517) * Improve support for multi error and convience logging * Fixing lint issues * Fixing typo * Adding support for new provider --- internal/definition/provider/provider.go | 145 ++++++++++++++++++ internal/definition/provider/provider_test.go | 74 +++++++++ 2 files changed, 219 insertions(+) create mode 100644 internal/definition/provider/provider.go create mode 100644 internal/definition/provider/provider_test.go diff --git a/internal/definition/provider/provider.go b/internal/definition/provider/provider.go new file mode 100644 index 00000000..121bf4f7 --- /dev/null +++ b/internal/definition/provider/provider.go @@ -0,0 +1,145 @@ +// Copyright Splunk, Inc. +// SPDX-License-Identifier: MPL-2.0 + +package provider + +import ( + "context" + "fmt" + "net" + "net/http" + "time" + + "github.com/hashicorp/go-retryablehttp" + "github.com/hashicorp/terraform-plugin-log/tflog" + "github.com/hashicorp/terraform-plugin-sdk/v2/diag" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/logging" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" + "github.com/signalfx/signalfx-go" + + "github.com/splunk-terraform/terraform-provider-signalfx/internal/definition/team" + pmeta "github.com/splunk-terraform/terraform-provider-signalfx/internal/providermeta" + tfext "github.com/splunk-terraform/terraform-provider-signalfx/internal/tfextension" + "github.com/splunk-terraform/terraform-provider-signalfx/version" +) + +func New() *schema.Provider { + return &schema.Provider{ + Schema: map[string]*schema.Schema{ + "auth_token": { + Type: schema.TypeString, + Optional: true, + DefaultFunc: schema.EnvDefaultFunc("SFX_AUTH_TOKEN", ""), + Description: "Splunk Observability Cloud auth token", + }, + "api_url": { + Type: schema.TypeString, + Optional: true, + DefaultFunc: schema.EnvDefaultFunc("SFX_API_URL", "https://api.signalfx.com"), + Description: "API URL for your Splunk Observability Cloud org, may include a realm", + }, + "custom_app_url": { + Type: schema.TypeString, + Optional: true, + DefaultFunc: schema.EnvDefaultFunc("SFX_CUSTOM_APP_URL", "https://app.signalfx.com"), + Description: "Application URL for your Splunk Observability Cloud org, often customized for organizations using SSO", + }, + "timeout_seconds": { + Type: schema.TypeInt, + Optional: true, + Default: 120, + Description: "Timeout duration for a single HTTP call in seconds. Defaults to 120", + }, + "retry_max_attempts": { + Type: schema.TypeInt, + Optional: true, + Default: 4, + Description: "Max retries for a single HTTP call. Defaults to 4", + }, + "retry_wait_min_seconds": { + Type: schema.TypeInt, + Optional: true, + Default: 1, + Description: "Minimum retry wait for a single HTTP call in seconds. Defaults to 1", + }, + "retry_wait_max_seconds": { + Type: schema.TypeInt, + Optional: true, + Default: 30, + Description: "Maximum retry wait for a single HTTP call in seconds. Defaults to 30", + }, + }, + ResourcesMap: map[string]*schema.Resource{ + "signalfx_team": team.NewResource(), + }, + DataSourcesMap: map[string]*schema.Resource{}, + ConfigureContextFunc: configureProvider, + } +} + +func configureProvider(ctx context.Context, data *schema.ResourceData) (any, diag.Diagnostics) { + var meta pmeta.Meta + for _, lookup := range pmeta.NewDefaultProviderLookups() { + if err := lookup.Do(ctx, &meta); err != nil { + tflog.Debug( + ctx, + "Issue trying to load external provider configuration, skipping", + tfext.ErrorLogFields(err), + ) + } + } + + if token, ok := data.GetOk("auth_token"); ok { + meta.AuthToken = token.(string) + } + if url, ok := data.GetOk("api_url"); ok { + meta.APIURL = url.(string) + } + if url, ok := data.GetOk("custom_app_url"); ok { + meta.CustomAppURL = url.(string) + } + + err := meta.Validate() + if err != nil { + return nil, tfext.AsErrorDiagnostics(err) + } + + var ( + attempts = data.Get("retry_max_attempts").(int) + timeout = time.Duration(int64(data.Get("timeout_seconds").(int))) * time.Second + waitmin = time.Duration(int64(data.Get("retry_wait_min_seconds").(int))) * time.Second + waitmax = time.Duration(int64((data.Get("retry_wait_max_seconds").(int)))) * time.Second + ) + + rc := retryablehttp.NewClient() + rc.RetryMax = attempts + rc.RetryWaitMin = waitmin + rc.RetryWaitMax = waitmax + rc.HTTPClient.Timeout = timeout + rc.HTTPClient.Transport = logging.NewSubsystemLoggingHTTPTransport("signalfx", &http.Transport{ + Proxy: http.ProxyFromEnvironment, + DialContext: (&net.Dialer{Timeout: 5 * time.Second}).DialContext, + TLSHandshakeTimeout: 5 * time.Second, + MaxIdleConns: 100, + MaxIdleConnsPerHost: 100, + }) + + meta.Client, err = signalfx.NewClient(meta.AuthToken, + signalfx.APIUrl(meta.APIURL), + signalfx.HTTPClient(rc.StandardClient()), + signalfx.UserAgent(fmt.Sprintf("Terraform terraform-provider-signalfx/%s", version.ProviderVersion)), + ) + + if err != nil { + return nil, tfext.AsErrorDiagnostics(err) + } + + tflog.Debug(ctx, "Configured settings for http client", tfext.NewLogFields(). + Field("attempts", attempts). + Duration("timeout", timeout). + Duration("wait_min", waitmin). + Duration("wait_max", waitmax), + ) + + return &meta, nil +} diff --git a/internal/definition/provider/provider_test.go b/internal/definition/provider/provider_test.go new file mode 100644 index 00000000..9cb6ef2f --- /dev/null +++ b/internal/definition/provider/provider_test.go @@ -0,0 +1,74 @@ +// Copyright Splunk, Inc. +// SPDX-License-Identifier: MPL-2.0 + +package provider + +import ( + "context" + "testing" + + "github.com/hashicorp/terraform-plugin-sdk/v2/diag" + "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" + "github.com/stretchr/testify/assert" +) + +func TestProviderValidation(t *testing.T) { + t.Parallel() + + assert.NoError(t, New().InternalValidate(), "Must not error loading provider") +} + +func TestProviderHasResource(t *testing.T) { + t.Parallel() + + p := New() + + expected := []string{ + "signalfx_team", + } + + for name := range p.ResourcesMap { + assert.Contains(t, expected, name, "Must have the resource defined as part of provider") + } + + for _, name := range expected { + assert.Contains(t, p.ResourcesMap, name, "Must have the expected resource defined in provider") + } +} + +func TestProviderConfiguration(t *testing.T) { + t.Parallel() + + for _, tc := range []struct { + name string + details map[string]any + expect diag.Diagnostics + }{ + { + name: "no details provided", + details: make(map[string]any), + expect: diag.Diagnostics{ + {Severity: diag.Error, Summary: "auth token not set"}, + }, + }, + { + name: "setting min required fields", + details: map[string]any{ + "auth_token": "hunter2", + "api_url": "api.us.signalfx.com", + }, + expect: nil, + }, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + actual := New().Configure( + context.Background(), + terraform.NewResourceConfigRaw(tc.details), + ) + + assert.Equal(t, tc.expect, actual, "Must match the expected details") + }) + } +}