amoghrajesh opened a new pull request, #55259:
URL: https://github.com/apache/airflow/pull/55259
<!--
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/
-->
## Why?
While working on https://github.com/apache/airflow/pull/54943, at a point I
reached a stage where the shared secrets masker project is dependent on shared
configuration project leading to issues like:
```
#57 1.962 Using Python 3.10.18 environment at: /home/airflow/.local
#57 4.552 × No solution found when resolving dependencies:
#57 4.552 ╰─▶ Because only apache-airflow[aiobotocore]==3.1.0 is
available and
#57 4.552 apache-airflow==3.1.0 depends on
apache-airflow-core==3.1.0, we can
#57 4.552 conclude that all versions of apache-airflow[aiobotocore]
depend on
#57 4.552 apache-airflow-core==3.1.0. (1)
#57 4.552
#57 4.552 Because apache-airflow-shared-configuration was not found
in
#57 4.552 the package registry and apache-airflow-core==3.1.0 depends
#57 4.552 on apache-airflow-shared-configuration, we can conclude
that
#57 4.552 apache-airflow-core==3.1.0 cannot be used.
#57 4.552 And because we know from (1) that all versions of
#57 4.552 apache-airflow[aiobotocore] depend on
apache-airflow-core==3.1.0, we
#57 4.552 can conclude that all versions of
apache-airflow[aiobotocore] cannot
#57 4.552 be used.
#57 4.552 And because you require apache-airflow[aiobotocore], we
can conclude
#57 4.552 that your requirements are unsatisfiable.
```
In the CI because now airflow-core will have to have dependencies of shared
projects in it. On inspecting it further, turns out we do not need to worry so
much about it, it is only for:
```
min_length_to_mask = conf.getint("logging", "min_length_masked_secret",
fallback=5)
secret_mask_adapter = conf.getimport("logging", "secret_mask_adapter",
fallback=None)
sensitive_fields = DEFAULT_SENSITIVE_FIELDS.copy()
sensitive_variable_fields = conf.get("core", "sensitive_var_conn_names")
```
These are simple configurations that could instead be injected while /
before creating a working instance of secrets masker, for both core as well as
task sdk. So my aim here is to decouple secrets masker from using conf and
eliminate circular dependencies in core and shared projects.
## Approach
1. Removed Configuration Dependencies:
- Removed all `conf.getint()`, `conf.get()`, and `conf.getimport()` calls
from `SecretsMasker` class
- Removed helper functions: `get_min_secret_length()`,
`get_sensitive_variables_fields()`, `_mask_adapter` property
- Eliminated direct import of `airflow.configuration.conf`
2. Added Configuration Properties:
- Added `min_length_to_mask: int = 5` property to
`SecretsMasker.__init__()`
- Added `sensitive_variables_fields: list[str] = []` property
- Added `secret_mask_adapter: Callable | None = None` property
3. Central Configuration:
- Created `_configure_secrets_masker()` function in
`airflow-core/src/airflow/settings.py`
- Configuration is now injected during Airflow initialization
- Both core and task-sdk secrets masker instances are configured with the
same values
## User impact?
- All public APIs remain unchanged (`mask_secret()`, `redact()`,
`should_hide_value_for_key()`, etc.)
- Configuration behavior is identical to before
- No breaking changes for end users
## Testing
- ✅ All existing unit tests pass
In addition to that ran some additional tests for peace of mind:
### Test 1: Basic masking
```
from airflow_shared.secrets_masker import mask_secret, redact
print('mask_secret test:', mask_secret('password123'))
print('redact test:', redact('My password is password123'))
mask_secret test: None
redact test: My password is ***
```
### Test 2: Configuration properties work
```
from airflow_shared.secrets_masker import SecretsMasker,
should_hide_value_for_key, _secrets_masker
masker = _secrets_masker()
masker.sensitive_variables_fields = ['password', 'secret']
masker.min_length_to_mask = 3
print('masker min_length_to_mask:', masker.min_length_to_mask)
print('should_hide_value_for_key(\"password\"):',
should_hide_value_for_key('password'))
print('Adding mask for abcd with name=password...')
masker.add_mask('abcd', 'password')
print('Patterns set:', masker.patterns)
print('Redacting with abcd:', masker.redact('Secret is abcd'))
masker min_length_to_mask: 3
should_hide_value_for_key("password"): True
Adding mask for abcd with name=password...
Patterns set: {'password123', 'abcd', 'secret123'}
Redacting with abcd: Secret is ***
```
## Test 3: Simple integration test that tests basic functionality + config +
deleting config module from sys.modules and testing it
```
import os
import sys
def test_basic_functionality():
print("=== Test 1: Basic Functionality ===")
from airflow_shared.secrets_masker import mask_secret, redact,
SecretsMasker
print('mask_secret("password123"):', mask_secret('password123'))
print('mask_secret("short"):', mask_secret('short'))
print('redact("My password is password123"):', redact('My password is
password123'))
masker = SecretsMasker()
masker.add_mask('secret123', 'password')
print('Instance redact:', masker.redact('The password is secret123'))
def test_configuration_properties():
print("\n=== Test 2: Configuration Properties ===")
from airflow_shared.secrets_masker import SecretsMasker
masker = SecretsMasker()
print('Default min_length_to_mask:', masker.min_length_to_mask)
print('Default sensitive_variables_fields:',
masker.sensitive_variables_fields)
masker.min_length_to_mask = 3
masker.sensitive_variables_fields = ['password', 'secret']
print('After config - min_length_to_mask:', masker.min_length_to_mask)
print('After config - sensitive_variables_fields:',
masker.sensitive_variables_fields)
masker.add_mask('abc', 'test')
print('Mask short secret with min_length=3:', masker.redact('Secret is
abc'))
def test_no_config_dependency():
print("\n=== Test 3: No Configuration Dependency ===")
if 'airflow.configuration' in sys.modules:
del sys.modules['airflow.configuration']
try:
from airflow_shared.secrets_masker import SecretsMasker, mask_secret
print('✅ Successfully imported without airflow.configuration')
print('✅ mask_secret works:', mask_secret('test123'))
except Exception as e:
print('❌ Failed:', e)
if __name__ == '__main__':
test_basic_functionality()
test_configuration_properties()
test_no_config_dependency()
print("\n✅ All manual tests completed!")
```
Output:
```
root@6bcaaa393bb6:/opt/airflow# python comprehensive.py
=== Test 1: Basic Functionality ===
mask_secret("password123"): None
mask_secret("short"): None
redact("My password is password123"): My password is ***
Instance redact: The password is secret123
=== Test 2: Configuration Properties ===
Default min_length_to_mask: 5
Default sensitive_variables_fields: []
After config - min_length_to_mask: 3
After config - sensitive_variables_fields: ['password', 'secret']
Mask short secret with min_length=3: Secret is abc
=== Test 3: No Configuration Dependency ===
✅ Successfully imported without airflow.configuration
✅ mask_secret works: None
✅ All manual tests completed!
```
<!-- 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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
