fix: migrating from github.com/hashicorp/vault-client-go to github.com/hashicorp/vault/api by armagankaratosun · Pull Request #8059 · cert-manager/cert-manager

armagankaratosun · 2025-09-09T19:33:03Z

Pull Request Motivation

The Vault E2E addon tests still depend on the deprecated github.com/hashicorp/vault-client-go, which has been archived since 2023-12-18. This PR migrates those tests to use the new github.com/hashicorp/vault/api client.

I verified it by running the E2E suite locally (kind), and all relevant tests passed:

[SynchronizedAfterSuite] PASSED [0.626 seconds]
------------------------------
[ReportAfterSuite] Autogenerated ReportAfterSuite for --junit-report
autogenerated by Ginkgo
[ReportAfterSuite] PASSED [0.058 seconds]
------------------------------

Ran 47 of 882 Specs in 84.281 seconds
SUCCESS! -- 47 Passed | 0 Failed | 0 Pending | 835 Skipped
--- PASS: TestE2E (84.65s)
PASS

Ginkgo ran 1 suite in 1m27.805810539s
Test Suite Passed

Kind

/kind cleanup

Release Note

Vault: Migrate Vault E2E add-on tests from deprecated `vault-client-go` to the new `vault/api` client.

cert-manager-prow · 2025-09-09T19:33:13Z

Hi @armagankaratosun. Thanks for your PR.

I'm waiting for a cert-manager member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

…m/hashicorp/vault/api Signed-off-by: Armagan Karatosun <armagankaratosun@gmail.com>

erikgb · 2025-09-09T19:41:32Z

@armagankaratosun, thanks! ❤️ Can you please fix the DCO so I can enable the CI?

armagankaratosun · 2025-09-09T19:41:56Z

@armagankaratosun, thanks! ❤️ Can you please fix the DCO so I can enable the CI?

Just did it.

erikgb · 2025-09-09T19:43:03Z

/ok-to-test

Signed-off-by: Armagan Karatosun <armagankaratosun@gmail.com>

Copilot

Pull Request Overview

This PR migrates the Vault E2E addon tests from the deprecated hashicorp/vault-client-go library to the maintained hashicorp/vault/api client library. The vault-client-go package has been archived since December 2023, making this migration necessary for continued maintenance and support.

Replaced all vault-client-go API calls with equivalent vault/api calls
Updated Go module dependencies to use the new Vault API client
Refactored Vault client initialization and TLS configuration for the new API

Reviewed Changes

Copilot reviewed 2 out of 3 changed files in this pull request and generated 2 comments.

File	Description
test/e2e/go.mod	Updated dependencies from deprecated vault-client-go to vault/api with required transitive dependencies
test/e2e/framework/addon/vault/setup.go	Complete migration of Vault client code from old API to new API including initialization, PKI operations, auth methods, and policy management

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

test/e2e/framework/addon/vault/setup.go

Signed-off-by: Armagan Karatosun <armagankaratosun@gmail.com>

Copilot

Pull Request Overview

Copilot reviewed 2 out of 3 changed files in this pull request and generated 1 comment.

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

test/e2e/framework/addon/vault/setup.go

erikgb

This is really great stuff, @armagankaratosun! Thanks a lot for fixing this! A couple of nit discussions to resolve before we can merge this.

/label tide/merge-method-squash
/approve

test/e2e/framework/addon/vault/setup.go

cert-manager-prow · 2025-09-10T19:45:21Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: erikgb

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [erikgb]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Signed-off-by: Armagan Karatosun <armagankaratosun@gmail.com>

erikgb

/lgtm

Thanks again, @armagankaratosun! 🚀

armagankaratosun · 2025-09-11T12:27:34Z

/lgtm

Thanks again, @armagankaratosun! 🚀

Well, thanks to you too for helping me shape this PR into its best version

This PR contains the following updates: | Package | Update | Change | |---|---|---| | [cert-manager](https://cert-manager.io) ([source](https://github.com/cert-manager/cert-manager)) | minor | `v1.18.2` -> `v1.19.0` | --- ### Release Notes <details> <summary>cert-manager/cert-manager (cert-manager)</summary> ### [`v1.19.0`](https://github.com/cert-manager/cert-manager/releases/tag/v1.19.0) [Compare Source](cert-manager/cert-manager@v1.18.2...v1.19.0) cert-manager is the easiest way to automatically manage certificates in Kubernetes and OpenShift clusters. This release focuses on expanding platform compatibility, improving deployment flexibility, enhancing observability, and addressing key reliability issues. > 📖 Read the full release notes at cert-manager.io: <https://cert-manager.io/docs/releases/release-notes/release-notes-1.19> Changes since `v1.18.0`: #### Feature - Add IPv6 rules to the default network policy ([#7726](cert-manager/cert-manager#7726), [@jcpunk](https://github.com/jcpunk)) - Add `global.nodeSelector` to helm chart to allow for a single `nodeSelector` to be set across all services. ([#7818](cert-manager/cert-manager#7818), [@StingRayZA](https://github.com/StingRayZA)) - Add a feature gate to default to Ingress `pathType` `Exact` in ACME HTTP01 Ingress challenge solvers. ([#7795](cert-manager/cert-manager#7795), [@sspreitzer](https://github.com/sspreitzer)) - Add generated `applyconfigurations` allowing clients to make type-safe server-side apply requests for cert-manager resources. ([#7866](cert-manager/cert-manager#7866), [@erikgb](https://github.com/erikgb)) - Added API defaults to issuer references group (cert-manager.io) and kind (Issuer). ([#7414](cert-manager/cert-manager#7414), [@erikgb](https://github.com/erikgb)) - Added `certmanager_certificate_challenge_status` Prometheus metric. ([#7736](cert-manager/cert-manager#7736), [@hjoshi123](https://github.com/hjoshi123)) - Added `protocol` field for `rfc2136` DNS01 provider ([#7881](cert-manager/cert-manager#7881), [@hjoshi123](https://github.com/hjoshi123)) - Added experimental field `hostUsers` flag to all pods. Not set by default. ([#7973](cert-manager/cert-manager#7973), [@hjoshi123](https://github.com/hjoshi123)) - Support configurable resource requests and limits for ACME HTTP01 solver pods through ClusterIssuer and Issuer specifications, allowing granular resource management that overrides global `--acme-http01-solver-resource-*` settings. ([#7972](cert-manager/cert-manager#7972), [@lunarwhite](https://github.com/lunarwhite)) - The `CAInjectorMerging` feature has been promoted to BETA and is now enabled by default ([#8017](cert-manager/cert-manager#8017), [@ThatsMrTalbot](https://github.com/ThatsMrTalbot)) - The controller, webhook and ca-injector now log their version and git commit on startup for easier debugging and support. ([#8072](cert-manager/cert-manager#8072), [@prasad89](https://github.com/prasad89)) - Updated `certificate` metrics to the collector approach. ([#7856](cert-manager/cert-manager#7856), [@hjoshi123](https://github.com/hjoshi123)) #### Bug or Regression - ACME: Increased challenge authorization timeout to 2 minutes to fix `error waiting for authorization` ([#7796](cert-manager/cert-manager#7796), [@hjoshi123](https://github.com/hjoshi123)) - BUGFIX: permitted URI domains were incorrectly used to set the excluded URI domains in the CSR's name constraints ([#7816](cert-manager/cert-manager#7816), [@kinolaev](https://github.com/kinolaev)) - Enforced ACME HTTP-01 solver validation to properly reject configurations when multiple ingress options (`class`, `ingressClassName`, `name`) are specified simultaneously ([#8021](cert-manager/cert-manager#8021), [@lunarwhite](https://github.com/lunarwhite)) - Increase maximum sizes of PEM certificates and chains which can be parsed in cert-manager, to handle leaf certificates with large numbers of DNS names or other identities ([#7961](cert-manager/cert-manager#7961), [@SgtCoDFish](https://github.com/SgtCoDFish)) - Reverted adding the `global.rbac.disableHTTPChallengesRole` Helm option. ([#7836](cert-manager/cert-manager#7836), [@inteon](https://github.com/inteon)) - This change removes the `path` label of core ACME client metrics and will require users to update their monitoring dashboards and alerting rules if using those metrics. ([#8109](cert-manager/cert-manager#8109), [@mladen-rusev-cyberark](https://github.com/mladen-rusev-cyberark)) - Use the latest version of `ingress-nginx` in E2E tests to ensure compatibility ([#7792](cert-manager/cert-manager#7792), [@wallrj](https://github.com/wallrj)) #### Other (Cleanup or Flake) - Helm: Fix naming template of `tokenrequest` RoleBinding resource to improve consistency ([#7761](cert-manager/cert-manager#7761), [@lunarwhite](https://github.com/lunarwhite)) - Improve error messages when certificates, CRLs or private keys fail admission due to malformed or missing PEM data ([#7928](cert-manager/cert-manager#7928), [@SgtCoDFish](https://github.com/SgtCoDFish)) - Major upgrade of Akamai SDK. NOTE: The new version has not been fully tested end-to-end due to the lack of cloud infrastructure. ([#8003](cert-manager/cert-manager#8003), [@hjoshi123](https://github.com/hjoshi123)) - Update kind images to include the Kubernetes 1.33 node image ([#7786](cert-manager/cert-manager#7786), [@wallrj](https://github.com/wallrj)) - Use `maps.Copy` for cleaner map handling ([#8092](cert-manager/cert-manager#8092), [@quantpoet](https://github.com/quantpoet)) - Vault: Migrate Vault E2E add-on tests from deprecated `vault-client-go` to the new `vault/api` client. ([#8059](cert-manager/cert-manager#8059), [@armagankaratosun](https://github.com/armagankaratosun)) </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR is behind base branch, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).  Reviewed-on: https://gitea.alexlebens.dev/alexlebens/infrastructure/pulls/1711 Co-authored-by: Renovate Bot <renovate-bot@alexlebens.net> Co-committed-by: Renovate Bot <renovate-bot@alexlebens.net>

wallrj-cyberark · 2025-10-15T16:09:29Z

@armagankaratosun We have released this. Please test and feedback: https://github.com/cert-manager/cert-manager/releases/tag/v1.19.1

cert-manager-prow bot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Sep 9, 2025

fix: migrating from github.com/hashicorp/vault-client-go to github.co…

2c6b454

…m/hashicorp/vault/api Signed-off-by: Armagan Karatosun <armagankaratosun@gmail.com>

armagankaratosun force-pushed the migrate-vault-api-1.20 branch from b34e1bd to 2c6b454 Compare September 9, 2025 19:36

cert-manager-prow bot added dco-signoff: yes Indicates that all commits in the pull request have the valid DCO sign-off message. and removed dco-signoff: no Indicates that at least one commit in this pull request is missing the DCO sign-off message. labels Sep 9, 2025

fixing the boilerplate issue

32513d2

Signed-off-by: Armagan Karatosun <armagankaratosun@gmail.com>

armagankaratosun force-pushed the migrate-vault-api-1.20 branch from 09c5e40 to 32513d2 Compare September 9, 2025 19:57

cert-manager-prow bot added dco-signoff: yes Indicates that all commits in the pull request have the valid DCO sign-off message. and removed dco-signoff: no Indicates that at least one commit in this pull request is missing the DCO sign-off message. labels Sep 9, 2025

erikgb requested review from Copilot and erikgb September 9, 2025 20:12

Copilot AI reviewed Sep 9, 2025

View reviewed changes

test/e2e/framework/addon/vault/setup.go Outdated Show resolved Hide resolved

test/e2e/framework/addon/vault/setup.go Show resolved Hide resolved

erikgb reviewed Sep 9, 2025

View reviewed changes

test/e2e/framework/addon/vault/setup.go Outdated Show resolved Hide resolved

armagankaratosun added 3 commits September 9, 2025 22:26

using context aware methods

404551f

Signed-off-by: Armagan Karatosun <armagankaratosun@gmail.com>

switch from Logical() to Sys()

2abaead

Signed-off-by: Armagan Karatosun <armagankaratosun@gmail.com>

fixing linting issues

e0a86d6

Signed-off-by: Armagan Karatosun <armagankaratosun@gmail.com>

erikgb requested a review from Copilot September 10, 2025 19:22

Copilot AI reviewed Sep 10, 2025

View reviewed changes

test/e2e/framework/addon/vault/setup.go Outdated Show resolved Hide resolved

erikgb reviewed Sep 10, 2025

View reviewed changes

test/e2e/framework/addon/vault/setup.go Outdated Show resolved Hide resolved

cert-manager-prow bot added the tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. label Sep 10, 2025

cert-manager-prow bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 10, 2025

removed old TODO comments

93c9064

Signed-off-by: Armagan Karatosun <armagankaratosun@gmail.com>

erikgb reviewed Sep 11, 2025

View reviewed changes

cert-manager-prow bot added the lgtm Indicates that a PR is ready to be merged. label Sep 11, 2025

cert-manager-prow bot merged commit bdd12d6 into cert-manager:master Sep 11, 2025
7 checks passed

armagankaratosun deleted the migrate-vault-api-1.20 branch September 11, 2025 12:43

Conversation

armagankaratosun commented Sep 9, 2025 • edited by wallrj-cyberark Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Motivation

Kind

Release Note

Uh oh!

cert-manager-prow bot commented Sep 9, 2025

Uh oh!

erikgb commented Sep 9, 2025

Uh oh!

armagankaratosun commented Sep 9, 2025

Uh oh!

erikgb commented Sep 9, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Uh oh!

erikgb left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cert-manager-prow bot commented Sep 10, 2025

Uh oh!

erikgb left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

armagankaratosun commented Sep 11, 2025

Uh oh!

wallrj-cyberark commented Oct 15, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

armagankaratosun commented Sep 9, 2025 •

edited by wallrj-cyberark

Loading