Avoid circular import problems when instantiating AWS SM backend

[Taragolis]

Fix circular import problems when instantiating AWS SecretsManagerBackend in apache-airflow-providers-amazon==5.0.0

before

❯ export AIRFLOW__SECRETS__BACKEND=airflow.providers.amazon.aws.secrets.secrets_manager.SecretsManagerBackend
❯ airflow version

cannot import name 'STATE_COLORS' from partially initialized module 'airflow.settings' (most likely due to a circular import) (/Users/taragolis/Projects/common/airflow/airflow/settings.py)
Traceback (most recent call last):
  File "/Users/taragolis/Projects/common/airflow/airflow/configuration.py", line 689, in getimport
    return import_string(full_qualified_path)
  File "/Users/taragolis/Projects/common/airflow/airflow/utils/module_loading.py", line 32, in import_string
    module = import_module(module_path)
  File "/Users/taragolis/.pyenv/versions/3.9.10/lib/python3.9/importlib/__init__.py", line 127, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
  File "<frozen importlib._bootstrap>", line 1030, in _gcd_import
  File "<frozen importlib._bootstrap>", line 1007, in _find_and_load
  File "<frozen importlib._bootstrap>", line 986, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 680, in _load_unlocked
  File "<frozen importlib._bootstrap_external>", line 850, in exec_module
  File "<frozen importlib._bootstrap>", line 228, in _call_with_frames_removed
  File "/Users/taragolis/Projects/common/airflow/airflow/providers/amazon/aws/secrets/secrets_manager.py", line 27, in <module>
    from airflow.models.connection import Connection
  File "/Users/taragolis/Projects/common/airflow/airflow/models/__init__.py", line 22, in <module>
    from airflow.models.baseoperator import BaseOperator, BaseOperatorLink
  File "/Users/taragolis/Projects/common/airflow/airflow/models/baseoperator.py", line 75, in <module>
    from airflow.models.mappedoperator import OperatorPartial, validate_mapping_kwargs
  File "/Users/taragolis/Projects/common/airflow/airflow/models/mappedoperator.py", line 71, in <module>
    from airflow.models.pool import Pool
  File "/Users/taragolis/Projects/common/airflow/airflow/models/pool.py", line 26, in <module>
    from airflow.ti_deps.dependencies_states import EXECUTION_STATES
  File "/Users/taragolis/Projects/common/airflow/airflow/ti_deps/dependencies_states.py", line 18, in <module>
    from airflow.utils.state import State
  File "/Users/taragolis/Projects/common/airflow/airflow/utils/state.py", line 22, in <module>
    from airflow.settings import STATE_COLORS
ImportError: cannot import name 'STATE_COLORS' from partially initialized module 'airflow.settings' (most likely due to a circular import) (/Users/taragolis/Projects/common/airflow/airflow/settings.py)

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/Users/taragolis/.pyenv/versions/3.9.10/envs/airflow-dev-env-39/bin/airflow", line 33, in <module>
    sys.exit(load_entry_point('apache-airflow', 'console_scripts', 'airflow')())
  File "/Users/taragolis/.pyenv/versions/3.9.10/envs/airflow-dev-env-39/bin/airflow", line 25, in importlib_load_entry_point
    return next(matches).load()
  File "/Users/taragolis/.pyenv/versions/3.9.10/lib/python3.9/importlib/metadata.py", line 77, in load
    module = import_module(match.group('module'))
  File "/Users/taragolis/.pyenv/versions/3.9.10/lib/python3.9/importlib/__init__.py", line 127, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
  File "<frozen importlib._bootstrap>", line 1030, in _gcd_import
  File "<frozen importlib._bootstrap>", line 1007, in _find_and_load
  File "<frozen importlib._bootstrap>", line 972, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 228, in _call_with_frames_removed
  File "<frozen importlib._bootstrap>", line 1030, in _gcd_import
  File "<frozen importlib._bootstrap>", line 1007, in _find_and_load
  File "<frozen importlib._bootstrap>", line 986, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 680, in _load_unlocked
  File "<frozen importlib._bootstrap_external>", line 850, in exec_module
  File "<frozen importlib._bootstrap>", line 228, in _call_with_frames_removed
  File "/Users/taragolis/Projects/common/airflow/airflow/__init__.py", line 35, in <module>
    from airflow import settings
  File "/Users/taragolis/Projects/common/airflow/airflow/settings.py", line 35, in <module>
    from airflow.configuration import AIRFLOW_HOME, WEBSERVER_CONFIG, conf  # NOQA F401
  File "/Users/taragolis/Projects/common/airflow/airflow/configuration.py", line 1649, in <module>
    secrets_backend_list = initialize_secrets_backends()
  File "/Users/taragolis/Projects/common/airflow/airflow/configuration.py", line 1571, in initialize_secrets_backends
    custom_secret_backend = get_custom_secret_backend()
  File "/Users/taragolis/Projects/common/airflow/airflow/configuration.py", line 1546, in get_custom_secret_backend
    secrets_backend_cls = conf.getimport(section='secrets', key='backend')
  File "/Users/taragolis/Projects/common/airflow/airflow/configuration.py", line 692, in getimport
    raise AirflowConfigException(
airflow.exceptions.AirflowConfigException: The object could not be loaded. Please check "backend" key in "secrets" section. Current value: "airflow.providers.amazon.aws.secrets.secrets_manager.SecretsManagerBackend"

after

❯ export AIRFLOW__SECRETS__BACKEND=airflow.providers.amazon.aws.secrets.secrets_manager.SecretsManagerBackend
❯ airflow version

2.4.0.dev0

cc: @o-nikolas @ferruzzi @vincbeck @potiuk

[Taragolis]

Personally I have no idea how to catch such an issue, rather than just manually run some simple command for all known SecretsBackends:

❯ export AIRFLOW__SECRETS__BACKEND=full.qualified.name.to.SB
❯ export AIRFLOW__SECRETS__BACKEND_KWARGS='{}'
❯ airflow version

[uranusjr]

There’s not really a good way to catch this, mainly due to airflow.models too eagerly imports everything. #24486 would be a good general solution to the problem in general.

[potiuk]

Yeah. We definitely import too much in some core packages.

And we should indeed add secrets manager tests. But I think #24486 is not the solution to this problem. It is a good approach but we have a problem with internal architecture, not with imports.

The real problem is not importing stuff, the problem is that we actually HAVE a circular dependency here and it's because our configuration approach is pretty badly intertwined with other parts of the system in this case we have:

• configuration can read values by delegating to secrets manager
• secrets manager uses configuration to configure itself

While I agree local imports and TYPE_CHECKING can provide some solution to the problem, they are very prone to trigger similar problems.

I think we should finally do something about it - I think ti's a bad approach that two internal components of systems are depending bi-directionally on each other :). The dependency between those should be one way - either conf is using secrets backend or secrets backend using conf. Basically we have secrets -> conf -> secret dependency chain which makes it very easy to turn it into circular import problem.

Probably the best solution is to split out the part that secret REALLY need from conf and make it an "internal conf" (and then conf -> secret -> "internal conf" will be the correct dependency chain that will not lead to accidental triggering of such cricular imports.