SQL: add support for API key to JDBC and CLI#142021
SQL: add support for API key to JDBC and CLI#142021luigidellaquila merged 14 commits intoelastic:mainfrom
Conversation
ℹ️ Important: Docs version tagging👋 Thanks for updating the docs! Just a friendly reminder that our docs are now cumulative. This means all 9.x versions are documented on the same page and published off of the main branch, instead of creating separate pages for each minor version. We use applies_to tags to mark version-specific features and changes. Expand for a quick overviewWhen to use applies_to tags:✅ At the page level to indicate which products/deployments the content applies to (mandatory) What NOT to do:❌ Don't remove or replace information that applies to an older version 🤔 Need help?
|
|
Pinging @elastic/es-analytical-engine (Team:Analytics) |
|
Hi @luigidellaquila, I've created a changelog YAML for you. |
| if (options.has(silentOption)) { | ||
| terminal.setVerbosity(Terminal.Verbosity.SILENT); | ||
| loggerFactory.setRootLevel(Level.OFF); | ||
| if (loggerFactory != null) { |
There was a problem hiding this comment.
Why is this needed now but wasn't needed so far?
There was a problem hiding this comment.
This is not a consequence of this PR.
The factory is null since #123742, so it started throwing NPE. I don't know why we didn't notice it before.
The problem is that our CLI does not depend on server, so we can't access LogConfigurator and LoggerFactoryImpl.
Let me see if we can do better, maybe we can setup a custom log factory
There was a problem hiding this comment.
It's more complex than I thought.
SQL CLI runs as a shadow JAR, so I can't access LoggerFactory.setInstance() directly.
I'll open an issue, but I guess the null checks are enough to unblock the component
| apiKey = settings.getProperty(AUTH_API_KEY); | ||
|
|
||
| // validate that only one authentication method is specified | ||
| if (StringUtils.hasText(apiKey) && StringUtils.hasText(user)) { |
There was a problem hiding this comment.
Shouldn't this validation also apply in other constructor as well?
| $ ./bin/elasticsearch-sql-cli https://sql_user:strongpassword@some.server:9200 | ||
| ``` | ||
|
|
||
| ### API Key Authentication [sql-cli-apikey] |
There was a problem hiding this comment.
I am wondering about --apikey <value> on the command line being visible via ps aux or /proc/<pid>/cmdline. The docs mention not to combine API key with basic auth, but don't
warn about this security aspect. Should this be mentioned in docs, and provide alternative suggestions?
There was a problem hiding this comment.
That's a good point. I'm adding a note to the docs.
|
|
||
| ### API Key Authentication [sql-cli-apikey] | ||
|
|
||
| As an alternative to basic authentication, you can use API key authentication with the `--apikey` option. API keys can be created using the [Create API key API](docs-content://deploy-manage/api-keys/elasticsearch-api-keys.md). The API key should be provided in its encoded form (the `encoded` value returned by the Create API key API): |
There was a problem hiding this comment.
This is for consistency, JDBC and CLI use all lowercase parameters.
| As an alternative to basic authentication, you can use API key authentication with the `--apikey` option. API keys can be created using the [Create API key API](docs-content://deploy-manage/api-keys/elasticsearch-api-keys.md). The API key should be provided in its encoded form (the `encoded` value returned by the Create API key API): | ||
|
|
||
| ```bash | ||
| $ ./bin/elasticsearch-sql-cli --apikey <encoded-api-key> https://some.server:9200 |
| } | ||
|
|
||
| @SuppressWarnings("this-escape") | ||
| private EmbeddedCli( |
There was a problem hiding this comment.
I think only this constructor needs the @suppressWarnings.
| @@ -0,0 +1,218 @@ | |||
| /* | |||
There was a problem hiding this comment.
I like comments in code, but in these test classes I think there are too many comments imo.
There was a problem hiding this comment.
Let me remove the redundant comments
| return cluster.split(",")[0]; | ||
| } | ||
|
|
||
| private String createApiKey(String name, String body) throws IOException { |
There was a problem hiding this comment.
I think you should clean up these api keys when the test finishes (or fails midway) in an @After method.
| } | ||
| } | ||
|
|
||
| private String createApiKey(String name, String body) throws IOException { |
There was a problem hiding this comment.
Same here. You should clean up these api keys when the test finishes (or fails midway) in an @After method.
astefan
left a comment
There was a problem hiding this comment.
It's LGTM, but I would look into extracting common code from both CliApiKeyIT and JdbcApiKeyIT in a base class and extend that instead (like the clean up logic, the method that creates the api keys etc)
|
Thanks @astefan! |
…on-sliced-reindex * upstream/main: Activity logging improvements (elastic#142901) Fix serialization of NodeGpuStatsResponse when no GPU is present (elastic#142937) Add doc on master elections in DistributedArchitectureGuide (elastic#142435) ESQL: Account for missing StubRelation due to SurrogateExpressions replacement (elastic#142882) Add BulkByScrollTask Serialization Tests (elastic#142697) Rebalance CI test partitions to reduce Part3 bottleneck (elastic#142930) Mute org.elasticsearch.xpack.esql.qa.multi_node.EsqlClientYamlIT test {p0=esql/40_tsdb/to_aggregate_metric_double with multi_values} elastic#142964 Bump OpenTelemetry dependencies (elastic#142323) SQL: add support for API key to JDBC and CLI (elastic#142021) Ensure requested capability exists (elastic#142695) Warn and fall back to local branches.json (elastic#142606) [CI] Mute testWithFetchFailures, testAddCompletionListenerScheduleErr… (elastic#142926) ESQL: Add support for ORC file format (elastic#142900) Update wolfi (versioned) (elastic#142948) Add BulkByScrollResponse Serialization Tests (elastic#142688) Run 25_id_generation with and without synthetic id (elastic#142770)
Adding support for API key authentication to SQL HTTP client, used by JDBC and CLI.
Also adding an option to use it in JDBC connection URL and as a CLI parameter
Developed using AI-assisted tooling