DRA: AdminAccess validate based on namespace label

Signed-off-by: Rita Zhang <[email protected]>

Author: ritazh

pkg/apis/resource/types.go

@@ -385,6 +385,15 @@ const (
 	DeviceConfigMaxSize      = 32
 )
 
+// DRAAdminNamespaceLabelKey is a label key used to grant administrative access
+// to certain resource.k8s.io API types within a namespace. When this label is
+// set on a namespace with the value "true" (case-sensitive), it allows the use
+// of adminAccess: true in any namespaced resource.k8s.io API types. Currently,
+// this permission applies to ResourceClaim and ResourceClaimTemplate objects.
+const (
+	DRAAdminNamespaceLabelKey = "resource.k8s.io/admin-access"
+)
+
 // DeviceRequest is a request for devices required for a claim.
 // This is typically a request for a single resource like a device, but can
 // also ask for several identical devices.

pkg/controller/resourceclaim/controller_test.go

@@ -40,6 +40,7 @@ import (
 	"k8s.io/kubernetes/pkg/controller"
 	"k8s.io/kubernetes/pkg/controller/resourceclaim/metrics"
 	"k8s.io/kubernetes/test/utils/ktesting"
+	"k8s.io/utils/ptr"
 )
 
 var (
@@ -61,13 +62,15 @@ var (
 	testClaimReserved      = reserveClaim(testClaimAllocated, testPodWithResource)
 	testClaimReservedTwice = reserveClaim(testClaimReserved, otherTestPod)
 
-	generatedTestClaim          = makeGeneratedClaim(podResourceClaimName, testPodName+"-"+podResourceClaimName+"-", testNamespace, className, 1, makeOwnerReference(testPodWithResource, true))
+	generatedTestClaim          = makeGeneratedClaim(podResourceClaimName, testPodName+"-"+podResourceClaimName+"-", testNamespace, className, 1, makeOwnerReference(testPodWithResource, true), nil)
+	generatedTestClaimWithAdmin = makeGeneratedClaim(podResourceClaimName, testPodName+"-"+podResourceClaimName+"-", testNamespace, className, 1, makeOwnerReference(testPodWithResource, true), ptr.To(true))
 	generatedTestClaimAllocated = allocateClaim(generatedTestClaim)
 	generatedTestClaimReserved  = reserveClaim(generatedTestClaimAllocated, testPodWithResource)
 
-	conflictingClaim    = makeClaim(testPodName+"-"+podResourceClaimName, testNamespace, className, nil)
-	otherNamespaceClaim = makeClaim(testPodName+"-"+podResourceClaimName, otherNamespace, className, nil)
-	template            = makeTemplate(templateName, testNamespace, className)
+	conflictingClaim        = makeClaim(testPodName+"-"+podResourceClaimName, testNamespace, className, nil)
+	otherNamespaceClaim     = makeClaim(testPodName+"-"+podResourceClaimName, otherNamespace, className, nil)
+	template                = makeTemplate(templateName, testNamespace, className, nil)
+	templateWithAdminAccess = makeTemplate(templateName, testNamespace, className, ptr.To(true))
 
 	testPodWithNodeName = func() *v1.Pod {
 		pod := testPodWithResource.DeepCopy()
@@ -78,6 +81,7 @@ var (
 		})
 		return pod
 	}()
+	adminAccessFeatureOffError = "admin access is requested, but the feature is disabled"
 )
 
 func TestSyncHandler(t *testing.T) {
@@ -93,7 +97,7 @@ func TestSyncHandler(t *testing.T) {
 		templates              []*resourceapi.ResourceClaimTemplate
 		expectedClaims         []resourceapi.ResourceClaim
 		expectedStatuses       map[string][]v1.PodResourceClaimStatus
-		expectedError          bool
+		expectedError          string
 		expectedMetrics        expectedMetrics
 	}{
 		{
@@ -109,6 +113,27 @@ func TestSyncHandler(t *testing.T) {
 			},
 			expectedMetrics: expectedMetrics{1, 0},
 		},
+		{
+			name:          "create with admin and feature gate off",
+			pods:          []*v1.Pod{testPodWithResource},
+			templates:     []*resourceapi.ResourceClaimTemplate{templateWithAdminAccess},
+			key:           podKey(testPodWithResource),
+			expectedError: adminAccessFeatureOffError,
+		},
+		{
+			name:           "create with admin and feature gate on",
+			pods:           []*v1.Pod{testPodWithResource},
+			templates:      []*resourceapi.ResourceClaimTemplate{templateWithAdminAccess},
+			key:            podKey(testPodWithResource),
+			expectedClaims: []resourceapi.ResourceClaim{*generatedTestClaimWithAdmin},
+			expectedStatuses: map[string][]v1.PodResourceClaimStatus{
+				testPodWithResource.Name: {
+					{Name: testPodWithResource.Spec.ResourceClaims[0].Name, ResourceClaimName: &generatedTestClaimWithAdmin.Name},
+				},
+			},
+			adminAccessEnabled: true,
+			expectedMetrics:    expectedMetrics{1, 0},
+		},
 		{
 			name: "nop",
 			pods: []*v1.Pod{func() *v1.Pod {
@@ -153,7 +178,7 @@ func TestSyncHandler(t *testing.T) {
 			pods:          []*v1.Pod{testPodWithResource},
 			templates:     nil,
 			key:           podKey(testPodWithResource),
-			expectedError: true,
+			expectedError: "resource claim template \"my-template\": resourceclaimtemplate.resource.k8s.io \"my-template\" not found",
 		},
 		{
 			name:           "find-existing-claim-by-label",
@@ -219,15 +244,15 @@ func TestSyncHandler(t *testing.T) {
 			key:            podKey(testPodWithResource),
 			claims:         []*resourceapi.ResourceClaim{conflictingClaim},
 			expectedClaims: []resourceapi.ResourceClaim{*conflictingClaim},
-			expectedError:  true,
+			expectedError:  "resource claim template \"my-template\": resourceclaimtemplate.resource.k8s.io \"my-template\" not found",
 		},
 		{
 			name:            "create-conflict",
 			pods:            []*v1.Pod{testPodWithResource},
 			templates:       []*resourceapi.ResourceClaimTemplate{template},
 			key:             podKey(testPodWithResource),
 			expectedMetrics: expectedMetrics{1, 1},
-			expectedError:   true,
+			expectedError:   "create ResourceClaim : Operation cannot be fulfilled on resourceclaims.resource.k8s.io \"fake name\": fake conflict",
 		},
 		{
 			name:            "stay-reserved-seen",
@@ -424,11 +449,12 @@ func TestSyncHandler(t *testing.T) {
 			}
 
 			err = ec.syncHandler(tCtx, tc.key)
-			if err != nil && !tc.expectedError {
-				t.Fatalf("unexpected error while running handler: %v", err)
+			if err != nil {
+				assert.ErrorContains(t, err, tc.expectedError, "the error message should have contained the expected error message")
+				return
 			}
-			if err == nil && tc.expectedError {
-				t.Fatalf("unexpected success")
+			if tc.expectedError != "" {
+				t.Fatalf("expected error, got none")
 			}
 
 			claims, err := fakeKubeClient.ResourceV1beta1().ResourceClaims("").List(tCtx, metav1.ListOptions{})
@@ -558,7 +584,7 @@ func makeClaim(name, namespace, classname string, owner *metav1.OwnerReference)
 	return claim
 }
 
-func makeGeneratedClaim(podClaimName, generateName, namespace, classname string, createCounter int, owner *metav1.OwnerReference) *resourceapi.ResourceClaim {
+func makeGeneratedClaim(podClaimName, generateName, namespace, classname string, createCounter int, owner *metav1.OwnerReference, adminAccess *bool) *resourceapi.ResourceClaim {
 	claim := &resourceapi.ResourceClaim{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:         fmt.Sprintf("%s-%d", generateName, createCounter),
@@ -570,6 +596,19 @@ func makeGeneratedClaim(podClaimName, generateName, namespace, classname string,
 	if owner != nil {
 		claim.OwnerReferences = []metav1.OwnerReference{*owner}
 	}
+	if adminAccess != nil {
+		claim.Spec = resourceapi.ResourceClaimSpec{
+			Devices: resourceapi.DeviceClaim{
+				Requests: []resourceapi.DeviceRequest{
+					{
+						Name:            "req-0",
+						DeviceClassName: "class",
+						AdminAccess:     adminAccess,
+					},
+				},
+			},
+		}
+	}
 
 	return claim
 }
@@ -618,10 +657,25 @@ func makePod(name, namespace string, uid types.UID, podClaims ...v1.PodResourceC
 	return pod
 }
 
-func makeTemplate(name, namespace, classname string) *resourceapi.ResourceClaimTemplate {
+func makeTemplate(name, namespace, classname string, adminAccess *bool) *resourceapi.ResourceClaimTemplate {
 	template := &resourceapi.ResourceClaimTemplate{
 		ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: namespace},
 	}
+	if adminAccess != nil {
+		template.Spec = resourceapi.ResourceClaimTemplateSpec{
+			Spec: resourceapi.ResourceClaimSpec{
+				Devices: resourceapi.DeviceClaim{
+					Requests: []resourceapi.DeviceRequest{
+						{
+							Name:            "req-0",
+							DeviceClassName: "class",
+							AdminAccess:     adminAccess,
+						},
+					},
+				},
+			},
+		}
+	}
 	return template
 }
 

pkg/controlplane/instance.go

@@ -63,7 +63,6 @@ import (
 	genericapiserver "k8s.io/apiserver/pkg/server"
 	serverstorage "k8s.io/apiserver/pkg/server/storage"
 	utilfeature "k8s.io/apiserver/pkg/util/feature"
-	clientdiscovery "k8s.io/client-go/discovery"
 	"k8s.io/client-go/kubernetes"
 	corev1client "k8s.io/client-go/kubernetes/typed/core/v1"
 	discoveryclient "k8s.io/client-go/kubernetes/typed/discovery/v1"
@@ -328,7 +327,7 @@ func (c CompletedConfig) New(delegationTarget genericapiserver.DelegationTarget)
 		return nil, err
 	}
 
-	restStorageProviders, err := c.StorageProviders(client.Discovery())
+	restStorageProviders, err := c.StorageProviders(client)
 	if err != nil {
 		return nil, err
 	}
@@ -379,7 +378,7 @@ func (c CompletedConfig) New(delegationTarget genericapiserver.DelegationTarget)
 
 }
 
-func (c CompletedConfig) StorageProviders(discovery clientdiscovery.DiscoveryInterface) ([]controlplaneapiserver.RESTStorageProvider, error) {
+func (c CompletedConfig) StorageProviders(client *kubernetes.Clientset) ([]controlplaneapiserver.RESTStorageProvider, error) {
 	legacyRESTStorageProvider, err := corerest.New(corerest.Config{
 		GenericConfig: *c.ControlPlane.NewCoreGenericConfig(),
 		Proxy: corerest.ProxyConfig{
@@ -425,9 +424,9 @@ func (c CompletedConfig) StorageProviders(discovery clientdiscovery.DiscoveryInt
 		// keep apps after extensions so legacy clients resolve the extensions versions of shared resource names.
 		// See https://github.com/kubernetes/kubernetes/issues/42392
 		appsrest.StorageProvider{},
-		admissionregistrationrest.RESTStorageProvider{Authorizer: c.ControlPlane.Generic.Authorization.Authorizer, DiscoveryClient: discovery},
+		admissionregistrationrest.RESTStorageProvider{Authorizer: c.ControlPlane.Generic.Authorization.Authorizer, DiscoveryClient: client.Discovery()},
 		eventsrest.RESTStorageProvider{TTL: c.ControlPlane.EventTTL},
-		resourcerest.RESTStorageProvider{},
+		resourcerest.RESTStorageProvider{NamespaceClient: client.CoreV1().Namespaces()},
 	}, nil
 }
 

pkg/controlplane/instance_test.go

@@ -490,7 +490,7 @@ func TestGenericStorageProviders(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	kube, err := completed.StorageProviders(client.Discovery())
+	kube, err := completed.StorageProviders(client)
 	if err != nil {
 		t.Fatal(err)
 	}

pkg/registry/resource/resourceclaim/storage/storage.go

@@ -18,12 +18,14 @@ package storage
 
 import (
 	"context"
+	"fmt"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apiserver/pkg/registry/generic"
 	genericregistry "k8s.io/apiserver/pkg/registry/generic/registry"
 	"k8s.io/apiserver/pkg/registry/rest"
+	"k8s.io/client-go/kubernetes/typed/core/v1"
 	"k8s.io/kubernetes/pkg/apis/resource"
 	"k8s.io/kubernetes/pkg/printers"
 	printersinternal "k8s.io/kubernetes/pkg/printers/internalversion"
@@ -38,19 +40,23 @@ type REST struct {
 }
 
 // NewREST returns a RESTStorage object that will work against ResourceClaims.
-func NewREST(optsGetter generic.RESTOptionsGetter) (*REST, *StatusREST, error) {
+func NewREST(optsGetter generic.RESTOptionsGetter, nsClient v1.NamespaceInterface) (*REST, *StatusREST, error) {
+	if nsClient == nil {
+		return nil, nil, fmt.Errorf("namespace client is required")
+	}
+	strategy := resourceclaim.NewStrategy(nsClient)
 	store := &genericregistry.Store{
 		NewFunc:                   func() runtime.Object { return &resource.ResourceClaim{} },
 		NewListFunc:               func() runtime.Object { return &resource.ResourceClaimList{} },
 		PredicateFunc:             resourceclaim.Match,
 		DefaultQualifiedResource:  resource.Resource("resourceclaims"),
 		SingularQualifiedResource: resource.Resource("resourceclaim"),
 
-		CreateStrategy:      resourceclaim.Strategy,
-		UpdateStrategy:      resourceclaim.Strategy,
-		DeleteStrategy:      resourceclaim.Strategy,
+		CreateStrategy:      strategy,
+		UpdateStrategy:      strategy,
+		DeleteStrategy:      strategy,
 		ReturnDeletedObject: true,
-		ResetFieldsStrategy: resourceclaim.Strategy,
+		ResetFieldsStrategy: strategy,
 
 		TableConvertor: printerstorage.TableConvertor{TableGenerator: printers.NewTableGenerator().With(printersinternal.AddHandlers)},
 	}
@@ -60,8 +66,9 @@ func NewREST(optsGetter generic.RESTOptionsGetter) (*REST, *StatusREST, error) {
 	}
 
 	statusStore := *store
-	statusStore.UpdateStrategy = resourceclaim.StatusStrategy
-	statusStore.ResetFieldsStrategy = resourceclaim.StatusStrategy
+	statusStrategy := resourceclaim.NewStatusStrategy(strategy)
+	statusStore.UpdateStrategy = statusStrategy
+	statusStore.ResetFieldsStrategy = statusStrategy
 
 	rest := &REST{store}
 

pkg/registry/resource/resourceclaim/storage/storage_test.go

@@ -30,6 +30,7 @@ import (
 	genericregistrytest "k8s.io/apiserver/pkg/registry/generic/testing"
 	"k8s.io/apiserver/pkg/registry/rest"
 	etcd3testing "k8s.io/apiserver/pkg/storage/etcd3/testing"
+	"k8s.io/client-go/kubernetes/fake"
 	"k8s.io/kubernetes/pkg/apis/resource"
 	_ "k8s.io/kubernetes/pkg/apis/resource/install"
 	"k8s.io/kubernetes/pkg/registry/registrytest"
@@ -43,7 +44,9 @@ func newStorage(t *testing.T) (*REST, *StatusREST, *etcd3testing.EtcdTestServer)
 		DeleteCollectionWorkers: 1,
 		ResourcePrefix:          "resourceclaims",
 	}
-	resourceClaimStorage, statusStorage, err := NewREST(restOptions)
+	fakeClient := fake.NewSimpleClientset()
+	mockNSClient := fakeClient.CoreV1().Namespaces()
+	resourceClaimStorage, statusStorage, err := NewREST(restOptions, mockNSClient)
 	if err != nil {
 		t.Fatalf("unexpected error from REST storage: %v", err)
 	}