From e5b8f654fe75d95b9afc84cd3fbb08a525bd2d6e Mon Sep 17 00:00:00 2001 From: Sharif Elgamal Date: Mon, 20 Sep 2021 18:13:24 -0700 Subject: [PATCH 1/7] renew minikube certs if expired --- pkg/minikube/bootstrapper/certs.go | 65 ++++++++++++++++++++++++++---- 1 file changed, 57 insertions(+), 8 deletions(-) diff --git a/pkg/minikube/bootstrapper/certs.go b/pkg/minikube/bootstrapper/certs.go index 3f5cd9897687..08c2ab01b934 100644 --- a/pkg/minikube/bootstrapper/certs.go +++ b/pkg/minikube/bootstrapper/certs.go @@ -18,6 +18,7 @@ package bootstrapper import ( "crypto/sha1" + "crypto/x509" "encoding/pem" "fmt" "io/ioutil" @@ -28,6 +29,7 @@ import ( "path/filepath" "sort" "strings" + "time" "github.com/otiai10/copy" "github.com/pkg/errors" @@ -51,12 +53,12 @@ func SetupCerts(cmd command.Runner, k8s config.KubernetesConfig, n config.Node) localPath := localpath.Profile(k8s.ClusterName) klog.Infof("Setting up %s for IP: %s\n", localPath, n.IP) - ccs, err := generateSharedCACerts() + ccs, regen, err := generateSharedCACerts() if err != nil { return errors.Wrap(err, "shared CA certs") } - xfer, err := generateProfileCerts(k8s, n, ccs) + xfer, err := generateProfileCerts(k8s, n, ccs, regen) if err != nil { return errors.Wrap(err, "profile certs") } @@ -148,7 +150,8 @@ type CACerts struct { } // generateSharedCACerts generates CA certs shared among profiles, but only if missing -func generateSharedCACerts() (CACerts, error) { +func generateSharedCACerts() (CACerts, bool, error) { + regenProfileCerts := false globalPath := localpath.MiniPath() cc := CACerts{ caCert: localpath.CACert(), @@ -175,22 +178,23 @@ func generateSharedCACerts() (CACerts, error) { } for _, ca := range caCertSpecs { - if canRead(ca.certPath) && canRead(ca.keyPath) { + if isValid(ca.certPath, ca.keyPath) { klog.Infof("skipping %s CA generation: %s", ca.subject, ca.keyPath) continue } + regenProfileCerts = true klog.Infof("generating %s CA: %s", ca.subject, ca.keyPath) if err := util.GenerateCACert(ca.certPath, ca.keyPath, ca.subject); err != nil { - return cc, errors.Wrap(err, "generate ca cert") + return cc, false, errors.Wrap(err, "generate ca cert") } } - return cc, nil + return cc, regenProfileCerts, nil } // generateProfileCerts generates profile certs for a profile -func generateProfileCerts(k8s config.KubernetesConfig, n config.Node, ccs CACerts) ([]string, error) { +func generateProfileCerts(k8s config.KubernetesConfig, n config.Node, ccs CACerts, regen bool) ([]string, error) { // Only generate these certs for the api server if !n.ControlPlane { @@ -289,12 +293,18 @@ func generateProfileCerts(k8s config.KubernetesConfig, n config.Node, ccs CACert kp = kp + "." + spec.hash } - if canRead(cp) && canRead(kp) { + if !regen && isValid(cp, kp) { klog.Infof("skipping %s signed cert generation: %s", spec.subject, kp) continue } klog.Infof("generating %s signed cert: %s", spec.subject, kp) + if canRead(cp) { + os.Remove(cp) + } + if canRead(kp) { + os.Remove(kp) + } err := util.GenerateSignedCert( cp, kp, spec.subject, spec.ips, spec.alternateNames, @@ -478,3 +488,42 @@ func canRead(path string) bool { defer f.Close() return true } + +// isValid checks a cert/key path and makes sure it's still valid +func isValid(certPath, keyPath string) bool { + if !canRead(keyPath) { + return false + } + + certFile, err := os.ReadFile(certPath) + if err != nil { + klog.Infof("failed to read cert file %s: %v", certPath, err) + os.Remove(certPath) + os.Remove(keyPath) + return false + } + + certData, _ := pem.Decode(certFile) + if certData == nil { + klog.Infof("failed to decode cert file %s", certPath) + os.Remove(certPath) + os.Remove(keyPath) + return false + } + + cert, err := x509.ParseCertificate(certData.Bytes) + if err != nil { + klog.Infof("failed to parse cert file %s: %v\n%v", certPath, err, string(certFile)) + os.Remove(certPath) + os.Remove(keyPath) + return false + } + + if cert.NotAfter.Before(time.Now()) { + os.Remove(certPath) + os.Remove(keyPath) + return false + } + + return true +} From 1212848649b844bb577516c79c277c3098763d42 Mon Sep 17 00:00:00 2001 From: Sharif Elgamal Date: Tue, 21 Sep 2021 16:17:52 -0700 Subject: [PATCH 2/7] 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 21e15a000b12..265146fe347c 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 4d18749ccd23..4f9d9d0d7134 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 08c2ab01b934..cee2edaf1842 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 4956284dc8a5..c23935cecf6b 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 e2990381a166..1b38c0d15eae 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 2fcc3ca69039..b338e207d957 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 2d45fabbaf20..5df290c478b4 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 7fc6a0137361..067b9800c951 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 adb9fdae043a..cac5c1db86c8 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 0dead5683d1c..ed8682519737 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()) + } +} From 5ee889c88573634bbf152f6c4c024213b83aada5 Mon Sep 17 00:00:00 2001 From: Sharif Elgamal Date: Tue, 21 Sep 2021 16:40:43 -0700 Subject: [PATCH 3/7] add docstring for TestCertExpiration --- test/integration/cert_options_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/integration/cert_options_test.go b/test/integration/cert_options_test.go index ed8682519737..0a2ca017b8fe 100644 --- a/test/integration/cert_options_test.go +++ b/test/integration/cert_options_test.go @@ -81,6 +81,9 @@ func TestCertOptions(t *testing.T) { } +// TestCertExpiration makes sure minikube can start after its profile certs have expired. +// It does this by configuring minikube certs to expire after 3 minutes, then waiting 3 minutes, then starting again. +// It also makes sure minikube prints a cert expiration warning to the user. func TestCertExpiration(t *testing.T) { if NoneDriver() { t.Skip("skipping: none driver does not support ssh or bundle docker") From 2799f24e124d13b351b4e1d973aec13538acb885 Mon Sep 17 00:00:00 2001 From: Sharif Elgamal Date: Tue, 21 Sep 2021 17:38:09 -0700 Subject: [PATCH 4/7] don't skip none driver, fix logging --- pkg/minikube/bootstrapper/certs.go | 3 ++- test/integration/cert_options_test.go | 3 --- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/pkg/minikube/bootstrapper/certs.go b/pkg/minikube/bootstrapper/certs.go index cee2edaf1842..b72d51be9d34 100644 --- a/pkg/minikube/bootstrapper/certs.go +++ b/pkg/minikube/bootstrapper/certs.go @@ -516,7 +516,7 @@ func isValid(certPath, keyPath string) bool { cert, err := x509.ParseCertificate(certData.Bytes) if err != nil { - klog.Infof("failed to parse cert file %s: %v\n%v", certPath, err, string(certFile)) + klog.Infof("failed to parse cert file %s: %v\n", certPath, err) os.Remove(certPath) os.Remove(keyPath) return false @@ -524,6 +524,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)}) + klog.Infof("cert expired %s: expiration: %s, now: %s", certPath, cert.NotAfter, time.Now()) os.Remove(certPath) os.Remove(keyPath) return false diff --git a/test/integration/cert_options_test.go b/test/integration/cert_options_test.go index 0a2ca017b8fe..3ff990b98c22 100644 --- a/test/integration/cert_options_test.go +++ b/test/integration/cert_options_test.go @@ -85,9 +85,6 @@ func TestCertOptions(t *testing.T) { // It does this by configuring minikube certs to expire after 3 minutes, then waiting 3 minutes, then starting again. // It also makes sure minikube prints a cert expiration warning to the user. 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") From 77051a35df3286de1fc76b3ab99149d956641fcf Mon Sep 17 00:00:00 2001 From: Sharif Elgamal Date: Wed, 22 Sep 2021 11:22:08 -0700 Subject: [PATCH 5/7] make sure to set expiration if not in existing config --- cmd/minikube/cmd/start_flags.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/cmd/minikube/cmd/start_flags.go b/cmd/minikube/cmd/start_flags.go index 265146fe347c..ca08701447db 100644 --- a/cmd/minikube/cmd/start_flags.go +++ b/cmd/minikube/cmd/start_flags.go @@ -583,6 +583,10 @@ func upgradeExistingConfig(cmd *cobra.Command, cc *config.ClusterConfig) { cc.KubernetesConfig.NodePort = viper.GetInt(apiServerPort) } + if cc.CertExpiration == 0 { + cc.CertExpiration = pkgutil.DefaultCertExpiration + } + } // updateExistingConfigFromFlags will update the existing config from the flags - used on a second start From 18d89d565cef17c9ef1f99ef55fa73829f2c1726 Mon Sep 17 00:00:00 2001 From: Sharif Elgamal Date: Thu, 23 Sep 2021 13:39:10 -0700 Subject: [PATCH 6/7] remove pkg install tests --- test/integration/pkg_install_test.go | 110 --------------------------- 1 file changed, 110 deletions(-) delete mode 100644 test/integration/pkg_install_test.go diff --git a/test/integration/pkg_install_test.go b/test/integration/pkg_install_test.go deleted file mode 100644 index 12bfc4710425..000000000000 --- a/test/integration/pkg_install_test.go +++ /dev/null @@ -1,110 +0,0 @@ -//go:build integration -// +build integration - -/* -Copyright 2021 The Kubernetes Authors All rights reserved. - -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 integration - -import ( - "context" - "fmt" - "os/exec" - "path/filepath" - "runtime" - "strings" - "testing" -) - -var distros = []string{ - "debian:sid", - "debian:latest", - "debian:10", - "debian:9", - "ubuntu:latest", - "ubuntu:20.10", - "ubuntu:20.04", - "ubuntu:18.04", -} - -var timeout = Minutes(10) - -// TestPackageInstall tests installation of .deb packages with minikube itself and with kvm2 driver -// on various debian/ubuntu docker images -func TestDebPackageInstall(t *testing.T) { - - ctx, cancel := context.WithTimeout(context.Background(), timeout) - defer cancel() - - rr, err := Run(t, exec.CommandContext(ctx, "docker", "version")) - if err != nil || rr.ExitCode != 0 { - t.Skip("docker is not installed") - } - - pkgDir, err := filepath.Abs(filepath.Dir(Target())) - if err != nil { - t.Errorf("failed to get minikube path: %v", err) - } - mkDebs, err := filepath.Glob(fmt.Sprintf("%s/minikube_*_%s.deb", pkgDir, runtime.GOARCH)) - if err != nil { - t.Errorf("failed to find minikube deb in %q: %v", pkgDir, err) - } - kvmDebs, err := filepath.Glob(fmt.Sprintf("%s/docker-machine-driver-kvm2_*_%s.deb", pkgDir, runtime.GOARCH)) - if err != nil { - t.Errorf("failed to find minikube deb in %q: %v", pkgDir, err) - } - - for _, distro := range distros { - distroImg := distro - testName := fmt.Sprintf("install_%s_%s", runtime.GOARCH, strings.ReplaceAll(distroImg, ":", "_")) - t.Run(testName, func(t *testing.T) { - // apt-get update; dpkg -i minikube_${ver}_${arch}.deb - t.Run("minikube", func(t *testing.T) { - for _, mkDeb := range mkDebs { - rr, err := dpkgInstall(ctx, t, distro, mkDeb) - if err != nil || rr.ExitCode != 0 { - t.Errorf("failed to install %q on %q: err=%v, exit=%d", - mkDeb, distroImg, err, rr.ExitCode) - } - } - }) - // apt-get update; apt-get install -y libvirt0; dpkg -i docker-machine-driver-kvm2_${ver}_${arch}.deb - t.Run("kvm2-driver", func(t *testing.T) { - for _, kvmDeb := range kvmDebs { - rr, err := dpkgInstallDriver(ctx, t, distro, kvmDeb) - if err != nil || rr.ExitCode != 0 { - t.Errorf("failed to install %q on %q: err=%v, exit=%d", - kvmDeb, distroImg, err, rr.ExitCode) - } - } - }) - }) - } -} - -func dpkgInstall(ctx context.Context, t *testing.T, image, deb string) (*RunResult, error) { - return Run(t, exec.CommandContext(ctx, - "docker", "run", "--rm", fmt.Sprintf("-v%s:/var/tmp", filepath.Dir(deb)), - image, - "sh", "-c", fmt.Sprintf("apt-get update; dpkg -i /var/tmp/%s", filepath.Base(deb)))) -} - -func dpkgInstallDriver(ctx context.Context, t *testing.T, image, deb string) (*RunResult, error) { - return Run(t, exec.CommandContext(ctx, - "docker", "run", "--rm", fmt.Sprintf("-v%s:/var/tmp", filepath.Dir(deb)), - image, - "sh", "-c", fmt.Sprintf("apt-get update; apt-get install -y libvirt0; dpkg -i /var/tmp/%s", filepath.Base(deb)))) -} From 4307e834dfa6e232f136858760e2598aeb798066 Mon Sep 17 00:00:00 2001 From: Sharif Elgamal Date: Fri, 24 Sep 2021 11:47:10 -0700 Subject: [PATCH 7/7] address review comments --- cmd/minikube/cmd/start_flags.go | 4 ++-- pkg/minikube/bootstrapper/certs.go | 1 + pkg/minikube/bootstrapper/certs_test.go | 2 +- pkg/minikube/constants/constants.go | 3 +++ pkg/util/crypto.go | 2 -- pkg/util/crypto_test.go | 2 +- 6 files changed, 8 insertions(+), 6 deletions(-) diff --git a/cmd/minikube/cmd/start_flags.go b/cmd/minikube/cmd/start_flags.go index ca08701447db..ba87db888eef 100644 --- a/cmd/minikube/cmd/start_flags.go +++ b/cmd/minikube/cmd/start_flags.go @@ -170,7 +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.") + startCmd.Flags().Duration(certExpiration, constants.DefaultCertExpiration, "Duration until minikube certificate expiration, defaults to three years (26280h).") } // initKubernetesFlags inits the commandline flags for Kubernetes related options @@ -584,7 +584,7 @@ func upgradeExistingConfig(cmd *cobra.Command, cc *config.ClusterConfig) { } if cc.CertExpiration == 0 { - cc.CertExpiration = pkgutil.DefaultCertExpiration + cc.CertExpiration = constants.DefaultCertExpiration } } diff --git a/pkg/minikube/bootstrapper/certs.go b/pkg/minikube/bootstrapper/certs.go index b72d51be9d34..ff843bea5bb9 100644 --- a/pkg/minikube/bootstrapper/certs.go +++ b/pkg/minikube/bootstrapper/certs.go @@ -493,6 +493,7 @@ func canRead(path string) bool { } // isValid checks a cert/key path and makes sure it's still valid +// if a cert is expired or otherwise invalid, it will be deleted func isValid(certPath, keyPath string) bool { if !canRead(keyPath) { return false diff --git a/pkg/minikube/bootstrapper/certs_test.go b/pkg/minikube/bootstrapper/certs_test.go index c23935cecf6b..b0c4e255d1ac 100644 --- a/pkg/minikube/bootstrapper/certs_test.go +++ b/pkg/minikube/bootstrapper/certs_test.go @@ -33,7 +33,7 @@ func TestSetupCerts(t *testing.T) { defer tests.RemoveTempDir(tempDir) k8s := config.ClusterConfig{ - CertExpiration: util.DefaultCertExpiration, + CertExpiration: constants.DefaultCertExpiration, KubernetesConfig: config.KubernetesConfig{ APIServerName: constants.APIServerName, DNSDomain: constants.ClusterDNSDomain, diff --git a/pkg/minikube/constants/constants.go b/pkg/minikube/constants/constants.go index 162f9027fc92..791c41558a44 100644 --- a/pkg/minikube/constants/constants.go +++ b/pkg/minikube/constants/constants.go @@ -116,6 +116,9 @@ const ( TimeFormat = time.RFC1123 // MaxResources is the value that can be passed into the memory and cpus flags to specify to use maximum resources MaxResources = "max" + + // DefaultCertExpiration is the amount of time in the future a certificate will expire in by default, which is 3 years + DefaultCertExpiration = time.Hour * 24 * 365 * 3 ) var ( diff --git a/pkg/util/crypto.go b/pkg/util/crypto.go index 067b9800c951..5222957390cf 100644 --- a/pkg/util/crypto.go +++ b/pkg/util/crypto.go @@ -35,8 +35,6 @@ 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) diff --git a/pkg/util/crypto_test.go b/pkg/util/crypto_test.go index cac5c1db86c8..b6db86436808 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, DefaultCertExpiration, + test.signerKeyPath, constants.DefaultCertExpiration, ) if err != nil && !test.err { t.Errorf("GenerateSignedCert() error = %v", err)