DRA: treat AdminAccess as a new feature gated field

Using the "normal" logic for a feature gated field simplifies the
implementation of the feature gate.

There is one (entirely theoretic!) problem with updating from 1.31: if a claim
was allocated in 1.31 with admin access, the status field was not set because
it didn't exist yet. If a driver now follows the current definition of "unset =
off", then it will not grant admin access even though it should. This is
theoretic because drivers are starting to support admin access with 1.32, so
there shouldn't be any claim where this problem could occur.

Author: pohly

api/openapi-spec/swagger.json

@@ -443,12 +443,12 @@ type DeviceRequest struct {
 	// any resource allocations.
 	//
 	// This is an alpha field and requires enabling the DRAAdminAccess
-	// feature gate.
+	// feature gate. Admin access is disabled if this field is unset or
+	// set to false, otherwise it is enabled.
 	//
 	// +optional
-	// +default=false
 	// +featureGate=DRAAdminAccess
-	AdminAccess bool
+	AdminAccess *bool
 }
 
 const (
@@ -787,19 +787,15 @@ type DeviceRequestAllocationResult struct {
 	// +required
 	Device string
 
-	// AdminAccess is a copy of the AdminAccess value in the
-	// request which caused this device to be allocated.
-	//
-	// New allocations are required to have this set when the DRAAdminAccess
-	// feature gate is enabled. Old allocations made
-	// by Kubernetes 1.31 do not have it yet. Clients which want to
-	// support Kubernetes 1.31 need to look up the request and retrieve
-	// the value from there if this field is not set.
+	// AdminAccess indicates that this device was allocated for
+	// administrative access. See the corresponding request field
+	// for a definition of mode.
 	//
 	// This is an alpha field and requires enabling the DRAAdminAccess
-	// feature gate.
+	// feature gate. Admin access is disabled if this field is unset or
+	// set to false, otherwise it is enabled.
 	//
-	// +required
+	// +optional
 	// +featureGate=DRAAdminAccess
 	AdminAccess *bool
 }

api/openapi-spec/v3/apis__resource.k8s.io__v1alpha3_openapi.json

@@ -32,11 +32,9 @@ import (
 	"k8s.io/apimachinery/pkg/util/validation/field"
 	"k8s.io/apiserver/pkg/cel"
 	"k8s.io/apiserver/pkg/cel/environment"
-	utilfeature "k8s.io/apiserver/pkg/util/feature"
 	dracel "k8s.io/dynamic-resource-allocation/cel"
 	corevalidation "k8s.io/kubernetes/pkg/apis/core/validation"
 	"k8s.io/kubernetes/pkg/apis/resource"
-	"k8s.io/kubernetes/pkg/features"
 )
 
 var (
@@ -345,9 +343,6 @@ func validateDeviceRequestAllocationResult(result resource.DeviceRequestAllocati
 	allErrs = append(allErrs, validateDriverName(result.Driver, fldPath.Child("driver"))...)
 	allErrs = append(allErrs, validatePoolName(result.Pool, fldPath.Child("pool"))...)
 	allErrs = append(allErrs, validateDeviceName(result.Device, fldPath.Child("device"))...)
-	if result.AdminAccess == nil && utilfeature.DefaultFeatureGate.Enabled(features.DRAAdminAccess) {
-		allErrs = append(allErrs, field.Required(fldPath.Child("adminAccess"), ""))
-	}
 	return allErrs
 }
 

pkg/apis/resource/types.go

@@ -478,27 +478,6 @@ func TestValidateClaimStatusUpdate(t *testing.T) {
 				return claim
 			},
 		},
-		"invalid-add-allocation-missing-admin-access": {
-			adminAccess: true,
-			wantFailures: field.ErrorList{
-				field.Required(field.NewPath("status", "allocation", "devices", "results").Index(0).Child("adminAccess"), ""),
-			},
-			oldClaim: validClaim,
-			update: func(claim *resource.ResourceClaim) *resource.ResourceClaim {
-				claim.Status.Allocation = &resource.AllocationResult{
-					Devices: resource.DeviceAllocationResult{
-						Results: []resource.DeviceRequestAllocationResult{{
-							Request:     goodName,
-							Driver:      goodName,
-							Pool:        goodName,
-							Device:      goodName,
-							AdminAccess: nil, // Intentionally not set.
-						}},
-					},
-				}
-				return claim
-			},
-		},
 		"okay-add-allocation-missing-admin-access": {
 			adminAccess: false,
 			oldClaim:    validClaim,

pkg/apis/resource/v1alpha3/zz_generated.conversion.go

@@ -681,7 +681,7 @@ func (ec *Controller) handleClaim(ctx context.Context, pod *v1.Pod, podClaim v1.
 
 func needsAdminAccess(claimTemplate *resourceapi.ResourceClaimTemplate) bool {
 	for _, request := range claimTemplate.Spec.Spec.Devices.Requests {
-		if request.AdminAccess {
+		if ptr.Deref(request.AdminAccess, false) {
 			return true
 		}
 	}

pkg/apis/resource/validation/validation.go

@@ -25,6 +25,7 @@ import (
 	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	api "k8s.io/kubernetes/pkg/apis/resource"
+	"k8s.io/utils/ptr"
 )
 
 func testResourceClaim(name string, namespace string, spec api.ResourceClaimSpec) *api.ResourceClaim {
@@ -101,7 +102,7 @@ func TestResourceClaimEvaluatorUsage(t *testing.T) {
 			claim: func() *api.ResourceClaim {
 				claim := validClaim.DeepCopy()
 				// Admins are *not* exempt from quota.
-				claim.Spec.Devices.Requests[0].AdminAccess = true
+				claim.Spec.Devices.Requests[0].AdminAccess = ptr.To(true)
 				return claim
 			}(),
 			usage: corev1.ResourceList{