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

Add APIM Policies #377

Open
wants to merge 83 commits into
base: main
Choose a base branch
from
Open

Add APIM Policies #377

wants to merge 83 commits into from

Conversation

marvinbuss
Copy link
Contributor

@marvinbuss marvinbuss commented Feb 28, 2023

Summary of the Pull Request

  • Add APIM Policies according to documented gaps
  • Add policy to deny public network access
  • Add policy to deny PIP assignment to APIM
  • Add policy for cross-tenant pes

PR Checklist

Validation Steps Performed

},
{
"anyOf": [
{
Copy link
Contributor

Choose a reason for hiding this comment

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

How would this work with the custom policy we have already included ("API Management services should use a virtual network")?

Copy link
Contributor Author

@marvinbuss marvinbuss Mar 2, 2023

Choose a reason for hiding this comment

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

This is more restricted and enforces to host the instance in Internal mode instead of allowing both Internal and External. I am assuming that by default the APIM instances in Corp should not be accessible from the internet. See link here.
If external access should be used they can host APIM in Internal mode and connect it to an App GW.

Copy link
Contributor

Choose a reason for hiding this comment

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

Do we have to distinguish at this level? Remember that this policySetDefinition will be assigned broadly at the LZ scope and apply to both cloud-native and corp connected. @victorar - any input from your PoV wrt to the other policies we have for APIM related to networking?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will follow your guidance and understand the concerns.
If we enforce this for corp and online Landing Zones, we should still be fine. Corp use-cases will then be able to rely on private endpoints whereas online landing zones can use an App GW with WAF and Public IP to accept public traffic.
Looking forward to your input @victorar and will remove it if it is too restrictive.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Please let me know how to continue here.

@marvinbuss
Copy link
Contributor Author

image

krnese and others added 12 commits March 2, 2023 12:26
* Add Azure Storage Policies

* Fix minor bug

* Update type

* Add policy for CORS rules

* Add policy for CMK for encryption scopes

* Remove policy for encryption scope

* Update display name

* Add list of allowed values for policy definition

* Update policy for encryption

* Add policy assignments

* Removed policy for cross tenant PEs

* Add missing parameters

* Update mg name
Base automatically changed from secure-by-default to main March 3, 2023 19:38
@marvinbuss marvinbuss changed the base branch from main to secure-by-default March 4, 2023 13:34
@marvinbuss marvinbuss changed the base branch from secure-by-default to main March 4, 2023 13:34
@marvinbuss
Copy link
Contributor Author

Ready to be merged
image

@marvinbuss marvinbuss requested a review from krnese March 4, 2023 14:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants