Skip to content

Conversation

@potiuk
Copy link
Member

@potiuk potiuk commented Nov 13, 2023

Move prepare-provider-packages to be run entirely in Breeze Python

This is a follow-up after #35586 and it depends on this one. It
moves the whole functionality of preparing provider packages to
breeze, removing the need of doing it in the Breeze CI image.

Since we have Python breeze with its own environment managed via
pipx we can now make sure that all the necessary packages are
installed in this environment and run package building in the
same environment Breeze uses.

Previously we have been running all the package building inside the
CI image for two reasons:

we could rely on the same version of build tools (wheel/setuptools)
being installed in the CI image
security of the provider package preparation that used setuptools
pre PEP-517 way of building packages that executed setup.py code
In order to isolate execution of potentially arbitrary code
in setup.py from the HOST environment in CI - where the host
environment might have access to secrets and tokens that would allow
it to break out of the sandbox for PRs coming from forks. The setup.py
file has been prepared by breeze using JINJA templates but it was
potentially possible to manipulate provider package directory structure
and get "Python" injection into generated setup.py, so it was safer
to run it in the isolated Breeze CI environment.

This PR makes it secure to run it in the Host environment,
because instead of generating setup.cfg and setup.py we generate
pyproject.toml with all the necessary information and we are using
PEP-517 compliant way of building provider packages - no arbitrary
code executed via setup.py is possible this way on the host,
so we can safely build provider packages in the host. We are
generating declarative pyproject.toml for that rather than imperative
setup.py, so we are safe to run the build process in the host without
being afraid of executing arbitrary code.

We are using flit as build tool - this is one of the popular build
tools - created by Python Packaging team. It is simple and not
too opinionated, it supports PEP-517 as well as PEP-621, so most of
the project mnetadata in pyproject toml can be added to PEP-621
compliant "project" section of pyproject.toml.

Together with the change we improves the process of generation of the
extracted sources for the providers. Originally we copied the whole
sources of Airflow to a single directory (provider_packages) and run
sequentially provider packages building from that single directory,
however it made it impossible to parallelise such builds - all
providers had to be built sequentially.

We change the approach now - instead of copying all airflow
sources once to the single directory, we build providers in separate
subdirectories of files/provider_packages/PROVIDER_ID and we only
copy there relevant sources (i.e. only provider's subfolder from
the "airflow/providers". This is quite a bit faster (each provider
only gets built using only its own sources so just scanning the
directory is faster) but it also allows to run package preparation
in parallel because each provider is fully isolated from others.

This PR also excludes not-needed prepare_providers_package.py
and unneded provider_packages folder used to prepare providers
before as well as bash script to build the providers and some
unused bash functions.

This change requires #35586 to be merged first - only last commit
counts.


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

potiuk commented Nov 13, 2023

cc: @uranusjr -> this is the second step of our simplifying and moving release preparation to be fully done in breeze. This one replaces the old setuptools build mechanism for flit. We got rid of setup.py in provider and switch the build/packaging to be PEP-517 and PEP-621 compliant.

I will later follo similar changes for Airflow as part of better security in our release process.

@potiuk potiuk force-pushed the move-prepare-providers-packages-to-breeze branch from d75cd72 to 2643b8c Compare November 13, 2023 22:58
@potiuk
Copy link
Member Author

potiuk commented Nov 13, 2023

Another side effect: preparing provider packages in CI dow from 8 miniutes to little over 1 minute.

@potiuk potiuk force-pushed the move-prepare-providers-packages-to-breeze branch 2 times, most recently from 68c3669 to 1219f8c Compare November 14, 2023 02:52
@potiuk
Copy link
Member Author

potiuk commented Nov 14, 2023

Right. I am pretty happy with what I came up with :). I hope we will get it reviewed and merged soon (after #35586) as it brings quite some improvements in speed and "supply chain" side of things when it comes to release management (more about it after I complete this quest).

Seems I got it working nicely. Just to make it easier for review and verification you can take a look at PROD images in this PR which are built using provider packages prepared in this PR. They pass all the tests in CI (incliding discovery of of providers from installed packages, discovery of plugins etc). We will need to run more test when the providers are first released using this mechanism but it generally looks good. You can pull the images and run bash script inside those images easily with this (these are AMD images so will run slow on ARM /Mac OS).:

docker run -it ghcr.io/apache/airflow/main/prod/python3.8:1219f8c13febe13271a5e109ad649c76c6c5a3fc bash

You will see that provider packages are installed from locally builld packages and they are all nicely discoverable and working,,

image

@uranusjr - i'd appreciate if you can take a look - maybe you will have some advice and keen eye reviewing the pyproject.toml template I came up with. You can also see generated pyproject.toml and the sources where packages are generated from easily with this PR. Just run this:

breeze release-management prepare-provider-packages --skip-tag-check --skip-deleting-generated-files --clean-dist --package-format both

The command above will create all provider packages in dist , but it will also leave all the generated and copied code in the dist/provider_packages (in a structure reflecting our provider structure) so that you can see where the packages were generated from. You will find generated pyproject.toml for every provider there (and no setup.py/setup.cfg).

I compared the provider packages before (built with setuptools) and after (built with flit) and I think I got them all right (I will do a more complete comparision before merging that one - on complete set of providers).

They are not identical but I believe those packages have everything needed:

  • entrypoints (plugins/provider_info) are good
  • non-python files are added automatically to both wheel and sdist (which is nice - no more need for MANIFEST.in and setup.cfg different ways of specifying them)
  • One change that I did - I added LICENCE file to the package instead of adding them to Metadata. I could not find a way on how to add extra file to metadata of wheel package with flit, but I think adding them as file in "airflow/providers/PROVIDER_ID> package is much more appropriate way of adding LICENCE as it remains in the package after being installed and you can always see it there. @uranusjr - any comments/insights are appreciated

Also the way packages are being generated in CI and locally when release manager generates them are a little nicer, it's easier to see what's going on:

Generating "PROD" packages:

image

Summary:

image

As mentioned elsewhere with changing to flit/pyproject.toml (which allowed us to move it out of docker environment to build the packages) the generation of all providers is WAAAAAAAY faster now:

Before: 13m58s

image

After: 21s (!!!!!):

image

@potiuk
Copy link
Member Author

potiuk commented Nov 15, 2023

Rebased the next one from "improve release process" which should be quite a but easier to review:

Screenshot 2023-11-15 at 03 41 27

@potiuk potiuk force-pushed the move-prepare-providers-packages-to-breeze branch 4 times, most recently from b538ad0 to 3fe5ea7 Compare November 15, 2023 18:24
@potiuk
Copy link
Member Author

potiuk commented Nov 15, 2023

All right. This looks good - added quite a few more test cases. But I thin it will be green :D

@potiuk potiuk force-pushed the move-prepare-providers-packages-to-breeze branch from 3fe5ea7 to 9410c20 Compare November 15, 2023 21:07
@potiuk potiuk force-pushed the move-prepare-providers-packages-to-breeze branch 2 times, most recently from 829916c to 736702b Compare November 16, 2023 15:35
@potiuk potiuk force-pushed the move-prepare-providers-packages-to-breeze branch 2 times, most recently from c474c13 to ace8031 Compare November 16, 2023 17:02
@potiuk
Copy link
Member Author

potiuk commented Nov 16, 2023

Would love some more reviews on that one (I know it's a lot but mostly a lot to remove)

@potiuk potiuk force-pushed the move-prepare-providers-packages-to-breeze branch from ace8031 to 7bda271 Compare November 16, 2023 22:34
@potiuk
Copy link
Member Author

potiuk commented Nov 17, 2023

🙏

This is a follow-up after #35586 and it depends on this one. It
moves the whole functionality of preparing provider packages to
breeze, removing the need of doing it in the Breeze CI image.

Since we have Python breeze with its own environment managed via
`pipx` we can now make sure that all the necessary packages are
installed in this environment and run package building in the
same environment Breeze uses.

Previously we have been running all the package building inside the
CI image for two reasons:

* we could rely on the same version of build tools (wheel/setuptools)
  being installed in the CI image
* security of the provider package preparation that used setuptools
  pre PEP-517 way of building packages that executed setup.py code

In order to isolate execution of potentially arbitrary code
in setup.py from the HOST environment in CI - where the host
environment might have access to secrets and tokens that would allow
it to break out of the sandbox for PRs coming from forks. The setup.py
file has been prepared by breeze using JINJA templates but it was
potentially possible to manipulate provider package directory structure
and get "Python" injection into generated setup.py, so it was safer
to run it in the isolated Breeze CI environment.

This PR makes it secure to run it in the Host environment,
because instead of generating setup.cfg and setup.py we generate
pyproject.toml with all the necessary information and we are using
PEP-517 compliant way of building provider packages - no arbitrary
code executed via setup.py is possible this way on the host,
so we can safely build provider packages in the host. We are
generating declarative pyproject.toml for that rather than imperative
setup.py, so we are safe to run the build process in the host without
being afraid of executing arbitrary code.

We are using flit as build tool - this is one of the popular build
tools - created by Python Packaging team. It is simple and not
too opinionated, it supports PEP-517 as well as PEP-621, so most of
the project mnetadata in pyproject toml can be added to PEP-621
compliant "project" section of pyproject.toml.

Together with the change we improves the process of generation of the
extracted sources for the providers. Originally we copied the whole
sources of Airflow to a single directory (provider_packages) and run
sequentially provider packages building from that single directory,
however it made it impossible to parallelise such builds - all
providers had to be built sequentially.

We change the approach now - instead of copying all airflow
sources once to the single directory, we build providers in separate
subdirectories of files/provider_packages/PROVIDER_ID and we only
copy there relevant sources (i.e. only provider's subfolder from
the "airflow/providers". This is quite a bit faster (each provider
only gets built using only its own sources so just scanning the
directory is faster) but it also allows to run package preparation
in parallel because each provider is fully isolated from others.

This PR also excludes not-needed `prepare_providers_package.py`
and unneded `provider_packages` folder used to prepare providers
before as well as bash script to build the providers and some
unused bash functions.
@potiuk potiuk force-pushed the move-prepare-providers-packages-to-breeze branch from 7bda271 to 4637379 Compare November 18, 2023 15:39
@potiuk
Copy link
Member Author

potiuk commented Nov 18, 2023

Rebased / fixed conflicts after @Taragolis changes in provider_documentaion . Still looking for reviews :)

@potiuk
Copy link
Member Author

potiuk commented Nov 18, 2023

Just random docker failure. Merging :).

@potiuk potiuk merged commit e18b519 into main Nov 18, 2023
potiuk added a commit to potiuk/airflow that referenced this pull request Nov 19, 2023
This folder has been removed as part of apache#35617 but has not been
removed from bind mount list.
potiuk added a commit that referenced this pull request Nov 19, 2023
This folder has been removed as part of #35617 but has not been
removed from bind mount list.
@ephraimbuddy ephraimbuddy added the changelog:skip Changes that should be skipped from the changelog (CI, tests, etc..) label Nov 20, 2023
@ephraimbuddy ephraimbuddy added this to the Airflow 2.8.0 milestone Nov 20, 2023
@potiuk potiuk deleted the move-prepare-providers-packages-to-breeze branch November 21, 2023 09:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:dev-tools area:helm-chart Airflow Helm Chart area:UI Related to UI/UX. For Frontend Developers. area:webserver Webserver related Issues changelog:skip Changes that should be skipped from the changelog (CI, tests, etc..)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants