force implementors of dyanmiccertificates providers to think about notify

Right now, `_, ok := provider.(Notifier); !ok` can mean one of two
things:

1. The provider does not support notification because the provided
   content is static.
2. The implementor of the provider hasn't gotten around to implementing
   Notifier yet.

These have very different implications. We should not force consumers of
these interfaces to have to figure out the static of Notifier across
sometimes numerous different implementations. Instead, we should force
implementors to implement Notifier, even if it's a noop.

Change-Id: Ie7a26697a9a17790bfaa58d67045663bcc71e3cb

pkg/controlplane/instance.go

@@ -464,9 +464,7 @@ func (c completedConfig) New(delegationTarget genericapiserver.DelegationTarget)
 
 		// prime values and start listeners
 		if m.ClusterAuthenticationInfo.ClientCA != nil {
-			if notifier, ok := m.ClusterAuthenticationInfo.ClientCA.(dynamiccertificates.Notifier); ok {
-				notifier.AddListener(controller)
-			}
+			m.ClusterAuthenticationInfo.ClientCA.AddListener(controller)
 			if controller, ok := m.ClusterAuthenticationInfo.ClientCA.(dynamiccertificates.ControllerRunner); ok {
 				// runonce to be sure that we have a value.
 				if err := controller.RunOnce(); err != nil {
@@ -476,9 +474,7 @@ func (c completedConfig) New(delegationTarget genericapiserver.DelegationTarget)
 			}
 		}
 		if m.ClusterAuthenticationInfo.RequestHeaderCA != nil {
-			if notifier, ok := m.ClusterAuthenticationInfo.RequestHeaderCA.(dynamiccertificates.Notifier); ok {
-				notifier.AddListener(controller)
-			}
+			m.ClusterAuthenticationInfo.RequestHeaderCA.AddListener(controller)
 			if controller, ok := m.ClusterAuthenticationInfo.RequestHeaderCA.(dynamiccertificates.ControllerRunner); ok {
 				// runonce to be sure that we have a value.
 				if err := controller.RunOnce(); err != nil {

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

@@ -32,6 +32,7 @@ import (
 	"k8s.io/apiserver/pkg/authentication/request/websocket"
 	"k8s.io/apiserver/pkg/authentication/request/x509"
 	"k8s.io/apiserver/pkg/authentication/token/cache"
+	"k8s.io/apiserver/pkg/server/dynamiccertificates"
 	webhooktoken "k8s.io/apiserver/plugin/pkg/authenticator/token/webhook"
 	authenticationclient "k8s.io/client-go/kubernetes/typed/authentication/v1"
 )
@@ -55,7 +56,7 @@ type DelegatingAuthenticatorConfig struct {
 	// CAContentProvider are the options for verifying incoming connections using mTLS and directly assigning to users.
 	// Generally this is the CA bundle file used to authenticate client certificates
 	// If this is nil, then mTLS will not be used.
-	ClientCertificateCAContentProvider CAContentProvider
+	ClientCertificateCAContentProvider dynamiccertificates.CAContentProvider
 
 	APIAudiences authenticator.Audiences
 

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

@@ -17,9 +17,8 @@ limitations under the License.
 package authenticatorfactory
 
 import (
-	"crypto/x509"
-
 	"k8s.io/apiserver/pkg/authentication/request/headerrequest"
+	"k8s.io/apiserver/pkg/server/dynamiccertificates"
 )
 
 type RequestHeaderConfig struct {
@@ -32,17 +31,7 @@ type RequestHeaderConfig struct {
 	ExtraHeaderPrefixes headerrequest.StringSliceProvider
 	// CAContentProvider the options for verifying incoming connections using mTLS.  Generally this points to CA bundle file which is used verify the identity of the front proxy.
 	//	It may produce different options at will.
-	CAContentProvider CAContentProvider
+	CAContentProvider dynamiccertificates.CAContentProvider
 	// AllowedClientNames is a list of common names that may be presented by the authenticating front proxy.  Empty means: accept any.
 	AllowedClientNames headerrequest.StringSliceProvider
 }
-
-// CAContentProvider provides ca bundle byte content
-type CAContentProvider interface {
-	// Name is just an identifier
-	Name() string
-	// CurrentCABundleContent provides ca bundle byte content
-	CurrentCABundleContent() []byte
-	// VerifyOptions provides VerifyOptions for authenticators
-	VerifyOptions() (x509.VerifyOptions, bool)
-}

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

@@ -20,21 +20,6 @@ import (
 	"bytes"
 )
 
-// CertKeyContentProvider provides a certificate and matching private key
-type CertKeyContentProvider interface {
-	// Name is just an identifier
-	Name() string
-	// CurrentCertKeyContent provides cert and key byte content
-	CurrentCertKeyContent() ([]byte, []byte)
-}
-
-// SNICertKeyContentProvider provides a certificate and matching private key as well as optional explicit names
-type SNICertKeyContentProvider interface {
-	CertKeyContentProvider
-	// SNINames provides names used for SNI. May return nil.
-	SNINames() []string
-}
-
 // certKeyContent holds the content for the cert and key
 type certKeyContent struct {
 	cert []byte

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

@@ -18,20 +18,8 @@ package dynamiccertificates
 
 import (
 	"bytes"
-	"crypto/x509"
 )
 
-// CAContentProvider provides ca bundle byte content
-type CAContentProvider interface {
-	// Name is just an identifier
-	Name() string
-	// CurrentCABundleContent provides ca bundle byte content.  Errors can be contained to the controllers initializing
-	// 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, bool)
-}
-
 // dynamicCertificateContent holds the content that overrides the baseTLSConfig
 type dynamicCertificateContent struct {
 	// clientCA holds the content for the clientCA bundle

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

@@ -58,7 +58,6 @@ type ConfigMapCAController struct {
 	preRunCaches []cache.InformerSynced
 }
 
-var _ Notifier = &ConfigMapCAController{}
 var _ CAContentProvider = &ConfigMapCAController{}
 var _ ControllerRunner = &ConfigMapCAController{}
 

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

@@ -35,18 +35,6 @@ import (
 // FileRefreshDuration is exposed so that integration tests can crank up the reload speed.
 var FileRefreshDuration = 1 * time.Minute
 
-// Listener is an interface to use to notify interested parties of a change.
-type Listener interface {
-	// Enqueue should be called when an input may have changed
-	Enqueue()
-}
-
-// Notifier is a way to add listeners
-type Notifier interface {
-	// AddListener is adds a listener to be notified of potential input changes
-	AddListener(listener Listener)
-}
-
 // ControllerRunner is a generic interface for starting a controller
 type ControllerRunner interface {
 	// RunOnce runs the sync loop a single time.  This useful for synchronous priming

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

@@ -47,7 +47,6 @@ type DynamicCertKeyPairContent struct {
 	queue workqueue.RateLimitingInterface
 }
 
-var _ Notifier = &DynamicCertKeyPairContent{}
 var _ CertKeyContentProvider = &DynamicCertKeyPairContent{}
 var _ ControllerRunner = &DynamicCertKeyPairContent{}
 

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

@@ -22,7 +22,6 @@ type DynamicFileSNIContent struct {
 	sniNames []string
 }
 
-var _ Notifier = &DynamicFileSNIContent{}
 var _ SNICertKeyContentProvider = &DynamicFileSNIContent{}
 var _ ControllerRunner = &DynamicFileSNIContent{}
 

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

@@ -0,0 +1,68 @@
+/*
+Copyright 2021 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package dynamiccertificates
+
+import (
+	"crypto/x509"
+)
+
+// Listener is an interface to use to notify interested parties of a change.
+type Listener interface {
+	// Enqueue should be called when an input may have changed
+	Enqueue()
+}
+
+// Notifier is a way to add listeners
+type Notifier interface {
+	// AddListener is adds a listener to be notified of potential input changes.
+	// This is a noop on static providers.
+	AddListener(listener Listener)
+}
+
+// CAContentProvider provides ca bundle byte content
+type CAContentProvider interface {
+	Notifier
+
+	// Name is just an identifier.
+	Name() string
+	// CurrentCABundleContent provides ca bundle byte content. Errors can be
+	// contained to the controllers initializing 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, bool)
+}
+
+// CertKeyContentProvider provides a certificate and matching private key.
+type CertKeyContentProvider interface {
+	Notifier
+
+	// Name is just an identifier.
+	Name() string
+	// CurrentCertKeyContent provides cert and key byte content.
+	CurrentCertKeyContent() ([]byte, []byte)
+}
+
+// SNICertKeyContentProvider provides a certificate and matching private key as
+// well as optional explicit names.
+type SNICertKeyContentProvider interface {
+	Notifier
+
+	CertKeyContentProvider
+	// SNINames provides names used for SNI. May return nil.
+	SNINames() []string
+}

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

@@ -208,6 +208,8 @@ func (c *nullCAContent) Name() string {
 	return c.name
 }
 
+func (c *nullCAContent) AddListener(Listener) {}
+
 // CurrentCABundleContent provides ca bundle byte content
 func (c *nullCAContent) CurrentCABundleContent() (cabundle []byte) {
 	return nil

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

@@ -46,6 +46,8 @@ func (c *staticCAContent) Name() string {
 	return c.name
 }
 
+func (c *staticCAContent) AddListener(Listener) {}
+
 // CurrentCABundleContent provides ca bundle byte content
 func (c *staticCAContent) CurrentCABundleContent() (cabundle []byte) {
 	return c.caBundle.caBundle
@@ -61,11 +63,6 @@ type staticCertKeyContent struct {
 	key  []byte
 }
 
-type staticSNICertKeyContent struct {
-	staticCertKeyContent
-	sniNames []string
-}
-
 // NewStaticCertKeyContent returns a CertKeyContentProvider that always returns the same value
 func NewStaticCertKeyContent(name string, cert, key []byte) (CertKeyContentProvider, error) {
 	// Ensure that the key matches the cert and both are valid
@@ -81,6 +78,23 @@ func NewStaticCertKeyContent(name string, cert, key []byte) (CertKeyContentProvi
 	}, nil
 }
 
+// Name is just an identifier
+func (c *staticCertKeyContent) Name() string {
+	return c.name
+}
+
+func (c *staticCertKeyContent) AddListener(Listener) {}
+
+// CurrentCertKeyContent provides cert and key content
+func (c *staticCertKeyContent) CurrentCertKeyContent() ([]byte, []byte) {
+	return c.cert, c.key
+}
+
+type staticSNICertKeyContent struct {
+	staticCertKeyContent
+	sniNames []string
+}
+
 // NewStaticSNICertKeyContent returns a SNICertKeyContentProvider that always returns the same value
 func NewStaticSNICertKeyContent(name string, cert, key []byte, sniNames ...string) (SNICertKeyContentProvider, error) {
 	// Ensure that the key matches the cert and both are valid
@@ -99,16 +113,8 @@ func NewStaticSNICertKeyContent(name string, cert, key []byte, sniNames ...strin
 	}, nil
 }
 
-// Name is just an identifier
-func (c *staticCertKeyContent) Name() string {
-	return c.name
-}
-
-// CurrentCertKeyContent provides cert and key content
-func (c *staticCertKeyContent) CurrentCertKeyContent() ([]byte, []byte) {
-	return c.cert, c.key
-}
-
 func (c *staticSNICertKeyContent) SNINames() []string {
 	return c.sniNames
 }
+
+func (c *staticSNICertKeyContent) AddListener(Listener) {}

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

@@ -26,7 +26,6 @@ import (
 
 type unionCAContent []CAContentProvider
 
-var _ Notifier = &unionCAContent{}
 var _ CAContentProvider = &unionCAContent{}
 var _ ControllerRunner = &unionCAContent{}
 
@@ -77,9 +76,7 @@ func (c unionCAContent) VerifyOptions() (x509.VerifyOptions, bool) {
 // AddListener adds a listener to be notified when the CA content changes.
 func (c unionCAContent) AddListener(listener Listener) {
 	for _, curr := range c {
-		if notifier, ok := curr.(Notifier); ok {
-			notifier.AddListener(listener)
-		}
+		curr.AddListener(listener)
 	}
 }
 

staging/src/k8s.io/apiserver/pkg/server/options/authentication_dynamic_request_header.go

@@ -26,7 +26,6 @@ import (
 )
 
 var _ dynamiccertificates.ControllerRunner = &DynamicRequestHeaderController{}
-var _ dynamiccertificates.Notifier = &DynamicRequestHeaderController{}
 var _ dynamiccertificates.CAContentProvider = &DynamicRequestHeaderController{}
 
 var _ headerrequest.RequestHeaderAuthRequestProvider = &DynamicRequestHeaderController{}

staging/src/k8s.io/apiserver/pkg/server/secure_serving.go

@@ -86,13 +86,14 @@ func (s *SecureServingInfo) tlsConfig(stopCh <-chan struct{}) (*tls.Config, erro
 			s.SNICerts,
 			nil, // TODO see how to plumb an event recorder down in here. For now this results in simply klog messages.
 		)
-		// register if possible
-		if notifier, ok := s.ClientCA.(dynamiccertificates.Notifier); ok {
-			notifier.AddListener(dynamicCertificateController)
+
+		if s.ClientCA != nil {
+			s.ClientCA.AddListener(dynamicCertificateController)
 		}
-		if notifier, ok := s.Cert.(dynamiccertificates.Notifier); ok {
-			notifier.AddListener(dynamicCertificateController)
+		if s.Cert != nil {
+			s.Cert.AddListener(dynamicCertificateController)
 		}
+
 		// start controllers if possible
 		if controller, ok := s.ClientCA.(dynamiccertificates.ControllerRunner); ok {
 			// runonce to try to prime data.  If this fails, it's ok because we fail closed.
@@ -113,10 +114,7 @@ func (s *SecureServingInfo) tlsConfig(stopCh <-chan struct{}) (*tls.Config, erro
 			go controller.Run(1, stopCh)
 		}
 		for _, sniCert := range s.SNICerts {
-			if notifier, ok := sniCert.(dynamiccertificates.Notifier); ok {
-				notifier.AddListener(dynamicCertificateController)
-			}
-
+			sniCert.AddListener(dynamicCertificateController)
 			if controller, ok := sniCert.(dynamiccertificates.ControllerRunner); ok {
 				// runonce to try to prime data.  If this fails, it's ok because we fail closed.
 				// Files are required to be populated already, so this is for convenience.