Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

VC-36593: Tests: Make sure --venafi-cloud can be used along with --client-id #588

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants