Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

force implementors of dyanmiccertificates providers to think about notify #100979

Merged
merged 1 commit into from
Apr 20, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 2 additions & 6 deletions pkg/controlplane/instance.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand All @@ -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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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)
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ type ConfigMapCAController struct {
preRunCaches []cache.InformerSynced
}

var _ Notifier = &ConfigMapCAController{}
var _ CAContentProvider = &ConfigMapCAController{}
var _ ControllerRunner = &ConfigMapCAController{}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ type DynamicCertKeyPairContent struct {
queue workqueue.RateLimitingInterface
}

var _ Notifier = &DynamicCertKeyPairContent{}
var _ CertKeyContentProvider = &DynamicCertKeyPairContent{}
var _ ControllerRunner = &DynamicCertKeyPairContent{}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ type DynamicFileSNIContent struct {
sniNames []string
}

var _ Notifier = &DynamicFileSNIContent{}
var _ SNICertKeyContentProvider = &DynamicFileSNIContent{}
var _ ControllerRunner = &DynamicFileSNIContent{}

Expand Down
Original file line number Diff line number Diff line change
@@ -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
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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) {}
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import (

type unionCAContent []CAContentProvider

var _ Notifier = &unionCAContent{}
var _ CAContentProvider = &unionCAContent{}
var _ ControllerRunner = &unionCAContent{}

Expand Down Expand Up @@ -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)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import (
)

var _ dynamiccertificates.ControllerRunner = &DynamicRequestHeaderController{}
var _ dynamiccertificates.Notifier = &DynamicRequestHeaderController{}
var _ dynamiccertificates.CAContentProvider = &DynamicRequestHeaderController{}

var _ headerrequest.RequestHeaderAuthRequestProvider = &DynamicRequestHeaderController{}
Expand Down
16 changes: 7 additions & 9 deletions staging/src/k8s.io/apiserver/pkg/server/secure_serving.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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.
Expand Down