From e24fc7a2c61cf69b84781ae91a8160d7319ea71f Mon Sep 17 00:00:00 2001 From: Jacob Bednarz Date: Wed, 11 Oct 2023 13:05:55 +1100 Subject: [PATCH 1/7] utils: add `UserAgentBuilder` Introduces a `UserAgentBuilder` method that allows us to build contextually conditional user agent strings based on the plugin and operator suffix provided by the user. Signed-off-by: Jacob Bednarz --- internal/consts/provider.go | 2 +- internal/utils/user_agent_builder.go | 42 +++++++++++++++++++++++ internal/utils/user_agent_builder_test.go | 30 ++++++++++++++++ 3 files changed, 73 insertions(+), 1 deletion(-) create mode 100644 internal/utils/user_agent_builder.go create mode 100644 internal/utils/user_agent_builder_test.go diff --git a/internal/consts/provider.go b/internal/consts/provider.go index aef7cd8bf2..e22ae0dab9 100644 --- a/internal/consts/provider.go +++ b/internal/consts/provider.go @@ -87,7 +87,7 @@ const ( // Deprecated: Use resource specific account ID values instead. AccountIDEnvVarKey = "CLOUDFLARE_ACCOUNT_ID" - UserAgentDefault = "terraform/%s terraform-plugin-sdk/%s terraform-provider-cloudflare/%s" + UserAgentDefault = "terraform-provider-cloudflare/%s terraform-plugin-%s/%s" // Schema key for the account ID configuration. AccountIDSchemaKey = "account_id" diff --git a/internal/utils/user_agent_builder.go b/internal/utils/user_agent_builder.go new file mode 100644 index 0000000000..b2ef61fe2f --- /dev/null +++ b/internal/utils/user_agent_builder.go @@ -0,0 +1,42 @@ +package utils + +import ( + "fmt" +) + +type UserAgentBuilderParams struct { + ProviderVersion *string + PluginVersion *string + PluginType *string + TerraformVersion *string + OperatorSuffix *string +} + +func (p *UserAgentBuilderParams) String() string { + var ua string + if p.ProviderVersion != nil { + ua += fmt.Sprintf("terraform-provider-cloudflare/%s", *p.ProviderVersion) + } + + if p.PluginType != nil { + ua += fmt.Sprintf(" %s", *p.PluginType) + } + + if p.PluginVersion != nil { + ua += fmt.Sprintf("/%s", *p.PluginVersion) + } + + // Operator suffix and Terraform version are mutually exclusive and we should + // only ever see one of them. + if p.OperatorSuffix != nil { + ua += fmt.Sprintf(" %s", *p.OperatorSuffix) + } else if p.TerraformVersion != nil { + ua += fmt.Sprintf(" terraform/%s", *p.TerraformVersion) + } + + return ua +} + +func BuildUserAgent(params UserAgentBuilderParams) string { + return params.String() +} diff --git a/internal/utils/user_agent_builder_test.go b/internal/utils/user_agent_builder_test.go new file mode 100644 index 0000000000..2405f7a556 --- /dev/null +++ b/internal/utils/user_agent_builder_test.go @@ -0,0 +1,30 @@ +package utils + +import ( + "reflect" + "testing" + + "github.com/cloudflare/cloudflare-go" +) + +func TestUserAgentBuilding(t *testing.T) { + tests := []struct { + input UserAgentBuilderParams + expect string + }{ + {input: UserAgentBuilderParams{ProviderVersion: cloudflare.StringPtr("1.0")}, expect: "terraform-provider-cloudflare/1.0"}, + {input: UserAgentBuilderParams{ProviderVersion: cloudflare.StringPtr("1.0"), PluginType: cloudflare.StringPtr("terraform-plugin-foo")}, expect: "terraform-provider-cloudflare/1.0 terraform-plugin-foo"}, + {input: UserAgentBuilderParams{ProviderVersion: cloudflare.StringPtr("1.0"), PluginType: cloudflare.StringPtr("terraform-plugin-foo"), PluginVersion: cloudflare.StringPtr("1.2.3")}, expect: "terraform-provider-cloudflare/1.0 terraform-plugin-foo/1.2.3"}, + {input: UserAgentBuilderParams{ProviderVersion: cloudflare.StringPtr("1.0"), PluginType: cloudflare.StringPtr("terraform-plugin-foo"), PluginVersion: cloudflare.StringPtr("1.2.3"), TerraformVersion: cloudflare.StringPtr("9.9.9")}, expect: "terraform-provider-cloudflare/1.0 terraform-plugin-foo/1.2.3 terraform/9.9.9"}, + {input: UserAgentBuilderParams{ProviderVersion: cloudflare.StringPtr("1.0"), OperatorSuffix: cloudflare.StringPtr("example/v88")}, expect: "terraform-provider-cloudflare/1.0 example/v88"}, + {input: UserAgentBuilderParams{ProviderVersion: cloudflare.StringPtr("1.0"), OperatorSuffix: cloudflare.StringPtr("example/v88"), TerraformVersion: cloudflare.StringPtr("1.2.3")}, expect: "terraform-provider-cloudflare/1.0 example/v88"}, + {input: UserAgentBuilderParams{ProviderVersion: cloudflare.StringPtr("1.0"), TerraformVersion: cloudflare.StringPtr("1.2.3")}, expect: "terraform-provider-cloudflare/1.0 terraform/1.2.3"}, + } + + for _, tc := range tests { + got := BuildUserAgent(tc.input) + if !reflect.DeepEqual(tc.expect, got) { + t.Fatalf("expected: %v, got: %v", tc.expect, got) + } + } +} From bc6eb6201360228ba1ec8df6d2eca386680391bd Mon Sep 17 00:00:00 2001 From: Jacob Bednarz Date: Wed, 11 Oct 2023 13:13:54 +1100 Subject: [PATCH 2/7] provider: add support for UA operator suffixes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Within the Terraform ecosystem, there are multiple ways of using the providers. Generally, it is through the Terraform CLI however, there are third party tools like Pulumi that rely on the schemas to build their own integrations. One issue with this is that the requests they make are lumped with the label of `terraform/` in the HTTP user agent which doesn’t allow observablility into those specific requests. To enable better visibility here, we are introducing a new schema field called a User Agent Operator suffix. It is configurable in the schema (`user_agent_operator_suffix`) and via the environment variable (`CLOUDFLARE_USER_AGENT_OPERATOR_SUFFIX `) and will provide a way for these third party integrations to denote the source of their requests. ⚠️This *is not* a recommended configuration change for most and when not explicitly configured will default to the existing `terraform/` string (as it always has). You should only change this value if you know what you are doing. Signed-off-by: Jacob Bednarz --- internal/consts/provider.go | 6 ++++ internal/framework/provider/provider.go | 41 ++++++++++++++++--------- internal/sdkv2provider/provider.go | 19 ++++++++++-- 3 files changed, 50 insertions(+), 16 deletions(-) diff --git a/internal/consts/provider.go b/internal/consts/provider.go index e22ae0dab9..4a3b3643af 100644 --- a/internal/consts/provider.go +++ b/internal/consts/provider.go @@ -43,6 +43,12 @@ const ( // Default value for the API base path. APIBasePathDefault = "/client/v4" + // Schema key for the User Agent operator suffix. + UserAgentOperatorSuffixSchemaKey = "user_agent_operator_suffix" + + // Environment variable key for the User Agent operator suffix + UserAgentOperatorSuffixEnvVarKey = "CLOUDFLARE_USER_AGENT_OPERATOR_SUFFIX" + // Schema key for the requests per second configuration. RPSSchemaKey = "rps" diff --git a/internal/framework/provider/provider.go b/internal/framework/provider/provider.go index d10c595af7..ed3b9fc5bd 100644 --- a/internal/framework/provider/provider.go +++ b/internal/framework/provider/provider.go @@ -31,7 +31,6 @@ import ( "github.com/hashicorp/terraform-plugin-mux/tf5to6server" "github.com/hashicorp/terraform-plugin-mux/tf6muxserver" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/logging" - "github.com/hashicorp/terraform-plugin-sdk/v2/meta" ) // Ensure CloudflareProvider satisfies various provider interfaces. @@ -47,17 +46,18 @@ type CloudflareProvider struct { // CloudflareProviderModel describes the provider data model. type CloudflareProviderModel struct { - APIKey types.String `tfsdk:"api_key"` - APIUserServiceKey types.String `tfsdk:"api_user_service_key"` - Email types.String `tfsdk:"email"` - MinBackOff types.Int64 `tfsdk:"min_backoff"` - RPS types.Int64 `tfsdk:"rps"` - APIBasePath types.String `tfsdk:"api_base_path"` - APIToken types.String `tfsdk:"api_token"` - Retries types.Int64 `tfsdk:"retries"` - MaxBackoff types.Int64 `tfsdk:"max_backoff"` - APIClientLogging types.Bool `tfsdk:"api_client_logging"` - APIHostname types.String `tfsdk:"api_hostname"` + APIKey types.String `tfsdk:"api_key"` + APIUserServiceKey types.String `tfsdk:"api_user_service_key"` + Email types.String `tfsdk:"email"` + MinBackOff types.Int64 `tfsdk:"min_backoff"` + RPS types.Int64 `tfsdk:"rps"` + APIBasePath types.String `tfsdk:"api_base_path"` + APIToken types.String `tfsdk:"api_token"` + Retries types.Int64 `tfsdk:"retries"` + MaxBackoff types.Int64 `tfsdk:"max_backoff"` + APIClientLogging types.Bool `tfsdk:"api_client_logging"` + APIHostname types.String `tfsdk:"api_hostname"` + UserAgentOperatorSuffix types.String `tfsdk:"user_agent_operator_suffix"` } func (p *CloudflareProvider) Metadata(ctx context.Context, req provider.MetadataRequest, resp *provider.MetadataResponse) { @@ -138,6 +138,11 @@ func (p *CloudflareProvider) Schema(ctx context.Context, req provider.SchemaRequ Optional: true, MarkdownDescription: fmt.Sprintf("Configure the base path used by the API client. Alternatively, can be configured using the `%s` environment variable.", consts.APIBasePathEnvVarKey), }, + + consts.UserAgentOperatorSuffixSchemaKey: schema.StringAttribute{ + Optional: true, + MarkdownDescription: fmt.Sprintf("A value to append to the HTTP User Agent for all API calls. This value is not something most users need to modify however, if you are using a non-standard provider or operator configuration, this is recommended to assist in uniquely identifying your traffic. **Setting this value will remove the Terraform version from the HTTP User Agent string and may have unintended consequences**. Alternatively, can be configured using the `%s` environment variable.", consts.UserAgentOperatorSuffixEnvVarKey), + }, }, } } @@ -234,8 +239,16 @@ func (p *CloudflareProvider) Configure(ctx context.Context, req provider.Configu options = append(options, cloudflare.Debug(logging.IsDebugOrHigher())) - ua := fmt.Sprintf(consts.UserAgentDefault, req.TerraformVersion, meta.SDKVersionString(), p.version) - options = append(options, cloudflare.UserAgent(ua)) + userAgentParams := utils.UserAgentBuilderParams{ + ProviderVersion: &p.version, + PluginType: cloudflare.StringPtr("terraform-plugin-framework"), + } + if !data.UserAgentOperatorSuffix.IsNull() { + userAgentParams.OperatorSuffix = cloudflare.StringPtr(data.UserAgentOperatorSuffix.String()) + } else { + userAgentParams.TerraformVersion = cloudflare.StringPtr(req.TerraformVersion) + } + options = append(options, cloudflare.UserAgent(userAgentParams.String())) config := Config{Options: options} diff --git a/internal/sdkv2provider/provider.go b/internal/sdkv2provider/provider.go index 04a6afd0c9..6547a1bcfc 100644 --- a/internal/sdkv2provider/provider.go +++ b/internal/sdkv2provider/provider.go @@ -151,6 +151,12 @@ func New(version string) func() *schema.Provider { Optional: true, Description: fmt.Sprintf("Configure the base path used by the API client. Alternatively, can be configured using the `%s` environment variable.", consts.APIBasePathEnvVarKey), }, + + consts.UserAgentOperatorSuffixSchemaKey: { + Type: schema.TypeString, + Optional: true, + Description: fmt.Sprintf("A value to append to the HTTP User Agent for all API calls. This value is not something most users need to modify however, if you are using a non-standard provider or operator configuration, this is recommended to assist in uniquely identifying your traffic. **Setting this value will remove the Terraform version from the HTTP User Agent string and may have unintended consequences**. Alternatively, can be configured using the `%s` environment variable.", consts.UserAgentOperatorSuffixEnvVarKey), + }, }, DataSourcesMap: map[string]*schema.Resource{ @@ -377,8 +383,17 @@ func configure(version string, p *schema.Provider) func(context.Context, *schema options = append(options, cloudflare.Debug(logging.IsDebugOrHigher())) - ua := fmt.Sprintf(consts.UserAgentDefault, p.TerraformVersion, meta.SDKVersionString(), version) - options = append(options, cloudflare.UserAgent(ua)) + userAgentParams := utils.UserAgentBuilderParams{ + ProviderVersion: cloudflare.StringPtr(version), + PluginType: cloudflare.StringPtr("terraform-plugin-sdk"), + PluginVersion: cloudflare.StringPtr(meta.SDKVersionString()), + } + if v, ok := d.GetOk(consts.UserAgentOperatorSuffixSchemaKey); ok { + userAgentParams.OperatorSuffix = cloudflare.StringPtr(v.(string)) + } else { + userAgentParams.TerraformVersion = cloudflare.StringPtr(p.TerraformVersion) + } + options = append(options, cloudflare.UserAgent(userAgentParams.String())) config := Config{Options: options} From ac886bd97caf71e979823d9afc0a0e1507909b54 Mon Sep 17 00:00:00 2001 From: Jacob Bednarz Date: Wed, 11 Oct 2023 13:14:51 +1100 Subject: [PATCH 3/7] `make docs` --- docs/index.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/index.md b/docs/index.md index 428c50f725..88ac5f9c61 100644 --- a/docs/index.md +++ b/docs/index.md @@ -64,3 +64,4 @@ resource "cloudflare_page_rule" "www" { - `min_backoff` (Number) Minimum backoff period in seconds after failed API calls. Alternatively, can be configured using the `CLOUDFLARE_MIN_BACKOFF` environment variable. - `retries` (Number) Maximum number of retries to perform when an API request fails. Alternatively, can be configured using the `CLOUDFLARE_RETRIES` environment variable. - `rps` (Number) RPS limit to apply when making calls to the API. Alternatively, can be configured using the `CLOUDFLARE_RPS` environment variable. +- `user_agent_operator_suffix` (String) A value to append to the HTTP User Agent for all API calls. This value is not something most users need to modify however, if you are using a non-standard provider or operator configuration, this is recommended to assist in uniquely identifying your traffic. **Setting this value will remove the Terraform version from the HTTP User Agent string and may have unintended consequences**. Alternatively, can be configured using the `CLOUDFLARE_USER_AGENT_OPERATOR_SUFFIX` environment variable. From dc00571fc5f13cd69c1dbecd06c1732a1295ddd2 Mon Sep 17 00:00:00 2001 From: Jacob Bednarz Date: Wed, 11 Oct 2023 13:21:58 +1100 Subject: [PATCH 4/7] add changelog entry --- .changelog/2831.txt | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 .changelog/2831.txt diff --git a/.changelog/2831.txt b/.changelog/2831.txt new file mode 100644 index 0000000000..7219778549 --- /dev/null +++ b/.changelog/2831.txt @@ -0,0 +1,7 @@ +```release-note:internal +provider: updated user agent string to now be `terraform-provider-cloudflare/ ` +``` + +```release-note:enhancement +provider: allow defining a user agent operator suffix through the schema field (`user_agent_operator_suffix`) and via the environment variable (`CLOUDFLARE_USER_AGENT_OPERATOR_SUFFIX`) +``` From dcbf049bfc0c689f2d45ba6ac1d36a1d9613475a Mon Sep 17 00:00:00 2001 From: Jacob Bednarz Date: Wed, 11 Oct 2023 13:59:37 +1100 Subject: [PATCH 5/7] add trailing period to lint --- internal/consts/provider.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/consts/provider.go b/internal/consts/provider.go index 4a3b3643af..867bd9d51a 100644 --- a/internal/consts/provider.go +++ b/internal/consts/provider.go @@ -46,7 +46,7 @@ const ( // Schema key for the User Agent operator suffix. UserAgentOperatorSuffixSchemaKey = "user_agent_operator_suffix" - // Environment variable key for the User Agent operator suffix + // Environment variable key for the User Agent operator suffix. UserAgentOperatorSuffixEnvVarKey = "CLOUDFLARE_USER_AGENT_OPERATOR_SUFFIX" // Schema key for the requests per second configuration. From b68b7891b77d483eae1a2f2769513549d50fd7b5 Mon Sep 17 00:00:00 2001 From: Jacob Bednarz Date: Wed, 11 Oct 2023 14:00:11 +1100 Subject: [PATCH 6/7] remove unused constant for UA --- internal/consts/provider.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/internal/consts/provider.go b/internal/consts/provider.go index 867bd9d51a..a92b4a14a2 100644 --- a/internal/consts/provider.go +++ b/internal/consts/provider.go @@ -93,8 +93,6 @@ const ( // Deprecated: Use resource specific account ID values instead. AccountIDEnvVarKey = "CLOUDFLARE_ACCOUNT_ID" - UserAgentDefault = "terraform-provider-cloudflare/%s terraform-plugin-%s/%s" - // Schema key for the account ID configuration. AccountIDSchemaKey = "account_id" From 39e9d6a9a398c74164502d4cff7388bd7a60c91f Mon Sep 17 00:00:00 2001 From: Jacob Bednarz Date: Thu, 12 Oct 2023 13:49:39 +1100 Subject: [PATCH 7/7] provider: introduce `FindGoModuleVersion` helper for module versions Before this change, we relied on the plugin libraries to expose their version. Unfortunately, this was incorrectly set in SDKv2[1] and non-existent in the framework[2]. The recommended approach is to instead dig into the build information via`runtime/debug.ReadBuildInfo()`[3] which is what we introduce here in a slightly safer and consistent way. [1]: https://github.com/hashicorp/terraform-plugin-sdk/issues/1257 [2]: https://github.com/hashicorp/terraform-plugin-framework/issues/855 [3]: https://pkg.go.dev/runtime/debug#ReadBuildInfo Signed-off-by: Jacob Bednarz --- internal/framework/provider/provider.go | 2 ++ internal/sdkv2provider/provider.go | 4 +-- internal/utils/get_go_module_version.go | 33 +++++++++++++++++++++++++ internal/utils/user_agent_builder.go | 23 ++++++++++++++--- 4 files changed, 56 insertions(+), 6 deletions(-) create mode 100644 internal/utils/get_go_module_version.go diff --git a/internal/framework/provider/provider.go b/internal/framework/provider/provider.go index ed3b9fc5bd..3fd68a7793 100644 --- a/internal/framework/provider/provider.go +++ b/internal/framework/provider/provider.go @@ -239,9 +239,11 @@ func (p *CloudflareProvider) Configure(ctx context.Context, req provider.Configu options = append(options, cloudflare.Debug(logging.IsDebugOrHigher())) + pluginVersion := utils.FindGoModuleVersion("github.com/hashicorp/terraform-plugin-framework") userAgentParams := utils.UserAgentBuilderParams{ ProviderVersion: &p.version, PluginType: cloudflare.StringPtr("terraform-plugin-framework"), + PluginVersion: pluginVersion, } if !data.UserAgentOperatorSuffix.IsNull() { userAgentParams.OperatorSuffix = cloudflare.StringPtr(data.UserAgentOperatorSuffix.String()) diff --git a/internal/sdkv2provider/provider.go b/internal/sdkv2provider/provider.go index 6547a1bcfc..5a032ac858 100644 --- a/internal/sdkv2provider/provider.go +++ b/internal/sdkv2provider/provider.go @@ -16,7 +16,6 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/helper/logging" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" - "github.com/hashicorp/terraform-plugin-sdk/v2/meta" ) func init() { @@ -383,10 +382,11 @@ func configure(version string, p *schema.Provider) func(context.Context, *schema options = append(options, cloudflare.Debug(logging.IsDebugOrHigher())) + pluginVersion := utils.FindGoModuleVersion("github.com/hashicorp/terraform-plugin-sdk/v2") userAgentParams := utils.UserAgentBuilderParams{ ProviderVersion: cloudflare.StringPtr(version), PluginType: cloudflare.StringPtr("terraform-plugin-sdk"), - PluginVersion: cloudflare.StringPtr(meta.SDKVersionString()), + PluginVersion: pluginVersion, } if v, ok := d.GetOk(consts.UserAgentOperatorSuffixSchemaKey); ok { userAgentParams.OperatorSuffix = cloudflare.StringPtr(v.(string)) diff --git a/internal/utils/get_go_module_version.go b/internal/utils/get_go_module_version.go new file mode 100644 index 0000000000..d2c1c94e60 --- /dev/null +++ b/internal/utils/get_go_module_version.go @@ -0,0 +1,33 @@ +package utils + +import ( + "runtime/debug" + "strings" + + "github.com/cloudflare/cloudflare-go" +) + +// FindGoModuleVersion digs into the build information and extracts the version +// of a module for use without the prefixed `v` (should it exist). +func FindGoModuleVersion(modulePath string) *string { + info, ok := debug.ReadBuildInfo() + if !ok { + // shouldn't ever happen but just in case we aren't using modules + return nil + } + + for _, mod := range info.Deps { + if mod.Path != modulePath { + continue + } + + version := mod.Version + if strings.HasPrefix(version, "v") { + version = strings.TrimPrefix(version, "v") + } + + return cloudflare.StringPtr(version) + } + + return nil +} diff --git a/internal/utils/user_agent_builder.go b/internal/utils/user_agent_builder.go index b2ef61fe2f..2daf9b2927 100644 --- a/internal/utils/user_agent_builder.go +++ b/internal/utils/user_agent_builder.go @@ -5,11 +5,24 @@ import ( ) type UserAgentBuilderParams struct { - ProviderVersion *string - PluginVersion *string - PluginType *string + // Version of `terraform-provider-cloudflare`. + ProviderVersion *string + + // Version of `terraform-plugin-*` libraries that we rely on for the internal + // operations. + PluginVersion *string + + // Which plugin is in use. Currently only available options are + // `terraform-plugin-sdk` and `terraform-plugin-framework`. + PluginType *string + + // Version of Terraform that is initiating the operation. Mutually exclusive + // with `OperatorSuffix`. TerraformVersion *string - OperatorSuffix *string + + // Customised operation suffix to append to the user agent for identifying + // traffic. Mutually exclusive with `TerraformVersion`. + OperatorSuffix *string } func (p *UserAgentBuilderParams) String() string { @@ -37,6 +50,8 @@ func (p *UserAgentBuilderParams) String() string { return ua } +// BuildUserAgent takes the `UserAgentBuilderParams` and contextually builds +// a HTTP user agent for making API calls. func BuildUserAgent(params UserAgentBuilderParams) string { return params.String() }