Skip to content
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.

Sign up for GitHub

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

Jump to bottom

tests/provider: Enable semgrep resource.Retry verification for resource.TimeoutError handling #15530

Merged
merged 6 commits into from
Oct 12, 2020
Merged

tests/provider: Enable semgrep resource.Retry verification for resource.TimeoutError handling #15530

merged 6 commits into from
Oct 12, 2020

Commits on Oct 7, 2020

  1. tests/provider: Enable semgrep resource.Retry verification for resour… 

    …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.
    @bflad
    bflad committed Oct 7, 2020
    Configuration menu
    Copy the full SHA
    5951921 View commit details
    Browse the repository at this point in the history

  2. tests/provider: Catch additional resource.Retry cases in semgrep by a… 

    …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:				}
```
    @bflad
    bflad committed Oct 7, 2020
    Configuration menu
    Copy the full SHA
    5847d38 View commit details
    Browse the repository at this point in the history

  3. service/lex: Additional resource.Timeout handling on updates and dele… 

    …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)
```
    @bflad
    bflad committed Oct 7, 2020
    Configuration menu
    Copy the full SHA
    13f4e32 View commit details
    Browse the repository at this point in the history

  4. resource/aws_glue_crawler: Check for resource.TimeoutError during upd… 

    …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)
```
    @bflad
    bflad committed Oct 7, 2020
    Configuration menu
    Copy the full SHA
    f73733a View commit details
    Browse the repository at this point in the history

  5. resource/aws_rds_cluster_parameter_group: Check for resource.TimeoutE… 

    …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)
```
    @bflad
    bflad committed Oct 7, 2020
    Configuration menu
    Copy the full SHA
    e91a4bc View commit details
    Browse the repository at this point in the history

  6. provider: Check for resource.TimeoutError in awserr.go helpers 

    Reference: #12985
    @bflad
    bflad committed Oct 7, 2020
    Configuration menu
    Copy the full SHA
    a126d4e View commit details
    Browse the repository at this point in the history