Skip to content

Conversation

@piotrek-janus
Copy link
Contributor

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

  • Is QA testing required?
  • Does PR contain unit tests?
  • Should QA create E2E tests for the change?

Additional QA Procedures (Optional)

Screenshots (if appropriate):

@piotrek-janus piotrek-janus requested a review from a team as a code owner June 17, 2025 07:37
@piotrek-janus piotrek-janus requested review from bwereszczak, Copilot, ikawalec and jdabrowski and removed request for a team, bwereszczak, ikawalec and jdabrowski June 17, 2025 07:37
Copilot

This comment was marked as outdated.

@piotrek-janus piotrek-janus requested a review from Copilot June 17, 2025 08:12
Copilot

This comment was marked as outdated.

@piotrek-janus piotrek-janus requested a review from Copilot June 17, 2025 09:19
Copilot

This comment was marked as outdated.

@piotrek-janus piotrek-janus requested a review from Copilot June 17, 2025 11:07
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 {
Copy link

Copilot AI Jun 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before merging extensions with mergo.Merge(sp.Ext, other.GetExtensions(), ...), ensure that sp.Ext is not nil to avoid a potential nil pointer dereference.

Copilot uses AI. Check for mistakes.
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.

3 participants