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.

Sign up for GitHub

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

Jump to bottom

UX: implement rotating keys api in ux-backend server #2469

Merged
merged 2 commits into from
Mar 7, 2024
Merged

UX: implement rotating keys api in ux-backend server #2469

merged 2 commits into from
Mar 7, 2024

Conversation

leelavg
Copy link
Contributor

@leelavg leelavg commented Feb 20, 2024

Implement rotating keys by deleting the secret containing onboarding public key. Creation of new private & public key pair when public key is deleted is already implemented in #2385.

The expectation is this new api endpoint be called via CLI or UI and get the existing private & public key pairs rotated if both the keys are being managed by us.

Part of [RHSTOR-5489]

@leelavg
Copy link
Contributor Author

leelavg commented Feb 20, 2024

/hold

testing in progress.

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 20, 2024
@leelavg
Copy link
Contributor Author

leelavg commented Feb 20, 2024

addressed golint.

@leelavg leelavg requested a review from nb-ohad February 20, 2024 12:41
@leelavg
Copy link
Contributor Author

leelavg commented Feb 20, 2024

/retest-required

nb-ohad
nb-ohad requested changes Feb 20, 2024
View reviewed changes
services/ux-backend/handlers/rotatekeys/handler.go Outdated Show resolved Hide resolved
services/ux-backend/handlers/rotatekeys/handler.go Outdated Show resolved Hide resolved
services/ux-backend/handlers/rotatekeys/handler.go Outdated Show resolved Hide resolved
services/ux-backend/main.go Outdated Show resolved Hide resolved
services/ux-backend/main.go Outdated Show resolved Hide resolved
@leelavg leelavg force-pushed the 5489-rotate-key branch 2 times, most recently from e74f3cb to 6c7751e Compare February 28, 2024 05:20
@leelavg
Copy link
Contributor Author

leelavg commented Feb 28, 2024

/unhold

tested, working as expected

# ===== Existing
# ko get secret onboarding-private-key -ojsonpath='{.metadata.creationTimestamp}'
2024-02-28T05:20:07Z

# ko get secret onboarding-ticket-key -ojsonpath='{.metadata.creationTimestamp}'
2024-02-28T05:20:07Z

# ===== Invoked rotation
# ko exec -it deploy/ocs-operator -- bash -c 'curl -H "Authorization: Bearer $(cat /var/run/secrets/kubernetes.io/serviceaccount/token)"  https://ux-backend-proxy:8888/rotate-keys -k -X POST -s'
Successfully rotated keys

# ko logs ux-backend-server-5b7478645c-gbwpn -f
I0228 05:53:09.151963       1 handler.go:34] POST method on /rotate-keys endpoint is invoked
I0228 05:53:09.166107       1 handler.go:65] onboarding keys are rotated successfully

# ===== Rotated
# ko get secret onboarding-private-key -ojsonpath='{.metadata.creationTimestamp}'
2024-02-28T05:53:26Z

# ko get secret onboarding-ticket-key -ojsonpath='{.metadata.creationTimestamp}'
2024-02-28T05:53:26Z

@openshift-ci openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 28, 2024
@leelavg leelavg requested a review from nb-ohad February 29, 2024 03:16
nb-ohad
nb-ohad requested changes Feb 29, 2024
View reviewed changes
services/ux-backend/handlers/constants.go Outdated Show resolved Hide resolved
services/ux-backend/handlers/rotatekeys/handler.go Outdated Show resolved Hide resolved
services/ux-backend/handlers/rotatekeys/handler.go Outdated Show resolved Hide resolved
@leelavg leelavg force-pushed the 5489-rotate-key branch 2 times, most recently from 8054c6d to 1ca1c8c Compare March 1, 2024 13:27
jarrpa
jarrpa reviewed Mar 2, 2024
View reviewed changes
Copy link
Member

@jarrpa jarrpa left a comment

Choose a reason for hiding this comment

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

Code looks good. If you get the chance, it would be good to update the commit message of ux: implement rotating keys api with a portion of the PR message describing how and when this API call is intended to be used.

nb-ohad
nb-ohad requested changes Mar 7, 2024
View reviewed changes
services/ux-backend/handlers/constants.go Outdated Show resolved Hide resolved
services/ux-backend/handlers/utils.go Outdated Show resolved Hide resolved
services/ux-backend/handlers/utils.go Outdated Show resolved Hide resolved
@leelavg leelavg force-pushed the 5489-rotate-key branch 2 times, most recently from 3f65d0b to 1014594 Compare March 7, 2024 10:42
@leelavg
Copy link
Contributor Author

leelavg commented Mar 7, 2024

it would be good to update the commit message

  • done.

leelavg added 2 commits March 7, 2024 16:14
@leelavg
ux: implement rotating keys api
015e679 
Implement rotating keys by deleting the secret containing onboarding
public key.

The expectation is this new api endpoint be called via CLI or UI and
get the existing private & public key pairs rotated.

Signed-off-by: Leela Venkaiah G <[email protected]>
@leelavg
ux: generated code/manifests
629351a 
Signed-off-by: Leela Venkaiah G <[email protected]>
@nb-ohad
Copy link
Contributor

nb-ohad commented Mar 7, 2024

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Mar 7, 2024
@openshift-ci OpenShift CI
Copy link
Contributor

openshift-ci bot commented Mar 7, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: leelavg, nb-ohad

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 7, 2024
@leelavg
Copy link
Contributor Author

leelavg commented Mar 7, 2024

/retest

@openshift-merge-bot openshift-merge-bot bot merged commit e4352ce into red-hat-storage:main Mar 7, 2024
11 checks passed
@leelavg leelavg mentioned this pull request Apr 19, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rchikatw rchikatw rchikatw left review comments

@jarrpa jarrpa jarrpa left review comments

@nb-ohad nb-ohad nb-ohad requested changes

Assignees

@nb-ohad nb-ohad

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@leelavg @nb-ohad @jarrpa @rchikatw