Config option to disable connector secrets for native connectors

jedrazb commented 2 months ago

Closes https://github.com/elastic/search-team/issues/8229

For Elastic-managed connectors in agentless we need to disable connector secrets on a service-level.

I added new "internal" option _use_native_connector_api_keys that is not documented in config.py , similar to how we handle the _force_allow_native flag in: https://github.com/elastic/connectors/blob/4e97330803087151952debbdfa35408ff240666d/connectors/preflight_check.py#L192

Let me know if this approach work for you!

Verification

I ran the setup e2e and verified that this allows native connectors to run without connector secrets. If flag is not provided to service config the logic defaults it to True which is backward compatible option.

Checklists

Pre-Review Checklist

[x] this PR does NOT contain credentials of any kind, such as API keys or username/passwords (double check config.yml.example)
[x] this PR has a meaningful title
[x] this PR links to all relevant github issues that it fixes or partially addresses
[x] if there is no GH issue, please create it. Each PR should have a link to an issue
[x] this PR has a thorough description
[x] Covered the changes with automated tests
[x] Tested the changes locally
[x] Added a label for each target release version (example: v7.13.2, v7.14.0, v8.0.0)

jedrazb commented 2 months ago

Which api key will be used in this case? The one that's specified in the config, right?

@artem-shelkovnikov yeah the default authentication method (whatever comes from the config) will be used.

This is the result of the fact that we don't call the _update_native_connector_authentication function in the if statement

https://github.com/elastic/connectors/blob/4e97330803087151952debbdfa35408ff240666d/connectors/sync_job_runner.py#L183

Which in turns overrides es config api_key used for authentication for this sync job execution.

https://github.com/elastic/connectors/blob/4e97330803087151952debbdfa35408ff240666d/connectors/sync_job_runner.py#L248-L251

I think it should be ok just to add extra assertion that _update_native_connector_authentication never gets called?

seanstory commented 2 months ago

Any reason why we shouldn't just remove all vestiges of connector secrets in 9.x? Why make it configurable instead of just removing all that code?

jedrazb commented 2 months ago

Any reason why we shouldn't just remove all vestiges of connector secrets in 9.x? Why make it configurable instead of just removing all that code?

@seanstory This isa a minimal change that unblocks us short-tem.

Connector secrets are already available in the connector service, CLI, and even through ES APIs for managing them. We should plan for a dedicated initiative to deprecate our current "secrets" for 9.x

jedrazb commented 2 months ago

@artem-shelkovnikov I addressed your comment to verify if es_config api key is changed in https://github.com/elastic/connectors/pull/2808/commits/897e692ccf480accd127bbfee472256b5ece694d (we are checking if the func that modifies es config is called)

elastic / connectors