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.

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

API Middleware for Keymanager Standard API Endpoints #9936

Merged
merged 37 commits into from
Dec 7, 2021

Conversation

rauljordan
Copy link
Contributor

@rauljordan rauljordan commented Nov 24, 2021

This PR adds API middleware support for our keymanager standard https://ethereum.github.io/keymanager-APIs/#/ to support translating grpc gateway responses into the expected types, such as hex strings and enums, to align with the
standard.

Changes

  • Support DELETE requests in the api middleware
  • Support DELETE requests in the cors middleware used by the API gateway
  • Regenerate protobufs
  • Amend the custom mux used by the validator client to be able to handle the regular gateway, the api middleware, and web requests
  • Small fix to delete keystores to return empty response if request has no public keys

Requirements

  • GET request working! Able to return the list of public keys as hex strings being used by the keymanager
  • DELETE request working! Able to successfully delete keystores and retrieve their slashing protection history. If key has been deleted before, it will still return slashing protection history and NOT_ACTIVE
  • POST request working! It is able to import keystores with the given password and their associated slashing protection history as well. For a script that is able to make this complex POST request, see the Gist prepared

https://gist.github.com/rauljordan/89b35a15032dedd77cf14cfec9886c18

@rauljordan rauljordan marked this pull request as ready for review December 2, 2021 22:35
@rauljordan rauljordan requested a review from a team as a code owner December 2, 2021 22:35
@rauljordan rauljordan added the Ready For Review A pull request ready for code review label Dec 2, 2021
@rauljordan rauljordan removed the Ready For Review A pull request ready for code review label Dec 2, 2021
@rauljordan rauljordan marked this pull request as draft December 2, 2021 22:45
@rauljordan rauljordan self-assigned this Dec 3, 2021
@rauljordan rauljordan added Ready For Review A pull request ready for code review API Api related tasks labels Dec 3, 2021
@rauljordan rauljordan marked this pull request as ready for review December 3, 2021 14:00
james-prysm
james-prysm previously approved these changes Dec 3, 2021
Copy link
Contributor

@james-prysm james-prysm left a comment

Choose a reason for hiding this comment

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

I think the only thing that stood out to me was that we need to remember to add put and patch as accessible functions and currently assumes put and patch as a post.. which may result in undesired effects if we don't remember to update.

api/gateway/apimiddleware/api_middleware.go Outdated Show resolved Hide resolved
validator/node/node.go Outdated Show resolved Hide resolved
validator/node/node.go Outdated Show resolved Hide resolved
validator/node/node.go Outdated Show resolved Hide resolved
api/gateway/gateway.go Outdated Show resolved Hide resolved
Co-authored-by: Radosław Kapka <rkapka@wp.pl>
rkapka
rkapka previously approved these changes Dec 7, 2021
Copy link
Contributor

@james-prysm james-prysm left a comment

Choose a reason for hiding this comment

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

didn't see anything that stood out to me and radek gave blessings

WithMaxCallRecvMsgSize(maxCallSize)
opts := []apigateway.Option{
apigateway.WithGatewayAddr(gatewayAddress),
apigateway.WithRemoteAddr(selfAddress),
Copy link
Contributor

Choose a reason for hiding this comment

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

oh this looks a lot more clear

@prylabs-bulldozer prylabs-bulldozer bot merged commit 424c8f6 into develop Dec 7, 2021
@delete-merged-branch delete-merged-branch bot deleted the api-middleware-keymanager branch December 7, 2021 20:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API Api related tasks Ready For Review A pull request ready for code review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants