Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't pass auth to opensearch client with empty login and password #39982

Merged
merged 2 commits into from
Oct 4, 2024

Conversation

pdebelak
Copy link
Contributor

This updates the opensearch hook to only pass the http_auth argument to the opensearch client if a login and password are part of the connection.

closes: #39979


^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in newsfragments.

@eladkal
Copy link
Contributor

eladkal commented Jun 8, 2024

cc @cjames23 can you take a look?

@pdebelak pdebelak force-pushed the opensearch-handle-no-auth branch 5 times, most recently from 424cbd2 to 5f60395 Compare June 12, 2024 15:39
@pdebelak pdebelak force-pushed the opensearch-handle-no-auth branch from 5f60395 to dd4badb Compare June 26, 2024 14:35
Copy link

@ranade1 ranade1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good.

@topherinternational
Copy link
Contributor

@eladkal @cjames23 is there any review action coming on this?

My org could really use this change, we hit AWS OpenSearch with no auth (instead we validate that the client is inside our VPC/VPN). To get around the bug, we currently subclass the OpenSearchHook to shim in pretty much this exact code change. Would be great to not have to have this subclass at all.

@topherinternational
Copy link
Contributor

@eladkal ping again on reviewing this? cjames seems to be off the grid.

@Owen-CH-Leung
Copy link
Contributor

LGTM. This aligns with the Elasticsearch provider behaviour too

This updates the opensearch hook to only pass the http_auth argument
to the opensearch client if a login and password are part of the
connection.
@eladkal eladkal force-pushed the opensearch-handle-no-auth branch from 3c9cc9f to fc3732a Compare October 4, 2024 08:19
@eladkal eladkal merged commit b0a18d9 into apache:main Oct 4, 2024
55 checks passed
joaopamaral pushed a commit to joaopamaral/airflow that referenced this pull request Oct 21, 2024
* Don't pass auth to opensearch client with empty login and password

This updates the opensearch hook to only pass the http_auth argument
to the opensearch client if a login and password are part of the
connection.

* Add typed dict for opensearch client arguments to satisfy mypy
ellisms pushed a commit to ellisms/airflow that referenced this pull request Nov 13, 2024
* Don't pass auth to opensearch client with empty login and password

This updates the opensearch hook to only pass the http_auth argument
to the opensearch client if a login and password are part of the
connection.

* Add typed dict for opensearch client arguments to satisfy mypy
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Opensearch hook passes auth even if login and password are blank
5 participants