service/iam: replace custom SchemaValidateFunc with provided ones

[melamity]

Community Note

• Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
• Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for pull request followers and do not help prioritize the request

Relates: #11872

Release note for CHANGELOG:

NONE

Output from acceptance testing:

--- PASS: TestAccAWSIAMGroup_nameChange (37.66s)
--- PASS: TestAccAWSIAMGroup_basic (41.33s)

--- PASS: TestAccAWSIAMInstanceProfile_withoutRole (22.81s)
--- PASS: TestAccAWSIAMInstanceProfile_basic (26.42s)
--- PASS: TestAccAWSIAMInstanceProfile_withRoleNotRoles (26.49s)
--- PASS: TestAccAWSIAMInstanceProfile_namePrefix (26.62s)

--- PASS: TestAccAWSIAMPolicy_disappears (18.30s)
--- PASS: TestAccAWSIAMPolicy_basic (25.80s)
--- PASS: TestAccAWSIAMPolicy_description (25.93s)
--- PASS: TestAccAWSIAMPolicy_namePrefix (26.13s)
--- PASS: TestAccAWSIAMPolicy_path (26.13s)
--- PASS: TestAccAWSIAMPolicy_policy (43.53s)

--- PASS: TestAccAWSIAMRole_badJSON (2.65s)
--- PASS: TestAccAWSIAMRole_disappears (18.17s)
--- PASS: TestAccAWSIAMRole_basic (24.06s)
--- PASS: TestAccAWSIAMRole_namePrefix (24.35s)
--- PASS: TestAccAWSIAMRole_force_detach_policies (29.42s)
--- PASS: TestAccAWSIAMRole_tags (39.39s)
--- PASS: TestAccAWSIAMRole_MaxSessionDuration (41.98s)
--- PASS: TestAccAWSIAMRole_testNameChange (50.72s)
--- PASS: TestAccAWSIAMRole_basicWithDescription (54.37s)
--- PASS: TestAccAWSIAMRole_PermissionsBoundary (85.03s)

--- PASS: TestAccAWSUser_disappears (18.00s)
--- PASS: TestAccAWSUser_ForceDestroy_LoginProfile (26.15s)
--- PASS: TestAccAWSUser_ForceDestroy_SSHKey (26.69s)
--- PASS: TestAccAWSUser_ForceDestroy_AccessKey (26.97s)
--- PASS: TestAccAWSUser_ForceDestroy_MFADevice (27.28s)
--- PASS: TestAccAWSUser_pathChange (38.76s)
--- PASS: TestAccAWSUser_nameChange (38.98s)
--- PASS: TestAccAWSUser_basic (39.10s)
--- PASS: TestAccAWSUser_tags (41.03s)
--- PASS: TestAccAWSUser_permissionsBoundary (87.66s)

[bflad]

Hi @megubyte 👋 Thank you for working on this! Beyond the simplifications here, implementing these "standard" validation functions will make some upcoming enhancements after Terraform Plugin SDK v2 more smooth so we can enable the plan-time validation to return the exact argument line in the source code.

These changes are looking good for the most part, my main suggestion would be to inline the validation functions where they are used once instead of introducing a wrapping SchemaValidateFunc, e.g.

ValidateFunc: validation.StringMatch(regexp.MustCompile(`^[0-9A-Za-z=,.@\-_+]+$`), "must only contain alphanumeric characters, hyphens, underscores, commas, periods, @ symbols, plus and equals signs"),

When it comes to the schema we prefer to keep things inline so there's less indirection when reading it (and will likely be the way some potential future code generation works as well). 😄 Thanks again.

[melamity]

Hi @bflad! 👋 Thanks for your feedback on my PR - I've gone ahead and inlined the calls to the validation methods and am currently running the acceptance tests again to ensure that I've not done anything silly. 😄

If this is approved, I see no real reason why I can't attempt to round off the entire tech debt issue that is linked to this, submitting one PR per service in cases like IAM where there are multiple resources affected, I took the opportunity of making a change to my commit message and PR title to the same spec which it seems everyone else uses - hope you don't mind!

[melamity]

Updated Acceptance Testing:

--- PASS: TestAccAWSIAMGroup_nameChange (43.81s)
--- PASS: TestAccAWSIAMGroup_basic (44.87s)

--- PASS: TestAccAWSIAMInstanceProfile_withoutRole (27.07s)
--- PASS: TestAccAWSIAMInstanceProfile_basic (30.53s)
--- PASS: TestAccAWSIAMInstanceProfile_withRoleNotRoles (31.05s)
--- PASS: TestAccAWSIAMInstanceProfile_namePrefix (31.07s)

--- PASS: TestAccAWSIAMPolicy_disappears (21.33s)
--- PASS: TestAccAWSIAMPolicy_description (29.71s)
--- PASS: TestAccAWSIAMPolicy_path (30.59s)
--- PASS: TestAccAWSIAMPolicy_namePrefix (30.65s)
--- PASS: TestAccAWSIAMPolicy_basic (31.89s)
--- PASS: TestAccAWSIAMPolicy_policy (50.61s)

--- PASS: TestAccAWSIAMRole_badJSON (3.33s)
--- PASS: TestAccAWSIAMRole_disappears (22.63s)
--- PASS: TestAccAWSIAMRole_namePrefix (30.06s)
--- PASS: TestAccAWSIAMRole_basic (30.74s)
--- PASS: TestAccAWSIAMRole_force_detach_policies (36.42s)
--- PASS: TestAccAWSIAMRole_tags (46.93s)
--- PASS: TestAccAWSIAMRole_MaxSessionDuration (51.21s)
--- PASS: TestAccAWSIAMRole_testNameChange (61.86s)
--- PASS: TestAccAWSIAMRole_basicWithDescription (66.90s)
--- PASS: TestAccAWSIAMRole_PermissionsBoundary (132.94s)

--- PASS: TestAccAWSUser_disappears (23.31s)
--- PASS: TestAccAWSUser_ForceDestroy_AccessKey (34.52s)
--- PASS: TestAccAWSUser_ForceDestroy_LoginProfile (34.52s)
--- PASS: TestAccAWSUser_ForceDestroy_SSHKey (34.65s)
--- PASS: TestAccAWSUser_ForceDestroy_MFADevice (34.97s)
--- PASS: TestAccAWSUser_nameChange (62.57s)
--- PASS: TestAccAWSUser_basic (62.83s)
--- PASS: TestAccAWSUser_pathChange (64.76s)
--- PASS: TestAccAWSUser_tags (64.98s)
--- PASS: TestAccAWSUser_permissionsBoundary (124.60s)

[bflad]

For the CI failures, looks like it just needs make fmt run on the branch and committed. 👍

[melamity]

Thanks for letting me know about that - I hadn't caught that when I committed the last time. I've checked the status of Travis and it looks like all is good this time. 👍 My apologies!

[anGie44]

LGTM 👍 Confirmed output of acceptance tests in TC:

--- PASS: TestAccAWSIAMPolicy_*
--- PASS: TestAccAWSIAMGroupPolicy_*
--- PASS: TestAccAWSIAMInstanceProfile_*
--- PASS: TestAccAWSIAMRolePolicy_*
--- PASS: TestAccAWSIAMGroup_*
--- PASS: TestAccAWSIAMRole_*
--- PASS: TestAccAWSIAMPolicyAttachment_*
--- PASS: TestAccAWSUserSSHKey_*
--- PASS: TestAccAWSIAMRolePolicy_*
--- PASS: TestAccAWSUserLoginProfile_*
--- PASS: TestAccAWSUser_*
--- PASS: TestAccAWSUserGroupMembership_*

[bflad]

LGTM 🚀

Output from acceptance testing:

--- PASS: TestAccAWSIAMGroup_basic (21.97s)
--- PASS: TestAccAWSIAMGroup_nameChange (21.69s)

--- PASS: TestAccAWSIAMGroupPolicy_basic (26.22s)
--- PASS: TestAccAWSIAMGroupPolicy_disappears (13.80s)
--- PASS: TestAccAWSIAMGroupPolicy_generatedName (24.02s)
--- PASS: TestAccAWSIAMGroupPolicy_namePrefix (25.32s)

--- PASS: TestAccAWSIAMGroupPolicyAttachment_basic (32.00s)

--- PASS: TestAccAWSIAMInstanceProfile_basic (21.45s)
--- PASS: TestAccAWSIAMInstanceProfile_namePrefix (19.69s)
--- PASS: TestAccAWSIAMInstanceProfile_withoutRole (15.18s)
--- PASS: TestAccAWSIAMInstanceProfile_withRoleNotRoles (19.70s)

--- PASS: TestAccAWSIAMPolicy_basic (17.82s)
--- PASS: TestAccAWSIAMPolicy_description (18.10s)
--- PASS: TestAccAWSIAMPolicy_disappears (13.14s)
--- PASS: TestAccAWSIAMPolicy_namePrefix (18.02s)
--- PASS: TestAccAWSIAMPolicy_path (14.94s)
--- PASS: TestAccAWSIAMPolicy_policy (25.54s)

--- PASS: TestAccAWSIAMPolicyAttachment_basic (37.86s)
--- PASS: TestAccAWSIAMPolicyAttachment_Groups_RenamedGroup (29.43s)
--- PASS: TestAccAWSIAMPolicyAttachment_paginatedEntities (334.74s)
--- PASS: TestAccAWSIAMPolicyAttachment_Roles_RenamedRole (35.41s)
--- PASS: TestAccAWSIAMPolicyAttachment_Users_RenamedUser (31.87s)

--- PASS: TestAccAWSIAMRole_badJSON (1.31s)
--- PASS: TestAccAWSIAMRole_basic (13.37s)
--- PASS: TestAccAWSIAMRole_basicWithDescription (28.01s)
--- PASS: TestAccAWSIAMRole_disappears (9.92s)
--- PASS: TestAccAWSIAMRole_force_detach_policies (16.90s)
--- PASS: TestAccAWSIAMRole_MaxSessionDuration (22.85s)
--- PASS: TestAccAWSIAMRole_namePrefix (13.37s)
--- PASS: TestAccAWSIAMRole_PermissionsBoundary (40.68s)
--- PASS: TestAccAWSIAMRole_tags (20.38s)
--- PASS: TestAccAWSIAMRole_testNameChange (27.12s)

--- PASS: TestAccAWSIAMRolePolicy_basic (24.33s)
--- PASS: TestAccAWSIAMRolePolicy_disappears (13.44s)
--- PASS: TestAccAWSIAMRolePolicy_generatedName (24.05s)
--- PASS: TestAccAWSIAMRolePolicy_invalidJSON (1.67s)
--- PASS: TestAccAWSIAMRolePolicy_namePrefix (23.58s)
--- PASS: TestAccAWSIAMRolePolicy_Policy_InvalidResource (7.51s)

--- PASS: TestAccAWSUser_basic (125.82s)
--- PASS: TestAccAWSUser_disappears (8.15s)
--- PASS: TestAccAWSUser_ForceDestroy_AccessKey (15.97s)
--- PASS: TestAccAWSUser_ForceDestroy_LoginProfile (24.25s)
--- PASS: TestAccAWSUser_ForceDestroy_MFADevice (15.65s)
--- PASS: TestAccAWSUser_ForceDestroy_SigningCertificate (13.71s)
--- PASS: TestAccAWSUser_ForceDestroy_SSHKey (14.45s)
--- PASS: TestAccAWSUser_nameChange (23.86s)
--- PASS: TestAccAWSUser_pathChange (99.29s)
--- PASS: TestAccAWSUser_permissionsBoundary (39.57s)
--- PASS: TestAccAWSUser_tags (19.52s)

--- PASS: TestAccAWSUserGroupMembership_basic (175.39s)

--- PASS: TestAccAWSUserLoginProfile_basic (26.41s)
--- PASS: TestAccAWSUserLoginProfile_keybase (17.15s)
--- PASS: TestAccAWSUserLoginProfile_keybaseDoesntExist (18.96s)
--- PASS: TestAccAWSUserLoginProfile_notAKey (12.70s)
--- PASS: TestAccAWSUserLoginProfile_PasswordLength (16.89s)

--- PASS: TestAccAWSUserPolicyAttachment_basic (29.04s)

--- PASS: TestAccAWSUserSSHKey_basic (119.15s)
--- PASS: TestAccAWSUserSSHKey_pemEncoding (105.54s)

[ghost]

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!