updated CSR enrollment to use Keyfactor Client SDK #44
Conversation
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
Contributor
joevanwanzeeleKF
commented
May 22, 2025
joevanwanzeeleKF
commented
- Updated the Hashicorp SDK libraries
- Incorporated the Keyfactor GO SDK for authentication and interaction with the Command API
…rsion and multiplexing support
There was a problem hiding this comment.
Pull Request Overview
This pull request updates the CSR enrollment functionality to use the Keyfactor GO SDK for authentication and interaction with the Command API and updates the Hashicorp SDK libraries. Key changes include removal of legacy revoked-path code, refactoring the certificate issuance and revocation endpoints to use the new Keyfactor API client, and updating documentation and dependency versions.
Reviewed Changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| readme_source.md | Added clarifying notes regarding certificate and chain retrieval. |
| path_revoke.go | Removed obsolete commented code related to certificate revocation. |
| path_certs.go | Updated API usage, modified callbacks, and replaced custom JSON validation with the standard json.Valid check. |
| installation.txt | Updated connection testing instructions to note CA certificate requirements in the Command database. |
| go.mod | Upgraded Go version and dependency versions for the Keyfactor and Hashicorp SDKs. |
| fields.go | Updated field description to reflect the use of dns_sans for common name alternatives. |
| cmd/keyfactor/main.go | Changed the plugin serving call to use ServeMultiplex and updated error handling accordingly. |
| client.go | Refactored the client initialisation to use the new Keyfactor API client and updated authentication configuration. |
| backend.go | Adjusted client caching and versioning to work with the new Keyfactor API client. |
| README.md | Mirrored readme_source.md updates for CA certificate and chain notes. |
| CHANGELOG.md | Documented the version bump and key changes in the changelog. |
Comments suppressed due to low confidence (1)
path_revoke.go:1
- The file contains only commented-out legacy code; consider removing it entirely if it is no longer needed to improve code maintainability.
/* ... entire file commented out ... */
Comment on lines
+532
to
+539
| b.Logger().Debug(fmt.Sprintf("retreiving the keyfactor ID for cert stored at path: %s", "kfId/"+serial)) | ||
|
|
||
| kfId, err := req.Storage.Get(ctx, "kfId/"+serial) //retrieve the keyfactor certificate ID, keyed by sn here | ||
| if err != nil { | ||
| b.Logger().Error("Unable to retreive Keyfactor certificate ID for cert with serial: "+serial, err) | ||
| b.Logger().Error("unable to retreive Keyfactor certificate ID for cert with serial: "+serial, err) | ||
| return nil, err | ||
| } | ||
|
|
||
| var keyfactorId int | ||
| b.Logger().Debug(fmt.Sprintf("retreived the logical storage entry, decoding...")) |
There was a problem hiding this comment.
Consider correcting the spelling of 'retreiving' to 'retrieving' for clarity.
Copilot uses AI. Check for mistakes.
Comment on lines
+532
to
+539
| b.Logger().Debug(fmt.Sprintf("retreiving the keyfactor ID for cert stored at path: %s", "kfId/"+serial)) | ||
|
|
||
| kfId, err := req.Storage.Get(ctx, "kfId/"+serial) //retrieve the keyfactor certificate ID, keyed by sn here | ||
| if err != nil { | ||
| b.Logger().Error("Unable to retreive Keyfactor certificate ID for cert with serial: "+serial, err) | ||
| b.Logger().Error("unable to retreive Keyfactor certificate ID for cert with serial: "+serial, err) | ||
| return nil, err | ||
| } | ||
|
|
||
| var keyfactorId int | ||
| b.Logger().Debug(fmt.Sprintf("retreived the logical storage entry, decoding...")) |
There was a problem hiding this comment.
Consider correcting the spelling of 'retreive' to 'retrieve' for clarity in error messages.
Copilot uses AI. Check for mistakes.
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.