fix(vault): exit token renewal when setup fails

[archy-rock3t-cloud]

renewToken called Renew() on a nil watcher after RenewSelf failed, panicking the operator.

• Return on setup errors.
• Stop() closes stopCh rather than sending.
• Set vh.client before the goroutine.

Checklist

• I have verified that my change is according to the deprecations & breaking changes policy
• Tests have been added (if applicable)
• Changelog has been updated and is aligned with our changelog requirements, only when the change impacts end users
• Commits are signed with Developer Certificate of Origin (DCO - learn more)

Fixes #7939

[snyk-io]

✅ Snyk checks have passed. No issues have been found so far.

Status	Scan Engine	Critical	High	Medium	Low	Total (0)
✅	Open Source Security	0	0	0	0	0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

[github-actions]

Thank you for your contribution! 🙏

Please understand that we will do our best to review your PR and give you feedback as soon as possible, but please bear with us if it takes a little longer as expected.

While you are waiting, make sure to:

• Add an entry in our changelog in alphabetical order and link related issue
• Update the documentation, if needed
• Add unit & e2e tests for your changes
• GitHub checks are passing
• Is the DCO check failing? Here is how you can fix DCO issues

Once the initial tests are successful, a KEDA member will ensure that the e2e tests are run. Once the e2e tests have been successfully completed, the PR may be merged at a later date. Please be patient.

Learn more about our contribution guide.

[rickbrouwer]

Thanks. Is it possible to add an unit test for this?

[archy-rock3t-cloud]

@rickbrouwer sure, updated

[rickbrouwer]

It isn't really affected by this PR, but maybe we can take it into account. But with the early return, a failed RenewSelf now means the token is never renewed again for the lifetime of this handler, while the operator keeps running until the token expires. Would it make sense to make that explicit in the log message, so the eventual 403s on secret resolution are easier to trace back? "failed to create the payload" does not really describe the failure. Would something like "Vault renew token: failed to renew the token, renewal is disabled for this handler" be clearer? Wdyt?

[wozniakjan]

/run-e2e vault
Update: You can check the progress here

passed tests: 3

Execution of tests/secret-providers/azure_keyvault/azure_keyvault_test.go, has passed after "one" attempts
Execution of tests/secret-providers/azure_keyvault_workload_identity/azure_keyvault_workload_identity_test.go, has passed after "one" attempts
Execution of tests/secret-providers/hashicorp_vault/hashicorp_vault_test.go, has passed after "two" attempts

failed tests: 0

[wozniakjan]

Would something like "Vault renew token: failed to renew the token, renewal is disabled for this handler" be clearer? Wdyt?

checking the error messages, I agree they are a bit confusing, but since they have been that way for a long time, I'm ok addressing those separately

means the token is never renewed again for the lifetime of this handle

wondering if it's possible to run the renewal in a forever async loop like it's done for the elastic forecast scaler?

keda/pkg/scalers/elastic_forecast_scaler.go

Line 140 in 5744068

func (s *elasticForecastScaler) forecastRenewal(ctx context.Context) {

[dttung2905]

/run-e2e vault
Update: You can check the progress here

⚠️ infrastructure failure: the e2e run did not complete normally

setup / teardown failures: 1

setup failed (tests/utils/setup_test.go): TestSetupArgoRollouts

passed tests: 0

failed tests: 0