Skip to content

Commit

Permalink
Fix
Browse files Browse the repository at this point in the history
  • Loading branch information
@ttakahashi21
ttakahashi21 committed Nov 29, 2022
1 parent 9ed9982 commit 791ebf9
Show file tree
Hide file tree
Showing 3 changed files with 130 additions and 114 deletions.
10 changes: 7 additions & 3 deletions deploy/kubernetes/rbac.yaml
View file
Expand Up @@ -57,9 +57,13 @@ rules:
- apiGroups: ["storage.k8s.io"]
resources: ["volumeattachments"]
verbs: ["get", "list", "watch"]
- apiGroups: ["gateway.networking.k8s.io"]
resources: ["referencegrants"]
verbs: ["get", "list", "watch"]
# (Alpha) Access to referencegrants is only needed when the CSI driver
# has the CrossNamespaceVolumeDataSource controller capability.
# In that case, external-provisioner requires "get", "list", "watch"
# permissions for "referencegrants" on "gateway.networking.k8s.io".
#- apiGroups: ["gateway.networking.k8s.io"]
# resources: ["referencegrants"]
# verbs: ["get", "list", "watch"]

---
kind: ClusterRoleBinding
Expand Down
193 changes: 84 additions & 109 deletions pkg/controller/controller.go
View file
Expand Up @@ -546,9 +546,12 @@ func (p *csiProvisioner) prepareProvision(ctx context.Context, claim *v1.Persist

if !utilfeature.DefaultFeatureGate.Enabled(features.CrossNamespaceVolumeDataSource) &&
claim.Spec.DataSourceRef != nil && claim.Spec.DataSourceRef.Namespace != nil && len(*claim.Spec.DataSourceRef.Namespace) > 0 {
return nil, controller.ProvisioningFinished, fmt.Errorf("error handling for DataSourceRef Type %s with non-empty namespace by Name %s: CrossNamespaceVolumeDataSource feature disabled", claim.Spec.DataSourceRef.Kind, claim.Spec.DataSourceRef.Name)
return nil, controller.ProvisioningFinished, fmt.Errorf("dataSourceRef namespace specified but the CrossNamespaceVolumeDataSource feature is disabled")
}

// normalize dataSource and dataSourceRef.
dataSource, _ := dataSource(claim)

migratedVolume := false
if p.supportsMigrationFromInTreePluginName != "" {
// NOTE: we cannot depend on PVC.Annotations[volume.beta.kubernetes.io/storage-provisioner] to get
Expand All @@ -570,16 +573,17 @@ func (p *csiProvisioner) prepareProvision(ctx context.Context, claim *v1.Persist

// Make sure the plugin is capable of fulfilling the requested options
rc := &requiredCapabilities{}
if claim.Spec.DataSource != nil {
if claim.Spec.DataSource != nil ||
(claim.Spec.DataSourceRef != nil && utilfeature.DefaultFeatureGate.Enabled(features.CrossNamespaceVolumeDataSource)) {
// PVC.Spec.DataSource.Name is the name of the VolumeSnapshot API object
if claim.Spec.DataSource.Name == "" {
if dataSource.Name == "" {
return nil, controller.ProvisioningFinished, fmt.Errorf("the PVC source not found for PVC %s", claim.Name)
}

switch claim.Spec.DataSource.Kind {
switch dataSource.Kind {
case snapshotKind:
if *(claim.Spec.DataSource.APIGroup) != snapshotAPIGroup {
return nil, controller.ProvisioningFinished, fmt.Errorf("the PVC source does not belong to the right APIGroup. Expected %s, Got %s", snapshotAPIGroup, *(claim.Spec.DataSource.APIGroup))
if dataSource.APIVersion != snapshotAPIGroup {
return nil, controller.ProvisioningFinished, fmt.Errorf("the PVC source does not belong to the right APIGroup. Expected %s, Got %s", snapshotAPIGroup, dataSource.APIVersion)
}
rc.snapshot = true
case pvcKind:
Expand All @@ -590,28 +594,7 @@ func (p *csiProvisioner) prepareProvision(ctx context.Context, claim *v1.Persist
p.eventRecorder.Event(claim, v1.EventTypeNormal, "Provisioning", fmt.Sprintf("Assuming an external populator will provision the volume"))
return nil, controller.ProvisioningFinished, &controller.IgnoredError{
Reason: fmt.Sprintf("data source (%s) is not handled by the provisioner, assuming an external populator will provision it",
claim.Spec.DataSource.Kind),
}
}
}
if utilfeature.DefaultFeatureGate.Enabled(features.CrossNamespaceVolumeDataSource) {
if claim.Spec.DataSourceRef != nil {
switch claim.Spec.DataSourceRef.Kind {
case snapshotKind:
if *(claim.Spec.DataSourceRef.APIGroup) != snapshotAPIGroup {
return nil, controller.ProvisioningFinished, fmt.Errorf("the PVC source does not belong to the right APIGroup. Expected %s, Got %s", snapshotAPIGroup, *(claim.Spec.DataSourceRef.APIGroup))
}
rc.snapshot = true
case pvcKind:
rc.clone = true
default:
// DataSourceRef is not VolumeSnapshot and PVC
// Assume external data populator to create the volume, and there is no more work for us to do
p.eventRecorder.Event(claim, v1.EventTypeNormal, "Provisioning", fmt.Sprintf("Assuming an external populator will provision the volume"))
return nil, controller.ProvisioningFinished, &controller.IgnoredError{
Reason: fmt.Sprintf("data Source (%s) is not handled by the provisioner, assuming an external populator will provision it",
claim.Spec.DataSourceRef.Kind),
}
dataSource.Kind),
}
}
}
Expand Down Expand Up @@ -666,7 +649,7 @@ func (p *csiProvisioner) prepareProvision(ctx context.Context, claim *v1.Persist
}

if (claim.Spec.DataSource != nil || claim.Spec.DataSourceRef != nil) && (rc.clone || rc.snapshot) {
volumeContentSource, err := p.getVolumeContentSource(ctx, claim, sc)
volumeContentSource, err := p.getVolumeContentSource(ctx, claim, sc, dataSource)
if err != nil && claim.Spec.DataSource != nil {
return nil, controller.ProvisioningNoChange, fmt.Errorf("error getting handle for DataSource Type %s by Name %s: %v", claim.Spec.DataSource.Kind, claim.Spec.DataSource.Name, err)
}
Expand All @@ -677,7 +660,7 @@ func (p *csiProvisioner) prepareProvision(ctx context.Context, claim *v1.Persist
}

if (claim.Spec.DataSource != nil || claim.Spec.DataSourceRef != nil) && rc.clone {
err = p.setCloneFinalizer(ctx, claim)
err = p.setCloneFinalizer(ctx, claim, dataSource)
if err != nil {
return nil, controller.ProvisioningNoChange, err
}
Expand Down Expand Up @@ -963,20 +946,16 @@ func (p *csiProvisioner) Provision(ctx context.Context, options controller.Provi
return pv, controller.ProvisioningFinished, nil
}

func (p *csiProvisioner) setCloneFinalizer(ctx context.Context, pvc *v1.PersistentVolumeClaim) error {
func (p *csiProvisioner) setCloneFinalizer(ctx context.Context, pvc *v1.PersistentVolumeClaim, dataSource v1.ObjectReference) error {
var claim *v1.PersistentVolumeClaim
var err error

if pvc.Spec.DataSourceRef != nil {
if pvc.Spec.DataSourceRef.Namespace != nil {
claim, err = p.claimLister.PersistentVolumeClaims(*pvc.Spec.DataSourceRef.Namespace).Get(pvc.Spec.DataSourceRef.Name)
} else {
claim, err = p.claimLister.PersistentVolumeClaims(pvc.Namespace).Get(pvc.Spec.DataSourceRef.Name)
}

} else if pvc.Spec.DataSource != nil {
claim, err = p.claimLister.PersistentVolumeClaims(pvc.Namespace).Get(pvc.Spec.DataSource.Name)
if len(dataSource.Namespace) > 0 {
claim, err = p.claimLister.PersistentVolumeClaims(dataSource.Namespace).Get(dataSource.Name)
} else {
claim, err = p.claimLister.PersistentVolumeClaims(pvc.Namespace).Get(dataSource.Name)
}

if err != nil {
return err
}
Expand Down Expand Up @@ -1031,72 +1010,47 @@ func removePrefixedParameters(param map[string]string) (map[string]string, error
// currently we provide Snapshot and PVC, the default case allows the provisioner to still create a volume
// so that an external controller can act upon it. Additional DataSource types can be added here with
// an appropriate implementation function
func (p *csiProvisioner) getVolumeContentSource(ctx context.Context, claim *v1.PersistentVolumeClaim, sc *storagev1.StorageClass) (*csi.VolumeContentSource, error) {
if claim.Spec.DataSourceRef != nil {
switch claim.Spec.DataSourceRef.Kind {
case snapshotKind:
return p.getSnapshotSource(ctx, claim, sc)
case pvcKind:
return p.getPVCSource(ctx, claim, sc)
default:
// For now we shouldn't pass other things to this function, but treat it as a noop and extend as needed
return nil, nil
}
} else {
switch claim.Spec.DataSource.Kind {
case snapshotKind:
return p.getSnapshotSource(ctx, claim, sc)
case pvcKind:
return p.getPVCSource(ctx, claim, sc)
default:
// For now we shouldn't pass other things to this function, but treat it as a noop and extend as needed
return nil, nil
}
func (p *csiProvisioner) getVolumeContentSource(ctx context.Context, claim *v1.PersistentVolumeClaim, sc *storagev1.StorageClass, dataSource v1.ObjectReference) (*csi.VolumeContentSource, error) {
switch dataSource.Kind {
case snapshotKind:
return p.getSnapshotSource(ctx, claim, sc, dataSource)
case pvcKind:
return p.getPVCSource(ctx, claim, sc, dataSource)
default:
// For now we shouldn't pass other things to this function, but treat it as a noop and extend as needed
return nil, nil
}
}

// getPVCSource verifies DataSource.Kind of type PersistentVolumeClaim, making sure that the requested PVC is available/ready
// returns the VolumeContentSource for the requested PVC
func (p *csiProvisioner) getPVCSource(ctx context.Context, claim *v1.PersistentVolumeClaim, sc *storagev1.StorageClass) (*csi.VolumeContentSource, error) {
func (p *csiProvisioner) getPVCSource(ctx context.Context, claim *v1.PersistentVolumeClaim, sc *storagev1.StorageClass, dataSource v1.ObjectReference) (*csi.VolumeContentSource, error) {

var sourcePVC *v1.PersistentVolumeClaim
var err error
if claim.Spec.DataSourceRef != nil {
if claim.Spec.DataSourceRef.Namespace != nil && len(*claim.Spec.DataSourceRef.Namespace) > 0 {
if utilfeature.DefaultFeatureGate.Enabled(features.CrossNamespaceVolumeDataSource) {
if claim.Namespace != *claim.Spec.DataSourceRef.Namespace {
if ok, err := p.IsGranted(ctx, claim); err != nil || !ok {
return nil, fmt.Errorf("accessing volume %s/%s from %s/%s isn't allowed", *claim.Spec.DataSourceRef.Namespace, claim.Spec.DataSourceRef.Name, claim.Namespace, claim.Name)
}
if len(dataSource.Namespace) > 0 {
if utilfeature.DefaultFeatureGate.Enabled(features.CrossNamespaceVolumeDataSource) {
if claim.Namespace != dataSource.Namespace {
if ok, err := p.IsGranted(ctx, claim); err != nil || !ok {
return nil, fmt.Errorf("accessing volume %s/%s from %s/%s isn't allowed", dataSource.Namespace, dataSource.Name, claim.Namespace, claim.Name)
}
sourcePVC, err = p.claimLister.PersistentVolumeClaims(*claim.Spec.DataSourceRef.Namespace).Get(claim.Spec.DataSourceRef.Name)

} else {
return nil, fmt.Errorf("error handling for DataSourceRef Type %s with non-empty namespace by Name %s: CrossNamespaceVolumeDataSource feature disabled", claim.Spec.DataSourceRef.Kind, claim.Spec.DataSourceRef.Name)
}
sourcePVC, err = p.claimLister.PersistentVolumeClaims(dataSource.Namespace).Get(dataSource.Name)

} else {
sourcePVC, err = p.claimLister.PersistentVolumeClaims(claim.Namespace).Get(claim.Spec.DataSourceRef.Name)
}
if err != nil {
return nil, fmt.Errorf("error getting PVC %s (namespace %q) from api server: %v", claim.Spec.DataSourceRef.Name, claim.Namespace, err)
}
if string(sourcePVC.Status.Phase) != "Bound" {
return nil, fmt.Errorf("the PVC DataSource %s must have a status of Bound. Got %v", claim.Spec.DataSourceRef.Name, sourcePVC.Status)
}
if sourcePVC.ObjectMeta.DeletionTimestamp != nil {
return nil, fmt.Errorf("the PVC DataSource %s is currently being deleted", claim.Spec.DataSourceRef.Name)
return nil, fmt.Errorf("error handling for DataSourceRef Type %s with non-empty namespace by Name %s: CrossNamespaceVolumeDataSource feature disabled", dataSource.Kind, dataSource.Name)
}
} else {
sourcePVC, err = p.claimLister.PersistentVolumeClaims(claim.Namespace).Get(claim.Spec.DataSource.Name)
if err != nil {
return nil, fmt.Errorf("error getting PVC %s (namespace %q) from api server: %v", claim.Spec.DataSource.Name, claim.Namespace, err)
}
if string(sourcePVC.Status.Phase) != "Bound" {
return nil, fmt.Errorf("the PVC DataSource %s must have a status of Bound. Got %v", claim.Spec.DataSource.Name, sourcePVC.Status)
}
if sourcePVC.ObjectMeta.DeletionTimestamp != nil {
return nil, fmt.Errorf("the PVC DataSource %s is currently being deleted", claim.Spec.DataSource.Name)
}
sourcePVC, err = p.claimLister.PersistentVolumeClaims(claim.Namespace).Get(dataSource.Name)
}
if err != nil {
return nil, fmt.Errorf("error getting PVC %s (namespace %q) from api server: %v", dataSource.Name, claim.Namespace, err)
}
if string(sourcePVC.Status.Phase) != "Bound" {
return nil, fmt.Errorf("the PVC DataSource %s must have a status of Bound. Got %v", dataSource.Name, sourcePVC.Status)
}
if sourcePVC.ObjectMeta.DeletionTimestamp != nil {
return nil, fmt.Errorf("the PVC DataSource %s is currently being deleted", dataSource.Name)
}

if sourcePVC.Spec.StorageClassName == nil {
Expand Down Expand Up @@ -1175,12 +1129,13 @@ func (p *csiProvisioner) getPVCSource(ctx context.Context, claim *v1.PersistentV
return volumeContentSource, nil
}

// TODO: This function should eventually be replaced by a common kubernetes library.
func (p *csiProvisioner) IsGranted(ctx context.Context, claim *v1.PersistentVolumeClaim) (bool, error) {
if !utilfeature.DefaultFeatureGate.Enabled(features.CrossNamespaceVolumeDataSource) {
return false, nil
}

// Get all ReferenceGrants in snapshot's namespace
// Get all ReferenceGrants in data source's namespace
referenceGrants, err := p.referenceGrantLister.ReferenceGrants(*claim.Spec.DataSourceRef.Namespace).List(labels.Everything())
if err != nil {
return false, fmt.Errorf("error getting ReferenceGrants in %s namespace from api server: %v", *claim.Spec.DataSourceRef.Namespace, err)
Expand All @@ -1202,7 +1157,7 @@ func (p *csiProvisioner) IsGranted(ctx context.Context, claim *v1.PersistentVolu
}

for _, to := range grant.Spec.To {
if claim.Spec.DataSourceRef.APIGroup != nil && (string(to.Group) != *claim.Spec.DataSourceRef.APIGroup || string(to.Kind) != claim.Spec.DataSourceRef.Kind) {
if (claim.Spec.DataSourceRef.APIGroup != nil && string(to.Group) != *claim.Spec.DataSourceRef.APIGroup) || string(to.Kind) != claim.Spec.DataSourceRef.Kind {
continue
}
if to.Name == nil || string(*to.Name) == "" || string(*to.Name) == claim.Spec.DataSourceRef.Name {
Expand All @@ -1217,32 +1172,28 @@ func (p *csiProvisioner) IsGranted(ctx context.Context, claim *v1.PersistentVolu
}

if !allowed {
return false, nil
return allowed, nil
}

return true, nil
}

// getSnapshotSource verifies DataSource.Kind of type VolumeSnapshot, making sure that the requested Snapshot is available/ready
// returns the VolumeContentSource for the requested snapshot
func (p *csiProvisioner) getSnapshotSource(ctx context.Context, claim *v1.PersistentVolumeClaim, sc *storagev1.StorageClass) (*csi.VolumeContentSource, error) {
if claim.Spec.DataSourceRef != nil {
if claim.Spec.DataSourceRef.Namespace != nil && len(*claim.Spec.DataSourceRef.Namespace) > 0 {
if utilfeature.DefaultFeatureGate.Enabled(features.CrossNamespaceVolumeDataSource) {
if claim.Namespace != *claim.Spec.DataSourceRef.Namespace {
if ok, err := p.IsGranted(ctx, claim); err != nil || !ok {
return nil, fmt.Errorf("accessing snapshot %s/%s from %s/%s isn't allowed", *claim.Spec.DataSourceRef.Namespace, claim.Spec.DataSourceRef.Name, claim.Namespace, claim.Name)
}
func (p *csiProvisioner) getSnapshotSource(ctx context.Context, claim *v1.PersistentVolumeClaim, sc *storagev1.StorageClass, dataSource v1.ObjectReference) (*csi.VolumeContentSource, error) {
if len(dataSource.Namespace) > 0 {
if utilfeature.DefaultFeatureGate.Enabled(features.CrossNamespaceVolumeDataSource) {
if claim.Namespace != dataSource.Namespace {
if ok, err := p.IsGranted(ctx, claim); err != nil || !ok {
return nil, fmt.Errorf("accessing snapshot %s/%s from %s/%s isn't allowed", dataSource.Namespace, dataSource.Name, claim.Namespace, claim.Name)
}
return p.getSnapshotSourceInternal(ctx, claim, sc, *claim.Spec.DataSourceRef.Namespace, claim.Spec.DataSourceRef.Name)
} else {
return nil, fmt.Errorf("error handling for DataSourceRef Type %s with non-empty namespace by Name %s: CrossNamespaceVolumeDataSource feature disabled", claim.Spec.DataSourceRef.Kind, claim.Spec.DataSourceRef.Name)
}
return p.getSnapshotSourceInternal(ctx, claim, sc, dataSource.Namespace, dataSource.Name)
} else {
return p.getSnapshotSourceInternal(ctx, claim, sc, claim.Namespace, claim.Spec.DataSourceRef.Name)
return nil, fmt.Errorf("error handling for DataSourceRef Type %s with non-empty namespace by Name %s: CrossNamespaceVolumeDataSource feature disabled", dataSource.Kind, dataSource.Name)
}
} else {
return p.getSnapshotSourceInternal(ctx, claim, sc, claim.Namespace, claim.Spec.DataSource.Name)
return p.getSnapshotSourceInternal(ctx, claim, sc, claim.Namespace, dataSource.Name)
}
}

Expand Down Expand Up @@ -2043,3 +1994,27 @@ func checkFinalizer(obj metav1.Object, finalizer string) bool {
func markAsMigrated(parent context.Context, hasMigrated bool) context.Context {
return context.WithValue(parent, connection.AdditionalInfoKey, connection.AdditionalInfo{Migrated: strconv.FormatBool(hasMigrated)})
}

func dataSource(claim *v1.PersistentVolumeClaim) (v1.ObjectReference, error) {
var dataSource v1.ObjectReference

if claim.Spec.DataSource != nil {
dataSource.Kind = claim.Spec.DataSource.Kind
dataSource.Name = claim.Spec.DataSource.Name

if claim.Spec.DataSource.APIGroup != nil {
dataSource.APIVersion = *claim.Spec.DataSource.APIGroup
}
} else if claim.Spec.DataSourceRef != nil {
dataSource.Kind = claim.Spec.DataSourceRef.Kind
dataSource.Name = claim.Spec.DataSourceRef.Name
if claim.Spec.DataSourceRef.APIGroup != nil {
dataSource.APIVersion = *claim.Spec.DataSourceRef.APIGroup
}
if claim.Spec.DataSourceRef.Namespace != nil {
dataSource.Namespace = *claim.Spec.DataSourceRef.Namespace
}
}

return dataSource, nil
}
41 changes: 39 additions & 2 deletions pkg/controller/controller_test.go
View file
Expand Up @@ -3569,7 +3569,7 @@ func TestProvisionFromSnapshot(t *testing.T) {
refGrant: newRefGrant("refGrant1", ns1, ns2, apiGrp, "VolumeSnapshot"),
snapNamespace: ns1,
},
"fail provision with xns volume snapshot data source with bad refgrant when CrossNamespaceVolumeDataSource feature enabled": {
"fail provision with xns volume snapshot data source with bad API Group refgrant when CrossNamespaceVolumeDataSource feature enabled": {
volOpts: controller.ProvisionOptions{
StorageClass: &storagev1.StorageClass{
ReclaimPolicy: &deletePolicy,
Expand Down Expand Up @@ -3603,7 +3603,44 @@ func TestProvisionFromSnapshot(t *testing.T) {
snapshotStatusReady: true,
expectErr: true,
xnsEnabled: true,
refGrant: newRefGrant("refGrant1", "badnamespace", ns2, apiGrp, "VolumeSnapshot"),
refGrant: newRefGrant("refGrant1", ns1, ns2, apiGrp, "PersistentVolumeClaim"),
snapNamespace: ns1,
},
"fail provision with xns volume snapshot data source with bad namespace refgrant when CrossNamespaceVolumeDataSource feature enabled": {
volOpts: controller.ProvisionOptions{
StorageClass: &storagev1.StorageClass{
ReclaimPolicy: &deletePolicy,
Parameters: map[string]string{},
Provisioner: "test-driver",
},
PVName: "test-name",
PVC: &v1.PersistentVolumeClaim{
ObjectMeta: metav1.ObjectMeta{
UID: "testid",
Annotations: driverNameAnnotation,
Namespace: ns2,
},
Spec: v1.PersistentVolumeClaimSpec{
StorageClassName: &snapClassName,
Resources: v1.ResourceRequirements{
Requests: v1.ResourceList{
v1.ResourceName(v1.ResourceStorage): resource.MustParse(strconv.FormatInt(requestedBytes, 10)),
},
},
AccessModes: []v1.PersistentVolumeAccessMode{v1.ReadWriteOnce},
DataSourceRef: &v1.TypedObjectReference{
Name: snapName,
Kind: "VolumeSnapshot",
APIGroup: &apiGrp,
Namespace: &ns1,
},
},
},
},
snapshotStatusReady: true,
expectErr: true,
xnsEnabled: true,
refGrant: newRefGrant("refGrant1", ns1, "badnamespace", "", "PersistentVolumeClaim"),
snapNamespace: ns1,
},
}
Expand Down

0 comments on commit 791ebf9

Please sign in to comment.