jerryshao opened a new pull request, #14024:
URL: https://github.com/apache/gravitino/pull/14024
### What changes were proposed in this pull request?
- Add a `trustedProxies` web server setting to `JettyServerConfig`: a
comma-separated list of IP literals or CIDR blocks (IPv4 or IPv6), empty by
default. Because it lives in `JettyServerConfig`, it applies to
`gravitino.server.webserver.trustedProxies`,
`gravitino.iceberg-rest.trustedProxies` and
`gravitino.lance-rest.trustedProxies`. Entries are parsed with Guava
`InetAddresses`, so host names are never resolved. An invalid entry fails
startup with an error that names the full key.
- Add `ClientAddressResolver` in server-common:
- If the TCP peer is not a trusted proxy, return the peer address and
ignore `X-Forwarded-For`.
- Otherwise, combine all `X-Forwarded-For` headers, walk the entries right
to left skipping trusted proxies, and return the first untrusted entry. If
every entry is trusted, return the left-most one.
- Strip ports and IPv6 brackets. A malformed or `unknown` entry stops the
walk at the last trusted hop.
- Return addresses in canonical form: IPv6 compressed per RFC 5952 and
IPv4-mapped IPv6 as plain IPv4. This also covers Jetty's bracketed IPv6 peer
form, `[0:0:0:0:0:0:0:1]`.
- Use the resolver in `RequestContextFilter`, in the `HttpAuditFilter`
fallback and in `IcebergRequestContext`. `IcebergRequestContext` now prefers
the address in `RequestContext` and no longer calls `getRemoteHost()`.
`GravitinoServer`, the Iceberg `RESTService` and `LanceRESTService` pass the
configured resolver to their filters.
- Fix stale Javadoc and comments. Document the key in the server, Iceberg
REST and Lance REST docs, and add a migration-guide note.
### Why are the changes needed?
Audit events used the first `X-Forwarded-For` entry as the client address
whenever the header was present, without checking which proxy the request
actually came from. The recorded address was therefore reliable only when every
request passed through a reverse proxy that overwrote the header.
Fix: #14021
### Does this PR introduce _any_ user-facing change?
Yes.
- New property keys: `gravitino.server.webserver.trustedProxies`,
`gravitino.iceberg-rest.trustedProxies` and
`gravitino.lance-rest.trustedProxies`.
- `X-Forwarded-For` is now ignored unless the TCP peer is a listed proxy.
Deployments behind a load balancer or ingress should set `trustedProxies`;
otherwise the audit log records the proxy address.
- IPv6 client addresses are recorded in compressed form, for example `::1`
instead of `[0:0:0:0:0:0:0:1]`.
### How was this patch tested?
- New `TestClientAddressResolver` cases cover:
- an untrusted peer with a forged header;
- a trusted proxy chain;
- CIDR, IPv6 and IPv4-mapped addresses;
- multiple headers, ports and brackets;
- malformed entries;
- canonical output.
- New `TestJettyServerConfig` cases cover parsing and full-key error
messages.
- New `TestJettyServer` cases send real requests over IPv4 and IPv6 loopback.
- Updated `TestRequestContextFilter`, `TestHttpAuditFilter` and
`TestIcebergRequestContext`.
- Ran `./gradlew :server-common:test :server:test
:iceberg:iceberg-rest-server:test :lance:lance-rest-server:test -PskipITs`.
- Manually started a server built from this branch with the audit log
enabled. Over IPv4 and IPv6, I checked the recorded address for the Gravitino
server, the Iceberg REST and Lance REST aux services, default and trusted
configurations, and invalid values.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
