Adding AzurePolicyMode to Installation #3618
Conversation
vikastigera
changed the title
Changes done to add Azure config to installation spec which has option to provide PolicyMode. This helps clients to have control over "control-plane" labels for namespaces.
api/v1/installation_types.go
Outdated
|
|
||
| type Azure struct { | ||
| // PolicyMode determines whether the "control-plane" label is applied to namespaces. It offers two options: Default and Manual. | ||
| // The Default option adds the "control-plane" label to namespaces that have the PodSecurityStandard set to privileged. |
There was a problem hiding this comment.
Why are we making this dependent on the PodSecurityStandard too? Did I miss something in the design or previous discussions that PodSecurityStandard was going to be a condition on setting the label?
There was a problem hiding this comment.
Yeah, these shouldn't be directly tied to each other. We just brought that up as another similar configuration.
There was a problem hiding this comment.
Are you suggesting to add label to all the namespaces in Default option ?
In the meeting we decided to add it only to the privileged namespaces
There was a problem hiding this comment.
Nope, not all namespaces.
We should only add it to namespaces that need this policy configured - that may or may not be the same set of namespaces as those that require PSSPrivileged, but the point is that doesn't mean hard equality between control-plane and PSSPrivileged, and it definitely isn't something the user should know about / be able to rely on.
There was a problem hiding this comment.
Got it, I'll update the documentation.
| } | ||
| return ns | ||
| } | ||
|
|
||
| func applyAzurePolicy(azure *operatorv1.Azure, pss PodSecurityStandard) bool { | ||
| if azure == nil || azure.PolicyMode == nil || *azure.PolicyMode == operatorv1.Default { |
There was a problem hiding this comment.
I think we should only possibly return true if the PolicyMode is Default.
There was a problem hiding this comment.
Is it related to the previous comment, return "true" instead of "PSSPrivileged == pss" or not to return true for nil cases ?
There was a problem hiding this comment.
I believe not having this config set (nil cases) should apply default behavior.
There was a problem hiding this comment.
It should, but it should also always default nil -> Default based on our kubebuilder tags
There was a problem hiding this comment.
In the unlikely case that we change the default value in the future, we would want nil to switch to that default here.
There was a problem hiding this comment.
I think the tests in the core_controller and the renderer reflect the desired outcomes for each permutation and the tests pass.
So I think it is ok to proceed with merging this PR.
* Adding AzurePolicyMode to Installationx Changes done to add Azure config to installation spec which has option to provide PolicyMode. This helps clients to have control over "control-plane" labels for namespaces. * Fixing merge failure * fixing test * Updating documentation
…v1.36 [Cherry-Pick] Adding AzurePolicyMode to Installation (#3618)
Changes done to add Azure config to installation spec which has option to provide PolicyMode. This helps clients to have control over "control-plane" labels for namespaces.
Description
https://tigera.atlassian.net/browse/EV-5377
For context in design doc : https://docs.google.com/document/d/1sttHDBs18_JoTCd-f2snZBn5kCWwqIst1mCseWu1hec/edit?usp=sharing
For PR author
make gen-filesmake gen-versionsFor PR reviewers
A note for code reviewers - all pull requests must have the following:
kind/bugif this is a bugfix.kind/enhancementif this is a a new feature.enterpriseif this PR applies to Calico Enterprise only.