Skip to content

Reject files mixing v1 and v2 registries.conf, even with empty fields#1753

Merged
rhatdan merged 2 commits intocontainers:mainfrom
mtrmac:empty-mixes
Dec 9, 2022
Merged

Reject files mixing v1 and v2 registries.conf, even with empty fields#1753
rhatdan merged 2 commits intocontainers:mainfrom
mtrmac:empty-mixes

Conversation

@mtrmac
Copy link
Copy Markdown
Collaborator

@mtrmac mtrmac commented Dec 9, 2022

We already rely on the TOML parser's behavior of setting specified arrays to [], but missing arrays to nil; so just use it to be more strict.

This can help situations like

[registries.search]
registries = []

[registries.insecure]
registries = []

[registries.block]
registries = []

unqualified-search-registries = ["docker.io"]

[[registry]]
...

although only indirectly: From the TOML parser's point of view, the unqualified-search-registries field is block.unqualified-search-registries and therefore invisible to us. Conceptually, the same thing could happen with

[arbitrary-unknown]
unqualified-search-registries = ["docker.io"]


[[registry]]
...

and we wouldn't warn about that.

Even so, the config file author specifying even empty fields is suspect and suggests a problem to be resolved.


Note that this could possibly break current users that generate slightly-nonsensical files.

…sConfNonempty

We will use it for more tests in the future.

Should not change behavior.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
We already rely on the TOML parser's behavior of setting specified
arays to [], but missing arrays to nil, so just use it to be more strict.

This can help situations like

> [registries.search]
> registries = []
>
> [registries.insecure]
> registries = []
>
> [registries.block]
> registries = []
>
> unqualified-search-registries = ["docker.io"]
>
> [[registry]]
> ...

although only indirectly: From the TOML parser's point of view,
the unqualified-search-registries field is block.unqualified-search-registries
and therefore invisible to us. Conceptually, the same thing could happen with
> [arbitrary-unknown]
> unqualified-search-registries
>
> [[registry]]
> ...

and we wouldn't warn about that.

Even so, the config file author specifying even empty fields is suspect
and suggests a problem to be resolved.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
@mtrmac
Copy link
Copy Markdown
Collaborator Author

mtrmac commented Dec 9, 2022

Fixes #1054 .

@mtrmac mtrmac added the kind/bug A defect in an existing functionality (or a PR fixing it) label Dec 9, 2022
@mtrmac mtrmac linked an issue Dec 9, 2022 that may be closed by this pull request
Copy link
Copy Markdown
Member

@vrothberg vrothberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@rhatdan
Copy link
Copy Markdown
Member

rhatdan commented Dec 9, 2022

LGTM

@rhatdan rhatdan merged commit fea6ff1 into containers:main Dec 9, 2022
@mtrmac mtrmac deleted the empty-mixes branch December 9, 2022 20:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/bug A defect in an existing functionality (or a PR fixing it)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Mixing v1 and v2 registries.conf is not rejected, surprising behavior

3 participants