fix: Invalid repo policy due to IAM eventual consistency #1165

frncmx · 2017-07-16T17:51:52Z

If I try to use a newly created IAM role's ARN in an ECR policy's
principal field, I got: 'Invalid repository policy provided'.

IAM changes need time to propagate to other services like S3, ECR.
There is no upper bound granted by AWS on propagation time, but
usually 1 minute should be enough.
https://forums.aws.amazon.com/thread.jspa?messageID=195539#195539

A similar retry logic is already present in aws/resource_aws_ecs_service.go
Compared to the ECS Service resource; isAWSErr() makes the retry logic simpler
and the dropped DEBUG statements did not add an extra info.
(Request-ID, error message, retry info is all visible in the log without the
extra Printf() call.)

Note: I did not find any way to fix this on the 'aws_iam_role'
resource side, since the IAM API reflects the changes immediately and
does not provide any info about the propagation status.

fixes #1164

Signed-off-by: Ferenc Szabo frncmx@gmail.com

frncmx · 2017-07-16T20:35:11Z

@radeksimko I've based this PR on hashicorp/terraform@fad019e

Any test I should write for this? If yes, please help me to start.
I've looked into aws/resource_aws_ecs_service_test.go but I did not find any test covering the case above.

radeksimko

This looks overall sensible thing to do. Even though I wasn't able to reproduce it I believe you this may happen.

Just one thing I'd change in this PR is how we catch the error. Matching just code InvalidParameterException seems too broad to me.

How about something like this?

if isAWSErr(err, "InvalidParameterException", "Invalid repository policy provided") {

Also with this approach ^ you can leave out the if !ok { condition and let it fall through to the return at the bottom.

frncmx · 2017-07-22T14:53:03Z

I'm surprised you did not manage to reproduce it based on #1164.

After your comment I thought: maybe your region handles propagation faster.
I've tried in us-east-1, eu-west-1 and eu-central-1 . I was always able to reproduce the issue. (1st try fails, 2nd succeeds.)
I also tried with Terraform v0.9.11 and v.0.10.0; the results were the same.

I'm just about to make the requested changes. Good points.
Also, I'm playing around with the Acceptance Test Framework.

Before retry in place:

$ make testacc TEST=./aws TESTARGS='-run=TestAccAWSEcrRepositoryPolicy'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccAWSEcrRepositoryPolicy -timeout 120m
=== RUN   TestAccAWSEcrRepositoryPolicy_basic
--- PASS: TestAccAWSEcrRepositoryPolicy_basic (29.69s)
=== RUN   TestAccAWSEcrRepositoryPolicy_iam
--- FAIL: TestAccAWSEcrRepositoryPolicy_iam (18.27s)
	testing.go:428: Step 0 error: Error applying: 1 error(s) occurred:
		
		* aws_ecr_repository_policy.default: 1 error(s) occurred:
		
		* aws_ecr_repository_policy.default: InvalidParameterException: Invalid parameter at 'PolicyText' failed to satisfy constraint: 'Invalid repository policy provided'
			status code: 400, request id: <req-id>
FAIL
exit status 1
FAIL	github.com/terraform-providers/terraform-provider-aws/aws	47.971s
make: *** [GNUmakefile:15: testacc] Error 1

After retry in place:

$ make testacc TEST=./aws TESTARGS='-run=TestAccAWSEcrRepositoryPolicy'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccAWSEcrRepositoryPolicy -timeout 120m
=== RUN   TestAccAWSEcrRepositoryPolicy_basic
--- PASS: TestAccAWSEcrRepositoryPolicy_basic (31.79s)
=== RUN   TestAccAWSEcrRepositoryPolicy_iam
--- PASS: TestAccAWSEcrRepositoryPolicy_iam (39.74s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	71.540s

frncmx · 2017-07-22T17:16:20Z

I've added a new log - retry in place - to the Gist provided with the original issue.
https://gist.github.com/frncmx/4016347c801357e5f255526ebac8f325#file-cr-attach-policy-with-retry-log

The retry is clearly happening...

$ \grep -B1 '\[TRACE\] Waiting' ecr-attach-policy.log 
2017/07/22 19:04:18 [DEBUG] plugin: terraform-provider-aws: 2017/07/22 19:04:18 [DEBUG] [aws-sdk-go] {"__type":"InvalidParameterException","message":"Invalid parameter at 'PolicyText' failed to satisfy constraint: 'Invalid repository policy provided'"}
2017/07/22 19:04:18 [DEBUG] plugin: terraform-provider-aws: 2017/07/22 19:04:18 [TRACE] Waiting 500ms before next try
--
2017/07/22 19:04:19 [DEBUG] plugin: terraform-provider-aws: 2017/07/22 19:04:19 [DEBUG] [aws-sdk-go] {"__type":"InvalidParameterException","message":"Invalid parameter at 'PolicyText' failed to satisfy constraint: 'Invalid repository policy provided'"}
2017/07/22 19:04:19 [DEBUG] plugin: terraform-provider-aws: 2017/07/22 19:04:19 [TRACE] Waiting 1s before next try
--
2017/07/22 19:04:21 [DEBUG] plugin: terraform-provider-aws: 2017/07/22 19:04:21 [DEBUG] [aws-sdk-go] {"__type":"InvalidParameterException","message":"Invalid parameter at 'PolicyText' failed to satisfy constraint: 'Invalid repository policy provided'"}
2017/07/22 19:04:21 [DEBUG] plugin: terraform-provider-aws: 2017/07/22 19:04:21 [TRACE] Waiting 2s before next try
--
2017/07/22 19:04:23 [DEBUG] plugin: terraform-provider-aws: 2017/07/22 19:04:23 [DEBUG] [aws-sdk-go] {"__type":"InvalidParameterException","message":"Invalid parameter at 'PolicyText' failed to satisfy constraint: 'Invalid repository policy provided'"}
2017/07/22 19:04:23 [DEBUG] plugin: terraform-provider-aws: 2017/07/22 19:04:23 [TRACE] Waiting 4s before next try
--
2017/07/22 19:04:28 [DEBUG] plugin: terraform-provider-aws: 2017/07/22 19:04:28 [DEBUG] [aws-sdk-go] {"__type":"InvalidParameterException","message":"Invalid parameter at 'PolicyText' failed to satisfy constraint: 'Invalid repository policy provided'"}
2017/07/22 19:04:28 [DEBUG] plugin: terraform-provider-aws: 2017/07/22 19:04:28 [TRACE] Waiting 8s before next try

If I try to use a newly created IAM role's ARN in an ECR policy's principal field, I got: 'Invalid repository policy provided'. IAM changes need time to propagate to other services like S3, ECR. There is no upper bound granted by AWS on propagation time, but usually 1 minute should be enough. https://forums.aws.amazon.com/thread.jspa?messageID=195539#195539 A similar retry logic is already present in aws/resource_aws_ecs_service.go Compared to the ECS Service resource; isAWSErr() makes the retry logic simpler and the dropped DEBUG statements did not add an extra info. (Request-ID, error message, retry info is all visible in the log without the extra Printf() call.) Note: I did not find any way to fix this on the 'aws_iam_role' resource side, since the IAM API reflects the changes immediately and does not provide any info about the propagation status. fixes #1164 Signed-off-by: Ferenc Szabo <frncmx@gmail.com>

frncmx · 2017-07-22T17:35:11Z

Changed as requested.

As requested in the contributing guideline, I've also tried to write my 1st acceptance test for Terraform. I'm really not sure about the Acc style.

radeksimko

LGTM, thanks.

frncmx · 2017-07-24T12:57:37Z

🍾 Thanks. :)

ghost · 2020-04-11T17:49:04Z

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!

radeksimko suggested changes Jul 18, 2017

View reviewed changes

radeksimko added bug Addresses a defect in current functionality. waiting-response Maintainers are waiting on response from community or contributor. labels Jul 18, 2017

radeksimko removed the waiting-response Maintainers are waiting on response from community or contributor. label Jul 23, 2017

radeksimko approved these changes Jul 24, 2017

View reviewed changes

radeksimko merged commit 19dbc2e into hashicorp:master Jul 24, 2017

frncmx deleted the fix/ecr-cannot-use-new-iam-role-in-policy branch July 24, 2017 09:55

frncmx mentioned this pull request Aug 21, 2017

interpolation does not wait for resources to complete creation? hashicorp/terraform#15536

Closed

sethcleveland mentioned this pull request Jan 23, 2020

aws_codebuildproject requires two "applys" when created with a new IAM role #11741

Closed

ghost locked and limited conversation to collaborators Apr 11, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Invalid repo policy due to IAM eventual consistency #1165

fix: Invalid repo policy due to IAM eventual consistency #1165

frncmx commented Jul 16, 2017 •

edited

Loading

frncmx commented Jul 16, 2017 •

edited

Loading

radeksimko left a comment •

edited

Loading

frncmx commented Jul 22, 2017 •

edited

Loading

frncmx commented Jul 22, 2017 •

edited

Loading

frncmx commented Jul 22, 2017 •

edited

Loading

radeksimko left a comment

frncmx commented Jul 24, 2017

ghost commented Apr 11, 2020

fix: Invalid repo policy due to IAM eventual consistency #1165

fix: Invalid repo policy due to IAM eventual consistency #1165

Conversation

frncmx commented Jul 16, 2017 • edited Loading

frncmx commented Jul 16, 2017 • edited Loading

radeksimko left a comment • edited Loading

Choose a reason for hiding this comment

frncmx commented Jul 22, 2017 • edited Loading

frncmx commented Jul 22, 2017 • edited Loading

frncmx commented Jul 22, 2017 • edited Loading

radeksimko left a comment

Choose a reason for hiding this comment

frncmx commented Jul 24, 2017

ghost commented Apr 11, 2020

frncmx commented Jul 16, 2017 •

edited

Loading

frncmx commented Jul 16, 2017 •

edited

Loading

radeksimko left a comment •

edited

Loading

frncmx commented Jul 22, 2017 •

edited

Loading

frncmx commented Jul 22, 2017 •

edited

Loading

frncmx commented Jul 22, 2017 •

edited

Loading