refactor: make pod-security-policy addon user-configurable #2463
Conversation
jackfrancis
force-pushed
the
addons-psp
branch
from
e9198e1
to
b62b1ec
Compare
jackfrancis
changed the title
| @@ -752,6 +758,13 @@ func (cs *ContainerService) setAddonsConfig(isUpgrade bool) { | |||
| // Remove deprecated "kube-proxy-daemonset addon" | |||
| o.KubernetesConfig.Addons = append(o.KubernetesConfig.Addons[:i], o.KubernetesConfig.Addons[i+1:]...) | |||
| } | |||
|
|
|||
| // Enable pod-security-policy addon during upgrade to 1.15 or greater scenarios, unless explicitly disabled | |||
| if isUpgrade && common.IsKubernetesVersionGe(o.OrchestratorVersion, "1.15.0") && !o.KubernetesConfig.IsAddonDisabled(common.PodSecurityPolicyAddonName) { | |||
There was a problem hiding this comment.
Being a bit conservative here to allow folks who definitely don't want the aks-engine PodSecurityPolicy implementation to explicitly disable it, and for upgrade to honor that.
| o.KubernetesConfig.EnablePodSecurityPolicy = to.BoolPtr(true) | ||
| o.KubernetesConfig.PodSecurityPolicyConfig = map[string]string{ | ||
| "data": base64DataPSP, | ||
| o.KubernetesConfig.Addons = []KubernetesAddon{ |
There was a problem hiding this comment.
Slightly academic, but keeping this UT around and converting it to make sense in the new pod-security-policy addons context
| @@ -3428,4 +3507,13 @@ func getDefaultAddons(version string) []KubernetesAddon { | |||
| }, | |||
| }, | |||
| } | |||
|
|
|||
| if common.IsKubernetesVersionGe(version, "1.15.0") { | |||
There was a problem hiding this comment.
Yay w/ the new addons UT interfaces we just add default addons in one place and get that coverage everywhere
| @@ -166,7 +166,7 @@ func getDefaultAdmissionControls(cs *ContainerService) (string, string) { | |||
| admissionControlValues := "NamespaceLifecycle,LimitRanger,ServiceAccount,DefaultStorageClass,DefaultTolerationSeconds,ValidatingAdmissionWebhook,ResourceQuota,ExtendedResourceToleration" | |||
|
|
|||
| // Pod Security Policy configuration | |||
| if to.Bool(o.KubernetesConfig.EnablePodSecurityPolicy) { | |||
| if o.KubernetesConfig.IsAddonEnabled(common.PodSecurityPolicyAddonName) { | |||
There was a problem hiding this comment.
validated that this apiserver defaults flow comes after the addons defaults flow (feel free to do so yourself, otherwise this would not work)
| @@ -345,10 +345,6 @@ func (cs *ContainerService) setOrchestratorDefaults(isUpgrade, isScale bool) { | |||
| a.OrchestratorProfile.KubernetesConfig.ExcludeMasterFromStandardLB = to.BoolPtr(DefaultExcludeMasterFromStandardLB) | |||
| } | |||
|
|
|||
| if common.IsKubernetesVersionGe(a.OrchestratorProfile.OrchestratorVersion, "1.15.0-beta.1") { | |||
There was a problem hiding this comment.
This is the concrete deprecation of the EnablePodSecurityPolicy flag for new cluster creates (i.e., we are basically ignoring it)
| @@ -3175,7 +3175,7 @@ func TestDefaultEnablePodSecurityPolicy(t *testing.T) { | |||
| MasterProfile: &MasterProfile{}, | |||
| }, | |||
| }, | |||
| expected: true, | |||
| expected: false, | |||
There was a problem hiding this comment.
these UT changes reflect that EnablePodSecurityPolicy is now functionally deprecated
| if !o.KubernetesConfig.IsRBACEnabled() { | ||
| return errors.Errorf("enablePodSecurityPolicy requires the enableRbac feature as a prerequisite") | ||
| } | ||
| minVersion, err := semver.Make("1.8.0") |
There was a problem hiding this comment.
Cleaning up this old validation while I'm here
Codecov Report
@@ Coverage Diff @@
## master #2463 +/- ##
==========================================
+ Coverage 72.57% 72.57% +<.01%
==========================================
Files 130 130
Lines 23913 23916 +3
==========================================
+ Hits 17354 17358 +4
Misses 5530 5530
+ Partials 1029 1028 -1 |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jackfrancis, mboersma The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
Related to #2251 |
Reason for Change:
This PR makes the
pod-security-policycomponent a user-configurable addon, exposed via the existingkubernetesConfig.addonsinterface.As an outcome, we officially deprecate the
enablePodSecurityPolicykubernetesConfigoption, as it is functionally equivalent to:The deprecation behavior for new cluster creates is to essentially ignore the
enablePodSecurityPolicyproperty (i.e., we don't throw a validation error). We also maintain pre-existing back-compat scenarios, and move theenablePodSecurityPolicysetting into thepod-security-policyaddon, as appropriate, during upgrade.Issue Fixed:
Requirements:
Notes: