Skip to content

feat(agent): split legacy username/password login into its own file - #819

Open
roeezis wants to merge 2 commits into
jetstack:masterfrom
roeezis:split/02-identity-refactor
Open

feat(agent): split legacy username/password login into its own file#819
roeezis wants to merge 2 commits into
jetstack:masterfrom
roeezis:split/02-identity-refactor

Conversation

@roeezis

@roeezis roeezis commented Aug 23, 2026

Copy link
Copy Markdown

Summary

Part 2 of the SMS/Conjur JWT authentication series (split out of #817 for reviewability). Stacked on #818 — includes #818's commit until that merges, so the diff here will shrink to just this PR's own change once #818 lands.

identity.go mixed shared client/token-cache plumbing with the CyberArk Identity username/password (UP) login flow. Moves the UP-specific code into username_password.go so the shared plumbing stays easy to find once a second login mechanism (Conjur JWT) is added alongside it.

No behavior change — pure extraction, plus unexporting ActionAnswer and exporting the mock's success credentials for other packages' tests.

Test plan

  • go build ./...
  • go test ./internal/cyberark/identity/...

@mladen-rusev-cyberark mladen-rusev-cyberark left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Verified the "no behaviour change" claim mechanically — sorting all lines of the pre-image identity.go against the sorted union of the post-image identity.go + username_password.go yields only comment/import differences, no code changes. The split is a good idea and lands cleanly.

One correction needed (inline), plus: the Python SDK reference link dropped from the Client doc comment is the only pointer in the repo to the reference implementation this client mirrors — worth moving to username_password.go rather than deleting.

Reviewed as this PR's own commit (2483110) rather than the cumulative diff against master.

Comment thread internal/cyberark/identity/mock.go Outdated
@roeezis
roeezis force-pushed the split/02-identity-refactor branch from 2483110 to 6e7e2fc Compare August 24, 2026 11:42
@roeezis

roeezis commented Aug 24, 2026

Copy link
Copy Markdown
Author

Fixed — deleted the duplicate actionAnswer const, mock.go now uses the single exported ActionAnswer. Also corrected the commit message.

rzisholz added 2 commits August 24, 2026 14:43
Introduces a small, isolated interface for reading a JWT from a file
path — the first piece of the upcoming Conjur JWT authentication path,
split out on its own since nothing else in this PR depends on it yet.
identity.go mixed the shared client/token-cache plumbing with the
CyberArk Identity username/password (UP) login flow. Move the UP-specific
code into username_password.go so the shared plumbing stays easy to find
once a second login mechanism (Conjur JWT) is added alongside it.

No behavior change — pure extraction, plus exporting the mock's success
credentials for other packages' tests.
@roeezis
roeezis force-pushed the split/02-identity-refactor branch from 6e7e2fc to 33e6b50 Compare August 24, 2026 11:45
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.

2 participants