config_test: add a failing test to showcase that uploader_id missing

I found that, in venafi cloud key pair service account mode, the
arbitrary path segment "/no" was now missing due to my refactoring. This
commit adds a unit test to show this.

Test result:

--- FAIL: Test_ValidateAndCombineConfig_VenafiCloudKeyPair (0.00s)
    --- FAIL: Test_ValidateAndCombineConfig_VenafiCloudKeyPair/server,_uploader_id,_and_cluster_name_are_correctly_passed (0.00s)
        envtest.go:188: fake api.venafi.cloud received request: POST /v1/oauth/token/serviceaccount
        envtest.go:188: fake api.venafi.cloud received request: POST /v1/tlspk/upload/clusterdata
        config_test.go:586:
                Error:          Not equal:
                                expected: "/v1/tlspk/upload/clusterdata/no"
                                actual  : "/v1/tlspk/upload/clusterdata"

                                Diff:
                                --- Expected
                                +++ Actual
                                @@ -1 +1 @@
                                -/v1/tlspk/upload/clusterdata/no
                                +/v1/tlspk/upload/clusterdata
                Test:           Test_ValidateAndCombineConfig_VenafiCloudKeyPair/server,_uploader_id,_and_cluster_name_are_correctly_passed

Author: maelvls

pkg/agent/config_test.go

@@ -81,7 +81,6 @@ func Test_ValidateAndCombineConfig(t *testing.T) {
 		assert.Equal(t, testutil.Undent(`
 			Using the Jetstack Secure OAuth auth mode since --credentials-file was specified without --venafi-cloud.
 			Both the 'period' field and --period are set. Using the value provided with --period.
-
 		`), gotLogs.String())
 		assert.Equal(t, 99*time.Minute, got.Period)
 	})
@@ -572,9 +571,44 @@ func Test_ValidateAndCombineConfig(t *testing.T) {
 	})
 }
 
+func Test_ValidateAndCombineConfig_VenafiCloudKeyPair(t *testing.T) {
+	t.Run("server, uploader_id, and cluster name are correctly passed", func(t *testing.T) {
+		srv, cert, setVenafiCloudAssert := testutil.FakeVenafiCloud(t)
+		setVenafiCloudAssert(func(t testing.TB, gotReq *http.Request) {
+			// Only care about /v1/tlspk/upload/clusterdata/:uploader_id?name=
+			if gotReq.URL.Path == "/v1/oauth/token/serviceaccount" {
+				return
+			}
+
+			assert.Equal(t, srv.URL, "https://"+gotReq.Host)
+			assert.Equal(t, "test cluster name", gotReq.URL.Query().Get("name"))
+			assert.Equal(t, "/v1/tlspk/upload/clusterdata/no", gotReq.URL.Path)
+		})
+
+		privKeyPath := withFile(t, fakePrivKeyPEM)
+		got, cl, err := ValidateAndCombineConfig(discardLogs(),
+			withConfig(testutil.Undent(`
+				server: `+srv.URL+`
+				period: 1h
+				cluster_id: "test cluster name"
+				venafi-cloud:
+				  uploader_id: no
+				  upload_path: /v1/tlspk/upload/clusterdata
+			`)),
+			withCmdLineFlags("--client-id", "5bc7d07c-45da-11ef-a878-523f1e1d7de1", "--private-key-path", privKeyPath),
+		)
+		testutil.TrustCA(t, cl, cert)
+		assert.Equal(t, VenafiCloudKeypair, got.AuthMode)
+		require.NoError(t, err)
+
+		err = cl.PostDataReadingsWithOptions(nil, client.Options{ClusterName: "test cluster name"})
+		require.NoError(t, err)
+	})
+}
+
 // Slower test cases due to envtest. That's why they are separated from the
 // other tests.
-func Test_ValidateAndCombineConfig_urlWhenVenafiConnection(t *testing.T) {
+func Test_ValidateAndCombineConfig_VenafiConnection(t *testing.T) {
 	_, cfg, kcl := testutil.WithEnvtest(t)
 	t.Setenv("KUBECONFIG", testutil.WithKubeconfig(t, cfg))
 	srv, cert, setVenafiCloudAssert := testutil.FakeVenafiCloud(t)
@@ -588,7 +622,7 @@ func Test_ValidateAndCombineConfig_urlWhenVenafiConnection(t *testing.T) {
 			  namespace: venafi
 			spec:
 			  vcp:
-			    url: "%s"
+			    url: "`+srv.URL+`"
 			    accessToken:
 			      - secret:
 			          name: accesstoken
@@ -626,7 +660,7 @@ func Test_ValidateAndCombineConfig_urlWhenVenafiConnection(t *testing.T) {
 			- kind: ServiceAccount
 			  name: venafi-connection
 			  namespace: venafi
-		`, srv.URL))) {
+		`))) {
 		require.NoError(t, kcl.Create(context.Background(), obj))
 	}
 
@@ -654,7 +688,7 @@ func Test_ValidateAndCombineConfig_urlWhenVenafiConnection(t *testing.T) {
 		require.NoError(t, err)
 
 		testutil.VenConnStartWatching(t, cl)
-		testutil.VenConnTrustCA(t, cl, cert)
+		testutil.TrustCA(t, cl, cert)
 
 		// TODO(mael): the client should keep track of the cluster ID, we
 		// shouldn't need to pass it as an option to

pkg/client/client_venafi_cloud.go

@@ -43,13 +43,15 @@ type (
 		accessToken   *venafiCloudAccessToken
 		baseURL       string
 		agentMetadata *api.AgentMetadata
-		client        *http.Client
 
 		uploaderID    string
 		uploadPath    string
 		privateKey    crypto.PrivateKey
 		jwtSigningAlg jwt.SigningMethod
 		lock          sync.RWMutex
+
+		// Made public for testing purposes.
+		Client *http.Client
 	}
 
 	VenafiSvcAccountCredentials struct {
@@ -109,7 +111,7 @@ func NewVenafiCloudClient(agentMetadata *api.AgentMetadata, credentials *VenafiS
 		credentials:   credentials,
 		baseURL:       baseURL,
 		accessToken:   &venafiCloudAccessToken{},
-		client:        &http.Client{Timeout: time.Minute},
+		Client:        &http.Client{Timeout: time.Minute},
 		uploaderID:    uploaderID,
 		uploadPath:    uploadPath,
 		privateKey:    privateKey,
@@ -270,7 +272,7 @@ func (c *VenafiCloudClient) Post(path string, body io.Reader) (*http.Response, e
 		req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", token.accessToken))
 	}
 
-	return c.client.Do(req)
+	return c.Client.Do(req)
 }
 
 // getValidAccessToken returns a valid access token. It will fetch a new access
@@ -325,7 +327,7 @@ func (c *VenafiCloudClient) updateAccessToken() error {
 }
 
 func (c *VenafiCloudClient) sendHTTPRequest(request *http.Request, responseObject interface{}) error {
-	response, err := c.client.Do(request)
+	response, err := c.Client.Do(request)
 	if err != nil {
 		return err
 	}

pkg/testutil/envtest.go

@@ -12,7 +12,6 @@ import (
 	"sync"
 	"testing"
 
-	"github.com/jetstack/preflight/pkg/client"
 	"github.com/jetstack/venafi-connection-lib/api/v1alpha1"
 	"github.com/stretchr/testify/require"
 	corev1 "k8s.io/api/core/v1"
@@ -25,6 +24,8 @@ import (
 	clientcmdapi "k8s.io/client-go/tools/clientcmd/api"
 	ctrlruntime "sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/envtest"
+
+	"github.com/jetstack/preflight/pkg/client"
 )
 
 // To see the API server logs, set:
@@ -123,21 +124,31 @@ func VenConnStartWatching(t *testing.T, cl client.Client) {
 	t.Cleanup(cancel)
 }
 
-func VenConnTrustCA(t *testing.T, cl client.Client, cert *x509.Certificate) {
+// Works with VenafiCloudClient and VenConnClient. Allows you to trust a given
+// CA.
+func TrustCA(t *testing.T, cl client.Client, cert *x509.Certificate) {
 	t.Helper()
-	require.IsType(t, &client.VenConnClient{}, cl)
-	vcCl := cl.(*client.VenConnClient)
+
+	var httpClient *http.Client
+	switch c := cl.(type) {
+	case *client.VenafiCloudClient:
+		httpClient = c.Client
+	case *client.VenConnClient:
+		httpClient = c.Client
+	default:
+		t.Fatalf("unsupported client type: %T", cl)
+	}
 
 	pool := x509.NewCertPool()
 	pool.AddCert(cert)
 
-	if vcCl.Client.Transport == nil {
-		vcCl.Client.Transport = http.DefaultTransport
+	if httpClient.Transport == nil {
+		httpClient.Transport = http.DefaultTransport
 	}
-	if vcCl.Client.Transport.(*http.Transport).TLSClientConfig == nil {
-		vcCl.Client.Transport.(*http.Transport).TLSClientConfig = &tls.Config{}
+	if httpClient.Transport.(*http.Transport).TLSClientConfig == nil {
+		httpClient.Transport.(*http.Transport).TLSClientConfig = &tls.Config{}
 	}
-	vcCl.Client.Transport.(*http.Transport).TLSClientConfig.RootCAs = pool
+	httpClient.Transport.(*http.Transport).TLSClientConfig.RootCAs = pool
 }
 
 // Parses the YAML manifest. Useful for inlining YAML manifests in Go test
@@ -180,10 +191,19 @@ func FakeVenafiCloud(t *testing.T) (_ *httptest.Server, _ *x509.Certificate, set
 		defer assertFnMu.Unlock()
 		assertFn(t, r)
 
+		if r.URL.Path == "/v1/oauth2/v2.0/756db001-280e-11ee-84fb-991f3177e2d0/token" {
+			_, _ = w.Write([]byte(`{"access_token":"VALID_ACCESS_TOKEN","expires_in":900,"token_type":"bearer"}`))
+			return
+		} else if r.URL.Path == "/v1/oauth/token/serviceaccount" {
+			_, _ = w.Write([]byte(`{"access_token":"VALID_ACCESS_TOKEN","expires_in":900,"token_type":"bearer"}`))
+			return
+		}
+
 		accessToken := strings.TrimPrefix(r.Header.Get("Authorization"), "Bearer ")
 		apiKey := r.Header.Get("tppl-api-key")
 		if accessToken != "VALID_ACCESS_TOKEN" && apiKey != "VALID_API_KEY" {
 			w.WriteHeader(http.StatusUnauthorized)
+			w.Write([]byte(`{"error":"expected header 'Authorization: Bearer VALID_ACCESS_TOKEN' or 'tppl-api-key: VALID_API_KEY', but got Authorization=` + r.Header.Get("Authorization") + ` and tppl-api-key=` + r.Header.Get("tppl-api-key")))
 			return
 		}
 		if r.URL.Path == "/v1/tlspk/upload/clusterdata/no" {
@@ -194,11 +214,7 @@ func FakeVenafiCloud(t *testing.T) (_ *httptest.Server, _ *x509.Certificate, set
 			}
 			_, _ = w.Write([]byte(`{"status":"ok","organization":"756db001-280e-11ee-84fb-991f3177e2d0"}`))
 		} else if r.URL.Path == "/v1/useraccounts" {
-			w.WriteHeader(http.StatusOK)
 			_, _ = w.Write([]byte(`{"user": {"username": "user","id": "76a126f0-280e-11ee-84fb-991f3177e2d0"}}`))
-
-		} else if r.URL.Path == "/v1/oauth2/v2.0/756db001-280e-11ee-84fb-991f3177e2d0/token" {
-			_, _ = w.Write([]byte(`{"access_token":"VALID_ACCESS_TOKEN","expires_in":900,"token_type":"bearer"}`))
 		} else {
 			w.WriteHeader(http.StatusInternalServerError)
 			_, _ = w.Write([]byte(`{"error":"unexpected path in the test server","path":"` + r.URL.Path + `"}`))