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.

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

Fix RegistryAuthenticator filesystem #65

Merged
merged 1 commit into from
Dec 15, 2023

Conversation

javilaadevinta
Copy link
Contributor

@javilaadevinta javilaadevinta commented Dec 15, 2023

RegistryAuthenticator was initializated with a mock of filesystem instead of OS filesystem.

This prevented to use containerd support.

Registry instances RegistryAuthenticator with a filesystem.
This filesystem is being used here:
https://github.com/adevinta/noe/blob/main/pkg/registry/login.go#L124

To walk over:

"/etc/containerd"

https://github.com/adevinta/noe/blob/main/pkg/registry/login.go#L217

As the filesystem is being mocked, it wont walk over the directory and it wont be able to use containerd.

Once this is solved merging this PR you would find this type of logs:

{"level":"info","msg":"Get containerd auth for https://docker.io","time":"2023-12-15T13:16:12Z"}

https://github.com/adevinta/noe/blob/main/pkg/registry/login.go#L150

RegistryAuthenticator was initializated with a mock of filesystem instead of OS filesystem.

This prevented to use containerd support.
@Fsero Fsero requested a review from miguelbernadi December 15, 2023 14:35
@Fsero
Copy link
Contributor

Fsero commented Dec 15, 2023

I think you should extend the description with all the findings that leads to this conclusion

@javilaadevinta
Copy link
Contributor Author

javilaadevinta commented Dec 15, 2023

@Fsero I extended a bit the description, let me know if is enough

@Fsero
Copy link
Contributor

Fsero commented Dec 15, 2023

@Fsero I extended a bit the description, let me know if is enough

much better, thanks

@miguelbernadi miguelbernadi merged commit 7fbdc30 into adevinta:main Dec 15, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants