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

Enable resolving a subset of the values in a value set #3301

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

kichristensen
Copy link
Contributor

What does this change

Allows us the only resolve a subset of the values, which can be used to increase security and reducing number of API calls.
Currently all existing calls still resolves the whole set, this can be improved in a future PR.

What issue does it fix

Closes #2609

Notes for the reviewer

Put any questions or notes for the reviewer here.

Checklist

  • Did you write tests?
  • Did you write documentation?
  • Did you change porter.yaml or a storage document record? Update the corresponding schema file.
  • If this is your first pull request, please add your name to the bottom of our Contributors list. Thank you for making Porter better! 🙇‍♀️

@kichristensen kichristensen force-pushed the subsetResolve branch 3 times, most recently from f341e7a to a3217c3 Compare December 31, 2024 00:25
…credentials

- Update ResolveAll to accept a list of required credential keys
- Add Keys() method to CredentialSet to get list of credential names
- Filter credentials during resolution to only process required ones
- Update credential provider interface and implementations

Signed-off-by: Kim Christensen <[email protected]>
- Add Keys() method to ParameterSet to get parameter names
- Update ResolveAll signature to accept list of required parameter keys
- Modify parameter resolution to skip parameters not in requested keys
- Update all parameter resolution calls to pass required keys

Signed-off-by: Kim Christensen <[email protected]>
…nd lookup

- Add GetCredential helper method to CredentialSet for safe credential lookup
- Remove slices dependency in favor of direct credential lookup
- Add tracing spans for better observability
- Improve error messages with proper error wrapping
- Change resolution logic to iterate through requested keys instead of all credentials

Signed-off-by: Kim Christensen <[email protected]>
…place updates

- Change GetCredential signature to return *secrets.SourceMap instead of value
- Update loadCredentials to use Keys() iterator for credential lookup
- Add explicit error handling for missing credentials
- Fix credential value updates to modify original values via pointers

Signed-off-by: Kim Christensen <[email protected]>
- Update ResolveAll to return ErrorOrNil() instead of raw error group
- Ensures consistent error handling when no errors occurred during resolution

Signed-off-by: Kim Christensen <[email protected]>
Signed-off-by: Kim Christensen <[email protected]>
@kichristensen kichristensen marked this pull request as ready for review January 2, 2025 09:55
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.

Resolve a subset of values in a value set
1 participant