-
Notifications
You must be signed in to change notification settings - Fork 9.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
resource/aws_glue_crawler: Suppress role difference when using ARN #6293
Conversation
The Glue API always returns the role as its name with path. Previously: ``` --- FAIL: TestAccAWSGlueCrawler_Role_ARN (22.42s) testing.go:538: Step 0 error: After applying this step, the plan was not empty: DIFF: UPDATE: aws_glue_crawler.test role: "tf-acc-test-8172536439889733682" => "arn:aws:iam::123456789012:role/tf-acc-test-8172536439889733682" ``` Output from acceptance testing: ``` --- PASS: TestAccAWSGlueCrawler_Role_ARN_NoPath (33.69s) --- PASS: TestAccAWSGlueCrawler_TablePrefix (62.37s) --- PASS: TestAccAWSGlueCrawler_Role_Name_Path (62.40s) --- PASS: TestAccAWSGlueCrawler_JdbcTarget_Exclusions (65.18s) --- PASS: TestAccAWSGlueCrawler_JdbcTarget (67.71s) --- PASS: TestAccAWSGlueCrawler_SchemaChangePolicy (68.43s) --- PASS: TestAccAWSGlueCrawler_S3Target_Exclusions (80.19s) --- PASS: TestAccAWSGlueCrawler_Schedule (85.71s) --- PASS: TestAccAWSGlueCrawler_S3Target_Multiple (85.83s) --- PASS: TestAccAWSGlueCrawler_Role_ARN_Path (86.12s) --- PASS: TestAccAWSGlueCrawler_Classifiers (87.78s) --- PASS: TestAccAWSGlueCrawler_JdbcTarget_Multiple (88.38s) --- PASS: TestAccAWSGlueCrawler_S3Target (90.89s) --- PASS: TestAccAWSGlueCrawler_Description (91.87s) --- PASS: TestAccAWSGlueCrawler_recreates (93.85s) --- PASS: TestAccAWSGlueCrawler_Configuration (101.18s) --- PASS: TestAccAWSGlueCrawler_DynamodbTarget (105.39s) ```
aws/resource_aws_iam_role.go
Outdated
@@ -400,3 +402,15 @@ func resourceAwsIamRoleDelete(d *schema.ResourceData, meta interface{}) error { | |||
return nil | |||
}) | |||
} | |||
|
|||
func getIAMRoleNameFromIAMRoleARN(inputARN string) (string, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When passed an ARN of a different resource type, this function will still return the contents of parsedARN.Resource
unmodified (because of the way TrimPrefix
works).
Since this function's name hints at being specific to roles, would it make sense to err when the resource is not a role?
Better yet, might it make sense to call it something generic, like maybe getResourceNameFromARN
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If it's generic, it'd probably have to handle the other prefixes: "user/", "group/", "policy/", "instance-profile/" etc. of which there are many. It might be safe to remove everything up through the first slash, but it's hard to be sure if that's right.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rehevkor5 That's exactly how I was imagining it working.
Only thing is to make sure that there are no rogue resources not adhering to type/name
format for that part of the ARN.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@alexsomesan @rehevkor5 ARNs take many forms and I have seen plenty of variations that are not documented here: https://docs.aws.amazon.com/general/latest/gr/aws-arns-and-namespaces.html
I can create a generic function as suggested but it should still explicitly remove the role/
for this specific usage, in my opinion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was afraid that would be the case when I mentioned rogue resources.
I remember running into those unusual ARNs every now and then.
Oh well, never mind then. Not worth the trouble.
The AWS docs about this are not great, either. In https://docs.aws.amazon.com/glue/latest/dg/aws-glue-api-crawler-crawling.html#aws-glue-api-crawler-crawling-Crawler it says only:
I will ask AWS to improve that documentation. It seems likely there may be similar issues with the Glue Jobs API. |
``` --- PASS: TestAccAWSGlueCrawler_Role_ARN_NoPath (31.73s) --- PASS: TestAccAWSGlueCrawler_recreates (34.70s) --- PASS: TestAccAWSGlueCrawler_Configuration (44.72s) --- PASS: TestAccAWSGlueCrawler_JdbcTarget_Exclusions (50.05s) --- PASS: TestAccAWSGlueCrawler_Role_Name_Path (52.81s) --- PASS: TestAccAWSGlueCrawler_TablePrefix (53.15s) --- PASS: TestAccAWSGlueCrawler_S3Target (61.47s) --- PASS: TestAccAWSGlueCrawler_S3Target_Exclusions (67.64s) --- PASS: TestAccAWSGlueCrawler_Schedule (67.68s) --- PASS: TestAccAWSGlueCrawler_JdbcTarget_Multiple (69.62s) --- PASS: TestAccAWSGlueCrawler_SchemaChangePolicy (70.07s) --- PASS: TestAccAWSGlueCrawler_Classifiers (77.97s) --- PASS: TestAccAWSGlueCrawler_DynamodbTarget (78.41s) --- PASS: TestAccAWSGlueCrawler_Role_ARN_Path (80.67s) --- PASS: TestAccAWSGlueCrawler_Description (87.32s) --- PASS: TestAccAWSGlueCrawler_JdbcTarget (98.37s) --- PASS: TestAccAWSGlueCrawler_S3Target_Multiple (101.88s) ```
Pushed update to remove
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This has been released in version 1.42.0 of the AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. |
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! |
Fixes #6292
Changes proposed in this pull request:
DiffSuppressFunc
toaws_glue_crawler
role
attribute to prevent difference of ARN in configuration and API response of name with path.Previously:
Output from acceptance testing: