TT-17702: Add Lister and Setter logic to Vault and Consul

[vladzabolotnyi]

Description

Ticket: https://tyktech.atlassian.net/browse/TT-17702

Related Issue

Motivation and Context

Test Coverage For This Change

Screenshots (if appropriate)

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Refactoring or add test (improvements in base code or adds test coverage to functionality)
• Documentation updates or improvements.

Checklist

• I have reviewed the guidelines for contributing to this repository.
• Make sure you are requesting to pull a topic/feature/bugfix branch (right side). If PRing from your fork, don't come from your master!
• Make sure you are making a pull request against our master branch (left side). Also, it would be best if you started your change off our latest master.
• My change requires a change to the documentation.
  • I have manually updated the README(s)/documentation accordingly.
  • If you've changed APIs, describe what needs to be updated in the documentation.
• I have updated the documentation accordingly.
• Modules and vendor dependencies have been updated; run go mod tidy && go mod vendor
• When updating library version must provide reason/explanation for this update.
• I have added tests to cover my changes.
• All new and existing tests passed.
• Check your code additions will not fail linting checks:
  • gofmt -s -w .
  • go vet ./...

[github-actions]

CLA Assistant Lite bot:
Thank you for your submission, we really appreciate it. Like many open-source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution. You can sign the CLA by just posting a Pull Request Comment same as the below format.

---

I have read the CLA Document and I hereby sign the CLA

---

_{You can retrigger this bot by commenting recheck in this Pull Request}

[probelabs]

This pull request introduces significant new capabilities to the kv (key-value) storage abstraction by adding write and list functionalities. Previously a read-only system for secret consumption, this PR adds Setter and Lister interfaces, enabling applications to write values back to secret stores like Vault and Consul, and to list key-value pairs under a given prefix.

A key addition is the resolver.ParseReference function, which allows parsing a kv:// URI into its components (store, path, field) without resolving the secret's value. This is a critical building block for implementing write-back or secret rotation logic, as it enables routing write operations to the correct backend.

The Lister interface has been modified to return a map of key-value pairs (map[string]string) instead of just a slice of keys ([]string), which is a breaking change but offers a more efficient way to retrieve secrets in bulk. Implementations for the new Setter interface are provided for both Vault and Consul, with careful handling of API specifics like Vault's KV v1 vs. v2 data formats. The PR is accompanied by extensive unit tests covering the new functionality.

Files Changed Analysis

• Total Changes: 10 files changed, with 831 additions and 24 deletions.
• Core Interface Changes (kv/provider.go): Defines the new Setter interface and modifies the Lister interface. It also centralizes the DefaultOperationTimeout.
• Provider Implementations (kv/providers/consul/consul.go, kv/providers/vault/vault.go): Implement the Set method for both providers and the List method for Consul. These include provider-specific logic for API interaction.
• New Public API (kv/resolver/reference.go): A new file introducing ParseReference, which exposes internal parsing logic to allow consumers to deconstruct kv:// URIs.
• Testing (*_test.go): The bulk of the new lines are dedicated to comprehensive unit tests for the Set and List operations, covering various scenarios including success cases, error handling, context cancellation, and authentication.

Architecture & Impact Assessment

• What this PR accomplishes: It evolves the kv package from a read-only secret fetching library into a read-write secret management library. This is a foundational change that enables higher-level features like automated secret rotation, dynamic configuration updates, and operational tooling.

• Key technical changes introduced:

  • Setter Interface: A new Set(ctx, key, value) method is added to the provider interface, allowing values to be written to the backend store.
  • Lister Interface Modification: The List method's signature is changed to return map[string]string, providing both keys and their values. This is a breaking change for existing Lister implementations but improves performance by avoiding N+1 lookups.
  • ParseReference Function: A new public function resolver.ParseReference is introduced. It allows services to deconstruct a kv:// URI without fetching its value, which is essential for routing write operations.

• Affected system components: Any service utilizing the kv package is affected. While existing read-only workflows remain unchanged, the new write capabilities open up new possibilities for dynamic interaction with secret stores. This change primarily impacts the kv abstraction layer and its direct consumers.

Write-Back Architectural Flow

The new components enable a clear architectural pattern for writing secrets back to a store.

graph TD
    subgraph sg1 [Application Layer]
        A[Service Logic] --|1. Set(kv://vault/secret/app#key, 'new-value')|--> B{Secret Management}
    end

    subgraph sg2 [KV Abstraction Layer]
        B --|2. Parse URI|--> C[resolver.ParseReference]
        C --|"3. Returns 'store: vault', 'path: secret/app'"|--> B
        B --|"4. Get provider for 'vault'"|--> D[Provider Registry]
        D --|5. Returns Vault Provider|--> B
        B --|6. Get Setter interface|--> E[kv.AsSetter]
        E --|7. Returns Setter|--> B
        B --|"8. Calls Set('secret/app', {'key':'new-value'})"|--> F[Vault Provider's Set Method]
    end

    subgraph sg3 [Backend]
        F --|9. HTTP PUT to Vault API|--> G[(HashiCorp Vault)]
    end

Loading

Scope Discovery & Context Expansion

• The introduction of Setter and ParseReference strongly suggests that this PR is a prerequisite for a future feature involving secret rotation or dynamic configuration management. A higher-level service can now be built to:
  1. Identify a secret to update via its kv:// reference.
  2. Use ParseReference to determine the target store and path.
  3. Retrieve the appropriate kv.Provider.
  4. Use AsSetter to acquire the write interface and call Set to update the value in the backend.
• The change to the Lister interface enhances operational tooling. It allows for efficient auditing, debugging, and dynamic loading of entire configuration sets from a path prefix, which is a common pattern in microservices.

Metadata

• Review Effort: 4 / 5
• Primary Label: feature

---

Powered by Visor from Probelabs

Last updated: 2026-07-24T06:56:32.156Z | Triggered by: pr_updated | Commit: 03dec08

💡 TIP: You can chat with Visor using /visor ask <your question>

[probelabs]

Security Issues (2)

Severity	Location	Issue
🟡 Warning	kv/providers/consul/consul.go:199-202	The Set operation uses a hardcoded default timeout (`kv.DefaultOperationTimeout`) instead of a configurable timeout. This can lead to inflexibility in environments with different network latencies or operational requirements, and may result in premature timeouts for slow Consul operations. 💡 Suggestion The `consulProvider` should have a `timeout` field, similar to the `vaultProvider`, which is initialized from the configuration. The `Set` method should then use this configurable timeout. This allows operators to tune performance and reliability according to their specific environment.
🟡 Warning	kv/providers/consul/consul.go:221-224	The List operation uses a hardcoded default timeout (`kv.DefaultOperationTimeout`) instead of a configurable timeout. Listing many keys could be a slow operation, and a hardcoded timeout may not be sufficient in all use cases, potentially causing requests to fail unnecessarily. 💡 Suggestion The `consulProvider` should have a `timeout` field, similar to the `vaultProvider`, which is initialized from the configuration. The `List` method should then use this configurable timeout, providing flexibility for different operational scenarios.

Architecture Issues (1)

Severity	Location	Issue
🟡 Warning	kv/providers/consul/consul.go:193-215	The Consul provider uses a hardcoded default timeout for `Set` and `List` operations, which is inconsistent with the Vault provider that supports a configurable timeout. This limits the flexibility of the Consul provider and creates a divergence in behavior and configuration options between providers. 💡 Suggestion To ensure consistent configuration and behavior across all providers, the Consul provider should be updated to support a configurable timeout. This involves adding a `Timeout` field to the `consul.Config` struct, storing it in the `consulProvider`, and using the `kv.EffectiveTimeout` helper function when creating contexts for `Set` and `List` operations, mirroring the implementation in the Vault provider.

Security Issues (2)

Severity	Location	Issue
🟡 Warning	kv/providers/consul/consul.go:199-202	The Set operation uses a hardcoded default timeout (`kv.DefaultOperationTimeout`) instead of a configurable timeout. This can lead to inflexibility in environments with different network latencies or operational requirements, and may result in premature timeouts for slow Consul operations. 💡 Suggestion The `consulProvider` should have a `timeout` field, similar to the `vaultProvider`, which is initialized from the configuration. The `Set` method should then use this configurable timeout. This allows operators to tune performance and reliability according to their specific environment.
🟡 Warning	kv/providers/consul/consul.go:221-224	The List operation uses a hardcoded default timeout (`kv.DefaultOperationTimeout`) instead of a configurable timeout. Listing many keys could be a slow operation, and a hardcoded timeout may not be sufficient in all use cases, potentially causing requests to fail unnecessarily. 💡 Suggestion The `consulProvider` should have a `timeout` field, similar to the `vaultProvider`, which is initialized from the configuration. The `List` method should then use this configurable timeout, providing flexibility for different operational scenarios.

\n\n ### Architecture Issues (1)

Severity	Location	Issue
🟡 Warning	kv/providers/consul/consul.go:193-215	The Consul provider uses a hardcoded default timeout for `Set` and `List` operations, which is inconsistent with the Vault provider that supports a configurable timeout. This limits the flexibility of the Consul provider and creates a divergence in behavior and configuration options between providers. 💡 Suggestion To ensure consistent configuration and behavior across all providers, the Consul provider should be updated to support a configurable timeout. This involves adding a `Timeout` field to the `consul.Config` struct, storing it in the `consulProvider`, and using the `kv.EffectiveTimeout` helper function when creating contexts for `Set` and `List` operations, mirroring the implementation in the Vault provider.

\n\n ### Performance Issues (2)

Severity	Location	Issue
🟡 Warning	kv/providers/consul/consul.go:193-214	The `Set` and `List` methods in the Consul provider use a hardcoded default timeout (`kv.DefaultOperationTimeout`). This is inconsistent with the Vault provider, which uses a configurable timeout with a default fallback. This lack of configurability can be problematic in environments with different network latencies or performance requirements, as it prevents tuning the timeout for Consul-specific operations. 💡 Suggestion To provide consistent behavior and better control over operation deadlines across providers, add a `Timeout` field to the `consul.Config` struct. Then, use `kv.EffectiveTimeout` to apply the configured timeout, falling back to the default if not set. This would align the Consul provider's behavior with the Vault provider's.
🟡 Warning	kv/providers/consul/consul.go:217	The `List` method for the Consul provider fetches all key-value pairs under a given prefix and loads them into memory at once to construct the returned map. If a prefix matches a very large number of keys, this can lead to excessive memory allocation, potentially causing performance degradation or Out-of-Memory (OOM) errors. While the check for an empty prefix is a good safeguard, a broad, non-empty prefix could still match a huge dataset, posing a resource exhaustion risk. 💡 Suggestion Given the `Lister` interface contract returns a `map`, in-memory collection is required. Add documentation to the `List` method to explicitly warn users about the potential memory impact of using broad prefixes. For a more robust solution, consider logging a warning if the number of returned items exceeds a high threshold (e.g., 10,000) to help diagnose potential performance issues in production.

Quality Issues (2)

Severity	Location	Issue
🟠 Error	kv/providers/consul/consul.go:227-245	The `List` method in the Consul provider does not apply a timeout to its context, unlike the `Set` method which uses `kv.DefaultOperationTimeout`. A long-running list operation against the Consul API could block indefinitely, leading to resource leaks or unresponsive behavior. All network calls should have a timeout. 💡 Suggestion Apply a timeout to the context for the `List` operation to ensure it does not hang. This would make it consistent with the `Set` method's implementation. 🔧 Suggested Fix func (cp *consulProvider) List(ctx context.Context, prefix string) (map[string]string, error) { if prefix == "" { return nil, errors.New("consul: list requires a non-empty prefix") } ctx, cancel := context.WithTimeout(ctx, kv.DefaultOperationTimeout) defer cancel() pairs, _, err := cp.kvClient.List(prefix, (&consulsdk.QueryOptions{}).WithContext(ctx)) if err != nil { return nil, &kv.StoreUnavailableError{KeyPath: prefix, Err: err} } out := make(map[string]string, len(pairs)) for _, p := range pairs { if strings.HasSuffix(p.Key, "/") { continue } out[p.Key] = string(p.Value) } return out, nil }
🟡 Warning	kv/provider.go:111	The `Lister` interface signature has been changed from returning `[]string` to `map[string]string`. This is a breaking change for any external consumers or implementers of this interface. Breaking changes should be clearly documented in the pull request description and release notes. 💡 Suggestion Acknowledge the breaking change in the PR description and consider if this change warrants a major version bump if this is a public library.

---

Powered by Visor from Probelabs

Last updated: 2026-07-24T06:56:14.098Z | Triggered by: pr_updated | Commit: 03dec08

💡 TIP: You can chat with Visor using /visor ask <your question>

[github-actions]

🚨 Jira Linter Failed

Commit: 03dec08
Failed at: 2026-07-24 06:54:19 UTC

The Jira linter failed to validate your PR. Please check the error details below:

🔍 Click to view error details

configuration error: Jira user email is required for API authentication

Next Steps

• Ensure your branch name contains a valid Jira ticket ID (e.g., ABC-123)
• Verify your PR title matches the branch's Jira ticket ID
• Check that the Jira ticket exists and is accessible

---

This comment will be automatically deleted once the linter passes.

[sonarqubecloud]

Quality Gate passed

Issues
7 New issues
0 Accepted issues

Measures
0 Security Hotspots
93.3% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud