diff --git a/go.mod b/go.mod index a95e19f7d..ce32395c6 100644 --- a/go.mod +++ b/go.mod @@ -60,7 +60,7 @@ require ( github.com/go-openapi/validate v0.24.0 // indirect github.com/go-test/deep v1.1.0 // indirect github.com/golang/protobuf v1.5.4 // indirect - github.com/google/go-cmp v0.6.0 // indirect + github.com/google/go-cmp v0.6.0 github.com/hashicorp/errwrap v1.1.0 // indirect github.com/hashicorp/go-checkpoint v0.5.0 // indirect github.com/hashicorp/go-cleanhttp v0.5.2 // indirect diff --git a/internal/clients/client.go b/internal/clients/client.go index 67f8ab33e..4a7d208e4 100644 --- a/internal/clients/client.go +++ b/internal/clients/client.go @@ -121,57 +121,14 @@ type ClientConfig struct { // NewClient creates a new Client that is capable of making HCP requests func NewClient(config ClientConfig) (*Client, error) { - // hasWorkloadIdentityToken is true if the config specified a direct token. - hasWorkloadIdentityToken := config.WorkloadIdentityToken != "" - // hasWorkloadIdentityTokenFile is true if the config specified a path to a - // file that contains the token. - hasWorkloadIdentityTokenFile := config.WorkloadIdentityTokenFile != "" - // hasWorkloadIdentityResource is true if the config specified a resource - // name to authenticate against. - hasWorkloadIdentityResource := config.WorkloadIdentityResourceName != "" - - // Overall, we consider workload identity authentication to be enabled if we - // have a token from either source (direct or within a file) and a resource - // name. - hasWorkloadIdentity := (hasWorkloadIdentityToken || hasWorkloadIdentityTokenFile) && hasWorkloadIdentityResource - // Build the HCP Config options opts := []hcpConfig.HCPConfigOption{hcpConfig.FromEnv()} if config.ClientID != "" && config.ClientSecret != "" { opts = append(opts, hcpConfig.WithClientCredentials(config.ClientID, config.ClientSecret)) } else if config.CredentialFile != "" { opts = append(opts, hcpConfig.WithCredentialFilePath(config.CredentialFile)) - } else if hasWorkloadIdentity { - switch { - case hasWorkloadIdentityToken: - // The direct token takes priority over the file path in case both - // are set, so we'll check that first. - cf := &auth.CredentialFile{ - Scheme: auth.CredentialFileSchemeWorkload, - Workload: &workload.IdentityProviderConfig{ - ProviderResourceName: config.WorkloadIdentityResourceName, - Token: &workload.CredentialTokenSource{ - Token: config.WorkloadIdentityToken, - }, - }, - } - opts = append(opts, hcpConfig.WithCredentialFile(cf)) - default: - // If we don't have the token directly, fall back to checking the - // file. We checked earlier that at least one of the two options - // were set, so if the token wasn't set the file information must - // be present. - cf := &auth.CredentialFile{ - Scheme: auth.CredentialFileSchemeWorkload, - Workload: &workload.IdentityProviderConfig{ - ProviderResourceName: config.WorkloadIdentityResourceName, - File: &workload.FileCredentialSource{ - Path: config.WorkloadIdentityTokenFile, - }, - }, - } - opts = append(opts, hcpConfig.WithCredentialFile(cf)) - } + } else if cf := loadCredentialFile(config); cf != nil { + opts = append(opts, hcpConfig.WithCredentialFile(cf)) } // Create the HCP Config @@ -224,6 +181,58 @@ func NewClient(config ClientConfig) (*Client, error) { return client, nil } +// loadCredentialFile loads the credential file from the given config. If the +// config does not specify workload identity authentication, this function +// returns nil. +func loadCredentialFile(config ClientConfig) *auth.CredentialFile { + // hasWorkloadIdentityToken is true if the config specified a direct token. + hasWorkloadIdentityToken := config.WorkloadIdentityToken != "" + // hasWorkloadIdentityTokenFile is true if the config specified a path to a + // file that contains the token. + hasWorkloadIdentityTokenFile := config.WorkloadIdentityTokenFile != "" + // hasWorkloadIdentityResource is true if the config specified a resource + // name to authenticate against. + hasWorkloadIdentityResource := config.WorkloadIdentityResourceName != "" + + // Overall, we consider workload identity authentication to be enabled if we + // have a token from either source (direct or within a file) and a resource + // name. + hasWorkloadIdentity := (hasWorkloadIdentityToken || hasWorkloadIdentityTokenFile) && hasWorkloadIdentityResource + + if !hasWorkloadIdentity { + return nil + } + + switch { + case hasWorkloadIdentityToken: + // The direct token takes priority over the file path in case both + // are set, so we'll check that first. + return &auth.CredentialFile{ + Scheme: auth.CredentialFileSchemeWorkload, + Workload: &workload.IdentityProviderConfig{ + ProviderResourceName: config.WorkloadIdentityResourceName, + Token: &workload.CredentialTokenSource{ + Token: config.WorkloadIdentityToken, + }, + }, + } + default: + // If we don't have the token directly, fall back to checking the + // file. We checked earlier that at least one of the two options + // were set, so if the token wasn't set the file information must + // be present. + return &auth.CredentialFile{ + Scheme: auth.CredentialFileSchemeWorkload, + Workload: &workload.IdentityProviderConfig{ + ProviderResourceName: config.WorkloadIdentityResourceName, + File: &workload.FileCredentialSource{ + Path: config.WorkloadIdentityTokenFile, + }, + }, + } + } +} + type providerMeta struct { ModuleName string `cty:"module_name"` } diff --git a/internal/clients/client_test.go b/internal/clients/client_test.go new file mode 100644 index 000000000..2d53fd68e --- /dev/null +++ b/internal/clients/client_test.go @@ -0,0 +1,81 @@ +package clients + +import ( + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/hashicorp/hcp-sdk-go/auth" + "github.com/hashicorp/hcp-sdk-go/auth/workload" +) + +func Test_loadCredentialFile(t *testing.T) { + tcs := map[string]struct { + config ClientConfig + want *auth.CredentialFile + }{ + "empty config": { + config: ClientConfig{}, + }, + "ignores with only resource": { + config: ClientConfig{ + WorkloadIdentityResourceName: "my_resource", + }, + }, + "loads with file": { + config: ClientConfig{ + WorkloadIdentityResourceName: "my_resource", + WorkloadIdentityTokenFile: "/path/to/token/file", + }, + want: &auth.CredentialFile{ + Scheme: auth.CredentialFileSchemeWorkload, + Workload: &workload.IdentityProviderConfig{ + ProviderResourceName: "my_resource", + File: &workload.FileCredentialSource{ + Path: "/path/to/token/file", + }, + }, + }, + }, + "loads with token": { + config: ClientConfig{ + WorkloadIdentityResourceName: "my_resource", + WorkloadIdentityToken: "my_token", + }, + want: &auth.CredentialFile{ + Scheme: auth.CredentialFileSchemeWorkload, + Workload: &workload.IdentityProviderConfig{ + ProviderResourceName: "my_resource", + Token: &workload.CredentialTokenSource{ + Token: "my_token", + }, + }, + }, + }, + "defaults to token": { + config: ClientConfig{ + WorkloadIdentityResourceName: "my_resource", + WorkloadIdentityTokenFile: "/path/to/token/file", + WorkloadIdentityToken: "my_token", + }, + want: &auth.CredentialFile{ + Scheme: auth.CredentialFileSchemeWorkload, + Workload: &workload.IdentityProviderConfig{ + ProviderResourceName: "my_resource", + Token: &workload.CredentialTokenSource{ + Token: "my_token", + }, + }, + }, + }, + } + + for name, tc := range tcs { + t.Run(name, func(t *testing.T) { + got := loadCredentialFile(tc.config) + if cmp.Diff(tc.want, got) != "" { + t.Errorf("mismatch (-want +got): %s", cmp.Diff(tc.want, got)) + } + }) + } + +} diff --git a/internal/provider/provider.go b/internal/provider/provider.go index c4d03da7d..722975008 100644 --- a/internal/provider/provider.go +++ b/internal/provider/provider.go @@ -13,6 +13,7 @@ import ( "github.com/hashicorp/terraform-plugin-framework-validators/listvalidator" "github.com/hashicorp/terraform-plugin-framework-validators/stringvalidator" "github.com/hashicorp/terraform-plugin-framework/datasource" + "github.com/hashicorp/terraform-plugin-framework/diag" "github.com/hashicorp/terraform-plugin-framework/path" "github.com/hashicorp/terraform-plugin-framework/provider" "github.com/hashicorp/terraform-plugin-framework/provider/schema" @@ -235,14 +236,10 @@ func (p *ProviderFramework) Configure(ctx context.Context, req provider.Configur return } - clientConfig.WorkloadIdentityTokenFile = elements[0].TokenFile.ValueString() - clientConfig.WorkloadIdentityToken = elements[0].Token.ValueString() - clientConfig.WorkloadIdentityResourceName = elements[0].ResourceName.ValueString() - - // This should have been validated by the schema, but we'll check it - // here just in case. - if clientConfig.WorkloadIdentityTokenFile == "" && clientConfig.WorkloadIdentityToken == "" { - resp.Diagnostics.AddError("invalid workload_identity", "at least one of `token_file` or `token` must be set") + var diags diag.Diagnostics + clientConfig, diags = readWorkloadIdentity(elements[0], clientConfig) + resp.Diagnostics.Append(diags...) + if diags.HasError() { return } } @@ -295,3 +292,17 @@ func (p *ProviderFramework) Configure(ctx context.Context, req provider.Configur resp.DataSourceData = client resp.ResourceData = client } + +func readWorkloadIdentity(model WorkloadIdentityFrameworkModel, clientConfig clients.ClientConfig) (clients.ClientConfig, diag.Diagnostics) { + clientConfig.WorkloadIdentityTokenFile = model.TokenFile.ValueString() + clientConfig.WorkloadIdentityToken = model.Token.ValueString() + clientConfig.WorkloadIdentityResourceName = model.ResourceName.ValueString() + + // This should have been validated by the schema, but we'll check it + // here just in case. + var diags diag.Diagnostics + if clientConfig.WorkloadIdentityTokenFile == "" && clientConfig.WorkloadIdentityToken == "" { + diags.AddError("invalid workload_identity", "at least one of `token_file` or `token` must be set") + } + return clientConfig, diags +} diff --git a/internal/provider/provider_test.go b/internal/provider/provider_test.go new file mode 100644 index 000000000..8c3b32ad7 --- /dev/null +++ b/internal/provider/provider_test.go @@ -0,0 +1,74 @@ +package provider + +import ( + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/hashicorp/terraform-plugin-framework/diag" + "github.com/hashicorp/terraform-plugin-framework/types/basetypes" + + "github.com/hashicorp/terraform-provider-hcp/internal/clients" +) + +func Test_readWorkloadIdentity(t *testing.T) { + tcs := map[string]struct { + model WorkloadIdentityFrameworkModel + want clients.ClientConfig + wantDiags diag.Diagnostics + }{ + "missing token and file": { + model: WorkloadIdentityFrameworkModel{ + ResourceName: basetypes.NewStringValue("my_resource"), + }, + wantDiags: diag.Diagnostics{ + diag.NewErrorDiagnostic("invalid workload_identity", "at least one of `token_file` or `token` must be set"), + }, + want: clients.ClientConfig{ + WorkloadIdentityResourceName: "my_resource", + }, + }, + "token": { + model: WorkloadIdentityFrameworkModel{ + ResourceName: basetypes.NewStringValue("my_resource"), + Token: basetypes.NewStringValue("my_token"), + }, + want: clients.ClientConfig{ + WorkloadIdentityResourceName: "my_resource", + WorkloadIdentityToken: "my_token", + }, + }, + "file": { + model: WorkloadIdentityFrameworkModel{ + ResourceName: basetypes.NewStringValue("my_resource"), + TokenFile: basetypes.NewStringValue("/path/to/token/file"), + }, + want: clients.ClientConfig{ + WorkloadIdentityResourceName: "my_resource", + WorkloadIdentityTokenFile: "/path/to/token/file", + }, + }, + "both": { + model: WorkloadIdentityFrameworkModel{ + ResourceName: basetypes.NewStringValue("my_resource"), + Token: basetypes.NewStringValue("my_token"), + TokenFile: basetypes.NewStringValue("/path/to/token/file"), + }, + want: clients.ClientConfig{ + WorkloadIdentityResourceName: "my_resource", + WorkloadIdentityToken: "my_token", + WorkloadIdentityTokenFile: "/path/to/token/file", + }, + }, + } + for name, tc := range tcs { + t.Run(name, func(t *testing.T) { + got, gotDiags := readWorkloadIdentity(tc.model, clients.ClientConfig{}) + if diff := cmp.Diff(tc.want, got); diff != "" { + t.Errorf("unexpected result (-want +got):\n%s", diff) + } + if diff := cmp.Diff(tc.wantDiags, gotDiags); diff != "" { + t.Errorf("unexpected diagnostics (-want +got):\n%s", diff) + } + }) + } +} diff --git a/internal/providersdkv2/provider.go b/internal/providersdkv2/provider.go index 0f3c28890..a198a7ad4 100644 --- a/internal/providersdkv2/provider.go +++ b/internal/providersdkv2/provider.go @@ -145,25 +145,11 @@ func configure(p *schema.Provider) func(context.Context, *schema.ResourceData) ( } // Read the workload_identity configuration - if v, ok := d.GetOk("workload_identity"); ok && len(v.([]interface{})) == 1 && v.([]interface{})[0] != nil { - wi := v.([]interface{})[0].(map[string]interface{}) - if tf, ok := wi["token_file"].(string); ok && tf != "" { - clientConfig.WorkloadIdentityTokenFile = tf - } - if t, ok := wi["token"].(string); ok && t != "" { - clientConfig.WorkloadIdentityToken = t - } - if rn, ok := wi["resource_name"].(string); ok && rn != "" { - clientConfig.WorkloadIdentityResourceName = rn - } - - if clientConfig.WorkloadIdentityTokenFile == "" && clientConfig.WorkloadIdentityToken == "" { - diags = append(diags, diag.Diagnostic{ - Severity: diag.Error, - Summary: "invalid workload_identity", - Detail: "at least of one of `token_file` or `token` must be set", - AttributePath: cty.GetAttrPath("workload_identity"), - }) + if d, ok := d.GetOk("workload_identity"); ok { + var moreDiags diag.Diagnostics + clientConfig, moreDiags = readWorkloadIdentity(d, clientConfig) + diags = append(diags, moreDiags...) + if moreDiags.HasError() { return nil, diags } } @@ -216,6 +202,32 @@ func configure(p *schema.Provider) func(context.Context, *schema.ResourceData) ( } } +func readWorkloadIdentity(v interface{}, clientConfig clients.ClientConfig) (clients.ClientConfig, diag.Diagnostics) { + var diags diag.Diagnostics + if len(v.([]interface{})) == 1 && v.([]interface{})[0] != nil { + wi := v.([]interface{})[0].(map[string]interface{}) + if tf, ok := wi["token_file"].(string); ok && tf != "" { + clientConfig.WorkloadIdentityTokenFile = tf + } + if t, ok := wi["token"].(string); ok && t != "" { + clientConfig.WorkloadIdentityToken = t + } + if rn, ok := wi["resource_name"].(string); ok && rn != "" { + clientConfig.WorkloadIdentityResourceName = rn + } + + if clientConfig.WorkloadIdentityTokenFile == "" && clientConfig.WorkloadIdentityToken == "" { + diags = append(diags, diag.Diagnostic{ + Severity: diag.Error, + Summary: "invalid workload_identity", + Detail: "at least one of `token_file` or `token` must be set", + AttributePath: cty.GetAttrPath("workload_identity"), + }) + } + } + return clientConfig, diags +} + // getProjectFromCredentials uses the configured client credentials to // fetch the associated organization and returns that organization's // single project. diff --git a/internal/providersdkv2/provider_test.go b/internal/providersdkv2/provider_test.go index 2172d9714..27dabae13 100644 --- a/internal/providersdkv2/provider_test.go +++ b/internal/providersdkv2/provider_test.go @@ -10,12 +10,17 @@ import ( "sync" "testing" + "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" "github.com/hashicorp/terraform-plugin-framework/providerserver" "github.com/hashicorp/terraform-plugin-go/tfprotov6" "github.com/hashicorp/terraform-plugin-mux/tf5to6server" "github.com/hashicorp/terraform-plugin-mux/tf6muxserver" + "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" + + "github.com/hashicorp/terraform-provider-hcp/internal/clients" "github.com/hashicorp/terraform-provider-hcp/internal/provider" "github.com/hashicorp/terraform-provider-hcp/version" ) @@ -142,3 +147,84 @@ func testConfig(res ...string) string { c = append(c, res...) return strings.Join(c, "\n") } + +func Test_readWorkloadIdentity(t *testing.T) { + tcs := map[string]struct { + config interface{} + want clients.ClientConfig + wantDiags diag.Diagnostics + }{ + "missing token and file": { + config: []interface{}{ + map[string]interface{}{ + "resource_name": "my_resource", + }, + }, + wantDiags: diag.Diagnostics{ + diag.Diagnostic{ + Severity: diag.Error, + Summary: "invalid workload_identity", + Detail: "at least one of `token_file` or `token` must be set", + }, + }, + want: clients.ClientConfig{ + WorkloadIdentityResourceName: "my_resource", + }, + }, + "token": { + config: []interface{}{ + map[string]interface{}{ + "resource_name": "my_resource", + "token": "my_token", + }, + }, + want: clients.ClientConfig{ + WorkloadIdentityResourceName: "my_resource", + WorkloadIdentityToken: "my_token", + }, + }, + "file": { + config: []interface{}{ + map[string]interface{}{ + "resource_name": "my_resource", + "token_file": "/path/to/token/file", + }, + }, + want: clients.ClientConfig{ + WorkloadIdentityResourceName: "my_resource", + WorkloadIdentityTokenFile: "/path/to/token/file", + }, + }, + "both": { + config: []interface{}{ + map[string]interface{}{ + "resource_name": "my_resource", + "token": "my_token", + "token_file": "/path/to/token/file", + }, + }, + want: clients.ClientConfig{ + WorkloadIdentityResourceName: "my_resource", + WorkloadIdentityToken: "my_token", + WorkloadIdentityTokenFile: "/path/to/token/file", + }, + }, + } + + for name, tc := range tcs { + t.Run(name, func(t *testing.T) { + // cty.Path objects are difficult to compare automatically as they + // contain unexported fields. We can ignore them for the purposes of + // this test. + ignoreAttributePath := cmpopts.IgnoreFields(diag.Diagnostic{}, "AttributePath") + + got, gotDiags := readWorkloadIdentity(tc.config, clients.ClientConfig{}) + if diff := cmp.Diff(tc.wantDiags, gotDiags, ignoreAttributePath); diff != "" { + t.Errorf("mismatch (-want +got):\n%s", diff) + } + if diff := cmp.Diff(tc.want, got); diff != "" { + t.Errorf("mismatch (-want +got):\n%s", diff) + } + }) + } +}