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

Dynamic cert kube apiserver wiring #83555

Merged
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
1 change: 1 addition & 0 deletions staging/src/k8s.io/apiserver/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ require (
github.com/coreos/go-semver v0.3.0 // indirect
github.com/coreos/go-systemd v0.0.0-20190321100706-95778dfbb74e
github.com/coreos/pkg v0.0.0-20180108230652-97fdf19511ea
github.com/davecgh/go-spew v1.1.1
github.com/docker/docker v0.7.3-0.20190327010347-be7ac8be2ae0
github.com/emicklei/go-restful v2.9.5+incompatible
github.com/evanphx/json-patch v4.2.0+incompatible
Expand Down
3 changes: 2 additions & 1 deletion staging/src/k8s.io/apiserver/pkg/server/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ go_library(
"//staging/src/k8s.io/apiserver/pkg/endpoints/request:go_default_library",
"//staging/src/k8s.io/apiserver/pkg/registry/generic:go_default_library",
"//staging/src/k8s.io/apiserver/pkg/registry/rest:go_default_library",
"//staging/src/k8s.io/apiserver/pkg/server/dynamiccertificates:go_default_library",
"//staging/src/k8s.io/apiserver/pkg/server/egressselector:go_default_library",
"//staging/src/k8s.io/apiserver/pkg/server/filters:go_default_library",
"//staging/src/k8s.io/apiserver/pkg/server/healthz:go_default_library",
Expand All @@ -107,7 +108,6 @@ go_library(
"//staging/src/k8s.io/apiserver/pkg/util/openapi:go_default_library",
"//staging/src/k8s.io/client-go/informers:go_default_library",
"//staging/src/k8s.io/client-go/rest:go_default_library",
"//staging/src/k8s.io/client-go/util/cert:go_default_library",
"//staging/src/k8s.io/component-base/logs:go_default_library",
"//vendor/github.com/coreos/go-systemd/daemon:go_default_library",
"//vendor/github.com/emicklei/go-restful:go_default_library",
Expand Down Expand Up @@ -135,6 +135,7 @@ filegroup(
name = "all-srcs",
srcs = [
":package-srcs",
"//staging/src/k8s.io/apiserver/pkg/server/dynamiccertificates:all-srcs",
"//staging/src/k8s.io/apiserver/pkg/server/egressselector:all-srcs",
"//staging/src/k8s.io/apiserver/pkg/server/filters:all-srcs",
"//staging/src/k8s.io/apiserver/pkg/server/healthz:all-srcs",
Expand Down
14 changes: 6 additions & 8 deletions staging/src/k8s.io/apiserver/pkg/server/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ package server

import (
"crypto/tls"
"crypto/x509"
"fmt"
"net"
"net/http"
Expand Down Expand Up @@ -56,14 +55,14 @@ import (
apiopenapi "k8s.io/apiserver/pkg/endpoints/openapi"
apirequest "k8s.io/apiserver/pkg/endpoints/request"
genericregistry "k8s.io/apiserver/pkg/registry/generic"
"k8s.io/apiserver/pkg/server/dynamiccertificates"
"k8s.io/apiserver/pkg/server/egressselector"
genericfilters "k8s.io/apiserver/pkg/server/filters"
"k8s.io/apiserver/pkg/server/healthz"
"k8s.io/apiserver/pkg/server/routes"
serverstore "k8s.io/apiserver/pkg/server/storage"
"k8s.io/client-go/informers"
restclient "k8s.io/client-go/rest"
certutil "k8s.io/client-go/util/cert"
"k8s.io/component-base/logs"
"k8s.io/klog"
openapicommon "k8s.io/kube-openapi/pkg/common"
Expand Down Expand Up @@ -240,7 +239,7 @@ type SecureServingInfo struct {
SNICerts map[string]*tls.Certificate

// ClientCA is the certificate bundle for all the signers that you'll recognize for incoming client certificates
ClientCA *x509.CertPool
ClientCA dynamiccertificates.CAContentProvider

// MinTLSVersion optionally overrides the minimum TLS version supported.
// Values are from tls package constants (https://golang.org/pkg/crypto/tls/#pkg-constants).
Expand Down Expand Up @@ -350,15 +349,14 @@ func DefaultOpenAPIConfig(getDefinitions openapicommon.GetOpenAPIDefinitions, de
func (c *AuthenticationInfo) ApplyClientCert(clientCAFile string, servingInfo *SecureServingInfo) error {
if servingInfo != nil {
if len(clientCAFile) > 0 {
clientCAs, err := certutil.CertsFromFile(clientCAFile)
clientCAProvider, err := dynamiccertificates.NewStaticCAContentFromFile(clientCAFile)
if err != nil {
return fmt.Errorf("unable to load client CA file: %v", err)
}
if servingInfo.ClientCA == nil {
servingInfo.ClientCA = x509.NewCertPool()
}
for _, cert := range clientCAs {
servingInfo.ClientCA.AddCert(cert)
servingInfo.ClientCA = clientCAProvider
} else {
servingInfo.ClientCA = dynamiccertificates.NewUnionCAContentProvider(servingInfo.ClientCA, clientCAProvider)
}
}
}
Expand Down
48 changes: 48 additions & 0 deletions staging/src/k8s.io/apiserver/pkg/server/dynamiccertificates/BUILD
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")

go_library(
name = "go_default_library",
srcs = [
"client_ca.go",
"static_content.go",
"tlsconfig.go",
"union_content.go",
"util.go",
],
importmap = "k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/server/dynamiccertificates",
importpath = "k8s.io/apiserver/pkg/server/dynamiccertificates",
visibility = ["//visibility:public"],
deps = [
"//staging/src/k8s.io/api/core/v1:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/util/runtime:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/util/wait:go_default_library",
"//staging/src/k8s.io/client-go/tools/events:go_default_library",
"//staging/src/k8s.io/client-go/util/cert:go_default_library",
"//staging/src/k8s.io/client-go/util/workqueue:go_default_library",
"//vendor/k8s.io/klog:go_default_library",
],
)

filegroup(
name = "package-srcs",
srcs = glob(["**"]),
tags = ["automanaged"],
visibility = ["//visibility:private"],
)

filegroup(
name = "all-srcs",
srcs = [":package-srcs"],
tags = ["automanaged"],
visibility = ["//visibility:public"],
)

go_test(
name = "go_default_test",
srcs = [
"client_ca_test.go",
"tlsconfig_test.go",
],
embed = [":go_default_library"],
deps = ["//vendor/github.com/davecgh/go-spew/spew:go_default_library"],
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
/*
Copyright 2019 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 (
"bytes"
)

// CAContentProvider provides ca bundle byte content
type CAContentProvider interface {
// Name is just an identifier
Name() string
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we only expect CAContentProvider implementations in this package? Maybe leave these methods private.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we only expect CAContentProvider implementations in this package? Maybe leave these methods private.

I'd like to have the option of writing an implementation downstream. Imagine something similar to watching the kube-system namespace.

// 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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suspect we may want to add an error, especially for later cases where this function may involve reading a file. It seems like currently we are using a return of nil to denote a problem reading?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suspect we may want to add an error, especially for later cases where this function may involve reading a file. It seems like currently we are using a return of nil to denote a problem reading?

I think you have two different controllers. One indicating failure to read the value and the consumer of this API just gets the latest value.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the error should be reported at a lower level by the controller doing the read. By the time we get all the way here, we want the best value possible.

}

// dynamicCertificateContent holds the content that overrides the baseTLSConfig
// TODO add the serving certs to this struct
type dynamicCertificateContent struct {
// clientCA holds the content for the clientCA bundle
clientCA caBundleContent
}

// caBundleContent holds the content for the clientCA bundle. Wrapping the bytes makes the Equals work nicely with the
// method receiver.
type caBundleContent struct {
caBundle []byte
}

func (c *dynamicCertificateContent) Equal(rhs *dynamicCertificateContent) bool {
if c == nil || rhs == nil {
return c == rhs
}

if !c.clientCA.Equal(&rhs.clientCA) {
return false
}

return true
}

func (c *caBundleContent) Equal(rhs *caBundleContent) bool {
if c == nil || rhs == nil {
return c == rhs
}

return bytes.Equal(c.caBundle, rhs.caBundle)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
/*
Copyright 2019 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 "testing"

func TestDynamicCertificateContentEquals(t *testing.T) {
tests := []struct {
name string
lhs *dynamicCertificateContent
rhs *dynamicCertificateContent
expected bool
}{
{
name: "both nil",
expected: true,
},
{
name: "lhs nil",
rhs: &dynamicCertificateContent{},
expected: false,
},
{
name: "rhs nil",
lhs: &dynamicCertificateContent{},
expected: false,
},
{
name: "same",
lhs: &dynamicCertificateContent{
clientCA: caBundleContent{caBundle: []byte("foo")},
},
rhs: &dynamicCertificateContent{
clientCA: caBundleContent{caBundle: []byte("foo")},
},
expected: true,
},
{
name: "different",
lhs: &dynamicCertificateContent{
clientCA: caBundleContent{caBundle: []byte("foo")},
},
rhs: &dynamicCertificateContent{
clientCA: caBundleContent{caBundle: []byte("bar")},
},
expected: false,
},
}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
actual := test.lhs.Equal(test.rhs)
if actual != test.expected {
t.Error(actual)
}
})
}
}

func TestCABundleContentEquals(t *testing.T) {
tests := []struct {
name string
lhs *caBundleContent
rhs *caBundleContent
expected bool
}{
{
name: "both nil",
expected: true,
},
{
name: "lhs nil",
rhs: &caBundleContent{},
expected: false,
},
{
name: "rhs nil",
lhs: &caBundleContent{},
expected: false,
},
{
name: "same",
lhs: &caBundleContent{caBundle: []byte("foo")},
rhs: &caBundleContent{caBundle: []byte("foo")},
expected: true,
},
{
name: "different",
lhs: &caBundleContent{caBundle: []byte("foo")},
rhs: &caBundleContent{caBundle: []byte("bar")},
expected: false,
},
}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
actual := test.lhs.Equal(test.rhs)
if actual != test.expected {
t.Error(actual)
}
})
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
/*
Copyright 2019 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 (
"fmt"
"io/ioutil"
)

type staticCAContent struct {
name string
caBundle []byte
}

// NewStaticCAContentFromFile returns a CAContentProvider based on a filename
func NewStaticCAContentFromFile(filename string) (CAContentProvider, error) {
if len(filename) == 0 {
return nil, fmt.Errorf("missing filename for ca bundle")
}

caBundle, err := ioutil.ReadFile(filename)
if err != nil {
return nil, err
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

error context

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

error context

the os will put a name on it. I don't want to hide the original error type unless I have to.

}
return NewStaticCAContent(filename, caBundle), nil
}

// NewStaticCAContent returns a CAContentProvider that always returns the same value
func NewStaticCAContent(name string, caBundle []byte) CAContentProvider {
return &staticCAContent{
name: name,
caBundle: caBundle,
}
}

// Name is just an identifier
func (c *staticCAContent) Name() string {
return c.name
}

// CurrentCABundleContent provides ca bundle byte content
func (c *staticCAContent) CurrentCABundleContent() (cabundle []byte) {
return c.caBundle
}