Open
Conversation
piotrek-janus
requested review from
bwereszczak,
Copilot,
ikawalec and
jdabrowski
and removed request for
a team,
bwereszczak,
ikawalec and
jdabrowski
There was a problem hiding this comment.
Pull Request Overview
This PR implements secret management via dedicated secret system APIs while transitioning configuration types from the legacy Rfc7396PatchOperation to the new api.Patch interface. Key changes include type updates across storage, data, and client layers; the addition of a new SecretsClient and error logging helper; and dependency and build tool upgrades.
Reviewed Changes
Copilot reviewed 35 out of 35 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| internal/cac/storage/multi.go | Updates Write/Read to use api.Patch and a custom merge mechanism |
| internal/cac/storage/dry.go | Similar type conversion from models.Rfc7396PatchOperation to api.Patch |
| internal/cac/logging/logging.go | Adds trace logging support and minor logging improvements |
| internal/cac/diff/diff.go | Converts diff functions to use api.Patch and cleans patches accordingly |
| internal/cac/data/* | Adapts validators/tests to work with the new api.Patch interface |
| internal/cac/client/* | Updates tenant/client read/write functions to support secrets and new type |
| internal/cac/api/model.go | Introduces the Patch interface and merging logic for server/tenant patches |
| go.mod, Dockerfile, cmd/* | Dependency updates and command adjustments per type changes |
Comments suppressed due to low confidence (1)
internal/cac/client/errors.go:18
- Consider renaming the custom error interface 'errr' to a more descriptive name (e.g., 'APIErrorInterface') to improve clarity.
else if e, ok := err.(errr); ok{
| func (tp *ServerPatch) GetExtensions() any { | ||
| return tp.Ext | ||
| } | ||
| func (sp *ServerPatch) Merge(other Patch) error { |
There was a problem hiding this comment.
Before merging extensions with mergo.Merge(sp.Ext, other.GetExtensions(), ...), ensure that sp.Ext is not nil to avoid a potential nil pointer dereference.
mbilski
approved these changes
Jun 17, 2025
ikawalec
approved these changes
Jun 24, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Jira task - https://secureauth.atlassian.net/browse/AUT-11295
Release Notes Description (public)
Manage secrets using dedicated secret system APIs
Implementation details (internal)
Information for QA
Additional QA Procedures (Optional)
Screenshots (if appropriate):