List typed config values can not be merged between multiple config files (PANTS_CONFIG_FILES)

[mpcusack-color]

Describe the bug
Originally asked in https://pantsbuild.slack.com/archives/C046T6T9U/p1646199988870439

No matter the syntax I am unable to get docker build args list merging to work.

Pants version
2.9.0

OS
Linux

Additional info

pants.toml:

...
[docker]
build_args.add = [
    "BASE_PYTHON_IMAGE=foo",
]

pants.ci.toml:

...
[docker]
# Expose aws auth to docker.
build_args.add = [
    "AWS_DEFAULT_REGION",
    "AWS_ACCESS_KEY_ID",
    "AWS_SECRET_ACCESS_KEY",
    "AWS_SESSION_TOKEN",
]

Dockerfile:

ARG BASE_PYTHON_IMAGE
FROM ${BASE_PYTHON_IMAGE}
...

Running in CI:

$ export PANTS_CONFIG_FILES=pants.ci.toml
$ ./pants package docker/image:target
...
  failed to solve with frontend dockerfile.v0: failed to create LLB definition: base name (${BASE_PYTHON_IMAGE}) should not be blank
...

[mpcusack-color]

Some more examples:

$ PANTS_CONFIG_FILES=pants.ci.toml ./pants help docker | grep -A 12 docker-build-args
  --docker-build-args="[<shell_str>, <shell_str>, ...]"
  PANTS_DOCKER_BUILD_ARGS
  build_args
      default: []
      current value: [
          "AWS_DEFAULT_REGION",
          "AWS_ACCESS_KEY_ID",
          "AWS_ROLE_ARN",
          "AWS_SECRET_ACCESS_KEY",
          "AWS_SESSION_TOKEN",
          "AWS_WEB_IDENTITY_TOKEN_FILE"
      ] (from pants.ci.toml)
      Global build arguments (for Docker `--build-arg` options) to use for all `docker build`

$ PANTS_CONFIG_FILES="+['pants.ci.toml']" ./pants help docker | grep -A 12 docker-build-args
  --docker-build-args="[<shell_str>, <shell_str>, ...]"
  PANTS_DOCKER_BUILD_ARGS
  build_args
      default: []
      current value: [
          "AWS_DEFAULT_REGION",
          "AWS_ACCESS_KEY_ID",
          "AWS_ROLE_ARN",
          "AWS_SECRET_ACCESS_KEY",
          "AWS_SESSION_TOKEN",
          "AWS_WEB_IDENTITY_TOKEN_FILE"
      ] (from pants.ci.toml)
      Global build arguments (for Docker `--build-arg` options) to use for all `docker build`

$ ./pants help docker | grep -A 7 docker-build-args
  --docker-build-args="[<shell_str>, <shell_str>, ...]"
  PANTS_DOCKER_BUILD_ARGS
  build_args
      default: []
      current value: [
          "COMMIT_SHA",
          "BASE_PYTHON_IMAGE=foo"
      ] (from pants.toml)

$ PANTS_CONFIG_FILES="['pants.toml', 'pants.ci.toml']" ./pants help docker | grep -A 12 docker-build-args
  --docker-build-args="[<shell_str>, <shell_str>, ...]"
  PANTS_DOCKER_BUILD_ARGS
  build_args
      default: []
      current value: [
          "AWS_DEFAULT_REGION",
          "AWS_ACCESS_KEY_ID",
          "AWS_ROLE_ARN",
          "AWS_SECRET_ACCESS_KEY",
          "AWS_SESSION_TOKEN",
          "AWS_WEB_IDENTITY_TOKEN_FILE"
      ] (from pants.ci.toml)
      Global build arguments (for Docker `--build-arg` options) to use for all `docker build`

$ PANTS_CONFIG_FILES="['pants.ci.toml', 'pants.toml']" ./pants help docker | grep -A 7 docker-build-args
  --docker-build-args="[<shell_str>, <shell_str>, ...]"
  PANTS_DOCKER_BUILD_ARGS
  build_args
      default: []
      current value: [
          "COMMIT_SHA",
          "BASE_PYTHON_IMAGE=foo"
      ] (from pants.toml)
      Global build arguments (for Docker `--build-arg` options) to use for all `docker build`

[mpcusack-color]

Merging from env seems to work:

PANTS_DOCKER_BUILD_ARGS="+['aaa=bbb']" ./pants help docker  | grep -A 7 docker-build-args
  --docker-build-args="[<shell_str>, <shell_str>, ...]"
  PANTS_DOCKER_BUILD_ARGS
  build_args
      default: []
      current value: [
          "COMMIT_SHA",
          "BASE_PYTHON_IMAGE=foo",
          "aaa=bbb"

[benjyw]

FWIW this also works:

pants.toml
-----------
[docker]
build_args.add = ["foo=bar"]


$ ./pants help docker --build-args="baz=qux"
<snip>
--docker-build-args="[<shell_str>, <shell_str>, ...]"
  PANTS_DOCKER_BUILD_ARGS
  build_args
      default: []
      current value: [
          "foo=bar",
          "baz=qux"
      ] (from pants.toml, from command-line flag)
</snip>

[benjyw]

OK, I can reproduce this at HEAD, and it's definitely a bug.

[benjyw]

It appears to be a bug in the options system (!) not specifically in the docker subsystem.

[mpcusack-color]

Yes, I experience the same thing with [GLOBAL] plugins. Glad it's not just me :-).

[benjyw]

This appears to be working as intended in some sense, but only because we didn't think of this case, apparently.

[mpcusack-color]

As intended? I thought the point of foo.add = [...] was to support this? It seems like that's being ignored currently.

[benjyw]

That works across defaults/env vars/config/flags, but the current semantics of multiple config files is that later files override the values in earlier files, so that only one value per set of config files is presented to the options system.

But the fact that you expected it to work differently, and I did too (and I wrote the options system back in the day) tells me that the existing logic is not good.

[benjyw]

The complication is that we parse config files before we know the type of the option, going to have to think about what to do here.

[mpcusack-color]

Thanks for the explanation. Makes sense.

Another use case/motivation:
Enabling certain plugins just for your local setup.

pants.toml:

[GLOBAL]
backend_packages.add = [
    "pants.backend.python",
    "pants.backend.python.lint.black",
    "pants.backend.python.lint.isort",
    "pants.backend.python.lint.pylint",
    "pants.backend.codegen.protobuf.python",
    "pants.backend.python.mixed_interpreter_constraints",
    "pants.backend.python.typecheck.mypy",
    "pants.backend.shell",
    "pants.backend.shell.lint.shfmt",
    "pants.backend.shell.lint.shellcheck",
    "pants.backend.docker",
    "pants.backend.docker.lint.hadolint",
]

~/.config/pants.toml (enabled via PANTS_CONFIG_FILES in .bashrc):

backend_packages.add = [
    "pants.backend.experimental.python.lint.autoflake",
    "pants.backend.experimental.python.lint.pyupgrade",
]

[mpcusack-color]

I guess one potential short term workaround is to move away from PANTS_CONFIG_FILES pointing to a secondary toml in these situations, and instead just set the overrides using all env vars. e,g, PANTS_BACKEND_PACKAGES="+['pants.backend.experimental.python.lint.autoflake', 'pants.backend.experimental.python.lint.pyupgrade']".

[Eric-Arellano]

@benjyw possibly a duplicate of #14679 btw, re: what to close once this is fixed.

[kaos]

Unassigning myself, due to

It appears to be a bug in the options system (!) not specifically in the docker subsystem.

[benjyw]

@mpcusack-color thanks for reporting this! It's now fixed in #14850 which will go into Pants 2.12.0.

Note that with the fix, putting just the add/remove values in a @fromfile works as you'd expect.

[stuhood]

We could probably also cherry-pick it to 2.11.x if needed?

[benjyw]

It builds on a bunch of other changes, so it might be complicated to cherry-pick

[stuhood]

It builds on a bunch of other changes, so it might be complicated to cherry-pick

Yea, I guess that more has landed since 2.11.x was cut than I realized. Thanks!