Skip to content

Commit

Permalink
Merge pull request kubernetes-sigs#263 from chuckha/control-plane-joi…
Browse files Browse the repository at this point in the history 
…n-certs

🐛 Separate certificate logic for joins
  • Loading branch information
@k8s-ci-robot
k8s-ci-robot authored Oct 2, 2019
2 parents faeb391 + 3aceccd commit b22e54b
Show file tree
Hide file tree
Showing 4 changed files with 120 additions and 59 deletions.
108 changes: 64 additions & 44 deletions controllers/kubeadmconfig_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ func (r *KubeadmConfigReconciler) Reconcile(req ctrl.Request) (_ ctrl.Result, re
return ctrl.Result{}, err
}

certificates := internalcluster.NewCertificatesForControlPlane(config.Spec.ClusterConfiguration)
certificates := internalcluster.NewCertificatesForInitialControlPlane(config.Spec.ClusterConfiguration)
if err := certificates.LookupOrGenerate(ctx, r.Client, cluster, config); err != nil {
log.Error(err, "unable to lookup or create cluster certificates")
return ctrl.Result{}, err
Expand Down Expand Up @@ -266,56 +266,41 @@ func (r *KubeadmConfigReconciler) Reconcile(req ctrl.Request) (_ ctrl.Result, re
if config.Spec.JoinConfiguration == nil {
log.Info("Creating default JoinConfiguration")
config.Spec.JoinConfiguration = &kubeadmv1beta1.JoinConfiguration{}
if util.IsControlPlaneMachine(machine) {
config.Spec.JoinConfiguration.ControlPlane = &kubeadmv1beta1.JoinControlPlane{}
}
}

certificates := internalcluster.NewCertificatesForWorker(config.Spec.JoinConfiguration.CACertPath)
if err := certificates.Lookup(ctx, r.Client, cluster); err != nil {
log.Error(err, "unable to lookup cluster certificates")
return ctrl.Result{}, err
}
if err := certificates.EnsureAllExist(); err != nil {
return ctrl.Result{}, err
}

hashes, err := certificates.GetByPurpose(secret.ClusterCA).Hashes()
if err != nil {
log.Error(err, "Unable to generate Cluster CA certificate hashes")
return ctrl.Result{}, err
}
// TODO: move this into reconcile.Discovery so that defaults for the Discovery are all in the same place
if config.Spec.JoinConfiguration.Discovery.BootstrapToken == nil {
config.Spec.JoinConfiguration.Discovery.BootstrapToken = &kubeadmv1beta1.BootstrapTokenDiscovery{}
}
config.Spec.JoinConfiguration.Discovery.BootstrapToken.CACertHashes = hashes
// it's a control plane join
if util.IsControlPlaneMachine(machine) {
if config.Spec.JoinConfiguration.ControlPlane == nil {
config.Spec.JoinConfiguration.ControlPlane = &kubeadmv1beta1.JoinControlPlane{}
}

// ensure that joinConfiguration.Discovery is properly set for joining node on the current cluster
if err := r.reconcileDiscovery(cluster, config); err != nil {
if requeueErr, ok := errors.Cause(err).(capierrors.HasRequeueAfterError); ok {
log.Info(err.Error())
return ctrl.Result{RequeueAfter: requeueErr.GetRequeueAfter()}, nil
certificates := internalcluster.NewCertificatesForJoiningControlPlane()
if err := certificates.Lookup(ctx, r.Client, cluster); err != nil {
log.Error(err, "unable to lookup cluster certificates")
return ctrl.Result{}, err
}
if err := certificates.EnsureAllExist(); err != nil {
return ctrl.Result{}, err
}
return ctrl.Result{}, err
}

joindata, err := kubeadmv1beta1.ConfigurationToYAML(config.Spec.JoinConfiguration)
if err != nil {
log.Error(err, "failed to marshal join configuration")
return ctrl.Result{}, err
}
// ensure that joinConfiguration.Discovery is properly set for joining node on the current cluster
if err := r.reconcileDiscovery(cluster, config, certificates); err != nil {
if requeueErr, ok := errors.Cause(err).(capierrors.HasRequeueAfterError); ok {
log.Info(err.Error())
return ctrl.Result{RequeueAfter: requeueErr.GetRequeueAfter()}, nil
}
return ctrl.Result{}, err
}

// it's a control plane join
if util.IsControlPlaneMachine(machine) {
if config.Spec.JoinConfiguration.ControlPlane == nil {
return ctrl.Result{}, errors.New("Machine is a ControlPlane, but JoinConfiguration.ControlPlane is not set in the KubeadmConfig object")
joinData, err := kubeadmv1beta1.ConfigurationToYAML(config.Spec.JoinConfiguration)
if err != nil {
log.Error(err, "failed to marshal join configuration")
return ctrl.Result{}, err
}

log.Info("Creating BootstrapData for the join control plane")

cloudJoinData, err := cloudinit.NewJoinControlPlane(&cloudinit.ControlPlaneJoinInput{
JoinConfiguration: joindata,
JoinConfiguration: joinData,
Certificates: certificates,
BaseUserData: cloudinit.BaseUserData{
AdditionalFiles: config.Spec.Files,
Expand All @@ -335,7 +320,32 @@ func (r *KubeadmConfigReconciler) Reconcile(req ctrl.Request) (_ ctrl.Result, re
return ctrl.Result{}, nil
}

// otherwise it is a node
// It's a worker join
certificates := internalcluster.NewCertificatesForWorker(config.Spec.JoinConfiguration.CACertPath)
if err := certificates.Lookup(ctx, r.Client, cluster); err != nil {
log.Error(err, "unable to lookup cluster certificates")
return ctrl.Result{}, err
}
if err := certificates.EnsureAllExist(); err != nil {
log.Error(err, "Missing certificates")
return ctrl.Result{}, err
}

// ensure that joinConfiguration.Discovery is properly set for joining node on the current cluster
if err := r.reconcileDiscovery(cluster, config, certificates); err != nil {
if requeueErr, ok := errors.Cause(err).(capierrors.HasRequeueAfterError); ok {
log.Info(err.Error())
return ctrl.Result{RequeueAfter: requeueErr.GetRequeueAfter()}, nil
}
return ctrl.Result{}, err
}

joinData, err := kubeadmv1beta1.ConfigurationToYAML(config.Spec.JoinConfiguration)
if err != nil {
log.Error(err, "failed to marshal join configuration")
return ctrl.Result{}, err
}

if config.Spec.JoinConfiguration.ControlPlane != nil {
return ctrl.Result{}, errors.New("Machine is a Worker, but JoinConfiguration.ControlPlane is set in the KubeadmConfig object")
}
Expand All @@ -350,7 +360,7 @@ func (r *KubeadmConfigReconciler) Reconcile(req ctrl.Request) (_ ctrl.Result, re
PostKubeadmCommands: config.Spec.PostKubeadmCommands,
Users: config.Spec.Users,
},
JoinConfiguration: joindata,
JoinConfiguration: joinData,
})
if err != nil {
log.Error(err, "failed to create a worker join configuration")
Expand Down Expand Up @@ -416,7 +426,7 @@ func (r *KubeadmConfigReconciler) MachineToBootstrapMapFunc(o handler.MapObject)
// The implementation func respect user provided discovery configurations, but in case some of them are missing, a valid BootstrapToken object
// is automatically injected into config.JoinConfiguration.Discovery.
// This allows to simplify configuration UX, by providing the option to delegate to CABPK the configuration of kubeadm join discovery.
func (r *KubeadmConfigReconciler) reconcileDiscovery(cluster *clusterv1.Cluster, config *bootstrapv1.KubeadmConfig) error {
func (r *KubeadmConfigReconciler) reconcileDiscovery(cluster *clusterv1.Cluster, config *bootstrapv1.KubeadmConfig, certificates internalcluster.Certificates) error {
log := r.Log.WithValues("kubeadmconfig", fmt.Sprintf("%s/%s", config.Namespace, config.Name))

// if config already contains a file discovery configuration, respect it without further validations
Expand All @@ -429,6 +439,16 @@ func (r *KubeadmConfigReconciler) reconcileDiscovery(cluster *clusterv1.Cluster,
config.Spec.JoinConfiguration.Discovery.BootstrapToken = &kubeadmv1beta1.BootstrapTokenDiscovery{}
}

// calculate the ca cert hashes if they are not already set
if len(config.Spec.JoinConfiguration.Discovery.BootstrapToken.CACertHashes) == 0 {
hashes, err := certificates.GetByPurpose(secret.ClusterCA).Hashes()
if err != nil {
log.Error(err, "Unable to generate Cluster CA certificate hashes")
return err
}
config.Spec.JoinConfiguration.Discovery.BootstrapToken.CACertHashes = hashes
}

// if BootstrapToken already contains an APIServerEndpoint, respect it; otherwise inject the APIServerEndpoint endpoint defined in cluster status
apiServerEndpoint := config.Spec.JoinConfiguration.Discovery.BootstrapToken.APIServerEndpoint
if apiServerEndpoint == "" {
Expand Down
37 changes: 26 additions & 11 deletions controllers/kubeadmconfig_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -400,7 +400,7 @@ func TestKubeadmConfigReconciler_Reconcile_GenerateCloudConfigData(t *testing.T)
}
}

// If a controlplane has an invalid JoinConfiguration then user intervention is required.
// If a control plane has no JoinConfiguration, then we will create a default and no error will occur
func TestKubeadmConfigReconciler_Reconcile_ErrorIfJoiningControlPlaneHasInvalidConfiguration(t *testing.T) {
// TODO: extract this kind of code into a setup function that puts the state of objects into an initialized controlplane (implies secrets exist)
cluster := newCluster("cluster")
Expand Down Expand Up @@ -436,8 +436,8 @@ func TestKubeadmConfigReconciler_Reconcile_ErrorIfJoiningControlPlaneHasInvalidC
},
}
_, err := k.Reconcile(request)
if err == nil {
t.Fatal("Expected error, got nil")
if err != nil {
t.Fatalf("Expected no error but got %v", err)
}
}

Expand Down Expand Up @@ -598,6 +598,11 @@ func TestKubeadmConfigReconciler_Reconcile_DisocveryReconcileBehaviors(t *testin
}

dummyCAHash := []string{"...."}
bootstrapToken := kubeadmv1beta1.Discovery{
BootstrapToken: &kubeadmv1beta1.BootstrapTokenDiscovery{
CACertHashes: dummyCAHash,
},
}
goodcluster := &clusterv1.Cluster{
Status: clusterv1.ClusterStatus{
APIEndpoints: []clusterv1.APIEndpoint{
Expand All @@ -619,7 +624,9 @@ func TestKubeadmConfigReconciler_Reconcile_DisocveryReconcileBehaviors(t *testin
cluster: goodcluster,
config: &bootstrapv1.KubeadmConfig{
Spec: bootstrapv1.KubeadmConfigSpec{
JoinConfiguration: &kubeadmv1beta1.JoinConfiguration{},
JoinConfiguration: &kubeadmv1beta1.JoinConfiguration{
Discovery: bootstrapToken,
},
},
},
validateDiscovery: func(c *bootstrapv1.KubeadmConfig) error {
Expand All @@ -633,8 +640,8 @@ func TestKubeadmConfigReconciler_Reconcile_DisocveryReconcileBehaviors(t *testin
if d.BootstrapToken.APIServerEndpoint != "example.com:6443" {
return errors.Errorf("BootstrapToken.APIServerEndpoint=example.com:6443 expected, got %q", d.BootstrapToken.APIServerEndpoint)
}
if d.BootstrapToken.UnsafeSkipCAVerification != true {
return errors.Errorf("BootstrapToken.UnsafeSkipCAVerification=true expected, got false")
if d.BootstrapToken.UnsafeSkipCAVerification == true {
return errors.Errorf("BootstrapToken.UnsafeSkipCAVerification=false expected, got true")
}
return nil
},
Expand Down Expand Up @@ -667,6 +674,7 @@ func TestKubeadmConfigReconciler_Reconcile_DisocveryReconcileBehaviors(t *testin
JoinConfiguration: &kubeadmv1beta1.JoinConfiguration{
Discovery: kubeadmv1beta1.Discovery{
BootstrapToken: &kubeadmv1beta1.BootstrapTokenDiscovery{
CACertHashes: dummyCAHash,
APIServerEndpoint: "bar.com:6443",
},
},
Expand All @@ -689,7 +697,8 @@ func TestKubeadmConfigReconciler_Reconcile_DisocveryReconcileBehaviors(t *testin
JoinConfiguration: &kubeadmv1beta1.JoinConfiguration{
Discovery: kubeadmv1beta1.Discovery{
BootstrapToken: &kubeadmv1beta1.BootstrapTokenDiscovery{
Token: "abcdef.0123456789abcdef",
CACertHashes: dummyCAHash,
Token: "abcdef.0123456789abcdef",
},
},
},
Expand Down Expand Up @@ -729,7 +738,7 @@ func TestKubeadmConfigReconciler_Reconcile_DisocveryReconcileBehaviors(t *testin

for _, tc := range testcases {
t.Run(tc.name, func(t *testing.T) {
err := k.reconcileDiscovery(tc.cluster, tc.config)
err := k.reconcileDiscovery(tc.cluster, tc.config, internalcluster.Certificates{})
if err != nil {
t.Errorf("expected nil, got error %v", err)
}
Expand Down Expand Up @@ -758,15 +767,21 @@ func TestKubeadmConfigReconciler_Reconcile_DisocveryReconcileFailureBehaviors(t
cluster: &clusterv1.Cluster{}, // cluster without endpoints
config: &bootstrapv1.KubeadmConfig{
Spec: bootstrapv1.KubeadmConfigSpec{
JoinConfiguration: &kubeadmv1beta1.JoinConfiguration{},
JoinConfiguration: &kubeadmv1beta1.JoinConfiguration{
Discovery: kubeadmv1beta1.Discovery{
BootstrapToken: &kubeadmv1beta1.BootstrapTokenDiscovery{
CACertHashes: []string{"item"},
},
},
},
},
},
},
}

for _, tc := range testcases {
t.Run(tc.name, func(t *testing.T) {
err := k.reconcileDiscovery(tc.cluster, tc.config)
err := k.reconcileDiscovery(tc.cluster, tc.config, internalcluster.Certificates{})
if err == nil {
t.Error("expected error, got nil")
}
Expand Down Expand Up @@ -1270,7 +1285,7 @@ func createSecrets(t *testing.T, cluster *clusterv1.Cluster, owner *bootstrapv1.
if owner.Spec.ClusterConfiguration == nil {
owner.Spec.ClusterConfiguration = &kubeadmv1beta1.ClusterConfiguration{}
}
certificates := internalcluster.NewCertificatesForControlPlane(owner.Spec.ClusterConfiguration)
certificates := internalcluster.NewCertificatesForInitialControlPlane(owner.Spec.ClusterConfiguration)
if err := certificates.Generate(); err != nil {
t.Fatal(err)
}
Expand Down
30 changes: 28 additions & 2 deletions internal/cluster/certificates.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,8 @@ var (
// Certificates are the certificates necessary to bootstrap a cluster.
type Certificates []*Certificate

// NewCertificatesForControlPlane returns a list of certificates configured for a control plane node
func NewCertificatesForControlPlane(config *v1beta1.ClusterConfiguration) Certificates {
// NewCertificatesForInitialControlPlane returns a list of certificates configured for a control plane node
func NewCertificatesForInitialControlPlane(config *v1beta1.ClusterConfiguration) Certificates {
if config.CertificatesDir == "" {
config.CertificatesDir = defaultCertificatesDir
}
Expand Down Expand Up @@ -122,6 +122,32 @@ func NewCertificatesForControlPlane(config *v1beta1.ClusterConfiguration) Certif
return certificates
}

// NewCertificatesForJoiningControlPlane gets any certs that exist and writes them to disk
func NewCertificatesForJoiningControlPlane() Certificates {
return Certificates{
&Certificate{
Purpose: secret.ClusterCA,
CertFile: filepath.Join(defaultCertificatesDir, "ca.crt"),
KeyFile: filepath.Join(defaultCertificatesDir, "ca.key"),
},
&Certificate{
Purpose: ServiceAccount,
CertFile: filepath.Join(defaultCertificatesDir, "sa.pub"),
KeyFile: filepath.Join(defaultCertificatesDir, "sa.key"),
},
&Certificate{
Purpose: FrontProxyCA,
CertFile: filepath.Join(defaultCertificatesDir, "front-proxy-ca.crt"),
KeyFile: filepath.Join(defaultCertificatesDir, "front-proxy-ca.key"),
},
&Certificate{
Purpose: EtcdCA,
CertFile: filepath.Join(defaultCertificatesDir, "etcd", "ca.crt"),
KeyFile: filepath.Join(defaultCertificatesDir, "etcd", "ca.key"),
},
}
}

// NewCertificatesForWorker return an initialized but empty set of CA certificates needed to bootstrap a cluster.
func NewCertificatesForWorker(caCertPath string) Certificates {
if caCertPath == "" {
Expand Down
4 changes: 2 additions & 2 deletions internal/cluster/certificates_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import (

func TestNewCertificatesForControlPlane_Stacked(t *testing.T) {
config := &v1beta1.ClusterConfiguration{}
certs := NewCertificatesForControlPlane(config)
certs := NewCertificatesForInitialControlPlane(config)
if certs.GetByPurpose(EtcdCA).KeyFile == "" {
t.Fatal("stacked control planes must define etcd CA key file")
}
Expand All @@ -37,7 +37,7 @@ func TestNewCertificatesForControlPlane_External(t *testing.T) {
},
}

certs := NewCertificatesForControlPlane(config)
certs := NewCertificatesForInitialControlPlane(config)
if certs.GetByPurpose(EtcdCA).KeyFile != "" {
t.Fatal("control planes with external etcd must *not* define the etcd key file")
}
Expand Down

0 comments on commit b22e54b

Please sign in to comment.