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

Update alzDefaultPolicyAssignments.bicep #729

Merged
merged 38 commits into from
Feb 27, 2024
Merged

Update alzDefaultPolicyAssignments.bicep #729

merged 38 commits into from
Feb 27, 2024

Conversation

VeronicaSea
Copy link
Contributor

@VeronicaSea VeronicaSea commented Feb 21, 2024

Overview/Summary

Those changes are used to have ALZ modules should be deployed optionally based on the SLZ flag to control the policy enforcement. All the SLZ modules should use the SLZ flag for SLZ policy enforcement or make it ‘Default’. Besides, the SLZ policy assignment modules supports policy Effect param, which we previously supported.

This PR fixes/adds/changes/removes

Issue 1 : Currently, all the ALZ modules will be deployed when the policy assignment file is invoked. ALZ modules should be deployed optionally based on the flag.

Issue 2 : The SLZ policy assignment modules are using ALZ flag to control the policy enforcement. All the SLZ modules should use the SLZ flag for SLZ policy enforcement or make it ‘Default’.

Issue 3 : The SLZ policy assignment modules don’t have policy Effect param, which we previously supported.

Breaking Changes

N/A

Testing Evidence

Replace this with any testing evidence to show that your Pull Request works/fixes as described and planned (include screenshots, if appropriate).

As part of this Pull Request I have

Issue 1 : Currently, all the ALZ modules will be deployed when the policy assignment file is invoked. ALZ modules should be deployed optionally based on the flag. 

Issue 2 : The SLZ policy assignment modules are using ALZ flag to control the policy enforcement. All the SLZ modules should use the SLZ flag for SLZ policy enforcement or make it ‘Default’. 

Issue 3 : The SLZ policy assignment modules don’t have policyEffect param, which we previously supported.
parPolicyEffect | Deny       | Set Parameter for effect type for all policy definitions
parPolicyEffect | Deny       | Set Parameter for effect type for all policy definitions
Effect type for all policy definitions
Effect type for all policy definitions
Copy link
Contributor

@oZakari oZakari left a comment

Choose a reason for hiding this comment

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

Hey @vericasea, once you incorporate the policy effect param into the UDT properties, can you also please add the new properties to the params within this parameters file?

Add sovereignty policy effect for different levels and parDisableSlzDefaultPolicies parameters.
@VeronicaSea
Copy link
Contributor Author

Hey @vericasea, once you incorporate the policy effect param into the UDT properties, can you also please add the new properties to the params within this parameters file?

Thanks and added them in the PR.

@VeronicaSea VeronicaSea reopened this Feb 26, 2024
@oZakari
Copy link
Contributor

oZakari commented Feb 27, 2024

Hey @vericasea, once you incorporate the policy effect param into the UDT properties, can you also please add the new properties to the params within this parameters file?

Thanks and added them in the PR.

Thanks @VeronicaSea, I've moved the params into the UDT for each param and added them to the parameters file.

I think the param to disable the SLZ policies is probably easier to understand if left out. You can still disable the deployment of the policy entirely with parTopLevelSovereigntyGlobalPoliciesEnable for the global policies or parPolicyAssignmentSovereigntyConfidential for the confidential sovereign policies. Now there is also the capability to add the audit effect which is essentially the same thing. Just think it's a bit cleaner, but let me know if you want to discuss further.

@oZakari oZakari closed this Feb 27, 2024
@oZakari oZakari reopened this Feb 27, 2024
@VeronicaSea VeronicaSea reopened this Feb 27, 2024
@@ -7,6 +7,9 @@ type policyAssignmentSovereigntyGlobalOptionsType = {

@sys.description('The list of locations that your organization can use to restrict deploying resources to. If left empty, only the deployment location will be allowed.')
parListOfAllowedLocations: string[]

@sys.description('The effect type for the Sovereignty Baseline - Global Policies Assignment.')
parPolicyEffect: ('Audit' | 'Deny' | 'Disabled' | 'AuditIfNotExists')
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is to broad as a parameter name and will cause confusion also seems a duplicate of parTopLevelSovereignGlobalPoliciesEnable?

Please can we align and make this simplified and clear to customers what the parameter does

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it explains in the description. It is already simplified and clear. We need it for different scope, like global, confidential.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@oZakari How do you think?

@oZakari oZakari merged commit c957061 into Azure:main Feb 27, 2024
7 checks passed
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.

4 participants