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] file.replace backslash_literal=True causing bytes issues #61944

Closed
6 tasks done
whytewolf opened this issue Apr 12, 2022 · 0 comments · Fixed by #62069
Closed
6 tasks done

[BUG] file.replace backslash_literal=True causing bytes issues #61944

whytewolf opened this issue Apr 12, 2022 · 0 comments · Fixed by #62069
Labels
Bug broken, incorrect, or confusing behavior good first issue good for someone new to salt severity-medium 3rd level, incorrect or bad functionality, confusing and lacks a work around VMware

Comments

@whytewolf
Copy link
Collaborator

whytewolf commented Apr 12, 2022

Description

file.replace with backslash_literal throwing the following traceback.

Traceback (most recent call last):
  File "/usr/lib/python3/dist-packages/salt/cli/caller.py", line 218, in call
    ret["return"] = self.minion.executors[fname](
  File "/usr/lib/python3/dist-packages/salt/loader/lazy.py", line 149, in __call__
    return self.loader.run(run_func, *args, **kwargs)
  File "/usr/lib/python3/dist-packages/salt/loader/lazy.py", line 1201, in run
    return self._last_context.run(self._run_as, _func_or_method, *args, **kwargs)
  File "/usr/lib/python3/dist-packages/salt/loader/lazy.py", line 1216, in _run_as
    return _func_or_method(*args, **kwargs)
  File "/usr/lib/python3/dist-packages/salt/executors/direct_call.py", line 10, in execute
    return func(*args, **kwargs)
  File "/usr/lib/python3/dist-packages/salt/loader/lazy.py", line 149, in __call__
    return self.loader.run(run_func, *args, **kwargs)
  File "/usr/lib/python3/dist-packages/salt/loader/lazy.py", line 1201, in run
    return self._last_context.run(self._run_as, _func_or_method, *args, **kwargs)
  File "/usr/lib/python3/dist-packages/salt/loader/lazy.py", line 1216, in _run_as
    return _func_or_method(*args, **kwargs)
  File "/usr/lib/python3/dist-packages/salt/modules/file.py", line 2566, in replace
    repl.replace("\\", "\\\\") if backslash_literal else repl,
TypeError: a bytes-like object is required, not 'str'

Setup

add a file to the filesystem at /tmp/test witht eh contents

a
b
c

and run salt-call file.replace path="/tmp/test" pattern='c' repl='c-replaced' backslash_literal=True -l debug

you will get the following tracepath

Traceback (most recent call last):
  File "/usr/lib/python3/dist-packages/salt/cli/caller.py", line 218, in call
    ret["return"] = self.minion.executors[fname](
  File "/usr/lib/python3/dist-packages/salt/loader/lazy.py", line 149, in __call__
    return self.loader.run(run_func, *args, **kwargs)
  File "/usr/lib/python3/dist-packages/salt/loader/lazy.py", line 1201, in run
    return self._last_context.run(self._run_as, _func_or_method, *args, **kwargs)
  File "/usr/lib/python3/dist-packages/salt/loader/lazy.py", line 1216, in _run_as
    return _func_or_method(*args, **kwargs)
  File "/usr/lib/python3/dist-packages/salt/executors/direct_call.py", line 10, in execute
    return func(*args, **kwargs)
  File "/usr/lib/python3/dist-packages/salt/loader/lazy.py", line 149, in __call__
    return self.loader.run(run_func, *args, **kwargs)
  File "/usr/lib/python3/dist-packages/salt/loader/lazy.py", line 1201, in run
    return self._last_context.run(self._run_as, _func_or_method, *args, **kwargs)
  File "/usr/lib/python3/dist-packages/salt/loader/lazy.py", line 1216, in _run_as
    return _func_or_method(*args, **kwargs)
  File "/usr/lib/python3/dist-packages/salt/modules/file.py", line 2566, in replace
    repl.replace("\\", "\\\\") if backslash_literal else repl,
TypeError: a bytes-like object is required, not 'str'

looks like the "\\" and "\\\\" should be bytes not strings.

Please be as specific as possible and give set-up details.

  • on-prem machine
  • VM (Virtualbox, KVM, etc. please specify)
  • VM running on a cloud service, please be explicit and add details
  • container (Kubernetes, Docker, containerd, etc. please specify)
  • or a combination, please be explicit
  • jails if it is FreeBSD

Expected behavior
the replace happens. or doesn't. but doesn't return an error.

Versions Report

salt --versions-report (Provided by running salt --versions-report. Please also mention any differences in master/minion versions.)
❯ salt-call --versions
Salt Version:
          Salt: 3004.1
 
Dependency Versions:
          cffi: Not Installed
      cherrypy: Not Installed
      dateutil: 2.7.3
     docker-py: Not Installed
         gitdb: Not Installed
     gitpython: Not Installed
        Jinja2: 2.10.1
       libgit2: Not Installed
      M2Crypto: Not Installed
          Mako: Not Installed
       msgpack: 0.6.2
  msgpack-pure: Not Installed
  mysql-python: Not Installed
     pycparser: Not Installed
      pycrypto: 2.6.1
  pycryptodome: 3.6.1
        pygit2: Not Installed
        Python: 3.8.10 (default, Mar 15 2022, 12:22:08)
  python-gnupg: 0.4.5
        PyYAML: 5.3.1
         PyZMQ: 18.1.1
         smmap: Not Installed
       timelib: Not Installed
       Tornado: 4.5.3
           ZMQ: 4.3.2
 
System Versions:
          dist: pop 20.04 focal
        locale: utf-8
       machine: x86_64
       release: 5.15.8-76051508-generic
        system: Linux
       version: Pop!_OS 20.04 focal
@whytewolf whytewolf added Bug broken, incorrect, or confusing behavior help-wanted Community help is needed to resolve this needs-triage good first issue good for someone new to salt VMware labels Apr 12, 2022
@OrangeDog OrangeDog added severity-medium 3rd level, incorrect or bad functionality, confusing and lacks a work around and removed needs-triage labels Apr 13, 2022
emmadionne1 added a commit to emmadionne1/salt that referenced this issue May 16, 2022
@nicholasmhughes nicholasmhughes removed help-wanted Community help is needed to resolve this needs-triage labels May 17, 2022
garethgreenaway pushed a commit to garethgreenaway/salt that referenced this issue May 23, 2022
Ch3LL pushed a commit that referenced this issue May 26, 2022
* Making the retry state system feature available when parallel is set to True.

* swapping out using time.sleep Salt function for Python time.sleep.

* Skipping parallel tests on Windows.

* Fixing items, values, and keys functions in the data module.  Moving integration to functional tests.  Converting unit test over to pytest.

* adding changleog.

* fixes #62044 add ignore_missing to file.comment state

* fixes #61662 fix file.comment reports changes in test mode

* add 61662.fixed changelog

* add final test mode run after comment is successful

* fix uncomment_regex

* fix uncomment_regex

* Try not to trigger the GLIBC race condition.

See https://sourceware.org/bugzilla/show_bug.cgi?id=19329

Fixes #62071

Signed-off-by: Pedro Algarvio <[email protected]>

* Add missing CLI example to ``network.fqdns``

Signed-off-by: Pedro Algarvio <[email protected]>

* Sleep between submitting new jobs to thread pool

Signed-off-by: Pedro Algarvio <[email protected]>

* Sleep before trying to resolve instead

Signed-off-by: Pedro Algarvio <[email protected]>

* Shorter sleeps

* fixes #61944 fixed backslash literal bytestring

* When states are running in parallel, ensure that the total run time produced by the highstate outputter takes that into account.

* removing unrelated change

* Adding changelog.

* Skip parallel test on Windows.

* skip pdbedit unit tests on ubuntu 2204 because md4 is not supported as a hash type

* refresh db at the beginning for destructive aptpkg functional tests

* fix failing test_pkgrepo_managed_absent and actually capture and keep signed-by information in salt/modules/aptpkg.py custom implementation of apt repository management

* Update tests/unit/modules/test_pdbedit.py

Co-authored-by: Pedro Algarvio <[email protected]>

* Disable hack to force allowing forking on macOS

* Use the newer nox platforms file which installs Py3.9

Signed-off-by: Pedro Algarvio <[email protected]>

* Support macOS defaulting to spawn as the default multiprocessing method

* Remove unused attribute

* Switch to ``salt.utils.platform.spawning_platform()``

Signed-off-by: Pedro Algarvio <[email protected]>

* Reconstruct client instances on spawning platforms

* Also skip this test for now on macOS

Signed-off-by: Pedro Algarvio <[email protected]>

* Reconstruct the ``State`` class on spawned processes.

Signed-off-by: Pedro Algarvio <[email protected]>

* Force a reference to ``salt.ext.tornado.iostream.StreamClosedError`` to avoid ``NameError`` issues

Signed-off-by: Pedro Algarvio <[email protected]>

* Skip, for now, problematic tests on spawning platforms

Signed-off-by: Pedro Algarvio <[email protected]>

* DeltaProxy minions do not work on spawning platforms

Signed-off-by: Pedro Algarvio <[email protected]>

* Avoid a ``NameError`` when failing to initialize the process list

Signed-off-by: Pedro Algarvio <[email protected]>

* Use Python 3.9 on macOS

Signed-off-by: Pedro Algarvio <[email protected]>

* Drop requirements on Darwin for Py<3.9

Signed-off-by: Pedro Algarvio <[email protected]>

* Same test run timeout as windows

Signed-off-by: Pedro Algarvio <[email protected]>

* Don't shell out! Access the database directly and properly escape values.

Signed-off-by: Pedro Algarvio <[email protected]>

* Fixes for the new macOS Catalina and BigSur CICD images

Signed-off-by: Pedro Algarvio <[email protected]>

* Migrate ``integration.modules.test_mac_pkgutil`` to PyTest and functional tests.

Fixes #60819

Signed-off-by: Pedro Algarvio <[email protected]>

* Add changelog for #57742

Fixes #57742

Signed-off-by: Pedro Algarvio <[email protected]>

* Add changelog entry.

Fixes #55847

Signed-off-by: Pedro Algarvio <[email protected]>

* initial Proxy Module

* inital pass of restconf states

* adding codeauthor

* adding restconf module

* doc update

* doc update

* doc update

* add requirements chefk to states module

* doc update

* doc update

* doc update

* add changelog entry

* Update salt/proxy/restconf.py

Co-authored-by: Wayne Werner <[email protected]>

* Update salt/proxy/restconf.py

Co-authored-by: Wayne Werner <[email protected]>

* remove noqa statements

* docs update & opts defaults

* docs update

* error checks + https transport method

* docs update

* docs update

* doing a fix for commit check

* adding test and updating modules

* removing old code

* DRY

* docs update

* output updates

* prepping for deepdiff switch

* states testing

* more tests

* changed changelog from fixed to added

* force pre-commit checks

* new style of diff output that is readable

* updating tests and output

* updating output style to make YAML diff more readable

* add test for restconf module

* adjusting based on pr review

* adjusting based on pr review

* adjusting true/false verify based on pr review

* change uri to path and clean args list

Co-authored-by: Wayne Werner <[email protected]>

* adjusting uri to path based on pr review

* adjusting capabilities path based on pr review

* Update salt/modules/restconf.py

Co-authored-by: Wayne Werner <[email protected]>

* adjusting val based on pr review

* caps to confirm to RFC 8040

* WIP: refactor based on feedback

* WIP: changed results to be any kind of falsey val

* linting - pre-commit

* updating docs

* Update salt/states/restconf.py

Co-authored-by: Wayne Werner <[email protected]>

* updating style

* fixes for lint and nox pass

* fixing

* blacken

* Update restconf.py

* add cli examples

* change logging type

* add pytest skip for ordereddict issue with legacy python

* add sys library for pytest mark skipif check

* pre-commit fixes

* Update salt/proxy/restconf.py

Co-authored-by: Pedro Algarvio <[email protected]>

* Update salt/states/restconf.py

Co-authored-by: Pedro Algarvio <[email protected]>

* Update tests/pytests/unit/modules/test_restconf.py

Co-authored-by: Pedro Algarvio <[email protected]>

* Update tests/pytests/unit/states/test_restconf.py

Co-authored-by: Pedro Algarvio <[email protected]>

* Update tests/pytests/unit/proxy/test_restconf.py

Co-authored-by: Pedro Algarvio <[email protected]>

* pre-commit fix

* update logging

* Update restconf.py

* some rest apis dont follow the standard correctly (cisco) and need another header

* fixes #61946 sync_after_install immutabledict error

* add configurable sync sleep for create

* add testing around the particular sync command called

* Adds the ability to get file version information on Windows

- Adds file.version that will get just the version
- Adds file.version_details to get additional information
- Adds tests
- Adds changelog

@amalaguti originally submitted a PR for this: #59770, but he has
since deleted his branch. Recognizing his contribution here.

* Update states.chef for version 16.x and 17.x Chef Infra Client output

* Support previous Chef version plus version 16.x and 17.x output.

* Remove legacy chef unit test

* Add unit tests for chef state

These tests cover the entire state, but are only concerned with the
ret["result"] of the outputs. It would be a good idea to test other
outputs as well - these tests could be modified to include assertions
about the other parts of ret. (Obviously they would need to be slightly
renamed. Maybe to `..._return_expected_ret`

* Update salt-bootstrap to 2022.03.15 release

* Update state.py

Fixing lint.

* Running pre-commit manually.

* swap out salt.utils.platform.is_windows for salt.utils.platform.spawning_platform.  Bump up duration for test_retry_option_success_parallel to 30 seconds to account for test running on Mac OS.

Co-authored-by: Megan Wilhite <[email protected]>
Co-authored-by: nicholasmhughes <[email protected]>
Co-authored-by: Pedro Algarvio <[email protected]>
Co-authored-by: emmadionne1 <[email protected]>
Co-authored-by: MKLeb <[email protected]>
Co-authored-by: Caleb Beard <[email protected]>
Co-authored-by: Pedro Algarvio <[email protected]>
Co-authored-by: Jamie Murphy <[email protected]>
Co-authored-by: Jamie (Bear) Murphy <[email protected]>
Co-authored-by: Jamie Murphy <[email protected]>
Co-authored-by: Wayne Werner <[email protected]>
Co-authored-by: Cassandra Durnford <[email protected]>
Co-authored-by: Jamie Murphy <[email protected]>
Co-authored-by: Jamie (Bear) Murphy <[email protected]>
Co-authored-by: Twangboy <[email protected]>
Co-authored-by: Eric Ham <[email protected]>
Co-authored-by: Wayne Werner <[email protected]>
Co-authored-by: krionbsd <[email protected]>
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 good first issue good for someone new to salt severity-medium 3rd level, incorrect or bad functionality, confusing and lacks a work around VMware
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants