-
Notifications
You must be signed in to change notification settings - Fork 812
feat: image module add copy func #2868
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
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: zuoxuesong-worker The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Summary of ChangesHello @zuoxuesong-worker, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the image module by introducing a new Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a new copy function to the image module, allowing for copying container images between file systems and container registries. The implementation is comprehensive, covering file-to-file, file-to-hub, hub-to-file, and hub-to-hub scenarios. I've identified a few issues, including a critical bug in argument parsing, some potential panics from unhandled errors, and several areas in the documentation (both in markdown and code comments) that could be clarified or corrected. Overall, this is a great feature addition, and with a few fixes, it will be ready to merge.
pkg/modules/image.go
Outdated
| cps.From.Type, _ = variable.StringVar(vars, cp, "from", "type") | ||
| cps.To.Type, _ = variable.StringVar(vars, cp, "to", "type") | ||
|
|
||
| srcPattern, _ := variable.StringVar(vars, cp, "to", "src_pattern") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pkg/modules/image.go
Outdated
| _ = unstructured.SetNestedField(hostVars, src.Reference.Registry, "module", "image", "src", "reference", "registry") | ||
| _ = unstructured.SetNestedField(hostVars, src.Reference.Repository, "module", "image", "src", "reference", "repository") | ||
| _ = unstructured.SetNestedField(hostVars, src.Reference.Reference, "module", "image", "src", "reference", "reference") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The errors returned by unstructured.SetNestedField are ignored. If setting these template variables fails, it could lead to silent errors during template parsing. It's safer to handle these errors.
if err := unstructured.SetNestedField(hostVars, src.Reference.Registry, "module", "image", "src", "reference", "registry"); err != nil {
return err
}
if err := unstructured.SetNestedField(hostVars, src.Reference.Repository, "module", "image", "src", "reference", "repository"); err != nil {
return err
}
if err := unstructured.SetNestedField(hostVars, src.Reference.Reference, "module", "image", "src", "reference", "reference"); err != nil {
return err
}
pkg/modules/image.go
Outdated
| a.Repo, _ = tmpl.ParseFunc(vars, a.Repo, func(b []byte) string { return string(b) }) | ||
| a.Username, _ = tmpl.ParseFunc(vars, a.Username, func(b []byte) string { return string(b) }) | ||
| a.Password, _ = tmpl.ParseFunc(vars, a.Password, func(b []byte) string { return string(b) }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pkg/modules/image.go
Outdated
| Credential: authFunc(i.To.Auths), | ||
| } | ||
| default: | ||
| return errors.New(fmt.Sprintf("invalid image type %q", i.From.Type)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pkg/modules/image.go
Outdated
| - prometheus:v2.45.0 | ||
| to: | ||
| type: hub | ||
| path: registry.example.com/{{ . }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The template {{ . }} is ambiguous and might be incorrect. It would be clearer and safer to use the full template variables, similar to the push example in the documentation, e.g., registry.example.com/{{ .module.image.src.reference.repository }}:{{ .module.image.src.reference.reference }}. This applies to other examples using this template as well.
| path: registry.example.com/{{ . }} | |
| path: registry.example.com/{{ .module.image.src.reference.repository }}:{{ .module.image.src.reference.reference }} |
| return err | ||
| } | ||
| if _, err = oras.Copy(ctx, src, src.Reference.Reference, dst, dst.Reference.Reference, oras.DefaultCopyOptions); err != nil { | ||
| return errors.Wrapf(err, "failed to push image %q to remote", img) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pkg/modules/image.go
Outdated
| pullAuths := make([]imageAuths, 0) | ||
| _ = variable.AnyVar(vars, cp, &pullAuths, "from", "auths") | ||
| for _, a := range pullAuths { | ||
| a.Repo, _ = tmpl.ParseFunc(vars, a.Repo, func(b []byte) string { return string(b) }) | ||
| a.Username, _ = tmpl.ParseFunc(vars, a.Username, func(b []byte) string { return string(b) }) | ||
| a.Password, _ = tmpl.ParseFunc(vars, a.Password, func(b []byte) string { return string(b) }) | ||
| cps.From.Auths = append(cps.From.Auths, a) | ||
| } | ||
|
|
||
| pullAuths = make([]imageAuths, 0) | ||
| _ = variable.AnyVar(vars, cp, &pullAuths, "to", "auths") | ||
| for _, a := range pullAuths { | ||
| a.Repo, _ = tmpl.ParseFunc(vars, a.Repo, func(b []byte) string { return string(b) }) | ||
| a.Username, _ = tmpl.ParseFunc(vars, a.Username, func(b []byte) string { return string(b) }) | ||
| a.Password, _ = tmpl.ParseFunc(vars, a.Password, func(b []byte) string { return string(b) }) | ||
| cps.To.Auths = append(cps.To.Auths, a) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
docs/zh/modules/image.md
Outdated
| | copy.to.type | 镜像目标类型 | 字符串 | 否 | - | | ||
| | copy.to.path | 镜像目标地址。如果类型为file,则为文件地址,如果类型为hub,则为仓库中的镜像地址 | 字符串 | 否 | - | | ||
| | copy.to.skip_tls_verify | 仅在hub类型中生效;目标镜像仓库是否跳过tls认证 | bool | 否 | - | | ||
| | copy.to.pattern | 正则表达式,过滤推送至目标的镜像 | 字符串 | 否 | - | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| | copy.from.auths.repo | 用于认证远程仓库的地址 | 字符串 | 否 | - | | ||
| | copy.from.auths.username | 用于认证远程仓库的用户名 | 字符串 | 否 | - | | ||
| | copy.from.auths.password | 用于认证远程仓库的密码 | 字符串 | 否 | - | | ||
| | copy.to | 模版语法,从本地目录镜像推送到的远程仓库镜像 | map | 否 | - | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
|
||
| if ia.copy != nil { | ||
| if err := ia.copy.copy(ctx, ha); err != nil { | ||
| return StdoutFailed, "failed to push image", err |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fa1510c to
8c30232
Compare
|
This PR has multiple commits, and the default merge method is: squash. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
Signed-off-by: [email protected] <[email protected]> feat: image module add copy func Signed-off-by: [email protected] <[email protected]> feat: image module add copy func Signed-off-by: [email protected] <[email protected]> feat: update some default config values (kubesphere#2866) feat: update some default config values Signed-off-by: [email protected] <[email protected]> bugfix: fix artifact image tag set func (kubesphere#2870) Signed-off-by: [email protected] <[email protected]> feat: k8s add haproxy image default value (kubesphere#2869) Signed-off-by: [email protected] <[email protected]> feat: kk 4.0 制品导出 支持skip_tls_verify 私仓镜像 kubesphere#2854 (kubesphere#2855) * feat: kk 4.0 制品导出 支持skip_tls_verify 私仓镜像 kubesphere#2854 * feat: update image skip tls verify func Signed-off-by: [email protected] <[email protected]> * feat: update image skip tls verify func Signed-off-by: [email protected] <[email protected]> * feat: update image skip tls verify func Signed-off-by: [email protected] <[email protected]> --------- Signed-off-by: [email protected] <[email protected]> Co-authored-by: [email protected] <[email protected]> feat: image module add copy func Signed-off-by: [email protected] <[email protected]>
4b1be07 to
052eadb
Compare
# Conflicts: # pkg/modules/image.go
faf482d to
114b21f
Compare
Signed-off-by: [email protected] <[email protected]> feat: image module add copy func Signed-off-by: [email protected] <[email protected]> feat: image module add copy func Signed-off-by: [email protected] <[email protected]> feat: image module add copy func Signed-off-by: [email protected] <[email protected]> feat: image module add copy func Signed-off-by: [email protected] <[email protected]>
114b21f to
a54e66b
Compare
|
|
/lgtm |
|
LGTM label has been added. DetailsGit tree hash: 8d7a4bd73b1a480f7a815918a8044491f067c1cf |
Signed-off-by: [email protected] <[email protected]> feat: add export copy func Signed-off-by: [email protected] <[email protected]> feat: image module add copy func (kubesphere#2868) * feat: image module add copy func Signed-off-by: [email protected] <[email protected]> feat: image module add copy func Signed-off-by: [email protected] <[email protected]> feat: image module add copy func Signed-off-by: [email protected] <[email protected]> feat: update some default config values (kubesphere#2866) feat: update some default config values Signed-off-by: [email protected] <[email protected]> bugfix: fix artifact image tag set func (kubesphere#2870) Signed-off-by: [email protected] <[email protected]> feat: k8s add haproxy image default value (kubesphere#2869) Signed-off-by: [email protected] <[email protected]> feat: kk 4.0 制品导出 支持skip_tls_verify 私仓镜像 kubesphere#2854 (kubesphere#2855) * feat: kk 4.0 制品导出 支持skip_tls_verify 私仓镜像 kubesphere#2854 * feat: update image skip tls verify func Signed-off-by: [email protected] <[email protected]> * feat: update image skip tls verify func Signed-off-by: [email protected] <[email protected]> * feat: update image skip tls verify func Signed-off-by: [email protected] <[email protected]> --------- Signed-off-by: [email protected] <[email protected]> Co-authored-by: [email protected] <[email protected]> feat: image module add copy func Signed-off-by: [email protected] <[email protected]> * feat: image module add copy func Signed-off-by: [email protected] <[email protected]> feat: image module add copy func Signed-off-by: [email protected] <[email protected]> feat: image module add copy func Signed-off-by: [email protected] <[email protected]> feat: image module add copy func Signed-off-by: [email protected] <[email protected]> feat: image module add copy func Signed-off-by: [email protected] <[email protected]> --------- Signed-off-by: [email protected] <[email protected]> feat: add export copy func Signed-off-by: [email protected] <[email protected]> feat: add export copy func Signed-off-by: [email protected] <[email protected]> feat: add export copy func Signed-off-by: [email protected] <[email protected]>
Signed-off-by: [email protected] <[email protected]> feat: add export copy func Signed-off-by: [email protected] <[email protected]> feat: image module add copy func (kubesphere#2868) * feat: image module add copy func Signed-off-by: [email protected] <[email protected]> feat: image module add copy func Signed-off-by: [email protected] <[email protected]> feat: image module add copy func Signed-off-by: [email protected] <[email protected]> feat: update some default config values (kubesphere#2866) feat: update some default config values Signed-off-by: [email protected] <[email protected]> bugfix: fix artifact image tag set func (kubesphere#2870) Signed-off-by: [email protected] <[email protected]> feat: k8s add haproxy image default value (kubesphere#2869) Signed-off-by: [email protected] <[email protected]> feat: kk 4.0 制品导出 支持skip_tls_verify 私仓镜像 kubesphere#2854 (kubesphere#2855) * feat: kk 4.0 制品导出 支持skip_tls_verify 私仓镜像 kubesphere#2854 * feat: update image skip tls verify func Signed-off-by: [email protected] <[email protected]> * feat: update image skip tls verify func Signed-off-by: [email protected] <[email protected]> * feat: update image skip tls verify func Signed-off-by: [email protected] <[email protected]> --------- Signed-off-by: [email protected] <[email protected]> Co-authored-by: [email protected] <[email protected]> feat: image module add copy func Signed-off-by: [email protected] <[email protected]> * feat: image module add copy func Signed-off-by: [email protected] <[email protected]> feat: image module add copy func Signed-off-by: [email protected] <[email protected]> feat: image module add copy func Signed-off-by: [email protected] <[email protected]> feat: image module add copy func Signed-off-by: [email protected] <[email protected]> feat: image module add copy func Signed-off-by: [email protected] <[email protected]> --------- Signed-off-by: [email protected] <[email protected]> feat: add export copy func Signed-off-by: [email protected] <[email protected]> feat: add export copy func Signed-off-by: [email protected] <[email protected]> feat: add export copy func Signed-off-by: [email protected] <[email protected]> feat: image module add copy func (kubesphere#2868) * feat: image module add copy func Signed-off-by: [email protected] <[email protected]> feat: image module add copy func Signed-off-by: [email protected] <[email protected]> feat: image module add copy func Signed-off-by: [email protected] <[email protected]> feat: update some default config values (kubesphere#2866) feat: update some default config values Signed-off-by: [email protected] <[email protected]> bugfix: fix artifact image tag set func (kubesphere#2870) Signed-off-by: [email protected] <[email protected]> feat: k8s add haproxy image default value (kubesphere#2869) Signed-off-by: [email protected] <[email protected]> feat: kk 4.0 制品导出 支持skip_tls_verify 私仓镜像 kubesphere#2854 (kubesphere#2855) * feat: kk 4.0 制品导出 支持skip_tls_verify 私仓镜像 kubesphere#2854 * feat: update image skip tls verify func Signed-off-by: [email protected] <[email protected]> * feat: update image skip tls verify func Signed-off-by: [email protected] <[email protected]> * feat: update image skip tls verify func Signed-off-by: [email protected] <[email protected]> --------- Signed-off-by: [email protected] <[email protected]> Co-authored-by: [email protected] <[email protected]> feat: image module add copy func Signed-off-by: [email protected] <[email protected]> * feat: image module add copy func Signed-off-by: [email protected] <[email protected]> feat: image module add copy func Signed-off-by: [email protected] <[email protected]> feat: image module add copy func Signed-off-by: [email protected] <[email protected]> feat: image module add copy func Signed-off-by: [email protected] <[email protected]> feat: image module add copy func Signed-off-by: [email protected] <[email protected]> --------- Signed-off-by: [email protected] <[email protected]>



What type of PR is this?
/kind feature
What this PR does / why we need it:
image module add copy func
Which issue(s) this PR fixes:
Fixes #
Special notes for reviewers:
Does this PR introduced a user-facing change?
Additional documentation, usage docs, etc.: