Merge pull request kubernetes#84864 from deads2k/optional-verify-opts

allow a verifyoptionsfunc to indicate that no certpool is available

Author: k8s-ci-robot

staging/src/k8s.io/apiserver/pkg/authentication/authenticatorfactory/requestheader.go

@@ -44,5 +44,5 @@ type CAContentProvider interface {
 	// CurrentCABundleContent provides ca bundle byte content
 	CurrentCABundleContent() []byte
 	// VerifyOptions provides VerifyOptions for authenticators
-	VerifyOptions() x509.VerifyOptions
+	VerifyOptions() (x509.VerifyOptions, bool)
 }

staging/src/k8s.io/apiserver/pkg/authentication/request/x509/verify_options.go

@@ -25,8 +25,8 @@ import (
 
 // StaticVerifierFn is a VerifyOptionFunc that always returns the same value.  This allows verify options that cannot change.
 func StaticVerifierFn(opts x509.VerifyOptions) VerifyOptionFunc {
-	return func() x509.VerifyOptions {
-		return opts
+	return func() (x509.VerifyOptions, bool) {
+		return opts, true
 	}
 }
 

staging/src/k8s.io/apiserver/pkg/authentication/request/x509/x509.go

@@ -83,8 +83,10 @@ func (f UserConversionFunc) User(chain []*x509.Certificate) (*authenticator.Resp
 }
 
 // VerifyOptionFunc is function which provides a shallow copy of the VerifyOptions to the authenticator.  This allows
-// for cases where the options (particularly the CAs) can change.
-type VerifyOptionFunc func() x509.VerifyOptions
+// for cases where the options (particularly the CAs) can change.  If the bool is false, then the returned VerifyOptions
+// are ignored and the authenticator will express "no opinion".  This allows a clear signal for cases where a CertPool
+// is eventually expected, but not currently present.
+type VerifyOptionFunc func() (x509.VerifyOptions, bool)
 
 // Authenticator implements request.Authenticator by extracting user info from verified client certificates
 type Authenticator struct {
@@ -111,7 +113,11 @@ func (a *Authenticator) AuthenticateRequest(req *http.Request) (*authenticator.R
 	}
 
 	// Use intermediates, if provided
-	optsCopy := a.verifyOptionsFn()
+	optsCopy, ok := a.verifyOptionsFn()
+	// if there are intentionally no verify options, then we cannot authenticate this request
+	if !ok {
+		return nil, false, nil
+	}
 	if optsCopy.Intermediates == nil && len(req.TLS.PeerCertificates) > 1 {
 		optsCopy.Intermediates = x509.NewCertPool()
 		for _, intermediate := range req.TLS.PeerCertificates[1:] {
@@ -169,7 +175,11 @@ func (a *Verifier) AuthenticateRequest(req *http.Request) (*authenticator.Respon
 	}
 
 	// Use intermediates, if provided
-	optsCopy := a.verifyOptionsFn()
+	optsCopy, ok := a.verifyOptionsFn()
+	// if there are intentionally no verify options, then we cannot authenticate this request
+	if !ok {
+		return nil, false, nil
+	}
 	if optsCopy.Intermediates == nil && len(req.TLS.PeerCertificates) > 1 {
 		optsCopy.Intermediates = x509.NewCertPool()
 		for _, intermediate := range req.TLS.PeerCertificates[1:] {

staging/src/k8s.io/apiserver/pkg/server/dynamiccertificates/client_ca.go

@@ -29,7 +29,7 @@ type CAContentProvider interface {
 	// the value.  By the time you get here, you should always be returning a value that won't fail.
 	CurrentCABundleContent() []byte
 	// VerifyOptions provides VerifyOptions for authenticators
-	VerifyOptions() x509.VerifyOptions
+	VerifyOptions() (x509.VerifyOptions, bool)
 }
 
 // dynamicCertificateContent holds the content that overrides the baseTLSConfig

staging/src/k8s.io/apiserver/pkg/server/dynamiccertificates/dynamic_cafile_content.go

@@ -215,8 +215,13 @@ func (c *DynamicFileCAContent) CurrentCABundleContent() (cabundle []byte) {
 }
 
 // VerifyOptions provides verifyoptions compatible with authenticators
-func (c *DynamicFileCAContent) VerifyOptions() x509.VerifyOptions {
-	return c.caBundle.Load().(*caBundleAndVerifier).verifyOptions
+func (c *DynamicFileCAContent) VerifyOptions() (x509.VerifyOptions, bool) {
+	uncastObj := c.caBundle.Load()
+	if uncastObj == nil {
+		return x509.VerifyOptions{}, false
+	}
+
+	return uncastObj.(*caBundleAndVerifier).verifyOptions, true
 }
 
 // newVerifyOptions creates a new verification func from a file.  It reads the content and then fails.

staging/src/k8s.io/apiserver/pkg/server/dynamiccertificates/static_content.go

@@ -66,8 +66,8 @@ func (c *staticCAContent) CurrentCABundleContent() (cabundle []byte) {
 	return c.caBundle.caBundle
 }
 
-func (c *staticCAContent) VerifyOptions() x509.VerifyOptions {
-	return c.caBundle.verifyOptions
+func (c *staticCAContent) VerifyOptions() (x509.VerifyOptions, bool) {
+	return c.caBundle.verifyOptions, true
 }
 
 type staticCertKeyContent struct {

staging/src/k8s.io/apiserver/pkg/server/dynamiccertificates/union_content.go

@@ -55,7 +55,12 @@ func (c unionCAContent) CurrentCABundleContent() []byte {
 }
 
 // CurrentCABundleContent provides ca bundle byte content
-func (c unionCAContent) VerifyOptions() x509.VerifyOptions {
+func (c unionCAContent) VerifyOptions() (x509.VerifyOptions, bool) {
+	currCABundle := c.CurrentCABundleContent()
+	if len(currCABundle) == 0 {
+		return x509.VerifyOptions{}, false
+	}
+
 	// TODO make more efficient.  This isn't actually used in any of our mainline paths.  It's called to build the TLSConfig
 	// TODO on file changes, but the actual authentication runs against the individual items, not the union.
 	ret, err := newCABundleAndVerifier(c.Name(), c.CurrentCABundleContent())
@@ -64,7 +69,7 @@ func (c unionCAContent) VerifyOptions() x509.VerifyOptions {
 		panic(err)
 	}
 
-	return ret.verifyOptions
+	return ret.verifyOptions, true
 }
 
 // AddListener adds a listener to be notified when the CA content changes.

test/integration/scheduler/extender_test.go

@@ -27,7 +27,7 @@ import (
 	"testing"
 	"time"
 
-	"k8s.io/api/core/v1"
+	v1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/wait"

test/integration/scheduler/taint_test.go

@@ -85,6 +85,8 @@ func TestTaintNodeByCondition(t *testing.T) {
 	defer algorithmprovider.ApplyFeatureGates()()
 
 	context = initTestScheduler(t, context, false, nil)
+	defer cleanupTest(t, context)
+
 	cs := context.clientSet
 	informers := context.informerFactory
 	nsName := context.ns.Name
@@ -655,6 +657,8 @@ func TestTaintBasedEvictions(t *testing.T) {
 	for i, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
 			context := initTestMaster(t, "taint-based-evictions", admission)
+			defer cleanupTest(t, context)
+
 			// Build clientset and informers for controllers.
 			externalClientset := kubernetes.NewForConfigOrDie(&restclient.Config{
 				QPS:           -1,