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

Expected imageRepository field to be validated #2758

Closed
randomvariable opened this issue Sep 21, 2022 · 6 comments · Fixed by kubernetes/kubernetes#112732
Closed

Expected imageRepository field to be validated #2758

randomvariable opened this issue Sep 21, 2022 · 6 comments · Fixed by kubernetes/kubernetes#112732
Labels
help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/bug Categorizes issue or PR as related to a bug. priority/backlog Higher priority than priority/awaiting-more-evidence.

Comments

@randomvariable
Copy link
Member

What keywords did you search in kubeadm issues before filing this one?

validate
imageRepository

Is this a BUG REPORT or FEATURE REQUEST?

Bug report

Versions

kubeadm version (use kubeadm version): v1.25.0

Environment:

  • Kubernetes version (use kubectl version): N/A
  • Cloud provider or hardware configuration: N/A
  • OS (e.g. from /etc/os-release): N/A
  • Kernel (e.g. uname -a): N/A
  • Container runtime (CRI) (e.g. containerd, cri-o): N/A
  • Container networking plugin (CNI) (e.g. Calico, Cilium): N/A
  • Others: N/A

What happened?

I modified a kind kubeadm configuration as follows:

apiServer:
  certSANs:
  - localhost
  - 127.0.0.1
  extraArgs:
    runtime-config: ""
apiVersion: kubeadm.k8s.io/v1beta3
clusterName: kind
controlPlaneEndpoint: kind-control-plane:6443
dns:
  imageRepository: |
    registry.contoso.com/kubernetes
      s

the key bit being

dns:
  imageRepository: |
    registry.contoso.com/kubernetes
      s

This was then passed into kubeadm init --config blah.yaml, and kubeadm proceeded until preflight with

[preflight] You can also perform this action in beforehand using 'kubeadm config images pull'
error execution phase preflight: [preflight] Some fatal errors occurred:
        [ERROR ImagePull]: failed to pull image registry.contoso.com/kubernetes
  s
/coredns:v1.9.3: output: E0921 13:07:35.156113    6667 remote_image.go:238] "PullImage from image servic
e failed" err="rpc error: code = Unknown desc = failed to parse image reference \"registry.contoso.com/kubernetes\\n  s\\n/coredns:v1.9.3\": invalid reference format" image=<
        registry.contoso.com/kubernetes
          s
        /coredns:v1.9.3
 >

What you expected to happen?

kubeadm to throw a validation error.

See also kubernetes-sigs/cluster-api#7257

How to reproduce it (as minimally and precisely as possible)?

See above.

Anything else we need to know?

@randomvariable randomvariable changed the title Validate imageRepository Expected imageRepository field to be validated Sep 21, 2022
@neolit123 neolit123 added kind/bug Categorizes issue or PR as related to a bug. priority/backlog Higher priority than priority/awaiting-more-evidence. labels Sep 21, 2022
@neolit123 neolit123 added this to the v1.26 milestone Sep 21, 2022
@neolit123 neolit123 added the help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. label Sep 22, 2022
@SataQiu
Copy link
Member

SataQiu commented Sep 23, 2022

Is there a common way/util to verify the imageRepository format for different image runtimes?
This seems to be an implementation based on regular expressions https://github.com/distribution/distribution/blob/main/reference/reference.go#L7

@neolit123
Copy link
Member

neolit123 commented Sep 23, 2022

not sure if a good idea to import the distribution package in kubeadm, might be fine though....can we use the go url stdlib instead? this is mentioned in kubernetes-sigs/cluster-api#7257

@pacoxu
Copy link
Member

pacoxu commented Sep 26, 2022

refer to kubernetes/kubernetes#103849, we plan to migrate to distribution v3, but it is pending due to many new transitive dependencies.

@neolit123
Copy link
Member

my vote is to just do some golang stdlib url validation

@SataQiu
Copy link
Member

SataQiu commented Sep 26, 2022

kubelet and kubectl currently uses github.com/docker/distribution/reference to validate image format.
I think kubeadm better keep consistent behavior with kubelet/kubectl.

If k/k migrate to distribution v3, we should also keep up to date.

https://github.com/kubernetes/kubernetes/blob/master/pkg/kubelet/images/image_manager.go#L23

https://github.com/kubernetes/kubernetes/blob/cac53883f4714452f3084a22e4be20d042a9df33/staging/src/k8s.io/kubectl/pkg/cmd/debug/debug.go#L266-L269

@neolit123
Copy link
Member

I think kubeadm better keep consistent behavior with kubelet.

ok, +1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/bug Categorizes issue or PR as related to a bug. priority/backlog Higher priority than priority/awaiting-more-evidence.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants