Skip to content

Add SSL_MUTUAL_TLS config option to Celery provider for one-way TLS support#64767

Merged
potiuk merged 1 commit intoapache:mainfrom
dandanseo123:fix/celery-ssl-skip-empty-key-cert
Apr 13, 2026
Merged

Add SSL_MUTUAL_TLS config option to Celery provider for one-way TLS support#64767
potiuk merged 1 commit intoapache:mainfrom
dandanseo123:fix/celery-ssl-skip-empty-key-cert

Conversation

@dandanseo123
Copy link
Copy Markdown
Contributor

Adds SSL_MUTUAL_TLS config option (default: True) to the [celery] section.

When False, SSL_KEY and SSL_CERT are skipped from broker_use_ssl, enabling one-way TLS (server verification only via SSL_CACERT). This is needed for setups using AMQP/Redis over TLS with token or password-based auth instead of client certificates.

When True (default), existing behavior is unchanged — SSL_KEY and SSL_CERT are required. A clear error is now raised if they are missing, replacing the previous cryptic [Errno 2] crash from py-amqp attempting to load empty certificate paths.

Supports both AMQP (amqps://) and Redis (rediss://) brokers.

related: #39210
follow-up: #64392


Was generative AI tooling used to co-author this PR?
  • Yes — Claude Code

Generated-by: Claude Code following the guidelines

@potiuk potiuk added the ready for maintainer review Set after triaging when all criteria pass. label Apr 6, 2026
@dandanseo123 dandanseo123 force-pushed the fix/celery-ssl-skip-empty-key-cert branch 3 times, most recently from 9608390 to 6508b71 Compare April 10, 2026 13:20
@kaxil kaxil requested a review from Copilot April 10, 2026 19:55
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Adds a new Celery TLS configuration flag to support one-way TLS (server verification only) while preserving the existing mutual TLS default behavior, plus documentation and tests.

Changes:

  • Introduces [celery] SSL_MUTUAL_TLS (default True) to toggle mutual vs one-way TLS.
  • Updates Celery broker SSL dict generation for AMQP/Redis based on SSL_MUTUAL_TLS, with a clearer error for missing client cert/key.
  • Adds unit tests and updates provider config documentation for the new option and requirements.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File Description
providers/celery/src/airflow/providers/celery/executors/default_celery.py Build broker_use_ssl conditionally based on SSL_MUTUAL_TLS, add clearer validation/warnings.
providers/celery/tests/unit/celery/executors/test_celery_executor.py Adds unit tests for Redis mutual TLS, AMQP missing key/cert error, and AMQP/Redis one-way TLS behavior.
providers/celery/src/airflow/providers/celery/get_provider_info.py Documents ssl_mutual_tls and clarifies when ssl_key/ssl_cert are required.
providers/celery/provider.yaml Adds ssl_mutual_tls config entry and clarifies ssl_key/ssl_cert requirements.

@dandanseo123 dandanseo123 force-pushed the fix/celery-ssl-skip-empty-key-cert branch 2 times, most recently from 955c422 to 654d419 Compare April 11, 2026 02:11
@dandanseo123 dandanseo123 force-pushed the fix/celery-ssl-skip-empty-key-cert branch from 654d419 to e50c1c9 Compare April 11, 2026 02:54
@dandanseo123 dandanseo123 force-pushed the fix/celery-ssl-skip-empty-key-cert branch from e50c1c9 to 43a93a9 Compare April 11, 2026 17:31
Copy link
Copy Markdown
Contributor

@jscheffl jscheffl left a comment

Choose a reason for hiding this comment

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

Oh, even cooler! As next relese cut is anyway in ~1.5 weeks I leave the PR open for another pair of eyes, but if no other maintainer on it I think it is good to merge.

@potiuk potiuk merged commit 58fb2fb into apache:main Apr 13, 2026
92 checks passed
@potiuk
Copy link
Copy Markdown
Member

potiuk commented Apr 13, 2026

Nice! And good feedback!

dandanseo123 pushed a commit to dandanseo123/airflow that referenced this pull request Apr 13, 2026
TestExecutorConf tests used conf.read_string() to set config values
like ssl_active=true without cleanup, leaking state into subsequent
tests. This caused CeleryExecutor SSL validation (added in apache#64767) to
raise ValueError in unrelated test_executor_loader tests.
@dandanseo123
Copy link
Copy Markdown
Contributor Author

@potiuk @jscheffl This PR exposed a pre-existing config leak in TestExecutorConf. I created a fix PR 65123 to address this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants