Skip to content

Conversation

maelvls
Copy link
Member

@maelvls maelvls commented Oct 7, 2024

Ref: VC-36593

While preparing for https://venafi.atlassian.net/browse/VC-36353, I found that I was using go run . --client-id redacted without --venafi-cloud.

Olu and I worried that it might be a combination of flags that is no longer possible due to the changes introduced in https://venafi.atlassian.net/browse/VC-36043.

To prevent any future breakage, this PR adds a unit test that enforces that it is possible to use --client-id along with --venafi-cloud, since this combination is used in the Helm chart.

@maelvls maelvls changed the title tests: make sure --venafi-cloud can be used along with --client-id VC-36593: Tests: Make sure --venafi-cloud can be used along with --client-id Oct 7, 2024
@maelvls maelvls requested a review from tfadeyi October 7, 2024 14:16
@maelvls maelvls requested review from wallrj and removed request for tfadeyi November 5, 2024 13:59
Copy link
Member

@wallrj wallrj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @maelvls

I ran the test locally with a modification so that I could see what log messages are produced by this combination.

diff --git a/pkg/agent/config_test.go b/pkg/agent/config_test.go index 71d8802..e3e64a7 100644 --- a/pkg/agent/config_test.go +++ b/pkg/agent/config_test.go @@ -259,8 +259,9 @@ func Test_ValidateAndCombineConfig(t *testing.T) { }) t.Run("venafi-cloud-keypair-auth: it is possible to use --client-id with --venafi-cloud", func(t *testing.T) { + l, b := recordLogs() privKeyPath := withFile(t, fakePrivKeyPEM) - got, cl, err := ValidateAndCombineConfig(discardLogs(), + got, cl, err := ValidateAndCombineConfig(l, withConfig(testutil.Undent(` server: "http://localhost:8080" period: 1h @@ -273,6 +274,7 @@ func Test_ValidateAndCombineConfig(t *testing.T) { require.NoError(t, err) assert.Equal(t, VenafiCloudKeypair, got.AuthMode) assert.IsType(t, &client.VenafiCloudClient{}, cl) + t.Log(b.String()) }) t.Run("jetstack-secure-oauth-auth: fail if organization_id or cluster_id is missing and --venafi-cloud not enabled", func(t *testing.T) {
$ go test ./pkg/agent/... -run "Test_ValidateAndCombineConfig/venafi-cloud-keypair-auth: it is possible to use --client-id with --venafi-cloud" -v --count 1 === RUN Test_ValidateAndCombineConfig === RUN Test_ValidateAndCombineConfig/venafi-cloud-keypair-auth:_it_is_possible_to_use_--client-id_with_--venafi-cloud  config_test.go:277: Using the Venafi Cloud Key Pair Service Account auth mode since --client-id and --private-key-path were specified.  Using period from config 1h0m0s  Loading upload_path from "venafi-cloud" configuration. --- PASS: Test_ValidateAndCombineConfig (0.00s)  --- PASS: Test_ValidateAndCombineConfig/venafi-cloud-keypair-auth:_it_is_possible_to_use_--client-id_with_--venafi-cloud (0.00s) === RUN Test_ValidateAndCombineConfig_VenafiCloudKeyPair --- PASS: Test_ValidateAndCombineConfig_VenafiCloudKeyPair (0.00s) === RUN Test_ValidateAndCombineConfig_VenafiConnection  envtest.go:51: Waiting for envtest to exit --- PASS: Test_ValidateAndCombineConfig_VenafiConnection (6.60s) PASS ok github.com/jetstack/preflight/pkg/agent 6.627s
  1. I can't remember the circumstances where client-id is and is not required? Is it only required when using private key based Venafi Cloud service accounts? And not required when using OIDC service accounts?
  2. I didn't understand why these tests require a Kubernetes API server via envtest?
$ go test ./pkg/agent/... -run "Test_ValidateAndCombineConfig/venafi-cloud-keypair-auth: it is possible to use --client-id with --venafi-cloud" -v --count 1 === RUN Test_ValidateAndCombineConfig === RUN Test_ValidateAndCombineConfig/venafi-cloud-keypair-auth:_it_is_possible_to_use_--client-id_with_--venafi-cloud  config_test.go:277: Using the Venafi Cloud Key Pair Service Account auth mode since --client-id and --private-key-path were specified.  Using period from config 1h0m0s  Loading upload_path from "venafi-cloud" configuration. --- PASS: Test_ValidateAndCombineConfig (0.00s)  --- PASS: Test_ValidateAndCombineConfig/venafi-cloud-keypair-auth:_it_is_possible_to_use_--client-id_with_--venafi-cloud (0.00s) === RUN Test_ValidateAndCombineConfig_VenafiCloudKeyPair --- PASS: Test_ValidateAndCombineConfig_VenafiCloudKeyPair (0.00s) === RUN Test_ValidateAndCombineConfig_VenafiConnection  config_test.go:631: KUBEBUILDER_ASSETS isn't set. You can run this test using `make test`.  But if you prefer not to use `make`, run these two commands first:  make _bin/tools/{kube-apiserver,etcd}  export KUBEBUILDER_ASSETS=$PWD/_bin/tools --- FAIL: Test_ValidateAndCombineConfig_VenafiConnection (0.00s) FAIL FAIL github.com/jetstack/preflight/pkg/agent 0.024s FAIL 

/approve
/lgtm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants