DataSource sync from git by ssh does not work

[jiuka]

NetBox version

v3.5.0

Python version

3.10

Steps to Reproduce

1. Add datasource with ssh git url (ex: ssh://[email protected]:2222/project/netbox_scripts.git)
2. Sync the datasource with netbox/manage.py syncdatasource --all --traceback

Expected Behavior

The Datasource is synced.

Observed Behavior

An exception is raised.

porcelain parses the username for ssh from the url. This leads to a error as porcelain tries to call a function while supplying the kwarg username twice.

$ /opt/netbox/netbox/manage.py syncdatasource --all --traceback
[1] Syncing netbox_scripts... Traceback (most recent call last):
  File "/opt/netbox/netbox/core/data_backends.py", line 103, in fetch
    porcelain.clone(
  File "/opt/netbox/venv/lib/python3.10/site-packages/dulwich/porcelain.py", line 505, in clone
    (client, path) = get_transport_and_path(
  File "/opt/netbox/venv/lib/python3.10/site-packages/dulwich/client.py", line 2361, in get_transport_and_path
    return _get_transport_and_path_from_url(
  File "/opt/netbox/venv/lib/python3.10/site-packages/dulwich/client.py", line 2300, in _get_transport_and_path_from_url
    return SSHGitClient.from_parsedurl(parsed, **kwargs), parsed.path
  File "/opt/netbox/venv/lib/python3.10/site-packages/dulwich/client.py", line 1714, in from_parsedurl
    return cls(
TypeError: dulwich.client.SSHGitClient() got multiple values for keyword argument 'username'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/opt/netbox/netbox/manage.py", line 10, in <module>
    execute_from_command_line(sys.argv)
  File "/opt/netbox/venv/lib/python3.10/site-packages/django/core/management/__init__.py", line 446, in execute_from_command_line
    utility.execute()
  File "/opt/netbox/venv/lib/python3.10/site-packages/django/core/management/__init__.py", line 440, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "/opt/netbox/venv/lib/python3.10/site-packages/django/core/management/base.py", line 402, in run_from_argv
    self.execute(*args, **cmd_options)
  File "/opt/netbox/venv/lib/python3.10/site-packages/django/core/management/base.py", line 448, in execute
    output = self.handle(*args, **options)
  File "/opt/netbox/netbox/core/management/commands/syncdatasource.py", line 36, in handle
    datasource.sync()
  File "/opt/netbox/netbox/core/models/data.py", line 155, in sync
    with backend.fetch() as local_path:
  File "/usr/lib/python3.10/contextlib.py", line 135, in __enter__
    return next(self.gen)
  File "/opt/netbox/netbox/core/data_backends.py", line 108, in fetch
    raise SyncError(f"Fetching remote data failed ({type(e).__name__}): {e} {self.params}")
core.exceptions.SyncError: Fetching remote data failed (TypeError): dulwich.client.SSHGitClient() got multiple values for keyword argument 'username' {'branch': 'main', 'password': '', 'username': 'git'}

It the username parameter is removed from netbox/core/data_backends.py#L104 it still fails as in ssh mode no password is allowed by porcelain.

(venv) netbox-staging@vm-netbox22:~$ /opt/netbox/netbox/manage.py syncdatasource --all --traceback
[1] Syncing netbox_scripts... Traceback (most recent call last):
  File "/opt/netbox/netbox/core/data_backends.py", line 103, in fetch
    porcelain.clone(
  File "/opt/netbox/venv/lib/python3.10/site-packages/dulwich/porcelain.py", line 508, in clone
    return client.clone(
  File "/opt/netbox/venv/lib/python3.10/site-packages/dulwich/client.py", line 705, in clone
    result = self.fetch(path, target, progress=progress, depth=depth)
  File "/opt/netbox/venv/lib/python3.10/site-packages/dulwich/client.py", line 782, in fetch
    result = self.fetch_pack(
  File "/opt/netbox/venv/lib/python3.10/site-packages/dulwich/client.py", line 1098, in fetch_pack
    proto, can_read, stderr = self._connect(b"upload-pack", path)
  File "/opt/netbox/venv/lib/python3.10/site-packages/dulwich/client.py", line 1747, in _connect
    con = self.ssh_vendor.run_command(
  File "/opt/netbox/venv/lib/python3.10/site-packages/dulwich/client.py", line 1571, in run_command
    raise NotImplementedError(
NotImplementedError: Setting password not supported by SubprocessSSHVendor.

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/opt/netbox/netbox/manage.py", line 10, in <module>
    execute_from_command_line(sys.argv)
  File "/opt/netbox/venv/lib/python3.10/site-packages/django/core/management/__init__.py", line 446, in execute_from_command_line
    utility.execute()
  File "/opt/netbox/venv/lib/python3.10/site-packages/django/core/management/__init__.py", line 440, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "/opt/netbox/venv/lib/python3.10/site-packages/django/core/management/base.py", line 402, in run_from_argv
    self.execute(*args, **cmd_options)
  File "/opt/netbox/venv/lib/python3.10/site-packages/django/core/management/base.py", line 448, in execute
    output = self.handle(*args, **options)
  File "/opt/netbox/netbox/core/management/commands/syncdatasource.py", line 36, in handle
    datasource.sync()
  File "/opt/netbox/netbox/core/models/data.py", line 155, in sync
    with backend.fetch() as local_path:
  File "/usr/lib/python3.10/contextlib.py", line 135, in __enter__
    return next(self.gen)
  File "/opt/netbox/netbox/core/data_backends.py", line 108, in fetch
    raise SyncError(f"Fetching remote data failed ({type(e).__name__}): {e} {self.params}")
core.exceptions.SyncError: Fetching remote data failed (NotImplementedError): Setting password not supported by SubprocessSSHVendor. {'branch': 'main', 'password': '', 'username': 'git'}

It the password parameter is removed too from netbox/core/data_backends.py#L104 the sync works with git over ssh and ssh key authentication.

[DanSheps]

I think the best we can do here is add some validation to prevent both using the username field and including the username within the URL, however I don't know if we actually want to do that as that seems like a lot of extra work (we might have to write a separate parser for the URL) for very little gain.

The second case should probably be handled better however I feel that this should be more documentation then a bug itself. "You can't use a username in the URL and the username/password backend fields."

[jiuka]

Both bug get trigger regardless if you enter a username and/or password in the backup parameters.
I think it would be necessary to build a dict with the parameters and not add the username and password parameter if the scheme is ssh. Something like:

    @contextmanager
    def fetch(self):
        local_path = tempfile.TemporaryDirectory()

        username = self.params.get('username')
        password = self.params.get('password')
        branch = self.params.get('branch')
        config = StackedConfig.default()

        if settings.HTTP_PROXIES and self.url_scheme in ('http', 'https'):
            if proxy := settings.HTTP_PROXIES.get(self.url_scheme):
                config.set("http", "proxy", proxy)

        clone_kwargs = dict(
            depth=1,
            branch=branch,
            username=username,
            password=password,
            config=config,
            quiet=True
        )

        if self.url_scheme in ('git'):
            del clone_kwargs['username']
            del clone_kwargs['password']

        logger.debug(f"Cloning git repo: {self.url}")
        try:
            porcelain.clone(
                self.url, local_path.name, errstream=porcelain.NoneStream(), **clone_kwargs
            )
        except BaseException as e:
            raise SyncError(f"Fetching remote data failed ({type(e).__name__}): {e}")

        yield local_path.name

        local_path.cleanup()

[tobiasge]

I will create a PR