azurefile-csi-driver icon indicating copy to clipboard operation
azurefile-csi-driver copied to clipboard

For v1.12.0 pull images by digest instead of tags

Open grtn316 opened this issue 3 years ago • 7 comments

What type of PR is this? /kind feature /kind bug

What this PR does / why we need it: feature: version 1.12.0 supports pulling images using digests instead of tags. This prevents pointing the tag to another digest and causing an accidental update in production scenarios.

bug: The installer is pointed at the version branch instead of the master branch. I have updated it to point to /master/depoy/$ver instead of /$ver/deploy/$ver. This makes it possible for forks to use their localized versions correctly without having to fork all of the version branches as well.

Which issue(s) this PR fixes: Fixes #

Requirements:

Special notes for your reviewer:

Release note:

none

grtn316 avatar Jul 28 '22 19:07 grtn316

Hi @grtn316. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions 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/test-infra repository.

k8s-ci-robot avatar Jul 28 '22 19:07 k8s-ci-robot

CLA Signed

The committers listed above are authorized under a signed CLA.

  • :white_check_mark: login: grtn316 / name: David Baumgarten (be67e4cd61f5cc2ca54b0b02ecf0a92cbab7965b, 32013382675fd7ddb5b10710dd24402ff43bbc73, bf3d4ab367d4333346bba05aef08c4d17f3f15fb, b4b21b993beace79318d742da204b3e923013c23)

Welcome @grtn316!

It looks like this is your first PR to kubernetes-sigs/azurefile-csi-driver 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/azurefile-csi-driver has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. :smiley:

k8s-ci-robot avatar Jul 28 '22 19:07 k8s-ci-robot

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: grtn316 Once this PR has been reviewed and has the lgtm label, please assign andyzhangx for approval by writing /assign @andyzhangx in a comment. For more information see:The Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found 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 avatar Jul 28 '22 19:07 k8s-ci-robot

hi @grtn316 thanks for the contribution. the image under mcr.microsoft.com/k8s/csi/azurefile-csi/ is immutable after publish, so it's not necessary to use digest, also using tags could easily show which version it is, the k8s upstream are also using tags in csi driver deployment.

andyzhangx avatar Jul 30 '22 13:07 andyzhangx

Hi @andyzhangx while I may not have all the details you require to magnify why using digests for the actual install is important, I will attempt to spotlight some examples and current issues that may help:

The images currently in the repo: mcr.microsoft.com/k8s/csi/azurefile-csi/ are tagged but also have digests. You can either reference the tag or the digest in the deployment yamls.

Risks of using tags in the deployment:

  • Underlying image for the tag can technically be changed without the cluster administrator knowing. This control is in the hands of the image owner versus the cluster owner (since they are using the install scripts that reference the tag). Resulting in randomly failed pods in production when the cluster admin is doing maintenace. I believe this could be the cause of this issue: #1020. We have not changed anything on our side and the issue suddenly showed up. We have been pinned to v1.12.0 since it came out. I may be able to recover an old cluster to look at the image:tag on one of the nodes to see if the digest matches the current image that is tagged in mcr.microsoft.com/k8s/csi/azurefile-csi/ but I suspect the underlying image for tag: 1.12.0 may have gotten updated at some point to fix a bug or security vulnerability and accidently included the change discovered in the #1020 issue.
  • When using tags, mirrors or malicious mirrors are able to republish a malicious image with the same name:tag and cluster admins could accidently pull down this image causing a security vulnerability.

Benefits of using digest references in the deployment yamls:

  • When the underlying image for a tag is updated/changed the cluster will not pull the new version as it is pinned to an immutable digest for that specific build.
  • Malicious images would have a new digest reducing the immediately blast radius since clusters do not target the new digest.

Comments: I was able to mirror the images from mcr.microsoft.com/k8s/csi/azurefile-csi/ using skopeo to an azure container registry and update the deployment yamls to target the digests for v1.12.0 instead of the tags (as seen in this PR). Tags can still be used for convenience but referencing the digest for the deployment creates a more consistent and secure deployment for users.

If my suspicion is correct for #1020 then this would invalidate that once an image is tagged in mcr.microsoft.com/k8s/csi/azurefile-csi/ that it will never be updated. Since the tag pointer is not technically immutable, I suspect you are referring to the process of how tags are currently maintained in that repository.

I also created a feature request here to address the digest topic #1062

grtn316 avatar Jul 30 '22 14:07 grtn316

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue or PR as fresh with /remove-lifecycle stale
  • Mark this issue or PR as rotten with /lifecycle rotten
  • Close this issue or PR with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

k8s-triage-robot avatar Oct 28 '22 15:10 k8s-triage-robot

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue or PR as fresh with /remove-lifecycle rotten
  • Close this issue or PR with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

k8s-triage-robot avatar Nov 27 '22 15:11 k8s-triage-robot

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

  • Reopen this PR with /reopen
  • Mark this PR as fresh with /remove-lifecycle rotten
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close

k8s-triage-robot avatar Dec 27 '22 16:12 k8s-triage-robot

@k8s-triage-robot: Closed this PR.

In response to this:

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

  • Reopen this PR with /reopen
  • Mark this PR as fresh with /remove-lifecycle rotten
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close

Instructions 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/test-infra repository.

k8s-ci-robot avatar Dec 27 '22 16:12 k8s-ci-robot