connectors
connectors copied to clipboard
added support to read proxy settings from env vars
part of https://github.com/elastic/connectors/issues/2017
This solves the issue of the connectors ignoring system proxy settings in env vars.
Added trust_env=True according to https://docs.aiohttp.org/en/stable/client_advanced.html#aiohttp-client-proxy-support
The use of proxy is needed when running the connector alongside elastic cluster behind a corporate firewall, a proxy is used to expose outgoing traffic. For example, to externally SaaS products like ServiceNow, Sharepoint intranet, Teams and Confluence
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
- [ ] Covered the changes with automated tests
- [x] Tested the changes locally
- [ ] Added a label for each target release version (example:
v7.13.2,v7.14.0,v8.0.0) - [ ] Considered corresponding documentation changes
- [ ] Contributed any configuration settings changes to the configuration reference
- [] if you added or changed Rich Configurable Fields for a Native Connector, you made a corresponding PR in Kibana
Changes Requiring Extra Attention
- [ ] Security-related changes (encryption, TLS, SSRF, etc)
- [ ] New external service dependencies added.
Related Pull Requests
https://github.com/elastic/connectors/pull/2266
Work around
In case this doesn't go through, it's possible to do this in the docker build with RUN sed -i 's/aiohttp.ClientSession(/aiohttp.ClientSession(trust_env=True,/g' /app/connectors/sources/sharepoint_online.py
❌ Author of the following commits did not sign a Contributor Agreement: 8dab87319868f04239e58c07606bd7625e52b710,
Please, read and sign the above mentioned agreement if you want to contribute to this project
@chris-gunawardena thanks for the contribution! I think this is a good change to add to Connectors. There are a few things we need to work on before merging first.
- I think it will be important that it is a configurable value. Ideally this would be configurable from the config.yml file. I'll try and find an example of how this could be done.
- This doesn't completely solve the proxy issue linked, because ENV vars can't be set on cloud. We still think it's worth merging for users running on self-managed infrastructure. Can you change the PR link to the issue to
Relates toinstead ofCloses? - Also, do you have any links to documentation that explains which ENV vars can be used like this?
@chris-gunawardena thanks for the contribution! I think this is a good change to add to Connectors. There are a few things we need to work on before merging first.
- I think it will be important that it is a configurable value. Ideally this would be configurable from the config.yml file. I'll try and find an example of how this could be done.
- This doesn't completely solve the proxy issue linked, because ENV vars can't be set on cloud. We still think it's worth merging for users running on self-managed infrastructure. Can you change the PR link to the issue to
Relates toinstead ofCloses?- Also, do you have any links to documentation that explains which ENV vars can be used like this?
We also need to add proxy support between the Connector and the Elasticsearch endpoint.
We also need to add proxy support between the Connector and the Elasticsearch endpoint.
I think that's why @navarone-feekery asked to change the PR description from closes to relates to. This change will have value on its own, even without also adding proxy support for Elasticsearch, which can be added in a separate PR.
buildkite test this