Merge pull request #60145 from php-coder/psp_authz_via_policy_group

Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

PSP plugin: allow authorizing via "use" verb in policy API group

**What this PR does / why we need it**:
In order to determine whether a service account/user has access to PSP, PodSecurityPolicy admission plugin tests whether a service account/user is authorized for "use" verb in `extensions` API group. As PSP is being migrated to `policy` API group, we need to support its new location. This PR adds such a support by checking in both API groups.

**Which issue(s) this PR fixes**:
Addressed to: https://github.com/kubernetes/features/issues/5
Follow-up to: https://github.com/kubernetes/kubernetes/pull/54933

examples/podsecuritypolicy/rbac/roles.yaml

@@ -5,7 +5,7 @@ metadata:
   name: restricted-psp-user
 rules:
 - apiGroups:
-  - extensions
+  - policy
   resources:
   - podsecuritypolicies
   resourceNames:
@@ -20,7 +20,7 @@ metadata:
   name: privileged-psp-user
 rules:
 - apiGroups:
-  - extensions
+  - policy
   resources:
   - podsecuritypolicies
   resourceNames:

plugin/pkg/admission/security/podsecuritypolicy/BUILD

@@ -13,6 +13,7 @@ go_library(
     deps = [
         "//pkg/apis/core:go_default_library",
         "//pkg/apis/extensions:go_default_library",
+        "//pkg/apis/policy:go_default_library",
         "//pkg/client/informers/informers_generated/internalversion:go_default_library",
         "//pkg/client/listers/extensions/internalversion:go_default_library",
         "//pkg/kubeapiserver/admission:go_default_library",
@@ -40,6 +41,7 @@ go_test(
         "//pkg/apis/core:go_default_library",
         "//pkg/apis/core/helper:go_default_library",
         "//pkg/apis/extensions:go_default_library",
+        "//pkg/apis/policy:go_default_library",
         "//pkg/client/informers/informers_generated/internalversion:go_default_library",
         "//pkg/controller:go_default_library",
         "//pkg/security/apparmor:go_default_library",

plugin/pkg/admission/security/podsecuritypolicy/admission.go

@@ -33,6 +33,7 @@ import (
 	"k8s.io/apiserver/pkg/authorization/authorizer"
 	api "k8s.io/kubernetes/pkg/apis/core"
 	"k8s.io/kubernetes/pkg/apis/extensions"
+	"k8s.io/kubernetes/pkg/apis/policy"
 	informers "k8s.io/kubernetes/pkg/client/informers/informers_generated/internalversion"
 	extensionslisters "k8s.io/kubernetes/pkg/client/listers/extensions/internalversion"
 	kubeapiserveradmission "k8s.io/kubernetes/pkg/kubeapiserver/admission"
@@ -354,11 +355,19 @@ func isAuthorizedForPolicy(user, sa user.Info, namespace, policyName string, aut
 }
 
 // authorizedForPolicy returns true if info is authorized to perform the "use" verb on the policy resource.
+// TODO: check against only the policy group when PSP will be completely moved out of the extensions
 func authorizedForPolicy(info user.Info, namespace string, policyName string, authz authorizer.Authorizer) bool {
+	// Check against extensions API group for backward compatibility
+	return authorizedForPolicyInAPIGroup(info, namespace, policyName, policy.GroupName, authz) ||
+		authorizedForPolicyInAPIGroup(info, namespace, policyName, extensions.GroupName, authz)
+}
+
+// authorizedForPolicyInAPIGroup returns true if info is authorized to perform the "use" verb on the policy resource in the specified API group.
+func authorizedForPolicyInAPIGroup(info user.Info, namespace, policyName, apiGroupName string, authz authorizer.Authorizer) bool {
 	if info == nil {
 		return false
 	}
-	attr := buildAttributes(info, namespace, policyName)
+	attr := buildAttributes(info, namespace, policyName, apiGroupName)
 	decision, reason, err := authz.Authorize(attr)
 	if err != nil {
 		glog.V(5).Infof("cannot authorize for policy: %v,%v", reason, err)
@@ -367,14 +376,14 @@ func authorizedForPolicy(info user.Info, namespace string, policyName string, au
 }
 
 // buildAttributes builds an attributes record for a SAR based on the user info and policy.
-func buildAttributes(info user.Info, namespace string, policyName string) authorizer.Attributes {
+func buildAttributes(info user.Info, namespace, policyName, apiGroupName string) authorizer.Attributes {
 	// check against the namespace that the pod is being created in to allow per-namespace PSP grants.
 	attr := authorizer.AttributesRecord{
 		User:            info,
 		Verb:            "use",
 		Namespace:       namespace,
 		Name:            policyName,
-		APIGroup:        extensions.GroupName,
+		APIGroup:        apiGroupName,
 		Resource:        "podsecuritypolicies",
 		ResourceRequest: true,
 	}

plugin/pkg/admission/security/podsecuritypolicy/admission_test.go

@@ -37,6 +37,7 @@ import (
 	kapi "k8s.io/kubernetes/pkg/apis/core"
 	"k8s.io/kubernetes/pkg/apis/core/helper"
 	"k8s.io/kubernetes/pkg/apis/extensions"
+	"k8s.io/kubernetes/pkg/apis/policy"
 	informers "k8s.io/kubernetes/pkg/client/informers/informers_generated/internalversion"
 	"k8s.io/kubernetes/pkg/controller"
 	"k8s.io/kubernetes/pkg/security/apparmor"
@@ -71,6 +72,11 @@ type TestAuthorizer struct {
 	// usernameToNamespaceToAllowedPSPs contains the map of allowed PSPs.
 	// if nil, all PSPs are allowed.
 	usernameToNamespaceToAllowedPSPs map[string]map[string]map[string]bool
+	// allowedAPIGroupName specifies an API Group name that contains PSP resources.
+	// In order to be authorized, AttributesRecord must have this group name.
+	// When empty, API Group name isn't taken into account.
+	// TODO: remove this when PSP will be completely moved out of the extensions and we'll lookup only in "policy" group.
+	allowedAPIGroupName string
 }
 
 func (t *TestAuthorizer) Authorize(a authorizer.Attributes) (authorized authorizer.Decision, reason string, err error) {
@@ -79,7 +85,8 @@ func (t *TestAuthorizer) Authorize(a authorizer.Attributes) (authorized authoriz
 	}
 	allowedInNamespace := t.usernameToNamespaceToAllowedPSPs[a.GetUser().GetName()][a.GetNamespace()][a.GetName()]
 	allowedClusterWide := t.usernameToNamespaceToAllowedPSPs[a.GetUser().GetName()][""][a.GetName()]
-	if allowedInNamespace || allowedClusterWide {
+	allowedAPIGroup := len(t.allowedAPIGroupName) == 0 || a.GetAPIGroup() == t.allowedAPIGroupName
+	if allowedAPIGroup && (allowedInNamespace || allowedClusterWide) {
 		return authorizer.DecisionAllow, "", nil
 	}
 	return authorizer.DecisionNoOpinion, "", nil
@@ -1996,8 +2003,9 @@ func TestPolicyAuthorization(t *testing.T) {
 		expectedPolicy string
 		inPolicies     []*extensions.PodSecurityPolicy
 		allowed        map[string]map[string]map[string]bool
+		allowedGroup   string
 	}{
-		"policy allowed by user": {
+		"policy allowed by user (extensions API Group)": {
 			user: &user.DefaultInfo{Name: "user"},
 			sa:   "sa",
 			ns:   "test",
@@ -2009,7 +2017,7 @@ func TestPolicyAuthorization(t *testing.T) {
 			inPolicies:     []*extensions.PodSecurityPolicy{policyWithName("policy")},
 			expectedPolicy: "policy",
 		},
-		"policy allowed by sa": {
+		"policy allowed by sa (extensions API Group)": {
 			user: &user.DefaultInfo{Name: "user"},
 			sa:   "sa",
 			ns:   "test",
@@ -2021,6 +2029,32 @@ func TestPolicyAuthorization(t *testing.T) {
 			inPolicies:     []*extensions.PodSecurityPolicy{policyWithName("policy")},
 			expectedPolicy: "policy",
 		},
+		"policy allowed by user (policy API Group)": {
+			user: &user.DefaultInfo{Name: "user"},
+			sa:   "sa",
+			ns:   "test",
+			allowed: map[string]map[string]map[string]bool{
+				"user": {
+					"test": {"policy": true},
+				},
+			},
+			inPolicies:     []*extensions.PodSecurityPolicy{policyWithName("policy")},
+			expectedPolicy: "policy",
+			allowedGroup:   policy.GroupName,
+		},
+		"policy allowed by sa (policy API Group)": {
+			user: &user.DefaultInfo{Name: "user"},
+			sa:   "sa",
+			ns:   "test",
+			allowed: map[string]map[string]map[string]bool{
+				serviceaccount.MakeUsername("test", "sa"): {
+					"test": {"policy": true},
+				},
+			},
+			inPolicies:     []*extensions.PodSecurityPolicy{policyWithName("policy")},
+			expectedPolicy: "policy",
+			allowedGroup:   policy.GroupName,
+		},
 		"no policies allowed": {
 			user:           &user.DefaultInfo{Name: "user"},
 			sa:             "sa",
@@ -2122,7 +2156,7 @@ func TestPolicyAuthorization(t *testing.T) {
 		var (
 			oldPod     *kapi.Pod
 			shouldPass = v.expectedPolicy != ""
-			authz      = &TestAuthorizer{usernameToNamespaceToAllowedPSPs: v.allowed}
+			authz      = &TestAuthorizer{usernameToNamespaceToAllowedPSPs: v.allowed, allowedAPIGroupName: v.allowedGroup}
 			canMutate  = true
 		)
 		pod := goodPod()