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

C7n org regioncheck update #5514

Merged
merged 4 commits into from Mar 28, 2020
Merged

C7n org regioncheck update #5514

merged 4 commits into from Mar 28, 2020

Conversation

JohnHillegass
Copy link
Collaborator

address an issue from PolicyCollection change in #4466

Looks like region is being set to the policy.config.region instead of policy.region now and this line is throwing an error when running c7n-org. I believe this can be traced back to changes made in #4466.

@kapilt
Copy link
Collaborator

kapilt commented Mar 28, 2020

thanks for the pr and flagging this. The fix here is actually to just remove this check. ie. p.region is not the same as p.config.region.. p.region was a conditional execution check, where as config.region is a requested region to execute the policy against. p.region and other condition attributes are no longer exposed, partly because they were non obvious pollution of the policy attributes, instead they are converted to conditions. re the fix, just remove these lines, policy.conditions are evaluated internal to a policy.run.

@JohnHillegass
Copy link
Collaborator Author

Awesome! That makes things even easier :) so much history in this tool haha. As usual, thanks for the explanation. I also submitted a small addition to the tests where I noticed a likely intended assertion missing.

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 f1353c1 into cloud-custodian:master Mar 28, 2020
@kapilt kapilt mentioned this pull request Apr 7, 2020
3 tasks
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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants