From 961690dcb43ee8c50fa030b4782545c7fb5a9884 Mon Sep 17 00:00:00 2001 From: Danil-Grigorev Date: Wed, 2 Oct 2024 11:49:25 +0200 Subject: [PATCH] Always use CAPI generated certificates for etcd management - Perform etcd memebership management only when the local certificates were created, and skip otherwise. Signed-off-by: Danil-Grigorev --- .../internal/cloudinit/controlplane_init.go | 1 - bootstrap/internal/ignition/ignition.go | 4 - bootstrap/internal/ignition/ignition_test.go | 14 +-- pkg/rke2/management_cluster.go | 24 +--- pkg/rke2/workload_cluster.go | 12 +- pkg/secret/certificates.go | 111 ------------------ 6 files changed, 10 insertions(+), 156 deletions(-) diff --git a/bootstrap/internal/cloudinit/controlplane_init.go b/bootstrap/internal/cloudinit/controlplane_init.go index b1bebc66..75bdae9c 100644 --- a/bootstrap/internal/cloudinit/controlplane_init.go +++ b/bootstrap/internal/cloudinit/controlplane_init.go @@ -37,7 +37,6 @@ runcmd: - '/opt/rke2-cis-script.sh'{{ end }} - 'systemctl enable rke2-server.service' - 'systemctl start rke2-server.service' - - '/var/lib/rancher/rke2/bin/kubectl create secret tls cluster-etcd -o yaml --dry-run=client -n kube-system --cert=/var/lib/rancher/rke2/server/tls/etcd/server-ca.crt --key=/var/lib/rancher/rke2/server/tls/etcd/server-ca.key --kubeconfig /etc/rancher/rke2/rke2.yaml | /var/lib/rancher/rke2/bin/kubectl apply -f- --kubeconfig /etc/rancher/rke2/rke2.yaml' - 'mkdir -p /run/cluster-api' - '{{ .SentinelFileCommand }}' {{- template "commands" .PostRKE2Commands }} diff --git a/bootstrap/internal/ignition/ignition.go b/bootstrap/internal/ignition/ignition.go index 81979211..bd8a19af 100644 --- a/bootstrap/internal/ignition/ignition.go +++ b/bootstrap/internal/ignition/ignition.go @@ -38,10 +38,6 @@ var ( "setenforce 0", "systemctl enable rke2-server.service", "systemctl start rke2-server.service", - "/var/lib/rancher/rke2/bin/kubectl create secret tls cluster-etcd -o yaml --dry-run=client -n kube-system " + - "--cert=/var/lib/rancher/rke2/server/tls/etcd/server-ca.crt --key=/var/lib/rancher/rke2/server/tls/etcd/server-ca.key " + - "--kubeconfig /etc/rancher/rke2/rke2.yaml |" + - " /var/lib/rancher/rke2/bin/kubectl apply -f- --kubeconfig /etc/rancher/rke2/rke2.yaml", "restorecon /etc/systemd/system/rke2-server.service", "mkdir -p /run/cluster-api /etc/cluster-api", "echo success | tee /run/cluster-api/bootstrap-success.complete /etc/cluster-api/bootstrap-success.complete > /dev/null", diff --git a/bootstrap/internal/ignition/ignition_test.go b/bootstrap/internal/ignition/ignition_test.go index 82af74f1..596aa8bd 100644 --- a/bootstrap/internal/ignition/ignition_test.go +++ b/bootstrap/internal/ignition/ignition_test.go @@ -17,11 +17,11 @@ limitations under the License. package ignition import ( - "fmt" - "compress/gzip" "bytes" - "io/ioutil" + "compress/gzip" "encoding/base64" + "fmt" + "io/ioutil" "strings" "testing" @@ -133,7 +133,7 @@ var _ = Describe("NewJoinWorker", func() { scriptContentsGzip, err := base64.StdEncoding.DecodeString(scriptContentsEnc) Expect(err).ToNot(HaveOccurred()) reader := bytes.NewReader(scriptContentsGzip) - gzreader, err := gzip.NewReader(reader); + gzreader, err := gzip.NewReader(reader) Expect(err).ToNot(HaveOccurred()) scriptContents, err := ioutil.ReadAll(gzreader) Expect(err).ToNot(HaveOccurred()) @@ -254,7 +254,7 @@ var _ = Describe("getControlPlaneRKE2Commands", func() { It("should return slice of control plane commands", func() { commands, err := getControlPlaneRKE2Commands(baseUserData) Expect(err).ToNot(HaveOccurred()) - Expect(commands).To(HaveLen(10)) + Expect(commands).To(HaveLen(9)) Expect(commands).To(ContainElements(fmt.Sprintf(controlPlaneCommand, baseUserData.RKE2Version), serverDeployCommands[0], serverDeployCommands[1])) }) @@ -262,7 +262,7 @@ var _ = Describe("getControlPlaneRKE2Commands", func() { baseUserData.AirGapped = true commands, err := getControlPlaneRKE2Commands(baseUserData) Expect(err).ToNot(HaveOccurred()) - Expect(commands).To(HaveLen(10)) + Expect(commands).To(HaveLen(9)) Expect(commands).To(ContainElements(airGappedControlPlaneCommand, serverDeployCommands[0], serverDeployCommands[1])) }) @@ -271,7 +271,7 @@ var _ = Describe("getControlPlaneRKE2Commands", func() { baseUserData.AirGappedChecksum = "abcd" commands, err := getControlPlaneRKE2Commands(baseUserData) Expect(err).ToNot(HaveOccurred()) - Expect(commands).To(HaveLen(11)) + Expect(commands).To(HaveLen(10)) Expect(commands).To(ContainElements(fmt.Sprintf(airGappedChecksumCommand, "abcd"), airGappedControlPlaneCommand, serverDeployCommands[0], serverDeployCommands[1])) }) diff --git a/pkg/rke2/management_cluster.go b/pkg/rke2/management_cluster.go index 57e3c45a..400a3c0d 100644 --- a/pkg/rke2/management_cluster.go +++ b/pkg/rke2/management_cluster.go @@ -28,7 +28,6 @@ import ( "time" "github.com/pkg/errors" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes/scheme" ctrlclient "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/log" @@ -150,30 +149,9 @@ func (m *Management) getEtcdCAKeyPair(ctx context.Context, cl ctrlclient.Reader, log.FromContext(ctx).Info("Secret is empty, skipping etcd client creation") return keypair, nil - } else if s.Labels != nil && s.Labels[secret.ExternalPurposeLabel] == string(secret.EtcdServerCA) { - return certificates[0].GetKeyPair(), nil } - // External certificate needs to be fetched otherwise, to sync the content - log.FromContext(ctx).Info("Local secret is not up-to-date, skipping etcd client creation") - - return keypair, nil -} - -func (m *Management) getRemoteKeyPair(ctx context.Context, remoteClient ctrlclient.Client, clusterKey ctrlclient.ObjectKey) (*certs.KeyPair, error) { - etcdCertificate := &secret.ExternalCertificate{ - Reader: remoteClient, - Purpose: secret.EtcdServerCA, - } - externalCertificates := secret.Certificates{etcdCertificate} - - if err := externalCertificates.LookupOrGenerate(ctx, m.Client, clusterKey, metav1.OwnerReference{}); err != nil { - log.FromContext(ctx).Error(err, "unable to lookup or create cluster certificates") - - return nil, err - } - - return etcdCertificate.GetKeyPair(), nil + return certificates[0].GetKeyPair(), nil } func generateClientCert(caCertEncoded, caKeyEncoded []byte, clientKey *rsa.PrivateKey) (tls.Certificate, error) { diff --git a/pkg/rke2/workload_cluster.go b/pkg/rke2/workload_cluster.go index ff9de26b..bbfebe23 100644 --- a/pkg/rke2/workload_cluster.go +++ b/pkg/rke2/workload_cluster.go @@ -115,17 +115,9 @@ func (m *Management) NewWorkload( } if apierrors.IsNotFound(err) || etcdKeyPair == nil { - log.FromContext(ctx).Info("Collecting etcd key pair from remote") + log.FromContext(ctx).Info("Cluster does not provide etcd certificates for creating child etcd ctrlclient.") - etcdKeyPair, err = m.getRemoteKeyPair(ctx, cl, clusterKey) - if ctrlclient.IgnoreNotFound(err) != nil { - return nil, err - } else if apierrors.IsNotFound(err) { - log.FromContext(ctx).Info("Cluster does not provide etcd certificates for creating child etcd ctrlclient." + - "Please scale up the CP nodes by one to bootstrap the etcd secret content.") - - return workload, nil - } + return workload, nil } clientCert, err := tls.X509KeyPair(etcdKeyPair.Cert, etcdKeyPair.Key) diff --git a/pkg/secret/certificates.go b/pkg/secret/certificates.go index 1cb452dc..1dd843e5 100644 --- a/pkg/secret/certificates.go +++ b/pkg/secret/certificates.go @@ -82,10 +82,6 @@ const ( // TenYears is the duration of one year. TenYears = time.Hour * 24 * 365 * 10 - - // ExternalPurposeLabel is a label set on external secrets, uniquely identifying their belonging - // to external source and used for a specified purpose. - ExternalPurposeLabel = "cluster.x-k8s.io/purpose" ) // Purpose is the name to append to the secret generated for a cluster. @@ -160,41 +156,9 @@ func (c *ManagedCertificate) Lookup(ctx context.Context, ctrlclient client.Reade return s, nil } -// ExternalCertificate represents a single certificate CA. -type ExternalCertificate struct { - client.Reader - Purpose Purpose - Generated bool - KeyPair *certs.KeyPair -} - -// SaveGenerated implements Certificate. -func (c *ExternalCertificate) SaveGenerated(ctx context.Context, cl client.Client, key types.NamespacedName, owner metav1.OwnerReference) error { - s := c.AsSecret(key, owner) - - if err := cl.Get(ctx, client.ObjectKeyFromObject(s), s); apierrors.IsNotFound(err) { - if err := cl.Create(ctx, c.AsSecret(key, owner)); client.IgnoreAlreadyExists(err) != nil { - return errors.WithStack(err) - } - } else if err != nil { - return errors.WithStack(err) - } - - source := c.AsSecret(key, owner) - s.Data = source.Data - s.Labels = source.Labels - - if err := cl.Update(ctx, s); client.IgnoreAlreadyExists(err) != nil { - return errors.WithStack(err) - } - - return nil -} - var ( _ CertificatesGenerator = &Certificates{} _ Certificate = &ManagedCertificate{} - _ Certificate = &ExternalCertificate{} ) // Certificates are the certificates necessary to bootstrap a cluster. @@ -311,74 +275,6 @@ func (c *ManagedCertificate) IsExternal() bool { return c.External } -// Lookup implements certificate lookup for external source. -func (c *ExternalCertificate) Lookup(ctx context.Context, _ client.Reader, _ client.ObjectKey) (*corev1.Secret, error) { - s := &corev1.Secret{} - key := client.ObjectKey{ - Name: Name("cluster", c.GetPurpose()), - Namespace: metav1.NamespaceSystem, - } - - if err := c.Get(ctx, key, s); err != nil { - if apierrors.IsNotFound(err) { - if c.IsExternal() { - return nil, errors.WithMessage(err, "external certificate not found") - } - - return nil, nil //nolint:nilnil - } - - return nil, errors.WithStack(err) - } - - return s, nil -} - -// AsFiles for external certificate is a no-op, due to being externally managed. -func (*ExternalCertificate) AsFiles() []bootstrapv1.File { - return []bootstrapv1.File{} -} - -// AsSecret implements Certificate. -func (c *ExternalCertificate) AsSecret(clusterName types.NamespacedName, owner metav1.OwnerReference) *corev1.Secret { - s := asExternalSecret(map[string][]byte{ - TLSKeyDataName: c.KeyPair.Key, - TLSCrtDataName: c.KeyPair.Cert, - }, c.GetPurpose(), clusterName, owner) - - return s -} - -// Generate implements key pair collection from external source. -func (c *ExternalCertificate) Generate() error { - return nil -} - -// GetKeyPair implements key pair retriever for ExternalCertificate. -func (c *ExternalCertificate) GetKeyPair() *certs.KeyPair { - return c.KeyPair -} - -// GetPurpose implements purpose check for ExternalCertificate. -func (c *ExternalCertificate) GetPurpose() Purpose { - return c.Purpose -} - -// IsExternal represents extenally managed scenario for ExternalCertificate so is always true. -func (*ExternalCertificate) IsExternal() bool { - return true -} - -// IsGenerated is always false for externally managed certificate. -func (c *ExternalCertificate) IsGenerated() bool { - return true -} - -// SetKeyPair sets ExternalCertificate key pair. -func (c *ExternalCertificate) SetKeyPair(keyPair *certs.KeyPair) { - c.KeyPair = keyPair -} - // Generate will generate any certificates that do not have KeyPair data. func (c Certificates) Generate() error { for _, certificate := range c { @@ -598,13 +494,6 @@ func generateServiceAccountKeys() (*certs.KeyPair, error) { }, nil } -func asExternalSecret(data map[string][]byte, purpose Purpose, clusterName types.NamespacedName, owner metav1.OwnerReference) *corev1.Secret { - secret := asSecret(data, purpose, clusterName, owner) - secret.Labels[ExternalPurposeLabel] = string(purpose) - - return secret -} - func asSecret(data map[string][]byte, purpose Purpose, clusterName types.NamespacedName, _ metav1.OwnerReference) *corev1.Secret { return &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{