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.

Sign up for GitHub

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

Jump to bottom

Adding Azure Firewall Manager and Azure Firewall Policies #194

Merged
merged 6 commits into from
Apr 26, 2021
Merged

Adding Azure Firewall Manager and Azure Firewall Policies #194

merged 6 commits into from
Apr 26, 2021

Conversation

v-fearam
Copy link
Contributor

@v-fearam v-fearam commented Apr 21, 2021 •
edited
Loading

Also was updated the AKS version to 1.20.5
It was added a base Firewall Policy

@v-fearam v-fearam requested a review from ckittel April 21, 2021 19:22
ckittel
ckittel reviewed Apr 23, 2021
View reviewed changes
Copy link
Member

@ckittel ckittel left a comment

Choose a reason for hiding this comment

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

Just one common question, are the dependsOn right here?

networking/hub-regionA.json Outdated Show resolved Hide resolved
networking/hub-regionA.json Outdated Show resolved Hide resolved
networking/hub-default.json Outdated Show resolved Hide resolved
networking/hub-default.json Outdated Show resolved Hide resolved
networking/hub-default.json Outdated Show resolved Hide resolved
@ckittel
Copy link
Member

ckittel commented Apr 23, 2021

Should we be building off of basePolicy though more? I mean we want to make sure we're using this feature the way it was designed, to ensure we have easy-to-manage policies for the regional (or even multi) hubs

@v-fearam
Copy link
Contributor Author

About base policies
https://docs.microsoft.com/en-us/azure/firewall-manager/policy-overview#hierarchical-policies
It is not mandatory, the idea is, if you have a basic organization policies you can define on the baseline policy and then inherit from it.
We need to invent that situation here, In my opinion is out of scope of the RI story

@ckittel
Copy link
Member

ckittel commented Apr 23, 2021 •
edited
Loading

Don't we have basic org polices? I think we even call them "ORG WIDE" or something. Hence my question :) And then in the multi-region, you'll be able to inherit it twice.

@v-fearam
Copy link
Contributor Author

v-fearam commented Apr 23, 2021 •
edited
Loading

About "Don't we have basic org polices? I think we even call them "ORG WIDE" or something. Hence my question :) And then in the multi-region, you'll be able to inherit it twice."

I will add task for next sprint..... Maybe in multicluster we can show it better later on.
Added ADO ticket 415529

done

ckittel
ckittel requested changes Apr 26, 2021
View reviewed changes
Copy link
Member

@ckittel ckittel left a comment

Choose a reason for hiding this comment

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

Only one small question, otherwise, looks nice. Helps to have a dedciated name for this.

networking/hub-default.json Outdated Show resolved Hide resolved
networking/hub-regionA.json Outdated Show resolved Hide resolved
ckittel
ckittel approved these changes Apr 26, 2021
View reviewed changes
Copy link
Member

@ckittel ckittel left a comment

Choose a reason for hiding this comment

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

🚀 Thanks!

@v-fearam v-fearam merged commit bf059c5 into main Apr 26, 2021
@v-fearam v-fearam deleted the feature/firewall-ruleset branch April 26, 2021 18:31
v-fearam added a commit that referenced this pull request May 4, 2021
@v-fearam
Revert "Adding Azure Firewall Manager and Azure Firewall Policies (#194…
1b4bf2b 
…)"

This reverts commit bf059c5.
@v-fearam v-fearam mentioned this pull request May 4, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ckittel ckittel ckittel approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@v-fearam @ckittel