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] Credentials with artifactory.downloaded do not work since python3 migration #63063

Closed
tomsolpa opened this issue Nov 15, 2022 · 7 comments
Assignees
Labels
Bug broken, incorrect, or confusing behavior python3 regarding Python 3 support

Comments

@tomsolpa
Copy link

tomsolpa commented Nov 15, 2022

Description
Since upgrading from python2 to python3, using credentials with the artifactory.downloaded state function stopped working.

Headers are not cleaned ("\n" remain) before using.

Generating the errors :

[ERROR   ] Invalid header value b'Basic abcdefghijklmnopqrstuvwxyr01234567==\n'
[ERROR   ] Invalid header value b'Basic abcdefghijklmnopqrstuvwxyr01234567==\n'
          ID: Download archive file from artifactory
    Function: artifactory.downloaded
      Result: False
     Comment: Invalid header value b'Basic abcdefghijklmnopqrstuvwxyr01234567==\n'
     Started: 14:55:45.977941
    Duration: 3.367 ms
     Changes:   

Setup
Salt-minion : 3003.3, 3004
Python 3.7.3, 3.9.2

Steps to reproduce

Download archive file from artifactory:
  artifactory.downloaded:
    - artifact:
       artifactory_url: "{{pillar['artifactory_url']}}"
       repository: "{{ pillar['artifactory_repository']}}"
       group_id: "{{ pillar['artifactory_group_id'] }}"
       artifact_id: "{{ pillar['artifactory_artifact_id'] }}"
       packaging: "{{pillar['artifactory_packaging']}}"
       version: "{{pillar['artifactory_version']}}"
       username: "{{pillar['artifactory_username']}}"
       password: "{{pillar['artifactory_password']}}"
    - target_file: /var/local/share/file-downloaded.tar.gz

Versions Report

Salt Version:
Salt: 3004

Dependency Versions:
cffi: Not Installed
cherrypy: Not Installed
dateutil: 2.8.1
docker-py: Not Installed
gitdb: Not Installed
gitpython: Not Installed
Jinja2: 2.11.3
libgit2: Not Installed
M2Crypto: Not Installed
Mako: Not Installed
msgpack: 1.0.0
msgpack-pure: Not Installed
mysql-python: Not Installed
pycparser: Not Installed
pycrypto: Not Installed
pycryptodome: 3.9.7
pygit2: Not Installed
Python: 3.9.2 (default, Feb 28 2021, 17:03:44)
python-gnupg: Not Installed
PyYAML: 5.3.1
PyZMQ: 20.0.0
smmap: Not Installed
timelib: Not Installed
Tornado: 4.5.3
ZMQ: 4.3.4

System Versions:
dist: debian 11 bullseye
locale: utf-8
machine: x86_64
release: 5.10.0-9-amd64
system: Linux
version: Debian GNU/Linux 11 bullseye

@tomsolpa tomsolpa added Bug broken, incorrect, or confusing behavior needs-triage labels Nov 15, 2022
@OrangeDog OrangeDog added the python3 regarding Python 3 support label Nov 15, 2022
@joshmcorreia
Copy link
Contributor

It should be noted that this was not caught because there are not unit tests for calling this method with credentials.

@szjur
Copy link

szjur commented Aug 3, 2023

This is caused by 4960c90 by @DmitryKuzmenko and @dwoz which started encoding credentials with salt.utils.hashutils.base64_encodestring() which clearly says that Among other possible differences, the "modern" encoder includes a newline ('\n') character after every 76 characters and always at the end of the encoded string. From what I can see httpclient has a regex for headers and most likely doesn't like that trailing \n.

@szjur
Copy link

szjur commented Aug 3, 2023

4960c90 should have changed base64.b64encode() to salt.utils.hashutils.base64_b64encode() not salt.utils.hashutils.base64_encodestring() and then all would have been fine and dandy.

@dwoz dwoz added this to the Argon v3008.0 milestone Dec 16, 2023
@dwoz dwoz modified the milestones: Argon v3008.0, Sulfur v3006.6 Dec 19, 2023
@dmurphy18
Copy link
Contributor

@tomsolpa Working on this but can you confirm the issue still exists with a version of Salt that is supported, that is, 3006.4 or 3006.5, since 3004 is EOL.

@dmurphy18
Copy link
Contributor

dmurphy18 commented Jan 2, 2024

@tomsolpa Ping, have you tried it on a supported version of Salt , that is , 3006.5

@dmurphy18
Copy link
Contributor

Closing since PR #65786 merged

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 python3 regarding Python 3 support
Projects
None yet
Development

No branches or pull requests

6 participants