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
r/aws_ecr_repository: handle err, nil output on read #30067
Conversation
Community NoteVoting for Prioritization
For Submitters
|
if outputRaw == nil { | ||
return create.DiagError(names.ECR, create.ErrActionReading, ResNameRepository, d.Id(), errors.New("empty output")) | ||
} |
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.
Shouldn't be necessary as FindRepository
already does the nil
check.
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.
In theory the added err
check should prevent this nil
check from ever executing, but since we do an unchecked type assertion immediately following (which caused the reported panic) it felt reasonable to add for absolute safety.
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 🚀.
% make testacc TESTARGS='-run=TestAccECRRepository_' PKG=ecr ACCTEST_PARALLELISM=2
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/ecr/... -v -count 1 -parallel 2 -run=TestAccECRRepository_ -timeout 180m
=== RUN TestAccECRRepository_basic
=== PAUSE TestAccECRRepository_basic
=== RUN TestAccECRRepository_disappears
=== PAUSE TestAccECRRepository_disappears
=== RUN TestAccECRRepository_tags
=== PAUSE TestAccECRRepository_tags
=== RUN TestAccECRRepository_immutability
=== PAUSE TestAccECRRepository_immutability
=== RUN TestAccECRRepository_Image_scanning
=== PAUSE TestAccECRRepository_Image_scanning
=== RUN TestAccECRRepository_Encryption_kms
=== PAUSE TestAccECRRepository_Encryption_kms
=== RUN TestAccECRRepository_Encryption_aes256
=== PAUSE TestAccECRRepository_Encryption_aes256
=== CONT TestAccECRRepository_basic
=== CONT TestAccECRRepository_Image_scanning
--- PASS: TestAccECRRepository_basic (19.42s)
=== CONT TestAccECRRepository_tags
--- PASS: TestAccECRRepository_Image_scanning (45.75s)
=== CONT TestAccECRRepository_immutability
--- PASS: TestAccECRRepository_tags (40.29s)
=== CONT TestAccECRRepository_Encryption_aes256
--- PASS: TestAccECRRepository_immutability (17.03s)
=== CONT TestAccECRRepository_Encryption_kms
--- PASS: TestAccECRRepository_Encryption_aes256 (43.93s)
=== CONT TestAccECRRepository_disappears
--- PASS: TestAccECRRepository_Encryption_kms (42.76s)
--- PASS: TestAccECRRepository_disappears (13.41s)
PASS
ok github.com/hashicorp/terraform-provider-aws/internal/service/ecr 123.098s
This functionality has been released in v4.59.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. Thank you! |
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. |
Description
Fix to properly handle errors that aren't
ResourceNotFound
and nil output values.Relations
Closes #30068
Output from Acceptance Testing