Re: [PR] Adding certificate validation for ssl in mongo hook [airflow]

2024-02-08 Thread via GitHub
amoghrajesh commented on code in PR #37214: URL: https://github.com/apache/airflow/pull/37214#discussion_r1483150140 ## airflow/providers/mongo/CHANGELOG.rst: ## @@ -27,6 +27,17 @@ Changelog - +3.7.0 +.. Review Comment: I will raise a follow up for this. Sorr

Re: [PR] Adding certificate validation for ssl in mongo hook [airflow]

2024-02-08 Thread via GitHub
potiuk commented on code in PR #37214: URL: https://github.com/apache/airflow/pull/37214#discussion_r1482985907 ## airflow/providers/mongo/CHANGELOG.rst: ## @@ -27,6 +27,17 @@ Changelog - +3.7.0 +.. Review Comment: Ah .. Right 🤦 -- This is an automated m

Re: [PR] Adding certificate validation for ssl in mongo hook [airflow]

2024-02-08 Thread via GitHub
Taragolis commented on code in PR #37214: URL: https://github.com/apache/airflow/pull/37214#discussion_r1482980113 ## airflow/providers/mongo/CHANGELOG.rst: ## @@ -27,6 +27,17 @@ Changelog - +3.7.0 +.. Review Comment: Post merge comment: I guess the ve

Re: [PR] Adding certificate validation for ssl in mongo hook [airflow]

2024-02-08 Thread via GitHub
potiuk merged PR #37214: URL: https://github.com/apache/airflow/pull/37214 -- 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: commits-unsubscr...@airflow.a

Re: [PR] Adding certificate validation for ssl in mongo hook [airflow]

2024-02-08 Thread via GitHub
amoghrajesh commented on PR #37214: URL: https://github.com/apache/airflow/pull/37214#issuecomment-1933671125 > Really Nice. Only docs/spellcheck fix and you can merge it. Just ran it locally and pushed a fix, we should be good! -- This is an automated message from the Apache Git Se

Re: [PR] Adding certificate validation for ssl in mongo hook [airflow]

2024-02-08 Thread via GitHub
amoghrajesh commented on code in PR #37214: URL: https://github.com/apache/airflow/pull/37214#discussion_r1482592429 ## airflow/providers/mongo/hooks/mongo.py: ## @@ -75,6 +74,23 @@ def __init__(self, mongo_conn_id: str = default_conn_name, *args, **kwargs) -> N self.c

Re: [PR] Adding certificate validation for ssl in mongo hook [airflow]

2024-02-08 Thread via GitHub
amoghrajesh commented on code in PR #37214: URL: https://github.com/apache/airflow/pull/37214#discussion_r1482586148 ## airflow/providers/mongo/hooks/mongo.py: ## @@ -75,6 +74,23 @@ def __init__(self, mongo_conn_id: str = default_conn_name, *args, **kwargs) -> N self.c

Re: [PR] Adding certificate validation for ssl in mongo hook [airflow]

2024-02-07 Thread via GitHub
potiuk commented on code in PR #37214: URL: https://github.com/apache/airflow/pull/37214#discussion_r1482507876 ## airflow/providers/mongo/hooks/mongo.py: ## @@ -75,6 +74,23 @@ def __init__(self, mongo_conn_id: str = default_conn_name, *args, **kwargs) -> N self.client

Re: [PR] Adding certificate validation for ssl in mongo hook [airflow]

2024-02-07 Thread via GitHub
potiuk commented on code in PR #37214: URL: https://github.com/apache/airflow/pull/37214#discussion_r1482507709 ## airflow/providers/mongo/hooks/mongo.py: ## @@ -75,6 +74,23 @@ def __init__(self, mongo_conn_id: str = default_conn_name, *args, **kwargs) -> N self.client

Re: [PR] Adding certificate validation for ssl in mongo hook [airflow]

2024-02-07 Thread via GitHub
potiuk commented on code in PR #37214: URL: https://github.com/apache/airflow/pull/37214#discussion_r1482505496 ## airflow/providers/mongo/hooks/mongo.py: ## @@ -75,6 +74,23 @@ def __init__(self, mongo_conn_id: str = default_conn_name, *args, **kwargs) -> N self.client

Re: [PR] Adding certificate validation for ssl in mongo hook [airflow]

2024-02-07 Thread via GitHub
potiuk commented on code in PR #37214: URL: https://github.com/apache/airflow/pull/37214#discussion_r1482504765 ## airflow/providers/mongo/hooks/mongo.py: ## @@ -75,6 +74,23 @@ def __init__(self, mongo_conn_id: str = default_conn_name, *args, **kwargs) -> N self.client

Re: [PR] Adding certificate validation for ssl in mongo hook [airflow]

2024-02-07 Thread via GitHub
amoghrajesh commented on PR #37214: URL: https://github.com/apache/airflow/pull/37214#issuecomment-1933376396 @potiuk this has been tested out with the following combinations: ``` { "mongo_http": { "conn_type": "mongo", "description": "", "login": "", "p

Re: [PR] Adding certificate validation for ssl in mongo hook [airflow]

2024-02-07 Thread via GitHub
amoghrajesh commented on PR #37214: URL: https://github.com/apache/airflow/pull/37214#issuecomment-1932485357 Sure, thanks for your review. I will take a look at this soon, mostly tomorrow ist morning -- This is an automated message from the Apache Git Service. To respond to the mes

Re: [PR] Adding certificate validation for ssl in mongo hook [airflow]

2024-02-07 Thread via GitHub
potiuk commented on PR #37214: URL: https://github.com/apache/airflow/pull/37214#issuecomment-1932296310 I think having "alllow_insecure" while setting "ssl=True" is precisely the vulnerability here, because it is unexpected. So i think when user sets "ssl=True" then allow_insecure should

Re: [PR] Adding certificate validation for ssl in mongo hook [airflow]

2024-02-07 Thread via GitHub
amoghrajesh commented on PR #37214: URL: https://github.com/apache/airflow/pull/37214#issuecomment-1932289134 @potiuk to maintain the older behaviour, I think one thing we can do is: in init: ``` if "ssl" is in self.extra and if "allow_insecure" is not in self.extras:

Re: [PR] Adding certificate validation for ssl in mongo hook [airflow]

2024-02-07 Thread via GitHub
potiuk commented on code in PR #37214: URL: https://github.com/apache/airflow/pull/37214#discussion_r1481653314 ## airflow/providers/mongo/hooks/mongo.py: ## @@ -75,6 +74,10 @@ def __init__(self, mongo_conn_id: str = default_conn_name, *args, **kwargs) -> N self.client

Re: [PR] Adding certificate validation for ssl in mongo hook [airflow]

2024-02-07 Thread via GitHub
potiuk commented on code in PR #37214: URL: https://github.com/apache/airflow/pull/37214#discussion_r1481653314 ## airflow/providers/mongo/hooks/mongo.py: ## @@ -75,6 +74,10 @@ def __init__(self, mongo_conn_id: str = default_conn_name, *args, **kwargs) -> N self.client

Re: [PR] Adding certificate validation for ssl in mongo hook [airflow]

2024-02-07 Thread via GitHub
amoghrajesh commented on code in PR #37214: URL: https://github.com/apache/airflow/pull/37214#discussion_r1481650748 ## airflow/providers/mongo/hooks/mongo.py: ## @@ -75,6 +74,10 @@ def __init__(self, mongo_conn_id: str = default_conn_name, *args, **kwargs) -> N self.c

Re: [PR] Adding certificate validation for ssl in mongo hook [airflow]

2024-02-07 Thread via GitHub
amoghrajesh commented on code in PR #37214: URL: https://github.com/apache/airflow/pull/37214#discussion_r1481643804 ## airflow/providers/mongo/hooks/mongo.py: ## @@ -75,6 +74,10 @@ def __init__(self, mongo_conn_id: str = default_conn_name, *args, **kwargs) -> N self.c

Re: [PR] Adding certificate validation for ssl in mongo hook [airflow]

2024-02-07 Thread via GitHub
potiuk commented on code in PR #37214: URL: https://github.com/apache/airflow/pull/37214#discussion_r1481609101 ## airflow/providers/mongo/hooks/mongo.py: ## @@ -75,6 +74,10 @@ def __init__(self, mongo_conn_id: str = default_conn_name, *args, **kwargs) -> N self.client

Re: [PR] Adding certificate validation for ssl in mongo hook [airflow]

2024-02-07 Thread via GitHub
amoghrajesh commented on code in PR #37214: URL: https://github.com/apache/airflow/pull/37214#discussion_r1481199290 ## airflow/providers/mongo/hooks/mongo.py: ## @@ -75,6 +74,10 @@ def __init__(self, mongo_conn_id: str = default_conn_name, *args, **kwargs) -> N self.c

Re: [PR] Adding certificate validation for ssl in mongo hook [airflow]

2024-02-07 Thread via GitHub
amoghrajesh commented on PR #37214: URL: https://github.com/apache/airflow/pull/37214#issuecomment-1931686673 > > That tip saved hassle for me, ran it via docker compose and it worked :D > > Yeah in breeze only available simple integration, which need to be run with `--integration mon

Re: [PR] Adding certificate validation for ssl in mongo hook [airflow]

2024-02-07 Thread via GitHub
Taragolis commented on PR #37214: URL: https://github.com/apache/airflow/pull/37214#issuecomment-1931678760 > That tip saved hassle for me, ran it via docker compose and it worked :D Yeah in breeze only available simple integration, which need to be run with `--integration mongo`, e.g

Re: [PR] Adding certificate validation for ssl in mongo hook [airflow]

2024-02-07 Thread via GitHub
amoghrajesh commented on code in PR #37214: URL: https://github.com/apache/airflow/pull/37214#discussion_r1481199290 ## airflow/providers/mongo/hooks/mongo.py: ## @@ -75,6 +74,10 @@ def __init__(self, mongo_conn_id: str = default_conn_name, *args, **kwargs) -> N self.c

Re: [PR] Adding certificate validation for ssl in mongo hook [airflow]

2024-02-07 Thread via GitHub
amoghrajesh commented on PR #37214: URL: https://github.com/apache/airflow/pull/37214#issuecomment-1931650831 Awesome. I was able to test it. Results: My connections defined: https://github.com/apache/airflow/assets/35884252/06a85bbd-579b-436b-9079-ec27499120c2";>

Re: [PR] Adding certificate validation for ssl in mongo hook [airflow]

2024-02-07 Thread via GitHub
amoghrajesh commented on PR #37214: URL: https://github.com/apache/airflow/pull/37214#issuecomment-1931637366 Thanks @Taragolis ! That tip saved hassle for me, ran it via docker compose and it worked :D -- This is an automated message from the Apache Git Service. To respond to the messa

Re: [PR] Adding certificate validation for ssl in mongo hook [airflow]

2024-02-07 Thread via GitHub
amoghrajesh commented on PR #37214: URL: https://github.com/apache/airflow/pull/37214#issuecomment-1931574781 > > Also tried adding mongo://127.0.0.1, this doesnt work either for uri > > I've wondering, did you run Airflow into the docker? I am running it using breeze, so yes, y

Re: [PR] Adding certificate validation for ssl in mongo hook [airflow]

2024-02-07 Thread via GitHub
Taragolis commented on code in PR #37214: URL: https://github.com/apache/airflow/pull/37214#discussion_r1481130143 ## airflow/providers/mongo/hooks/mongo.py: ## @@ -75,6 +74,10 @@ def __init__(self, mongo_conn_id: str = default_conn_name, *args, **kwargs) -> N self.cli

Re: [PR] Adding certificate validation for ssl in mongo hook [airflow]

2024-02-07 Thread via GitHub
Taragolis commented on PR #37214: URL: https://github.com/apache/airflow/pull/37214#issuecomment-1931540047 > Also tried adding mongo://127.0.0.1, this doesnt work either for uri I've wondering, did you run Airflow into the docker? -- This is an automated message from the Apache Git

Re: [PR] Adding certificate validation for ssl in mongo hook [airflow]

2024-02-06 Thread via GitHub
amoghrajesh commented on PR #37214: URL: https://github.com/apache/airflow/pull/37214#issuecomment-1931432371 Also tried adding mongo://127.0.0.1, this doesnt work either for uri -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub

Re: [PR] Adding certificate validation for ssl in mongo hook [airflow]

2024-02-06 Thread via GitHub
amoghrajesh commented on PR #37214: URL: https://github.com/apache/airflow/pull/37214#issuecomment-1931415839 Right now I am testing with the DAG: ``` from airflow import DAG from airflow.providers.mongo.hooks.mongo import MongoHook from airflow.operators.python_operator import Py

Re: [PR] Adding certificate validation for ssl in mongo hook [airflow]

2024-02-06 Thread via GitHub
amoghrajesh commented on code in PR #37214: URL: https://github.com/apache/airflow/pull/37214#discussion_r1480978086 ## airflow/providers/mongo/hooks/mongo.py: ## @@ -98,12 +101,13 @@ def get_conn(self) -> MongoClient: # If we are using SSL disable requiring certs fro

Re: [PR] Adding certificate validation for ssl in mongo hook [airflow]

2024-02-06 Thread via GitHub
eladkal commented on code in PR #37214: URL: https://github.com/apache/airflow/pull/37214#discussion_r1480958341 ## airflow/providers/mongo/hooks/mongo.py: ## @@ -98,12 +101,13 @@ def get_conn(self) -> MongoClient: # If we are using SSL disable requiring certs from sp

Re: [PR] Adding certificate validation for ssl in mongo hook [airflow]

2024-02-06 Thread via GitHub
amoghrajesh commented on code in PR #37214: URL: https://github.com/apache/airflow/pull/37214#discussion_r1480909864 ## airflow/providers/mongo/hooks/mongo.py: ## @@ -98,12 +101,13 @@ def get_conn(self) -> MongoClient: # If we are using SSL disable requiring certs fro

Re: [PR] Adding certificate validation for ssl in mongo hook [airflow]

2024-02-06 Thread via GitHub
amoghrajesh commented on code in PR #37214: URL: https://github.com/apache/airflow/pull/37214#discussion_r1480908546 ## airflow/providers/mongo/hooks/mongo.py: ## @@ -98,12 +101,13 @@ def get_conn(self) -> MongoClient: # If we are using SSL disable requiring certs fro

[PR] Adding certificate validation for ssl in mongo hook [airflow]

2024-02-06 Thread via GitHub
amoghrajesh opened a new pull request, #37214: URL: https://github.com/apache/airflow/pull/37214 Currently, we are skipping tls certificate validation in mongo hook even if ssl is set to true. This needs to be handled better --- **^ Add meaningful description ab