Ensure version "*" is passed instead of "" for all authz checks (#116937)
* ensure version * is passed instead of for all authz checks * unexport match function * remove allversion constant
This commit is contained in:
		@@ -29,7 +29,6 @@ import (
 | 
				
			|||||||
	"k8s.io/apimachinery/pkg/util/sets"
 | 
						"k8s.io/apimachinery/pkg/util/sets"
 | 
				
			||||||
	certificatesinformers "k8s.io/client-go/informers/certificates/v1"
 | 
						certificatesinformers "k8s.io/client-go/informers/certificates/v1"
 | 
				
			||||||
	clientset "k8s.io/client-go/kubernetes"
 | 
						clientset "k8s.io/client-go/kubernetes"
 | 
				
			||||||
 | 
					 | 
				
			||||||
	capihelper "k8s.io/kubernetes/pkg/apis/certificates"
 | 
						capihelper "k8s.io/kubernetes/pkg/apis/certificates"
 | 
				
			||||||
	"k8s.io/kubernetes/pkg/controller/certificates"
 | 
						"k8s.io/kubernetes/pkg/controller/certificates"
 | 
				
			||||||
)
 | 
					)
 | 
				
			||||||
@@ -63,12 +62,12 @@ func recognizers() []csrRecognizer {
 | 
				
			|||||||
	recognizers := []csrRecognizer{
 | 
						recognizers := []csrRecognizer{
 | 
				
			||||||
		{
 | 
							{
 | 
				
			||||||
			recognize:      isSelfNodeClientCert,
 | 
								recognize:      isSelfNodeClientCert,
 | 
				
			||||||
			permission:     authorization.ResourceAttributes{Group: "certificates.k8s.io", Resource: "certificatesigningrequests", Verb: "create", Subresource: "selfnodeclient"},
 | 
								permission:     authorization.ResourceAttributes{Group: "certificates.k8s.io", Resource: "certificatesigningrequests", Verb: "create", Subresource: "selfnodeclient", Version: "*"},
 | 
				
			||||||
			successMessage: "Auto approving self kubelet client certificate after SubjectAccessReview.",
 | 
								successMessage: "Auto approving self kubelet client certificate after SubjectAccessReview.",
 | 
				
			||||||
		},
 | 
							},
 | 
				
			||||||
		{
 | 
							{
 | 
				
			||||||
			recognize:      isNodeClientCert,
 | 
								recognize:      isNodeClientCert,
 | 
				
			||||||
			permission:     authorization.ResourceAttributes{Group: "certificates.k8s.io", Resource: "certificatesigningrequests", Verb: "create", Subresource: "nodeclient"},
 | 
								permission:     authorization.ResourceAttributes{Group: "certificates.k8s.io", Resource: "certificatesigningrequests", Verb: "create", Subresource: "nodeclient", Version: "*"},
 | 
				
			||||||
			successMessage: "Auto approving kubelet client certificate after SubjectAccessReview.",
 | 
								successMessage: "Auto approving kubelet client certificate after SubjectAccessReview.",
 | 
				
			||||||
		},
 | 
							},
 | 
				
			||||||
	}
 | 
						}
 | 
				
			||||||
 
 | 
				
			|||||||
@@ -19,11 +19,10 @@ package subjectaccessreview
 | 
				
			|||||||
import (
 | 
					import (
 | 
				
			||||||
	"context"
 | 
						"context"
 | 
				
			||||||
	"errors"
 | 
						"errors"
 | 
				
			||||||
 | 
						"reflect"
 | 
				
			||||||
	"strings"
 | 
						"strings"
 | 
				
			||||||
	"testing"
 | 
						"testing"
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	"reflect"
 | 
					 | 
				
			||||||
 | 
					 | 
				
			||||||
	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 | 
						metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 | 
				
			||||||
	"k8s.io/apiserver/pkg/authentication/user"
 | 
						"k8s.io/apiserver/pkg/authentication/user"
 | 
				
			||||||
	"k8s.io/apiserver/pkg/authorization/authorizer"
 | 
						"k8s.io/apiserver/pkg/authorization/authorizer"
 | 
				
			||||||
@@ -181,6 +180,7 @@ func TestCreate(t *testing.T) {
 | 
				
			|||||||
			expectedAttrs: authorizer.AttributesRecord{
 | 
								expectedAttrs: authorizer.AttributesRecord{
 | 
				
			||||||
				User:            &user.DefaultInfo{Name: "bob"},
 | 
									User:            &user.DefaultInfo{Name: "bob"},
 | 
				
			||||||
				ResourceRequest: true,
 | 
									ResourceRequest: true,
 | 
				
			||||||
 | 
									APIVersion:      "*",
 | 
				
			||||||
			},
 | 
								},
 | 
				
			||||||
			expectedStatus: authorizationapi.SubjectAccessReviewStatus{
 | 
								expectedStatus: authorizationapi.SubjectAccessReviewStatus{
 | 
				
			||||||
				Allowed: false,
 | 
									Allowed: false,
 | 
				
			||||||
 
 | 
				
			|||||||
@@ -29,7 +29,7 @@ func ResourceAttributesFrom(user user.Info, in authorizationapi.ResourceAttribut
 | 
				
			|||||||
		Verb:            in.Verb,
 | 
							Verb:            in.Verb,
 | 
				
			||||||
		Namespace:       in.Namespace,
 | 
							Namespace:       in.Namespace,
 | 
				
			||||||
		APIGroup:        in.Group,
 | 
							APIGroup:        in.Group,
 | 
				
			||||||
		APIVersion:      in.Version,
 | 
							APIVersion:      matchAllVersionIfEmpty(in.Version),
 | 
				
			||||||
		Resource:        in.Resource,
 | 
							Resource:        in.Resource,
 | 
				
			||||||
		Subresource:     in.Subresource,
 | 
							Subresource:     in.Subresource,
 | 
				
			||||||
		Name:            in.Name,
 | 
							Name:            in.Name,
 | 
				
			||||||
@@ -77,3 +77,11 @@ func AuthorizationAttributesFrom(spec authorizationapi.SubjectAccessReviewSpec)
 | 
				
			|||||||
 | 
					
 | 
				
			||||||
	return authorizationAttributes
 | 
						return authorizationAttributes
 | 
				
			||||||
}
 | 
					}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					// matchAllVersionIfEmpty returns a "*" if the version is unspecified
 | 
				
			||||||
 | 
					func matchAllVersionIfEmpty(version string) string {
 | 
				
			||||||
 | 
						if len(version) == 0 {
 | 
				
			||||||
 | 
							return "*"
 | 
				
			||||||
 | 
						}
 | 
				
			||||||
 | 
						return version
 | 
				
			||||||
 | 
					}
 | 
				
			||||||
 
 | 
				
			|||||||
@@ -133,6 +133,35 @@ func TestAuthorizationAttributesFrom(t *testing.T) {
 | 
				
			|||||||
				ResourceRequest: true,
 | 
									ResourceRequest: true,
 | 
				
			||||||
			},
 | 
								},
 | 
				
			||||||
		},
 | 
							},
 | 
				
			||||||
 | 
							{
 | 
				
			||||||
 | 
								name: "resource with no version",
 | 
				
			||||||
 | 
								args: args{
 | 
				
			||||||
 | 
									spec: authorizationapi.SubjectAccessReviewSpec{
 | 
				
			||||||
 | 
										User: "bob",
 | 
				
			||||||
 | 
										ResourceAttributes: &authorizationapi.ResourceAttributes{
 | 
				
			||||||
 | 
											Namespace:   "myns",
 | 
				
			||||||
 | 
											Verb:        "create",
 | 
				
			||||||
 | 
											Group:       "extensions",
 | 
				
			||||||
 | 
											Resource:    "deployments",
 | 
				
			||||||
 | 
											Subresource: "scale",
 | 
				
			||||||
 | 
											Name:        "mydeployment",
 | 
				
			||||||
 | 
										},
 | 
				
			||||||
 | 
									},
 | 
				
			||||||
 | 
								},
 | 
				
			||||||
 | 
								want: authorizer.AttributesRecord{
 | 
				
			||||||
 | 
									User: &user.DefaultInfo{
 | 
				
			||||||
 | 
										Name: "bob",
 | 
				
			||||||
 | 
									},
 | 
				
			||||||
 | 
									APIGroup:        "extensions",
 | 
				
			||||||
 | 
									APIVersion:      "*",
 | 
				
			||||||
 | 
									Namespace:       "myns",
 | 
				
			||||||
 | 
									Verb:            "create",
 | 
				
			||||||
 | 
									Resource:        "deployments",
 | 
				
			||||||
 | 
									Subresource:     "scale",
 | 
				
			||||||
 | 
									Name:            "mydeployment",
 | 
				
			||||||
 | 
									ResourceRequest: true,
 | 
				
			||||||
 | 
								},
 | 
				
			||||||
 | 
							},
 | 
				
			||||||
	}
 | 
						}
 | 
				
			||||||
	for _, tt := range tests {
 | 
						for _, tt := range tests {
 | 
				
			||||||
		t.Run(tt.name, func(t *testing.T) {
 | 
							t.Run(tt.name, func(t *testing.T) {
 | 
				
			||||||
 
 | 
				
			|||||||
		Reference in New Issue
	
	Block a user