From 1212848649b844bb577516c79c277c3098763d42 Mon Sep 17 00:00:00 2001 From: Sharif Elgamal Date: Tue, 21 Sep 2021 16:17:52 -0700 Subject: [PATCH] make cert expiration configurable and add test --- cmd/minikube/cmd/start_flags.go | 4 +++ pkg/minikube/bootstrapper/bootstrapper.go | 2 +- pkg/minikube/bootstrapper/certs.go | 10 +++++-- pkg/minikube/bootstrapper/certs_test.go | 11 ++++--- pkg/minikube/bootstrapper/kubeadm/kubeadm.go | 2 +- pkg/minikube/config/types.go | 1 + pkg/minikube/node/start.go | 4 +-- pkg/util/crypto.go | 6 ++-- pkg/util/crypto_test.go | 2 +- test/integration/cert_options_test.go | 31 +++++++++++++++++++- 10 files changed, 58 insertions(+), 15 deletions(-) diff --git a/cmd/minikube/cmd/start_flags.go b/cmd/minikube/cmd/start_flags.go index 21e15a000b..265146fe34 100644 --- a/cmd/minikube/cmd/start_flags.go +++ b/cmd/minikube/cmd/start_flags.go @@ -122,6 +122,7 @@ const ( defaultSSHPort = 22 listenAddress = "listen-address" extraDisks = "extra-disks" + certExpiration = "cert-expiration" ) var ( @@ -169,6 +170,7 @@ func initMinikubeFlags() { startCmd.Flags().StringVarP(&outputFormat, "output", "o", "text", "Format to print stdout in. Options include: [text,json]") startCmd.Flags().StringP(trace, "", "", "Send trace events. Options include: [gcp]") startCmd.Flags().Int(extraDisks, 0, "Number of extra disks created and attached to the minikube VM (currently only implemented for hyperkit and kvm2 drivers)") + startCmd.Flags().Duration(certExpiration, time.Hour*24*365, "Duration until minikube certificate expiration, defaults to one year.") } // initKubernetesFlags inits the commandline flags for Kubernetes related options @@ -455,6 +457,7 @@ func generateNewConfigFromFlags(cmd *cobra.Command, k8sVersion string, drvName s SSHKey: viper.GetString(sshSSHKey), SSHPort: viper.GetInt(sshSSHPort), ExtraDisks: viper.GetInt(extraDisks), + CertExpiration: viper.GetDuration(certExpiration), KubernetesConfig: config.KubernetesConfig{ KubernetesVersion: k8sVersion, ClusterName: ClusterFlagValue(), @@ -652,6 +655,7 @@ func updateExistingConfigFromFlags(cmd *cobra.Command, existing *config.ClusterC updateStringFromFlag(cmd, &cc.KubernetesConfig.ServiceCIDR, serviceCIDR) updateBoolFromFlag(cmd, &cc.KubernetesConfig.ShouldLoadCachedImages, cacheImages) updateIntFromFlag(cmd, &cc.KubernetesConfig.NodePort, apiServerPort) + updateDurationFromFlag(cmd, &cc.CertExpiration, certExpiration) if cmd.Flags().Changed(kubernetesVersion) { cc.KubernetesConfig.KubernetesVersion = getKubernetesVersion(existing) diff --git a/pkg/minikube/bootstrapper/bootstrapper.go b/pkg/minikube/bootstrapper/bootstrapper.go index 4d18749ccd..4f9d9d0d71 100644 --- a/pkg/minikube/bootstrapper/bootstrapper.go +++ b/pkg/minikube/bootstrapper/bootstrapper.go @@ -44,7 +44,7 @@ type Bootstrapper interface { GenerateToken(config.ClusterConfig) (string, error) // LogCommands returns a map of log type to a command which will display that log. LogCommands(config.ClusterConfig, LogOptions) map[string]string - SetupCerts(config.KubernetesConfig, config.Node) error + SetupCerts(config.ClusterConfig, config.Node) error GetAPIServerStatus(string, int) (string, error) } diff --git a/pkg/minikube/bootstrapper/certs.go b/pkg/minikube/bootstrapper/certs.go index 08c2ab01b9..cee2edaf18 100644 --- a/pkg/minikube/bootstrapper/certs.go +++ b/pkg/minikube/bootstrapper/certs.go @@ -44,13 +44,14 @@ import ( "k8s.io/minikube/pkg/minikube/constants" "k8s.io/minikube/pkg/minikube/kubeconfig" "k8s.io/minikube/pkg/minikube/localpath" + "k8s.io/minikube/pkg/minikube/out" "k8s.io/minikube/pkg/minikube/vmpath" "k8s.io/minikube/pkg/util" ) // SetupCerts gets the generated credentials required to talk to the APIServer. -func SetupCerts(cmd command.Runner, k8s config.KubernetesConfig, n config.Node) error { - localPath := localpath.Profile(k8s.ClusterName) +func SetupCerts(cmd command.Runner, k8s config.ClusterConfig, n config.Node) error { + localPath := localpath.Profile(k8s.KubernetesConfig.ClusterName) klog.Infof("Setting up %s for IP: %s\n", localPath, n.IP) ccs, regen, err := generateSharedCACerts() @@ -194,13 +195,14 @@ func generateSharedCACerts() (CACerts, bool, error) { } // generateProfileCerts generates profile certs for a profile -func generateProfileCerts(k8s config.KubernetesConfig, n config.Node, ccs CACerts, regen bool) ([]string, error) { +func generateProfileCerts(cfg config.ClusterConfig, n config.Node, ccs CACerts, regen bool) ([]string, error) { // Only generate these certs for the api server if !n.ControlPlane { return []string{}, nil } + k8s := cfg.KubernetesConfig profilePath := localpath.Profile(k8s.ClusterName) serviceIP, err := util.GetServiceClusterIP(k8s.ServiceCIDR) @@ -309,6 +311,7 @@ func generateProfileCerts(k8s config.KubernetesConfig, n config.Node, ccs CACert cp, kp, spec.subject, spec.ips, spec.alternateNames, spec.caCertPath, spec.caKeyPath, + cfg.CertExpiration, ) if err != nil { return xfer, errors.Wrapf(err, "generate signed cert for %q", spec.subject) @@ -520,6 +523,7 @@ func isValid(certPath, keyPath string) bool { } if cert.NotAfter.Before(time.Now()) { + out.WarningT("Certificate {{.certPath}} has expired. Generating a new one...", out.V{"certPath": filepath.Base(certPath)}) os.Remove(certPath) os.Remove(keyPath) return false diff --git a/pkg/minikube/bootstrapper/certs_test.go b/pkg/minikube/bootstrapper/certs_test.go index 4956284dc8..c23935cecf 100644 --- a/pkg/minikube/bootstrapper/certs_test.go +++ b/pkg/minikube/bootstrapper/certs_test.go @@ -32,10 +32,13 @@ func TestSetupCerts(t *testing.T) { tempDir := tests.MakeTempDir() defer tests.RemoveTempDir(tempDir) - k8s := config.KubernetesConfig{ - APIServerName: constants.APIServerName, - DNSDomain: constants.ClusterDNSDomain, - ServiceCIDR: constants.DefaultServiceCIDR, + k8s := config.ClusterConfig{ + CertExpiration: util.DefaultCertExpiration, + KubernetesConfig: config.KubernetesConfig{ + APIServerName: constants.APIServerName, + DNSDomain: constants.ClusterDNSDomain, + ServiceCIDR: constants.DefaultServiceCIDR, + }, } if err := os.Mkdir(filepath.Join(tempDir, "certs"), 0777); err != nil { diff --git a/pkg/minikube/bootstrapper/kubeadm/kubeadm.go b/pkg/minikube/bootstrapper/kubeadm/kubeadm.go index e2990381a1..1b38c0d15e 100644 --- a/pkg/minikube/bootstrapper/kubeadm/kubeadm.go +++ b/pkg/minikube/bootstrapper/kubeadm/kubeadm.go @@ -847,7 +847,7 @@ func (k *Bootstrapper) DeleteCluster(k8s config.KubernetesConfig) error { } // SetupCerts sets up certificates within the cluster. -func (k *Bootstrapper) SetupCerts(k8s config.KubernetesConfig, n config.Node) error { +func (k *Bootstrapper) SetupCerts(k8s config.ClusterConfig, n config.Node) error { return bootstrapper.SetupCerts(k.c, k8s, n) } diff --git a/pkg/minikube/config/types.go b/pkg/minikube/config/types.go index 2fcc3ca690..b338e207d9 100644 --- a/pkg/minikube/config/types.go +++ b/pkg/minikube/config/types.go @@ -84,6 +84,7 @@ type ClusterConfig struct { Network string // only used by docker driver MultiNodeRequested bool ExtraDisks int // currently only implemented for hyperkit and kvm2 + CertExpiration time.Duration } // KubernetesConfig contains the parameters used to configure the VM Kubernetes. diff --git a/pkg/minikube/node/start.go b/pkg/minikube/node/start.go index 2d45fabbaf..5df290c478 100644 --- a/pkg/minikube/node/start.go +++ b/pkg/minikube/node/start.go @@ -156,7 +156,7 @@ func Start(starter Starter, apiServer bool) (*kubeconfig.Settings, error) { return nil, errors.Wrap(err, "Failed to get bootstrapper") } - if err = bs.SetupCerts(starter.Cfg.KubernetesConfig, *starter.Node); err != nil { + if err = bs.SetupCerts(*starter.Cfg, *starter.Node); err != nil { return nil, errors.Wrap(err, "setting up certs") } @@ -445,7 +445,7 @@ func setupKubeAdm(mAPI libmachine.API, cfg config.ClusterConfig, n config.Node, exit.Error(reason.KubernetesInstallFailed, "Failed to update cluster", err) } - if err := bs.SetupCerts(cfg.KubernetesConfig, n); err != nil { + if err := bs.SetupCerts(cfg, n); err != nil { exit.Error(reason.GuestCert, "Failed to setup certs", err) } diff --git a/pkg/util/crypto.go b/pkg/util/crypto.go index 7fc6a01373..067b9800c9 100644 --- a/pkg/util/crypto.go +++ b/pkg/util/crypto.go @@ -35,6 +35,8 @@ import ( "k8s.io/minikube/pkg/util/lock" ) +const DefaultCertExpiration = time.Hour * 24 * 365 + // GenerateCACert generates a CA certificate and RSA key for a common name func GenerateCACert(certPath, keyPath string, name string) error { priv, err := rsa.GenerateKey(rand.Reader, 2048) @@ -65,7 +67,7 @@ func GenerateCACert(certPath, keyPath string, name string) error { // Any parent directories of the certPath or keyPath will be created as needed with file mode 0755. // GenerateSignedCert generates a signed certificate and key -func GenerateSignedCert(certPath, keyPath, cn string, ips []net.IP, alternateDNS []string, signerCertPath, signerKeyPath string) error { +func GenerateSignedCert(certPath, keyPath, cn string, ips []net.IP, alternateDNS []string, signerCertPath, signerKeyPath string, expiration time.Duration) error { klog.Infof("Generating cert %s with IP's: %s", certPath, ips) signerCertBytes, err := ioutil.ReadFile(signerCertPath) if err != nil { @@ -99,7 +101,7 @@ func GenerateSignedCert(certPath, keyPath, cn string, ips []net.IP, alternateDNS Organization: []string{"system:masters"}, }, NotBefore: time.Now().Add(time.Hour * -24), - NotAfter: time.Now().Add(time.Hour * 24 * 365), + NotAfter: time.Now().Add(expiration), KeyUsage: x509.KeyUsageKeyEncipherment | x509.KeyUsageDigitalSignature, ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth, x509.ExtKeyUsageClientAuth}, diff --git a/pkg/util/crypto_test.go b/pkg/util/crypto_test.go index adb9fdae04..cac5c1db86 100644 --- a/pkg/util/crypto_test.go +++ b/pkg/util/crypto_test.go @@ -140,7 +140,7 @@ func TestGenerateSignedCert(t *testing.T) { t.Run(test.description, func(t *testing.T) { err := GenerateSignedCert( certPath, keyPath, "minikube", ips, alternateDNS, test.signerCertPath, - test.signerKeyPath, + test.signerKeyPath, DefaultCertExpiration, ) if err != nil && !test.err { t.Errorf("GenerateSignedCert() error = %v", err) diff --git a/test/integration/cert_options_test.go b/test/integration/cert_options_test.go index 0dead5683d..ed86825197 100644 --- a/test/integration/cert_options_test.go +++ b/test/integration/cert_options_test.go @@ -24,6 +24,7 @@ import ( "os/exec" "strings" "testing" + "time" ) // TestCertOptions makes sure minikube certs respect the --apiserver-ips and --apiserver-names parameters @@ -37,7 +38,6 @@ func TestCertOptions(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), Minutes(30)) defer CleanupWithLogs(t, profile, cancel) - // Use the most verbose logging for the simplest test. If it fails, something is very wrong. args := append([]string{"start", "-p", profile, "--memory=2048", "--apiserver-ips=127.0.0.1", "--apiserver-ips=192.168.15.15", "--apiserver-names=localhost", "--apiserver-names=www.google.com", "--apiserver-port=8555"}, StartArgs()...) // We can safely override --apiserver-name with @@ -80,3 +80,32 @@ func TestCertOptions(t *testing.T) { } } + +func TestCertExpiration(t *testing.T) { + if NoneDriver() { + t.Skip("skipping: none driver does not support ssh or bundle docker") + } + MaybeParallel(t) + + profile := UniqueProfileName("cert-expiration") + ctx, cancel := context.WithTimeout(context.Background(), Minutes(30)) + defer CleanupWithLogs(t, profile, cancel) + + args := append([]string{"start", "-p", profile, "--memory=2048", "--cert-expiration=3m"}, StartArgs()...) + + rr, err := Run(t, exec.CommandContext(ctx, Target(), args...)) + if err != nil { + t.Errorf("failed to start minikube with args: %q : %v", rr.Command(), err) + } + + // Now wait 3 minutes for the certs to expire and make sure minikube starts properly + time.Sleep(time.Minute * 3) + args = append([]string{"start", "-p", profile, "--memory=2048", "--cert-expiration=8760h"}, StartArgs()...) + rr, err = Run(t, exec.CommandContext(ctx, Target(), args...)) + if err != nil { + t.Errorf("failed to start minikube after cert expiration: %q : %v", rr.Command(), err) + } + if !strings.Contains(rr.Output(), "expired") { + t.Errorf("minikube start output did not warn about expired certs: %v", rr.Output()) + } +}