Skip to content

implement image check for oci pull#635

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

implement image check for oci pull#635
mac641 wants to merge 7 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
Comment thread cmd/metal-api/internal/service/image-service.go Outdated
Comment thread cmd/metal-api/internal/service/partition-service.go Outdated
Comment thread go.mod Outdated
@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

Comment thread cmd/metal-api/internal/service/image-service.go Outdated
Comment thread cmd/metal-api/internal/service/image-service.go Outdated
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?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

To be frank, I haven't dug deeper into updateImage. To my understanding this is part of the imageResource which is part of the old installation method where tarballs had to be downloaded and extracted. We don't need it for oci images since we can use github.com/google/go-containerregistry/pkg/v1/remote to download/verify/... the images from a registry.

Comment thread cmd/metal-api/internal/service/image-service.go Outdated
Comment thread cmd/metal-api/internal/service/image-service.go Outdated
Comment thread cmd/metal-api/internal/service/image-service.go Outdated
Comment thread cmd/metal-api/internal/service/image-service.go Outdated
Comment thread cmd/metal-api/internal/service/image-service.go Outdated
}

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?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Since we don't have any registry yet that needs authentication, but still want to support it, I can't think of a better way than passing over nil for now.

Feel free to suggest a solution that's more to your liking and I'm happy to add it.

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.

Not right now. Could you create an issue for this? Then we can tackle this in the future.

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