Skip to content

Comments

Block tarball members pointing outside tarball#1831

Merged
jeremybeard merged 3 commits intomainfrom
no-outside-tarball-refs
Apr 8, 2025
Merged

Block tarball members pointing outside tarball#1831
jeremybeard merged 3 commits intomainfrom
no-outside-tarball-refs

Conversation

@jeremybeard
Copy link
Contributor

@jeremybeard jeremybeard commented Apr 7, 2025

Description

This changes DAG deploys so that the bundle tarball members cannot symlink to files outside of the tarball. Such external files are not included in DAG deploys and so are not available on the Airflow containers.

This check is only added for deploys to Airflow 3 deployments because in Airflow 3 DAG deploys do not support external symlinks and we want to block those at the source. It is possible that bad symlinks are being deployed to existing Airflow 2 deployments and we do not want to unnecessarily break those deploys.

🎟 Issue(s)

Resolves #1825

🧪 Functional Testing

  • Added unit tests
  • Manually checked with symlink in dags dir

📸 Screenshots

Screenshot 2025-04-07 at 3 30 52 PM

📋 Checklist

  • Rebased from the main (or release if patching) branch (before testing)
  • Ran make test before taking out of draft
  • Ran make lint before taking out of draft
  • Added/updated applicable tests
  • Tested against Astro-API (if necessary).
  • Tested against Houston-API and Astronomer (if necessary).
  • Communicated to/tagged owners of respective clients potentially impacted by these changes.
  • Updated any related documentation

Comment on lines 279 to 280
// only validate symlinks for Airflow 3.x (i.e. don't break backwards compatibility with Airflow 2.x deployments)
validateSymlinks := airflowversions.AirflowMajorVersionForRuntimeVersion(deployInfo.currentVersion) == "3"
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: maybe we could move this outside the if/else block to save ourself from repeating

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah good call. I've centralized it in UploadBundle

@jeremybeard jeremybeard merged commit 9df1a48 into main Apr 8, 2025
3 of 4 checks passed
@jeremybeard jeremybeard deleted the no-outside-tarball-refs branch April 8, 2025 13:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Block Airflow 3 DAG deploys where tarball members point outside of tarball

2 participants