You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "bugraoz93 (via GitHub)" <gi...@apache.org> on 2025/11/09 18:33:30 UTC
[PR] Add dummy Fernet key for integration tests [airflow]
bugraoz93 opened a new pull request, #58112:
URL: https://github.com/apache/airflow/pull/58112
<!--
Licensed to the Apache Software Foundation (ASF) under one
or more contributor license agreements. See the NOTICE file
distributed with this work for additional information
regarding copyright ownership. The ASF licenses this file
to you under the Apache License, Version 2.0 (the
"License"); you may not use this file except in compliance
with the License. You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing,
software distributed under the License is distributed on an
"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, either express or implied. See the License for the
specific language governing permissions and limitations
under the License.
-->
<!--
Thank you for contributing! Please make sure that your code changes
are covered with tests. And in case of new features or big changes
remember to adjust the documentation.
Feel free to ping committers for the review!
In case of an existing issue, reference it using one of the following:
closes: #ISSUE
related: #ISSUE
How to write a good git commit message:
http://chris.beams.io/posts/git-commit/
-->
<!-- Please keep an empty line above the dashes. -->
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#pull-request-guidelines)** for more information.
In case of fundamental code changes, an Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+Proposals)) is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
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 [airflow-core/newsfragments](https://github.com/apache/airflow/tree/main/airflow-core/newsfragments).
--
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
Re: [PR] Add dummy Fernet key for integration tests [airflow]
Posted by "jason810496 (via GitHub)" <gi...@apache.org>.
jason810496 commented on code in PR #58112:
URL: https://github.com/apache/airflow/pull/58112#discussion_r2514344924
##########
airflow-core/docs/howto/docker-compose/docker-compose.yaml:
##########
@@ -60,7 +60,11 @@ x-airflow-common:
AIRFLOW__DATABASE__SQL_ALCHEMY_CONN: postgresql+psycopg2://airflow:airflow@postgres/airflow
AIRFLOW__CELERY__RESULT_BACKEND: db+postgresql://airflow:airflow@postgres/airflow
AIRFLOW__CELERY__BROKER_URL: redis://:@redis:6379/0
- AIRFLOW__CORE__FERNET_KEY: ''
+ #
+ # Please Do not use this Fernet key in any deployments! Please generate your own key.
+ # This is specifically generated for tests and not as default. It could fail any static level code checks.
+ #
+ AIRFLOW__CORE__FERNET_KEY: 'dGVzdF9rZXlfdGVzdF9rZXlfdGVzdF9rZXlfdGVzdF8='
Review Comment:
Agree with Amogh and Jarek for generating key in runtime.
--
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
Re: [PR] Add dummy Fernet key for integration tests [airflow]
Posted by "bugraoz93 (via GitHub)" <gi...@apache.org>.
bugraoz93 commented on code in PR #58112:
URL: https://github.com/apache/airflow/pull/58112#discussion_r2515063113
##########
airflow-core/docs/howto/docker-compose/docker-compose.yaml:
##########
@@ -60,7 +60,11 @@ x-airflow-common:
AIRFLOW__DATABASE__SQL_ALCHEMY_CONN: postgresql+psycopg2://airflow:airflow@postgres/airflow
AIRFLOW__CELERY__RESULT_BACKEND: db+postgresql://airflow:airflow@postgres/airflow
AIRFLOW__CELERY__BROKER_URL: redis://:@redis:6379/0
- AIRFLOW__CORE__FERNET_KEY: ''
+ #
+ # Please Do not use this Fernet key in any deployments! Please generate your own key.
+ # This is specifically generated for tests and not as default. It could fail any static level code checks.
+ #
+ AIRFLOW__CORE__FERNET_KEY: 'dGVzdF9rZXlfdGVzdF9rZXlfdGVzdF9rZXlfdGVzdF8='
Review Comment:
Good idea! It makes sense! I will update it soon
--
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
Re: [PR] Add dummy Fernet key for integration tests [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #58112:
URL: https://github.com/apache/airflow/pull/58112#discussion_r2514026817
##########
airflow-core/docs/howto/docker-compose/docker-compose.yaml:
##########
@@ -60,7 +60,11 @@ x-airflow-common:
AIRFLOW__DATABASE__SQL_ALCHEMY_CONN: postgresql+psycopg2://airflow:airflow@postgres/airflow
AIRFLOW__CELERY__RESULT_BACKEND: db+postgresql://airflow:airflow@postgres/airflow
AIRFLOW__CELERY__BROKER_URL: redis://:@redis:6379/0
- AIRFLOW__CORE__FERNET_KEY: ''
+ #
+ # Please Do not use this Fernet key in any deployments! Please generate your own key.
+ # This is specifically generated for tests and not as default. It could fail any static level code checks.
+ #
+ AIRFLOW__CORE__FERNET_KEY: 'dGVzdF9rZXlfdGVzdF9rZXlfdGVzdF9rZXlfdGVzdF8='
Review Comment:
Good idea. We can even generate it dynamically in this case - we don't care what the key is, as long as it is there and we would not have to keep something hard-coded that looks like a valid private key.
--
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
Re: [PR] Add dummy Fernet key for integration tests [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #58112:
URL: https://github.com/apache/airflow/pull/58112#discussion_r2517504451
##########
devel-common/src/tests_common/test_utils/fernet.py:
##########
@@ -0,0 +1,48 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+from __future__ import annotations
+
+from pathlib import Path
+
+import yaml
+from cryptography.fernet import Fernet
+
+
+def generate_fernet_key_string() -> str:
+ """Generate a new Fernet key."""
+ return Fernet.generate_key().decode()
+
+
+def update_environment_variable_from_compose_yaml(file_path: str | Path) -> None:
Review Comment:
One comment here - I somehow do not like the idea of dynamically generating the compose file, that sounds like something rather difficult to debug. While it's ok to do it now, I feel that better solution would be to read the key from a fernet key passed as a variable to the compose file. This can be done - for example by having several compose files combined together (you just pass those files as array of compse files). I will submit my change soon for task-sdk testing to show how I have done that to make testing easier (once I solve one last problem).
--
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
Re: [PR] Add dummy Fernet key for integration tests [airflow]
Posted by "amoghrajesh (via GitHub)" <gi...@apache.org>.
amoghrajesh commented on code in PR #58112:
URL: https://github.com/apache/airflow/pull/58112#discussion_r2512990364
##########
airflow-core/docs/howto/docker-compose/docker-compose.yaml:
##########
@@ -60,7 +60,11 @@ x-airflow-common:
AIRFLOW__DATABASE__SQL_ALCHEMY_CONN: postgresql+psycopg2://airflow:airflow@postgres/airflow
AIRFLOW__CELERY__RESULT_BACKEND: db+postgresql://airflow:airflow@postgres/airflow
AIRFLOW__CELERY__BROKER_URL: redis://:@redis:6379/0
- AIRFLOW__CORE__FERNET_KEY: ''
+ #
+ # Please Do not use this Fernet key in any deployments! Please generate your own key.
+ # This is specifically generated for tests and not as default. It could fail any static level code checks.
+ #
+ AIRFLOW__CORE__FERNET_KEY: 'dGVzdF9rZXlfdGVzdF9rZXlfdGVzdF9rZXlfdGVzdF8='
Review Comment:
Like this for example:
```python
common_env = compose_config["x-airflow-common"].get("environment", {})
common_env.update(docker_compose_env_vars)
```
--
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
Re: [PR] Add dummy Fernet key for integration tests [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #58112:
URL: https://github.com/apache/airflow/pull/58112#issuecomment-3539418400
```
___________________
< Big Banner, yeah! >
-------------------
\ ^__^
\ (oo)\_______
(__)\ )\/\
||----w |
|| ||
```
Yes. Why not... But we already have another candidate for that banner - avoiding COW when forking ... #58365 (cc: @wjddn279 )
--
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
Re: [PR] Add dummy Fernet key for integration tests [airflow]
Posted by "bugraoz93 (via GitHub)" <gi...@apache.org>.
bugraoz93 commented on PR #58112:
URL: https://github.com/apache/airflow/pull/58112#issuecomment-3539447928
I agree. We should definitely have it where we can for sure reconsider. I will nudge people for another round and will start a lazy consensus on it while creating those follow-ups :)
--
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
Re: [PR] Add dummy Fernet key for integration tests [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #58112:
URL: https://github.com/apache/airflow/pull/58112#issuecomment-3508760868
Yeah. I think yeah - we should get some good agreement, but for now setting the key seem a good idea ?
--
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
Re: [PR] Add dummy Fernet key for integration tests [airflow]
Posted by "bugraoz93 (via GitHub)" <gi...@apache.org>.
bugraoz93 commented on code in PR #58112:
URL: https://github.com/apache/airflow/pull/58112#discussion_r2530488524
##########
devel-common/src/tests_common/test_utils/fernet.py:
##########
@@ -0,0 +1,48 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+from __future__ import annotations
+
+from pathlib import Path
+
+import yaml
+from cryptography.fernet import Fernet
+
+
+def generate_fernet_key_string() -> str:
+ """Generate a new Fernet key."""
+ return Fernet.generate_key().decode()
+
+
+def update_environment_variable_from_compose_yaml(file_path: str | Path) -> None:
Review Comment:
cc: @potiuk @jscheffl, how is it looking now?
We can do the `--skip-mounting-local-volumes` in another PR? What do you 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
Re: [PR] Add dummy Fernet key for integration tests [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #58112:
URL: https://github.com/apache/airflow/pull/58112#issuecomment-3508762103
Added version/tests labels.
--
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
Re: [PR] Add dummy Fernet key for integration tests [airflow]
Posted by "amoghrajesh (via GitHub)" <gi...@apache.org>.
amoghrajesh commented on code in PR #58112:
URL: https://github.com/apache/airflow/pull/58112#discussion_r2512984252
##########
airflow-core/docs/howto/docker-compose/docker-compose.yaml:
##########
@@ -60,7 +60,11 @@ x-airflow-common:
AIRFLOW__DATABASE__SQL_ALCHEMY_CONN: postgresql+psycopg2://airflow:airflow@postgres/airflow
AIRFLOW__CELERY__RESULT_BACKEND: db+postgresql://airflow:airflow@postgres/airflow
AIRFLOW__CELERY__BROKER_URL: redis://:@redis:6379/0
- AIRFLOW__CORE__FERNET_KEY: ''
+ #
+ # Please Do not use this Fernet key in any deployments! Please generate your own key.
+ # This is specifically generated for tests and not as default. It could fail any static level code checks.
+ #
+ AIRFLOW__CORE__FERNET_KEY: 'dGVzdF9rZXlfdGVzdF9rZXlfdGVzdF9rZXlfdGVzdF8='
Review Comment:
Can we just add it in the test bootstrap time in python? Why to really complicate the docker compose when not needed.
--
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
Re: [PR] Add dummy Fernet key for integration tests [airflow]
Posted by "amoghrajesh (via GitHub)" <gi...@apache.org>.
amoghrajesh commented on code in PR #58112:
URL: https://github.com/apache/airflow/pull/58112#discussion_r2512990364
##########
airflow-core/docs/howto/docker-compose/docker-compose.yaml:
##########
@@ -60,7 +60,11 @@ x-airflow-common:
AIRFLOW__DATABASE__SQL_ALCHEMY_CONN: postgresql+psycopg2://airflow:airflow@postgres/airflow
AIRFLOW__CELERY__RESULT_BACKEND: db+postgresql://airflow:airflow@postgres/airflow
AIRFLOW__CELERY__BROKER_URL: redis://:@redis:6379/0
- AIRFLOW__CORE__FERNET_KEY: ''
+ #
+ # Please Do not use this Fernet key in any deployments! Please generate your own key.
+ # This is specifically generated for tests and not as default. It could fail any static level code checks.
+ #
+ AIRFLOW__CORE__FERNET_KEY: 'dGVzdF9rZXlfdGVzdF9rZXlfdGVzdF9rZXlfdGVzdF8='
Review Comment:
Like this for example:
```python
with open(DOCKER_COMPOSE_FILE_PATH) as f:
compose_config = yaml.safe_load(f)
common_env = compose_config["x-airflow-common"].get("environment", {})
common_env.update(docker_compose_env_vars)
```
--
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
Re: [PR] Add dummy Fernet key for integration tests [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk merged PR #58112:
URL: https://github.com/apache/airflow/pull/58112
--
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
Re: [PR] Add dummy Fernet key for integration tests [airflow]
Posted by "jscheffl (via GitHub)" <gi...@apache.org>.
jscheffl commented on PR #58112:
URL: https://github.com/apache/airflow/pull/58112#issuecomment-3508740151
As via slack: I think we should discuss first how we handle this not to have security side-effects
--
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
Re: [PR] Add dummy Fernet key for integration tests [airflow]
Posted by "jscheffl (via GitHub)" <gi...@apache.org>.
jscheffl commented on code in PR #58112:
URL: https://github.com/apache/airflow/pull/58112#discussion_r2519230641
##########
devel-common/src/tests_common/test_utils/fernet.py:
##########
@@ -0,0 +1,48 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+from __future__ import annotations
+
+from pathlib import Path
+
+import yaml
+from cryptography.fernet import Fernet
+
+
+def generate_fernet_key_string() -> str:
+ """Generate a new Fernet key."""
+ return Fernet.generate_key().decode()
+
+
+def update_environment_variable_from_compose_yaml(file_path: str | Path) -> None:
Review Comment:
You can also set the OS-ENV with a variable and it seems to wotk to have `${FERNET_KEY}` as value and compose will resolve from ENV. Also with the bash expression a default can be given like `${FERNET_KEY:-default}` can be given, just no shell expression is allowed. Passing via OS-ENV might be ebtter than re-writing and no stacked compose is needed.
(Otherwise you can also write key/value pairs in a ENV file and load this into compose... but if it is about one key that can be passed in the ENV as well)
--
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
Re: [PR] Add dummy Fernet key for integration tests [airflow]
Posted by "bugraoz93 (via GitHub)" <gi...@apache.org>.
bugraoz93 commented on code in PR #58112:
URL: https://github.com/apache/airflow/pull/58112#discussion_r2516276439
##########
airflow-core/docs/howto/docker-compose/docker-compose.yaml:
##########
@@ -60,7 +60,11 @@ x-airflow-common:
AIRFLOW__DATABASE__SQL_ALCHEMY_CONN: postgresql+psycopg2://airflow:airflow@postgres/airflow
AIRFLOW__CELERY__RESULT_BACKEND: db+postgresql://airflow:airflow@postgres/airflow
AIRFLOW__CELERY__BROKER_URL: redis://:@redis:6379/0
- AIRFLOW__CORE__FERNET_KEY: ''
+ #
+ # Please Do not use this Fernet key in any deployments! Please generate your own key.
+ # This is specifically generated for tests and not as default. It could fail any static level code checks.
+ #
+ AIRFLOW__CORE__FERNET_KEY: 'dGVzdF9rZXlfdGVzdF9rZXlfdGVzdF9rZXlfdGVzdF8='
Review Comment:
Locally worked with airflowctl. I will follow up soon if it fails somewhere
--
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