-
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
tests/provider: Enable semgrep resource.Retry verification for resource.TimeoutError handling #15530
Conversation
…ce.TimeoutError handling Reference: #12985 The `aws_lambda_function` resource does actually check for `resource.TimeoutError`, but due to how it performs dual timeout retries for EC2 throttling, it is very different that other `resource.Retry()` code handling.
Looks like #15527 covered Lex creation issues, but not deletion. 🤦 I will add that on this pull request since they are the last reports left on the main branch: $ semgrep
running 2 rules...
100%|████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████|2/2
aws/resource_aws_lex_bot.go
severity:warning rule:helper-schema-resource-Retry-without-TimeoutError-check: Check resource.Retry() errors with tfresource.TimedOut()
374: err := resource.Retry(d.Timeout(schema.TimeoutDelete), func() *resource.RetryError {
375: _, err := conn.DeleteBot(&lexmodelbuildingservice.DeleteBotInput{
376: Name: aws.String(d.Id()),
377: })
378:
379: if isAWSErr(err, lexmodelbuildingservice.ErrCodeConflictException, "") {
380: return resource.RetryableError(fmt.Errorf("%q: there is a pending operation, bot still deleting", d.Id()))
381: }
382: if err != nil {
383: return resource.NonRetryableError(err)
384: }
385:
386: return nil
387: })
388:
389: if err != nil {
390: return fmt.Errorf("error deleting bot %s: %w", d.Id(), err)
391: }
392:
393: _, err = waiter.LexBotDeleted(conn, d.Id())
394:
395: return err
aws/resource_aws_lex_bot_alias.go
severity:warning rule:helper-schema-resource-Retry-without-TimeoutError-check: Check resource.Retry() errors with tfresource.TimedOut()
261: err := resource.Retry(d.Timeout(schema.TimeoutDelete), func() *resource.RetryError {
262: _, err := conn.DeleteBotAlias(&lexmodelbuildingservice.DeleteBotAliasInput{
263: BotName: aws.String(botName),
264: Name: aws.String(botAliasName),
265: })
266:
267: if isAWSErr(err, lexmodelbuildingservice.ErrCodeConflictException, "") {
268: return resource.RetryableError(fmt.Errorf("'%q': bot alias still deleting", d.Id()))
269: }
270: if err != nil {
271: return resource.NonRetryableError(err)
272: }
273:
274: return nil
275: })
276: if err != nil {
277: return fmt.Errorf("error deleting bot alias '%s': %w", d.Id(), err)
278: }
279:
280: _, err = waiter.LexBotAliasDeleted(conn, botAliasName, botName)
281:
282: return err
aws/resource_aws_lex_intent.go
severity:warning rule:helper-schema-resource-Retry-without-TimeoutError-check: Check resource.Retry() errors with tfresource.TimedOut()
472: err := resource.Retry(d.Timeout(schema.TimeoutDelete), func() *resource.RetryError {
473: _, err := conn.DeleteIntent(&lexmodelbuildingservice.DeleteIntentInput{
474: Name: aws.String(d.Id()),
475: })
476:
477: if isAWSErr(err, lexmodelbuildingservice.ErrCodeConflictException, "") {
478: return resource.RetryableError(fmt.Errorf("%q: there is a pending operation, intent still deleting", d.Id()))
479: }
480: if err != nil {
481: return resource.NonRetryableError(err)
482: }
483:
484: return nil
485: })
486:
487: if err != nil {
488: return fmt.Errorf("error deleting intent %s: %w", d.Id(), err)
489: }
490:
491: _, err = waiter.LexIntentDeleted(conn, d.Id())
492:
493: return err
aws/resource_aws_lex_slot_type.go
severity:warning rule:helper-schema-resource-Retry-without-TimeoutError-check: Check resource.Retry() errors with tfresource.TimedOut()
260: err := resource.Retry(d.Timeout(schema.TimeoutDelete), func() *resource.RetryError {
261: _, err := conn.DeleteSlotType(&lexmodelbuildingservice.DeleteSlotTypeInput{
262: Name: aws.String(d.Id()),
263: })
264:
265: if tfawserr.ErrCodeEquals(err, lexmodelbuildingservice.ErrCodeConflictException) {
266: return resource.RetryableError(fmt.Errorf("%q: there is a pending operation, slot type still deleting", d.Id()))
267: }
268: if err != nil {
269: return resource.NonRetryableError(err)
270: }
271:
272: return nil
273: })
274: if err != nil {
275: return fmt.Errorf("error deleting slot type %s: %w", d.Id(), err)
276: }
277:
278: _, err = waiter.LexSlotTypeDeleted(conn, d.Id())
279:
280: return err |
…llowing any return Now returning a few more reports: ``` aws/awserr.go severity:warning rule:helper-schema-resource-Retry-without-TimeoutError-check: Check resource.Retry() errors with tfresource.TimedOut() 39: err := resource.Retry(2*time.Minute, func() *resource.RetryError { 40: var err error 41: resp, err = f() 42: if err != nil { 43: awsErr, ok := err.(awserr.Error) 44: if ok && awsErr.Code() == code { 45: return resource.RetryableError(err) 46: } 47: return resource.NonRetryableError(err) 48: } 49: return nil 50: }) 51: return resp, err 58: err := resource.Retry(1*time.Minute, func() *resource.RetryError { 59: var err error 60: resp, err = f() 61: if err != nil { 62: awsErr, ok := err.(awserr.Error) 63: if ok { 64: for _, code := range codes { 65: if awsErr.Code() == code { 66: return resource.RetryableError(err) 67: } 68: } 69: } 70: return resource.NonRetryableError(err) 71: } 72: return nil 73: }) 74: return resp, err aws/resource_aws_glue_crawler.go severity:warning rule:helper-schema-resource-Retry-without-TimeoutError-check: Check resource.Retry() errors with tfresource.TimedOut() 488: err = resource.Retry(1*time.Minute, func() *resource.RetryError { 489: _, err := glueConn.UpdateCrawler(updateCrawlerInput) 490: if err != nil { 491: if isAWSErr(err, glue.ErrCodeInvalidInputException, "Service is unable to assume role") { 492: return resource.RetryableError(err) 493: } 494: // InvalidInputException: Unable to retrieve connection tf-acc-test-8656357591012534997: User: arn:aws:sts::*******:assumed-role/tf-acc-test-8656357591012534997/AWS-Crawler is not authorized to perform: glue:GetConnection on resource: * (Service: AmazonDataCatalog; Status Code: 400; Error Code: AccessDeniedException; Request ID: 4d72b66f-9c75-11e8-9faf-5b526c7be968) 495: if isAWSErr(err, glue.ErrCodeInvalidInputException, "is not authorized") { 496: return resource.RetryableError(err) 497: } 498: return resource.NonRetryableError(err) 499: } 500: return nil 501: }) 502: 503: if err != nil { 504: return fmt.Errorf("error updating Glue crawler: %s", err) 505: } aws/resource_aws_lex_bot.go severity:warning rule:helper-schema-resource-Retry-without-TimeoutError-check: Check resource.Retry() errors with tfresource.TimedOut() 352: err := resource.Retry(d.Timeout(schema.TimeoutUpdate), func() *resource.RetryError { 353: _, err := conn.PutBot(input) 354: 355: if isAWSErr(err, lexmodelbuildingservice.ErrCodeConflictException, "") { 356: return resource.RetryableError(fmt.Errorf("%q: bot still updating", d.Id())) 357: } 358: if err != nil { 359: return resource.NonRetryableError(err) 360: } 361: 362: return nil 363: }) 364: if err != nil { 365: return fmt.Errorf("error updating bot %s: %w", d.Id(), err) 366: } 367: 368: return resourceAwsLexBotRead(d, meta) aws/resource_aws_lex_bot_alias.go severity:warning rule:helper-schema-resource-Retry-without-TimeoutError-check: Check resource.Retry() errors with tfresource.TimedOut() 232: err := resource.Retry(d.Timeout(schema.TimeoutUpdate), func() *resource.RetryError { 233: _, err := conn.PutBotAlias(input) 234: 235: // IAM eventual consistency 236: if tfawserr.ErrMessageContains(err, lexmodelbuildingservice.ErrCodeBadRequestException, "Lex can't access your IAM role") { 237: return resource.RetryableError(err) 238: } 239: if tfawserr.ErrCodeEquals(err, lexmodelbuildingservice.ErrCodeConflictException) { 240: return resource.RetryableError(fmt.Errorf("%q bot alias still updating", d.Id())) 241: } 242: if err != nil { 243: return resource.NonRetryableError(err) 244: } 245: 246: return nil 247: }) 248: if err != nil { 249: return fmt.Errorf("error updating bot alias '%s': %w", d.Id(), err) 250: } 251: 252: return resourceAwsLexBotAliasRead(d, meta) aws/resource_aws_lex_intent.go severity:warning rule:helper-schema-resource-Retry-without-TimeoutError-check: Check resource.Retry() errors with tfresource.TimedOut() 450: err := resource.Retry(d.Timeout(schema.TimeoutUpdate), func() *resource.RetryError { 451: _, err := conn.PutIntent(input) 452: 453: if isAWSErr(err, lexmodelbuildingservice.ErrCodeConflictException, "") { 454: return resource.RetryableError(fmt.Errorf("%q: intent still updating", d.Id())) 455: } 456: if err != nil { 457: return resource.NonRetryableError(err) 458: } 459: 460: return nil 461: }) 462: if err != nil { 463: return fmt.Errorf("error updating intent %s: %w", d.Id(), err) 464: } 465: 466: return resourceAwsLexIntentRead(d, meta) aws/resource_aws_lex_slot_type.go severity:warning rule:helper-schema-resource-Retry-without-TimeoutError-check: Check resource.Retry() errors with tfresource.TimedOut() 238: err := resource.Retry(d.Timeout(schema.TimeoutUpdate), func() *resource.RetryError { 239: _, err := conn.PutSlotType(input) 240: 241: if tfawserr.ErrCodeEquals(err, lexmodelbuildingservice.ErrCodeConflictException) { 242: return resource.RetryableError(fmt.Errorf("%q: slot type still updating", d.Id())) 243: } 244: if err != nil { 245: return resource.NonRetryableError(err) 246: } 247: 248: return nil 249: }) 250: if err != nil { 251: return fmt.Errorf("error updating slot type %s: %w", d.Id(), err) 252: } 253: 254: return resourceAwsLexSlotTypeRead(d, meta) aws/resource_aws_rds_cluster_parameter_group.go severity:warning rule:helper-schema-resource-Retry-without-TimeoutError-check: Check resource.Retry() errors with tfresource.TimedOut() 260: err := resource.Retry(3*time.Minute, func() *resource.RetryError { 261: _, err := rdsconn.ResetDBClusterParameterGroup(&resetOpts) 262: if err != nil { 263: if isAWSErr(err, "InvalidDBParameterGroupState", "has pending changes") { 264: return resource.RetryableError(err) 265: } 266: return resource.NonRetryableError(err) 267: } 268: return nil 269: }) 270: if err != nil { 271: return fmt.Errorf("error resetting DB Cluster Parameter Group: %s", err) 272: } ```
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 🚀
No code changes
…tion Reference: #12985 Reference: #15555 Output from acceptance testing (flaky test referenced above): ``` --- PASS: TestAccAwsLexBot_abortStatement (36.78s) --- PASS: TestAccAwsLexBot_basic (20.87s) --- PASS: TestAccAwsLexBot_childDirected (37.36s) --- PASS: TestAccAwsLexBot_clarificationPrompt (36.02s) --- PASS: TestAccAwsLexBot_description (37.97s) --- PASS: TestAccAwsLexBot_detectSentiment (36.49s) --- PASS: TestAccAwsLexBot_disappears (17.14s) --- PASS: TestAccAwsLexBot_enableModelImprovements (36.92s) --- PASS: TestAccAwsLexBot_idleSessionTtlInSeconds (38.34s) --- PASS: TestAccAwsLexBot_intents (38.02s) --- PASS: TestAccAwsLexBot_locale (41.30s) --- PASS: TestAccAwsLexBot_voiceId (38.14s) --- FAIL: TestAccAwsLexBot_version_serial (191.37s) --- PASS: TestAccAwsLexBot_version_serial/LexBotAlias_botVersion (59.23s) --- PASS: TestAccAwsLexBot_version_serial/DataSourceLexBot_withVersion (38.17s) --- PASS: TestAccAwsLexBot_version_serial/DataSourceLexBotAlias_basic (40.04s) --- FAIL: TestAccAwsLexBot_version_serial/LexBot_createVersion (53.93s) --- PASS: TestAccAwsLexBotAlias_basic (22.51s) --- PASS: TestAccAwsLexBotAlias_conversationLogsAudio (34.56s) --- PASS: TestAccAwsLexBotAlias_conversationLogsBoth (34.67s) --- PASS: TestAccAwsLexBotAlias_conversationLogsText (37.18s) --- PASS: TestAccAwsLexBotAlias_description (41.39s) --- PASS: TestAccAwsLexBotAlias_disappears (20.62s) --- PASS: TestAccAwsLexIntent_basic (17.45s) --- PASS: TestAccAwsLexIntent_conclusionStatement (31.39s) --- PASS: TestAccAwsLexIntent_confirmationPromptAndRejectionStatement (31.79s) --- PASS: TestAccAwsLexIntent_createVersion (33.13s) --- PASS: TestAccAwsLexIntent_dialogCodeHook (37.68s) --- PASS: TestAccAwsLexIntent_disappears (11.51s) --- PASS: TestAccAwsLexIntent_followUpPrompt (32.16s) --- PASS: TestAccAwsLexIntent_fulfillmentActivity (37.31s) --- PASS: TestAccAwsLexIntent_sampleUtterances (32.04s) --- PASS: TestAccAwsLexIntent_slots (32.24s) --- PASS: TestAccAwsLexIntent_slotsCustom (19.81s) --- PASS: TestAccAwsLexIntent_updateWithExternalChange (27.48s) --- PASS: TestAccAwsLexSlotType_basic (17.43s) --- PASS: TestAccAwsLexSlotType_createVersion (32.57s) --- PASS: TestAccAwsLexSlotType_description (60.90s) --- PASS: TestAccAwsLexSlotType_disappears (11.48s) --- PASS: TestAccAwsLexSlotType_enumerationValues (31.94s) --- PASS: TestAccAwsLexSlotType_name (33.23s) --- PASS: TestAccAwsLexSlotType_valueSelectionStrategy (31.72s) ```
…ates Reference: #12985 Output from acceptance testing: ``` --- PASS: TestAccAWSGlueCrawler_CatalogTarget (89.73s) --- PASS: TestAccAWSGlueCrawler_CatalogTarget_Multiple (119.60s) --- PASS: TestAccAWSGlueCrawler_Classifiers (82.63s) --- PASS: TestAccAWSGlueCrawler_Configuration (51.29s) --- PASS: TestAccAWSGlueCrawler_Description (78.75s) --- PASS: TestAccAWSGlueCrawler_disappears (36.35s) --- PASS: TestAccAWSGlueCrawler_DynamodbTarget (65.68s) --- PASS: TestAccAWSGlueCrawler_DynamodbTarget_scanAll (67.11s) --- PASS: TestAccAWSGlueCrawler_DynamodbTarget_scanRate (82.80s) --- PASS: TestAccAWSGlueCrawler_JdbcTarget (78.99s) --- PASS: TestAccAWSGlueCrawler_JdbcTarget_Exclusions (69.96s) --- PASS: TestAccAWSGlueCrawler_JdbcTarget_Multiple (84.61s) --- PASS: TestAccAWSGlueCrawler_RemoveTablePrefix (56.80s) --- PASS: TestAccAWSGlueCrawler_Role_ARN_NoPath (39.43s) --- PASS: TestAccAWSGlueCrawler_Role_ARN_Path (42.92s) --- PASS: TestAccAWSGlueCrawler_Role_Name_Path (41.28s) --- PASS: TestAccAWSGlueCrawler_S3Target (73.86s) --- PASS: TestAccAWSGlueCrawler_S3Target_ConnectionName (46.83s) --- PASS: TestAccAWSGlueCrawler_S3Target_Exclusions (73.32s) --- PASS: TestAccAWSGlueCrawler_S3Target_Multiple (84.81s) --- PASS: TestAccAWSGlueCrawler_Schedule (87.07s) --- PASS: TestAccAWSGlueCrawler_SchemaChangePolicy (70.30s) --- PASS: TestAccAWSGlueCrawler_SecurityConfiguration (65.60s) --- PASS: TestAccAWSGlueCrawler_TablePrefix (47.08s) --- PASS: TestAccAWSGlueCrawler_Tags (83.61s) ```
…rror during updates Reference: #12985 Output from acceptance testing: ``` --- PASS: TestAccAWSDBClusterParameterGroup_basic (68.16s) --- PASS: TestAccAWSDBClusterParameterGroup_disappears (15.87s) --- PASS: TestAccAWSDBClusterParameterGroup_generatedName (20.43s) --- PASS: TestAccAWSDBClusterParameterGroup_generatedName_Parameter (21.14s) --- PASS: TestAccAWSDBClusterParameterGroup_namePrefix (19.83s) --- PASS: TestAccAWSDBClusterParameterGroup_namePrefix_Parameter (21.19s) --- PASS: TestAccAWSDBClusterParameterGroup_only (20.18s) --- PASS: TestAccAWSDBClusterParameterGroup_updateParameters (34.40s) --- PASS: TestAccAWSDBClusterParameterGroup_withApplyMethod (21.46s) ```
52bfc2e
to
a126d4e
Compare
I was able to get Output from acceptance testing (created #15555 for the flaky Lex test):
|
This has been released in version 3.11.0 of the Terraform AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template for triage. Thanks! |
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! |
Community Note
Reference: #15522
Reference: #15523
Reference: #15524
Reference: #15526
Reference: #15527
Reference: #15529
Closes #12985
Release note for CHANGELOG:
The
aws_lambda_function
resource does actually check forresource.TimeoutError
, but due to how it performs dual timeout retries for EC2 throttling, it is very different than otherresource.Retry()
code handling so it is easier just to ignore in place.Output from acceptance testing: N/A (CI testing)