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

[BUG] Jinja 3.1 moved Markup #61848

Closed
nicholasmhughes opened this issue Mar 24, 2022 · 10 comments · Fixed by #61856
Closed

[BUG] Jinja 3.1 moved Markup #61848

nicholasmhughes opened this issue Mar 24, 2022 · 10 comments · Fixed by #61856
Assignees
Labels
Bug broken, incorrect, or confusing behavior Core relates to code central or existential to Salt dependency underlying Salt dependency issue severity-medium 3rd level, incorrect or bad functionality, confusing and lacks a work around
Milestone

Comments

@nicholasmhughes
Copy link
Collaborator

Description
Jinja 3.1 was released today and removed some previously deprecated code. At minimum, this breaks this line in salt/utils/jinja.py as Markup should now be imported from MarkupSafe:

from jinja2 import BaseLoader, Markup, TemplateNotFound, nodes

The base Jinja requirement is currently not pinned, so we'll either need to require <3.1 or change the import and require >=3.1 (or narrow the pin even further).

@nicholasmhughes nicholasmhughes added Bug broken, incorrect, or confusing behavior needs-triage labels Mar 24, 2022
@nicholasmhughes
Copy link
Collaborator Author

Oh, nice... it's breaking Actions on PRs: https://github.com/saltstack/salt/runs/5679196982?check_suite_focus=true

@jpic
Copy link
Contributor

jpic commented Mar 24, 2022

PR to pin jinja2 #61850
Will need to backport... ie. issue 3002.6 etc

jpic added a commit to jpic/salt that referenced this issue Mar 25, 2022
Fixes:

salt-master[18967]:   File "/usr/local/lib/python3.8/dist-packages/salt/utils/jinja.py", line 28, in <module>
salt-master[18967]:     from jinja2 import BaseLoader, Markup, TemplateNotFound, nodes
salt-master[18967]: ImportError: cannot import name 'Markup' from 'jinja2' (/usr/local/lib/python3.8/dist-packages/jinja2/__init__.py)
jpic added a commit to jpic/salt that referenced this issue Mar 25, 2022
Fixes:

salt-master[18967]:   File "/usr/local/lib/python3.8/dist-packages/salt/utils/jinja.py", line 28, in <module>
salt-master[18967]:     from jinja2 import BaseLoader, Markup, TemplateNotFound, nodes
salt-master[18967]: ImportError: cannot import name 'Markup' from 'jinja2' (/usr/local/lib/python3.8/dist-packages/jinja2/__init__.py)
@OrangeDog OrangeDog added severity-medium 3rd level, incorrect or bad functionality, confusing and lacks a work around Core relates to code central or existential to Salt dependency underlying Salt dependency issue and removed needs-triage labels Mar 25, 2022
@OrangeDog OrangeDog added this to the Approved milestone Mar 25, 2022
@OrangeDog
Copy link
Contributor

Or change the import to handle both locations and any version of Jinja...

@jpic
Copy link
Contributor

jpic commented Mar 25, 2022

Of course, but it might not be the only fix we need: https://github.com/pallets/jinja/issues?q=is%3Aissue+is%3Aclosed

I believe in the short term we need to have the saltstack bootstrap script to work again, even with older saltstack versions. As jinja maintainers said in the issue where importing escape from jinja doesn't work anymore: we should be pinning versions pallets/jinja#1626

The saltstack bootstrap script should install dependencies that are tested against the saltstack version it's installing.

But also, new saltstack versions should support new jinja2 versions that's for sure.

@Ch3LL
Copy link
Contributor

Ch3LL commented Mar 25, 2022

@jpic can you please open up an issue in the salt-bootstrap repo https://github.com/saltstack/salt-bootstrap with more details of how you are running into this issue with the bootstrap script install.

I have submitted #61856 to fix this issue in Salt.

@jpic
Copy link
Contributor

jpic commented Mar 25, 2022

There you go but I'm not sure what you want to change in salt-bootstrap ... Remove -U in pip install of requirements?
saltstack/salt-bootstrap#1810

@Ch3LL
Copy link
Contributor

Ch3LL commented Mar 25, 2022

Lets discuss in that issue

@blackliner
Copy link

This broke our CI bootstrapping procedure, any plans how/when this gets fixed? For now we use a user data script with sudo pip install markupsafe==2.0.1 jinja2==3.0

@jpic
Copy link
Contributor

jpic commented Mar 30, 2022

For people who still need to bootstrap saltstack with git, see workaround documented here saltstack/salt-bootstrap#1810 (comment)

Good luck everyone!

@MartinEmrich
Copy link

If Jinja2 came from the Saltstack RPM repository (here on EL7 or AL2), a pip3-installed version seems to be prioritized. after

pip3 uninstall markupsafe ; pip3 uninstall Jinja2

salt-minion works again. (but that might break whatever installed them via pip in the first place)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug broken, incorrect, or confusing behavior Core relates to code central or existential to Salt dependency underlying Salt dependency issue severity-medium 3rd level, incorrect or bad functionality, confusing and lacks a work around
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants