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

Propagation of NodePublishVolumeRequest.VolumeContext to providers #475

Closed
micahhausler opened this issue Mar 9, 2021 · 4 comments · Fixed by #848
Closed

Propagation of NodePublishVolumeRequest.VolumeContext to providers #475

micahhausler opened this issue Mar 9, 2021 · 4 comments · Fixed by #848
Assignees
Labels
feature/rotation kind/feature Categorizes issue or PR as related to a new feature.
Milestone

Comments

@micahhausler
Copy link
Member

Describe the solution you'd like

I was wondering if it is intentional to filter out VolumeContext (Pod .spec.volumes[].csi.volumeAttributes) parameters passed on to providers by limiting the key to:

  • csi.storage.k8s.io/pod.name
  • csi.storage.k8s.io/pod.namespace
  • csi.storage.k8s.io/pod.uid
  • csi.storage.k8s.io/serviceAccount.name
  • SecretProviderClass .Spec.Parameters

Is the pod creator considered untrusted? It might be convenient for providers to have access to volumeAttributes for per-volume metadata.

@micahhausler micahhausler added the kind/feature Categorizes issue or PR as related to a new feature. label Mar 9, 2021
@aramase
Copy link
Member

aramase commented Mar 15, 2021

The volume attributes are user defined apart from the default ones added by kubelet.

csi.storage.k8s.io/ephemeral:true 
csi.storage.k8s.io/pod.name:nginx-78cfc8db98-5s5rr 
csi.storage.k8s.io/pod.namespace:default 
csi.storage.k8s.io/pod.uid:9f88edc9-4b36-4405-9802-7aa621df338c 
csi.storage.k8s.io/serviceAccount.name:default 
secretProviderClass:testspc

The change should be minimal in NodePublishVolume. However we need to reconstruct these attributes in the rotation logic as rotation isn't invoked by kubelet.

// Set these parameters to mimic the exact same attributes we get as part of NodePublishVolumeRequest
parameters[csipodname] = podName
parameters[csipodnamespace] = podNamespace
parameters[csipoduid] = string(pod.UID)
parameters[csipodsa] = pod.Spec.ServiceAccountName

If we explore the option of doing rotation based on the requiresRepublish and it's feasible, then we might be able to just rely solely on NodePublishVolume and pass the whole volume attributes as is to the provider.

@aramase aramase added this to the v0.1.0 milestone May 28, 2021
@tam7t
Copy link
Contributor

tam7t commented Jun 15, 2021

Based on some of the discussion on #585 we likely don't want plugins to rely on having this information (since some may be missing during rotation requests) but that a good start is to pass all attributes on any real request.

IMO we should do this and close it once we pass all attributes, and work on #585 toward providing more attributes in a backwards-compatible way for the rotation feature.

/assign @aramase

@aramase aramase removed this from the v0.1.0 milestone Jul 21, 2021
@tam7t tam7t added this to the v1.1.0 milestone Oct 13, 2021
@k8s-triage-robot
Copy link

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-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jan 11, 2022
@aramase
Copy link
Member

aramase commented Jan 31, 2022

/remove-lifecycle stale

@k8s-ci-robot k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jan 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature/rotation kind/feature Categorizes issue or PR as related to a new feature.
Projects
None yet
5 participants