fix(issue-25): provider fails for account_id with leading zero

[maunzCache]

Description

An AWS account id is a "number" of 12 digits. However, as it can contain leading zeros e.g. 000000000000 would be a valid AWS account id, this must not be treated as an number value inside code but rather as a string type.

During usage of the awsteam provider, this constraint failed at least two times for the resources:

• awsteam_approvers_account
• awsteam_eligibility_group

Both resources were written to ignore this fact. Thus I replaced it with a fitting logic.

Relations

Closes #25
Closes #27

[brittandeyoung]

@maunzCache Thank you for your contribution.

I cannot merge this PR as is. This would cause a breaking change for the provider as it would change the type for a field from int to string.

We will need to find a way to validate this field as an int64 field. Maybe we just check to make sure that the number is not longer than 12?

Is this something you would like to work on an pose another fix?

[jarrod-mg]

FYI, AWS Account IDs are always exactly 12 digits - https://docs.aws.amazon.com/accounts/latest/reference/manage-acct-identifiers.html . "012345678912" is a valid account ID (except for the fact that it is reserved for documentation!). "12345678912" is not.

[maunzCache]

I cannot think of another way but a breaking change. The type is just wrong and it needs to be changed.

We could do a workaround where numbers with less than 12 digits get filled up with a zero internally on the left side when being used as a string. But that is a complicated mess. Anyways, the int64 value is never used as far as i know but parsed back as a string anyway. I think it would be easier just to release a major version to signal the breaking change.

Edit: I thought about it a bit more. The walkaround would introduce a new bug where typos are autocorrected with leading zeros. Honestly, i don't see an alternative to breaking it aka fixing it because using int64 is a bug.

[brittandeyoung]

I think you may be right @maunzCache. I will work on getting this change merged and prepare the major release.

[maunzCache]

I just noticed i forgot to run the generate for the api change. Will fix that asap.sorry for the inconvenience

[brittandeyoung]

@maunzCache I have update the rest of the project to support moving account_id to a string. I will be merging this and preparing a major release. This will release with version 1.0.0

[brittandeyoung]

LGTM 🚀

make testacc                                                                                                                                                               
TF_ACC=1 go test ./... -v "-run=TestAcc" -timeout 120m
?       github.com/brittandeyoung/terraform-provider-awsteam    [no test files]
?       github.com/brittandeyoung/terraform-provider-awsteam/internal/acctest   [no test files]
?       github.com/brittandeyoung/terraform-provider-awsteam/internal/envvar    [no test files]
?       github.com/brittandeyoung/terraform-provider-awsteam/internal/names     [no test files]
?       github.com/brittandeyoung/terraform-provider-awsteam/internal/sdk/awsteam       [no test files]
=== RUN   TestAccApproversAccountResource_basic
--- PASS: TestAccApproversAccountResource_basic (6.05s)
=== RUN   TestAccApproversAccountResource_accountId
--- PASS: TestAccApproversAccountResource_accountId (2.68s)
=== RUN   TestAccApproversOUResource_basic
--- PASS: TestAccApproversOUResource_basic (5.08s)
=== RUN   TestAccEligibilityGroupResource_basic
--- PASS: TestAccEligibilityGroupResource_basic (5.82s)
=== RUN   TestAccEligibilityUserResource_basic
--- PASS: TestAccEligibilityUserResource_basic (3.72s)
=== RUN   TestAccSettings_serial
=== PAUSE TestAccSettings_serial
=== CONT  TestAccSettings_serial
    settings_test.go:23: Skipping Settings Tests, Environment variable AWSTEAM_RUN_SETTINGS_TESTS is not set to true
--- SKIP: TestAccSettings_serial (0.00s)
PASS
ok      github.com/brittandeyoung/terraform-provider-awsteam/internal/provider  23.928s