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

Derive kubelet serving certificate CSR template from node status addresses #65594

Merged
merged 3 commits into from
Jul 12, 2018
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion pkg/kubelet/certificate/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ go_library(
"//pkg/kubelet/apis/kubeletconfig:go_default_library",
"//pkg/kubelet/metrics:go_default_library",
"//staging/src/k8s.io/api/certificates/v1beta1:go_default_library",
"//staging/src/k8s.io/api/core/v1:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/types:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/util/net:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/util/wait:go_default_library",
Expand All @@ -32,9 +33,13 @@ go_library(

go_test(
name = "go_default_test",
srcs = ["transport_test.go"],
srcs = [
"kubelet_test.go",
"transport_test.go",
],
embed = [":go_default_library"],
deps = [
"//staging/src/k8s.io/api/core/v1:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/runtime/serializer:go_default_library",
"//staging/src/k8s.io/client-go/kubernetes/typed/certificates/v1beta1:go_default_library",
Expand Down
59 changes: 54 additions & 5 deletions pkg/kubelet/certificate/kubelet.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,12 @@ import (
"crypto/x509/pkix"
"fmt"
"net"
"sort"

"github.com/prometheus/client_golang/prometheus"

certificates "k8s.io/api/certificates/v1beta1"
"k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/types"
clientset "k8s.io/client-go/kubernetes"
clientcertificates "k8s.io/client-go/kubernetes/typed/certificates/v1beta1"
Expand All @@ -35,7 +37,7 @@ import (

// NewKubeletServerCertificateManager creates a certificate manager for the kubelet when retrieving a server certificate
// or returns an error.
func NewKubeletServerCertificateManager(kubeClient clientset.Interface, kubeCfg *kubeletconfig.KubeletConfiguration, nodeName types.NodeName, ips []net.IP, hostnames []string, certDirectory string) (certificate.Manager, error) {
func NewKubeletServerCertificateManager(kubeClient clientset.Interface, kubeCfg *kubeletconfig.KubeletConfiguration, nodeName types.NodeName, getAddresses func() []v1.NodeAddress, certDirectory string) (certificate.Manager, error) {
var certSigningRequestClient clientcertificates.CertificateSigningRequestInterface
if kubeClient != nil && kubeClient.CertificatesV1beta1() != nil {
certSigningRequestClient = kubeClient.CertificatesV1beta1().CertificateSigningRequests()
Expand All @@ -59,16 +61,25 @@ func NewKubeletServerCertificateManager(kubeClient clientset.Interface, kubeCfg
)
prometheus.MustRegister(certificateExpiration)

m, err := certificate.NewManager(&certificate.Config{
CertificateSigningRequestClient: certSigningRequestClient,
Template: &x509.CertificateRequest{
getTemplate := func() *x509.CertificateRequest {
hostnames, ips := addressesToHostnamesAndIPs(getAddresses())
// don't return a template if we have no addresses to request for
if len(hostnames) == 0 && len(ips) == 0 {
return nil
}
return &x509.CertificateRequest{
Subject: pkix.Name{
CommonName: fmt.Sprintf("system:node:%s", nodeName),
Organization: []string{"system:nodes"},
},
DNSNames: hostnames,
IPAddresses: ips,
},
}
}

m, err := certificate.NewManager(&certificate.Config{
CertificateSigningRequestClient: certSigningRequestClient,
GetTemplate: getTemplate,
Usages: []certificates.KeyUsage{
// https://tools.ietf.org/html/rfc5280#section-4.2.1.3
//
Expand All @@ -92,6 +103,44 @@ func NewKubeletServerCertificateManager(kubeClient clientset.Interface, kubeCfg
return m, nil
}

func addressesToHostnamesAndIPs(addresses []v1.NodeAddress) (dnsNames []string, ips []net.IP) {
seenDNSNames := map[string]bool{}
seenIPs := map[string]bool{}
for _, address := range addresses {
if len(address.Address) == 0 {
continue
}

switch address.Type {
case v1.NodeHostName:
if ip := net.ParseIP(address.Address); ip != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

Hrm. Is there any scenario where the node hostname can like an IP, but is not actually an IP and would not be appropriate to interpret as an IP?

Copy link
Member Author

Choose a reason for hiding this comment

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

Is there any scenario where the node hostname can like an IP, but is not actually an IP and would not be appropriate to interpret as an IP?

No? A hostname that parses as an IP is an IP... SNI wouldn't treat it as a DNS name, cert verification would require an IP SAN (c.f. https://golang.org/src/crypto/x509/verify.go?s=28297:28349#L914)

seenIPs[address.Address] = true
} else {
seenDNSNames[address.Address] = true
}
case v1.NodeExternalIP, v1.NodeInternalIP:
if ip := net.ParseIP(address.Address); ip != nil {
seenIPs[address.Address] = true
}
case v1.NodeExternalDNS, v1.NodeInternalDNS:
seenDNSNames[address.Address] = true
}
}

for dnsName := range seenDNSNames {
dnsNames = append(dnsNames, dnsName)
}
for ip := range seenIPs {
ips = append(ips, net.ParseIP(ip))
}

// return in stable order
sort.Strings(dnsNames)
sort.Slice(ips, func(i, j int) bool { return ips[i].String() < ips[j].String() })

return dnsNames, ips
}

// NewKubeletClientCertificateManager sets up a certificate manager without a
// client that can be used to sign new certificates (or rotate). It answers with
// whatever certificate it is initialized with. If a CSR client is set later, it
Expand Down
101 changes: 101 additions & 0 deletions pkg/kubelet/certificate/kubelet_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
/*
Copyright 2018 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 certificate

import (
"net"
"reflect"
"testing"

"k8s.io/api/core/v1"
)

func TestAddressesToHostnamesAndIPs(t *testing.T) {
tests := []struct {
name string
addresses []v1.NodeAddress
wantDNSNames []string
wantIPs []net.IP
}{
{
name: "empty",
addresses: nil,
wantDNSNames: nil,
wantIPs: nil,
},
{
name: "ignore empty values",
addresses: []v1.NodeAddress{{Type: v1.NodeHostName, Address: ""}},
wantDNSNames: nil,
wantIPs: nil,
},
{
name: "ignore invalid IPs",
addresses: []v1.NodeAddress{
{Type: v1.NodeInternalIP, Address: "1.2"},
{Type: v1.NodeExternalIP, Address: "3.4"},
},
wantDNSNames: nil,
wantIPs: nil,
},
{
name: "dedupe values",
addresses: []v1.NodeAddress{
{Type: v1.NodeHostName, Address: "hostname"},
{Type: v1.NodeExternalDNS, Address: "hostname"},
{Type: v1.NodeInternalDNS, Address: "hostname"},
{Type: v1.NodeInternalIP, Address: "1.1.1.1"},
{Type: v1.NodeExternalIP, Address: "1.1.1.1"},
},
wantDNSNames: []string{"hostname"},
wantIPs: []net.IP{net.ParseIP("1.1.1.1")},
},
{
name: "order values",
addresses: []v1.NodeAddress{
{Type: v1.NodeHostName, Address: "hostname-2"},
{Type: v1.NodeExternalDNS, Address: "hostname-1"},
{Type: v1.NodeInternalDNS, Address: "hostname-3"},
{Type: v1.NodeInternalIP, Address: "2.2.2.2"},
{Type: v1.NodeExternalIP, Address: "1.1.1.1"},
{Type: v1.NodeInternalIP, Address: "3.3.3.3"},
},
wantDNSNames: []string{"hostname-1", "hostname-2", "hostname-3"},
wantIPs: []net.IP{net.ParseIP("1.1.1.1"), net.ParseIP("2.2.2.2"), net.ParseIP("3.3.3.3")},
},
{
name: "handle IP and DNS hostnames",
addresses: []v1.NodeAddress{
{Type: v1.NodeHostName, Address: "hostname"},
{Type: v1.NodeHostName, Address: "1.1.1.1"},
},
wantDNSNames: []string{"hostname"},
wantIPs: []net.IP{net.ParseIP("1.1.1.1")},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
gotDNSNames, gotIPs := addressesToHostnamesAndIPs(tt.addresses)
if !reflect.DeepEqual(gotDNSNames, tt.wantDNSNames) {
t.Errorf("addressesToHostnamesAndIPs() gotDNSNames = %v, want %v", gotDNSNames, tt.wantDNSNames)
}
if !reflect.DeepEqual(gotIPs, tt.wantIPs) {
t.Errorf("addressesToHostnamesAndIPs() gotIPs = %v, want %v", gotIPs, tt.wantIPs)
}
})
}
}
40 changes: 4 additions & 36 deletions pkg/kubelet/kubelet.go
Original file line number Diff line number Diff line change
Expand Up @@ -372,8 +372,6 @@ func NewMainKubelet(kubeCfg *kubeletconfiginternal.KubeletConfiguration,
hostname := nodeutil.GetHostname(hostnameOverride)
// Query the cloud provider for our node name, default to hostname
nodeName := types.NodeName(hostname)
cloudIPs := []net.IP{}
cloudNames := []string{}
if kubeDeps.Cloud != nil {
var err error
instances, ok := kubeDeps.Cloud.Instances()
Expand All @@ -387,25 +385,6 @@ func NewMainKubelet(kubeCfg *kubeletconfiginternal.KubeletConfiguration,
}

glog.V(2).Infof("cloud provider determined current node name to be %s", nodeName)

if utilfeature.DefaultFeatureGate.Enabled(features.RotateKubeletServerCertificate) {
nodeAddresses, err := instances.NodeAddresses(context.TODO(), nodeName)
if err != nil {
return nil, fmt.Errorf("failed to get the addresses of the current instance from the cloud provider: %v", err)
}
for _, nodeAddress := range nodeAddresses {
switch nodeAddress.Type {
case v1.NodeExternalIP, v1.NodeInternalIP:
ip := net.ParseIP(nodeAddress.Address)
if ip != nil && !ip.IsLoopback() {
cloudIPs = append(cloudIPs, ip)
}
case v1.NodeExternalDNS, v1.NodeInternalDNS, v1.NodeHostName:
cloudNames = append(cloudNames, nodeAddress.Address)
}
}
}

}

if kubeDeps.PodConfig == nil {
Expand Down Expand Up @@ -747,21 +726,7 @@ func NewMainKubelet(kubeCfg *kubeletconfiginternal.KubeletConfiguration,
klet.statusManager = status.NewManager(klet.kubeClient, klet.podManager, klet)

if kubeCfg.ServerTLSBootstrap && kubeDeps.TLSOptions != nil && utilfeature.DefaultFeatureGate.Enabled(features.RotateKubeletServerCertificate) {
var ips []net.IP
cfgAddress := net.ParseIP(kubeCfg.Address)
if cfgAddress == nil || cfgAddress.IsUnspecified() {
localIPs, err := allGlobalUnicastIPs()
if err != nil {
return nil, err
}
ips = localIPs
} else {
ips = []net.IP{cfgAddress}
}

ips = append(ips, cloudIPs...)
names := append([]string{klet.GetHostname(), hostnameOverride}, cloudNames...)
klet.serverCertificateManager, err = kubeletcertificate.NewKubeletServerCertificateManager(klet.kubeClient, kubeCfg, klet.nodeName, ips, names, certDirectory)
klet.serverCertificateManager, err = kubeletcertificate.NewKubeletServerCertificateManager(klet.kubeClient, kubeCfg, klet.nodeName, klet.getLastObservedNodeAddresses, certDirectory)
if err != nil {
return nil, fmt.Errorf("failed to initialize certificate manager: %v", err)
}
Expand Down Expand Up @@ -893,6 +858,9 @@ type Kubelet struct {
iptClient utilipt.Interface
rootDirectory string

lastObservedNodeAddressesMux sync.Mutex
lastObservedNodeAddresses []v1.NodeAddress

// onRepeatedHeartbeatFailure is called when a heartbeat operation fails more than once. optional.
onRepeatedHeartbeatFailure func()

Expand Down
30 changes: 16 additions & 14 deletions pkg/kubelet/kubelet_node_status.go
Original file line number Diff line number Diff line change
Expand Up @@ -428,6 +428,7 @@ func (kl *Kubelet) tryUpdateNodeStatus(tryNumber int) error {
if err != nil {
return err
}
kl.setLastObservedNodeAddresses(updatedNode.Status.Addresses)
// If update finishes successfully, mark the volumeInUse as reportedInUse to indicate
// those volumes are already updated in the node's status
kl.volumeManager.MarkVolumesAsReportedInUse(updatedNode.Status.VolumesInUse)
Expand Down Expand Up @@ -493,20 +494,10 @@ func (kl *Kubelet) setNodeAddress(node *v1.Node) error {
return fmt.Errorf("failed to get node address from cloud provider that matches ip: %v", kl.nodeIP)
}

// Only add a NodeHostName address if the cloudprovider did not specify one
// (we assume the cloudprovider knows best)
var addressNodeHostName *v1.NodeAddress
for i := range nodeAddresses {
if nodeAddresses[i].Type == v1.NodeHostName {
addressNodeHostName = &nodeAddresses[i]
break
}
}
if addressNodeHostName == nil {
hostnameAddress := v1.NodeAddress{Type: v1.NodeHostName, Address: kl.GetHostname()}
nodeAddresses = append(nodeAddresses, hostnameAddress)
} else {
glog.V(2).Infof("Using Node Hostname from cloudprovider: %q", addressNodeHostName.Address)
// Only add a NodeHostName address if the cloudprovider did not specify any addresses.
// (we assume the cloudprovider is authoritative if it specifies any addresses)
if len(nodeAddresses) == 0 {
nodeAddresses = []v1.NodeAddress{{Type: v1.NodeHostName, Address: kl.GetHostname()}}
}
node.Status.Addresses = nodeAddresses
} else {
Expand Down Expand Up @@ -1072,6 +1063,17 @@ func (kl *Kubelet) setNodeStatus(node *v1.Node) {
}
}

func (kl *Kubelet) setLastObservedNodeAddresses(addresses []v1.NodeAddress) {
kl.lastObservedNodeAddressesMux.Lock()
defer kl.lastObservedNodeAddressesMux.Unlock()
kl.lastObservedNodeAddresses = addresses
}
func (kl *Kubelet) getLastObservedNodeAddresses() []v1.NodeAddress {
kl.lastObservedNodeAddressesMux.Lock()
defer kl.lastObservedNodeAddressesMux.Unlock()
return kl.lastObservedNodeAddresses
}

// defaultNodeStatusFuncs is a factory that generates the default set of
// setNodeStatus funcs
func (kl *Kubelet) defaultNodeStatusFuncs() []func(*v1.Node) error {
Expand Down
7 changes: 0 additions & 7 deletions pkg/util/node/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,13 +64,6 @@ func GetPreferredNodeAddress(node *v1.Node, preferredAddressTypes []v1.NodeAddre
return address.Address, nil
}
}
// If hostname was requested and no Hostname address was registered...
if addressType == v1.NodeHostName {
// ...fall back to the kubernetes.io/hostname label for compatibility with kubelets before 1.5
if hostname, ok := node.Labels[kubeletapis.LabelHostname]; ok && len(hostname) > 0 {
return hostname, nil
}
}
}
return "", fmt.Errorf("no preferred addresses found; known addresses: %v", node.Status.Addresses)
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/util/node/node_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,13 +61,13 @@ func TestGetPreferredAddress(t *testing.T) {
Preferences: []v1.NodeAddressType{v1.NodeHostName, v1.NodeExternalIP},
ExpectAddress: "status-hostname",
},
"found label address": {
"label address ignored": {
Labels: map[string]string{kubeletapis.LabelHostname: "label-hostname"},
Addresses: []v1.NodeAddress{
{Type: v1.NodeExternalIP, Address: "1.2.3.5"},
},
Preferences: []v1.NodeAddressType{v1.NodeHostName, v1.NodeExternalIP},
ExpectAddress: "label-hostname",
ExpectAddress: "1.2.3.5",
},
}

Expand Down
1 change: 1 addition & 0 deletions staging/src/k8s.io/client-go/util/certificate/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ go_library(
"//staging/src/k8s.io/api/certificates/v1beta1:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/api/errors:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/util/runtime:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/util/wait:go_default_library",
"//staging/src/k8s.io/client-go/kubernetes/typed/certificates/v1beta1:go_default_library",
"//staging/src/k8s.io/client-go/util/cert:go_default_library",
Expand Down
Loading