From d840ce3447104fccc024566714baf110642582d4 Mon Sep 17 00:00:00 2001 From: Chris Marget Date: Wed, 17 Jul 2024 08:26:54 -0400 Subject: [PATCH 1/5] consolodate APSTRA_ environment variables into `utils` package --- apstra/provider.go | 28 +++++++++++----------------- apstra/utils/client.go | 16 ++++++++++------ 2 files changed, 21 insertions(+), 23 deletions(-) diff --git a/apstra/provider.go b/apstra/provider.go index e40de2bb..027e296b 100644 --- a/apstra/provider.go +++ b/apstra/provider.go @@ -31,12 +31,6 @@ const ( defaultTag = "v0.0.0" defaultCommit = "devel" - envApiTimeout = "APSTRA_API_TIMEOUT" - envBlueprintMutexEnabled = "APSTRA_BLUEPRINT_MUTEX_ENABLED" - envBlueprintMutexMessage = "APSTRA_BLUEPRINT_MUTEX_MESSAGE" - envExperimental = "APSTRA_EXPERIMENTAL" - envTlsNoVerify = "APSTRA_TLS_VALIDATION_DISABLED" - blueprintMutexMessage = "locked by terraform at $DATE" osxCertErrStringMatch = "certificate is not trusted" @@ -189,45 +183,45 @@ type providerConfig struct { } func (o *providerConfig) fromEnv(_ context.Context, diags *diag.Diagnostics) { - if s, ok := os.LookupEnv(envTlsNoVerify); ok && o.TlsNoVerify.IsNull() { + if s, ok := os.LookupEnv(utils.EnvTlsNoVerify); ok && o.TlsNoVerify.IsNull() { v, err := strconv.ParseBool(s) if err != nil { - diags.AddError(fmt.Sprintf("error parsing environment variable %q", envTlsNoVerify), err.Error()) + diags.AddError(fmt.Sprintf("error parsing environment variable %q", utils.EnvTlsNoVerify), err.Error()) } o.TlsNoVerify = types.BoolValue(v) } - if s, ok := os.LookupEnv(envBlueprintMutexEnabled); ok && o.MutexEnable.IsNull() { + if s, ok := os.LookupEnv(utils.EnvBlueprintMutexEnabled); ok && o.MutexEnable.IsNull() { v, err := strconv.ParseBool(s) if err != nil { - diags.AddError(fmt.Sprintf("error parsing environment variable %q", envBlueprintMutexEnabled), err.Error()) + diags.AddError(fmt.Sprintf("error parsing environment variable %q", utils.EnvBlueprintMutexEnabled), err.Error()) } o.MutexEnable = types.BoolValue(v) } - if s, ok := os.LookupEnv(envBlueprintMutexMessage); ok && o.MutexMessage.IsNull() { + if s, ok := os.LookupEnv(utils.EnvBlueprintMutexMessage); ok && o.MutexMessage.IsNull() { if len(s) < 1 { - diags.AddError(fmt.Sprintf("error parsing environment variable %q", envBlueprintMutexMessage), + diags.AddError(fmt.Sprintf("error parsing environment variable %q", utils.EnvBlueprintMutexMessage), fmt.Sprintf("minimum string length 1; got %q", s)) } o.MutexMessage = types.StringValue(s) } - if s, ok := os.LookupEnv(envExperimental); ok && o.Experimental.IsNull() { + if s, ok := os.LookupEnv(utils.EnvApstraExperimental); ok && o.Experimental.IsNull() { v, err := strconv.ParseBool(s) if err != nil { - diags.AddError(fmt.Sprintf("error parsing environment variable %q", envExperimental), err.Error()) + diags.AddError(fmt.Sprintf("error parsing environment variable %q", utils.EnvApstraExperimental), err.Error()) } o.Experimental = types.BoolValue(v) } - if s, ok := os.LookupEnv(envApiTimeout); ok && o.ApiTimeout.IsNull() { + if s, ok := os.LookupEnv(utils.EnvApiTimeout); ok && o.ApiTimeout.IsNull() { v, err := strconv.ParseInt(s, 0, 64) if err != nil { - diags.AddError(fmt.Sprintf("error parsing environment variable %q", envApiTimeout), err.Error()) + diags.AddError(fmt.Sprintf("error parsing environment variable %q", utils.EnvApiTimeout), err.Error()) } if v < 0 { - diags.AddError(fmt.Sprintf("invalid value in environment variable %q", envApiTimeout), + diags.AddError(fmt.Sprintf("invalid value in environment variable %q", utils.EnvApiTimeout), fmt.Sprintf("minimum permitted value is 0, got %d", v)) } o.ApiTimeout = types.Int64Value(v) diff --git a/apstra/utils/client.go b/apstra/utils/client.go index ab1b664b..9be00ac3 100644 --- a/apstra/utils/client.go +++ b/apstra/utils/client.go @@ -14,12 +14,16 @@ import ( ) const ( - EnvApstraUrl = "APSTRA_URL" - EnvApstraUsername = "APSTRA_USER" - EnvApstraPassword = "APSTRA_PASS" - EnvApstraLogfile = "APSTRA_LOG" - EnvApstraExperimental = "APSTRA_EXPERIMENTAL" - EnvTlsKeyLogFile = "SSLKEYLOGFILE" + EnvApiTimeout = "APSTRA_API_TIMEOUT" + EnvApstraUrl = "APSTRA_URL" + EnvBlueprintMutexEnabled = "APSTRA_BLUEPRINT_MUTEX_ENABLED" + EnvBlueprintMutexMessage = "APSTRA_BLUEPRINT_MUTEX_MESSAGE" + EnvTlsNoVerify = "APSTRA_TLS_VALIDATION_DISABLED" + EnvApstraUsername = "APSTRA_USER" + EnvApstraPassword = "APSTRA_PASS" + EnvApstraLogfile = "APSTRA_LOG" + EnvApstraExperimental = "APSTRA_EXPERIMENTAL" + EnvTlsKeyLogFile = "SSLKEYLOGFILE" urlEncodeMsg = ` Note that when the Username or Password fields contain special characters and are From 468716f7405c49448573fe9a404d9cc26a82b8ba Mon Sep 17 00:00:00 2001 From: Chris Marget Date: Wed, 17 Jul 2024 08:54:13 -0400 Subject: [PATCH 2/5] add prefix to env var lookups --- apstra/provider.go | 35 ++++++++++++++------- apstra/resource_template_rack_based_test.go | 2 +- apstra/test_utils/test_utils.go | 2 +- apstra/utils/client.go | 12 +++---- 4 files changed, 31 insertions(+), 20 deletions(-) diff --git a/apstra/provider.go b/apstra/provider.go index 027e296b..10fd8c0e 100644 --- a/apstra/provider.go +++ b/apstra/provider.go @@ -168,6 +168,16 @@ func (p *Provider) Schema(_ context.Context, _ provider.SchemaRequest, resp *pro Optional: true, Validators: []validator.Int64{int64validator.AtLeast(0)}, }, + "env_var_prefix": schema.StringAttribute{ + MarkdownDescription: fmt.Sprintf("This attribute defines a prefix which redefines all of the " + + "`APSTRA_*` environment variables. For example, setting `env_var_prefix = \"FOO_\"` will cause " + + "the provider to learn the Apstra service URL from the \"FOO_APSTRA_URL\" environment variable " + + "rather than the \"APSTRA_URL\" environment variable. This capability is intended to be used " + + "when configuring multiple instances of the Apstra provider (which talk to multiple Apstra " + + "servers) in a single Terraform project."), + Optional: true, + Validators: []validator.String{stringvalidator.LengthAtLeast(1)}, + }, }, } } @@ -180,48 +190,49 @@ type providerConfig struct { MutexMessage types.String `tfsdk:"blueprint_mutex_message"` Experimental types.Bool `tfsdk:"experimental"` ApiTimeout types.Int64 `tfsdk:"api_timeout"` + EnvVarPrefix types.String `tfsdk:"env_var_prefix"` } func (o *providerConfig) fromEnv(_ context.Context, diags *diag.Diagnostics) { - if s, ok := os.LookupEnv(utils.EnvTlsNoVerify); ok && o.TlsNoVerify.IsNull() { + if s, ok := os.LookupEnv(o.EnvVarPrefix.String() + utils.EnvTlsNoVerify); ok && o.TlsNoVerify.IsNull() { v, err := strconv.ParseBool(s) if err != nil { - diags.AddError(fmt.Sprintf("error parsing environment variable %q", utils.EnvTlsNoVerify), err.Error()) + diags.AddError(fmt.Sprintf("error parsing environment variable %q", o.EnvVarPrefix.String()+utils.EnvTlsNoVerify), err.Error()) } o.TlsNoVerify = types.BoolValue(v) } - if s, ok := os.LookupEnv(utils.EnvBlueprintMutexEnabled); ok && o.MutexEnable.IsNull() { + if s, ok := os.LookupEnv(o.EnvVarPrefix.String() + utils.EnvBlueprintMutexEnabled); ok && o.MutexEnable.IsNull() { v, err := strconv.ParseBool(s) if err != nil { - diags.AddError(fmt.Sprintf("error parsing environment variable %q", utils.EnvBlueprintMutexEnabled), err.Error()) + diags.AddError(fmt.Sprintf("error parsing environment variable %q", o.EnvVarPrefix.String()+utils.EnvBlueprintMutexEnabled), err.Error()) } o.MutexEnable = types.BoolValue(v) } - if s, ok := os.LookupEnv(utils.EnvBlueprintMutexMessage); ok && o.MutexMessage.IsNull() { + if s, ok := os.LookupEnv(o.EnvVarPrefix.String() + utils.EnvBlueprintMutexMessage); ok && o.MutexMessage.IsNull() { if len(s) < 1 { - diags.AddError(fmt.Sprintf("error parsing environment variable %q", utils.EnvBlueprintMutexMessage), + diags.AddError(fmt.Sprintf("error parsing environment variable %q", o.EnvVarPrefix.String()+utils.EnvBlueprintMutexMessage), fmt.Sprintf("minimum string length 1; got %q", s)) } o.MutexMessage = types.StringValue(s) } - if s, ok := os.LookupEnv(utils.EnvApstraExperimental); ok && o.Experimental.IsNull() { + if s, ok := os.LookupEnv(o.EnvVarPrefix.String() + utils.EnvApstraExperimental); ok && o.Experimental.IsNull() { v, err := strconv.ParseBool(s) if err != nil { - diags.AddError(fmt.Sprintf("error parsing environment variable %q", utils.EnvApstraExperimental), err.Error()) + diags.AddError(fmt.Sprintf("error parsing environment variable %q", o.EnvVarPrefix.String()+utils.EnvApstraExperimental), err.Error()) } o.Experimental = types.BoolValue(v) } - if s, ok := os.LookupEnv(utils.EnvApiTimeout); ok && o.ApiTimeout.IsNull() { + if s, ok := os.LookupEnv(o.EnvVarPrefix.String() + o.EnvVarPrefix.String() + utils.EnvApiTimeout); ok && o.ApiTimeout.IsNull() { v, err := strconv.ParseInt(s, 0, 64) if err != nil { - diags.AddError(fmt.Sprintf("error parsing environment variable %q", utils.EnvApiTimeout), err.Error()) + diags.AddError(fmt.Sprintf("error parsing environment variable %q", o.EnvVarPrefix.String()+utils.EnvApiTimeout), err.Error()) } if v < 0 { - diags.AddError(fmt.Sprintf("invalid value in environment variable %q", utils.EnvApiTimeout), + diags.AddError(fmt.Sprintf("invalid value in environment variable %q", o.EnvVarPrefix.String()+utils.EnvApiTimeout), fmt.Sprintf("minimum permitted value is 0, got %d", v)) } o.ApiTimeout = types.Int64Value(v) @@ -268,7 +279,7 @@ func (p *Provider) Configure(ctx context.Context, req provider.ConfigureRequest, } // Create the Apstra client configuration from the URL and the environment. - clientCfg, err := utils.NewClientConfig(config.Url.ValueString()) + clientCfg, err := utils.NewClientConfig(config.Url.ValueString(), config.EnvVarPrefix.ValueString()) if err != nil { resp.Diagnostics.AddError("Error creating Apstra client configuration", err.Error()) return diff --git a/apstra/resource_template_rack_based_test.go b/apstra/resource_template_rack_based_test.go index e1650cd6..e0e2db50 100644 --- a/apstra/resource_template_rack_based_test.go +++ b/apstra/resource_template_rack_based_test.go @@ -43,7 +43,7 @@ func TestResourceTemplateRackBased(t *testing.T) { t.Fatalf("apstra url environment variable (%s) must be set and non-empty", utils.EnvApstraUrl) } - clientCfg, err := utils.NewClientConfig(apstraUrl) + clientCfg, err := utils.NewClientConfig(apstraUrl, "") if err != nil { t.Fatal(err) } diff --git a/apstra/test_utils/test_utils.go b/apstra/test_utils/test_utils.go index 6831cfef..91ed6112 100644 --- a/apstra/test_utils/test_utils.go +++ b/apstra/test_utils/test_utils.go @@ -41,7 +41,7 @@ func GetTestClient(t testing.TB, ctx context.Context) *apstra.Client { t.Fatal(err) } - clientCfg, err := utils.NewClientConfig("") + clientCfg, err := utils.NewClientConfig("", "") if err != nil { t.Fatal(err) } diff --git a/apstra/utils/client.go b/apstra/utils/client.go index 9be00ac3..7b60d511 100644 --- a/apstra/utils/client.go +++ b/apstra/utils/client.go @@ -34,10 +34,10 @@ substitutions: %s` ) -func NewClientConfig(apstraUrl string) (*apstra.ClientCfg, error) { +func NewClientConfig(apstraUrl, envVarPrefix string) (*apstra.ClientCfg, error) { // Populate raw URL string from config or environment. if apstraUrl == "" { - apstraUrl = os.Getenv(EnvApstraUrl) + apstraUrl = os.Getenv(envVarPrefix + EnvApstraUrl) } if apstraUrl == "" { @@ -66,7 +66,7 @@ func NewClientConfig(apstraUrl string) (*apstra.ClientCfg, error) { // Determine the Apstra username. user := parsedUrl.User.Username() if user == "" { - if val, ok := os.LookupEnv(EnvApstraUsername); ok { + if val, ok := os.LookupEnv(envVarPrefix + EnvApstraUsername); ok { user = val } else { return nil, errors.New("unable to determine apstra username - " + fmt.Sprintf(urlEncodeMsg, UrlEscapeTable())) @@ -76,7 +76,7 @@ func NewClientConfig(apstraUrl string) (*apstra.ClientCfg, error) { // Determine the Apstra password. pass, found := parsedUrl.User.Password() if !found { - if val, ok := os.LookupEnv(EnvApstraPassword); ok { + if val, ok := os.LookupEnv(envVarPrefix + EnvApstraPassword); ok { pass = val } else { return nil, errors.New("unable to determine apstra password") @@ -88,7 +88,7 @@ func NewClientConfig(apstraUrl string) (*apstra.ClientCfg, error) { // Set up a logger. var logger *log.Logger - if logFileName, ok := os.LookupEnv(EnvApstraLogfile); ok { + if logFileName, ok := os.LookupEnv(envVarPrefix + EnvApstraLogfile); ok { logFile, err := os.OpenFile(logFileName, os.O_WRONLY|os.O_APPEND|os.O_CREATE, 0644) if err != nil { return nil, err @@ -114,7 +114,7 @@ func NewClientConfig(apstraUrl string) (*apstra.ClientCfg, error) { }, } - _, experimental := os.LookupEnv(EnvApstraExperimental) + _, experimental := os.LookupEnv(envVarPrefix + EnvApstraExperimental) // Create the clientCfg return &apstra.ClientCfg{ From dd3ce53924bc6f922e3d141784f535dbb6d23ccd Mon Sep 17 00:00:00 2001 From: Chris Marget Date: Wed, 17 Jul 2024 09:03:33 -0400 Subject: [PATCH 3/5] improve doc string --- apstra/provider.go | 4 ++-- docs/index.md | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/apstra/provider.go b/apstra/provider.go index 10fd8c0e..275a1823 100644 --- a/apstra/provider.go +++ b/apstra/provider.go @@ -171,8 +171,8 @@ func (p *Provider) Schema(_ context.Context, _ provider.SchemaRequest, resp *pro "env_var_prefix": schema.StringAttribute{ MarkdownDescription: fmt.Sprintf("This attribute defines a prefix which redefines all of the " + "`APSTRA_*` environment variables. For example, setting `env_var_prefix = \"FOO_\"` will cause " + - "the provider to learn the Apstra service URL from the \"FOO_APSTRA_URL\" environment variable " + - "rather than the \"APSTRA_URL\" environment variable. This capability is intended to be used " + + "the provider to learn the Apstra service URL from the `FOO_APSTRA_URL` environment variable " + + "rather than the `APSTRA_URL` environment variable. This capability is intended to be used " + "when configuring multiple instances of the Apstra provider (which talk to multiple Apstra " + "servers) in a single Terraform project."), Optional: true, diff --git a/docs/index.md b/docs/index.md index 4491284a..0b0f6d2c 100644 --- a/docs/index.md +++ b/docs/index.md @@ -63,6 +63,7 @@ may be set via environment variables: `APSTRA_API_TIMEOUT`, - `api_timeout` (Number) Timeout in seconds for completing API transactions with the Apstra server. Omit for default value of 10 seconds. Value of 0 results in infinite timeout. - `blueprint_mutex_enabled` (Boolean) Blueprint mutexes are indicators that changes are being made in a staging Blueprint and other automation processes (including other instances of Terraform) should wait before beginning to make changes of their own. Setting this attribute 'true' causes the provider to lock a blueprint-specific mutex before making any changes. [More info here](https://github.com/Juniper/terraform-provider-apstra/blob/main/kb/blueprint_mutex.md). - `blueprint_mutex_message` (String) Blueprint mutexes are signals that changes are being made in a staging Blueprint and other automation processes (including other instances of Terraform) should wait before beginning to make changes of their own. The mutexes embed a human-readable field to reduce confusion in the event a mutex needs to be cleared manually. This attribute overrides the default message in that field: "locked by terraform at $DATE". +- `env_var_prefix` (String) This attribute defines a prefix which redefines all of the `APSTRA_*` environment variables. For example, setting `env_var_prefix = "FOO_"` will cause the provider to learn the Apstra service URL from the `FOO_APSTRA_URL` environment variable rather than the `APSTRA_URL` environment variable. This capability is intended to be used when configuring multiple instances of the Apstra provider (which talk to multiple Apstra servers) in a single Terraform project. - `experimental` (Boolean) Enable *experimental* features. In this release that means: - Set the `experimental` flag in the underlying Apstra SDK client object. Doing so permits connections to Apstra instances not supported by the SDK. - `tls_validation_disabled` (Boolean) Set 'true' to disable TLS certificate validation. From 00030532ad048445e72433c1e3d79974c4bf1072 Mon Sep 17 00:00:00 2001 From: Chris Marget Date: Wed, 17 Jul 2024 09:29:13 -0400 Subject: [PATCH 4/5] move env var constants to `constants` package --- apstra/constants/env_vars.go | 13 +++++++++ apstra/constants/{constants.go => errors.go} | 3 -- apstra/constants/numeric.go | 6 ++++ apstra/provider.go | 28 +++++++++--------- apstra/resource_template_rack_based_test.go | 5 ++-- apstra/test_utils/test_utils.go | 13 +++++---- apstra/utils/client.go | 30 +++++++------------- 7 files changed, 55 insertions(+), 43 deletions(-) create mode 100644 apstra/constants/env_vars.go rename apstra/constants/{constants.go => errors.go} (90%) create mode 100644 apstra/constants/numeric.go diff --git a/apstra/constants/env_vars.go b/apstra/constants/env_vars.go new file mode 100644 index 00000000..743f044d --- /dev/null +++ b/apstra/constants/env_vars.go @@ -0,0 +1,13 @@ +package constants + +const ( + EnvApiTimeout = "APSTRA_API_TIMEOUT" + EnvBlueprintMutexEnabled = "APSTRA_BLUEPRINT_MUTEX_ENABLED" + EnvBlueprintMutexMessage = "APSTRA_BLUEPRINT_MUTEX_MESSAGE" + EnvExperimental = "APSTRA_EXPERIMENTAL" + EnvLogfile = "APSTRA_LOG" + EnvPassword = "APSTRA_PASS" + EnvTlsNoVerify = "APSTRA_TLS_VALIDATION_DISABLED" + EnvUrl = "APSTRA_URL" + EnvUsername = "APSTRA_USER" +) diff --git a/apstra/constants/constants.go b/apstra/constants/errors.go similarity index 90% rename from apstra/constants/constants.go rename to apstra/constants/errors.go index 3e4d65f7..145a7ce2 100644 --- a/apstra/constants/constants.go +++ b/apstra/constants/errors.go @@ -6,7 +6,4 @@ const ( ErrProviderBug = "Provider Bug. Please report this issue to the provider maintainers." ErrInvalidConfig = "invalid configuration" ErrStringParse = "failed to parse string value" - - L3MtuMin = 1280 - L3MtuMax = 9216 ) diff --git a/apstra/constants/numeric.go b/apstra/constants/numeric.go new file mode 100644 index 00000000..1ce7a1ca --- /dev/null +++ b/apstra/constants/numeric.go @@ -0,0 +1,6 @@ +package constants + +const ( + L3MtuMax = 9216 + L3MtuMin = 1280 +) diff --git a/apstra/provider.go b/apstra/provider.go index 275a1823..879ef54d 100644 --- a/apstra/provider.go +++ b/apstra/provider.go @@ -12,6 +12,8 @@ import ( "sync" "time" + "github.com/Juniper/terraform-provider-apstra/apstra/constants" + "github.com/Juniper/apstra-go-sdk/apstra" "github.com/Juniper/terraform-provider-apstra/apstra/compatibility" "github.com/Juniper/terraform-provider-apstra/apstra/utils" @@ -126,8 +128,8 @@ func (p *Provider) Schema(_ context.Context, _ provider.SchemaRequest, resp *pro "[standard syntax](https://datatracker.ietf.org/doc/html/rfc1738#section-3.1). Care should be " + "taken to ensure that these credentials aren't accidentally committed to version control, etc... " + "The preferred approach is to pass the credentials as environment variables `" + - utils.EnvApstraUsername + "` and `" + utils.EnvApstraPassword + "`.\n If `url` is omitted, " + - "environment variable `" + utils.EnvApstraUrl + "` can be used to in its place.\n When the " + + constants.EnvUsername + "` and `" + constants.EnvPassword + "`.\n If `url` is omitted, " + + "environment variable `" + constants.EnvUrl + "` can be used to in its place.\n When the " + "username or password are embedded in the URL string, any special characters must be " + "URL-encoded. For example, `pass^word` would become `pass%5eword`.", Optional: true, @@ -194,45 +196,45 @@ type providerConfig struct { } func (o *providerConfig) fromEnv(_ context.Context, diags *diag.Diagnostics) { - if s, ok := os.LookupEnv(o.EnvVarPrefix.String() + utils.EnvTlsNoVerify); ok && o.TlsNoVerify.IsNull() { + if s, ok := os.LookupEnv(o.EnvVarPrefix.String() + constants.EnvTlsNoVerify); ok && o.TlsNoVerify.IsNull() { v, err := strconv.ParseBool(s) if err != nil { - diags.AddError(fmt.Sprintf("error parsing environment variable %q", o.EnvVarPrefix.String()+utils.EnvTlsNoVerify), err.Error()) + diags.AddError(fmt.Sprintf("error parsing environment variable %q", o.EnvVarPrefix.String()+constants.EnvTlsNoVerify), err.Error()) } o.TlsNoVerify = types.BoolValue(v) } - if s, ok := os.LookupEnv(o.EnvVarPrefix.String() + utils.EnvBlueprintMutexEnabled); ok && o.MutexEnable.IsNull() { + if s, ok := os.LookupEnv(o.EnvVarPrefix.String() + constants.EnvBlueprintMutexEnabled); ok && o.MutexEnable.IsNull() { v, err := strconv.ParseBool(s) if err != nil { - diags.AddError(fmt.Sprintf("error parsing environment variable %q", o.EnvVarPrefix.String()+utils.EnvBlueprintMutexEnabled), err.Error()) + diags.AddError(fmt.Sprintf("error parsing environment variable %q", o.EnvVarPrefix.String()+constants.EnvBlueprintMutexEnabled), err.Error()) } o.MutexEnable = types.BoolValue(v) } - if s, ok := os.LookupEnv(o.EnvVarPrefix.String() + utils.EnvBlueprintMutexMessage); ok && o.MutexMessage.IsNull() { + if s, ok := os.LookupEnv(o.EnvVarPrefix.String() + constants.EnvBlueprintMutexMessage); ok && o.MutexMessage.IsNull() { if len(s) < 1 { - diags.AddError(fmt.Sprintf("error parsing environment variable %q", o.EnvVarPrefix.String()+utils.EnvBlueprintMutexMessage), + diags.AddError(fmt.Sprintf("error parsing environment variable %q", o.EnvVarPrefix.String()+constants.EnvBlueprintMutexMessage), fmt.Sprintf("minimum string length 1; got %q", s)) } o.MutexMessage = types.StringValue(s) } - if s, ok := os.LookupEnv(o.EnvVarPrefix.String() + utils.EnvApstraExperimental); ok && o.Experimental.IsNull() { + if s, ok := os.LookupEnv(o.EnvVarPrefix.String() + constants.EnvExperimental); ok && o.Experimental.IsNull() { v, err := strconv.ParseBool(s) if err != nil { - diags.AddError(fmt.Sprintf("error parsing environment variable %q", o.EnvVarPrefix.String()+utils.EnvApstraExperimental), err.Error()) + diags.AddError(fmt.Sprintf("error parsing environment variable %q", o.EnvVarPrefix.String()+constants.EnvExperimental), err.Error()) } o.Experimental = types.BoolValue(v) } - if s, ok := os.LookupEnv(o.EnvVarPrefix.String() + o.EnvVarPrefix.String() + utils.EnvApiTimeout); ok && o.ApiTimeout.IsNull() { + if s, ok := os.LookupEnv(o.EnvVarPrefix.String() + o.EnvVarPrefix.String() + constants.EnvApiTimeout); ok && o.ApiTimeout.IsNull() { v, err := strconv.ParseInt(s, 0, 64) if err != nil { - diags.AddError(fmt.Sprintf("error parsing environment variable %q", o.EnvVarPrefix.String()+utils.EnvApiTimeout), err.Error()) + diags.AddError(fmt.Sprintf("error parsing environment variable %q", o.EnvVarPrefix.String()+constants.EnvApiTimeout), err.Error()) } if v < 0 { - diags.AddError(fmt.Sprintf("invalid value in environment variable %q", o.EnvVarPrefix.String()+utils.EnvApiTimeout), + diags.AddError(fmt.Sprintf("invalid value in environment variable %q", o.EnvVarPrefix.String()+constants.EnvApiTimeout), fmt.Sprintf("minimum permitted value is 0, got %d", v)) } o.ApiTimeout = types.Int64Value(v) diff --git a/apstra/resource_template_rack_based_test.go b/apstra/resource_template_rack_based_test.go index e0e2db50..4768ac15 100644 --- a/apstra/resource_template_rack_based_test.go +++ b/apstra/resource_template_rack_based_test.go @@ -6,6 +6,7 @@ import ( "context" "fmt" apiversions "github.com/Juniper/terraform-provider-apstra/apstra/api_versions" + "github.com/Juniper/terraform-provider-apstra/apstra/constants" "github.com/Juniper/terraform-provider-apstra/apstra/utils" "github.com/hashicorp/go-version" "github.com/hashicorp/terraform-plugin-testing/helper/acctest" @@ -38,9 +39,9 @@ resource "apstra_template_rack_based" "test" { func TestResourceTemplateRackBased(t *testing.T) { ctx := context.Background() - apstraUrl, ok := os.LookupEnv(utils.EnvApstraUrl) + apstraUrl, ok := os.LookupEnv(constants.EnvUrl) if !ok || apstraUrl == "" { - t.Fatalf("apstra url environment variable (%s) must be set and non-empty", utils.EnvApstraUrl) + t.Fatalf("apstra url environment variable (%s) must be set and non-empty", constants.EnvUrl) } clientCfg, err := utils.NewClientConfig(apstraUrl, "") diff --git a/apstra/test_utils/test_utils.go b/apstra/test_utils/test_utils.go index 91ed6112..58fed215 100644 --- a/apstra/test_utils/test_utils.go +++ b/apstra/test_utils/test_utils.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "github.com/Juniper/apstra-go-sdk/apstra" + "github.com/Juniper/terraform-provider-apstra/apstra/constants" "github.com/Juniper/terraform-provider-apstra/apstra/utils" "github.com/hashicorp/hcl/v2/hclsimple" "net/http" @@ -76,23 +77,23 @@ func TestCfgFileToEnv() error { } if testCfg.Url != "" { - err = os.Setenv(utils.EnvApstraUrl, testCfg.Url) + err = os.Setenv(constants.EnvUrl, testCfg.Url) if err != nil { - return fmt.Errorf("failed setting environment variable %q - %w", utils.EnvApstraUrl, err) + return fmt.Errorf("failed setting environment variable %q - %w", constants.EnvUrl, err) } } if testCfg.Username != "" { - err = os.Setenv(utils.EnvApstraUsername, testCfg.Username) + err = os.Setenv(constants.EnvUsername, testCfg.Username) if err != nil { - return fmt.Errorf("failed setting environment variable %q - %w", utils.EnvApstraUsername, err) + return fmt.Errorf("failed setting environment variable %q - %w", constants.EnvUsername, err) } } if testCfg.Password != "" { - err = os.Setenv(utils.EnvApstraPassword, testCfg.Password) + err = os.Setenv(constants.EnvPassword, testCfg.Password) if err != nil { - return fmt.Errorf("failed setting environment variable %q - %w", utils.EnvApstraPassword, err) + return fmt.Errorf("failed setting environment variable %q - %w", constants.EnvPassword, err) } } diff --git a/apstra/utils/client.go b/apstra/utils/client.go index 7b60d511..bb236e2b 100644 --- a/apstra/utils/client.go +++ b/apstra/utils/client.go @@ -4,28 +4,20 @@ import ( "crypto/tls" "errors" "fmt" - "github.com/Juniper/apstra-go-sdk/apstra" "io" "log" "net/http" "net/url" "os" "strings" + + "github.com/Juniper/apstra-go-sdk/apstra" + "github.com/Juniper/terraform-provider-apstra/apstra/constants" ) const ( - EnvApiTimeout = "APSTRA_API_TIMEOUT" - EnvApstraUrl = "APSTRA_URL" - EnvBlueprintMutexEnabled = "APSTRA_BLUEPRINT_MUTEX_ENABLED" - EnvBlueprintMutexMessage = "APSTRA_BLUEPRINT_MUTEX_MESSAGE" - EnvTlsNoVerify = "APSTRA_TLS_VALIDATION_DISABLED" - EnvApstraUsername = "APSTRA_USER" - EnvApstraPassword = "APSTRA_PASS" - EnvApstraLogfile = "APSTRA_LOG" - EnvApstraExperimental = "APSTRA_EXPERIMENTAL" - EnvTlsKeyLogFile = "SSLKEYLOGFILE" - - urlEncodeMsg = ` + envTlsKeyLogFile = "SSLKEYLOGFILE" + urlEncodeMsg = ` Note that when the Username or Password fields contain special characters and are embedded in the URL, they must be URL-encoded by substituting '%%' in place of each special character. The following table demonstrates some common @@ -37,7 +29,7 @@ substitutions: func NewClientConfig(apstraUrl, envVarPrefix string) (*apstra.ClientCfg, error) { // Populate raw URL string from config or environment. if apstraUrl == "" { - apstraUrl = os.Getenv(envVarPrefix + EnvApstraUrl) + apstraUrl = os.Getenv(envVarPrefix + constants.EnvUrl) } if apstraUrl == "" { @@ -66,7 +58,7 @@ func NewClientConfig(apstraUrl, envVarPrefix string) (*apstra.ClientCfg, error) // Determine the Apstra username. user := parsedUrl.User.Username() if user == "" { - if val, ok := os.LookupEnv(envVarPrefix + EnvApstraUsername); ok { + if val, ok := os.LookupEnv(envVarPrefix + constants.EnvUsername); ok { user = val } else { return nil, errors.New("unable to determine apstra username - " + fmt.Sprintf(urlEncodeMsg, UrlEscapeTable())) @@ -76,7 +68,7 @@ func NewClientConfig(apstraUrl, envVarPrefix string) (*apstra.ClientCfg, error) // Determine the Apstra password. pass, found := parsedUrl.User.Password() if !found { - if val, ok := os.LookupEnv(envVarPrefix + EnvApstraPassword); ok { + if val, ok := os.LookupEnv(envVarPrefix + constants.EnvPassword); ok { pass = val } else { return nil, errors.New("unable to determine apstra password") @@ -88,7 +80,7 @@ func NewClientConfig(apstraUrl, envVarPrefix string) (*apstra.ClientCfg, error) // Set up a logger. var logger *log.Logger - if logFileName, ok := os.LookupEnv(envVarPrefix + EnvApstraLogfile); ok { + if logFileName, ok := os.LookupEnv(envVarPrefix + constants.EnvLogfile); ok { logFile, err := os.OpenFile(logFileName, os.O_WRONLY|os.O_APPEND|os.O_CREATE, 0644) if err != nil { return nil, err @@ -98,7 +90,7 @@ func NewClientConfig(apstraUrl, envVarPrefix string) (*apstra.ClientCfg, error) // Set up the TLS session key log. var klw io.Writer - if fileName, ok := os.LookupEnv(EnvTlsKeyLogFile); ok { + if fileName, ok := os.LookupEnv(envTlsKeyLogFile); ok { klw, err = newKeyLogWriter(fileName) if err != nil { return nil, err @@ -114,7 +106,7 @@ func NewClientConfig(apstraUrl, envVarPrefix string) (*apstra.ClientCfg, error) }, } - _, experimental := os.LookupEnv(envVarPrefix + EnvApstraExperimental) + _, experimental := os.LookupEnv(envVarPrefix + constants.EnvExperimental) // Create the clientCfg return &apstra.ClientCfg{ From c86e57c20c908ad95b38115934089f2a66dcf9cc Mon Sep 17 00:00:00 2001 From: Chris Marget Date: Wed, 17 Jul 2024 09:30:37 -0400 Subject: [PATCH 5/5] whitespace --- apstra/provider.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/apstra/provider.go b/apstra/provider.go index 879ef54d..fef23d31 100644 --- a/apstra/provider.go +++ b/apstra/provider.go @@ -12,10 +12,9 @@ import ( "sync" "time" - "github.com/Juniper/terraform-provider-apstra/apstra/constants" - "github.com/Juniper/apstra-go-sdk/apstra" "github.com/Juniper/terraform-provider-apstra/apstra/compatibility" + "github.com/Juniper/terraform-provider-apstra/apstra/constants" "github.com/Juniper/terraform-provider-apstra/apstra/utils" "github.com/hashicorp/terraform-plugin-framework-validators/int64validator" "github.com/hashicorp/terraform-plugin-framework-validators/stringvalidator"