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

[salt-cloud] Non-ascii chars cause an exception in salt-cloud #49523

Closed
MTecknology opened this issue Sep 6, 2018 · 9 comments

Comments

Projects
None yet
4 participants
@MTecknology
Copy link
Contributor

commented Sep 6, 2018

Description of Issue/Question

If a bootstrap script produces a non-ascii character, salt-cloud will produce an exception when trying to print that text to the console. I suspect this is a case of some missing .encode('utf-8')'s.

Steps to Reproduce Issue

The following bootstrap script is the simplest example I can come up with that reproduces the issue.

#!/usr/bin/env python
print(u'\u2018')

It will produce an error similar to this:

UnicodeEncodeError: 'ascii' codec can't encode character u'\u2018' in position 89: ordinal not in range(128)

Versions Report

root@prd-salt-01:/etc/salt# salt --versions-report
Salt Version:
Salt: 2018.3.2

System Versions:
dist: debian 9.5
locale: ANSI_X3.4-1968
machine: x86_64
release: 4.15.18-4-pve
system: Linux
version: debian 9.5

@Ch3LL

This comment has been minimized.

Copy link
Contributor

commented Sep 6, 2018

thanks for the PR. Looks like there are some comments on the PR there to change the encoding as he stated utf-8 will not work for everyone. I would also take a look at salt.utils.data there are some useful functions there that also handle cases for python2 and python3.

@Ch3LL Ch3LL added this to the Approved milestone Sep 6, 2018

@rbjorklin

This comment has been minimized.

Copy link
Contributor

commented Sep 28, 2018

I had a quick look and all of these might be duplicates:

#49304
#48825
#49816

Which will all be solved once this has been completed: #37692

@Ch3LL

This comment has been minimized.

Copy link
Contributor

commented Sep 28, 2018

thanks for pointing that out. i'll ping those issues to confirm so :)

@Ch3LL

This comment has been minimized.

Copy link
Contributor

commented Sep 28, 2018

oh i commented before viewing the issue you referenced. We should keep this open as we probably just need to update in vt.py to handle this since we added a bunch of unicode work in the 2018.3 release.

@Ch3LL

This comment has been minimized.

Copy link
Contributor

commented Sep 28, 2018

i'm wondering if we need to do something similar to #49525 which was closed because utf-8 wont work for everyone. i'm wondering if right there we just need to do salt.utils.to_unicode.

@MTecknology

This comment has been minimized.

Copy link
Contributor Author

commented Oct 5, 2018

I'd be interesting in taking another look at this. Can you show me an example or two so I could make sure I implement this properly? It's not as simple as just replacing foo.encode('utf-8') with salt.utils.to_unicode(foo), is it?

@Ch3LL

This comment has been minimized.

Copy link
Contributor

commented Oct 8, 2018

yes i believe that might work. without being familiar with the code or really diving in i'm guessing its either the use case for to_unicode, salt.utils.data.decode(), or salt.utils.data.encode()

@damon-atkins

This comment has been minimized.

Copy link
Member

commented Oct 21, 2018

This should fail the locale: ANSI_X3.4-1968 i.e. ASCII not UTF-8

If you expect input to be UTF-8 you need your local to be UTF-8 or tell salt you expect it to be UTF-8 input not hard code it. It will break other platforms.

@damon-atkins

This comment has been minimized.

Copy link
Member

commented Oct 21, 2018

Another example is output_encoding has been added in modules salt.modules.cmdmod

Need to default to the users settings for input and output unless a setting is provided to allow the user to override in a state/module option such as output_encoding. Where as sls files must all be treated as utf-8 text files otherwise they will not work across platforms with different language setting outputs.

@rallytime rallytime closed this in 5f0baa1 Oct 22, 2018

rallytime added a commit that referenced this issue Oct 22, 2018

Merge pull request #50146 from MTecknology/fix-49523
Improve handling of non-ascii characters in terminal output. (Fixes: #49523)

rallytime added a commit to rallytime/salt that referenced this issue Oct 23, 2018

gitebra pushed a commit to gitebra/salt that referenced this issue Oct 25, 2018

Merge remote-tracking branch 'upstream/develop' into develop
* upstream/develop: (270 commits)
  Add 2017.7.8 previous release
  Update release versions for the develop branch
  Ignore testinfra 1.17.0
  shutdown typo 2
  shutdown typo
  added force recognition to reboot_host
  added shutdown_host to vmware cloud
  Change versionadded
  add 2 second timeout on DNS socket check
  Added brief documentation and unit test.
  Added `method_call` jinja filter.
  add release note
  remove extra spaces in http.query state comment output. Add test for status_type=pcre
  correct typo
  Support regex status types in http.query state
  Add test_to_none
  Improve handling of non-ascii characters in terminal output. (Fixes: saltstack#49523)
  make toml serializer discoverable in docs
  Fix saltstack#50142, stringutils.to_none
  fix pylint issues that seemingly appeared out of nowhere
  ...

rallytime added a commit to rallytime/salt that referenced this issue Oct 25, 2018

Fix salt-cloud UnicodeEncodeError when writing to stdout
This is a partial back-port of saltstack#50146.

The full back-port was reverted due to the original change causing all
of the salt-ssh tests to fail.

This change fixes the salt-cloud error, but allows salt-ssh to work
correctly.

Refs saltstack#49523, saltstack#50146, saltstack#50174, saltstack#50231, and saltstack#50235
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.