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.

Sign up for GitHub

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

Jump to bottom

Add user specified image to CRI ContainerConfig #118652

Merged
merged 1 commit into from
Jul 6, 2023
Merged

Add user specified image to CRI ContainerConfig #118652

merged 1 commit into from
Jul 6, 2023

Conversation

saschagrunert
Copy link
Member

@saschagrunert saschagrunert commented Jun 14, 2023 •
edited
Loading

What type of PR is this?

/kind cleanup

What this PR does / why we need it:

The container config image references either an image ID or a digest, but not the original image from the container config. We require the image for signature verification to ensure that we actually verify the correct image.

Which issue(s) this PR fixes:

Refers to cri-o/cri-o#7046

Special notes for your reviewer:

None

Does this PR introduce a user-facing change?

None

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

None

@k8s-ci-robot k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. release-note-none Denotes a PR that doesn't merit a release note. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. area/kubelet sig/node Categorizes an issue or PR as relevant to SIG Node. and removed do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Jun 14, 2023
@saschagrunert saschagrunert mentioned this pull request Jun 14, 2023
Closed
@saschagrunert saschagrunert changed the title WIP: Add user specified image to CRI ContainerConfig Add user specified image to CRI ContainerConfig Jun 14, 2023
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 14, 2023
@saschagrunert
Copy link
Member Author

/hold

for discussion

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 14, 2023
@saschagrunert saschagrunert force-pushed the cri-container-config-user-defined-image branch from bcd38cc to 19c7e7f Compare June 14, 2023 08:43
@saschagrunert
Copy link
Member Author

/unhold that's the way to go

@saschagrunert saschagrunert removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 15, 2023
@bart0sh
Copy link
Contributor

bart0sh commented Jun 20, 2023

/triage accepted
/priority important-longterm

@k8s-ci-robot k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Jun 20, 2023
@saschagrunert saschagrunert changed the title Add user specified image to CRI ContainerConfig WIP: Add user specified image to CRI ContainerConfig Jun 22, 2023
@k8s-ci-robot k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 22, 2023
@saschagrunert saschagrunert force-pushed the cri-container-config-user-defined-image branch from 78cd81d to 5ccd2a1 Compare June 22, 2023 09:34
@saschagrunert
Copy link
Member Author

/unhold

We need the auth and the image name to be able to verify images on container creation. The auth is required as well for private registries.

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 26, 2023
@saschagrunert saschagrunert force-pushed the cri-container-config-user-defined-image branch from 5ccd2a1 to 148dd5b Compare July 5, 2023 18:09
@k8s-ci-robot k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Jul 5, 2023
@saschagrunert
Copy link
Member Author

Auth can be supported later on and we now focus on the user specified image for now.

PTAL @kubernetes/sig-node-pr-reviews

mikebrow
mikebrow reviewed Jul 5, 2023
View reviewed changes
Copy link
Member

@mikebrow mikebrow left a comment

Choose a reason for hiding this comment

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

maybe consider adding UserSpecifiedImage to the ImageSpec struct vs at the root level of ContainerConfig...

@saschagrunert
Copy link
Member Author

maybe consider adding UserSpecifiedImage to the ImageSpec struct vs at the root level of ContainerConfig...

Sure, I can add it to the ImageSpec.

@saschagrunert
Add user specified image to CRI ContainerConfig
20a25cb 
The container config image references either an image ID or a digest,
but not the original image from the container config. We require the
image for signature verification to ensure that we actually verify the
correct image.

Signed-off-by: Sascha Grunert <sgrunert@redhat.com>
@saschagrunert saschagrunert force-pushed the cri-container-config-user-defined-image branch from 148dd5b to 20a25cb Compare July 6, 2023 06:40
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mikebrow, mrunalp, saschagrunert

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 6, 2023
@mrunalp
Copy link
Contributor

mrunalp commented Jul 6, 2023

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 6, 2023
@k8s-ci-robot
Copy link
Contributor

LGTM label has been added.

Git tree hash: cf7dbb62839c46688c7be2034a1d235704173d20

@k8s-ci-robot k8s-ci-robot merged commit f66ba8c into kubernetes:master Jul 6, 2023
@k8s-ci-robot k8s-ci-robot added this to the v1.28 milestone Jul 6, 2023
@saschagrunert saschagrunert deleted the cri-container-config-user-defined-image branch July 6, 2023 17:20
@saschagrunert saschagrunert mentioned this pull request Jul 7, 2023
Closed
saschagrunert added a commit to saschagrunert/kubernetes that referenced this pull request Jul 7, 2023
@saschagrunert
UPSTREAM: <drop>: Add user specified image to CRI ContainerConfig
798d800 
The container config image references either an image ID or a digest,
but not the original image from the container config. We require the
image for signature verification to ensure that we actually verify the
correct image.

Cherry-pick of kubernetes#118652

Signed-off-by: Sascha Grunert <sgrunert@redhat.com>
@saschagrunert saschagrunert mentioned this pull request Dec 12, 2023
Closed
@mtrmac mtrmac mentioned this pull request Aug 28, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mtrmac mtrmac mtrmac left review comments

@mikebrow mikebrow mikebrow approved these changes

@mrunalp mrunalp mrunalp approved these changes

@haircommander haircommander Awaiting requested review from haircommander

@pacoxu pacoxu Awaiting requested review from pacoxu

Assignees

@mrunalp mrunalp

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/kubelet cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. release-note-none Denotes a PR that doesn't merit a release note. sig/node Categorizes an issue or PR as relevant to SIG Node. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
SIG Node: code and documentation PRs
Archived in project
Milestone
v1.28
Development

Successfully merging this pull request may close these issues.
6 participants
@saschagrunert @bart0sh @k8s-ci-robot @mrunalp @mtrmac @mikebrow