Skip to content

K
k3s
Commit 137137cb authored by fabriziopandini's avatar fabriziopandini
Browse files
Options

fix-upgrade-certs-renew

parent b4aa81b9
Expand all Hide whitespace changes
Inline Side-by-side
Showing with 135 additions and 196 deletions
cmd/kubeadm/app/cmd/upgrade/apply.go
View file @ 137137cb
......@@ -52,6 +52,7 @@ type applyFlags struct {
force bool
dryRun bool
etcdUpgrade bool
renewCerts bool
criSocket string
imagePullTimeout time.Duration
}
......@@ -67,6 +68,7 @@ func NewCmdApply(apf *applyPlanFlags) *cobra.Command {
applyPlanFlags: apf,
imagePullTimeout: defaultImagePullTimeout,
etcdUpgrade: true,
renewCerts: true,
// Don't set criSocket to a default value here, as this will override the setting in the stored config in RunApply below.
}
......@@ -90,6 +92,7 @@ func NewCmdApply(apf *applyPlanFlags) *cobra.Command {
cmd.Flags().BoolVarP(&flags.force, "force", "f", flags.force, "Force upgrading although some requirements might not be met. This also implies non-interactive mode.")
cmd.Flags().BoolVar(&flags.dryRun, options.DryRun, flags.dryRun, "Do not change any state, just output what actions would be performed.")
cmd.Flags().BoolVar(&flags.etcdUpgrade, "etcd-upgrade", flags.etcdUpgrade, "Perform the upgrade of etcd.")
cmd.Flags().BoolVar(&flags.renewCerts, "certificate-renewal", flags.renewCerts, "Perform the renewal of certificates used by component changed during upgrades.")
cmd.Flags().DurationVar(&flags.imagePullTimeout, "image-pull-timeout", flags.imagePullTimeout, "The maximum amount of time to wait for the control plane pods to be downloaded.")
// The CRI socket flag is deprecated here, since it should be taken from the NodeRegistrationOptions for the current
......@@ -231,7 +234,7 @@ func PerformControlPlaneUpgrade(flags *applyFlags, client clientset.Interface, w
}
// Don't save etcd backup directory if etcd is HA, as this could cause corruption
return PerformStaticPodUpgrade(client, waiter, internalcfg, flags.etcdUpgrade)
return PerformStaticPodUpgrade(client, waiter, internalcfg, flags.etcdUpgrade, flags.renewCerts)
}
// GetPathManagerForUpgrade returns a path manager properly configured for the given InitConfiguration.
......@@ -241,14 +244,14 @@ func GetPathManagerForUpgrade(internalcfg *kubeadmapi.InitConfiguration, etcdUpg
}
// PerformStaticPodUpgrade performs the upgrade of the control plane components for a static pod hosted cluster
func PerformStaticPodUpgrade(client clientset.Interface, waiter apiclient.Waiter, internalcfg *kubeadmapi.InitConfiguration, etcdUpgrade bool) error {
func PerformStaticPodUpgrade(client clientset.Interface, waiter apiclient.Waiter, internalcfg *kubeadmapi.InitConfiguration, etcdUpgrade, renewCerts bool) error {
pathManager, err := GetPathManagerForUpgrade(internalcfg, etcdUpgrade)
if err != nil {
return err
}
// The arguments oldEtcdClient and newEtdClient, are uninitialized because passing in the clients allow for mocking the client during testing
return upgrade.StaticPodControlPlane(client, waiter, pathManager, internalcfg, etcdUpgrade, nil, nil)
return upgrade.StaticPodControlPlane(client, waiter, pathManager, internalcfg, etcdUpgrade, renewCerts, nil, nil)
}
// DryRunStaticPodUpgrade fakes an upgrade of the control plane
......
cmd/kubeadm/app/cmd/upgrade/node.go
View file @ 137137cb
......@@ -66,6 +66,7 @@ type controlplaneUpgradeFlags struct {
advertiseAddress string
nodeName string
etcdUpgrade bool
renewCerts bool
dryRun bool
}
......@@ -114,6 +115,7 @@ func NewCmdUpgradeControlPlane() *cobra.Command {
kubeConfigPath: constants.GetKubeletKubeConfigPath(),
advertiseAddress: "",
etcdUpgrade: true,
renewCerts: true,
dryRun: false,
}
......@@ -151,6 +153,7 @@ func NewCmdUpgradeControlPlane() *cobra.Command {
options.AddKubeConfigFlag(cmd.Flags(), &flags.kubeConfigPath)
cmd.Flags().BoolVar(&flags.dryRun, options.DryRun, flags.dryRun, "Do not change any state, just output the actions that would be performed.")
cmd.Flags().BoolVar(&flags.etcdUpgrade, "etcd-upgrade", flags.etcdUpgrade, "Perform the upgrade of etcd.")
cmd.Flags().BoolVar(&flags.renewCerts, "certificate-renewal", flags.renewCerts, "Perform the renewal of certificates used by component changed during upgrades.")
return cmd
}
......@@ -221,18 +224,13 @@ func RunUpgradeControlPlane(flags *controlplaneUpgradeFlags) error {
return errors.Wrap(err, "unable to fetch the kubeadm-config ConfigMap")
}
// Rotate API server certificate if needed
if err := upgrade.BackupAPIServerCertIfNeeded(cfg, flags.dryRun); err != nil {
return errors.Wrap(err, "unable to rotate API server certificate")
}
// Upgrade the control plane and etcd if installed on this node
fmt.Printf("[upgrade] Upgrading your Static Pod-hosted control plane instance to version %q...\n", cfg.KubernetesVersion)
if flags.dryRun {
return DryRunStaticPodUpgrade(cfg)
}
if err := PerformStaticPodUpgrade(client, waiter, cfg, flags.etcdUpgrade); err != nil {
if err := PerformStaticPodUpgrade(client, waiter, cfg, flags.etcdUpgrade, flags.renewCerts); err != nil {
return errors.Wrap(err, "couldn't complete the static pod upgrade")
}
......
cmd/kubeadm/app/phases/upgrade/BUILD
View file @ 137137cb
......@@ -15,7 +15,6 @@ go_library(
visibility = ["//visibility:public"],
deps = [
"//cmd/kubeadm/app/apis/kubeadm:go_default_library",
"//cmd/kubeadm/app/apis/kubeadm/v1beta2:go_default_library",
"//cmd/kubeadm/app/constants:go_default_library",
"//cmd/kubeadm/app/images:go_default_library",
"//cmd/kubeadm/app/phases/addons/dns:go_default_library",
......@@ -45,7 +44,6 @@ go_library(
"//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/util/version:go_default_library",
"//staging/src/k8s.io/client-go/kubernetes:go_default_library",
"//staging/src/k8s.io/client-go/util/cert:go_default_library",
"//vendor/github.com/pkg/errors:go_default_library",
],
)
......
cmd/kubeadm/app/phases/upgrade/postupgrade.go
View file @ 137137cb
......@@ -17,7 +17,6 @@ limitations under the License.
package upgrade
import (
"fmt"
"os"
"path/filepath"
"time"
......@@ -29,15 +28,12 @@ import (
errorsutil "k8s.io/apimachinery/pkg/util/errors"
"k8s.io/apimachinery/pkg/util/version"
clientset "k8s.io/client-go/kubernetes"
certutil "k8s.io/client-go/util/cert"
kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
kubeadmapiv1beta2 "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/v1beta2"
kubeadmconstants "k8s.io/kubernetes/cmd/kubeadm/app/constants"
"k8s.io/kubernetes/cmd/kubeadm/app/phases/addons/dns"
"k8s.io/kubernetes/cmd/kubeadm/app/phases/addons/proxy"
"k8s.io/kubernetes/cmd/kubeadm/app/phases/bootstraptoken/clusterinfo"
nodebootstraptoken "k8s.io/kubernetes/cmd/kubeadm/app/phases/bootstraptoken/node"
certsphase "k8s.io/kubernetes/cmd/kubeadm/app/phases/certs"
kubeletphase "k8s.io/kubernetes/cmd/kubeadm/app/phases/kubelet"
patchnodephase "k8s.io/kubernetes/cmd/kubeadm/app/phases/patchnode"
"k8s.io/kubernetes/cmd/kubeadm/app/phases/uploadconfig"
......@@ -101,11 +97,6 @@ func PerformPostUpgradeTasks(client clientset.Interface, cfg *kubeadmapi.InitCon
errs = append(errs, err)
}
// Rotate the kube-apiserver cert and key if needed
if err := BackupAPIServerCertIfNeeded(cfg, dryRun); err != nil {
errs = append(errs, err)
}
// Upgrade kube-dns/CoreDNS and kube-proxy
if err := dns.EnsureDNSAddon(&cfg.ClusterConfiguration, client); err != nil {
errs = append(errs, err)
......@@ -152,37 +143,6 @@ func removeOldDNSDeploymentIfAnotherDNSIsUsed(cfg *kubeadmapi.ClusterConfigurati
}, 10)
}
// BackupAPIServerCertIfNeeded rotates the kube-apiserver certificate if older than 180 days
func BackupAPIServerCertIfNeeded(cfg *kubeadmapi.InitConfiguration, dryRun bool) error {
certAndKeyDir := kubeadmapiv1beta2.DefaultCertificatesDir
shouldBackup, err := shouldBackupAPIServerCertAndKey(certAndKeyDir)
if err != nil {
// Don't fail the upgrade phase if failing to determine to backup kube-apiserver cert and key.
return errors.Wrap(err, "[postupgrade] WARNING: failed to determine to backup kube-apiserver cert and key")
}
if !shouldBackup {
return nil
}
// If dry-running, just say that this would happen to the user and exit
if dryRun {
fmt.Println("[postupgrade] Would rotate the API server certificate and key.")
return nil
}
// Don't fail the upgrade phase if failing to backup kube-apiserver cert and key, just continue rotating the cert
// TODO: We might want to reconsider this choice.
if err := backupAPIServerCertAndKey(certAndKeyDir); err != nil {
fmt.Printf("[postupgrade] WARNING: failed to backup kube-apiserver cert and key: %v\n", err)
}
return certsphase.CreateCertAndKeyFilesWithCA(
&certsphase.KubeadmCertAPIServer,
&certsphase.KubeadmCertRootCA,
cfg,
)
}
func writeKubeletConfigFiles(client clientset.Interface, cfg *kubeadmapi.InitConfiguration, newK8sVer *version.Version, dryRun bool) error {
kubeletDir, err := GetKubeletDir(dryRun)
if err != nil {
......@@ -228,20 +188,6 @@ func GetKubeletDir(dryRun bool) (string, error) {
return kubeadmconstants.KubeletRunDirectory, nil
}
// backupAPIServerCertAndKey backups the old cert and key of kube-apiserver to a specified directory.
func backupAPIServerCertAndKey(certAndKeyDir string) error {
subDir := filepath.Join(certAndKeyDir, "expired")
if err := os.Mkdir(subDir, 0700); err != nil {
return errors.Wrapf(err, "failed to created backup directory %s", subDir)
}
filesToMove := map[string]string{
filepath.Join(certAndKeyDir, kubeadmconstants.APIServerCertName): filepath.Join(subDir, kubeadmconstants.APIServerCertName),
filepath.Join(certAndKeyDir, kubeadmconstants.APIServerKeyName): filepath.Join(subDir, kubeadmconstants.APIServerKeyName),
}
return moveFiles(filesToMove)
}
// moveFiles moves files from one directory to another.
func moveFiles(files map[string]string) error {
filesToRecover := map[string]string{}
......@@ -264,21 +210,3 @@ func rollbackFiles(files map[string]string, originalErr error) error {
}
return errors.Errorf("couldn't move these files: %v. Got errors: %v", files, errorsutil.NewAggregate(errs))
}
// shouldBackupAPIServerCertAndKey checks if the cert of kube-apiserver will be expired in 180 days.
func shouldBackupAPIServerCertAndKey(certAndKeyDir string) (bool, error) {
apiServerCert := filepath.Join(certAndKeyDir, kubeadmconstants.APIServerCertName)
certs, err := certutil.CertsFromFile(apiServerCert)
if err != nil {
return false, errors.Wrapf(err, "couldn't load the certificate file %s", apiServerCert)
}
if len(certs) == 0 {
return false, errors.New("no certificate data found")
}
if time.Since(certs[0].NotBefore) > expiry {
return true, nil
}
return false, nil
}
cmd/kubeadm/app/phases/upgrade/postupgrade_test.go
View file @ 137137cb
......@@ -21,40 +21,13 @@ import (
"path/filepath"
"strings"
"testing"
"time"
"github.com/pkg/errors"
kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
"k8s.io/kubernetes/cmd/kubeadm/app/constants"
certsphase "k8s.io/kubernetes/cmd/kubeadm/app/phases/certs"
testutil "k8s.io/kubernetes/cmd/kubeadm/test"
)
func TestBackupAPIServerCertAndKey(t *testing.T) {
tmpdir := testutil.SetupTempDir(t)
defer os.RemoveAll(tmpdir)
os.Chmod(tmpdir, 0766)
certPath := filepath.Join(tmpdir, constants.APIServerCertName)
certFile, err := os.OpenFile(certPath, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, 0666)
if err != nil {
t.Fatalf("Failed to create cert file %s: %v", certPath, err)
}
defer certFile.Close()
keyPath := filepath.Join(tmpdir, constants.APIServerKeyName)
keyFile, err := os.OpenFile(keyPath, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, 0666)
if err != nil {
t.Fatalf("Failed to create key file %s: %v", keyPath, err)
}
defer keyFile.Close()
if err := backupAPIServerCertAndKey(tmpdir); err != nil {
t.Fatalf("Failed to backup cert and key in dir %s: %v", tmpdir, err)
}
}
func TestMoveFiles(t *testing.T) {
tmpdir := testutil.SetupTempDir(t)
defer os.RemoveAll(tmpdir)
......@@ -128,59 +101,3 @@ func TestRollbackFiles(t *testing.T) {
t.Fatalf("Expected error contains %q, got %v", errString, err)
}
}
func TestShouldBackupAPIServerCertAndKey(t *testing.T) {
cfg := &kubeadmapi.InitConfiguration{
LocalAPIEndpoint: kubeadmapi.APIEndpoint{AdvertiseAddress: "1.2.3.4"},
ClusterConfiguration: kubeadmapi.ClusterConfiguration{
Networking: kubeadmapi.Networking{ServiceSubnet: "10.96.0.0/12", DNSDomain: "cluster.local"},
},
NodeRegistration: kubeadmapi.NodeRegistrationOptions{Name: "test-node"},
}
for desc, test := range map[string]struct {
adjustedExpiry time.Duration
expected bool
}{
"default: cert not older than 180 days doesn't needs to backup": {
expected: false,
},
"cert older than 180 days need to backup": {
adjustedExpiry: expiry + 100*time.Hour,
expected: true,
},
} {
t.Run(desc, func(t *testing.T) {
tmpdir := testutil.SetupTempDir(t)
defer os.RemoveAll(tmpdir)
cfg.CertificatesDir = tmpdir
caCert, caKey, err := certsphase.KubeadmCertRootCA.CreateAsCA(cfg)
if err != nil {
t.Fatalf("failed creation of ca cert and key: %v", err)
}
caCert.NotBefore = caCert.NotBefore.Add(-test.adjustedExpiry).UTC()
err = certsphase.KubeadmCertAPIServer.CreateFromCA(cfg, caCert, caKey)
if err != nil {
t.Fatalf("Test %s: failed creation of cert and key: %v", desc, err)
}
certAndKey := []string{filepath.Join(tmpdir, constants.APIServerCertName), filepath.Join(tmpdir, constants.APIServerKeyName)}
for _, path := range certAndKey {
if _, err := os.Stat(path); os.IsNotExist(err) {
t.Fatalf("Test %s: %s not exist: %v", desc, path, err)
}
}
shouldBackup, err := shouldBackupAPIServerCertAndKey(tmpdir)
if err != nil {
t.Fatalf("Test %s: failed to check shouldBackupAPIServerCertAndKey: %v", desc, err)
}
if shouldBackup != test.expected {
t.Fatalf("Test %s: shouldBackupAPIServerCertAndKey expected %v, got %v", desc, test.expected, shouldBackup)
}
})
}
}
cmd/kubeadm/app/phases/upgrade/staticpods_test.go
View file @ 137137cb
......@@ -505,6 +505,7 @@ func TestStaticPodControlPlane(t *testing.T) {
pathMgr,
newcfg,
true,
true,
fakeTLSEtcdClient{
TLS: false,
},
......@@ -637,19 +638,20 @@ func TestCleanupDirs(t *testing.T) {
}
}
func TestRenewCerts(t *testing.T) {
func TestRenewCertsByComponent(t *testing.T) {
caCert, caKey := certstestutil.SetupCertificateAuthorithy(t)
t.Run("all certs exist, should be rotated", func(t *testing.T) {
})
tests := []struct {
name string
component string
skipCreateCA bool
shouldErrorOnRenew bool
certsShouldExist []*certsphase.KubeadmCert
name string
component string
externalCA bool
externalFrontProxyCA bool
skipCreateEtcdCA bool
shouldErrorOnRenew bool
certsShouldExist []*certsphase.KubeadmCert
}{
{
name: "all certs exist, should be rotated",
name: "all certs exist, should be rotated for etcd",
component: constants.Etcd,
certsShouldExist: []*certsphase.KubeadmCert{
&certsphase.KubeadmCertEtcdServer,
......@@ -658,16 +660,37 @@ func TestRenewCerts(t *testing.T) {
},
},
{
name: "just renew API cert",
name: "all certs exist, should be rotated for apiserver",
component: constants.KubeAPIServer,
certsShouldExist: []*certsphase.KubeadmCert{
&certsphase.KubeadmCertEtcdAPIClient,
&certsphase.KubeadmCertAPIServer,
&certsphase.KubeadmCertKubeletClient,
&certsphase.KubeadmCertFrontProxyClient,
},
},
{
name: "external CA, renew only certificates not signed by CA",
component: constants.KubeAPIServer,
certsShouldExist: []*certsphase.KubeadmCert{
&certsphase.KubeadmCertEtcdAPIClient,
&certsphase.KubeadmCertFrontProxyClient,
},
externalCA: true,
},
{
name: "external front-proxy-CA, renew only certificates not signed by front-proxy-CA",
component: constants.KubeAPIServer,
certsShouldExist: []*certsphase.KubeadmCert{
&certsphase.KubeadmCertEtcdAPIClient,
&certsphase.KubeadmCertAPIServer,
&certsphase.KubeadmCertKubeletClient,
},
externalFrontProxyCA: true,
},
{
name: "ignores other compnonents",
skipCreateCA: true,
component: constants.KubeScheduler,
name: "ignores other compnonents",
component: constants.KubeScheduler,
},
{
name: "missing a cert to renew",
......@@ -681,7 +704,7 @@ func TestRenewCerts(t *testing.T) {
{
name: "no CA, cannot continue",
component: constants.Etcd,
skipCreateCA: true,
skipCreateEtcdCA: true,
shouldErrorOnRenew: true,
},
}
......@@ -695,9 +718,21 @@ func TestRenewCerts(t *testing.T) {
cfg := testutil.GetDefaultInternalConfig(t)
cfg.CertificatesDir = tmpDir
if !test.skipCreateCA {
if err := pkiutil.WriteCertAndKey(tmpDir, constants.CACertAndKeyBaseName, caCert, caKey); err != nil {
t.Fatalf("couldn't write out CA: %v", err)
}
if test.externalCA {
os.Remove(filepath.Join(tmpDir, constants.CAKeyName))
}
if err := pkiutil.WriteCertAndKey(tmpDir, constants.FrontProxyCACertAndKeyBaseName, caCert, caKey); err != nil {
t.Fatalf("couldn't write out front-proxy-CA: %v", err)
}
if test.externalFrontProxyCA {
os.Remove(filepath.Join(tmpDir, constants.FrontProxyCAKeyName))
}
if !test.skipCreateEtcdCA {
if err := pkiutil.WriteCertAndKey(tmpDir, constants.EtcdCACertAndKeyBaseName, caCert, caKey); err != nil {
t.Fatalf("couldn't write out CA: %v", err)
t.Fatalf("couldn't write out etcd-CA: %v", err)
}
}
......@@ -719,7 +754,7 @@ func TestRenewCerts(t *testing.T) {
}
// Renew everything
err := renewCerts(cfg, test.component)
err := renewCertsByComponent(cfg, test.component)
if test.shouldErrorOnRenew {
if err == nil {
t.Fatal("expected renewal error, got nothing")
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment