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

azure - update firewall-rules filter to use effective rule set #4756

Merged

Conversation

@logachev
Copy link
Collaborator

commented Sep 10, 2019

We didn't consider Default action previously. That way, if all rules were properly configured, but Firewall was disabled, we won't be able to find resources with violation without extra filters in c7n.

This change updates the behavior to consider Default action by appending 0.0.0.0/0 CIDR if default action is Allow (since Azure resources can't explicitly block certain IPs).

Update tests to verify changed behavior for _query_rules

logachev and others added 2 commits Sep 9, 2019

@logachev logachev force-pushed the logachev:kiril/fix_firewall_default_action branch 2 times, most recently from b5fc53e to 1478420 Sep 10, 2019

@logachev logachev force-pushed the logachev:kiril/fix_firewall_default_action branch from 1478420 to f2c8ede Sep 10, 2019

- '131.107.160.2-131.107.160.3'
- 10.20.20.0/24
- AzureCloud
- Portal

This comment has been minimized.

Copy link
@stefangordon

stefangordon Sep 10, 2019

Collaborator

Shouldn't this be a bypass field? And then we should actually remove it from the IP set we use for normal IP filtering and treat it as a separate thing? That is what the action does.

logachev added 4 commits Sep 10, 2019
Merge branch 'kiril/fix_firewall_default_action' of github.com:logach…
…ev/cloud-custodian into kiril/fix_firewall_default_action
parts = ip_range_string.replace(' ', '').split(',')

# Exclude magic strings representing Portal and Azure Cloud
parts = list(set(parts) - set(PORTAL_IPS + AZURE_CLOUD_IPS))

This comment has been minimized.

Copy link
@stefangordon

stefangordon Sep 11, 2019

Collaborator

minor: I think we should do a contains check on the portal IP set and then remove it.

Only the total set represents the bypass (and the portal checkbox) - but if a user just manually added a subset of the portal set then it would be incorrect to remove them here.

Kirill Logachev and others added 3 commits Sep 11, 2019
Kirill Logachev Kirill Logachev
stefangordon and others added 2 commits Sep 12, 2019

@logachev logachev merged commit d7afef7 into cloud-custodian:master Sep 12, 2019

15 checks passed

Custodian - CI Build #20190912.5 succeeded
Details
Custodian - CI (Container Cask) Container Cask succeeded
Details
Custodian - CI (Docs) Docs succeeded
Details
Custodian - CI (Lint) Lint succeeded
Details
Custodian - CI (Test Python27) Test Python27 succeeded
Details
Custodian - CI (Test Python36) Test Python36 succeeded
Details
Custodian - CI (Test Python37) Test Python37 succeeded
Details
Custodian - CI (Test Win2019 Python36) Test Win2019 Python36 succeeded
Details
Custodian - CI (Test Win2019 Python37) Test Win2019 Python37 succeeded
Details
codecov/patch 100% of diff hit (target 89.47%)
Details
codecov/project/azure 91.87% (+0.06%) compared to 4f5bedd
Details
codecov/project/custodian 90.52% (-0.01%) compared to 4f5bedd
Details
codecov/project/gcp 91.25% remains the same compared to 4f5bedd
Details
codecov/project/k8s 93.72% remains the same compared to 4f5bedd
Details
codecov/project/mailer 62.13% remains the same compared to 4f5bedd
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.