Merge pull request #35430 from jsafrane/remove-pv-annotations

Automatic merge from submit-queue

Remove PV annotations for quobyte provisioner

This is the last provisioner that uses annotations to pass secrets from provisioner to deleter.

Fixes #34822

@johscheuer, I don't have access to quobyte, please take look and retest the plugin. An e2e test for quobyte would be nice!

@kubernetes/sig-storage

pkg/volume/quobyte/quobyte.go

@@ -343,21 +343,17 @@ func (provisioner *quobyteVolumeProvisioner) Provision() (*api.PersistentVolume,
 	if provisioner.options.PVC.Spec.Selector != nil {
 		return nil, fmt.Errorf("claim Selector is not supported")
 	}
-	var apiServer, adminSecretName, quobyteUser, quobytePassword string
-	adminSecretNamespace := "default"
 	provisioner.config = "BASE"
 	provisioner.tenant = "DEFAULT"
 
+	cfg, err := parseAPIConfig(provisioner.plugin, provisioner.options.Parameters)
+	if err != nil {
+		return nil, err
+	}
 	for k, v := range provisioner.options.Parameters {
 		switch goStrings.ToLower(k) {
 		case "registry":
 			provisioner.registry = v
-		case "adminsecretname":
-			adminSecretName = v
-		case "adminsecretnamespace":
-			adminSecretNamespace = v
-		case "quobyteapiserver":
-			apiServer = v
 		case "user":
 			provisioner.user = v
 		case "group":
@@ -366,41 +362,22 @@ func (provisioner *quobyteVolumeProvisioner) Provision() (*api.PersistentVolume,
 			provisioner.tenant = v
 		case "quobyteconfig":
 			provisioner.config = v
+		case "adminsecretname",
+			"adminsecretnamespace",
+			"quobyteapiserver":
+			continue
 		default:
 			return nil, fmt.Errorf("invalid option %q for volume plugin %s", k, provisioner.plugin.GetPluginName())
 		}
 	}
 
-	secretMap, err := util.GetSecretForPV(adminSecretNamespace, adminSecretName, quobytePluginName, provisioner.plugin.host.GetKubeClient())
-	if err != nil {
-		return nil, err
-	}
-
-	var ok bool
-	if quobyteUser, ok = secretMap["user"]; !ok {
-		return nil, fmt.Errorf("Missing \"user\" in secret")
-	}
-
-	if quobytePassword, ok = secretMap["password"]; !ok {
-		return nil, fmt.Errorf("Missing \"password\" in secret")
-	}
-
 	if !validateRegistry(provisioner.registry) {
 		return nil, fmt.Errorf("Quoybte registry missing or malformed: must be a host:port pair or multiple pairs separated by commas")
 	}
 
-	if len(apiServer) == 0 {
-		return nil, fmt.Errorf("Quoybte API server missing or malformed: must be a http(s)://host:port pair or multiple pairs separated by commas")
-	}
-
 	// create random image name
 	provisioner.volume = fmt.Sprintf("kubernetes-dynamic-pvc-%s", uuid.NewUUID())
 
-	cfg := &quobyteAPIConfig{
-		quobyteAPIServer: apiServer,
-		quobyteUser:      quobyteUser,
-		quobytePassword:  quobytePassword,
-	}
 	manager := &quobyteVolumeManager{
 		config: cfg,
 	}
@@ -419,13 +396,6 @@ func (provisioner *quobyteVolumeProvisioner) Provision() (*api.PersistentVolume,
 	pv.Spec.Capacity = api.ResourceList{
 		api.ResourceName(api.ResourceStorage): resource.MustParse(fmt.Sprintf("%dGi", sizeGB)),
 	}
-
-	util.AddVolumeAnnotations(pv, map[string]string{
-		annotationQuobyteAPIServer:          apiServer,
-		annotationQuobyteAPISecret:          adminSecretName,
-		annotationQuobyteAPISecretNamespace: adminSecretNamespace,
-	})
-
 	return pv, nil
 }
 
@@ -434,41 +404,63 @@ func (deleter *quobyteVolumeDeleter) GetPath() string {
 }
 
 func (deleter *quobyteVolumeDeleter) Delete() error {
-	var quobyteUser, quobytePassword string
+	class, err := util.GetClassForVolume(deleter.plugin.host.GetKubeClient(), deleter.pv)
-	annotations, err := util.ParseVolumeAnnotations(deleter.pv, []string{
-		annotationQuobyteAPISecret,
-		annotationQuobyteAPISecretNamespace,
-		annotationQuobyteAPIServer})
-
 	if err != nil {
 		return err
 	}
 
-	secretMap, err := util.GetSecretForPV(
+	cfg, err := parseAPIConfig(deleter.plugin, class.Parameters)
-		annotations[annotationQuobyteAPISecretNamespace],
-		annotations[annotationQuobyteAPISecret],
-		quobytePluginName,
-		deleter.plugin.host.GetKubeClient())
-
 	if err != nil {
 		return err
 	}
-
-	var ok bool
-	if quobyteUser, ok = secretMap["user"]; !ok {
-		return fmt.Errorf("Missing \"user\" in secret")
-	}
-
-	if quobytePassword, ok = secretMap["password"]; !ok {
-		return fmt.Errorf("Missing \"password\" in secret")
-	}
-
 	manager := &quobyteVolumeManager{
-		config: &quobyteAPIConfig{
+		config: cfg,
-			quobyteUser:      quobyteUser,
-			quobytePassword:  quobytePassword,
-			quobyteAPIServer: annotations[annotationQuobyteAPIServer],
-		},
 	}
 	return manager.deleteVolume(deleter)
 }
+
+// Parse API configuration (url, username and password) out of class.Parameters.
+func parseAPIConfig(plugin *quobytePlugin, params map[string]string) (*quobyteAPIConfig, error) {
+	var apiServer, secretName string
+	secretNamespace := "default"
+
+	deleteKeys := []string{}
+
+	for k, v := range params {
+		switch goStrings.ToLower(k) {
+		case "adminsecretname":
+			secretName = v
+			deleteKeys = append(deleteKeys, k)
+		case "adminsecretnamespace":
+			secretNamespace = v
+			deleteKeys = append(deleteKeys, k)
+		case "quobyteapiserver":
+			apiServer = v
+			deleteKeys = append(deleteKeys, k)
+		}
+	}
+
+	if len(apiServer) == 0 {
+		return nil, fmt.Errorf("Quoybte API server missing or malformed: must be a http(s)://host:port pair or multiple pairs separated by commas")
+	}
+
+	secretMap, err := util.GetSecretForPV(secretNamespace, secretName, quobytePluginName, plugin.host.GetKubeClient())
+	if err != nil {
+		return nil, err
+	}
+
+	cfg := &quobyteAPIConfig{
+		quobyteAPIServer: apiServer,
+	}
+
+	var ok bool
+	if cfg.quobyteUser, ok = secretMap["user"]; !ok {
+		return nil, fmt.Errorf("Missing \"user\" in secret %s/%s", secretNamespace, secretName)
+	}
+
+	if cfg.quobytePassword, ok = secretMap["password"]; !ok {
+		return nil, fmt.Errorf("Missing \"password\" in secret %s/%s", secretNamespace, secretName)
+	}
+
+	return cfg, nil
+}

pkg/volume/util/util.go

@@ -147,36 +147,6 @@ func GetSecretForPV(secretNamespace, secretName, volumePluginName string, kubeCl
 	return secret, nil
 }
 
-// AddVolumeAnnotations adds a golang Map as annotation to a PersistentVolume
-func AddVolumeAnnotations(pv *api.PersistentVolume, annotations map[string]string) {
-	if pv.Annotations == nil {
-		pv.Annotations = map[string]string{}
-	}
-
-	for k, v := range annotations {
-		pv.Annotations[k] = v
-	}
-}
-
-// ParseVolumeAnnotations reads the defined annoations from a PersistentVolume
-func ParseVolumeAnnotations(pv *api.PersistentVolume, parseAnnotations []string) (map[string]string, error) {
-	result := map[string]string{}
-
-	if pv.Annotations == nil {
-		return result, fmt.Errorf("cannot parse volume annotations: no annotations found")
-	}
-
-	for _, annotation := range parseAnnotations {
-		if val, ok := pv.Annotations[annotation]; ok {
-			result[annotation] = val
-		} else {
-			return result, fmt.Errorf("cannot parse volume annotations: annotation %s not found", annotation)
-		}
-	}
-
-	return result, nil
-}
-
 func GetClassForVolume(kubeClient clientset.Interface, pv *api.PersistentVolume) (*storage.StorageClass, error) {
 	// TODO: replace with a real attribute after beta
 	className, found := pv.Annotations["volume.beta.kubernetes.io/storage-class"]