fix: model Docker's identifier grammar and split it from the volume discriminator - #146
Merged
Merged
Conversation
…iscriminator Closes #143.
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
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.
Closes #143.
Docker enforces two unrelated rules that compose2pod had collapsed into one pattern,
stores.NAME_PATTERN, which was wrong for each in a different direction. They are tworules now (three, counting the long volume form), each measured against
docker compose configv5.1.2.The name grammar
values.NAME_GRAMMARis[a-zA-Z0-9._-]+. Applied at the gate to the keys of top-levelservices,volumes,secretsandconfigs, and to a service's long-formnetworksmapping key.
That list is measured, not derived. A top-level
networks:key is not held to it --networks: {"a b": {}}on its own is a document Docker accepts -- and neither is aservice's short-form list entry. Only the long-form mapping key is. The issue text
asserted the top-level
networkskey was checked; it is not, andtests/conformance/corpus/networks_top_level_name_unchecked.yamlpins the exception sothe grammar cannot later be tidied into applying everywhere.
Closed by this:
services,volumes,secretsandconfigskeys such asa b,a/b,a:b,a#b,ab!,a+b,a~b,a@b,a$b,"",a\nbandäwere allaccepted here and all rejected by Docker. Rule one, not a narrowing.
Lifted by this:
.a,-aand_aas secret/config names.stores.NAME_PATTERNrequired an alphanumeric first character Docker does not.
The volume discriminator
Not a grammar question at all, which is where my first pass got it wrong. Docker reads a
short-form source beginning with
.,/or~as a host path and every otherspelling as a volume name --
a/b,d/e/f,a b,a@b,a+b,a\bincluded, eachrefers to undefined volumewith no declaration.parsing._is_named_volume_sourceisthat rule now.
An earlier version of this file used the same leading-character rule without
~andswept
~/datainto "named";~was the missing prefix, not the grammar. Widening thename pattern instead would have read
.env:/app/.envas a named volume, which is thelive dotfile-bind regression the split exists to avoid.
The long form is a third rule:
type: volumehas already said which kind the entry is,so any source names a volume --
source: /absandsource: ./rare both undefined-volumeerrors to Docker where the short form reads the same strings as paths.
A
${VAR}-carrying source stays unnamed in both syntaxes. Docker rejects${VAR}:/xonly because it interpolates an unset variable to empty first: with
VAR=dataorVAR=/hostit accepts either reading (measured). That is ADR-0006's host-stateboundary, not a hole.
Evidence
over-rejections introduced. The only documents where the two oracles still differ are
the two
${VAR}sources above, which are the documented carve-out.just test-ci1566 passed, 100% coverage.just test-conformance952 passed,over-rejections unchanged at 19.
test_identifier_grammar_matches_docker: 11 refused and 4 acceptednames across the two enforcing positions, plus a hostile pair on the remaining blocks,
driven off
NAME_CHECKED_TOP_LEVEL_BLOCKSso a block added there is probed at once. Itasserts the verdict in both directions rather than calling
assert_rulefor itsside effect, because
assert_ruletolerates over-rejection and an over-rejection on theaccepted names is exactly the defect this lifts.
.envstill binds to/proj/.env,-astill emits
-v "-a:/x",${V}still expands at run time. No new podman claim, so nointegration row is owed.
ADR-0006
"The hard rule has no exceptions left" was false when I wrote it three days ago, and the
paragraph now says so and says why the harness could not have caught it: the generated
matrix varies a key's value over hostile shapes and never touches a map key, so no
probe could reach a name. The new axis closes that blind spot.
Review
Two review passes found real defects, both fixed here. The first version implemented the
discriminator as "matches the grammar and no leading dot", which left ten measured
rule-one holes open and carried a docstring claiming
a/bwas a bind -- it is a namedvolume. The generated probe also discarded its verdict, so its accept-side cases asserted
nothing.