Skip to content

Add list of raw_vars whose values are auto-escaped #615

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Jul 23, 2016
Merged

Add list of raw_vars whose values are auto-escaped #615

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8f5177f
Add list of raw_vars whose values are auto-escaped
fullyint Jul 7, 2016
3fda4a7
Use comprehensions
fullyint Jul 18, 2016
7983ff3
Raise error if raw_vars not defined as list
fullyint Jul 18, 2016
6807515
Template the raw_vars variable before processing
fullyint Jul 18, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions group_vars/all/main.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,3 +17,12 @@ wordpress_env_defaults:
wp_siteurl: "${WP_HOME}/wp"

site_env: "{{ wordpress_env_defaults | combine(item.value.env | default({}), vault_wordpress_sites[item.key].env) }}"

# Values of raw_vars will be wrapped in `{% raw %}` to avoid templating problems if values include `{%` and `{{`.
# Will recurse dicts/lists. `*` is wildcard for one or more dict keys, list indices, or strings. Example:
# - vault_wordpress_sites.*.*_salt -- matches vault_wordpress_sites.example.com.env.secure_auth_salt etc.
# Will not function for var names or topmost dict keys that contain a period ('.').
raw_vars:
- vault_mail_password
- vault_mysql_root_password
- vault_wordpress_sites
42 changes: 33 additions & 9 deletions lib/trellis/plugins/vars/vars.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
from __future__ import (absolute_import, division, print_function)
__metaclass__ = type

import re

from ansible import __version__
from ansible.errors import AnsibleError

Expand All @@ -10,6 +12,9 @@
# These imports will produce Traceback in Ansible 1.x, so place after version check
from __main__ import cli
from ansible.compat.six import iteritems
from ansible.parsing.dataloader import DataLoader
from ansible.parsing.yaml.objects import AnsibleMapping, AnsibleSequence, AnsibleUnicode
from ansible.template import Templar


class VarsModule(object):
Expand All @@ -18,16 +23,35 @@ class VarsModule(object):
def __init__(self, inventory):
self.inventory = inventory
self.inventory_basedir = inventory.basedir()
self.loader = DataLoader()
self._options = cli.options if cli else None

# Wrap salts and keys variables in {% raw %} to prevent jinja templating errors
def wrap_salts_in_raw(self, host, hostvars):
if 'vault_wordpress_sites' in hostvars:
for name, site in hostvars['vault_wordpress_sites'].iteritems():
for key, value in site['env'].iteritems():
if key.endswith(('_key', '_salt')) and not value.startswith(('{% raw', '{%raw')):
hostvars['vault_wordpress_sites'][name]['env'][key] = ''.join(['{% raw %}', value, '{% endraw %}'])
host.vars['vault_wordpress_sites'] = hostvars['vault_wordpress_sites']
def raw_triage(self, key_string, item, patterns):
# process dict values
if isinstance(item, AnsibleMapping):
return dict((key,self.raw_triage('.'.join([key_string, key]), value, patterns)) for key,value in item.iteritems())

# process list values
elif isinstance(item, AnsibleSequence):
return [self.raw_triage('.'.join([key_string, str(i)]), value, patterns) for i,value in enumerate(item)]

# wrap values if they match raw_vars pattern
elif isinstance(item, AnsibleUnicode):
match = next((pattern for pattern in patterns if re.match(pattern, key_string)), None)
return ''.join(['{% raw %}', item, '{% endraw %}']) if not item.startswith(('{% raw', '{%raw')) and match else item

def raw_vars(self, host, hostvars):
if 'raw_vars' not in hostvars:
return
Copy link
Member

@swalkinshaw swalkinshaw Jul 17, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if it's better to move the default vars into here.

Pros: users shouldn't have to worry about these too much so they are kept out of the way and harder to mess up.
Cons: to disable them would require raw_vars: [] and maybe less visibility?

Copy link
Contributor Author

@fullyint fullyint Jul 17, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That "visibility" was my focus. My overarching goal was to transition this functionality from "hidden" -- which for users is possibly surprising and certainly non-configurable -- to "out in the open" and easy to use.

I agree most users won't understand it and won't touch it, in which case they won't mess it up. In that sense, I think it is like neighboring obscure vars in group_vars/all/main.yml, such as wordpress_env_defaults or site_env.

For users who do try to use it, actually adding patterns versus merely disabling it, I think they'll have the easiest time if they readily see the full list of raw_vars default patterns right there in a group_vars file.

Would users really want to adjust patterns? I think a conceivable example would be that someone wants to add an env var in vault_wordpress_sites that should not be wrapped in {% raw %}. They'd need to adjust the raw_vars to be more specific, just wrapping the right parts, such as:

raw_vars:
  - vault_wordpress_sites.*.*_password       # for `admin_password` (dev only) and `db_password`
  - vault_wordpress_sites.*.*_salt
  - vault_wordpress_sites.*.*_key

On the other hand, if we decide that visibility and configurability aren't the goal or aren't worth it, then I suppose moving the defaults into vars.py makes sense. In that case, we could consider whether the python code would be "safer" being more targeted to those defaults, not supporting the wildcard stuff and being more "hard-coded" like it has been for WP keys and salts.


raw_vars = Templar(variables=hostvars, loader=self.loader).template(hostvars['raw_vars'])
if not isinstance(raw_vars, list):
raise AnsibleError('The `raw_vars` variable must be defined as a list.')

patterns = [re.sub(r'\*', '(.)*', re.sub(r'\.', '\.', var)) for var in raw_vars if var.split('.')[0] in hostvars]
keys = set(pattern.split('\.')[0] for pattern in patterns)
for key in keys:
host.vars[key] = self.raw_triage(key, hostvars[key], patterns)

def cli_options(self):
options = []
Expand All @@ -52,7 +76,7 @@ def cli_options(self):
return ' '.join(options)

def get_host_vars(self, host, vault_password=None):
self.wrap_salts_in_raw(host, host.get_group_vars())
self.raw_vars(host, host.get_group_vars())
host.vars['cli_options'] = self.cli_options()
host.vars['cli_ask_pass'] = getattr(self._options, 'ask_pass', False)
return {}