You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2022/04/20 03:33:45 UTC
[GitHub] [airflow] uranusjr opened a new pull request, #23105: Catch Fernet decode error in Connection
uranusjr opened a new pull request, #23105:
URL: https://github.com/apache/airflow/pull/23105
Fix #20267.
This allows a connection with bad Fernet key to still be rendered in the UI, and thus be deleted.
The logic is basically copied from Variable.
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on a diff in pull request #23105: Catch Fernet decode error in Connection
Posted by GitBox <gi...@apache.org>.
uranusjr commented on code in PR #23105:
URL: https://github.com/apache/airflow/pull/23105#discussion_r853819527
##########
airflow/models/connection.py:
##########
@@ -278,16 +287,10 @@ def password(cls):
"""Password. The value is decrypted/encrypted when reading/setting the value."""
return synonym('_password', descriptor=property(cls.get_password, cls.set_password))
- def get_extra(self) -> Dict:
+ def get_extra(self) -> Optional[str]:
Review Comment:
No this should be str to begin with; this function is used to construct the declared_attr `extra`, which is in turn used to construct the property `extra_dejson`:
https://github.com/apache/airflow/blob/501a3c3fbefbcc0d6071a00eb101110fc4733e08/airflow/models/connection.py#L394-L408
It should be quite obvious `extra` can’t be a dict from how this property is implementation.
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on a diff in pull request #23105: Catch Fernet decode error in Connection
Posted by GitBox <gi...@apache.org>.
uranusjr commented on code in PR #23105:
URL: https://github.com/apache/airflow/pull/23105#discussion_r853819527
##########
airflow/models/connection.py:
##########
@@ -278,16 +287,10 @@ def password(cls):
"""Password. The value is decrypted/encrypted when reading/setting the value."""
return synonym('_password', descriptor=property(cls.get_password, cls.set_password))
- def get_extra(self) -> Dict:
+ def get_extra(self) -> Optional[str]:
Review Comment:
No this should be str to begin with; this function is used to construct the declared_attr `extra`, which is in turn used to construct the property `extra_dejson`:
https://github.com/apache/airflow/blob/501a3c3fbefbcc0d6071a00eb101110fc4733e08/airflow/models/connection.py#L394=L408
It should be quite obvious `extra` can’t be a dict from how this property is implementation.
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on a diff in pull request #23105: Catch Fernet decode error in Connection
Posted by GitBox <gi...@apache.org>.
potiuk commented on code in PR #23105:
URL: https://github.com/apache/airflow/pull/23105#discussion_r857901076
##########
airflow/models/connection.py:
##########
@@ -278,16 +287,10 @@ def password(cls):
"""Password. The value is decrypted/encrypted when reading/setting the value."""
return synonym('_password', descriptor=property(cls.get_password, cls.set_password))
- def get_extra(self) -> Dict:
+ def get_extra(self) -> Optional[str]:
Review Comment:
But @dstandish recently implemented changes where extra_dejson is just left for compatibility but extra is supposed to be Dict - and whenever we see it is a string it will raise deprecation warnings.
https://github.com/apache/airflow/pull/21816
With the Airflow 3.0 vision that only Dict will be allowed.
But I agree `extra_dejson` suspiciously assume that extra is a string and I think it is used in a number of places - so possibly it should be corrected even for 2.3.0 (and some `if "str" else assume dict and return it` should be implemented? @dstandish ?
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #23105: Catch Fernet decode error in Connection
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #23105:
URL: https://github.com/apache/airflow/pull/23105#issuecomment-1262963842
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 5 days if no further activity occurs. Thank you for your contributions.
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ashb commented on a diff in pull request #23105: Catch Fernet decode error in Connection
Posted by GitBox <gi...@apache.org>.
ashb commented on code in PR #23105:
URL: https://github.com/apache/airflow/pull/23105#discussion_r853800009
##########
airflow/models/connection.py:
##########
@@ -278,16 +287,10 @@ def password(cls):
"""Password. The value is decrypted/encrypted when reading/setting the value."""
return synonym('_password', descriptor=property(cls.get_password, cls.set_password))
- def get_extra(self) -> Dict:
+ def get_extra(self) -> Optional[str]:
Review Comment:
```suggestion
def get_extra(self) -> Optional[Dict]:
```
I think?
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #23105: Catch Fernet decode error in Connection
Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #23105:
URL: https://github.com/apache/airflow/pull/23105#issuecomment-1108888618
Adding it to 2.3.0 potentially (might be not needed and maybe a separate dejson fix should be added?)
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ashb commented on a diff in pull request #23105: Catch Fernet decode error in Connection
Posted by GitBox <gi...@apache.org>.
ashb commented on code in PR #23105:
URL: https://github.com/apache/airflow/pull/23105#discussion_r853799407
##########
airflow/models/connection.py:
##########
@@ -187,6 +188,21 @@ def _normalize_conn_type(conn_type):
conn_type = conn_type.replace('-', '_')
return conn_type
+ def _safe_fernet_decode(self, key: str, value: str) -> Optional[str]:
+ fernet = get_fernet()
+ if not fernet.is_encrypted:
+ raise AirflowException(
+ f"Can't decrypt encrypted {key!r} for login={self.login} "
+ f"FERNET_KEY configuration is missing"
+ )
+ try:
+ return fernet.decrypt(bytes(value, "utf-8")).decode()
+ except InvalidFernetToken:
+ self.log.error("Can't decrypt %s for login=%s, invalid token or value", key, self.login)
+ except Exception:
+ self.log.error("Can't decrypt %s for login=%s, FERNET_KEY configuration missing", key, self.login)
Review Comment:
Let's change these to use conn_id -- login (aka username) doesn't uniquely identify the connection.
```suggestion
f"Can't decrypt encrypted {key!r} for conn_id={self.conn_id} "
f"FERNET_KEY configuration is missing"
)
try:
return fernet.decrypt(bytes(value, "utf-8")).decode()
except InvalidFernetToken:
self.log.error("Can't decrypt %s for conn_id=%s, invalid token or value", key, self.conn_id)
except Exception:
self.log.error("Can't decrypt %s for conn_id=%s, FERNET_KEY configuration missing", key, self.conn_id)
```
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #23105: Catch Fernet decode error in Connection
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #23105:
URL: https://github.com/apache/airflow/pull/23105#issuecomment-1213605928
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 5 days if no further activity occurs. Thank you for your contributions.
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] closed pull request #23105: Catch Fernet decode error in Connection
Posted by GitBox <gi...@apache.org>.
github-actions[bot] closed pull request #23105: Catch Fernet decode error in Connection
URL: https://github.com/apache/airflow/pull/23105
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org