Skip to content

Commit

Permalink
fix: generic provider format (#303)
Browse files Browse the repository at this point in the history
* fix: generic provider roleARN format

* fix: generic provider missing region

* chore: Add docs and fix testdata

* chore: Refactor generic provider assume_role map support

- Align closer to TF assume role block to expect map in both cases
- Add genericProvider validation to component validation
- Add tests for genericProvider validation function

* chore: don't override region if provided

---------

Co-authored-by: Vincent De Smet <[email protected]>
  • Loading branch information
zackhee997 and vincenthsh authored May 21, 2024
1 parent 095fd29 commit bb88bac
Show file tree
Hide file tree
Showing 7 changed files with 151 additions and 10 deletions.
27 changes: 22 additions & 5 deletions config/v2/resolvers.go
Original file line number Diff line number Diff line change
Expand Up @@ -810,19 +810,36 @@ func resolveGenericProvider(
if p.Enabled != nil {
enabled = *p.Enabled
}
// special assume role config block handling
// if assume_role map is provided, add region
requiresRegion := false
for key, value := range p.Config {
if value == nil {
delete(config, key)
} else {
// specially for AWS associate assume role
if key == "assume_role" {
tmp := fmt.Sprintf("arn:aws:iam::%s:role/%s", *awsConfig.AccountID, value)
config["assume_role"] = tmp
} else {
switch key {
case "assume_role":
// build assume_role_block
// ref: https://registry.terraform.io/providers/hashicorp/awscc/latest/docs#assume-role
// ValidateAWSProvider should ensure AccountID is not nil
// GenericProvider.Validate ensures can cast and "role" key exists
valueMap := value.(map[string]any)
if valueMap["role_arn"] == nil {
valueMap["role_arn"] = fmt.Sprintf("arn:aws:iam::%s:role/%s", *awsConfig.AccountID, valueMap["role"])
delete(valueMap, "role")
}
config[key] = valueMap
requiresRegion = true
default:
config[key] = value
}
}
}
if requiresRegion && config["region"] == nil {
// inherit resolved awsConfig region and accountID
// TODO: handle additionalRegions/additionalProvider configuration?
config["region"] = *awsConfig.Region
}
return source, customProvider, version, enabled
}

Expand Down
14 changes: 12 additions & 2 deletions config/v2/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ func (c *Config) Validate() ([]string, error) {
errs = multierror.Append(errs, c.ValidateSnowflakeProviders())
errs = multierror.Append(errs, c.ValidateBlessProviders())
errs = multierror.Append(errs, c.ValidateOktaProviders())
errs = multierror.Append(errs, c.ValidateGenericProviders())
errs = multierror.Append(errs, c.validateModules())
errs = multierror.Append(errs, c.ValidateTravis())
errs = multierror.Append(errs, c.ValidateGithubActionsCI())
Expand Down Expand Up @@ -200,7 +201,16 @@ func (p *GenericProvider) Validate(name string, component string) error {
if p.Version == nil || len(*p.Version) == 0 {
errs = multierror.Append(errs, fmt.Errorf("required provider %q requires version in %s", name, component))
}
return errs
if p.Config["assume_role"] != nil {
// "role" key must be provided in assume_role map
valueMap, ok := p.Config["assume_role"].(map[string]any)
if !ok {
errs = multierror.Append(errs, fmt.Errorf("required provider %q requires 'assume_role' to be map (cast failed) in %s", name, component))
} else if valueMap["role"] == nil && valueMap["role_arn"] == nil {
errs = multierror.Append(errs, fmt.Errorf("required provider %q requires 'assume_role' map to contain at least 'role' key in %s (received: %v)", name, component, valueMap))
}
}
return errs.ErrorOrNil()
}

func (p *SnowflakeProvider) Validate(component string) error {
Expand Down Expand Up @@ -277,7 +287,7 @@ func (c *Config) ValidateGenericProviders() error {
}
}
})
return errs
return errs.ErrorOrNil()
}

func (c *Config) ValidateTravis() error {
Expand Down
76 changes: 76 additions & 0 deletions config/v2/validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,82 @@ func TestValidateAWSProvider(t *testing.T) {
}
}

func TestValidateGenericProvider(t *testing.T) {
validGeneric := &GenericProvider{
Source: "foo",
CommonProvider: CommonProvider{
Version: util.StrPtr("1.1.1"),
},
}
validAssumeRole := &GenericProvider{
Source: "foo",
CommonProvider: CommonProvider{
Version: util.StrPtr("1.1.1"),
},
Config: map[string]any{
"assume_role": map[string]any{
"role": "foo",
},
},
}

invalidNothing := &GenericProvider{}
invalidSource := &GenericProvider{
Source: "",
}
invalidAssumeRoleNothing := &GenericProvider{
Source: "foo",
CommonProvider: CommonProvider{
Version: util.StrPtr("1.1.1"),
},
Config: map[string]any{
"assume_role": map[string]string{},
},
}
invalidAssumeRoleConfig := &GenericProvider{
Source: "foo",
CommonProvider: CommonProvider{
Version: util.StrPtr("1.1.1"),
},
Config: map[string]any{
"assume_role": map[string]string{
"foo": "bar",
},
},
}

type args struct {
p *GenericProvider
component string
}
tests := []struct {
name string
args args
wantErr bool
}{
{"valid generic", args{validGeneric, "valid"}, false},
{"invalid generic nothing", args{invalidNothing, "invalid"}, true},
{"invalid generic source", args{invalidSource, "invalid"}, true},
{"valid generic assume_role", args{validAssumeRole, "valid-role"}, false},
{"invalid generic assume_role nothing", args{invalidAssumeRoleNothing, "invalid-nothing"}, true},
{"invalid generic assume_role missing role", args{invalidAssumeRoleConfig, "invalid-role-missing"}, true},
}
for _, test := range tests {
tt := test
t.Run(tt.name, func(t *testing.T) {
assert := require.New(t)
err := tt.args.p.Validate("generic", tt.args.component)

if tt.wantErr {
assert.Error(err)
} else {
assert.NoError(err)
}
// t.Errorf("genericProvider.Validate() error = %v, wantErr %v", err, tt.wantErr)
})
}
}

func TestConfig_ValidateAWSProviders(t *testing.T) {
tests := []struct {
fileName string
Expand Down
13 changes: 12 additions & 1 deletion testdata/generic_providers_yaml/fogg.yml
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@ defaults:
bar:
version: "~> 0.1.0"
source: "czi/bar"
fred:
version: "~> 0.1.0"
source: "czi/bar"
qux:
version: "~> 0.1.0"
source: "czi/qux"
Expand All @@ -48,7 +51,15 @@ envs:
custom_provider: false
config:
baz_token: prod_token_arn
aws_assume_role: "TerraformExecutionRole"
assume_role:
role: TerraformExecutionRole
session_name: "foo"
fred:
custom_provider: false
config:
assume_role:
role_arn: arn:aws:iam::1111111111111111:role/TerraformExecutionRole
session_name: "foo"
components:
network: {}
stg:
Expand Down
19 changes: 17 additions & 2 deletions testdata/generic_providers_yaml/terraform/envs/prd/network/fogg.tf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 6 additions & 0 deletions testdata/generic_providers_yaml/terraform/global/fogg.tf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit bb88bac

Please sign in to comment.