xiangfu0 opened a new pull request, #19232:
URL: https://github.com/apache/pinot/pull/19232
## Summary
- Disable local filesystem sources for Controller `/ingestFromURI` by
default.
- Validate the URI, loaded filesystem class, and effective delegated
filesystem before parsing input or creating ingestion working state.
- Keep explicitly configured remote filesystems working, including
request-scoped delegated implementations.
- Avoid global registration of request-provided filesystem classes and close
request-scoped implementations after use.
- Return generic client errors without echoing submitted paths or filesystem
class names.
- Add a compatibility-only opt-in,
`controller.ingestFromURI.allowLocalFileSystem`, which remains disabled by
default and documents the controller-local file access risk.
This adapts `dd6520c7267` for the 1.5.x release line and strengthens
validation ordering and delegated-filesystem handling.
## Root cause
Filesystem selection previously happened at copy time. Unknown schemes could
be registered globally before the resulting implementation was fully validated,
while ingestion working directories and input metadata processing happened
before URI policy validation. Validation also needed to inspect nested
`NoClosePinotFS` delegates and subclasses rather than only the requested scheme
or direct class name.
## User impact
Remote filesystem ingestion remains supported. Local URI ingestion now
requires the explicit compatibility setting above; operators enabling it should
only allow trusted callers and paths because the Controller process can read
files visible to its local account.
## How to reproduce
1. Start a Controller with the default configuration and create an offline
table named `foo_OFFLINE`.
2. Submit a local URI to the endpoint:
```bash
curl -X POST -F 'file=@/dev/null' \
'http://localhost:9000/ingestFromURI?tableNameWithType=foo_OFFLINE&batchConfigMapStr=%7B%22inputFormat%22%3A%22csv%22%7D&sourceURIStr=file%3A%2F%2F%2Ftmp%2Finput.csv'
```
3. Before this change, the request reaches local copy/ingestion setup. With
this change it returns a generic HTTP 400 before a working directory or segment
is created.
4. Set `controller.ingestFromURI.allowLocalFileSystem=true` only to verify
the explicit compatibility behavior.
## Validation
- `./mvnw -pl pinot-controller -am -Dskip.npm=true
-Dtest=PinotFSFactoryTest,ControllerConfTest,FileIngestionHelperTest,PinotIngestionRestletResourceStatelessTest
-Dsurefire.failIfNoSpecifiedTests=false test`
- 25 reactor modules passed
- 37 selected tests passed: 2 SPI and 35 Controller
- `./mvnw spotless:apply -pl pinot-spi,pinot-controller -Dskip.npm=true`
- `./mvnw checkstyle:check -pl pinot-spi,pinot-controller -Dskip.npm=true`
- `./mvnw license:format -pl pinot-spi,pinot-controller -Dskip.npm=true`
- `./mvnw license:check -pl pinot-spi,pinot-controller -Dskip.npm=true`
- `./mvnw test-compile -pl pinot-spi,pinot-controller -Dskip.npm=true
-Dmaven.compiler.showDeprecation=true -Dmaven.compiler.showWarnings=true
'-Dmaven.compiler.compilerArgs=-Xlint:all'`
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
