Skip to content

implement image check for oci pull#635

Open
mac641 wants to merge 6 commits intomasterfrom
oci-pull
Open

implement image check for oci pull#635
mac641 wants to merge 6 commits intomasterfrom
oci-pull

Conversation

@mac641
Copy link
Copy Markdown
Contributor

@mac641 mac641 commented Dec 2, 2025

@metal-robot metal-robot bot added the area: control-plane Affects the metal-stack control-plane area. label Dec 2, 2025
@metal-robot metal-robot bot added this to Development Dec 2, 2025
@mac641 mac641 changed the title temp: comment checkImageURL in order for oci images to pass implement image check for oci pull Jan 22, 2026
… to reduce code duplication

  - fix previous copy-paste errors
@mac641 mac641 marked this pull request as ready for review January 29, 2026 12:31
@mac641 mac641 requested a review from a team as a code owner January 29, 2026 12:31
@mac641 mac641 requested review from majst01 and mwennrich and removed request for mwennrich January 29, 2026 12:31
@github-project-automation github-project-automation bot moved this to In Progress in Development Jan 29, 2026
@majst01
Copy link
Copy Markdown
Contributor

majst01 commented Jan 29, 2026

test would be nice

if res.StatusCode >= 400 {
return fmt.Errorf("image:%s is not accessible under:%s status:%s", id, url, res.Status)
func checkImageURL(id, inputURL string, ociCredentials authn.Authenticator) error {
parsedURL := strings.Split(inputURL, "://")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should use url.Parse here.

return fmt.Errorf("image:%s is not accessible under:%s error:%w", id, inputURL, err)
}
default:
return fmt.Errorf("image:%s with url:%s has unkown protocol", id, inputURL)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return fmt.Errorf("image:%s with url:%s has unkown protocol", id, inputURL)
return fmt.Errorf("image: %q with url: %s has unknown protocol", id, inputURL)

Comment on lines 414 to 445
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I understand this correctly, updateImage is impossible with oci images that require any kind of auth. Is this intended?


_, err = remote.Image(ref, remote.WithAuth(ociCredentials))
if err != nil {
return fmt.Errorf("image:%s is not accessible under:%s error:%w", id, inputURL, err)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return fmt.Errorf("image:%s is not accessible under:%s error:%w", id, inputURL, err)
return fmt.Errorf("image: %q is not accessible under: %s, error: %w", id, inputURL, err)

case "oci":
ref, err := name.ParseReference(parsedURL[1])
if err != nil {
return fmt.Errorf("image reference:%s could not be parsed. error:%w", inputURL, err)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return fmt.Errorf("image reference:%s could not be parsed. error:%w", inputURL, err)
return fmt.Errorf("image reference: %q could not be parsed, error: %w", inputURL, err)

case "http", "https":
_, err := url.ParseRequestURI(inputURL)
if err != nil {
return fmt.Errorf("image:%s could not be parsed. error:%w", inputURL, err)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return fmt.Errorf("image:%s could not be parsed. error:%w", inputURL, err)
return fmt.Errorf("image: %q could not be parsed, error: %w", inputURL, err)


res, err := http.Head(inputURL)
if err != nil {
return fmt.Errorf("image:%s is not accessible under:%s error:%w", id, inputURL, err)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return fmt.Errorf("image:%s is not accessible under:%s error:%w", id, inputURL, err)
return fmt.Errorf("image: %q is not accessible under: %s, error: %w", id, inputURL, err)

return fmt.Errorf("image:%s is not accessible under:%s error:%w", id, inputURL, err)
}
if res.StatusCode >= 400 {
return fmt.Errorf("image:%s is not accessible under:%s status:%s", id, inputURL, res.Status)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return fmt.Errorf("image:%s is not accessible under:%s status:%s", id, inputURL, res.Status)
return fmt.Errorf("image: %q is not accessible under: %s, status: %s", id, inputURL, res.Status)

}

err = checkImageURL("image", imageURL)
err = checkImageURL("image", imageURL, nil)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here and within that file: is nil intentional?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: control-plane Affects the metal-stack control-plane area.

Projects

Status: In Progress

Development

Successfully merging this pull request may close these issues.

3 participants