Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Undefined conf when using AWS secrets manager backend and sql_alchemy_conn_secret #15685

Closed
vasyharan opened this issue May 5, 2021 · 4 comments · Fixed by #16088
Closed
Labels
kind:bug This is a clearly a bug

Comments

@vasyharan
Copy link

Apache Airflow version: 2.2.0

Kubernetes version (if you are using kubernetes) (use kubectl version): 1.19 (server), 1.21 (client)

Environment:

  • Cloud provider or hardware configuration: AWS EKS
  • OS (e.g. from /etc/os-release): Docker image (apache/airflow:2.0.2-python3.7)
  • Kernel (e.g. uname -a):
  • Install tools:
  • Others:

What happened:

airflow bin fails during configuration initialization (stack below). I see a similar issue reported here: #13254, but my error is slightly different.

  File "/home/airflow/.local/bin/airflow", line 5, in <module>
    from airflow.__main__ import main
  File "/home/airflow/.local/lib/python3.7/site-packages/airflow/__init__.py", line 34, in <module>
    from airflow import settings
  File "/home/airflow/.local/lib/python3.7/site-packages/airflow/settings.py", line 37, in <module>
    from airflow.configuration import AIRFLOW_HOME, WEBSERVER_CONFIG, conf  # NOQA F401
  File "/home/airflow/.local/lib/python3.7/site-packages/airflow/configuration.py", line 1098, in <module>
    conf = initialize_config()
  File "/home/airflow/.local/lib/python3.7/site-packages/airflow/configuration.py", line 860, in initialize_config
    conf.validate()
  File "/home/airflow/.local/lib/python3.7/site-packages/airflow/configuration.py", line 199, in validate
    self._validate_config_dependencies()
  File "/home/airflow/.local/lib/python3.7/site-packages/airflow/configuration.py", line 227, in _validate_config_dependencies
    is_sqlite = "sqlite" in self.get('core', 'sql_alchemy_conn')
  File "/home/airflow/.local/lib/python3.7/site-packages/airflow/configuration.py", line 328, in get
    option = self._get_environment_variables(deprecated_key, deprecated_section, key, section)
  File "/home/airflow/.local/lib/python3.7/site-packages/airflow/configuration.py", line 394, in _get_environment_variables
    option = self._get_env_var_option(section, key)
  File "/home/airflow/.local/lib/python3.7/site-packages/airflow/configuration.py", line 298, in _get_env_var_option
    return _get_config_value_from_secret_backend(os.environ[env_var_secret_path])
  File "/home/airflow/.local/lib/python3.7/site-packages/airflow/configuration.py", line 83, in _get_config_value_from_secret_backend
    secrets_client = get_custom_secret_backend()
  File "/home/airflow/.local/lib/python3.7/site-packages/airflow/configuration.py", line 999, in get_custom_secret_backend
    secrets_backend_cls = conf.getimport(section='secrets', key='backend')
NameError: name 'conf' is not defined

What you expected to happen:

airflow to correctly initialize the configuration.

How to reproduce it:
airflow.cfg

[core]
# ...
sql_alchemy_conn_secret = some-key

# ...
[secrets]
backend = airflow.contrib.secrets.aws_secrets_manager.SecretsManagerBackend
backend_kwargs = {config_prefix: 'airflow/config', connections_prefix: 'airflow/connections', variables_prefix: 'airflow/variables'}

Anything else we need to know:

@vasyharan vasyharan added the kind:bug This is a clearly a bug label May 5, 2021
@boring-cyborg
Copy link

boring-cyborg bot commented May 5, 2021

Thanks for opening your first issue here! Be sure to follow the issue template!

@uranusjr
Copy link
Member

uranusjr commented May 6, 2021

There's a circular reference happening here. The secret backend needs to read configs, but the config wants to read the secret backend during initialisation. Not sure how best to fix this...

@potiuk
Copy link
Member

potiuk commented May 7, 2021

Yeah. We do have a problem with 'conf' causing circular references (not regular circular imports, just references because we have circular dependencies between modules in our code which IMHO should be unentangled and make the dependencies straightforward. It's pretty deeply embedded in the way how configuration is done and the fact that we 'plug-in' the code (like secret backends) to conf which then through chain of dependencies goes back and requires conf (but there are other, similar places with conf being in the middle of circular dependencies).

We have some hacks in the code I solved already quite a few of those in the past (for example here: #13260), but it was more of the band-aid rather than final solution.

Happy to brainstorm virtually and make some attempt to do it @uranusjr if you would like to (but likely something that we can attempt after 2.1 is released). That might also help in finally getting rid of the last pylint_todo.txt entries - which are there mainly because of the conf becoming the reason for circular dependencies (just to be clear - those are circular dependencies, not imports, those are pretty different problems).

@krnhotwings
Copy link

Recently ran into this issue. Adding an additional FYI that this does not occur in 2.0.1

The error seems to only occur when sql_alchemy_conn_secret is set. Does not occur when the following are set: core.fernet_key_secret, webserver.secret_key_secret

potiuk added a commit to potiuk/airflow that referenced this issue May 27, 2021
There was a problem that when we initialized configuration, we've run
validate() which - among others - checkd if the connection is an `sqlite`
but when the SQLAlchemy connection was not configured via variable but
via secret manager, it has fallen back to secret_backend, which should
be configured via conf and initialized.
The problem is that the "conf" object is not yet created, because
the "validate()" method has not finished yet and
"initialize_configuration" has not yet returned.
This led to snake eating its own tail.

This PR defers the validate() method to after secret backends have
been initialized. The effect of it is that secret backends might
be initialized with configuration that is not valid, but there are
no real negative consequences of this.

Fixes: apache#16079
Fixes: apache#15685

starting
potiuk added a commit that referenced this issue May 27, 2021
)

There was a problem that when we initialized configuration, we've run
validate() which - among others - checkd if the connection is an `sqlite`
but when the SQLAlchemy connection was not configured via variable but
via secret manager, it has fallen back to secret_backend, which should
be configured via conf and initialized.
The problem is that the "conf" object is not yet created, because
the "validate()" method has not finished yet and
"initialize_configuration" has not yet returned.
This led to snake eating its own tail.

This PR defers the validate() method to after secret backends have
been initialized. The effect of it is that secret backends might
be initialized with configuration that is not valid, but there are
no real negative consequences of this.

Fixes: #16079
Fixes: #15685

starting
jhtimmins pushed a commit to astronomer/airflow that referenced this issue Jun 3, 2021
…che#16088)

There was a problem that when we initialized configuration, we've run
validate() which - among others - checkd if the connection is an `sqlite`
but when the SQLAlchemy connection was not configured via variable but
via secret manager, it has fallen back to secret_backend, which should
be configured via conf and initialized.
The problem is that the "conf" object is not yet created, because
the "validate()" method has not finished yet and
"initialize_configuration" has not yet returned.
This led to snake eating its own tail.

This PR defers the validate() method to after secret backends have
been initialized. The effect of it is that secret backends might
be initialized with configuration that is not valid, but there are
no real negative consequences of this.

Fixes: apache#16079
Fixes: apache#15685

starting

(cherry picked from commit 65519ab)
ashb pushed a commit that referenced this issue Jun 22, 2021
)

There was a problem that when we initialized configuration, we've run
validate() which - among others - checkd if the connection is an `sqlite`
but when the SQLAlchemy connection was not configured via variable but
via secret manager, it has fallen back to secret_backend, which should
be configured via conf and initialized.
The problem is that the "conf" object is not yet created, because
the "validate()" method has not finished yet and
"initialize_configuration" has not yet returned.
This led to snake eating its own tail.

This PR defers the validate() method to after secret backends have
been initialized. The effect of it is that secret backends might
be initialized with configuration that is not valid, but there are
no real negative consequences of this.

Fixes: #16079
Fixes: #15685

starting

(cherry picked from commit 65519ab)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:bug This is a clearly a bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants