Repository navigation
feat: registry mirrors - #564
Conversation
Signed-off-by: Argannor <arga@argannor.com>
|
|
||
| [host."{{ .RegistryAddress }}"] | ||
| capabilities = ["pull", "resolve"] | ||
| skip_verify = true No newline at end of file |
There was a problem hiding this comment.
it would be nice if we had a way to make this optional, not sure if there's an elegant way to do so though.
There was a problem hiding this comment.
We can remove it be default and
- introduce an extra flag to skip verify for mirrors, or
- make it part of the current flags syntax
- skip verify only for registries configured as http://
There was a problem hiding this comment.
we could consider adding a second flag for insecure registries? what do you think about that? Maybe --insecure-registry-mirror that follows the same code path but toggles verification skip? That way it can be configured for individual mirrors. What do you think?
There was a problem hiding this comment.
Added --insecure-registry-mirrors (plural, as it applies to all specified mirrors) as an optional flag that defaults to false if not set. The skip_verify = true line will be omitted if the flag is not present.
|
/e2e |
|
Looks great overall, left one small comment, hoping we can get a second look by someone like @punkwalker |
Signed-off-by: Argannor <arga@argannor.com>
|
Hi @squidboylan, thank you for the initial review. Code is now adjusted, @punkwalker do you mind taking a look so we can finalize this? |
punkwalker
left a comment
punkwalker
left a comment
There was a problem hiding this comment.
@Argannor Few comment otherwise LGTM
| _, err = parseRegistryMirrors(registryMirrors) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| return nil |
There was a problem hiding this comment.
nit: Can we just return the err here.
| if address == "" { | ||
| return nil, fmt.Errorf("registry address cannot be empty in mirror: %s", mirror) | ||
| } |
There was a problem hiding this comment.
should we check for valid URL format for address?
There was a problem hiding this comment.
Added input validation and reference to the containerd docs for both target (real registry) and address of the mirror, and added test cases for the newly added validation
Signed-off-by: Argannor <arga@argannor.com>
Signed-off-by: Argannor <arga@argannor.com>
|
Thank you for the review @punkwalker! I adjusted the code according to your comments and ran |
|
/e2e |
This MR adds the option to configure registry mirrors.
Closes #563