-
Notifications
You must be signed in to change notification settings - Fork 157
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
add support for dynamodb_table #244
Conversation
Codecov Report
@@ Coverage Diff @@
## main #244 +/- ##
==========================================
- Coverage 70.20% 70.13% -0.07%
==========================================
Files 217 221 +4
Lines 4866 4922 +56
==========================================
+ Hits 3416 3452 +36
- Misses 1180 1195 +15
- Partials 270 275 +5
|
4fa457f
to
fca6f2a
Compare
|
||
func (r *AwsDynamodbTable) NormalizeForState() (resource.Resource, error) { | ||
// set default to empty struct for timeouts | ||
if r.Timeouts == nil { |
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.
It's a Terraform only related field, for consistency purpose it's more straightforward to handle it like in other resource by adding a diff:"-"
annotation.
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.
ho you're right I thought these were real timeout not only used by tf...
message := fmt.Sprintf("Ignoring %s from drift calculation: Listing %s is forbidden.", listError.SupplierType(), listError.ListedTypeError()) | ||
logrus.Debugf(message) | ||
//logrus.Debugf(message) |
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.
You can remove this line π
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.
π€¦
@@ -21,9 +21,9 @@ func HandleResourceEnumerationError(err error, alertr *alerter.Alerter) error { | |||
return err | |||
} | |||
|
|||
if reqerr.StatusCode() == 403 { | |||
if reqerr.StatusCode() == 403 || strings.Contains(reqerr.Code(), "AccessDenied") { |
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.
To be sure to catch only this case maybe we could check the status code too ?
if reqerr.StatusCode() == 403 || (reqerr.StatusCode() == 400 && reqerr.Code() == "AccessDeniedException") {
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.
From what we saw AccessDeniedException is always access denied it does not depends on the http code so I remove te 400 check but no matter what we do we won't catch everything, 403 on dynamo is still another error.
fca6f2a
to
ba6dd26
Compare
ba6dd26
to
73ffe82
Compare
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.
Seems good !
Description
add support for dynamodb_table