Skip to content

Conversation

@vchiapaikeo
Copy link
Contributor

@vchiapaikeo vchiapaikeo commented Apr 17, 2023

Created in response to a stale PR at the direction of @eladkal: #28846

This was created to address issues that occurred in a FIPS enabled environment. This allows you to set the CACHING_HASH_METHOD config value to override the default of md5. Also updates the serialized_dag hash code to work even in a FIPS environment.

Testing:

Started up breeze and ensured webserver started fine:

image

Inspected serialized dag dag_hash field:

image

Modified the hash method to sha256 and restarted breeze with this configuration in files/airflow-breeze-config/init.sh:

export AIRFLOW__WEBSERVER__CACHING_HASH_METHOD=sha256
export AIRFLOW__WEBSERVER__EXPOSE_CONFIG=True

Checked configuration:

image

Rechecked dag hash field:

image

Test a simple dag:

image

To explicit md5:

export AIRFLOW__WEBSERVER__CACHING_HASH_METHOD=md5
export AIRFLOW__WEBSERVER__EXPOSE_CONFIG=True

image

image


^ 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.

@potiuk
Copy link
Member

potiuk commented Apr 17, 2023

Nice, thanks for picking this one up!

@potiuk potiuk merged commit 522083c into apache:main Apr 17, 2023
caching_hash_method:
description: |
The caching algorithm used by the webserver. Must be a valid hashlib function name.
version_added:
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess we should put 2.7.0 here?

Copy link
Member

Choose a reason for hiding this comment

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

Unless @ephraimbuddy will decide to merge in to 2.6.0 ?

Copy link
Member

Choose a reason for hiding this comment

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

I know I wrote on slack that it is 2.7 - but on a hindsight - unless it's not too late - that one looks super safe

Copy link
Member

Choose a reason for hiding this comment

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

Ah. I see it is missing at all :(

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah whoops! Let me know if I can do a fast follow up here to add 2.7.0 here @potiuk

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's tag it 2.6.0. I will no longer do the RC today

Copy link
Member

Choose a reason for hiding this comment

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

Yeah. Can you please add 2.6.0 added PR @vchiapaikeo :)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, not a problem. PR: #30688

@potiuk potiuk added this to the Airflow 2.6.0 milestone Apr 17, 2023
wookiist pushed a commit to wookiist/airflow that referenced this pull request Apr 19, 2023
@ephraimbuddy ephraimbuddy added the type:improvement Changelog: Improvements label Apr 21, 2023
ephraimbuddy pushed a commit that referenced this pull request Apr 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:serialization area:webserver Webserver related Issues type:improvement Changelog: Improvements

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants