Skip to content

Commit

Permalink
Add new reserved prefixed parameter keys
Browse files Browse the repository at this point in the history 
This PR adds new reserved prefixed parameter keys which are stripped
out of parameter list, and adds deprecation notice for old keys and
keep their behavior the same.

```
csiParameterPrefix = "csi.storage.k8s.io/"
prefixedSnapshotterSecretNameKey      = csiParameterPrefix + "snapshotter-secret-name"
prefixedSnapshotterSecretNamespaceKey = csiParameterPrefix + "snapshotter-secret-namespace"
```
  • Loading branch information
@xing-yang
xing-yang committed Dec 1, 2018
1 parent 7cd3ef6 commit 1874380
Show file tree
Hide file tree
Showing 5 changed files with 232 additions and 56 deletions.
6 changes: 5 additions & 1 deletion pkg/controller/csi_handler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,11 @@ func (handler *csiHandler) CreateSnapshot(snapshot *crdv1.VolumeSnapshot, volume
if err != nil {
return "", "", 0, 0, false, err
}
return handler.csiConnection.CreateSnapshot(ctx, snapshotName, volume, parameters, snapshotterCredentials)
newParameters, err := stripPrefixedParameters(parameters)
if err != nil {
return "", "", 0, 0, false, fmt.Errorf("failed to strip CSI Parameters of prefixed keys: %v", err)
}
return handler.csiConnection.CreateSnapshot(ctx, snapshotName, volume, newParameters, snapshotterCredentials)
}

func (handler *csiHandler) DeleteSnapshot(content *crdv1.VolumeSnapshotContent, snapshotterCredentials map[string]string) error {
Expand Down
8 changes: 4 additions & 4 deletions pkg/controller/snapshot_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -542,11 +542,11 @@ func (ctrl *csiSnapshotController) getCreateSnapshotInput(snapshot *crdv1.Volume
contentName := GetSnapshotContentNameForSnapshot(snapshot)

// Resolve snapshotting secret credentials.
snapshotterSecretRef, err := GetSecretReference(class.Parameters, contentName, snapshot)
snapshotterSecretRef, err := getSecretReference(class.Parameters, contentName, snapshot)
if err != nil {
return nil, nil, "", nil, err
}
snapshotterCredentials, err := GetCredentials(ctrl.client, snapshotterSecretRef)
snapshotterCredentials, err := getCredentials(ctrl.client, snapshotterSecretRef)
if err != nil {
return nil, nil, "", nil, err
}
Expand Down Expand Up @@ -710,11 +710,11 @@ func (ctrl *csiSnapshotController) deleteSnapshotContentOperation(content *crdv1
if snapshotClass, err := ctrl.classLister.Get(*snapshotClassName); err == nil {
// Resolve snapshotting secret credentials.
// No VolumeSnapshot is provided when resolving delete secret names, since the VolumeSnapshot may or may not exist at delete time.
snapshotterSecretRef, err := GetSecretReference(snapshotClass.Parameters, content.Name, nil)
snapshotterSecretRef, err := getSecretReference(snapshotClass.Parameters, content.Name, nil)
if err != nil {
return err
}
snapshotterCredentials, err = GetCredentials(ctrl.client, snapshotterSecretRef)
snapshotterCredentials, err = getCredentials(ctrl.client, snapshotterSecretRef)
if err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/controller/snapshot_create_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ func TestCreateSnapshotSync(t *testing.T) {
initialContents: nocontents,
expectedContents: nocontents,
initialSnapshots: newSnapshotArray("snap7-2", invalidSecretClass, "", "snapuid7-2", "claim7-2", false, nil, nil, nil),
expectedSnapshots: newSnapshotArray("snap7-2", invalidSecretClass, "", "snapuid7-2", "claim7-2", false, newVolumeError("Failed to create snapshot: failed to get input parameters to create snapshot snap7-2: \"csiSnapshotterSecretName and csiSnapshotterSecretNamespace parameters must be specified together\""), nil, nil),
expectedSnapshots: newSnapshotArray("snap7-2", invalidSecretClass, "", "snapuid7-2", "claim7-2", false, newVolumeError("Failed to create snapshot: failed to get input parameters to create snapshot snap7-2: \"failed to get name and namespace template from params: either name and namespace for Snapshotter secrets specified, Both must be specified\""), nil, nil),
initialClaims: newClaimArray("claim7-2", "pvc-uid7-2", "1Gi", "volume7-2", v1.ClaimBound, &classEmpty),
initialVolumes: newVolumeArray("volume7-2", "pv-uid7-2", "pv-handle7-2", "1Gi", "pvc-uid7-2", "claim7-2", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty),
expectedEvents: []string{"Warning SnapshotCreationFailed"},
Expand Down
153 changes: 127 additions & 26 deletions pkg/controller/util.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ package controller

import (
"fmt"
"strings"

"github.com/golang/glog"
crdv1 "github.com/kubernetes-csi/external-snapshotter/pkg/apis/volumesnapshot/v1alpha1"
"k8s.io/api/core/v1"
Expand All @@ -37,12 +39,41 @@ var (
keyFunc = cache.DeletionHandlingMetaNamespaceKeyFunc
)

const snapshotterSecretNameKey = "csiSnapshotterSecretName"
const snapshotterSecretNamespaceKey = "csiSnapshotterSecretNamespace"
type secretParamsType struct {
name string
deprecatedSecretNameKey string
deprecatedSecretNamespaceKey string
secretNameKey string
secretNamespaceKey string
}

const (
// CSI Parameters prefixed with csiParameterPrefix are not passed through
// to the driver on CreateSnapshotRequest calls. Instead they are intended
// to be used by the CSI external-snapshotter and maybe used to populate
// fields in subsequent CSI calls or Kubernetes API objects.
csiParameterPrefix = "csi.storage.k8s.io/"

prefixedSnapshotterSecretNameKey = csiParameterPrefix + "snapshotter-secret-name"
prefixedSnapshotterSecretNamespaceKey = csiParameterPrefix + "snapshotter-secret-namespace"

// Name of finalizer on VolumeSnapshotContents that are bound by VolumeSnapshots
const VolumeSnapshotContentFinalizer = "snapshot.storage.kubernetes.io/volumesnapshotcontent-protection"
const VolumeSnapshotFinalizer = "snapshot.storage.kubernetes.io/volumesnapshot-protection"
// [Deprecated] CSI Parameters that are put into fields but
// NOT stripped from the parameters passed to CreateSnapshot
snapshotterSecretNameKey = "csiSnapshotterSecretName"
snapshotterSecretNamespaceKey = "csiSnapshotterSecretNamespace"

// Name of finalizer on VolumeSnapshotContents that are bound by VolumeSnapshots
VolumeSnapshotContentFinalizer = "snapshot.storage.kubernetes.io/volumesnapshotcontent-protection"
VolumeSnapshotFinalizer = "snapshot.storage.kubernetes.io/volumesnapshot-protection"
)

var snapshotterSecretParams = secretParamsType{
name: "Snapshotter",
deprecatedSecretNameKey: snapshotterSecretNameKey,
deprecatedSecretNamespaceKey: snapshotterSecretNamespaceKey,
secretNameKey: prefixedSnapshotterSecretNameKey,
secretNamespaceKey: prefixedSnapshotterSecretNamespaceKey,
}

func snapshotKey(vs *crdv1.VolumeSnapshot) string {
return fmt.Sprintf("%s/%s", vs.Namespace, vs.Name)
Expand Down Expand Up @@ -130,9 +161,52 @@ func IsDefaultAnnotation(obj metav1.ObjectMeta) bool {
return false
}

// GetSecretReference returns a reference to the secret specified in the given nameKey and namespaceKey parameters, or an error if the parameters are not specified correctly.
// if neither the name or namespace parameter are set, a nil reference and no error is returned.
// no lookup of the referenced secret is performed, and the secret may or may not exist.
func getSecretNameAndNamespaceTemplate(secret secretParamsType, snapshotClassParams map[string]string) (nameTemplate, namespaceTemplate string, err error) {
numName := 0
numNamespace := 0
if t, ok := snapshotClassParams[secret.deprecatedSecretNameKey]; ok {
nameTemplate = t
numName += 1
glog.Warning(deprecationWarning(secret.deprecatedSecretNameKey, secret.secretNameKey, ""))
}
if t, ok := snapshotClassParams[secret.deprecatedSecretNamespaceKey]; ok {
namespaceTemplate = t
numNamespace += 1
glog.Warning(deprecationWarning(secret.deprecatedSecretNamespaceKey, secret.secretNamespaceKey, ""))
}
if t, ok := snapshotClassParams[secret.secretNameKey]; ok {
nameTemplate = t
numName += 1
}
if t, ok := snapshotClassParams[secret.secretNamespaceKey]; ok {
namespaceTemplate = t
numNamespace += 1
}

if numName > 1 || numNamespace > 1 {
// Double specified error
return "", "", fmt.Errorf("%s secrets specified in paramaters with both \"csi\" and \"%s\" keys", secret.name, csiParameterPrefix)
} else if numName != numNamespace {
// Not both 0 or both 1
return "", "", fmt.Errorf("either name and namespace for %s secrets specified, Both must be specified", secret.name)
} else if numName == 1 {
// Case where we've found a name and a namespace template
if nameTemplate == "" || namespaceTemplate == "" {
return "", "", fmt.Errorf("%s secrets specified in parameters but value of either namespace or name is empty", secret.name)
}
return nameTemplate, namespaceTemplate, nil
} else if numName == 0 {
// No secrets specified
return "", "", nil
} else {
// THIS IS NOT A VALID CASE
return "", "", fmt.Errorf("unknown error with getting secret name and namespace templates")
}
}

// getSecretReference returns a reference to the secret specified in the given nameTemplate
// and namespaceTemplate, or an error if the templates are not specified correctly.
// No lookup of the referenced secret is performed, and the secret may or may not exist.
//
// supported tokens for name resolution:
// - ${volumesnapshotcontent.name}
Expand All @@ -145,20 +219,17 @@ func IsDefaultAnnotation(obj metav1.ObjectMeta) bool {
// - ${volumesnapshot.namespace}
//
// an error is returned in the following situations:
// - only one of name or namespace is provided
// - the name or namespace parameter contains a token that cannot be resolved
// - the nameTemplate or namespaceTemplate contains a token that cannot be resolved
// - the resolved name is not a valid secret name
// - the resolved namespace is not a valid namespace name
func GetSecretReference(snapshotClassParams map[string]string, snapContentName string, snapshot *crdv1.VolumeSnapshot) (*v1.SecretReference, error) {
nameTemplate, hasName := snapshotClassParams[snapshotterSecretNameKey]
namespaceTemplate, hasNamespace := snapshotClassParams[snapshotterSecretNamespaceKey]

if !hasName && !hasNamespace {
return nil, nil
func getSecretReference(snapshotClassParams map[string]string, snapContentName string, snapshot *crdv1.VolumeSnapshot) (*v1.SecretReference, error) {
nameTemplate, namespaceTemplate, err := getSecretNameAndNamespaceTemplate(snapshotterSecretParams, snapshotClassParams)
if err != nil {
return nil, fmt.Errorf("failed to get name and namespace template from params: %v", err)
}

if len(nameTemplate) == 0 || len(namespaceTemplate) == 0 {
return nil, fmt.Errorf("%s and %s parameters must be specified together", snapshotterSecretNameKey, snapshotterSecretNamespaceKey)
if nameTemplate == "" && namespaceTemplate == "" {
return nil, nil
}

ref := &v1.SecretReference{}
Expand All @@ -174,15 +245,15 @@ func GetSecretReference(snapshotClassParams map[string]string, snapContentName s

resolvedNamespace, err := resolveTemplate(namespaceTemplate, namespaceParams)
if err != nil {
return nil, fmt.Errorf("error resolving %s value %q: %v", snapshotterSecretNamespaceKey, namespaceTemplate, err)
return nil, fmt.Errorf("error resolving value %q: %v", namespaceTemplate, err)
}
glog.V(4).Infof("GetSecretReference namespaceTemplate %s, namespaceParams: %+v, resolved %s", namespaceTemplate, namespaceParams, resolvedNamespace)

if len(validation.IsDNS1123Label(resolvedNamespace)) > 0 {
if namespaceTemplate != resolvedNamespace {
return nil, fmt.Errorf("%s parameter %q resolved to %q which is not a valid namespace name", snapshotterSecretNamespaceKey, namespaceTemplate, resolvedNamespace)
return nil, fmt.Errorf("%q resolved to %q which is not a valid namespace name", namespaceTemplate, resolvedNamespace)
}
return nil, fmt.Errorf("%s parameter %q is not a valid namespace name", snapshotterSecretNamespaceKey, namespaceTemplate)
return nil, fmt.Errorf("%q is not a valid namespace name", namespaceTemplate)
}
ref.Namespace = resolvedNamespace

Expand All @@ -199,13 +270,13 @@ func GetSecretReference(snapshotClassParams map[string]string, snapContentName s
}
resolvedName, err := resolveTemplate(nameTemplate, nameParams)
if err != nil {
return nil, fmt.Errorf("error resolving %s value %q: %v", snapshotterSecretNameKey, nameTemplate, err)
return nil, fmt.Errorf("error resolving value %q: %v", nameTemplate, err)
}
if len(validation.IsDNS1123Subdomain(resolvedName)) > 0 {
if nameTemplate != resolvedName {
return nil, fmt.Errorf("%s parameter %q resolved to %q which is not a valid secret name", snapshotterSecretNameKey, nameTemplate, resolvedName)
return nil, fmt.Errorf("%q resolved to %q which is not a valid secret name", nameTemplate, resolvedName)
}
return nil, fmt.Errorf("%s parameter %q is not a valid secret name", snapshotterSecretNameKey, nameTemplate)
return nil, fmt.Errorf("%q is not a valid secret name", nameTemplate)
}
ref.Name = resolvedName

Expand All @@ -229,8 +300,8 @@ func resolveTemplate(template string, params map[string]string) (string, error)
return resolved, nil
}

// GetCredentials retrieves credentials stored in v1.SecretReference
func GetCredentials(k8s kubernetes.Interface, ref *v1.SecretReference) (map[string]string, error) {
// getCredentials retrieves credentials stored in v1.SecretReference
func getCredentials(k8s kubernetes.Interface, ref *v1.SecretReference) (map[string]string, error) {
if ref == nil {
return nil, nil
}
Expand Down Expand Up @@ -271,3 +342,33 @@ func isSnapshotDeletionCandidate(snapshot *crdv1.VolumeSnapshot) bool {
func needToAddSnapshotFinalizer(snapshot *crdv1.VolumeSnapshot) bool {
return snapshot.ObjectMeta.DeletionTimestamp == nil && !slice.ContainsString(snapshot.ObjectMeta.Finalizers, VolumeSnapshotFinalizer, nil)
}

func deprecationWarning(deprecatedParam, newParam, removalVersion string) string {
if removalVersion == "" {
removalVersion = "a future release"
}
newParamPhrase := ""
if len(newParam) != 0 {
newParamPhrase = fmt.Sprintf(", please use \"%s\" instead", newParam)
}
return fmt.Sprintf("\"%s\" is deprecated and will be removed in %s%s", deprecatedParam, removalVersion, newParamPhrase)
}

func stripPrefixedParameters(param map[string]string) (map[string]string, error) {
newParam := map[string]string{}
for k, v := range param {
if strings.HasPrefix(k, csiParameterPrefix) {
// Check if its well known
switch k {
case prefixedSnapshotterSecretNameKey:
case prefixedSnapshotterSecretNamespaceKey:
default:
return map[string]string{}, fmt.Errorf("found unknown parameter key \"%s\" with reserved namespace %s", k, csiParameterPrefix)
}
} else {
// Don't strip, add this key-value to new map
newParam[k] = v
}
}
return newParam, nil
}
Loading

0 comments on commit 1874380

Please sign in to comment.