Merge pull request #6889 from tstromberg/symlink-inversion

Fix inverted certificate symlink creation logic

Author: medyagh

pkg/minikube/bootstrapper/certs.go

@@ -130,9 +130,8 @@ func SetupCerts(cmd command.Runner, k8s config.KubernetesConfig, n config.Node)
 		}
 	}
 
-	// configure CA certificates
-	if err := configureCACerts(cmd, caCerts); err != nil {
-		return errors.Wrapf(err, "Configuring CA certs")
+	if err := installCertSymlinks(cmd, caCerts); err != nil {
+		return errors.Wrapf(err, "certificate symlinks")
 	}
 	return nil
 }
@@ -318,9 +317,9 @@ func getSubjectHash(cr command.Runner, filePath string) (string, error) {
 	return stringHash, nil
 }
 
-// configureCACerts looks up and installs all uploaded PEM certificates in /usr/share/ca-certificates to system-wide certificate store (/etc/ssl/certs).
+// installCertSymlinks installs certs in /usr/share/ca-certificates into system-wide certificate store (/etc/ssl/certs).
 // OpenSSL binary required in minikube ISO
-func configureCACerts(cr command.Runner, caCerts map[string]string) error {
+func installCertSymlinks(cr command.Runner, caCerts map[string]string) error {
 	hasSSLBinary := true
 	_, err := cr.RunCmd(exec.Command("openssl", "version"))
 	if err != nil {
@@ -334,7 +333,8 @@ func configureCACerts(cr command.Runner, caCerts map[string]string) error {
 	for _, caCertFile := range caCerts {
 		dstFilename := path.Base(caCertFile)
 		certStorePath := path.Join(vmpath.GuestCertStoreDir, dstFilename)
-		cmd := fmt.Sprintf("test -f %s || ln -fs %s %s", caCertFile, certStorePath, caCertFile)
+		// If the cert really exists, add a named symlink
+		cmd := fmt.Sprintf("test -f %s && ln -fs %s %s", caCertFile, caCertFile, certStorePath)
 		if _, err := cr.RunCmd(exec.Command("sudo", "/bin/bash", "-c", cmd)); err != nil {
 			return errors.Wrapf(err, "create symlink for %s", caCertFile)
 		}

pkg/minikube/bootstrapper/certs_test.go

@@ -52,8 +52,8 @@ func TestSetupCerts(t *testing.T) {
 	}
 
 	expected := map[string]string{
-		`sudo /bin/bash -c "test -f /usr/share/ca-certificates/mycert.pem || ln -fs /etc/ssl/certs/mycert.pem /usr/share/ca-certificates/mycert.pem"`:             "-",
-		`sudo /bin/bash -c "test -f /usr/share/ca-certificates/minikubeCA.pem || ln -fs /etc/ssl/certs/minikubeCA.pem /usr/share/ca-certificates/minikubeCA.pem"`: "-",
+		`sudo /bin/bash -c "test -f /usr/share/ca-certificates/mycert.pem && ln -fs /usr/share/ca-certificates/mycert.pem /etc/ssl/certs/mycert.pem"`:             "-",
+		`sudo /bin/bash -c "test -f /usr/share/ca-certificates/minikubeCA.pem && ln -fs /usr/share/ca-certificates/minikubeCA.pem /etc/ssl/certs/minikubeCA.pem"`: "-",
 	}
 	f := command.NewFakeCommandRunner()
 	f.SetCommandToOutput(expected)

pkg/minikube/command/ssh_runner.go

@@ -241,11 +241,13 @@ func (s *SSHRunner) sameFileExists(f assets.CopyableFile, dst string) (bool, err
 	if err != nil {
 		return false, err
 	}
+	glog.Infof("found %s: %d bytes, modified at %s", dst, dstSize, dstModTime)
 
 	// compare sizes and modtimes
 	if srcSize != dstSize {
 		return false, errors.New("source file and destination file are different sizes")
 	}
+
 	return srcModTime.Equal(dstModTime), nil
 }
 

test/integration/functional_test.go

@@ -28,6 +28,7 @@ import (
 	"net/url"
 	"os"
 	"os/exec"
+	"path"
 	"path/filepath"
 	"regexp"
 	"runtime"
@@ -57,10 +58,17 @@ func TestFunctional(t *testing.T) {
 	profile := UniqueProfileName("functional")
 	ctx, cancel := context.WithTimeout(context.Background(), Minutes(40))
 	defer func() {
+		if !*cleanup {
+			return
+		}
 		p := localSyncTestPath()
 		if err := os.Remove(p); err != nil {
 			t.Logf("unable to remove %s: %v", p, err)
 		}
+		p = localTestCertPath()
+		if err := os.Remove(p); err != nil {
+			t.Logf("unable to remove %s: %v", p, err)
+		}
 		CleanupWithLogs(t, profile, cancel)
 	}()
 
@@ -110,6 +118,7 @@ func TestFunctional(t *testing.T) {
 			{"SSHCmd", validateSSHCmd},
 			{"MySQL", validateMySQL},
 			{"FileSync", validateFileSync},
+			{"CertSync", validateCertSync},
 			{"UpdateContextCmd", validateUpdateContextCmd},
 			{"DockerEnv", validateDockerEnv},
 			{"NodeLabels", validateNodeLabels},
@@ -731,6 +740,16 @@ func localSyncTestPath() string {
 	return filepath.Join(localpath.MiniPath(), "/files", vmSyncTestPath())
 }
 
+// testCert is name of the test certificate installed
+func testCert() string {
+	return fmt.Sprintf("%d.pem", os.Getpid())
+}
+
+// localTestCertPath is where the test file will be synced into the VM
+func localTestCertPath() string {
+	return filepath.Join(localpath.MiniPath(), "/certs", testCert())
+}
+
 // Copy extra file into minikube home folder for file sync test
 func setupFileSync(ctx context.Context, t *testing.T, profile string) {
 	p := localSyncTestPath()
@@ -739,6 +758,11 @@ func setupFileSync(ctx context.Context, t *testing.T, profile string) {
 	if err != nil {
 		t.Fatalf("copy: %v", err)
 	}
+
+	err = copy.Copy("./testdata/minikube_test.pem", localTestCertPath())
+	if err != nil {
+		t.Fatalf("copy: %v", err)
+	}
 }
 
 // validateFileSync to check existence of the test file
@@ -766,6 +790,39 @@ func validateFileSync(ctx context.Context, t *testing.T, profile string) {
 	}
 }
 
+// validateCertSync to check existence of the test certificate
+func validateCertSync(ctx context.Context, t *testing.T, profile string) {
+	if NoneDriver() {
+		t.Skipf("skipping: ssh unsupported by none")
+	}
+
+	want, err := ioutil.ReadFile("./testdata/minikube_test.pem")
+	if err != nil {
+		t.Errorf("test file not found: %v", err)
+	}
+
+	// Check both the installed & reference certs (they should be symlinked)
+	paths := []string{
+		path.Join("/etc/ssl/certs", testCert()),
+		path.Join("/usr/share/ca-certificates", testCert()),
+		// hashed path generated by: 'openssl x509 -hash -noout -in testCert()'
+		"/etc/ssl/certs/51391683.0",
+	}
+	for _, vp := range paths {
+		t.Logf("Checking for existence of %s within VM", vp)
+		rr, err := Run(t, exec.CommandContext(ctx, Target(), "-p", profile, "ssh", fmt.Sprintf("cat %s", vp)))
+		if err != nil {
+			t.Errorf("%s failed: %v", rr.Args, err)
+		}
+
+		// Strip carriage returned by ssh
+		got := strings.Replace(rr.Stdout.String(), "\r", "", -1)
+		if diff := cmp.Diff(string(want), got); diff != "" {
+			t.Errorf("minikube_test.pem -> %s mismatch (-want +got):\n%s", vp, diff)
+		}
+	}
+}
+
 // validateUpdateContextCmd asserts basic "update-context" command functionality
 func validateUpdateContextCmd(ctx context.Context, t *testing.T, profile string) {
 	rr, err := Run(t, exec.CommandContext(ctx, Target(), "-p", profile, "update-context", "--alsologtostderr", "-v=2"))

test/integration/testdata/minikube_test.pem

@@ -0,0 +1,22 @@
+-----BEGIN CERTIFICATE-----
+MIIDsDCCApgCCQD5n0OIsOYIjDANBgkqhkiG9w0BAQsFADCBmTELMAkGA1UEBhMC
+VVMxEzARBgNVBAgMCkNhbGlmb3JuaWExFjAUBgNVBAcMDVNhbiBGcmFuY2lzY28x
+ETAPBgNVBAoMCG1pbmlrdWJlMRYwFAYDVQQLDA1QYXJ0eSBQYXJyb3RzMREwDwYD
+VQQDDAhtaW5pa3ViZTEfMB0GCSqGSIb3DQEJARYQbWluaWt1YmVAY25jZi5pbzAe
+Fw0yMDAzMDQyMTU2MjZaFw0yMTAzMDQyMTU2MjZaMIGZMQswCQYDVQQGEwJVUzET
+MBEGA1UECAwKQ2FsaWZvcm5pYTEWMBQGA1UEBwwNU2FuIEZyYW5jaXNjbzERMA8G
+A1UECgwIbWluaWt1YmUxFjAUBgNVBAsMDVBhcnR5IFBhcnJvdHMxETAPBgNVBAMM
+CG1pbmlrdWJlMR8wHQYJKoZIhvcNAQkBFhBtaW5pa3ViZUBjbmNmLmlvMIIBIjAN
+BgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA/qVMQ/234ul5yWI1yaHvV4pZ5Ffy
+M0bSMjzZUwlsvzerXzF3WrdpeZs5GzBNBWL/Db9KziGHCtfX9j5okJqPvB2lxdL5
+d5hFIYSORSemYLX2kdlnlykY5fzmFLKIUO9xXs0YNF4JUMEBgGK6n/BdLvXDUULZ
+26QOKs6+iH7TAL4RtozxQ8YXKQArdmpeAvxy2PSZGvVk1htKtyuKQsiFqH3oRleK
+3mljXfC5LsoIJHqd/8lAsckH87+IfwYnJ1CNJM2gueaCf+HmudVrvXfHaszh1Wh1
+9HKPE95Azi6CKoBGlRGFxt8UR72YIcTjC/lYxzbHeCpU7RCiXfsC0iMTlQIDAQAB
+MA0GCSqGSIb3DQEBCwUAA4IBAQBhsKnghyBki4NOnK5gHm7ow+7S+xvkjJhXBQ6i
+/xQD4/GCZ1tH5iFHXmo+bt4jB9hvKLyN5M5a8TlDwwqTLIoPDQJh37UpSCwbY/6z
+nE2aP3N2ue1/DeY60tgAh1c1uJDMeTiFbalJqSkneaHpNfvEQhUORFoN4yQSVEYg
++T9mzTAWQ55TeBgbRevmA25nXHdPAae1MvJWDeG+aJfhq1I2hCwaitJ3iSwgn2ew
+637It/aBkMLhsCKTHxlXDGUX401ddbc0ZiC308cyMbis3iBeh4RBjkFxP8eIWFmK
+sos/dyqdua742L1cOKYFbLJfjA1VyxJQUxQvWKkbaq0xi7ao
+-----END CERTIFICATE-----