Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

hosts.toml file resolution does not seem to account for default port being ommitted #3047

Closed
Tracked by #3072
apostasie opened this issue May 31, 2024 · 4 comments · Fixed by #3410
Closed
Tracked by #3072

hosts.toml file resolution does not seem to account for default port being ommitted #3047

apostasie opened this issue May 31, 2024 · 4 comments · Fixed by #3410
Labels
bug Something isn't working

Comments

@apostasie
Copy link
Contributor

apostasie commented May 31, 2024 •
edited
Loading

Description

nerdctl login https://foo:443
vs.
nerdctl login https://foo

If the hosts.toml path contains the port, the second one will fail.
On the other hand, if it does not, the first one will fail.

Responsibility for walking the host dir is in dockerconfigresolver.go (NewHostOptions > HostDir()).
This does not try to be smart wrt what the host contains (port or no port).

We should patch this so:

  • if scheme is https, passing :443 or not passing it should work
  • all of this should work with hosts.toml files path containing the port or not

Steps to reproduce the issue

See description

Describe the results you received and expected

Fail / not fail.

What version of nerdctl are you using?

1.7.6

Are you using a variant of nerdctl? (e.g., Rancher Desktop)

None

Host information

No response
@apostasie apostasie added the kind/unconfirmed-bug-claim Unconfirmed bug claim label May 31, 2024
@apostasie apostasie changed the title hosts.toml file resolution does not account for default port being ommitted.Login against an https server with a custom certificate and default port will fail dependent on whether the port is specified and how the hosts.toml is configured hosts.toml file resolution does not account for default port being ommitted. May 31, 2024
@apostasie apostasie changed the title hosts.toml file resolution does not account for default port being ommitted. hosts.toml file resolution does not account for default port being ommitted May 31, 2024
@apostasie apostasie changed the title hosts.toml file resolution does not account for default port being ommitted hosts.toml file resolution does not seem to account for default port being ommitted May 31, 2024
@apostasie apostasie mentioned this issue Jun 1, 2024
Open
@Zheaoli Zheaoli added bug Something isn't working and removed kind/unconfirmed-bug-claim Unconfirmed bug claim labels Jun 7, 2024
@Zheaoli
Copy link
Member

Zheaoli commented Jun 7, 2024

Seems we need rework the whole login and auth part = =

@apostasie
Copy link
Contributor Author

Seems we need rework the whole login and auth part = =

Yes, that is what I am thinking.
I am on it.
Will start with revamping testing.

@apostasie apostasie mentioned this issue Jun 7, 2024
Open
18 tasks
@apostasie apostasie mentioned this issue Jul 30, 2024
Closed
apostasie added a commit to apostasie/nerdctl that referenced this issue Sep 4, 2024
@apostasie
Hostsstore resolution cleanup
23987e3 
As described in containerd#3047, hosts.toml file lookup will treat
https://foo:443 and https://foo differently, possibly leading to divergent behaviors *for the same registry*.
This PR makes it so that a registry URL using the default https port (443) will ALSO lookup files
stored in a "portless" directory.

Finally, as dockerconfigresolver.go will soon go under significant changes, the hosts.toml resolution functions have been also
isolated in their separate, own file.

Signed-off-by: apostasie <[email protected]>
apostasie added a commit to apostasie/nerdctl that referenced this issue Sep 4, 2024
@apostasie
Hostsstore resolution cleanup
a49487d 
As described in containerd#3047, hosts.toml file lookup will treat
https://foo:443 and https://foo differently, possibly leading to divergent behaviors *for the same registry*.
This PR makes it so that a registry URL using the default https port (443) will ALSO lookup files
stored in a "portless" directory.

Finally, as dockerconfigresolver.go will soon go under significant changes, the hosts.toml resolution functions have been also
isolated in their separate, own file.

Signed-off-by: apostasie <[email protected]>
@apostasie apostasie mentioned this issue Sep 4, 2024
Merged
apostasie added a commit to apostasie/nerdctl that referenced this issue Sep 4, 2024
@apostasie
Hostsstore resolution cleanup
1a4a5a9 
As described in containerd#3047, hosts.toml file lookup will treat
https://foo:443 and https://foo differently, possibly leading to divergent behaviors *for the same registry*.
This PR makes it so that a registry URL using the default https port (443) will ALSO lookup files
stored in a "portless" directory.

Finally, as dockerconfigresolver.go will soon go under significant changes, the hosts.toml resolution functions have been also
isolated in their separate, own file.

Signed-off-by: apostasie <[email protected]>
@djdongjin
Copy link
Member

nerdctl login https://foo:443
vs.
nerdctl login https://foo

What's the behavior in docker? e.g., if you docker login https://foo:443, can you docker pull an image from https://foo (and vice versa)?

@apostasie
Copy link
Contributor Author

nerdctl login https://foo:443
vs.
nerdctl login https://foo

What's the behavior in docker? e.g., if you docker login https://foo:443, can you docker pull an image from https://foo (and vice versa)?

Docker does not support hosts.toml, so, it does not seem like the above would matter here?

Anyhow, here is a tentative proposal to clarify behavior wrt --insecure-registry, hosts.toml, and credentials:
#3417

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
3 participants
@Zheaoli @djdongjin @apostasie