Skip to content

Commit 888bc2e

Browse files
potiukpotiuk
authored
Fix retrieval of deprecated non-config values (#23723)
It turned out that deprecation of config values did not work as intended. While deprecation worked fine when the value was specified in configuration value it did not work when `run_as_user` was used. In those cases the "as_dict" option was used to generate temporary configuratin and this temporary configuration contained default value for the new configuration value - for example it caused that the generated temporary value contained: ``` [database] sql_alchemy_conn=sqlite:///{AIRFLOW_HOME}/airflow.db ``` Even if the deprecated `core/sql_alchemy_conn` was set (and no new `database/sql_alchemy_conn` was set at the same time. This effectively rendered the old installation that did not convert to the new "database" configuration not working for run_as_user, because the tasks run with "run_as_user" used wrong, empty sqlite database instaead of the one configured for Airflow. Also during adding tests, it turned out that the mechanism was also not working as intended before - in case `_CMD` or `_SECRET` were used as environment variables rather than configuration. In those cases both _CMD and _SECRET should be evaluated during as_dict() evaluation, because the "run_as_user" might have not enough permission to run the command or retrieve secret. The _cmd and _secret variables were only evaluated during as_dict() when they were in the config file (note that this only happens when include_cmd, include_env, include_secret are set to True). The changes implemented in this PR fix both problems: * the _CMD and _SECRET env vars are evaluated during as_dict when the respective include_* is set * the defaults are only set for the values that have deprecations in case the deprecations have no values set in either of the ways: * in config file * in env variable * in _cmd (via config file or env variable) * in _secret (via config file or env variable) Fixes: #23679
1 parent ce8ea66 commit 888bc2e

File tree

8 files changed

+849
-189
lines changed

8 files changed

+849
-189
lines changed

‎airflow/config_templates/config.yml.schema.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@
3636
},
3737
"sensitive": {
3838
"type": "boolean",
39-
"description": "When true, this option is sensitive and can be specified using AIRFLOW__{section}___{name}__SECRET or AIRFLOW__{section}___{name}__CMD environment variables. See: airflow.configuration.AirflowConfigParser.sensitive_config_values"
39+
"description": "When true, this option is sensitive and can be specified using AIRFLOW__{section}___{name}__SECRET or AIRFLOW__{section}___{name}_CMD environment variables. See: airflow.configuration.AirflowConfigParser.sensitive_config_values"
4040
}
4141
},
4242
"required": [

‎airflow/configuration.py‎

Lines changed: 164 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,8 @@
5959
ConfigSectionSourcesType = Dict[str, Union[str, Tuple[str, str]]]
6060
ConfigSourcesType = Dict[str, ConfigSectionSourcesType]
6161

62+
ENV_VAR_PREFIX = 'AIRFLOW__'
63+
6264

6365
def _parse_sqlite_version(s: str) -> Tuple[int, ...]:
6466
match = _SQLITE3_VERSION_PATTERN.match(s)
@@ -144,7 +146,7 @@ class AirflowConfigParser(ConfigParser):
144146
"""Custom Airflow Configparser supporting defaults and deprecated options"""
145147

146148
# These configuration elements can be fetched as the stdout of commands
147-
# following the "{section}__{name}__cmd" pattern, the idea behind this
149+
# following the "{section}__{name}_cmd" pattern, the idea behind this
148150
# is to not store password on boxes in text files.
149151
# These configs can also be fetched from Secrets backend
150152
# following the "{section}__{name}__secret" pattern
@@ -435,10 +437,8 @@ def _create_future_warning(name: str, section: str, current_value: Any, new_valu
435437
FutureWarning,
436438
)
437439

438-
ENV_VAR_PREFIX = 'AIRFLOW__'
439-
440440
def _env_var_name(self, section: str, key: str) -> str:
441-
return f'{self.ENV_VAR_PREFIX}{section.upper()}__{key.upper()}'
441+
return f'{ENV_VAR_PREFIX}{section.upper()}__{key.upper()}'
442442

443443
def _get_env_var_option(self, section: str, key: str):
444444
# must have format AIRFLOW__{SECTION}__{KEY} (note double underscore)
@@ -461,23 +461,53 @@ def _get_env_var_option(self, section: str, key: str):
461461

462462
def _get_cmd_option(self, section: str, key: str):
463463
fallback_key = key + '_cmd'
464-
# if this is a valid command key...
465464
if (section, key) in self.sensitive_config_values:
466465
if super().has_option(section, fallback_key):
467466
command = super().get(section, fallback_key)
468467
return run_command(command)
469468
return None
470469

470+
def _get_cmd_option_from_config_sources(
471+
self, config_sources: ConfigSourcesType, section: str, key: str
472+
) -> Optional[str]:
473+
fallback_key = key + '_cmd'
474+
if (section, key) in self.sensitive_config_values:
475+
section_dict = config_sources.get(section)
476+
if section_dict is not None:
477+
command_value = section_dict.get(fallback_key)
478+
if command_value is not None:
479+
if isinstance(command_value, str):
480+
command = command_value
481+
else:
482+
command = command_value[0]
483+
return run_command(command)
484+
return None
485+
471486
def _get_secret_option(self, section: str, key: str) -> Optional[str]:
472487
"""Get Config option values from Secret Backend"""
473488
fallback_key = key + '_secret'
474-
# if this is a valid secret key...
475489
if (section, key) in self.sensitive_config_values:
476490
if super().has_option(section, fallback_key):
477491
secrets_path = super().get(section, fallback_key)
478492
return _get_config_value_from_secret_backend(secrets_path)
479493
return None
480494

495+
def _get_secret_option_from_config_sources(
496+
self, config_sources: ConfigSourcesType, section: str, key: str
497+
) -> Optional[str]:
498+
fallback_key = key + '_secret'
499+
if (section, key) in self.sensitive_config_values:
500+
section_dict = config_sources.get(section)
501+
if section_dict is not None:
502+
secrets_path_value = section_dict.get(fallback_key)
503+
if secrets_path_value is not None:
504+
if isinstance(secrets_path_value, str):
505+
secrets_path = secrets_path_value
506+
else:
507+
secrets_path = secrets_path_value[0]
508+
return _get_config_value_from_secret_backend(secrets_path)
509+
return None
510+
481511
def get_mandatory_value(self, section: str, key: str, **kwargs) -> str:
482512
value = self.get(section, key, **kwargs)
483513
if value is None:
@@ -859,7 +889,16 @@ def as_dict(
859889
('airflow.cfg', self),
860890
]
861891

862-
self._replace_config_with_display_sources(config_sources, configs, display_source, raw)
892+
self._replace_config_with_display_sources(
893+
config_sources,
894+
configs,
895+
display_source,
896+
raw,
897+
self.deprecated_options,
898+
include_cmds=include_cmds,
899+
include_env=include_env,
900+
include_secret=include_secret,
901+
)
863902

864903
# add env vars and overwrite because they have priority
865904
if include_env:
@@ -889,7 +928,7 @@ def _include_secrets(
889928
raw: bool,
890929
):
891930
for (section, key) in self.sensitive_config_values:
892-
value: Optional[str] = self._get_secret_option(section, key)
931+
value: Optional[str] = self._get_secret_option_from_config_sources(config_sources, section, key)
893932
if value:
894933
if not display_sensitive:
895934
value = '< hidden >'
@@ -910,17 +949,20 @@ def _include_commands(
910949
raw: bool,
911950
):
912951
for (section, key) in self.sensitive_config_values:
913-
opt = self._get_cmd_option(section, key)
952+
opt = self._get_cmd_option_from_config_sources(config_sources, section, key)
914953
if not opt:
915954
continue
955+
opt_to_set: Union[str, Tuple[str, str], None] = opt
916956
if not display_sensitive:
917-
opt = '< hidden >'
957+
opt_to_set = '< hidden >'
918958
if display_source:
919-
opt = (opt, 'cmd')
959+
opt_to_set = (str(opt_to_set), 'cmd')
920960
elif raw:
921-
opt = opt.replace('%', '%%')
922-
config_sources.setdefault(section, OrderedDict()).update({key: opt})
923-
del config_sources[section][key + '_cmd']
961+
opt_to_set = str(opt_to_set).replace('%', '%%')
962+
if opt_to_set is not None:
963+
dict_to_update: Dict[str, Union[str, Tuple[str, str]]] = {key: opt_to_set}
964+
config_sources.setdefault(section, OrderedDict()).update(dict_to_update)
965+
del config_sources[section][key + '_cmd']
924966

925967
def _include_envs(
926968
self,
@@ -930,7 +972,7 @@ def _include_envs(
930972
raw: bool,
931973
):
932974
for env_var in [
933-
os_environment for os_environment in os.environ if os_environment.startswith(self.ENV_VAR_PREFIX)
975+
os_environment for os_environment in os.environ if os_environment.startswith(ENV_VAR_PREFIX)
934976
]:
935977
try:
936978
_, section, key = env_var.split('__', 2)
@@ -1010,13 +1052,82 @@ def _replace_config_with_display_sources(
10101052
configs: Iterable[Tuple[str, ConfigParser]],
10111053
display_source: bool,
10121054
raw: bool,
1055+
deprecated_options: Dict[Tuple[str, str], Tuple[str, str, str]],
1056+
include_env: bool,
1057+
include_cmds: bool,
1058+
include_secret: bool,
10131059
):
10141060
for (source_name, config) in configs:
10151061
for section in config.sections():
10161062
AirflowConfigParser._replace_section_config_with_display_sources(
1017-
config, config_sources, display_source, raw, section, source_name
1063+
config,
1064+
config_sources,
1065+
display_source,
1066+
raw,
1067+
section,
1068+
source_name,
1069+
deprecated_options,
1070+
configs,
1071+
include_env=include_env,
1072+
include_cmds=include_cmds,
1073+
include_secret=include_secret,
10181074
)
10191075

1076+
@staticmethod
1077+
def _deprecated_value_is_set_in_config(
1078+
deprecated_section: str,
1079+
deprecated_key: str,
1080+
configs: Iterable[Tuple[str, ConfigParser]],
1081+
) -> bool:
1082+
for config_type, config in configs:
1083+
if config_type == 'default':
1084+
continue
1085+
try:
1086+
deprecated_section_array = config.items(section=deprecated_section, raw=True)
1087+
for (key_candidate, _) in deprecated_section_array:
1088+
if key_candidate == deprecated_key:
1089+
return True
1090+
except NoSectionError:
1091+
pass
1092+
return False
1093+
1094+
@staticmethod
1095+
def _deprecated_variable_is_set(deprecated_section: str, deprecated_key: str) -> bool:
1096+
return (
1097+
os.environ.get(f'{ENV_VAR_PREFIX}{deprecated_section.upper()}__{deprecated_key.upper()}')
1098+
is not None
1099+
)
1100+
1101+
@staticmethod
1102+
def _deprecated_command_is_set_in_config(
1103+
deprecated_section: str, deprecated_key: str, configs: Iterable[Tuple[str, ConfigParser]]
1104+
) -> bool:
1105+
return AirflowConfigParser._deprecated_value_is_set_in_config(
1106+
deprecated_section=deprecated_section, deprecated_key=deprecated_key + "_cmd", configs=configs
1107+
)
1108+
1109+
@staticmethod
1110+
def _deprecated_variable_command_is_set(deprecated_section: str, deprecated_key: str) -> bool:
1111+
return (
1112+
os.environ.get(f'{ENV_VAR_PREFIX}{deprecated_section.upper()}__{deprecated_key.upper()}_CMD')
1113+
is not None
1114+
)
1115+
1116+
@staticmethod
1117+
def _deprecated_secret_is_set_in_config(
1118+
deprecated_section: str, deprecated_key: str, configs: Iterable[Tuple[str, ConfigParser]]
1119+
) -> bool:
1120+
return AirflowConfigParser._deprecated_value_is_set_in_config(
1121+
deprecated_section=deprecated_section, deprecated_key=deprecated_key + "_secret", configs=configs
1122+
)
1123+
1124+
@staticmethod
1125+
def _deprecated_variable_secret_is_set(deprecated_section: str, deprecated_key: str) -> bool:
1126+
return (
1127+
os.environ.get(f'{ENV_VAR_PREFIX}{deprecated_section.upper()}__{deprecated_key.upper()}_SECRET')
1128+
is not None
1129+
)
1130+
10201131
@staticmethod
10211132
def _replace_section_config_with_display_sources(
10221133
config: ConfigParser,
@@ -1025,9 +1136,46 @@ def _replace_section_config_with_display_sources(
10251136
raw: bool,
10261137
section: str,
10271138
source_name: str,
1139+
deprecated_options: Dict[Tuple[str, str], Tuple[str, str, str]],
1140+
configs: Iterable[Tuple[str, ConfigParser]],
1141+
include_env: bool,
1142+
include_cmds: bool,
1143+
include_secret: bool,
10281144
):
10291145
sect = config_sources.setdefault(section, OrderedDict())
10301146
for (k, val) in config.items(section=section, raw=raw):
1147+
deprecated_section, deprecated_key, _ = deprecated_options.get((section, k), (None, None, None))
1148+
if deprecated_section and deprecated_key:
1149+
if source_name == 'default':
1150+
# If deprecated entry has some non-default value set for any of the sources requested,
1151+
# We should NOT set default for the new entry (because it will override anything
1152+
# coming from the deprecated ones)
1153+
if AirflowConfigParser._deprecated_value_is_set_in_config(
1154+
deprecated_section, deprecated_key, configs
1155+
):
1156+
continue
1157+
if include_env and AirflowConfigParser._deprecated_variable_is_set(
1158+
deprecated_section, deprecated_key
1159+
):
1160+
continue
1161+
if include_cmds and (
1162+
AirflowConfigParser._deprecated_variable_command_is_set(
1163+
deprecated_section, deprecated_key
1164+
)
1165+
or AirflowConfigParser._deprecated_command_is_set_in_config(
1166+
deprecated_section, deprecated_key, configs
1167+
)
1168+
):
1169+
continue
1170+
if include_secret and (
1171+
AirflowConfigParser._deprecated_variable_secret_is_set(
1172+
deprecated_section, deprecated_key
1173+
)
1174+
or AirflowConfigParser._deprecated_secret_is_set_in_config(
1175+
deprecated_section, deprecated_key, configs
1176+
)
1177+
):
1178+
continue
10311179
if display_source:
10321180
sect[k] = (val, source_name)
10331181
else:

‎tests/config_templates/deprecated.cfg‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
#
2+
# Licensed to the Apache Software Foundation (ASF) under one
3+
# or more contributor license agreements. See the NOTICE file
4+
# distributed with this work for additional information
5+
# regarding copyright ownership. The ASF licenses this file
6+
# to you under the Apache License, Version 2.0 (the
7+
# "License"); you may not use this file except in compliance
8+
# with the License. You may obtain a copy of the License at
9+
#
10+
# http://www.apache.org/licenses/LICENSE-2.0
11+
#
12+
# Unless required by applicable law or agreed to in writing,
13+
# software distributed under the License is distributed on an
14+
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
# KIND, either express or implied. See the License for the
16+
# specific language governing permissions and limitations
17+
# under the License.
18+
19+
20+
# This is the template for Airflow's unit test configuration. When Airflow runs
21+
# unit tests, it looks for a configuration file at $AIRFLOW_HOME/unittests.cfg.
22+
# If it doesn't exist, Airflow uses this template to generate it by replacing
23+
# variables in curly braces with their global values from configuration.py.
24+
25+
# Users should not modify this file; they should customize the generated
26+
# unittests.cfg instead.
27+
[core]
28+
sql_alchemy_conn = mysql://

‎tests/config_templates/deprecated_cmd.cfg‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
#
2+
# Licensed to the Apache Software Foundation (ASF) under one
3+
# or more contributor license agreements. See the NOTICE file
4+
# distributed with this work for additional information
5+
# regarding copyright ownership. The ASF licenses this file
6+
# to you under the Apache License, Version 2.0 (the
7+
# "License"); you may not use this file except in compliance
8+
# with the License. You may obtain a copy of the License at
9+
#
10+
# http://www.apache.org/licenses/LICENSE-2.0
11+
#
12+
# Unless required by applicable law or agreed to in writing,
13+
# software distributed under the License is distributed on an
14+
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
# KIND, either express or implied. See the License for the
16+
# specific language governing permissions and limitations
17+
# under the License.
18+
19+
20+
# This is the template for Airflow's unit test configuration. When Airflow runs
21+
# unit tests, it looks for a configuration file at $AIRFLOW_HOME/unittests.cfg.
22+
# If it doesn't exist, Airflow uses this template to generate it by replacing
23+
# variables in curly braces with their global values from configuration.py.
24+
25+
# Users should not modify this file; they should customize the generated
26+
# unittests.cfg instead.
27+
28+
[core]
29+
sql_alchemy_conn_cmd = echo -n "postgresql://"

‎tests/config_templates/deprecated_secret.cfg‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
#
2+
# Licensed to the Apache Software Foundation (ASF) under one
3+
# or more contributor license agreements. See the NOTICE file
4+
# distributed with this work for additional information
5+
# regarding copyright ownership. The ASF licenses this file
6+
# to you under the Apache License, Version 2.0 (the
7+
# "License"); you may not use this file except in compliance
8+
# with the License. You may obtain a copy of the License at
9+
#
10+
# http://www.apache.org/licenses/LICENSE-2.0
11+
#
12+
# Unless required by applicable law or agreed to in writing,
13+
# software distributed under the License is distributed on an
14+
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
# KIND, either express or implied. See the License for the
16+
# specific language governing permissions and limitations
17+
# under the License.
18+
19+
20+
# This is the template for Airflow's unit test configuration. When Airflow runs
21+
# unit tests, it looks for a configuration file at $AIRFLOW_HOME/unittests.cfg.
22+
# If it doesn't exist, Airflow uses this template to generate it by replacing
23+
# variables in curly braces with their global values from configuration.py.
24+
25+
# Users should not modify this file; they should customize the generated
26+
# unittests.cfg instead.
27+
28+
[core]
29+
sql_alchemy_conn_secret = secret_path

0 commit comments

Comments
 (0)