Skip to content

Conversation

sofixa
Copy link
Contributor

@sofixa sofixa commented Jan 14, 2025

Add support for the Vault KV subkeys API path.

Cf. #2015

@sofixa sofixa requested a review from a team as a code owner January 14, 2025 16:53
@sreeram77
Copy link
Member

Hi @sofixa, couple of test cases are failing in TestShimKVv2Path. Could you please take a look at it?

@sofixa
Copy link
Contributor Author

sofixa commented Jan 28, 2025

Hi @sreeram77 , I can't test locally because there is no documentation on how to do that. It just vaguely says "install Nomad, Consul, Vault"... which I have, and am running locally. I get this when running the tests:

failed to start consul server: api unavailable
FAIL    github.com/hashicorp/consul-template/dependency 3.142s

Which isn't a very helpful.

How can I actually run this locally to be able to debug the failing test / my failing code?

@sreeram77
Copy link
Member

sreeram77 commented Jan 29, 2025

Hi @sofixa
make test should ideally work. In your case, you can alternatively try go test ./... -run TestShimKVv2Path which will run only the test cases for the modified function. If you're still getting the same error, there might be something wrong with the consul installation/binary.

@sofixa
Copy link
Contributor Author

sofixa commented Jan 29, 2025

What's the setup I need for Consul? Any test I try to run, it tries to start the Consul server via consul/sdk/testutil, but fails at https://github.com/hashicorp/consul/blob/4bca5c5d219a7d616f6ae19cca1bc5af913455c3/sdk/testutil/server.go#L467 . How could I even debug this? It doesn't seem related to my Consul setup, because it starts a test server via the SDK directly.

Adrian

@sofixa
Copy link
Contributor Author

sofixa commented Jan 29, 2025

Managed to work around the failing Consul dependency by creating a separate file calling the same function and go run-ing it directly. It was a dumb extra / in the test cases, fixed, all tests should be OK now.

Copy link
Member

@sreeram77 sreeram77 left a comment

Choose a reason for hiding this comment

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

LGTM!

@sreeram77 sreeram77 merged commit 716e206 into hashicorp:main Feb 3, 2025
26 of 29 checks passed
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