From bb16ff05e4cb3b2707cf35304ad5766626eb999d Mon Sep 17 00:00:00 2001 From: Matt Hagenbuch Date: Mon, 12 Feb 2024 14:06:11 -0500 Subject: [PATCH] deprecate environment in favor of api_url (#206) --- .go-version | 2 +- Makefile | 6 ++-- README.md | 8 +---- client/client.go | 28 +++++------------ client/client_test.go | 21 +++---------- client/destinations_test.go | 6 ++-- client/inferred_service_rules_test.go | 3 +- client/metric_conditions_test.go | 6 ++-- client/metric_dashboards_test.go | 6 ++-- client/saml_group_mappings_test.go | 3 +- client/user_role_binding_test.go | 3 +- docs/index.md | 3 +- exporter/exporter.go | 10 ++++--- exporter/exporter_test.go | 43 +++++++++++++++++++++++++++ lightstep/provider.go | 26 ++++++++++++++-- 15 files changed, 100 insertions(+), 74 deletions(-) diff --git a/.go-version b/.go-version index a0f550d6..82e24bf2 100644 --- a/.go-version +++ b/.go-version @@ -1 +1 @@ -1.89.0 \ No newline at end of file +1.90.0 diff --git a/Makefile b/Makefile index 004e41a7..e9234a6d 100644 --- a/Makefile +++ b/Makefile @@ -51,7 +51,7 @@ endif LIGHTSTEP_API_KEY=${LIGHTSTEP_API_KEY_PUBLIC} \ LIGHTSTEP_ORG="terraform-provider" \ LIGHTSTEP_PROJECT="terraform-provider-test" \ - LIGHTSTEP_ENV="public" \ + LIGHTSTEP_API_BASE_URL="https://api.lightstep.com" \ go test -v ./lightstep .PHONY: test-local @@ -62,7 +62,7 @@ test-local: LIGHTSTEP_ORG="terraform-provider" \ LIGHTSTEP_PROJECT="terraform-provider-test" \ LIGHTSTEP_API_RATE_LIMIT=100 \ - LIGHTSTEP_ENV="public" \ + LIGHTSTEP_API_BASE_URL="https://api.lightstep.com" \ go test -v ./lightstep -test.run TestAccSAMLGroupMappings .PHONY: test-staging @@ -72,7 +72,7 @@ test-staging: LIGHTSTEP_API_KEY=${LIGHTSTEP_STAGING_API_KEY} \ LIGHTSTEP_ORG="terraform-provider" \ LIGHTSTEP_PROJECT="terraform-provider-test" \ - LIGHTSTEP_ENV="staging" \ + LIGHTSTEP_API_BASE_URL="https://api-staging.lightstep.com" \ go test -v ./lightstep .PHONY: ensure-clean-repo diff --git a/README.md b/README.md index 79123072..a63a6374 100644 --- a/README.md +++ b/README.md @@ -60,16 +60,10 @@ It's possible to export an existing Lightstep dashboard to HCL code using the pr The `exporter` utility is built-in to the provider binary and requires certain environment variables to be set: -For the LIGHTSTEP_ENV variable: - -- public = app.lightstep.com -- meta = app-meta.lightstep.com -- staging = app-staging.lightstep.com - ``` $ export LIGHTSTEP_API_KEY=.... $ export LIGHTSTEP_ORG=your-org -$ export LIGHTSTEP_ENV=public +$ export LIGHTSTEP_API_BASE_URL='https://api.lightstep.com' # exports to console dashboard id = rZbPJ33q from project terraform-shop $ go run github.com/lightstep/terraform-provider-lightstep exporter lightstep_dashboard terraform-shop rZbPJ33q diff --git a/client/client.go b/client/client.go index 38b06495..4d1b4a7a 100644 --- a/client/client.go +++ b/client/client.go @@ -65,7 +65,7 @@ type genericAPIResponse[T any] struct { type Client struct { apiKey string - baseURL string + baseUrl string orgName string client *retryablehttp.Client rateLimiter *rate.Limiter @@ -74,26 +74,12 @@ type Client struct { } // NewClient gets a client for the public API -func NewClient(apiKey string, orgName string, env string) *Client { - return NewClientWithUserAgent(apiKey, orgName, env, fmt.Sprintf("%s/%s", DefaultUserAgent, version.ProviderVersion)) +func NewClient(apiKey string, orgName string, baseUrl string) *Client { + return NewClientWithUserAgent(apiKey, orgName, baseUrl, fmt.Sprintf("%s/%s", DefaultUserAgent, version.ProviderVersion)) } -func NewClientWithUserAgent(apiKey string, orgName string, env string, userAgent string) *Client { - // Let the user override the API base URL. - // e.g. http://localhost:8080 - envBaseURL := os.Getenv("LIGHTSTEP_API_BASE_URL") - - var baseURL string - if envBaseURL != "" { - // User specified a base URL, let that take priority. - baseURL = envBaseURL - } else if env == "public" { - baseURL = "https://api.lightstep.com" - } else { - baseURL = fmt.Sprintf("https://api-%v.lightstep.com", env) - } - - fullBaseURL := fmt.Sprintf("%s/public/v0.2/%v", baseURL, orgName) +func NewClientWithUserAgent(apiKey string, orgName string, baseUrl string, userAgent string) *Client { + fullBaseUrl := fmt.Sprintf("%s/public/v0.2/%v", baseUrl, orgName) rateLimitStr := os.Getenv("LIGHTSTEP_API_RATE_LIMIT") rateLimit, err := strconv.Atoi(rateLimitStr) @@ -108,7 +94,7 @@ func NewClientWithUserAgent(apiKey string, orgName string, env string, userAgent return &Client{ apiKey: apiKey, orgName: orgName, - baseURL: fullBaseURL, + baseUrl: fullBaseUrl, userAgent: userAgent, rateLimiter: rate.NewLimiter(rate.Limit(rateLimit), 1), client: newClient, @@ -121,7 +107,7 @@ func (c *Client) CallAPI(ctx context.Context, httpMethod string, suffix string, return callAPI( ctx, c, - fmt.Sprintf("%v/%v", c.baseURL, suffix), + fmt.Sprintf("%v/%v", c.baseUrl, suffix), httpMethod, Headers{ "Authorization": fmt.Sprintf("bearer %v", c.apiKey), diff --git a/client/client_test.go b/client/client_test.go index d8bd9410..d45a6ea6 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -1,26 +1,13 @@ package client import ( - "github.com/stretchr/testify/assert" "testing" + + "github.com/stretchr/testify/assert" ) func TestNew_public(t *testing.T) { t.Parallel() - - c := NewClient("api-key", "org-name", "public") - assert.Equal(t, "https://api.lightstep.com/public/v0.2/org-name", c.baseURL) -} - -func TestNew_other(t *testing.T) { - t.Parallel() - c := NewClient("api-key", "org-name", "other") - assert.Equal(t, "https://api-other.lightstep.com/public/v0.2/org-name", c.baseURL) -} - -func TestNew_env_var_provided_baseURL(t *testing.T) { - // Parallel not used here due to t.Setenv. - t.Setenv("LIGHTSTEP_API_BASE_URL", "http://localhost:8080") - c := NewClient("api-key", "org-name", "public") - assert.Equal(t, "http://localhost:8080/public/v0.2/org-name", c.baseURL) + c := NewClient("api-key", "org-name", "https://api.lightstep.com") + assert.Equal(t, "https://api.lightstep.com/public/v0.2/org-name", c.baseUrl) } diff --git a/client/destinations_test.go b/client/destinations_test.go index 18850b2e..d9fcde48 100644 --- a/client/destinations_test.go +++ b/client/destinations_test.go @@ -18,8 +18,7 @@ func Test_DeleteDestionation_when_connection_is_closed(t *testing.T) { })) defer server.Close() - t.Setenv("LIGHTSTEP_API_BASE_URL", server.URL) - c := NewClient("api", "blars", "staging") + c := NewClient("api", "blars", server.URL) err := c.DeleteDestination(context.Background(), "tacoman", "hi") assert.NotNil(t, err) @@ -36,8 +35,7 @@ func Test_DeleteDestination_when_connection_has_wrong_content_length(t *testing. })) defer server.Close() - t.Setenv("LIGHTSTEP_API_BASE_URL", server.URL) - c := NewClient("api", "blars", "staging") + c := NewClient("api", "blars", server.URL) err := c.DeleteDestination(context.Background(), "tacoman", "hi") assert.NotNil(t, err) diff --git a/client/inferred_service_rules_test.go b/client/inferred_service_rules_test.go index ffcf8247..ee48cb6d 100644 --- a/client/inferred_service_rules_test.go +++ b/client/inferred_service_rules_test.go @@ -44,8 +44,7 @@ func Test_InferredServiceRule_GetInferredServiceRulesReturnsRule(t *testing.T) { })) defer server.Close() - t.Setenv("LIGHTSTEP_API_BASE_URL", server.URL) - apiClient := NewClient("api", "blars", "staging") + apiClient := NewClient("api", "blars", server.URL) inferredServiceRule, err := apiClient.GetInferredServiceRule( context.Background(), "terraform-provider-tests", diff --git a/client/metric_conditions_test.go b/client/metric_conditions_test.go index 92e122bf..0649bd14 100644 --- a/client/metric_conditions_test.go +++ b/client/metric_conditions_test.go @@ -18,8 +18,7 @@ func Test_DeleteUnifiedCondition_when_connection_is_closed(t *testing.T) { })) defer server.Close() - t.Setenv("LIGHTSTEP_API_BASE_URL", server.URL) - c := NewClient("api", "blars", "staging") + c := NewClient("api", "blars", server.URL) err := c.DeleteUnifiedCondition(context.Background(), "tacoman", "hi") assert.NotNil(t, err) @@ -36,8 +35,7 @@ func Test_DeleteUnifiedCondition_when_connection_has_wrong_content_length(t *tes })) defer server.Close() - t.Setenv("LIGHTSTEP_API_BASE_URL", server.URL) - c := NewClient("api", "blars", "staging") + c := NewClient("api", "blars", server.URL) err := c.DeleteUnifiedCondition(context.Background(), "tacoman", "hi") assert.NotNil(t, err) diff --git a/client/metric_dashboards_test.go b/client/metric_dashboards_test.go index fb38d5b1..2a21b845 100644 --- a/client/metric_dashboards_test.go +++ b/client/metric_dashboards_test.go @@ -39,8 +39,7 @@ func Test_DeleteMetricDashboard_when_connection_is_closed(t *testing.T) { })) defer server.Close() - t.Setenv("LIGHTSTEP_API_BASE_URL", server.URL) - c := NewClient("api", "blars", "staging") + c := NewClient("api", "blars", server.URL) err := c.DeleteUnifiedDashboard(context.Background(), "tacoman", "hi") assert.NotNil(t, err) @@ -56,8 +55,7 @@ func Test_DeleteMetricDashboard_when_connection_has_wrong_content_length(t *test })) defer server.Close() - t.Setenv("LIGHTSTEP_API_BASE_URL", server.URL) - c := NewClient("api", "blars", "staging") + c := NewClient("api", "blars", server.URL) err := c.DeleteUnifiedDashboard(context.Background(), "tacoman", "hi") assert.NotNil(t, err) diff --git a/client/saml_group_mappings_test.go b/client/saml_group_mappings_test.go index cca72f47..7a4f918f 100644 --- a/client/saml_group_mappings_test.go +++ b/client/saml_group_mappings_test.go @@ -39,8 +39,7 @@ func TestSAMLGroupMappings(t *testing.T) { })) defer server.Close() - t.Setenv("LIGHTSTEP_API_BASE_URL", server.URL) - c := NewClient("api", "blars", "staging") + c := NewClient("api", "blars", server.URL) sgm, err := c.ListSAMLGroupMappings(context.Background()) assert.NoError(t, err) diff --git a/client/user_role_binding_test.go b/client/user_role_binding_test.go index d379eb12..acdebe02 100644 --- a/client/user_role_binding_test.go +++ b/client/user_role_binding_test.go @@ -35,8 +35,7 @@ func Test_UserRoleBinding(t *testing.T) { })) defer server.Close() - t.Setenv("LIGHTSTEP_API_BASE_URL", server.URL) - c := NewClient("api", "blars", "staging") + c := NewClient("api", "blars", server.URL) rb, err := c.ListRoleBinding(context.Background(), "project with spaces", "project editor") assert.NoError(t, err) diff --git a/docs/index.md b/docs/index.md index 73cab3d0..aea408ad 100644 --- a/docs/index.md +++ b/docs/index.md @@ -35,4 +35,5 @@ provider "lightstep" { - `api_key` (String) The API Key for a Lightstep organization. - `api_key_env_var` (String) Environment variable for Lightstep API key. -- `environment` (String) The name of the Lightstep environment, must be one of: staging, meta, public. +- `api_url` (String) The base URL for the Lightstep API. This setting takes precedent over 'environment'. For example, https://api.lightstep.com +- `environment` (String, Deprecated) The name of the Lightstep environment, must be one of: staging, meta, public. Deprecated in favor of `api_url` diff --git a/exporter/exporter.go b/exporter/exporter.go index 13821515..70f141a0 100644 --- a/exporter/exporter.go +++ b/exporter/exporter.go @@ -169,9 +169,11 @@ func Run(args ...string) error { } // default to public API environment - lightstepEnv := "public" - if len(os.Getenv("LIGHTSTEP_ENV")) > 0 { - lightstepEnv = os.Getenv("LIGHTSTEP_ENV") + lightstepUrl := "https://api.lightstep.com" + if len(os.Getenv("LIGHTSTEP_API_URL")) > 0 { + lightstepUrl = os.Getenv("LIGHTSTEP_API_URL") + } else if len(os.Getenv("LIGHTSTEP_API_BASE_URL")) > 0 { + lightstepUrl = os.Getenv("LIGHTSTEP_API_BASE_URL") } if len(args) < 4 { @@ -182,7 +184,7 @@ func Run(args ...string) error { log.Fatalf("error: only dashboard resources are supported at this time") } - c := client.NewClient(os.Getenv("LIGHTSTEP_API_KEY"), os.Getenv("LIGHTSTEP_ORG"), lightstepEnv) + c := client.NewClient(os.Getenv("LIGHTSTEP_API_KEY"), os.Getenv("LIGHTSTEP_ORG"), lightstepUrl) d, err := c.GetUnifiedDashboard(context.Background(), args[3], args[4]) if err != nil { log.Fatalf("error: could not get dashboard: %v", err) diff --git a/exporter/exporter_test.go b/exporter/exporter_test.go index 45c73225..16d492dc 100644 --- a/exporter/exporter_test.go +++ b/exporter/exporter_test.go @@ -2,11 +2,17 @@ package exporter import ( "bytes" + "encoding/json" "fmt" + "net/http" + "net/http/httptest" + "os" "strings" "testing" + "time" "github.com/lightstep/terraform-provider-lightstep/client" + "github.com/stretchr/testify/assert" ) func TestExportToHCL(t *testing.T) { @@ -100,3 +106,40 @@ EOT`, }) } } + +func TestRunExport(t *testing.T) { + done := make(chan struct{}) + + var server *httptest.Server + server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, "/public/v0.2/blars-org/projects/blars/metric_dashboards/123abc", r.URL.Path) + body, err := json.Marshal(map[string]client.UnifiedDashboard{ + "data": { + ID: "123abc", + Type: "metric_dashboard", + Attributes: client.UnifiedDashboardAttributes{ + Name: "blars dashboard", + Charts: []client.UnifiedChart{}, + }, + }, + }) + assert.NoError(t, err) + _, err = w.Write(body) + assert.NoError(t, err) + w.WriteHeader(http.StatusOK) + close(done) + })) + defer server.Close() + + os.Setenv("LIGHTSTEP_API_KEY", "api-key") + os.Setenv("LIGHTSTEP_ORG", "blars-org") + os.Setenv("LIGHTSTEP_API_URL", server.URL) + err := Run("exporter", "export", "lightstep_dashboard", "blars", "123abc") + assert.NoError(t, err) + + select { + case <-time.After(10 * time.Second): + t.Fatal("timeout waiting for HTTP handler to be exercised") + case <-done: + } +} diff --git a/lightstep/provider.go b/lightstep/provider.go index d63e0a63..bdba21f9 100644 --- a/lightstep/provider.go +++ b/lightstep/provider.go @@ -30,7 +30,14 @@ func Provider() *schema.Provider { Optional: true, DefaultFunc: schema.EnvDefaultFunc("LIGHTSTEP_ENV", "public"), ValidateFunc: validation.StringMatch(regexp.MustCompile(`^(staging|meta|public)$`), "Must be one of: staging, meta, public"), - Description: "The name of the Lightstep environment, must be one of: staging, meta, public.", + Description: "The name of the Lightstep environment, must be one of: staging, meta, public. Deprecated in favor of `api_url`", + Deprecated: "This field is deprecated and will be removed in a future release. Please use the `api_url` field instead.", + }, + "api_url": { + Type: schema.TypeString, + Optional: true, + DefaultFunc: schema.MultiEnvDefaultFunc([]string{"LIGHTSTEP_API_URL", "LIGHTSTEP_API_BASE_URL"}, ""), + Description: "The base URL for the Lightstep API. This setting takes precedent over 'environment'. For example, https://api.lightstep.com", }, "api_key": { Type: schema.TypeString, @@ -93,10 +100,25 @@ func configureProvider(_ context.Context, d *schema.ResourceData) (interface{}, apiKey = apiKeyEnv } + // TODO remove this code once `environment` is fully deprecated + var baseUrl string + optionalUrl := d.Get("api_url").(string) + if len(optionalUrl) > 0 { + baseUrl = optionalUrl + } else { + // get the url from the `environment` provider attribute + env := d.Get("environment").(string) + if env == "public" { + baseUrl = "https://api.lightstep.com" + } else { + baseUrl = fmt.Sprintf("https://api-%v.lightstep.com", env) + } + } + client := client.NewClientWithUserAgent( apiKey, d.Get("organization").(string), - d.Get("environment").(string), + baseUrl, fmt.Sprintf("%s/%s (terraform %s)", "terraform-provider-lightstep", version.ProviderVersion, meta.SDKVersionString()), )