Repository: kubernetes/kubernetes (language: go)

Issue:
PersistentVolumeLabel admission blocks PVs on Azure + vSphere
### What happened?

In 1.30, the PersistentVolumeLabel admission plugin blocks creation of in-tree AzureDisk and vSphere PVs:

```
persistentvolumes "pvc-2d9b3999-f60d-4797-923f-210f1f75025d" is forbidden:
error querying AzureDisk volume pvc-2d9b3999-f60d-4797-923f-210f1f75025d:
unable to build Azure cloud provider for AzureDisk
```

The reason is that the cloud provider was removed (https://github.com/kubernetes/kubernetes/pull/122857), but the admission plugin still calls GetCloudProvider there, which fails: https://github.com/kubernetes/kubernetes/blob/646fbe6d0a3fa1a100d90a429f039324de9c8138/plugin/pkg/admission/storage/persistentvolume/label/admission.go#L285


### What did you expect to happen?

The PV should be admitted + created.

### How can we reproduce it (as minimally and precisely as possible)?

Enable PersistentVolumeLabel admission plugin. For example, when running local-up-cluster.sh:
```
ALLOW_PRIVILEGED=true ENABLE_ADMISSION_PLUGINS="NamespaceLifecycle,LimitRanger,ServiceAccount,DefaultStorageClass,DefaultTolerationSeconds,Priority,MutatingAdmissionWebhook,ValidatingAdmissionWebhook,ResourceQuota,NodeRestriction,PersistentVolumeLabel" bash -x hack/local-up-cluster.sh
```

Create AzureDisk PV on _any_ 1.30 cluster, you don't need Azure cloud:

```
kubectl create -f <<EOF
apiVersion: v1
kind: PersistentVolume
metadata:
  name: myvol
spec:
  accessModes:
  - ReadWriteOnce
  capacity:
    storage: 5Gi
  azureDisk:
    diskName: test2.vhd
    diskURI: https://someacount.blob.core.windows.net/vhds/test2.vhd
EOF
```

### Anything else we need to know?

/sig storage
/priority important-soon

### Kubernetes version

<details>

Today's master
```console
$ kubectl version
Server Version: v1.31.0-alpha.0.24+9791f0d1f39f3f
```

</details>


### Cloud provider

<details>
Azure
</details>


### OS version

_No response_

### Install tools

<details>
local-up-cluster.sh
</details>


### Container runtime (CRI) and version (if applicable)

<details>

</details>


### Related plugins (CNI, CSI, ...) and versions (if applicable)

<details>

</details>



Discussion / hints:
This issue is currently awaiting triage.

If a SIG or subproject determines this is a relevant issue, they will accept it by applying the `triage/accepted` label and provide further guidance.

The `triage/accepted` label can be added by org members by writing `/triage accepted` in a comment.


<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
</details>
I am fixing the worst bug in https://github.com/kubernetes/kubernetes/pull/124505 and I plan to backport it to 1.30. 

However, we should consider removing the whole PersistentVolumeLabel admission plugin. Most of the cloud providers were removed, only GCE remains and IMO even that one will be removed sooner or later.  The admission plugin has no way to get volume zone + region and is therefore useless.

The admission said (in 1.26, where we had all in-tree cloud providers):
https://github.com/kubernetes/kubernetes/blob/b46a3f887ca979b1a5d14fd39cb1af43e7e5d12d/plugin/pkg/admission/storage/persistentvolume/label/admission.go#L74-L76

Such webhook never materialized for AWS, Azure nor vSphere and I am not aware of any plans for GCE.
> Such webhook never materialized for AWS, Azure nor vSphere and I am not aware of any plans for GCE.

It did materialize in https://github.com/kubernetes-sigs/cloud-pv-admission-labeler

So removing it from k/k is the best way forward.

Fix patch:
diff --git a/pkg/kubeapiserver/options/plugins.go b/pkg/kubeapiserver/options/plugins.go
index 845dc78102a35..6b63781117f3b 100644
--- a/pkg/kubeapiserver/options/plugins.go
+++ b/pkg/kubeapiserver/options/plugins.go
@@ -49,7 +49,6 @@ import (
 	"k8s.io/kubernetes/plugin/pkg/admission/runtimeclass"
 	"k8s.io/kubernetes/plugin/pkg/admission/security/podsecurity"
 	"k8s.io/kubernetes/plugin/pkg/admission/serviceaccount"
-	"k8s.io/kubernetes/plugin/pkg/admission/storage/persistentvolume/label"
 	"k8s.io/kubernetes/plugin/pkg/admission/storage/persistentvolume/resize"
 	"k8s.io/kubernetes/plugin/pkg/admission/storage/storageclass/setdefault"
 	"k8s.io/kubernetes/plugin/pkg/admission/storage/storageobjectinuseprotection"
@@ -82,7 +81,6 @@ var AllOrderedPlugins = []string{
 	podtolerationrestriction.PluginName,     // PodTolerationRestriction
 	eventratelimit.PluginName,               // EventRateLimit
 	extendedresourcetoleration.PluginName,   // ExtendedResourceToleration
-	label.PluginName,                        // PersistentVolumeLabel
 	setdefault.PluginName,                   // DefaultStorageClass
 	storageobjectinuseprotection.PluginName, // StorageObjectInUseProtection
 	gc.PluginName,                           // OwnerReferencesPermissionEnforcement
@@ -126,7 +124,6 @@ func RegisterAllAdmissionPlugins(plugins *admission.Plugins) {
 	exists.Register(plugins)
 	noderestriction.Register(plugins)
 	nodetaint.Register(plugins)
-	label.Register(plugins) // DEPRECATED, future PVs should not rely on labels for zone topology
 	podnodeselector.Register(plugins)
 	podtolerationrestriction.Register(plugins)
 	runtimeclass.Register(plugins)
diff --git a/plugin/pkg/admission/storage/persistentvolume/label/OWNERS b/plugin/pkg/admission/storage/persistentvolume/label/OWNERS
deleted file mode 100644
index 8debd4060f2e2..0000000000000
--- a/plugin/pkg/admission/storage/persistentvolume/label/OWNERS
+++ /dev/null
@@ -1,10 +0,0 @@
-# See the OWNERS docs at https://go.k8s.io/owners
-
-reviewers:
-  - andrewsykim
-  - dims
-  - msau42
-approvers:
-  - andrewsykim
-  - dims
-  - msau42
diff --git a/plugin/pkg/admission/storage/persistentvolume/label/admission.go b/plugin/pkg/admission/storage/persistentvolume/label/admission.go
deleted file mode 100644
index 29f3ffe4942d3..0000000000000
--- a/plugin/pkg/admission/storage/persistentvolume/label/admission.go
+++ /dev/null
@@ -1,309 +0,0 @@
-/*
-Copyright 2015 The Kubernetes Authors.
-
-Licensed under the Apache License, Version 2.0 (the "License");
-you may not use this file except in compliance with the License.
-You may obtain a copy of the License at
-
-    http://www.apache.org/licenses/LICENSE-2.0
-
-Unless required by applicable law or agreed to in writing, software
-distributed under the License is distributed on an "AS IS" BASIS,
-WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-See the License for the specific language governing permissions and
-limitations under the License.
-*/
-
-package label
-
-import (
-	"bytes"
-	"context"
-	"errors"
-	"fmt"
-	"io"
-	"sync"
-
-	v1 "k8s.io/api/core/v1"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apiserver/pkg/admission"
-	cloudprovider "k8s.io/cloud-provider"
-	cloudvolume "k8s.io/cloud-provider/volume"
-	volumehelpers "k8s.io/cloud-provider/volume/helpers"
-	persistentvolume "k8s.io/component-helpers/storage/volume"
-	"k8s.io/klog/v2"
-	api "k8s.io/kubernetes/pkg/apis/core"
-	k8s_api_v1 "k8s.io/kubernetes/pkg/apis/core/v1"
-	kubeapiserveradmission "k8s.io/kubernetes/pkg/kubeapiserver/admission"
-)
-
-const (
-	// PluginName is the name of persistent volume label admission plugin
-	PluginName = "PersistentVolumeLabel"
-)
-
-// Register registers a plugin
-func Register(plugins *admission.Plugins) {
-	plugins.Register(PluginName, func(config io.Reader) (admission.Interface, error) {
-		persistentVolumeLabelAdmission := newPersistentVolumeLabel()
-		return persistentVolumeLabelAdmission, nil
-	})
-}
-
-var _ = admission.Interface(&persistentVolumeLabel{})
-
-type persistentVolumeLabel struct {
-	*admission.Handler
-
-	mutex            sync.Mutex
-	cloudConfig      []byte
-	azurePVLabeler   cloudprovider.PVLabeler
-	vspherePVLabeler cloudprovider.PVLabeler
-}
-
-var _ admission.MutationInterface = &persistentVolumeLabel{}
-var _ kubeapiserveradmission.WantsCloudConfig = &persistentVolumeLabel{}
-
-// newPersistentVolumeLabel returns an admission.Interface implementation which adds labels to PersistentVolume CREATE requests,
-// based on the labels provided by the underlying cloud provider.
-//
-// As a side effect, the cloud provider may block invalid or non-existent volumes.
-func newPersistentVolumeLabel() *persistentVolumeLabel {
-	// DEPRECATED: in a future release, we will use mutating admission webhooks to apply PV labels.
-	// Once the mutating admission webhook is used for Azure, and GCE,
-	// this admission controller will be removed.
-	klog.Warning("PersistentVolumeLabel admission controller is deprecated. " +
-		"Please remove this controller from your configuration files and scripts.")
-	return &persistentVolumeLabel{
-		Handler: admission.NewHandler(admission.Create),
-	}
-}
-
-func (l *persistentVolumeLabel) SetCloudConfig(cloudConfig []byte) {
-	l.cloudConfig = cloudConfig
-}
-
-func nodeSelectorRequirementKeysExistInNodeSelectorTerms(reqs []api.NodeSelectorRequirement, terms []api.NodeSelectorTerm) bool {
-	for _, req := range reqs {
-		for _, term := range terms {
-			for _, r := range term.MatchExpressions {
-				if r.Key == req.Key {
-					return true
-				}
-			}
-		}
-	}
-	return false
-}
-
-func (l *persistentVolumeLabel) Admit(ctx context.Context, a admission.Attributes, o admission.ObjectInterfaces) (err error) {
-	if a.GetResource().GroupResource() != api.Resource("persistentvolumes") {
-		return nil
-	}
-	obj := a.GetObject()
-	if obj == nil {
-		return nil
-	}
-	volume, ok := obj.(*api.PersistentVolume)
-	if !ok {
-		return nil
-	}
-
-	volumeLabels, err := l.findVolumeLabels(volume)
-	if err != nil {
-		return admission.NewForbidden(a, err)
-	}
-
-	requirements := make([]api.NodeSelectorRequirement, 0)
-	if len(volumeLabels) != 0 {
-		if volume.Labels == nil {
-			volume.Labels = make(map[string]string)
-		}
-		for k, v := range volumeLabels {
-			// We (silently) replace labels if they are provided.
-			// This should be OK because they are in the kubernetes.io namespace
-			// i.e. we own them
-			volume.Labels[k] = v
-
-			// Set NodeSelectorRequirements based on the labels
-			var values []string
-			if k == v1.LabelTopologyZone || k == v1.LabelFailureDomainBetaZone {
-				zones, err := volumehelpers.LabelZonesToSet(v)
-				if err != nil {
-					return admission.NewForbidden(a, fmt.Errorf("failed to convert label string for Zone: %s to a Set", v))
-				}
-				// zone values here are sorted for better testability.
-				values = zones.List()
-			} else {
-				values = []string{v}
-			}
-			requirements = append(requirements, api.NodeSelectorRequirement{Key: k, Operator: api.NodeSelectorOpIn, Values: values})
-		}
-
-		if volume.Spec.NodeAffinity == nil {
-			volume.Spec.NodeAffinity = new(api.VolumeNodeAffinity)
-		}
-		if volume.Spec.NodeAffinity.Required == nil {
-			volume.Spec.NodeAffinity.Required = new(api.NodeSelector)
-		}
-		if len(volume.Spec.NodeAffinity.Required.NodeSelectorTerms) == 0 {
-			// Need at least one term pre-allocated whose MatchExpressions can be appended to
-			volume.Spec.NodeAffinity.Required.NodeSelectorTerms = make([]api.NodeSelectorTerm, 1)
-		}
-		if nodeSelectorRequirementKeysExistInNodeSelectorTerms(requirements, volume.Spec.NodeAffinity.Required.NodeSelectorTerms) {
-			klog.V(4).Infof("NodeSelectorRequirements for cloud labels %v conflict with existing NodeAffinity %v. Skipping addition of NodeSelectorRequirements for cloud labels.",
-				requirements, volume.Spec.NodeAffinity)
-		} else {
-			for _, req := range requirements {
-				for i := range volume.Spec.NodeAffinity.Required.NodeSelectorTerms {
-					volume.Spec.NodeAffinity.Required.NodeSelectorTerms[i].MatchExpressions = append(volume.Spec.NodeAffinity.Required.NodeSelectorTerms[i].MatchExpressions, req)
-				}
-			}
-		}
-	}
-
-	return nil
-}
-
-func (l *persistentVolumeLabel) findVolumeLabels(volume *api.PersistentVolume) (map[string]string, error) {
-	existingLabels := volume.Labels
-
-	// All cloud providers set only these two labels.
-	topologyLabelGA := true
-	domain, domainOK := existingLabels[v1.LabelTopologyZone]
-	region, regionOK := existingLabels[v1.LabelTopologyRegion]
-	// If they don't have GA labels we should check for failuredomain beta labels
-	// TODO: remove this once all the cloud provider change to GA topology labels
-	if !domainOK || !regionOK {
-		topologyLabelGA = false
-		domain, domainOK = existingLabels[v1.LabelFailureDomainBetaZone]
-		region, regionOK = existingLabels[v1.LabelFailureDomainBetaRegion]
-	}
-
-	isDynamicallyProvisioned := metav1.HasAnnotation(volume.ObjectMeta, persistentvolume.AnnDynamicallyProvisioned)
-	if isDynamicallyProvisioned && domainOK && regionOK {
-		// PV already has all the labels and we can trust the dynamic provisioning that it provided correct values.
-		if topologyLabelGA {
-			return map[string]string{
-				v1.LabelTopologyZone:   domain,
-				v1.LabelTopologyRegion: region,
-			}, nil
-		}
-		return map[string]string{
-			v1.LabelFailureDomainBetaZone:   domain,
-			v1.LabelFailureDomainBetaRegion: region,
-		}, nil
-
-	}
-
-	// Either missing labels or we don't trust the user provided correct values.
-	switch {
-	case volume.Spec.AzureDisk != nil:
-		labels, err := l.findAzureDiskLabels(volume)
-		if err != nil {
-			return nil, fmt.Errorf("error querying AzureDisk volume %s: %v", volume.Spec.AzureDisk.DiskName, err)
-		}
-		return labels, nil
-	case volume.Spec.VsphereVolume != nil:
-		labels, err := l.findVsphereVolumeLabels(volume)
-		if err != nil {
-			return nil, fmt.Errorf("error querying vSphere Volume %s: %v", volume.Spec.VsphereVolume.VolumePath, err)
-		}
-		return labels, nil
-	}
-	// Unrecognized volume, do not add any labels
-	return nil, nil
-}
-
-// getAzurePVLabeler returns the Azure implementation of PVLabeler
-func (l *persistentVolumeLabel) getAzurePVLabeler() (cloudprovider.PVLabeler, error) {
-	l.mutex.Lock()
-	defer l.mutex.Unlock()
-
-	if l.azurePVLabeler == nil {
-		var cloudConfigReader io.Reader
-		if len(l.cloudConfig) > 0 {
-			cloudConfigReader = bytes.NewReader(l.cloudConfig)
-		}
-
-		cloudProvider, err := cloudprovider.GetCloudProvider("azure", cloudConfigReader)
-		if err != nil || cloudProvider == nil {
-			return nil, err
-		}
-
-		azurePVLabeler, ok := cloudProvider.(cloudprovider.PVLabeler)
-		if !ok {
-			return nil, errors.New("Azure cloud provider does not implement PV labeling")
-		}
-		l.azurePVLabeler = azurePVLabeler
-	}
-
-	return l.azurePVLabeler, nil
-}
-
-func (l *persistentVolumeLabel) findAzureDiskLabels(volume *api.PersistentVolume) (map[string]string, error) {
-	// Ignore any volumes that are being provisioned
-	if volume.Spec.AzureDisk.DiskName == cloudvolume.ProvisionedVolumeName {
-		return nil, nil
-	}
-
-	pvlabler, err := l.getAzurePVLabeler()
-	if err != nil {
-		return nil, err
-	}
-	if pvlabler == nil {
-		return nil, fmt.Errorf("unable to build Azure cloud provider for AzureDisk")
-	}
-
-	pv := &v1.PersistentVolume{}
-	err = k8s_api_v1.Convert_core_PersistentVolume_To_v1_PersistentVolume(volume, pv, nil)
-	if err != nil {
-		return nil, fmt.Errorf("failed to convert PersistentVolume to core/v1: %q", err)
-	}
-	return pvlabler.GetLabelsForVolume(context.TODO(), pv)
-}
-
-func (l *persistentVolumeLabel) findVsphereVolumeLabels(volume *api.PersistentVolume) (map[string]string, error) {
-	pvlabler, err := l.getVspherePVLabeler()
-	if err != nil {
-		return nil, err
-	}
-	if pvlabler == nil {
-		return nil, fmt.Errorf("unable to build vSphere cloud provider")
-	}
-
-	pv := &v1.PersistentVolume{}
-	err = k8s_api_v1.Convert_core_PersistentVolume_To_v1_PersistentVolume(volume, pv, nil)
-	if err != nil {
-		return nil, fmt.Errorf("failed to convert PersistentVolume to core/v1: %q", err)
-	}
-	labels, err := pvlabler.GetLabelsForVolume(context.TODO(), pv)
-	if err != nil {
-		return nil, err
-	}
-
-	return labels, nil
-}
-
-func (l *persistentVolumeLabel) getVspherePVLabeler() (cloudprovider.PVLabeler, error) {
-	l.mutex.Lock()
-	defer l.mutex.Unlock()
-
-	if l.vspherePVLabeler == nil {
-		var cloudConfigReader io.Reader
-		if len(l.cloudConfig) > 0 {
-			cloudConfigReader = bytes.NewReader(l.cloudConfig)
-		}
-		cloudProvider, err := cloudprovider.GetCloudProvider("vsphere", cloudConfigReader)
-		if err != nil || cloudProvider == nil {
-			return nil, err
-		}
-		vspherePVLabeler, ok := cloudProvider.(cloudprovider.PVLabeler)
-		if !ok {
-			// GetCloudProvider failed
-			return nil, errors.New("vSphere Cloud Provider does not implement PV labeling")
-		}
-		l.vspherePVLabeler = vspherePVLabeler
-	}
-	return l.vspherePVLabeler, nil
-}
diff --git a/plugin/pkg/admission/storage/persistentvolume/label/doc.go b/plugin/pkg/admission/storage/persistentvolume/label/doc.go
deleted file mode 100644
index 56d4e627f364c..0000000000000
--- a/plugin/pkg/admission/storage/persistentvolume/label/doc.go
+++ /dev/null
@@ -1,19 +0,0 @@
-/*
-Copyright 2014 The Kubernetes Authors.
-
-Licensed under the Apache License, Version 2.0 (the "License");
-you may not use this file except in compliance with the License.
-You may obtain a copy of the License at
-
-    http://www.apache.org/licenses/LICENSE-2.0
-
-Unless required by applicable law or agreed to in writing, software
-distributed under the License is distributed on an "AS IS" BASIS,
-WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-See the License for the specific language governing permissions and
-limitations under the License.
-*/
-
-// Package label created persistent volumes with zone information
-// as provided by the cloud provider
-package label // import "k8s.io/kubernetes/plugin/pkg/admission/storage/persistentvolume/label"


Test patch:
diff --git a/hack/make-rules/test-cmd.sh b/hack/make-rules/test-cmd.sh
index d9bf2bd2385fe..acff1bc638d4e 100755
--- a/hack/make-rules/test-cmd.sh
+++ b/hack/make-rules/test-cmd.sh
@@ -56,7 +56,7 @@ function run_kube_apiserver() {
 
   # Admission Controllers to invoke prior to persisting objects in cluster
   ENABLE_ADMISSION_PLUGINS="LimitRanger,ResourceQuota"
-  DISABLE_ADMISSION_PLUGINS="ServiceAccount,PersistentVolumeLabel,DefaultStorageClass,DefaultTolerationSeconds,MutatingAdmissionWebhook,ValidatingAdmissionWebhook,StorageObjectInUseProtection"
+  DISABLE_ADMISSION_PLUGINS="ServiceAccount,DefaultStorageClass,DefaultTolerationSeconds,MutatingAdmissionWebhook,ValidatingAdmissionWebhook,StorageObjectInUseProtection"
 
   # Include RBAC (to exercise bootstrapping), and AlwaysAllow to allow all actions
   AUTHORIZATION_MODE="RBAC,AlwaysAllow"
diff --git a/plugin/pkg/admission/storage/persistentvolume/label/admission_test.go b/plugin/pkg/admission/storage/persistentvolume/label/admission_test.go
deleted file mode 100644
index 7318d6b170577..0000000000000
--- a/plugin/pkg/admission/storage/persistentvolume/label/admission_test.go
+++ /dev/null
@@ -1,666 +0,0 @@
-/*
-Copyright 2015 The Kubernetes Authors.
-
-Licensed under the Apache License, Version 2.0 (the "License");
-you may not use this file except in compliance with the License.
-You may obtain a copy of the License at
-
-    http://www.apache.org/licenses/LICENSE-2.0
-
-Unless required by applicable law or agreed to in writing, software
-distributed under the License is distributed on an "AS IS" BASIS,
-WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-See the License for the specific language governing permissions and
-limitations under the License.
-*/
-
-package label
-
-import (
-	"context"
-	"errors"
-	"reflect"
-	"sort"
-	"testing"
-
-	v1 "k8s.io/api/core/v1"
-	apierrors "k8s.io/apimachinery/pkg/api/errors"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/runtime/schema"
-	"k8s.io/apiserver/pkg/admission"
-	admissiontesting "k8s.io/apiserver/pkg/admission/testing"
-	cloudprovider "k8s.io/cloud-provider"
-	persistentvolume "k8s.io/component-helpers/storage/volume"
-	api "k8s.io/kubernetes/pkg/apis/core"
-)
-
-type mockVolumes struct {
-	volumeLabels      map[string]string
-	volumeLabelsError error
-}
-
-var _ cloudprovider.PVLabeler = &mockVolumes{}
-
-func (v *mockVolumes) GetLabelsForVolume(ctx context.Context, pv *v1.PersistentVolume) (map[string]string, error) {
-	return v.volumeLabels, v.volumeLabelsError
-}
-
-func mockVolumeFailure(err error) *mockVolumes {
-	return &mockVolumes{volumeLabelsError: err}
-}
-
-func mockVolumeLabels(labels map[string]string) *mockVolumes {
-	return &mockVolumes{volumeLabels: labels}
-}
-
-func Test_PVLAdmission(t *testing.T) {
-	testcases := []struct {
-		name            string
-		handler         *persistentVolumeLabel
-		pvlabeler       cloudprovider.PVLabeler
-		preAdmissionPV  *api.PersistentVolume
-		postAdmissionPV *api.PersistentVolume
-		err             error
-	}{
-		{
-			name:    "non-cloud PV ignored",
-			handler: newPersistentVolumeLabel(),
-			pvlabeler: mockVolumeLabels(map[string]string{
-				"a":                  "1",
-				"b":                  "2",
-				v1.LabelTopologyZone: "1__2__3",
-			}),
-			preAdmissionPV: &api.PersistentVolume{
-				ObjectMeta: metav1.ObjectMeta{Name: "noncloud", Namespace: "myns"},
-				Spec: api.PersistentVolumeSpec{
-					PersistentVolumeSource: api.PersistentVolumeSource{
-						HostPath: &api.HostPathVolumeSource{
-							Path: "/",
-						},
-					},
-				},
-			},
-			postAdmissionPV: &api.PersistentVolume{
-				ObjectMeta: metav1.ObjectMeta{Name: "noncloud", Namespace: "myns"},
-				Spec: api.PersistentVolumeSpec{
-					PersistentVolumeSource: api.PersistentVolumeSource{
-						HostPath: &api.HostPathVolumeSource{
-							Path: "/",
-						},
-					},
-				},
-			},
-			err: nil,
-		},
-		{
-			name:      "cloud provider error blocks creation of volume",
-			handler:   newPersistentVolumeLabel(),
-			pvlabeler: mockVolumeFailure(errors.New("invalid volume")),
-			preAdmissionPV: &api.PersistentVolume{
-				ObjectMeta: metav1.ObjectMeta{Name: "vSpherePV", Namespace: "myns"},
-				Spec: api.PersistentVolumeSpec{
-					PersistentVolumeSource: api.PersistentVolumeSource{
-						VsphereVolume: &api.VsphereVirtualDiskVolumeSource{
-							VolumePath: "123",
-						},
-					},
-				},
-			},
-			postAdmissionPV: &api.PersistentVolume{
-				ObjectMeta: metav1.ObjectMeta{Name: "vSpherePV", Namespace: "myns"},
-				Spec: api.PersistentVolumeSpec{
-					PersistentVolumeSource: api.PersistentVolumeSource{
-						VsphereVolume: &api.VsphereVirtualDiskVolumeSource{
-							VolumePath: "123",
-						},
-					},
-				},
-			},
-			err: apierrors.NewForbidden(schema.ParseGroupResource("persistentvolumes"), "vSpherePV", errors.New("error querying vSphere Volume 123: invalid volume")),
-		},
-		{
-			name:      "cloud provider returns no labels",
-			handler:   newPersistentVolumeLabel(),
-			pvlabeler: mockVolumeLabels(map[string]string{}),
-			preAdmissionPV: &api.PersistentVolume{
-				ObjectMeta: metav1.ObjectMeta{Name: "awsebs", Namespace: "myns"},
-				Spec: api.PersistentVolumeSpec{
-					PersistentVolumeSource: api.PersistentVolumeSource{
-						AWSElasticBlockStore: &api.AWSElasticBlockStoreVolumeSource{
-							VolumeID: "123",
-						},
-					},
-				},
-			},
-			postAdmissionPV: &api.PersistentVolume{
-				ObjectMeta: metav1.ObjectMeta{Name: "awsebs", Namespace: "myns"},
-				Spec: api.PersistentVolumeSpec{
-					PersistentVolumeSource: api.PersistentVolumeSource{
-						AWSElasticBlockStore: &api.AWSElasticBlockStoreVolumeSource{
-							VolumeID: "123",
-						},
-					},
-				},
-			},
-			err: nil,
-		},
-		{
-			name:      "cloud provider returns nil, nil",
-			handler:   newPersistentVolumeLabel(),
-			pvlabeler: mockVolumeFailure(nil),
-			preAdmissionPV: &api.PersistentVolume{
-				ObjectMeta: metav1.ObjectMeta{Name: "awsebs", Namespace: "myns"},
-				Spec: api.PersistentVolumeSpec{
-					PersistentVolumeSource: api.PersistentVolumeSource{
-						AWSElasticBlockStore: &api.AWSElasticBlockStoreVolumeSource{
-							VolumeID: "123",
-						},
-					},
-				},
-			},
-			postAdmissionPV: &api.PersistentVolume{
-				ObjectMeta: metav1.ObjectMeta{Name: "awsebs", Namespace: "myns"},
-				Spec: api.PersistentVolumeSpec{
-					PersistentVolumeSource: api.PersistentVolumeSource{
-						AWSElasticBlockStore: &api.AWSElasticBlockStoreVolumeSource{
-							VolumeID: "123",
-						},
-					},
-				},
-			},
-			err: nil,
-		},
-		{
-			name:    "existing Beta labels from dynamic provisioning are not changed",
-			handler: newPersistentVolumeLabel(),
-			pvlabeler: mockVolumeLabels(map[string]string{
-				v1.LabelFailureDomainBetaZone:   "domain1",
-				v1.LabelFailureDomainBetaRegion: "region1",
-			}),
-			preAdmissionPV: &api.PersistentVolume{
-				ObjectMeta: metav1.ObjectMeta{
-					Name: "awsebs", Namespace: "myns",
-					Labels: map[string]string{
-						v1.LabelFailureDomainBetaZone:   "existingDomain",
-						v1.LabelFailureDomainBetaRegion: "existingRegion",
-					},
-					Annotations: map[string]string{
-						persistentvolume.AnnDynamicallyProvisioned: "kubernetes.io/aws-ebs",
-					},
-				},
-				Spec: api.PersistentVolumeSpec{
-					PersistentVolumeSource: api.PersistentVolumeSource{
-						AWSElasticBlockStore: &api.AWSElasticBlockStoreVolumeSource{
-							VolumeID: "123",
-						},
-					},
-				},
-			},
-			postAdmissionPV: &api.PersistentVolume{
-				ObjectMeta: metav1.ObjectMeta{
-					Name:      "awsebs",
-					Namespace: "myns",
-					Labels: map[string]string{
-						v1.LabelFailureDomainBetaZone:   "existingDomain",
-						v1.LabelFailureDomainBetaRegion: "existingRegion",
-					},
-					Annotations: map[string]string{
-						persistentvolume.AnnDynamicallyProvisioned: "kubernetes.io/aws-ebs",
-					},
-				},
-				Spec: api.PersistentVolumeSpec{
-					PersistentVolumeSource: api.PersistentVolumeSource{
-						AWSElasticBlockStore: &api.AWSElasticBlockStoreVolumeSource{
-							VolumeID: "123",
-						},
-					},
-					NodeAffinity: &api.VolumeNodeAffinity{
-						Required: &api.NodeSelector{
-							NodeSelectorTerms: []api.NodeSelectorTerm{
-								{
-									MatchExpressions: []api.NodeSelectorRequirement{
-										{
-											Key:      v1.LabelFailureDomainBetaRegion,
-											Operator: api.NodeSelectorOpIn,
-											Values:   []string{"existingRegion"},
-										},
-										{
-											Key:      v1.LabelFailureDomainBetaZone,
-											Operator: api.NodeSelectorOpIn,
-											Values:   []string{"existingDomain"},
-										},
-									},
-								},
-							},
-						},
-					},
-				},
-			},
-			err: nil,
-		},
-		{
-			name:    "existing GA labels from dynamic provisioning are not changed",
-			handler: newPersistentVolumeLabel(),
-			pvlabeler: mockVolumeLabels(map[string]string{
-				v1.LabelTopologyZone:   "domain1",
-				v1.LabelTopologyRegion: "region1",
-			}),
-			preAdmissionPV: &api.PersistentVolume{
-				ObjectMeta: metav1.ObjectMeta{
-					Name: "awsebs", Namespace: "myns",
-					Labels: map[string]string{
-						v1.LabelTopologyZone:   "existingDomain",
-						v1.LabelTopologyRegion: "existingRegion",
-					},
-					Annotations: map[string]string{
-						persistentvolume.AnnDynamicallyProvisioned: "kubernetes.io/aws-ebs",
-					},
-				},
-				Spec: api.PersistentVolumeSpec{
-					PersistentVolumeSource: api.PersistentVolumeSource{
-						AWSElasticBlockStore: &api.AWSElasticBlockStoreVolumeSource{
-							VolumeID: "123",
-						},
-					},
-				},
-			},
-			postAdmissionPV: &api.PersistentVolume{
-				ObjectMeta: metav1.ObjectMeta{
-					Name:      "awsebs",
-					Namespace: "myns",
-					Labels: map[string]string{
-						v1.LabelTopologyZone:   "existingDomain",
-						v1.LabelTopologyRegion: "existingRegion",
-					},
-					Annotations: map[string]string{
-						persistentvolume.AnnDynamicallyProvisioned: "kubernetes.io/aws-ebs",
-					},
-				},
-				Spec: api.PersistentVolumeSpec{
-					PersistentVolumeSource: api.PersistentVolumeSource{
-						AWSElasticBlockStore: &api.AWSElasticBlockStoreVolumeSource{
-							VolumeID: "123",
-						},
-					},
-					NodeAffinity: &api.VolumeNodeAffinity{
-						Required: &api.NodeSelector{
-							NodeSelectorTerms: []api.NodeSelectorTerm{
-								{
-									MatchExpressions: []api.NodeSelectorRequirement{
-										{
-											Key:      v1.LabelTopologyRegion,
-											Operator: api.NodeSelectorOpIn,
-											Values:   []string{"existingRegion"},
-										},
-										{
-											Key:      v1.LabelTopologyZone,
-											Operator: api.NodeSelectorOpIn,
-											Values:   []string{"existingDomain"},
-										},
-									},
-								},
-							},
-						},
-					},
-				},
-			},
-			err: nil,
-		},
-		{
-			name:    "existing labels from user are changed",
-			handler: newPersistentVolumeLabel(),
-			pvlabeler: mockVolumeLabels(map[string]string{
-				v1.LabelTopologyZone:   "domain1",
-				v1.LabelTopologyRegion: "region1",
-			}),
-			preAdmissionPV: &api.PersistentVolume{
-				ObjectMeta: metav1.ObjectMeta{
-					Name: "vSpherePV", Namespace: "myns",
-					Labels: map[string]string{
-						v1.LabelTopologyZone:   "existingDomain",
-						v1.LabelTopologyRegion: "existingRegion",
-					},
-				},
-				Spec: api.PersistentVolumeSpec{
-					PersistentVolumeSource: api.PersistentVolumeSource{
-						VsphereVolume: &api.VsphereVirtualDiskVolumeSource{
-							VolumePath: "123",
-						},
-					},
-				},
-			},
-			postAdmissionPV: &api.PersistentVolume{
-				ObjectMeta: metav1.ObjectMeta{
-					Name:      "vSpherePV",
-					Namespace: "myns",
-					Labels: map[string]string{
-						v1.LabelTopologyZone:   "domain1",
-						v1.LabelTopologyRegion: "region1",
-					},
-				},
-				Spec: api.PersistentVolumeSpec{
-					PersistentVolumeSource: api.PersistentVolumeSource{
-						VsphereVolume: &api.VsphereVirtualDiskVolumeSource{
-							VolumePath: "123",
-						},
-					},
-					NodeAffinity: &api.VolumeNodeAffinity{
-						Required: &api.NodeSelector{
-							NodeSelectorTerms: []api.NodeSelectorTerm{
-								{
-									MatchExpressions: []api.NodeSelectorRequirement{
-										{
-											Key:      v1.LabelTopologyRegion,
-											Operator: api.NodeSelectorOpIn,
-											Values:   []string{"region1"},
-										},
-										{
-											Key:      v1.LabelTopologyZone,
-											Operator: api.NodeSelectorOpIn,
-											Values:   []string{"domain1"},
-										},
-									},
-								},
-							},
-						},
-					},
-				},
-			},
-			err: nil,
-		},
-		{
-			name:    "Azure Disk PV labeled correctly",
-			handler: newPersistentVolumeLabel(),
-			pvlabeler: mockVolumeLabels(map[string]string{
-				"a":                           "1",
-				"b":                           "2",
-				v1.LabelFailureDomainBetaZone: "1__2__3",
-			}),
-			preAdmissionPV: &api.PersistentVolume{
-				ObjectMeta: metav1.ObjectMeta{
-					Name:      "azurepd",
-					Namespace: "myns",
-				},
-				Spec: api.PersistentVolumeSpec{
-					PersistentVolumeSource: api.PersistentVolumeSource{
-						AzureDisk: &api.AzureDiskVolumeSource{
-							DiskName: "123",
-						},
-					},
-				},
-			},
-			postAdmissionPV: &api.PersistentVolume{
-				ObjectMeta: metav1.ObjectMeta{
-					Name:      "azurepd",
-					Namespace: "myns",
-					Labels: map[string]string{
-						"a":                           "1",
-						"b":                           "2",
-						v1.LabelFailureDomainBetaZone: "1__2__3",
-					},
-				},
-				Spec: api.PersistentVolumeSpec{
-					PersistentVolumeSource: api.PersistentVolumeSource{
-						AzureDisk: &api.AzureDiskVolumeSource{
-							DiskName: "123",
-						},
-					},
-					NodeAffinity: &api.VolumeNodeAffinity{
-						Required: &api.NodeSelector{
-							NodeSelectorTerms: []api.NodeSelectorTerm{
-								{
-									MatchExpressions: []api.NodeSelectorRequirement{
-										{
-											Key:      "a",
-											Operator: api.NodeSelectorOpIn,
-											Values:   []string{"1"},
-										},
-										{
-											Key:      "b",
-											Operator: api.NodeSelectorOpIn,
-											Values:   []string{"2"},
-										},
-										{
-											Key:      v1.LabelFailureDomainBetaZone,
-											Operator: api.NodeSelectorOpIn,
-											Values:   []string{"1", "2", "3"},
-										},
-									},
-								},
-							},
-						},
-					},
-				},
-			},
-			err: nil,
-		},
-		{
-			name:    "vSphere PV non-conflicting affinity rules added",
-			handler: newPersistentVolumeLabel(),
-			pvlabeler: mockVolumeLabels(map[string]string{
-				"d": "1",
-				"e": "2",
-				"f": "3",
-			}),
-			preAdmissionPV: &api.PersistentVolume{
-				ObjectMeta: metav1.ObjectMeta{
-					Name:      "vSpherePV",
-					Namespace: "myns",
-					Labels: map[string]string{
-						"a": "1",
-						"b": "2",
-						"c": "3",
-					},
-				},
-				Spec: api.PersistentVolumeSpec{
-					PersistentVolumeSource: api.PersistentVolumeSource{
-						VsphereVolume: &api.VsphereVirtualDiskVolumeSource{
-							VolumePath: "123",
-						},
-					},
-					NodeAffinity: &api.VolumeNodeAffinity{
-						Required: &api.NodeSelector{
-							NodeSelectorTerms: []api.NodeSelectorTerm{
-								{
-									MatchExpressions: []api.NodeSelectorRequirement{
-										{
-											Key:      "a",
-											Operator: api.NodeSelectorOpIn,
-											Values:   []string{"1"},
-										},
-										{
-											Key:      "b",
-											Operator: api.NodeSelectorOpIn,
-											Values:   []string{"2"},
-										},
-										{
-											Key:      "c",
-											Operator: api.NodeSelectorOpIn,
-											Values:   []string{"3"},
-										},
-									},
-								},
-							},
-						},
-					},
-				},
-			},
-			postAdmissionPV: &api.PersistentVolume{
-				ObjectMeta: metav1.ObjectMeta{
-					Name:      "vSpherePV",
-					Namespace: "myns",
-					Labels: map[string]string{
-						"a": "1",
-						"b": "2",
-						"c": "3",
-						"d": "1",
-						"e": "2",
-						"f": "3",
-					},
-				},
-				Spec: api.PersistentVolumeSpec{
-					PersistentVolumeSource: api.PersistentVolumeSource{
-						VsphereVolume: &api.VsphereVirtualDiskVolumeSource{
-							VolumePath: "123",
-						},
-					},
-					NodeAffinity: &api.VolumeNodeAffinity{
-						Required: &api.NodeSelector{
-							NodeSelectorTerms: []api.NodeSelectorTerm{
-								{
-									MatchExpressions: []api.NodeSelectorRequirement{
-										{
-											Key:      "a",
-											Operator: api.NodeSelectorOpIn,
-											Values:   []string{"1"},
-										},
-										{
-											Key:      "b",
-											Operator: api.NodeSelectorOpIn,
-											Values:   []string{"2"},
-										},
-										{
-											Key:      "c",
-											Operator: api.NodeSelectorOpIn,
-											Values:   []string{"3"},
-										},
-										{
-											Key:      "d",
-											Operator: api.NodeSelectorOpIn,
-											Values:   []string{"1"},
-										},
-										{
-											Key:      "e",
-											Operator: api.NodeSelectorOpIn,
-											Values:   []string{"2"},
-										},
-										{
-											Key:      "f",
-											Operator: api.NodeSelectorOpIn,
-											Values:   []string{"3"},
-										},
-									},
-								},
-							},
-						},
-					},
-				},
-			},
-			err: nil,
-		},
-		{
-			name:    "vSphere PV labeled correctly",
-			handler: newPersistentVolumeLabel(),
-			pvlabeler: mockVolumeLabels(map[string]string{
-				"a":                           "1",
-				"b":                           "2",
-				v1.LabelFailureDomainBetaZone: "1__2__3",
-			}),
-			preAdmissionPV: &api.PersistentVolume{
-				ObjectMeta: metav1.ObjectMeta{
-					Name:      "vSpherePV",
-					Namespace: "myns",
-				},
-				Spec: api.PersistentVolumeSpec{
-					PersistentVolumeSource: api.PersistentVolumeSource{
-						VsphereVolume: &api.VsphereVirtualDiskVolumeSource{
-							VolumePath: "123",
-						},
-					},
-				},
-			},
-			postAdmissionPV: &api.PersistentVolume{
-				ObjectMeta: metav1.ObjectMeta{
-					Name:      "vSpherePV",
-					Namespace: "myns",
-					Labels: map[string]string{
-						"a":                           "1",
-						"b":                           "2",
-						v1.LabelFailureDomainBetaZone: "1__2__3",
-					},
-				},
-				Spec: api.PersistentVolumeSpec{
-					PersistentVolumeSource: api.PersistentVolumeSource{
-						VsphereVolume: &api.VsphereVirtualDiskVolumeSource{
-							VolumePath: "123",
-						},
-					},
-					NodeAffinity: &api.VolumeNodeAffinity{
-						Required: &api.NodeSelector{
-							NodeSelectorTerms: []api.NodeSelectorTerm{
-								{
-									MatchExpressions: []api.NodeSelectorRequirement{
-										{
-											Key:      "a",
-											Operator: api.NodeSelectorOpIn,
-											Values:   []string{"1"},
-										},
-										{
-											Key:      "b",
-											Operator: api.NodeSelectorOpIn,
-											Values:   []string{"2"},
-										},
-										{
-											Key:      v1.LabelFailureDomainBetaZone,
-											Operator: api.NodeSelectorOpIn,
-											Values:   []string{"1", "2", "3"},
-										},
-									},
-								},
-							},
-						},
-					},
-				},
-			},
-			err: nil,
-		},
-	}
-
-	for _, testcase := range testcases {
-		t.Run(testcase.name, func(t *testing.T) {
-			setPVLabeler(testcase.handler, testcase.pvlabeler)
-			handler := admissiontesting.WithReinvocationTesting(t, admission.NewChainHandler(testcase.handler))
-
-			err := handler.Admit(context.TODO(), admission.NewAttributesRecord(testcase.preAdmissionPV, nil, api.Kind("PersistentVolume").WithVersion("version"), testcase.preAdmissionPV.Namespace, testcase.preAdmissionPV.Name, api.Resource("persistentvolumes").WithVersion("version"), "", admission.Create, &metav1.CreateOptions{}, false, nil), nil)
-			if !reflect.DeepEqual(err, testcase.err) {
-				t.Logf("expected error: %q", testcase.err)
-				t.Logf("actual error: %q", err)
-				t.Error("unexpected error when admitting PV")
-			}
-
-			// sort node selector match expression by key because they are added out of order in the admission controller
-			sortMatchExpressions(testcase.preAdmissionPV)
-			if !reflect.DeepEqual(testcase.preAdmissionPV, testcase.postAdmissionPV) {
-				t.Logf("expected PV: %+v", testcase.postAdmissionPV)
-				t.Logf("actual PV: %+v", testcase.preAdmissionPV)
-				t.Error("unexpected PV")
-			}
-
-		})
-	}
-}
-
-// setPVLabler applies the given mock pvlabeler to implement PV labeling for all cloud providers.
-// Given we mock out the values of the labels anyways, assigning the same mock labeler for every
-// provider does not reduce test coverage but it does simplify/clean up the tests here because
-// the provider is then decided based on the type of PV (EBS, GCEPD, Azure Disk, etc)
-func setPVLabeler(handler *persistentVolumeLabel, pvlabeler cloudprovider.PVLabeler) {
-	handler.azurePVLabeler = pvlabeler
-	handler.vspherePVLabeler = pvlabeler
-}
-
-// sortMatchExpressions sorts a PV's node selector match expressions by key name if it is not nil
-func sortMatchExpressions(pv *api.PersistentVolume) {
-	if pv.Spec.NodeAffinity == nil ||
-		pv.Spec.NodeAffinity.Required == nil ||
-		pv.Spec.NodeAffinity.Required.NodeSelectorTerms == nil {
-		return
-	}
-
-	match := pv.Spec.NodeAffinity.Required.NodeSelectorTerms[0].MatchExpressions
-	sort.Slice(match, func(i, j int) bool {
-		return match[i].Key < match[j].Key
-	})
-
-	pv.Spec.NodeAffinity.Required.NodeSelectorTerms[0].MatchExpressions = match
-}