feat: parse "runArgs" using docker/cli's pflag parser - #90
Merged
Conversation
A devcontainer.json's "runArgs" is spliced into a "docker run" command line, but the rules read it by matching entries one at a time, which disagrees with Docker wherever the entry's meaning depends on the flag before it or on the flag's own arity. ["--label", "--cap-drop=ALL"] drops no capability, "--privileged=true" is privileged, "--cap-drop=all" drops every capability, and "-v/var/run/docker.sock:/x" and ["-itv", "/var/run/docker.sock:/x"] both mount the Docker socket. Add package dockerargs, which reads a "runArgs" array the way pflag — the parser docker/cli uses — reads an argv, and move the seven rules that inspect "runArgs" onto it. A rule now asks for a flag's values by the flag's name, so "-v" and "--volume" are the same flag, while "--net" stays the separate one docker/cli registers. The flag table it parses with is generated by cmd/dockerflagsgen, which builds the command docker/cli builds and reads its flags back out. A hand-written arity table would go stale the first time Docker adds a flag, and stale silently. The generator is a module of its own, so decolint depends on neither docker/cli nor pflag and the repository's build, tests and linter never reach it; CI regenerates the table and fails on a diff, so a docker/cli release arrives as a diff to review. Two tests keep the table honest: a differential test that compares random argvs against a real pflag.FlagSet built from the table, and a second spelling-out of the whole table, so regenerating it fails a test rather than changing which entry a rule reads a value from. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XayP4PXkSJxnTvESxGM3ZR
nozaq
changed the title
A "go build ./..." run from inside cmd/dockerflagsgen leaves the binary beside the source, where it was committed by accident. Ignore it, as the repository already ignores the docgen binary for the same reason. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XayP4PXkSJxnTvESxGM3ZR
Choosing the destination in one place leaves generate with a single write and main with a single error path, instead of one per destination. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XayP4PXkSJxnTvESxGM3ZR
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This change adds a
dockerargspackage that parses devcontainer.json"runArgs"arrays the same way pflag (docker/cli's argument parser) does, enabling rules to correctly identify flag values regardless of how they're written in the array.Previously, rules matched array entries directly, which failed when flags consumed following entries (e.g.,
["--label", "--cap-drop=ALL"]would incorrectly treat--cap-drop=ALLas the label value). Now rules askdockerargsfor a flag's values, which handles all entry forms pflag recognizes.Key Changes
New
dockerargspackage (dockerargs/dockerargs.go):Parse()function reads a"runArgs"array as adocker runcommand line, recognizing all pflag entry forms:--flag=value,-fvalue,-f=value, bare flags, and shorthand runs like-itvFlagtype describes a docker run flag (name, shorthand, type, whether it takes a value)Argtype represents one parsed flag occurrence with its value and array indexIsTrue()helper for boolean flag valuesGenerated flag table (
dockerargs/runflags.go):docker runflag from docker/cli v29.7.1, including hidden and deprecated onescmd/dockerflagsgento stay in sync with docker/cli releasesGenerator tool (
cmd/dockerflagsgen/):Updated rules to use
dockerargs.Parse():no_privileged_container,no_cap_add_all,require_cap_drop_all,require_no_new_privileges,no_seccomp_override,no_seccomp_unconfined,no_docker_socket_mount"runArgs"arrays regardless of entry formTest coverage:
Parse()covering all entry forms and edge casesCI integration (
ci.yml):dockerflagsjob that regenerates the table and fails on diffs, ensuring docker/cli updates are reviewedNotable Implementation Details
Parse()deliberately diverges from pflag in two places (both documented): it reads past the--terminator and the image name, since a broken"runArgs"array is already in the devcontainer.json and reporting what it says is more useful than falling silentgo.modpinning docker/cli, keeping decolint's dependencies cleanhttps://claude.ai/code/session_01XayP4PXkSJxnTvESxGM3ZR