Self parse user data #306

ttaub · 2017-01-25T02:29:48Z

This PR adds the ability to parse variables referenced in a userdata file.

UserData.yaml

#!/bin/sh
echo Hey, {{name}}

stack.yaml

- name: stack
   variables: 
     name: tom
     UserData: ${file plain:file:/path/to/file}

stack.py - somewhere where userdata needs to be passed in

...
UserData = self.parse_user_data( variables['Userdata'] )
...

this will convert all variables enclosed in {{ ... }} to their respective values and base64 encode the entire string.

…with variables

phobologic · 2017-01-25T22:09:05Z

stacker/blueprints/base.py

+        the data file to base64 after it is processed.
+
+         Args:
+            raw_userdata (str of userdata):


Just str instead of str of userdata. Also, make sure you put the return statement in the docstring as well - see: http://sphinxcontrib-napoleon.readthedocs.io/en/latest/example_google.html

phobologic · 2017-01-25T22:16:57Z

stacker/blueprints/base.py

+        variables = self.get_variables()
+        parameters = self.get_cfn_parameters()
+
+        for match in pattern.finditer(raw_user_data):


Lets go with the standard python string.Template method of templating: https://docs.python.org/3/library/string.html#template-strings

Also, please double check that the parameters aren't availabe in get_variables - seems strange, but worth double checking (@mhahn might know)

I decided to stick to just using a regex because the standard delimiter in the Template strings is $ which is very commonly used in UserData for other purposes. We can subclass the Template class and define our own custom rules for the delimiters, but I would argue that what we have right now is cleaner.

Parameters are in get_variables, however, they are stored as CFNParameter objects. In my new commit I addressed this is a cleaner way.

phobologic · 2017-01-25T22:17:38Z

stacker/blueprints/base.py

+            start_index = match.end()
+
+        user_data.append(raw_user_data[start_index:])
+        res = Join("", user_data)


Since this is just a template, and you're not doing anything w/ Ref, etc, you probably won't need any of this join logic - it'll just be a template.

phobologic · 2017-01-26T21:04:19Z

stacker/blueprints/base.py

+                 refs replaced with their resolved values.
+
+        """
+        pattern = re.compile(r'{{([::|\w]+)}}')


One issue w/ building your own templating engine is that there's no way to escape this with the current implementation, right? Python actually has a second string formatting/templating system that you can use perhaps? https://docs.python.org/3/library/string.html#format-string-syntax

phobologic · 2017-01-26T21:05:50Z

stacker/blueprints/base.py

+
+            key = match.group(1)
+
+            if key in variables:


I wouldn't usually check for the existence of a key - instead you should use try/except:

try: v = variables[key] except KeyError: raise MissingVariable(self.name, key)

Then work with v for the rest of the logic, outside of the block.

phobologic · 2017-01-26T21:06:32Z

stacker/blueprints/base.py

+
+        res += raw_user_data[start_index:]
+
+        return base64.b64encode(res)


It might be better to not encode this at all, letting the user choose whether to encode it either using base64 or the Base64 troposphere method. what do you think?

I think that's fair. They could extend the script if they wanted to.

ttaub · 2017-01-27T17:59:54Z

The way userdata templating works is fixed, it uses the default string template provided by python. The main problem with my previous attempt was the way I was passing in the userdata was as a plain string using the file lookup handler which was passing its output to the default output handler and causing all sorts of problems. Now, the function only requires the path and it reads the file itself.

Example config file: test.yaml

...
UserDataPath: file://tmp.txt
#file:// is required for it to work 
...

Example template file: test.py

...
variables = self.get_variables()
...
userdata=self.read_user_data(variables['UserDataPath'])
...

ttaub · 2017-01-27T22:03:19Z

I can move this to stacker_blueprints, I think it may be a better fit.

phobologic · 2017-01-27T22:14:48Z

stacker/blueprints/base.py

+        It supports referencing template variables to create userdata
+        that's supplemented with information from the data, as commonly
+        required when creating EC2 userdata files. Automatically, encodes
+        the data file to base64 after it is processed.


I don't think this is true any longer, right?

this indentation looks off

phobologic · 2017-01-27T22:15:33Z

stacker/blueprints/base.py

@@ -1,11 +1,13 @@
 import copy
 import hashlib
 import logging
+from string import Template as StringTemplate


I would just use import string here, and then use Template as string.Template later

phobologic · 2017-01-27T22:18:17Z

stacker/blueprints/base.py

+            MissingVariable: Raised when a variable is in the user_data that
+                             is not given in the blueprint
+
+        """


This appears to be over indented - I think the whole docstring block is actually.

phobologic · 2017-01-27T22:18:33Z

stacker/blueprints/base.py

+        the data file to base64 after it is processed.
+
+        Args:
+            raw_user_data (str): the user data with the cloud-init info


missing some Args here.

mwildehahn

looks good overall

mwildehahn · 2017-01-30T04:12:53Z

stacker/blueprints/base.py

+        It supports referencing template variables to create userdata
+        that's supplemented with information from the data, as commonly
+        required when creating EC2 userdata files. Automatically, encodes
+        the data file to base64 after it is processed.


this indentation looks off

mwildehahn · 2017-01-30T04:13:08Z

stacker/blueprints/base.py

@@ -220,7 +222,48 @@ def resolve_variable(var_name, var_def, provided_variable, blueprint_name):
    return value


+def parse_user_data(variables, raw_user_data, blueprint_name):
+    """Translate a userdata file to into the file contents.


this sentence doesn't make sense

mwildehahn · 2017-01-30T04:16:30Z

stacker/blueprints/base.py

+
+    try:
+        res = template.substitute(variable_values)
+    except Exception as e:


we should be catching the exact exception that gets raised

…r_data Keeping branch up to date with release-1.0

phobologic · 2017-01-30T20:42:01Z

stacker/blueprints/base.py

@@ -219,23 +220,28 @@ def resolve_variable(var_name, var_def, provided_variable, blueprint_name):


 def parse_user_data(variables, raw_user_data, blueprint_name):
-    """Translate a userdata file to into the file contents.
+    """Parses the user data file


Maybe Parse the given user data and render it as a template

phobologic · 2017-01-30T20:44:09Z

stacker/blueprints/base.py


    res = ""

    try:
        res = template.substitute(variable_values)
-    except Exception as e:
-        raise MissingVariable(blueprint_name, e)
+    except ValueError:


how does this get raised? Not sure I understand from the exception, seems similar to the MissingVariable?

This gets raised when the placeholder name for the variable in the template is actually invalid. E.g ${100} raises a ValueError because the placeholder name (100) can not only be a number.

This is also explained in the Raises section of the comments.

phobologic · 2017-01-30T20:44:42Z

stacker/exceptions.py

@@ -44,6 +54,14 @@ def __init__(self, blueprint_name, variable_name, *args, **kwargs):
        super(MissingVariable, self).__init__(message, *args, **kwargs)


+class InvalidUserdataParameter(Exception):


Where is this used?

…r_data Up to date

phobologic

Minor changes requested, but this looks good to me.

phobologic · 2017-01-30T21:28:15Z

stacker/blueprints/base.py

+                                    is not valid. E.g ${100} would raise this.
+
+
+        """


This indentation is still off, should be flush with the start of the doc string.

phobologic · 2017-01-30T21:28:54Z

stacker/blueprints/base.py

+        MissingVariable: Raised when a variable is in the user_data that
+                         is not given in the blueprint
+
+        InvalidUserdataPlaceholder: Raised when a placeholder name in user_data


user_data -> raw_user_data - this actually happens in multiple places in the doc string.

phobologic · 2017-01-30T21:32:07Z

stacker/exceptions.py

+class InvalidUserdataPlaceholder(Exception):
+
+    def __init__(self, blueprint_name, *args, **kwargs):
+        message = "Could not parse userdata in blueprint \"%s\". " % (


I'd include the text from the ValueError that raises this - would make it easier to track down the issues:

>>> t.substitute({"100": "blue", "a": "apple"}) Traceback (most recent call last): File "<stdin>", line 1, in <module> File "/usr/local/Cellar/python/2.7.12_1/Frameworks/Python.framework/Versions/2.7/lib/python2.7/string.py", line 176, in substitute return self.pattern.sub(convert, self.template) File "/usr/local/Cellar/python/2.7.12_1/Frameworks/Python.framework/Versions/2.7/lib/python2.7/string.py", line 173, in convert self._invalid(mo) File "/usr/local/Cellar/python/2.7.12_1/Frameworks/Python.framework/Versions/2.7/lib/python2.7/string.py", line 146, in _invalid (lineno, colno)) ValueError: Invalid placeholder in string: line 2, col 1

The line # & col # seem useful. You can get that from .args of the original error.

Self parse user data

Tom Taubkin added 4 commits January 24, 2017 17:04

Added parsing of user data to base blueprint, so that userdata works …

795ebfd

…with variables

Updated comments in parse_user_data

3a19de3

Userdata works with parameters now

46bc428

Added tests for new parsing user data with cfn parameters

bc5676d

phobologic requested changes Jan 25, 2017

View reviewed changes

Reverted back to simply returning a base64 encoded string

b3ffba5

phobologic requested changes Jan 26, 2017

View reviewed changes

Changed custom regex to string templates

9f0f668

phobologic requested changes Jan 27, 2017

View reviewed changes

mwildehahn reviewed Jan 30, 2017

View reviewed changes

Tom Taubkin added 4 commits January 30, 2017 10:43

Merge remote-tracking branch 'origin/release-1.0' into self_parse_use…

0496068

…r_data Keeping branch up to date with release-1.0

Fixed comment formatting and added a new exception for userdata

e7ea56c

Added tests for new exceptions

08f93bd

Minor wording change

e04cf9e

phobologic requested changes Jan 30, 2017

View reviewed changes

Tom Taubkin added 2 commits January 30, 2017 13:07

Merge remote-tracking branch 'origin/release-1.0' into self_parse_use…

63ab875

…r_data Up to date

Better comments and removed unused exception

7c9143f

phobologic approved these changes Jan 30, 2017

View reviewed changes

Tom Taubkin added 2 commits January 30, 2017 14:10

Fixed up comments and better error messages

5cffa64

Pep8

8f52dd2

ttaub merged commit c3fbc36 into release-1.0 Jan 30, 2017

ttaub deleted the self_parse_user_data branch January 30, 2017 22:24

phrohdoh pushed a commit to phrohdoh/stacker that referenced this pull request Dec 18, 2018

Merge pull request cloudtools#306 from remind101/self_parse_user_data

80161c2

Self parse user data

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Self parse user data #306

Self parse user data #306

ttaub commented Jan 25, 2017

phobologic Jan 25, 2017

ttaub Jan 26, 2017

phobologic Jan 25, 2017

ttaub Jan 26, 2017

phobologic Jan 25, 2017

ttaub Jan 26, 2017

phobologic Jan 26, 2017

phobologic Jan 26, 2017

phobologic Jan 26, 2017

ttaub Jan 26, 2017

ttaub commented Jan 27, 2017

ttaub commented Jan 27, 2017

phobologic Jan 27, 2017

mwildehahn Jan 30, 2017

phobologic Jan 27, 2017

phobologic Jan 27, 2017

phobologic Jan 27, 2017

mwildehahn left a comment

mwildehahn Jan 30, 2017

mwildehahn Jan 30, 2017

mwildehahn Jan 30, 2017

phobologic Jan 30, 2017

phobologic Jan 30, 2017

ttaub Jan 30, 2017

ttaub Jan 30, 2017

phobologic Jan 30, 2017

phobologic left a comment

phobologic Jan 30, 2017

phobologic Jan 30, 2017

phobologic Jan 30, 2017


		res += raw_user_data[start_index:]

		return base64.b64encode(res)

		@@ -44,6 +54,14 @@ def __init__(self, blueprint_name, variable_name, args, *kwargs):
		super(MissingVariable, self).__init__(message, args, *kwargs)


		class InvalidUserdataParameter(Exception):

Self parse user data #306

Self parse user data #306

Conversation

ttaub commented Jan 25, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ttaub commented Jan 27, 2017

ttaub commented Jan 27, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mwildehahn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

phobologic left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment