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

gRPC Gateway Removal #14089

Merged
merged 95 commits into from
Sep 4, 2024
Merged

gRPC Gateway Removal #14089

merged 95 commits into from
Sep 4, 2024

Conversation

james-prysm
Copy link
Contributor

@james-prysm james-prysm commented Jun 6, 2024

  • I have read CONTRIBUTING.md.
  • I have made an appropriate entry to CHANGELOG.md.
  • I have added a description to this PR with sufficient context for reviewers to understand this PR.

What type of PR is this?

Cleanup

What does this PR do? Why is it needed?

  • rename GRPC flags to HTTP flags
    • the disable-grpc-gateway flag will be deprecated and removed in a future release.
      The following flags are being renamed:
    • grpc-gateway-host is renamed to http-host. The old name can still be used as an alias.
    • grpc-gateway-port is renamed to http-port.
    • grpc-gateway-corsdomain is renamed to http-cors-domain.
  • removes all gateway mentions in code
  • fully removes support of HTTP based gRPC endpoints ( internal endpoints) breaking change
  • removes unneeded configuration around HTTP server
  • removes most of gateway dependencies
  • removes validator gRPC service entirely

GRPC gateway dependency still in code due to the following dependencies
go.opencensus.io@v0.18.0 github.com/grpc-ecosystem/grpc-gateway@v1.5.0
go.etcd.io/etcd@v0.0.0-20191023171146-3cf2f69b5738 github.com/grpc-ecosystem/grpc-gateway@v1.9.5 <- used by Prometheus dependency

BREAKING CHANGES

The HTTP implementation of gRPC endpoints has been removed as part of this PR. This was never openly recommended to be used and was primarily used internally with our systems.
ONLY EFFECTS THE HTTP VERSION
The gRPC implementation of these endpoints still exist and continue to work for the time being.
They are listed below and are prefixed with eth/v1alpha1

  • /eth/v1alpha1/node/syncing
  • /node/peers
  • /eth/v1alpha1/node/genesis
  • /eth/v1alpha1/node/version
  • /eth/v1alpha1/node/services
  • /eth/v1alpha1/node/p2p
  • /eth/v1alpha1/node/peer
  • /eth/v1alpha1/node/eth1/connections
  • /eth/v1alpha1/validator/duties
  • /eth/v1alpha1/validator/duties/stream
  • /eth/v1alpha1/validator/domain
  • /eth/v1alpha1/validator/chainstart/stream
  • /eth/v1alpha1/validator/activation/stream
  • /eth/v1alpha1/validator/index
  • /eth/v1alpha1/validator/status
  • /eth/v1alpha1/validator/statuses
  • /eth/v1alpha2/validator/block GET,POST
  • /eth/v1alpha1/validator/prepare_beacon_proposer GET,POST
  • /eth/v1alpha1/validator/fee_recipient_by_pub_key
  • /eth/v1alpha1/validator/attestation GET,POST
  • /eth/v1alpha1/validator/aggregate GET,POST
  • /eth/v1alpha1/validator/exit
  • /eth/v1alpha1/validator/subnet/subscribe
  • /eth/v1alpha1/validator/doppelganger
  • /eth/v1alpha1/validator/sync_message_block_root
  • /eth/v1alpha1/validator/sync_message
  • /eth/v1alpha1/sync_subcommittee_index
  • /eth/v1alpha1/validator/contribution_and_proof
  • /eth/v1alpha1/validator/signed_contribution_and_proof
  • /eth/v1alpha1/validator/blocks/stream
  • /eth/v1alpha1/validator/registration
  • /eth/v1alpha1/slasher/attestations/slashable
  • /eth/v1alpha1/slasher/blocks/slashable
  • /eth/v1alpha1/slasher/attestations/highest
  • /eth/v1alpha1/health/logs/stream
  • /eth/v1alpha1/debug/state
  • /eth/v1alpha1/debug/block
  • /eth/v1alpha1/debug/logging
  • /eth/v1alpha1/debug/peers
  • /eth/v1alpha1/debug/peer
  • /eth/v1alpha1/debug/inclusion
  • /eth/v1alpha1/beacon/attestations
  • /eth/v1alpha1/beacon/attestations/indexed
  • /eth/v1alpha1/beacon/attestations/stream
  • /eth/v1alpha1/beacon/attestations/indexed/stream
  • /eth/v1alpha1/beacon/attestations/pool
  • /eth/v1alpha2/beacon/blocks
  • /eth/v1alpha1/beacon/blocks/stream
  • /eth/v1alpha1/beacon/chainhead/stream
  • /eth/v1alpha1/beacon/chainhead
  • /eth/v1alpha1/beacon/committees
  • /eth/v1alpha1/validators/balances
  • /eth/v1alpha1/validators GET, POST
  • /eth/v1alpha1/validators/activesetchanges
  • /eth/v1alpha1/validators/queue
  • /eth/v1alpha1/validators/performance
  • /eth/v1alpha1/validators/assignments
  • /eth/v1alpha1/validators/participation
  • /eth/v1alpha1/beacon/config
  • /eth/v1alpha1/beacon/validators/info/stream
  • /eth/v1alpha1/beacon/slashings/attester/submit
  • /eth/v1alpha1/beacon/slashings/proposer/submit
  • /eth/v1alpha1/beacon/individual_votes

PLEASE USE THE BEACON API REST ENDPOINTS INSTEAD
https://ethereum.github.io/beacon-APIs/?urls.primaryName=dev#/
if the HTTP REST version is not available the API is for internal use only and can only be accessed via gRPC

Tested

  • Web sanity checks with use of API endpoints
  • E2E via TestEndToEnd_MinimalConfig_ValidatorRESTApi

Which issues(s) does this PR fix?

Fixes #12682

Other notes for review

@james-prysm james-prysm added Cleanup Code health! API Api related tasks labels Jun 6, 2024
deps.bzl Show resolved Hide resolved
@james-prysm james-prysm changed the title Gateway removal -WIP gRPC Gateway Removal Jun 11, 2024
@james-prysm james-prysm marked this pull request as ready for review June 11, 2024 21:50
@james-prysm james-prysm requested a review from a team as a code owner June 11, 2024 21:50
@james-prysm james-prysm added the Breaking Changes Breaking changes for a major release label Jun 11, 2024
@james-prysm james-prysm added the Blocked Blocked by research or external factors label Jun 11, 2024
flags.GRPCGatewayHost,
flags.GRPCGatewayPort,
flags.GPRCGatewayCorsDomain,
flags.DeprecatedDisableGRPCGateway,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

will move this

CHANGELOG.md Outdated Show resolved Hide resolved
api/server/httprest/options.go Show resolved Hide resolved
api/server/httprest/options.go Outdated Show resolved Hide resolved
api/server/httprest/options.go Show resolved Hide resolved
config/features/deprecated_flags.go Outdated Show resolved Hide resolved
validator/rpc/server.go Outdated Show resolved Hide resolved
validator/rpc/server.go Outdated Show resolved Hide resolved
rkapka
rkapka previously approved these changes Sep 3, 2024
Comment on lines 28 to 34
// WithTimeout allows changing the timeout value for API calls.
func WithTimeout(seconds uint64) Option {
return func(g *Server) error {
g.cfg.timeout = time.Second * time.Duration(seconds)
return nil
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Why is this in seconds? Why not have the argument be time.Duration? Then it can be any unit of time, not just seconds.

g.server = &http.Server{
Addr: g.cfg.httpAddr,
Handler: handler,
ReadHeaderTimeout: time.Second,
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this use the cfg.timeout?

Copy link
Contributor

@saolyn saolyn left a comment

Choose a reason for hiding this comment

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

lgtm!

@james-prysm james-prysm added this pull request to the merge queue Sep 4, 2024
Merged via the queue into develop with commit 45fd3eb Sep 4, 2024
18 checks passed
@james-prysm james-prysm deleted the gateway-removal branch September 4, 2024 15:47
rkapka added a commit that referenced this pull request Sep 5, 2024
* wip passing e2e

* reverting temp comment

* remove unneeded comments

* fixing merge errors

* fixing more bugs from merge

* fixing test

* WIP moving code around and fixing tests

* unused linting

* gaz

* temp removing these tests as we need placeholder/wrapper APIs for them with the removal of the gateway

* attempting to remove dependencies to gRPC gateway , 1 mroe left in deps.bzl

* renaming flags and other gateway services to http

* goimport

* fixing deepsource

* git mv

* Update validator/package/validator.yaml

Co-authored-by: Radosław Kapka <rkapka@wp.pl>

* Update validator/package/validator.yaml

Co-authored-by: Radosław Kapka <rkapka@wp.pl>

* Update cmd/beacon-chain/flags/base.go

Co-authored-by: Radosław Kapka <rkapka@wp.pl>

* Update cmd/beacon-chain/flags/base.go

Co-authored-by: Radosław Kapka <rkapka@wp.pl>

* Update cmd/beacon-chain/flags/base.go

Co-authored-by: Radosław Kapka <rkapka@wp.pl>

* addressing feedback

* missed lint

* renaming import

* reversal based on feedback

* fixing web ui registration

* don't require mux handler

* gaz

* removing gRPC service from validator completely, merged with http service, renames are a work in progress

* updating go.sum

* linting

* trailing white space

* realized there was more cleanup i could do with code reuse

* adding wrapper for routes

* reverting version

* fixing dependencies from merging develop

* gaz

* fixing unit test

* fixing dependencies

* reverting unit test

* fixing conflict

* updating change log

* Update log.go

Co-authored-by: Preston Van Loon <pvanloon@offchainlabs.com>

* gaz

* Update api/server/httprest/server.go

Co-authored-by: Preston Van Loon <pvanloon@offchainlabs.com>

* addressing some feedback

* forgot to remove deprecated flag in usage

* gofmt

* fixing test

* fixing deepsource issue

* moving deprecated flag and adding timeout handler

* missed removal of a flag

* fixing test:

* Update CHANGELOG.md

Co-authored-by: Radosław Kapka <rkapka@wp.pl>

* addressing feedback

* updating comments based on feedback

* removing unused field for now, we can add it back in if we need to use the option

* removing unused struct

* changing api-timeout flag based on feedback

---------

Co-authored-by: Radosław Kapka <rkapka@wp.pl>
Co-authored-by: Preston Van Loon <pvanloon@offchainlabs.com>
james-prysm added a commit that referenced this pull request Sep 6, 2024
* wip passing e2e

* reverting temp comment

* remove unneeded comments

* fixing merge errors

* fixing more bugs from merge

* fixing test

* WIP moving code around and fixing tests

* unused linting

* gaz

* temp removing these tests as we need placeholder/wrapper APIs for them with the removal of the gateway

* attempting to remove dependencies to gRPC gateway , 1 mroe left in deps.bzl

* renaming flags and other gateway services to http

* goimport

* fixing deepsource

* git mv

* Update validator/package/validator.yaml

Co-authored-by: Radosław Kapka <rkapka@wp.pl>

* Update validator/package/validator.yaml

Co-authored-by: Radosław Kapka <rkapka@wp.pl>

* Update cmd/beacon-chain/flags/base.go

Co-authored-by: Radosław Kapka <rkapka@wp.pl>

* Update cmd/beacon-chain/flags/base.go

Co-authored-by: Radosław Kapka <rkapka@wp.pl>

* Update cmd/beacon-chain/flags/base.go

Co-authored-by: Radosław Kapka <rkapka@wp.pl>

* addressing feedback

* missed lint

* renaming import

* reversal based on feedback

* fixing web ui registration

* don't require mux handler

* gaz

* removing gRPC service from validator completely, merged with http service, renames are a work in progress

* updating go.sum

* linting

* trailing white space

* realized there was more cleanup i could do with code reuse

* adding wrapper for routes

* reverting version

* fixing dependencies from merging develop

* gaz

* fixing unit test

* fixing dependencies

* reverting unit test

* fixing conflict

* updating change log

* Update log.go

Co-authored-by: Preston Van Loon <pvanloon@offchainlabs.com>

* gaz

* Update api/server/httprest/server.go

Co-authored-by: Preston Van Loon <pvanloon@offchainlabs.com>

* addressing some feedback

* forgot to remove deprecated flag in usage

* gofmt

* fixing test

* fixing deepsource issue

* moving deprecated flag and adding timeout handler

* missed removal of a flag

* fixing test:

* Update CHANGELOG.md

Co-authored-by: Radosław Kapka <rkapka@wp.pl>

* addressing feedback

* updating comments based on feedback

* removing unused field for now, we can add it back in if we need to use the option

* removing unused struct

* changing api-timeout flag based on feedback

---------

Co-authored-by: Radosław Kapka <rkapka@wp.pl>
Co-authored-by: Preston Van Loon <pvanloon@offchainlabs.com>
skylenet added a commit to ethpandaops/ethereum-package that referenced this pull request Sep 10, 2024
Related to the deprecation of the gRPC gateway:
prysmaticlabs/prysm#14089

Currently the `mix-with-tools.yaml` test was failing due to the
keymanager being enabled:

```
  == FINISHED SERVICE 'vc-2-nethermind-prysm' LOGS ===================================
  Caused by: An error occurred while waiting for all TCP and UDP ports to be open
  Caused by: Unsuccessful ports check for IP '172.16.0.34' and port spec '{privatePortSpec:0xc00091a0c0}', even after '240' retries with '500' milliseconds in between retries. Timeout '2m0s' has been reached
  Caused by: An error occurred while calling network address '172.16.0.34:5056' with port protocol 'TCP' and using time out '200ms'
  Caused by: dial tcp 172.16.0.34:5056: connect: connection refused

Error encountered running Starlark code.
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API Api related tasks Breaking Changes Breaking changes for a major release Cleanup Code health! Ready For Review A pull request ready for code review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Deprecate and remove grpc-gateway (JSON responses to gRPC services)
4 participants