Added Loader in Jinja2 Environment initialisation

[onegambler]

Description:

It adds a FileSystemLoader to the Jinja2 Environment object. http://jinja.pocoo.org/docs/2.10/api/#loaders

This will allow to import/include jinja2 templates from other templates:

• http://jinja.pocoo.org/docs/2.10/templates/#import
• http://jinja.pocoo.org/docs/2.10/templates/#include

so that we can export/reuse templates:
{% from script/communication_engine/phrases/greetings.j2' import greeting %}

I had to refactor a bit the get_default_config_dir because I was having issue with circular dependency. If there's a nicer/better way to achieve this I'm happy to refactor.

Pull request in home-assistant.github.io with documentation (if applicable): home-assistant/home-assistant.github.io#

I will create a PR for the documentation if people are happy with the change/PR

Example entry for configuration.yaml (if applicable):

No changes

Checklist:

• The code change is tested and works locally.
• Local tests pass with tox. Your PR cannot be merged unless tests pass

If user exposed functionality or configuration variables are added/changed:

• Documentation added/updated in home-assistant.github.io

If the code communicates with devices, web services, or third-party tools:

• New dependencies have been added to the REQUIREMENTS variable (example).
• New dependencies are only imported inside functions that use them (example).
• New dependencies have been added to requirements_all.txt by running script/gen_requirements_all.py.
• New files were added to .coveragerc.

If the code does not interact with devices:

• Tests have been added to verify that the new code works.

[homeassistant]

Hello @onegambler,

When attempting to inspect the commits of your pull request for CLA signature status among all authors we encountered commit(s) which were not linked to a GitHub account, thus not allowing us to determine their status(es).

The commits that are missing a linked GitHub account are the following:

• e6b5a4ad2b97816523294aeae1f44165558ddd0a - This commit has something that looks like an email address (roberto.magale@skyscanner.net). Maybe try linking that to GitHub?.
• 3cbdc59fa5a8a2a2e575e1cfeb934199d6b3050e - This commit has something that looks like an email address (roberto.magale@skyscanner.net). Maybe try linking that to GitHub?.

Unfortunately, we are unable to accept this pull request until this situation is corrected.

Here are your options:

1. If you had an email address set for the commit that simply wasn't linked to your GitHub account you can link that email now and it will retroactively apply to your commits. The simplest way to do this is to click the link to one of the above commits and look for a blue question mark in a blue circle in the top left. Hovering over that bubble will show you what email address you used. Clicking on that button will take you to your email address settings on GitHub. Just add the email address on that page and you're all set. GitHub has more information about this option in their help center.

2. If you didn't use an email address at all, it was an invalid email, or it's one you can't link to your GitHub, you will need to change the authorship information of the commit and your global Git settings so this doesn't happen again going forward. GitHub provides some great instructions on how to change your authorship information in their help center.

   • If you only made a single commit you should be able to run

     git commit --amend --author="Author Name <email@address.com>"
     

     (substituting Author Name and email@address.com for your actual information) to set the authorship information.
   • If you made more than one commit and the commit with the missing authorship information is not the most recent one you have two options:
     1. You can re-create all commits missing authorship information. This is going to be the easiest solution for developers that aren't extremely confident in their Git and command line skills.
     2. You can use this script that GitHub provides to rewrite history. Please note: this should be used only if you are very confident in your abilities and understand its impacts.
   • Whichever method you choose, I will come by to re-check the pull request once you push the fixes to this branch.

We apologize for this inconvenience, especially since it usually bites new contributors to Home Assistant. We hope you understand the need for us to protect ourselves and the great community we all have built legally. The best thing to come out of this is that you only need to fix this once and it benefits the entire Home Assistant and GitHub community.

Thanks, I look forward to checking this PR again soon! ❤️

[pvizeli]

I'm not sure if that is a feature that is really needed and not a big security problem.

Anyway. Revert all of you change and pin only one folder inside configuration to load templates from with hass.config.path('templates') all other is not needed to change.

[onegambler]

I see. I wasn't aware of concerns around the use of that. Do let me know if this can not be accepted.

Also, thank you for your comment. I am not sure I understand, ENV = TemplateEnvironment(...)does not have access to a hass object.

I am also unable to directly import config.py into template.py because of circular dependency: config -> config_validation -> template.

[balloob]

I think it's ok to add this feature. However, all paths for include/import have to be approved by hass.config.allowed_path, including if an include itself contains an include.

Also, don't use get_default_config_dir, use hass.config.config_dir instead. This means that you have to create a function that takes in hass and returns the environment. It should cache the created env inside hass.data['template_env'].

[onegambler]

I refactored the code. I managed to just modify template.py. There are some cases when hass is not initialised so I kept a default TemplateEnvironment without loader when hass is not defined.

Not sure about the solution, also I added lazy initialisation on the TemplateEnvironment and locks to avoid concurrency issues (might be overkilling).

@balloob I was having a look on how to use hass.config.allowed_path. The path I am setting is the configuration one ( so it will work with anything within .homeassistant). In my understanding allowed_path are external paths. Aren't paths within .homeassistant already allowed by default?

The path is used by the loader to decide which is the root path to start from when looking for a template to import, so we can do

{% from script/communication_engine/phrases/greetings.j2' import greeting %}
instead of
{% from /home/homeassistant/.homeassistant/script/communication_engine/phrases/greetings.j2' import greeting %}

Is necessary to add a check on allowed_path?

If we were to add the path to the allowed_path then we would have to create a loader per each allowed path in the list, even for those not used for templates.
Another options would be to create a new field template_paths alongisde allowed_path where we define the paths for templates.

Personally I am keeping my templates to import within .homeassistant config folder (as per previous example), so I wouldn't have to define an allowed path entry.

Thanks

[pvizeli]

Yes, .homeassistant is not allow default, or you can read stuff like password list or data from any config point.

[onegambler]

Hi,
I added a new config parameter, template_dirs so that is possible to do:

homeassistant:
  template_dirs:
    - /home/.homeassistant/templates

in this way the user can explicitly specify if and where templates can be found. If no dirs are defined then it will use the "old" TemplateEnvironment.

Jinja allows you to use different loaders at the same time, wrapping them with a ChoiceLoader.

So for each path defined in template_dirs, the template environment will have a specific FileSystemLoader pointing to that specific dir and no other paths will be discoverable/accessible by the import/include.

Let me know your thoughts,
Thanks

[dale3h]

@onegambler May I ask why this was closed?

[onegambler]

Hi dale,

Apologies, I just didn't think people were that interested in the change.

[balloob]

The problem with includes is that all templates are rendered within the event loop and thus are not allowed to do any I/O.