venconn: test that venconn's url is used, not server field

Author: maelvls

pkg/agent/config_test.go

@@ -2,17 +2,24 @@ package agent
 
 import (
 	"bytes"
+	"context"
+	"crypto/x509"
 	"fmt"
 	"io"
 	"log"
+	"net/http"
 	"os"
 	"strings"
 	"testing"
 	"time"
 
 	"github.com/jetstack/preflight/pkg/client"
+	"github.com/jetstack/preflight/pkg/testutil"
 	"github.com/kylelemons/godebug/diff"
+	"github.com/spf13/cobra"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"gopkg.in/yaml.v3"
 )
 
 func TestGetConfiguration(t *testing.T) {
@@ -188,6 +195,100 @@ func TestGetConfiguration(t *testing.T) {
 	})
 }
 
+// Slower test cases due to envtest. That's why they are separated from the
+// other tests.
+func Test_getConfiguration_urlWhenVenafiConnection(t *testing.T) {
+	t.Run("the server field is ignored when VenafiConnection is used", func(t *testing.T) {
+		_, restCfg, kcl := testutil.WithEnvtest(t)
+		os.Setenv("KUBECONFIG", testutil.WithKubeconfig(t, restCfg))
+		srv, fakeCrt, setVenafiCloudAssert := testutil.FakeVenafiCloud(t)
+		for _, obj := range testutil.Parse(
+			testutil.VenConnRBAC + testutil.Undent(fmt.Sprintf(`
+			---
+			apiVersion: jetstack.io/v1alpha1
+			kind: VenafiConnection
+			metadata:
+			  name: venafi-components
+			  namespace: venafi
+			spec:
+			  vcp:
+			    url: "%s"
+			    accessToken:
+			      - secret:
+			          name: accesstoken
+			          fields: [accesstoken]
+			---
+			apiVersion: v1
+			kind: Secret
+			metadata:
+			  name: accesstoken
+			  namespace: venafi
+			stringData:
+			  accesstoken: VALID_ACCESS_TOKEN
+			---
+			apiVersion: rbac.authorization.k8s.io/v1
+			kind: Role
+			metadata:
+			  name: venafi-connection-accesstoken-reader
+			  namespace: venafi
+			rules:
+			- apiGroups: [""]
+			  resources: ["secrets"]
+			  verbs: ["get"]
+			  resourceNames: ["accesstoken"]
+			---
+			apiVersion: rbac.authorization.k8s.io/v1
+			kind: RoleBinding
+			metadata:
+			  name: venafi-connection-accesstoken-reader
+			  namespace: venafi
+			roleRef:
+			  apiGroup: rbac.authorization.k8s.io
+			  kind: Role
+			  name: venafi-connection-accesstoken-reader
+			subjects:
+			- kind: ServiceAccount
+			  name: venafi-connection
+			  namespace: venafi`, srv.URL))) {
+			require.NoError(t, kcl.Create(context.Background(), obj))
+		}
+
+		// The URL received by the fake Venafi Cloud server should be the one
+		// coming from the VenafiConnection, not the one from the config.
+		setVenafiCloudAssert(func(t testing.TB, r *http.Request) {
+			assert.Equal(t, srv.URL, "https://"+r.Host)
+		})
+
+		cfg, err := ParseConfig([]byte(testutil.Undent(`
+			server: "http://should-be-ignored"
+			period: 1h
+			`)), true)
+		assert.NoError(t, err)
+
+		_, cl, err := getConfiguration(discardLogs(t),
+			cfg,
+			withCmdLineFlags("--venafi-connection", "venafi-components", "--install-namespace", "venafi"),
+		)
+		assert.NoError(t, err)
+
+		// `Start(ctx)` needs to be stopped before the apiserver is stopped.
+		// https://github.com/jetstack/venafi-connection-lib/pull/158#issuecomment-1949002322
+		ctx, cancel := context.WithCancel(context.Background())
+		t.Cleanup(cancel)
+		go func() {
+			require.NoError(t, cl.(*client.VenConnClient).Start(ctx))
+		}()
+		certPool := x509.NewCertPool()
+		certPool.AddCert(fakeCrt)
+		tr := http.DefaultTransport.(*http.Transport).Clone()
+		tr.TLSClientConfig.RootCAs = certPool
+		cl.(*client.VenConnClient).Client.Transport = tr
+
+		err = cl.PostDataReadingsWithOptions(nil, client.Options{ClusterName: "test cluster name"})
+		assert.NoError(t, err)
+	})
+}
+
 // Fills in the `server` and `period` as they appear in each and every test
 // case.
 func fillRequired(c Config) Config {
@@ -457,15 +558,42 @@ func withFile(t testing.TB, content string) string {
 	return f.Name()
 }
 
-func withLogs(t testing.TB) (*log.Logger, *bytes.Buffer) {
+func withLogs(_ testing.TB) (*log.Logger, *bytes.Buffer) {
 	b := bytes.Buffer{}
 	return log.New(&b, "", 0), &b
 }
 
-func discardLogs(t testing.TB) *log.Logger {
+func discardLogs(_ testing.TB) *log.Logger {
 	return log.New(io.Discard, "", 0)
 }
 
+// Shortcut for ParseConfig.
+func withConfig(s string) Config {
+	var cfg Config
+
+	err := yaml.Unmarshal([]byte(s), &cfg)
+	if err != nil {
+		panic(err)
+	}
+	return cfg
+}
+
+func withCmdLineFlags(flags ...string) AgentCmdFlags {
+	parsed := withoutCmdLineFlags()
+	agentCmd := &cobra.Command{}
+	InitAgentCmdFlags(agentCmd, &parsed)
+	err := agentCmd.ParseFlags(flags)
+	if err != nil {
+		panic(err)
+	}
+
+	return parsed
+}
+
+func withoutCmdLineFlags() AgentCmdFlags {
+	return AgentCmdFlags{}
+}
+
 const fakeKubeconfig = `
 apiVersion: v1
 clusters:

pkg/client/client_venconn.go

@@ -29,10 +29,15 @@ import (
 type VenConnClient struct {
 	agentMetadata *api.AgentMetadata
 	connHandler   venafi_client.ConnectionHandler
-	installNS     string       // Namespace in which the agent is running in.
-	venConnName   string       // Name of the VenafiConnection resource to use.
-	venConnNS     string       // Namespace of the VenafiConnection resource to use.
-	client        *http.Client // Used to make HTTP requests to Venafi Cloud.
+	installNS     string // Namespace in which the agent is running in.
+	venConnName   string // Name of the VenafiConnection resource to use.
+	venConnNS     string // Namespace of the VenafiConnection resource to use.
+
+	// Used to make HTTP requests to Venafi Cloud. This field is public for
+	// testing purposes so that we can configure trusted CAs; there should be a
+	// way to do that without messing with the client directly (e.g., a flag to
+	// pass a custom CA?), but it's not there yet.
+	Client *http.Client
 }
 
 // NewVenConnClient lets you make requests to the Venafi Cloud backend using the
@@ -111,7 +116,7 @@ func NewVenConnClient(restcfg *rest.Config, agentMetadata *api.AgentMetadata, in
 		installNS:     installNS,
 		venConnName:   venConnName,
 		venConnNS:     venConnNS,
-		client:        vcpClient,
+		Client:        vcpClient,
 	}, nil
 }
 
@@ -180,7 +185,7 @@ func (c *VenConnClient) PostDataReadingsWithOptions(readings []*api.DataReading,
 	}
 	req.URL.RawQuery = q.Encode()
 
-	res, err := c.client.Do(req)
+	res, err := c.Client.Do(req)
 	if err != nil {
 		return err
 	}

pkg/testutil/testutil.go

@@ -215,6 +215,7 @@ func FakeVenafiCloud(t *testing.T) (_ *httptest.Server, _ *x509.Certificate, set
 		if r.URL.Path == "/v1/tlspk/upload/clusterdata/no" {
 			if r.URL.Query().Get("name") != "test cluster name" {
 				w.WriteHeader(http.StatusBadRequest)
+				_, _ = w.Write([]byte(`{"error":"unexpected name query param in the test server: ` + r.URL.Query().Get("name") + `"}`))
 				return
 			}
 			_, _ = w.Write([]byte(`{"status":"ok","organization":"756db001-280e-11ee-84fb-991f3177e2d0"}`))