PyYAML 5.1 change breaking environment variables

[Luzifer]

Given this configuration file:

client:
  hosts:
    - ${HOST}
  port: 443
  # ...

Expected Behavior

HOST variable is read and inserted into configuration as it was before

Actual Behavior

YAML loader issues a warning and then curator tries to resolve ${HOST} (literally) through DNS

Related: yaml/pyyaml#265 - PyYAML 5.1 changed the default loader from Loader (equals UnsafeLoader) to None which is evaluated as FullLoader which then is not able to load env vars.

Steps to Reproduce the Problem

1. Use above configuration
2. Install PyYAML >=5.1
3. Execute using above configuration

Specifications

• Version: 5.6.0
• Platform: Alpine v3.9, Python 3.7.2

Possible fixes

diff --git a/curator/utils.py b/curator/utils.py
index bc79ed5..ef170ae 100644
--- a/curator/utils.py
+++ b/curator/utils.py
@@ -50,7 +50,7 @@ def get_yaml(path):
     yaml.add_constructor('!single', single_constructor)

     try:
-        return yaml.load(read_file(path))
+        return yaml.load(read_file(path), Loader=yaml.UnsafeLoader)
     except yaml.scanner.ScannerError as err:
         print('Unable to read/parse YAML file: {0}'.format(path))
         print(err)

Note: This fix does exactly the opposite of #1105 and would be a collision with that one.

Alternate (not recommended) way to work around: Pin PyYAML to 3.13 which is the latest stable version before 5.1 but also has a high severity CVE-2017-18342.

[perlpunk]

It should actually not be necessary to change to UnsafeLoader, and we will fix that in the next release.
I should note that PyYAML does not know anything about loading environment variables, so I suspect this project adds some custom constructors.
The custom constructors added via yaml.add_constructor currently (in 5.1) go to the wrong default which is UnsafeLoader. This will be fixed in the next release. So it's probably a good idea to wait (not more than a couple of days I hope) for 5.2.

[chathsuom]

it also breaks int variables

2019-03-21 00:08:43,386 ERROR     Schema error: expected int for dictionary value @ data['unit_count']
2019-03-21 00:08:43,386 ERROR     Schema error: Configuration: filter: Location: Action ID "1", action "snapshot", filter #1: {'filtertype': 'age', 'source': 'name', 'direction': 'older', 'timestring': '%Y.%m.%d', 'unit': 'days', 'unit_count': '${ARCHIVE_AFTER_DAYS:10}'}: Bad Value: "${ARCHIVE_AFTER_DAYS:10}", expected int for dictionary value @ data['unit_count']. Check configuration file.

[ricardofbarros]

I'm also experiencing this problem with int variables.

[hyadav5]

I am also facing this issue with int variables.

[ricardofbarros]

@untergeek I can see that the "issue" is fixed in master by locking the PyYAML dep to 3.12. Any ETA to release a curator version with this fix?

[untergeek]

Sticking with 3.12 is going to be a stop-gap. There were some constructor issues in PyYAML 5.1 which are targeted for 5.2, but are not yet released yet, so I can't make things work with 5.x yet. I will try to have a Curator build in time for the 7.0 release. It still won't be Curator v6, but hopefully I can make things work with this aging API, still.

[piteur]

Any plan to fix soon ?
I'm currently using the older release since my house-keeping scripts were failing with the newer release.

[untergeek]

@piteur that implies you installed via pip. You can still use the newer version by also running pip install -U pyyaml==3.12 after you install Curator. It will replace the 5.1 version with 3.12, which Curator will be happy with.

[lzecca78]

thanks @untergeek , you save me from a bloody day.

[bryder]

as a FYI pyyaml 3.12 will not build with python 3.7 when using the C extensions.

Should I open an issue to pin curator to pyyaml-3.13 instead of 3.12?

[untergeek]

Sure. Thanks. The problem for me is that I can’t test Python 3.7 in Travis CI yet, as it isn’t available (or wasn’t last I checked). I was also unaware they released pyyaml 3.13

[untergeek]

This should be addressed in #1596 as there were no necessary changes to the integration tests.