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.

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

aws - cloudfront - logging filter for distributions #5577

Merged
merged 16 commits into from Apr 27, 2020

Conversation

Lucas-Irvine
Copy link
Contributor

Filter for Logging enabled in Cloudfront Distributions. This is currently not possible using a value filter because https://boto3.amazonaws.com/v1/documentation/api/latest/reference/services/cloudfront.html#CloudFront.Client.list_distributions does not return the Logging Field.

c7n/resources/cloudfront.py Outdated Show resolved Hide resolved
c7n/resources/cloudfront.py Outdated Show resolved Hide resolved
.get('DistributionConfig')
except (client.exceptions.NoSuchResource, client.exceptions.NoSuchDistribution):
r[self.annotation_key] = {}
except Exception as e:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks like this is missing test coverage on error handling here (re patch coverage ci check), this is probably easier via mock or passing in a synthetic resource, as an example of using a mock to check error handling https://github.com/cloud-custodian/cloud-custodian/blob/master/tests/test_cwl.py#L92

tests/test_cloudfront.py Outdated Show resolved Hide resolved
@Lucas-Irvine
Copy link
Contributor Author

Thanks for the review @kapilt. I think I was able to address all your comments, and I'll definitely be using MagicMock to test exception handling in the future.

@JohnHillegass
Copy link
Collaborator

@Lucas-Irvine given that this enables checking to see if logging is enabled on the distributions, are you also planning to add an action to enable logging if it is disabled?

@kapilt
Copy link
Collaborator

kapilt commented Apr 27, 2020

@Lucas-Irvine given that this enables checking to see if logging is enabled on the distributions, are you also planning to add an action to enable logging if it is disabled?

that's already available.

@JohnHillegass
Copy link
Collaborator

Oh wow, thanks for confirming that. I see it in set-attributes now 🤦‍♂️nevermind @Lucas-Irvine

@JohnHillegass JohnHillegass changed the title AWS - Add Filter for Logging in Cloudfront Distributions aws - cloudfront - logging filter for distributions Apr 27, 2020
@Lucas-Irvine
Copy link
Contributor Author

Will add a streaming-distribution-config filter and a set-attributes action for streaming distributions as well.

Copy link
Collaborator

@kapilt kapilt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, thanks

@kapilt kapilt merged commit 800840a into cloud-custodian:master Apr 27, 2020
fidelito pushed a commit to fidelito/cloud-custodian that referenced this pull request May 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants