Skip to content
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.

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

feat(map): update to v5 map.jinja #37

Merged
merged 3 commits into from
Apr 13, 2021

Conversation

baby-gnu
Copy link

@baby-gnu baby-gnu commented Mar 17, 2021

PR progress checklist (to be filled in by reviewers)

  • Changes to documentation are appropriate (or tick if not required)
  • Changes to tests are appropriate (or tick if not required)
  • Reviews completed

What type of PR is this?

Primary type

  • [build] Changes related to the build system
  • [chore] Changes to the build process or auxiliary tools and libraries such as documentation generation
  • [ci] Changes to the continuous integration configuration
  • [feat] A new feature
  • [fix] A bug fix
  • [perf] A code change that improves performance
  • [refactor] A code change that neither fixes a bug nor adds a feature
  • [revert] A change used to revert a previous commit
  • [style] Changes that do not affect the meaning of the code (white-space, formatting, missing semi-colons, etc.)

Secondary type

  • [docs] Documentation changes
  • [test] Adding missing or correcting existing tests

Does this PR introduce a BREAKING CHANGE?

Yes.

BREAKING CHANGE: map.jinja now export a generic mapdata variable

BREAKING CHANGE: the parameters per grains are now under cert/parameters/

Related issues and/or pull requests

saltstack-formulas/template-formula#225

Describe the changes you're proposing

feature(map): update to generic v5 map.jinja

The cert-formula requires a dedicated map.jinja configuration to skip values from salt["config.get"](cert) for compatibility.

Pillar / config required to test the proposed changes

Debug log showing how the proposed changes work

Documentation checklist

  • Updated the README (e.g. Available states).
  • Updated pillar.example.

Testing checklist

  • Included in Kitchen (i.e. under state_top).
  • Covered by new/existing tests (e.g. InSpec, Serverspec, etc.).
  • Updated the relevant test pillar.

Additional context

@baby-gnu baby-gnu requested a review from a team as a code owner March 17, 2021 13:35
@baby-gnu
Copy link
Author

I made the PR to investigate the issue.

This formula need to setup things before executing the real cert state and this is failing because of the with context import:

  1. test/salt/states/setup-certs-to-remove.sls use context import
    {% from "cert/map.jinja" import map with context %}
  2. cert/map.jinja define tplroot as first directory of tpldir
    {%- set tplroot = tpldir.split("/")[0] %}
  3. cert/map.jinja load libmapstack.jinja in tplroot but this one is defined to states/ because of with context import
    {%- set tplroot = tpldir.split("/")[0] %}
    {%- from tplroot ~ "/libmapstack.jinja" import mapstack %}
    

This means that we should solve saltstack-formulas/apache-formula#295, i.e. explicitely do import… without context.

The `cert-formula` requires a dedicated `map.jinja` configuration to
skip values from `salt["config.get"](cert)` for compatibility.

BREAKING CHANGE: `map.jinja` now export a generic `mapdata` variable

BREAKING CHANGE: the parameters per grains are now under `cert/parameters/`
The setup of certificates to remove during the test need to use
`map.jinja` but it fails to load the YAML files since the `tplroot` is
not the `cert` formula directory in this case.

We need to enable the import of `mapdata` from outside the
`cert-formula` since `tplroot` can't be set correctly when the import:

- is done from another top directory than the `cert`
- the import is done `with context`

In this case, the `tpldir` is set to the directory of the importer
`.sls` file instead of the `.jinja` imported one.

We force the `without context` which permits to directly use `tpldir`
as the `tplroot` which is the directory of the imported file.

BREAKING CHANGE: `map.jinja` import must use `without context`

BREAKING CHANGE: `libmapstack.jinja` import must use `without context`

BREAKING CHANGE: `libmatchers.jinja` import must use `without context`
We need to update the reference files to include the `map.jinja`
sources configuration.
@myii myii merged commit 89edc88 into saltstack-formulas:master Apr 13, 2021
@saltstack-formulas-travis

🎉 This PR is included in version 1.0.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@baby-gnu baby-gnu deleted the feature/v5-map.jinja branch April 14, 2021 08:01
myii added a commit to myii/ssf-formula that referenced this pull request Apr 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants