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

external git pillar docs ref incorrect #56127

Open
apex-omontgomery opened this issue Feb 11, 2020 · 7 comments
Open

external git pillar docs ref incorrect #56127

apex-omontgomery opened this issue Feb 11, 2020 · 7 comments
Labels
Bug broken, incorrect, or confusing behavior Confirmed Salt engineer has confirmed bug/feature - often including a MCVE doc-rework confusing, misleading, or wrong Documentation Relates to Salt documentation git and git fs guide Pillar severity-low 4th level, cosemtic problems, work around exists time-estimate-sprint
Milestone

Comments

@apex-omontgomery
Copy link

Description of Issue

Docs for git external pillar don't properly show the ref format that pygit2 uses. This causes errors that don't indicate a formatting issue.

docs example

ext_pillar:
  - git:
    (....)
    # SSH key authentication
    - master git@other-git-server:pillardata-ssh.git:
    (...)
    # HTTPS authentication
    - master https://other-git-server/pillardata-https.git:
    (...)

pygit2 example

>>> from pygit2 import clone_repository
>>> repo_url = 'git://github.com/libgit2/pygit2.git'
>>> repo_path = '/path/to/create/repository'
>>> repo = clone_repository(repo_url, repo_path) # Clones a non-bare repository

http error:

Traceback (most recent call last):
  File "test.py", line 11, in <module>
    repo = clone_repository(repo_url, repo_path)
  File "/usr/lib/python2.7/dist-packages/pygit2/__init__.py", line 255, in clone_repository
    check_error(err)
  File "/usr/lib/python2.7/dist-packages/pygit2/errors.py", line 64, in check_error
    raise GitError(message)
_pygit2.GitError: Failed to resolve address for https: Name or service not known

ssh error

Traceback (most recent call last):
  File "test.py", line 12, in <module>
    repo = clone_repository(repo_url, repo_path)
  File "/usr/lib/python2.7/dist-packages/pygit2/__init__.py", line 255, in clone_repository
    check_error(err)
  File "/usr/lib/python2.7/dist-packages/pygit2/errors.py", line 64, in check_error
    raise GitError(message)
_pygit2.GitError: Failed to start SSH session: Unable to exchange encryption keys

Setup

(Please provide relevant configs and/or SLS files (Be sure to remove sensitive info).)

Repo used
httpref https://github.com/wimo7083/test-external-pillar.git
sshref git@github:wimo7083/test-external-pillar.git

# master config
ext_pillar:
   - git:
     - master git://github.com/wimo7083/test-external-pillar.git # works
     # these variants dont work
     #- master git@github.com:wimo7083/test-external-pillar.git # 
     #- master git://github.com/wimo7083/test-external-pillar.git # 
     #- master git@github.com:wimo7083/test-external-pillar.git #matches docs
     #- master https://github.com/wimo7083/test-external-pillar.git matches docs
salt-ssh '*' pillar.items
salt '*' pillar.items

Steps to Reproduce Issue

(Include debug logs if possible and relevant.)

only with pygit2

from pygit2 import clone_repository
repo_path = '/home/wmontgomery/tmp'
# try different variations... 
#repo_url = 'https://github.com/wimo7083/test-external-pillar.git'
repo_url = 'git://github.com/wimo7083/test-external-pillar.git'
repo = clone_repository(repo_url, repo_path)
print(repo)

Versions Report

(Provided by running salt --versions-report. Please also mention any differences in master/minion versions.)

libssl1.0.0/xenial-updates,xenial-security,now 1.0.2g-1ubuntu4.15 amd64 [installed,automatic]
openssl/xenial-updates,xenial-security,now 1.0.2g-1ubuntu4.15 amd64 [installed,automatic]
Salt Version:
           Salt: 2016.3.8

Dependency Versions:
           cffi: 1.5.2
       cherrypy: 17.4.2
       dateutil: Not Installed
          gitdb: 0.6.4
      gitpython: 1.0.1
          ioflo: Not Installed
         Jinja2: 2.10.3
        libgit2: 0.24.0
        libnacl: Not Installed
       M2Crypto: Not Installed
           Mako: Not Installed
   msgpack-pure: Not Installed
 msgpack-python: 0.5.6
   mysql-python: Not Installed
      pycparser: 2.14
       pycrypto: 2.6.1
         pygit2: 0.24.0
         Python: 2.7.12 (default, Oct  8 2019, 14:14:10)
   python-gnupg: Not Installed
         PyYAML: 5.3
          PyZMQ: 18.1.1
           RAET: Not Installed
          smmap: 0.9.0
        timelib: Not Installed
        Tornado: 5.1.1
            ZMQ: 4.3.2

System Versions:
           dist: Ubuntu 16.04 xenial
        machine: x86_64
        release: 4.15.0-72-generic
         system: Linux
        version: Ubuntu 16.04 xenial
@apex-omontgomery
Copy link
Author

If this is not related to my old salt-version and persists in current salt-versions I can make the PR.

@DmitryKuzmenko
Copy link
Contributor

Confirmed on v3000. salt '*' saltutil.refresh_pillar works for git://github.com/wimo7083/test-external-pillar.git and fails for git@github.com:wimo7083/test-external-pillar.git with this in log:

[DEBUG   ] Current fetch URL for git_pillar remote 'master git@github.com:wimo7083/test-external-pillar.git': git@github.com:wimo7083/test-external-pillar.git (desired: git@github.com:wimo7083/test-external-pillar.git)
[DEBUG   ] Current refspecs for git_pillar remote 'master git@github.com:wimo7083/test-external-pillar.git': ['+refs/heads/*:refs/remotes/origin/*', '+refs/tags/*:refs/tags/*'] (desired: ['+refs/heads/*:refs/remotes/origin/*', '+refs/tags/*:refs/tags/*'])
[DEBUG   ] Current http.sslVerify for git_pillar remote 'master git@github.com:wimo7083/test-external-pillar.git': true (desired: true)
[ERROR   ] Failed to checkout master from git_pillar remote 'master git@github.com:wimo7083/test-external-pillar.git': remote ref does not exist

@DmitryKuzmenko DmitryKuzmenko added Bug broken, incorrect, or confusing behavior Confirmed Salt engineer has confirmed bug/feature - often including a MCVE Documentation Relates to Salt documentation severity-low 4th level, cosemtic problems, work around exists P4 Priority 4 Pillar labels Feb 12, 2020
@DmitryKuzmenko DmitryKuzmenko added this to the Approved milestone Feb 12, 2020
@sagetherage sagetherage removed the P4 Priority 4 label Jun 3, 2020
@sagetherage sagetherage added the doc-rework confusing, misleading, or wrong label Aug 6, 2020
@sagetherage
Copy link
Contributor

@saltstack/docs-working-group

@rossengeorgiev
Copy link
Contributor

Could someone clarify which is the correct format. I've always used the one from the docs (see below), and it works.

Should I be using ssh://.... instead?

If the answer is yes, then I would like to object. Salt should be doing whatever transformation necessary and keep the use of the established format that git uses, and is provided by the various frontends (e.g. github, gitlab, etc). The config format should be consistent regardless of what library is used underneath.

# grep -A 3 ext_pillar /etc/salt/master
ext_pillar:
- git:
  - master git@gitlab.example.com:xxx/yyyy.git:
    - root: pillar
salt --versions-report
# salt --versions-report
Salt Version:
           Salt: 2017.7.8

Dependency Versions:
           cffi: 1.6.0
       cherrypy: Not Installed
       dateutil: 1.5
      docker-py: Not Installed
          gitdb: Not Installed
      gitpython: Not Installed
          ioflo: Not Installed
         Jinja2: 2.7.2
        libgit2: 0.26.3
        libnacl: Not Installed
       M2Crypto: 0.21.1
           Mako: Not Installed
   msgpack-pure: Not Installed
 msgpack-python: 0.5.6
   mysql-python: Not Installed
      pycparser: 2.14
       pycrypto: 2.6.1
   pycryptodome: 3.9.7
         pygit2: 0.26.4
         Python: 2.7.5 (default, Apr  2 2020, 13:16:51)
   python-gnupg: Not Installed
         PyYAML: 3.11
          PyZMQ: 15.3.0
           RAET: Not Installed
          smmap: Not Installed
        timelib: Not Installed
        Tornado: 4.2.1
            ZMQ: 4.1.4

System Versions:
           dist: centos 7.8.2003 Core
         locale: UTF-8
        machine: x86_64
        release: 3.10.0-1127.10.1.el7.x86_64
         system: Linux
        version: CentOS Linux 7.8.2003 Core

@dwoz
Copy link
Contributor

dwoz commented Feb 17, 2021

Could someone clarify which is the correct format. I've always used the one from the docs (see below), and it works.

Should I be using ssh://.... instead?

If the answer is yes, then I would like to object. Salt should be doing whatever transformation necessary and keep the use of the established format that git uses, and is provided by the various frontends (e.g. github, gitlab, etc). The config format should be consistent regardless of what library is used underneath.

# grep -A 3 ext_pillar /etc/salt/master
ext_pillar:
- git:
  - master git@gitlab.example.com:xxx/yyyy.git:
    - root: pillar

salt --versions-report

Git supports all of the url formats in this issue as noted in the man page for git-clone

      In general, URLs contain information about the transport protocol, the address of the remote server, and the path to the repository. Depending on the transport protocol, some of this information may
       be absent.

       Git supports ssh, git, http, and https protocols (in addition, ftp, and ftps can be used for fetching, but this is inefficient and deprecated; do not use it).

       The native transport (i.e. git:// URL) does no authentication and should be used with caution on unsecured networks.

       The following syntaxes may be used with them:

       •   ssh://[user@]host.xz[:port]/path/to/repo.git/

       •   git://host.xz[:port]/path/to/repo.git/

       •   http[s]://host.xz[:port]/path/to/repo.git/

       •   ftp[s]://host.xz[:port]/path/to/repo.git/

       An alternative scp-like syntax may also be used with the ssh protocol:

       •   [user@]host.xz:path/to/repo.git/

       This syntax is only recognized if there are no slashes before the first colon. This helps differentiate a local path that contains a colon. For example the local path foo:bar could be specified as
       an absolute path or ./foo:bar to avoid being misinterpreted as an ssh url.

       The ssh and git protocols additionally support ~username expansion:

       •   ssh://[user@]host.xz[:port]/~[user]/path/to/repo.git/

       •   git://host.xz[:port]/~[user]/path/to/repo.git/

       •   [user@]host.xz:/~[user]/path/to/repo.git/

       For local repositories, also supported by Git natively, the following syntaxes may be used:

       •   /path/to/repo.git/

       •   file:///path/to/repo.git/

@rossengeorgiev pygit2 and therefore Salt should also support all of these. I have not dug into this issue enough to know why it's not working for the OP but if your configuration works today it is safe to assume it will keep working in the future.

@waynew
Copy link
Contributor

waynew commented Feb 17, 2021

I'm wondering if it might be an SSL issue? libgit2/libgit2#3786

@sagetherage
Copy link
Contributor

There is an official libssh2 deprecation announcement on libgit2/libgit2#5225

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 Confirmed Salt engineer has confirmed bug/feature - often including a MCVE doc-rework confusing, misleading, or wrong Documentation Relates to Salt documentation git and git fs guide Pillar severity-low 4th level, cosemtic problems, work around exists time-estimate-sprint
Projects
None yet
Development

No branches or pull requests

7 participants