scuba icon indicating copy to clipboard operation
scuba copied to clipboard

Scuba crashes with GitLab CI image dictionary

Open xanarin opened this issue 1 year ago • 2 comments

Currently, if one specifies a .scuba.yml like this:

aliases:
  build: !from_yaml .gitlab-ci.yml build

With a GitLab CI configuration file like this:

build:
  stage: build
  image:
    name: debian:12
  script:
    - echo "Ran successfully"

When one runs scuba build, they will see the following stack trace:

$ scuba build
Traceback (most recent call last):
  File "/home/wsloan/.local/bin/scuba", line 8, in <module>
    sys.exit(main())
             ^^^^^^
  File "/home/wsloan/.local/pipx/venvs/scuba/lib/python3.11/site-packages/scuba/__main__.py", line 184, in main
    rc = run_scuba(scuba_args) or 0
         ^^^^^^^^^^^^^^^^^^^^^
  File "/home/wsloan/.local/pipx/venvs/scuba/lib/python3.11/site-packages/scuba/__main__.py", line 138, in run_scuba
    dive = ScubaDive(
           ^^^^^^^^^^
  File "/home/wsloan/.local/pipx/venvs/scuba/lib/python3.11/site-packages/scuba/scuba.py", line 104, in __init__
    self.__setup_native_run()
  File "/home/wsloan/.local/pipx/venvs/scuba/lib/python3.11/site-packages/scuba/scuba.py", line 294, in __setup_native_run
    ep = get_image_entrypoint(self.context.image)
         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/wsloan/.local/pipx/venvs/scuba/lib/python3.11/site-packages/scuba/dockerutil.py", line 131, in get_image_entrypoint
    return _get_image_config(image, "Entrypoint")
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/wsloan/.local/pipx/venvs/scuba/lib/python3.11/site-packages/scuba/dockerutil.py", line 114, in _get_image_config
    info = docker_inspect_or_pull(image)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/wsloan/.local/pipx/venvs/scuba/lib/python3.11/site-packages/scuba/dockerutil.py", line 82, in docker_inspect_or_pull
    return docker_inspect(image)
           ^^^^^^^^^^^^^^^^^^^^^
  File "/home/wsloan/.local/pipx/venvs/scuba/lib/python3.11/site-packages/scuba/dockerutil.py", line 59, in docker_inspect
    cp = _run_docker("inspect", "--type", "image", image, capture=True)
         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/wsloan/.local/pipx/venvs/scuba/lib/python3.11/site-packages/scuba/dockerutil.py", line 49, in _run_docker
    return subprocess.run(docker_args, **kw)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/subprocess.py", line 548, in run
    with Popen(*popenargs, **kwargs) as process:
         ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/subprocess.py", line 1024, in __init__
    self._execute_child(args, executable, preexec_fn, close_fds,
  File "/usr/lib/python3.11/subprocess.py", line 1834, in _execute_child
    self.pid = _fork_exec(
               ^^^^^^^^^^^
TypeError: expected str, bytes or os.PathLike object, not dict

One can argue that Scuba shouldn't handle this use of image as a dictionary with multiple members, but at the very least Scuba shouldn't crash in this case and should give the user a helpful error message. I think it would also be fine for Scuba to accept this format of image, though I don't think we can correctly support the entrypoint element of the image dictionary.

xanarin avatar Sep 20 '23 20:09 xanarin

Bug

One can argue that Scuba shouldn't handle this use of image as a dictionary with multiple members, but at the very least Scuba shouldn't crash in this case and should give the user a helpful error message.

Agreed.

It looks like this is only a problem for images: in an alias because the config code uses a raw dict.get() call there. Because dict.get() returns Any, it can be assigned to the Optional[str] type which is not correct:

                image=node.get("image"),

The top-level images: probably does not exhibit this however, because the config code for that uses the _get_str() helper which raises a ConfigError at runtime if the value is not a str:

        self._image = _get_str(data, "image")

I think it would be an easy "fix" (for the crash) to make them both use the helper function.

Feature

I think it would also be fine for Scuba to accept this format of image

Would you mind opening a separate "enhancement" issue that requests adding support for the long-form images:? I think it's okay to include entrypoint (and any other crazy sub-keys they have these days) in that discussion. We can always peel it off and defer it if necessary.

though I don't think we can correctly support the entrypoint element of the image dictionary.

I don't see why not. scuba.yml already has an entrypoint configuration option, so I think it would be purely a matter of internally handling the two config schemas (and aborting if they're both set, I guess).

JonathonReinhart avatar Sep 21 '23 06:09 JonathonReinhart

Would you mind opening a separate "enhancement" issue that requests adding support for the long-form images:?

Opened #230. Cool, I had forgotten that Scuba supports an entrypoint configuration option. In that case I think we should support it when including a GitLab CI file

xanarin avatar Sep 26 '23 03:09 xanarin