Do not use tlsconfig.Clone, which is unsafe in go 1.7 if the config is already being used

[cyli]

Rather than clone the config, since we already have utilities to create a new TLS configuration for the client and server using the TLS certs themselves and the root pool, just recreate a new TLS configuration rather than attempt to clone the existing one.

Accessing the certificates in the tls.Config object does not cause a data race:

package main_test

import (
	"crypto/tls"
	"fmt"
	"net/http"
	"net/http/httptest"
	"testing"

	"github.com/stretchr/testify/require"
)

// TestDockerTLSConfigClone attempts to trigger a data race when calling config.Clone while a server is running
func TestDockerTLSConfigClone(t *testing.T) {
	client := http.Client{
		Transport: &http.Transport{
			TLSClientConfig: &tls.Config{
				InsecureSkipVerify: true,
			},
		},
	}

	s := httptest.NewTLSServer(http.DefaultServeMux)
	defer s.Close()
	go func() {
		fmt.Println(s.TLS.Certificates)
	}()

	done := make(chan struct{})
	go func() {
		defer close(done)
		_, err := client.Get(s.URL)
		require.NoError(t, err)
	}()

	<-done
}

is fine under go 1.7 and go 1.8

Fixes #2012.

[cyli]

Both are fine, since the manager certificate has both as DNS names, but for consistency I changed this to the manager role instead.

[aaronlehmann]

Wouldn't this reintroduce this issue: #1983 (comment) ?

[cyli]

Ah right... :| in that case I will go with your suggestion of keeping a copy of the TLS config. I was hoping to do most of the cloning in the config. I actually wonder if we need to take out a lock on SecurityConfig, though, as well because RenewTLSConfigNow might update everything with an old root CA after UpdateRootCA is called.

[cyli]

I added a test which attempted to simultaneously update the certs and update the root CA. It looks like I do need a lock on the root CA, because otherwise renewing the cert would overwrite the root CA changes. :|

[codecov-io]

Codecov Report

Merging #2013 into master will decrease coverage by -0.17%.
The diff coverage is 72.41%.

@@            Coverage Diff             @@
##           master    #2013      +/-   ##
==========================================
- Coverage   53.96%   53.79%   -0.17%     
==========================================
  Files         109      109              
  Lines       18997    18980      -17     
==========================================
- Hits        10251    10211      -40     
- Misses       7527     7532       +5     
- Partials     1219     1237      +18

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dd916da...5636661. Read the comment docs.

[cyli]

cc @diogomonica

[aaronlehmann]

LGTM

[aaronlehmann]

Merging to fix CI problems. @diogomonica: feel free to review post-merge.