Skip to content

Commit

Permalink
Fix YAML Anchor handling (#516)
Browse files Browse the repository at this point in the history
After we switched to using a more modern version of the YAML library, we ran into a problem where YAML Merge Maps started to fail. Others have seen the same problem:

  * https://github.com/zerwes/hiyapyco/pull/14/files
  * ssato/python-anyconfig#87
  * yaml/pyyaml#64
  * saltstack/salt#2092
  * zerwes/hiyapyco#7

In our case, we need to patch the Amazon-provided `construct_mapping` method to run through the `flatten_mapping()` function.
  • Loading branch information
diranged authored Feb 17, 2022
1 parent 8040c99 commit 5d637a9
Show file tree
Hide file tree
Showing 6 changed files with 53 additions and 41 deletions.
1 change: 1 addition & 0 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ jobs:
- image: circleci/python:3.7
environment:
DRY: true
USER: bogus-string
steps:
- checkout
- save_cache:
Expand Down
39 changes: 0 additions & 39 deletions .travis.yml

This file was deleted.

23 changes: 23 additions & 0 deletions examples/anchors.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
actor: group.Sync
desc: main stage
options:
acts:
- &Stage1
actor: group.Async
desc: stage 1
options:
acts:
- actor: rightscale.server_array.Clone
desc: copy serverA
options: {dest: serverA, source: kingpin-integration-testing}
- actor: rightscale.server_array.Clone
desc: copy serverB
options: {dest: serverB, source: kingpin-integration-testing}
- actor: rightscale.server_array.Clone
desc: copy serverC
options: {dest: serverC, source: kingpin-integration-testing}
- &Stage2
<<: *Stage1
desc: stage 2
- <<: *Stage2
desc: stage 3
8 changes: 8 additions & 0 deletions kingpin/test/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,14 @@ def test_convert_script_to_dict(self):
ret = utils.convert_script_to_dict(instance, {})
self.assertIsInstance(ret, dict)

# Should definitly support YAML with anchors
dirname, filename = os.path.split(os.path.abspath(__file__))
examples = "%s/../../examples" % dirname
simple = "%s/anchors.yaml" % examples
instance = open(simple)
ret = utils.convert_script_to_dict(instance, {})
self.assertIsInstance(ret, dict)

def test_convert_script_to_dict_bad_name(self):
instance = io.StringIO() # Empty buffer will fail json
instance.name = "Somefile.HAHA"
Expand Down
21 changes: 20 additions & 1 deletion kingpin/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,12 @@
import re
import sys
import io
import cfn_tools
from io import IOBase
from json.decoder import JSONDecodeError
import cfn_tools
from cfn_tools.yaml_loader import CfnYamlLoader
from cfn_tools.yaml_loader import construct_mapping as aws_construct_mapping


from tornado import gen
from tornado import ioloop
Expand All @@ -59,6 +62,22 @@
# THREADPOOL_SIZE = 10
# THREADPOOL = futures.ThreadPoolExecutor(THREADPOOL_SIZE)

# https://github.com/yaml/pyyaml/issues/64
# https://github.com/zerwes/hiyapyco/issues/7
#
# The AWS-provided cfn_tools code does not call the `flatten_mapping()`
# function in their constructor_mapping func. Because of this, YAML Merge
# anchors fail to parse. This little hack below overrides the function to make
# sure that the YAML parsing of merged maps works properly.
def construct_mapping(self, node, deep=False):
self.flatten_mapping(node)
mapping = aws_construct_mapping(self, node, deep)
return mapping


# Override the constructor reference for "tag:yaml.org,2002:map" to ours above.
CfnYamlLoader.add_constructor("tag:yaml.org,2002:map", construct_mapping)


def str_to_class(string):
"""Method that converts a string name into a usable Class name
Expand Down
2 changes: 1 addition & 1 deletion kingpin/version.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,4 @@
# Copyright 2018 Nextdoor.com, Inc


__version__ = "2.0.0"
__version__ = "2.0.1"

0 comments on commit 5d637a9

Please sign in to comment.