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

[Traffic Control] Fully remove client IP param injection #17803

Merged
merged 1 commit into from
May 20, 2024

Conversation

williampsmith
Copy link
Contributor

Description

This removes the plumbing to get client IP through json rpc requests by injecting into the request param.

Things NOT touched:

  1. Did not remove plumbing of the client IP from transaction orchestrator forward. This allows for us to later route client IP through other interfaces such as graphql, which has far bettewr support for such things. This is fine as these fields are all optional and should not change anything
  2. We still accept and segment proxied requests in traffic control policies, again, so that other interfaces may forward and everything will work as before.

Test plan

Existing tests


Release notes

Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required.

For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates.

  • Protocol:
  • Nodes (Validators and Full nodes):
  • Indexer:
  • JSON-RPC:
  • GraphQL:
  • CLI:
  • Rust SDK:

Copy link

vercel bot commented May 17, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
sui-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 18, 2024 3:58am
3 Ignored Deployments
Name Status Preview Comments Updated (UTC)
multisig-toolkit ⬜️ Ignored (Inspect) Visit Preview May 18, 2024 3:58am
sui-kiosk ⬜️ Ignored (Inspect) Visit Preview May 18, 2024 3:58am
sui-typescript-docs ⬜️ Ignored (Inspect) Visit Preview May 18, 2024 3:58am

@williampsmith williampsmith requested a review from amnn May 17, 2024 18:46
@williampsmith williampsmith force-pushed the william/remove-ip-param-injection branch from c955294 to 5e746bb Compare May 18, 2024 03:55
@williampsmith williampsmith enabled auto-merge (squash) May 18, 2024 03:57
@williampsmith williampsmith merged commit f0b20d9 into main May 20, 2024
48 checks passed
@williampsmith williampsmith deleted the william/remove-ip-param-injection branch May 20, 2024 14:30
williampsmith added a commit that referenced this pull request May 20, 2024
This removes the plumbing to get client IP through json rpc requests by
injecting into the request param.

Things NOT touched:

1. Did not remove plumbing of the client IP from transaction
orchestrator forward. This allows for us to later route client IP
through other interfaces such as graphql, which has far bettewr support
for such things. This is fine as these fields are all optional and
should not change anything
2. We still accept and segment proxied requests in traffic control
policies, again, so that other interfaces may forward and everything
will work as before.

Existing tests

---

Check each box that your changes affect. If none of the boxes relate to
your changes, release notes aren't required.

For each box you select, include information after the relevant heading
that describes the impact of your changes that a user might notice and
any actions they must take to implement updates.

- [ ] Protocol:
- [ ] Nodes (Validators and Full nodes):
- [ ] Indexer:
- [ ] JSON-RPC:
- [ ] GraphQL:
- [ ] CLI:
- [ ] Rust SDK:
@williampsmith williampsmith mentioned this pull request May 20, 2024
7 tasks
williampsmith added a commit that referenced this pull request May 20, 2024
This removes the plumbing to get client IP through json rpc requests by
injecting into the request param.

Things NOT touched:

1. Did not remove plumbing of the client IP from transaction
orchestrator forward. This allows for us to later route client IP
through other interfaces such as graphql, which has far bettewr support
for such things. This is fine as these fields are all optional and
should not change anything
2. We still accept and segment proxied requests in traffic control
policies, again, so that other interfaces may forward and everything
will work as before.

Existing tests

---

Check each box that your changes affect. If none of the boxes relate to
your changes, release notes aren't required.

For each box you select, include information after the relevant heading
that describes the impact of your changes that a user might notice and
any actions they must take to implement updates.

- [ ] Protocol:
- [ ] Nodes (Validators and Full nodes):
- [ ] Indexer:
- [ ] JSON-RPC:
- [ ] GraphQL:
- [ ] CLI:
- [ ] Rust SDK:
williampsmith added a commit that referenced this pull request May 20, 2024
This removes the plumbing to get client IP through json rpc requests by
injecting into the request param.

Things NOT touched:

1. Did not remove plumbing of the client IP from transaction
orchestrator forward. This allows for us to later route client IP
through other interfaces such as graphql, which has far bettewr support
for such things. This is fine as these fields are all optional and
should not change anything
2. We still accept and segment proxied requests in traffic control
policies, again, so that other interfaces may forward and everything
will work as before.

Existing tests

---

Check each box that your changes affect. If none of the boxes relate to
your changes, release notes aren't required.

For each box you select, include information after the relevant heading
that describes the impact of your changes that a user might notice and
any actions they must take to implement updates.

- [ ] Protocol:
- [ ] Nodes (Validators and Full nodes):
- [ ] Indexer:
- [ ] JSON-RPC:
- [ ] GraphQL:
- [ ] CLI:
- [ ] Rust SDK:
williampsmith added a commit that referenced this pull request May 20, 2024
This removes the plumbing to get client IP through json rpc requests by
injecting into the request param.

Things NOT touched:

1. Did not remove plumbing of the client IP from transaction
orchestrator forward. This allows for us to later route client IP
through other interfaces such as graphql, which has far bettewr support
for such things. This is fine as these fields are all optional and
should not change anything
2. We still accept and segment proxied requests in traffic control
policies, again, so that other interfaces may forward and everything
will work as before.

Existing tests

---

Check each box that your changes affect. If none of the boxes relate to
your changes, release notes aren't required.

For each box you select, include information after the relevant heading
that describes the impact of your changes that a user might notice and
any actions they must take to implement updates.

- [ ] Protocol:
- [ ] Nodes (Validators and Full nodes):
- [ ] Indexer:
- [ ] JSON-RPC:
- [ ] GraphQL:
- [ ] CLI:
- [ ] Rust SDK:
@williampsmith williampsmith mentioned this pull request May 20, 2024
7 tasks
williampsmith added a commit that referenced this pull request May 20, 2024
This removes the plumbing to get client IP through json rpc requests by
injecting into the request param.

Things NOT touched:

1. Did not remove plumbing of the client IP from transaction
orchestrator forward. This allows for us to later route client IP
through other interfaces such as graphql, which has far bettewr support
for such things. This is fine as these fields are all optional and
should not change anything
2. We still accept and segment proxied requests in traffic control
policies, again, so that other interfaces may forward and everything
will work as before.

Existing tests

---

Check each box that your changes affect. If none of the boxes relate to
your changes, release notes aren't required.

For each box you select, include information after the relevant heading
that describes the impact of your changes that a user might notice and
any actions they must take to implement updates.

- [ ] Protocol:
- [ ] Nodes (Validators and Full nodes):
- [ ] Indexer:
- [ ] JSON-RPC:
- [ ] GraphQL:
- [ ] CLI:
- [ ] Rust SDK:
williampsmith added a commit that referenced this pull request May 20, 2024
This removes the plumbing to get client IP through json rpc requests by
injecting into the request param.

Things NOT touched:

1. Did not remove plumbing of the client IP from transaction
orchestrator forward. This allows for us to later route client IP
through other interfaces such as graphql, which has far bettewr support
for such things. This is fine as these fields are all optional and
should not change anything
2. We still accept and segment proxied requests in traffic control
policies, again, so that other interfaces may forward and everything
will work as before.

Existing tests

---

Check each box that your changes affect. If none of the boxes relate to
your changes, release notes aren't required.

For each box you select, include information after the relevant heading
that describes the impact of your changes that a user might notice and
any actions they must take to implement updates.

- [ ] Protocol:
- [ ] Nodes (Validators and Full nodes):
- [ ] Indexer:
- [ ] JSON-RPC:
- [ ] GraphQL:
- [ ] CLI:
- [ ] Rust SDK:
williampsmith added a commit that referenced this pull request May 20, 2024
This removes the plumbing to get client IP through json rpc requests by
injecting into the request param.

Things NOT touched:

1. Did not remove plumbing of the client IP from transaction
orchestrator forward. This allows for us to later route client IP
through other interfaces such as graphql, which has far bettewr support
for such things. This is fine as these fields are all optional and
should not change anything
2. We still accept and segment proxied requests in traffic control
policies, again, so that other interfaces may forward and everything
will work as before.

Existing tests

---

Check each box that your changes affect. If none of the boxes relate to
your changes, release notes aren't required.

For each box you select, include information after the relevant heading
that describes the impact of your changes that a user might notice and
any actions they must take to implement updates.

- [ ] Protocol:
- [ ] Nodes (Validators and Full nodes):
- [ ] Indexer:
- [ ] JSON-RPC:
- [ ] GraphQL:
- [ ] CLI:
- [ ] Rust SDK:
williampsmith added a commit that referenced this pull request May 20, 2024
## Description 

CP the following PRs into 1.24: 
#17408
#17639
#17656
#17761
#17803
#17655

## Test plan 

All logic in this PR is a revert, so just check that existing tests pass

## Release notes

Check each box that your changes affect. If none of the boxes relate to
your changes, release notes aren't required.

For each box you select, include information after the relevant heading
that describes the impact of your changes that a user might notice and
any actions they must take to implement updates.

- [ ] Protocol: 
- [ ] Nodes (Validators and Full nodes): 
- [ ] Indexer: 
- [ ] JSON-RPC: 
- [ ] GraphQL: 
- [ ] CLI: 
- [ ] Rust SDK:
williampsmith added a commit that referenced this pull request May 20, 2024
## Description 

CP the following PRs into 1.25: 
#17639
#17656
#17761
#17803
#17655
#17831




## Test plan 

How did you test the new or updated feature?

---

## Release notes

Check each box that your changes affect. If none of the boxes relate to
your changes, release notes aren't required.

For each box you select, include information after the relevant heading
that describes the impact of your changes that a user might notice and
any actions they must take to implement updates.

- [ ] Protocol: 
- [ ] Nodes (Validators and Full nodes): 
- [ ] Indexer: 
- [ ] JSON-RPC: 
- [ ] GraphQL: 
- [ ] CLI: 
- [ ] Rust SDK:
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.

None yet

2 participants