alexlebens/external-snapshotter
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Merge pull request #78 from xing-yang/csi_secret

Browse Source 
Add new reserved prefixed parameter keys
This commit is contained in:
Kubernetes Prow Robot
2018-12-04 00:52:54 -08:00
committed by GitHub
parent f3b2562724 0cf36adb91
commit 1e1a432d5e
5 changed files with 235 additions and 56 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
pkg/controller/csi_handler.go
View File

@@ -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 := removePrefixedParameters(parameters)
if err != nil {
return "", "", 0, 0, false, fmt.Errorf("failed to remove 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 {

8
pkg/controller/snapshot_controller.go
View File

@@ -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
}
@@ -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
}

2
pkg/controller/snapshot_create_test.go
View File

@@ -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"},

156
pkg/controller/util.go
View File

@@ -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"
@@ -37,12 +39,41 @@ var (
keyFunc = cache.DeletionHandlingMetaNamespaceKeyFunc
)
const snapshotterSecretNameKey = "csiSnapshotterSecretName"
const snapshotterSecretNamespaceKey = "csiSnapshotterSecretNamespace"
type deprecatedSecretParamsMap struct {
name string
deprecatedSecretNameKey string
deprecatedSecretNamespaceKey string
secretNameKey string
secretNamespaceKey string
}
// 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"
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"
// [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 = deprecatedSecretParamsMap{
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)
@@ -130,9 +161,54 @@ 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.
// verifyAndGetSecretNameAndNamespaceTemplate gets the values (templates) associated
// with the parameters specified in "secret" and verifies that they are specified correctly.
func verifyAndGetSecretNameAndNamespaceTemplate(secret deprecatedSecretParamsMap, 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}
@@ -145,20 +221,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 := verifyAndGetSecretNameAndNamespaceTemplate(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{}
@@ -174,15 +247,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
@@ -199,13 +272,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
@@ -229,8 +302,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
}
@@ -271,3 +344,34 @@ 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 removePrefixedParameters(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
// Deprecated parameters prefixed with "csi" are not stripped to preserve backwards compatibility
newParam[k] = v
}
}
return newParam, nil
}

119
pkg/controller/util_test.go
View File

@@ -17,7 +17,6 @@ limitations under the License.
package controller
import (
"fmt"
crdv1 "github.com/kubernetes-csi/external-snapshotter/pkg/apis/volumesnapshot/v1alpha1"
"k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -31,53 +30,50 @@ func TestGetSecretReference(t *testing.T) {
snapContentName string
snapshot *crdv1.VolumeSnapshot
expectRef *v1.SecretReference
expectErr error
expectErr bool
}{
"no params": {
params: nil,
expectRef: nil,
expectErr: nil,
},
"empty err": {
params: map[string]string{snapshotterSecretNameKey: "", snapshotterSecretNamespaceKey: ""},
expectErr: fmt.Errorf("csiSnapshotterSecretName and csiSnapshotterSecretNamespace parameters must be specified together"),
expectErr: true,
},
"name, no namespace": {
"[deprecated] name, no namespace": {
params: map[string]string{snapshotterSecretNameKey: "foo"},
expectErr: fmt.Errorf("csiSnapshotterSecretName and csiSnapshotterSecretNamespace parameters must be specified together"),
expectErr: true,
},
"namespace, no name": {
params: map[string]string{snapshotterSecretNamespaceKey: "foo"},
expectErr: fmt.Errorf("csiSnapshotterSecretName and csiSnapshotterSecretNamespace parameters must be specified together"),
params: map[string]string{prefixedSnapshotterSecretNamespaceKey: "foo"},
expectErr: true,
},
"simple - valid": {
params: map[string]string{snapshotterSecretNameKey: "name", snapshotterSecretNamespaceKey: "ns"},
params: map[string]string{prefixedSnapshotterSecretNameKey: "name", prefixedSnapshotterSecretNamespaceKey: "ns"},
snapshot: &crdv1.VolumeSnapshot{},
expectRef: &v1.SecretReference{Name: "name", Namespace: "ns"},
expectErr: nil,
},
"simple - valid, no pvc": {
"[deprecated] simple - valid, no pvc": {
params: map[string]string{snapshotterSecretNameKey: "name", snapshotterSecretNamespaceKey: "ns"},
snapshot: nil,
expectRef: &v1.SecretReference{Name: "name", Namespace: "ns"},
expectErr: nil,
},
"simple - invalid name": {
params: map[string]string{snapshotterSecretNameKey: "bad name", snapshotterSecretNamespaceKey: "ns"},
params: map[string]string{prefixedSnapshotterSecretNameKey: "bad name", prefixedSnapshotterSecretNamespaceKey: "ns"},
snapshot: &crdv1.VolumeSnapshot{},
expectRef: nil,
expectErr: fmt.Errorf(`csiSnapshotterSecretName parameter "bad name" is not a valid secret name`),
expectErr: true,
},
"simple - invalid namespace": {
"[deprecated] simple - invalid namespace": {
params: map[string]string{snapshotterSecretNameKey: "name", snapshotterSecretNamespaceKey: "bad ns"},
snapshot: &crdv1.VolumeSnapshot{},
expectRef: nil,
expectErr: fmt.Errorf(`csiSnapshotterSecretNamespace parameter "bad ns" is not a valid namespace name`),
expectErr: true,
},
"template - valid": {
params: map[string]string{
snapshotterSecretNameKey: "static-${volumesnapshotcontent.name}-${volumesnapshot.namespace}-${volumesnapshot.name}-${volumesnapshot.annotations['akey']}",
snapshotterSecretNamespaceKey: "static-${volumesnapshotcontent.name}-${volumesnapshot.namespace}",
prefixedSnapshotterSecretNameKey: "static-${volumesnapshotcontent.name}-${volumesnapshot.namespace}-${volumesnapshot.name}-${volumesnapshot.annotations['akey']}",
prefixedSnapshotterSecretNamespaceKey: "static-${volumesnapshotcontent.name}-${volumesnapshot.namespace}",
},
snapContentName: "snapcontentname",
snapshot: &crdv1.VolumeSnapshot{
@@ -88,7 +84,6 @@ func TestGetSecretReference(t *testing.T) {
},
},
expectRef: &v1.SecretReference{Name: "static-snapcontentname-snapshotnamespace-snapshotname-avalue", Namespace: "static-snapcontentname-snapshotnamespace"},
expectErr: nil,
},
"template - invalid namespace tokens": {
params: map[string]string{
@@ -97,7 +92,7 @@ func TestGetSecretReference(t *testing.T) {
},
snapshot: &crdv1.VolumeSnapshot{},
expectRef: nil,
expectErr: fmt.Errorf(`error resolving csiSnapshotterSecretNamespace value "mynamespace${bar}": invalid tokens: ["bar"]`),
expectErr: true,
},
"template - invalid name tokens": {
params: map[string]string{
@@ -106,15 +101,23 @@ func TestGetSecretReference(t *testing.T) {
},
snapshot: &crdv1.VolumeSnapshot{},
expectRef: nil,
expectErr: fmt.Errorf(`error resolving csiSnapshotterSecretName value "myname${foo}": invalid tokens: ["foo"]`),
expectErr: true,
},
}
for k, tc := range testcases {
t.Run(k, func(t *testing.T) {
ref, err := GetSecretReference(tc.params, tc.snapContentName, tc.snapshot)
if !reflect.DeepEqual(err, tc.expectErr) {
t.Errorf("Expected %v, got %v", tc.expectErr, err)
ref, err := getSecretReference(tc.params, tc.snapContentName, tc.snapshot)
if err != nil {
if tc.expectErr {
return
} else {
t.Fatalf("Did not expect error but got: %v", err)
}
} else {
if tc.expectErr {
t.Fatalf("Expected error but got none")
}
}
if !reflect.DeepEqual(ref, tc.expectRef) {
t.Errorf("Expected %v, got %v", tc.expectRef, ref)
@@ -122,3 +125,71 @@ func TestGetSecretReference(t *testing.T) {
})
}
}
func TestRemovePrefixedCSIParams(t *testing.T) {
testcases := []struct {
name string
params map[string]string
expectedParams map[string]string
expectErr bool
}{
{
name: "no prefix",
params: map[string]string{"csiFoo": "bar", "bim": "baz"},
expectedParams: map[string]string{"csiFoo": "bar", "bim": "baz"},
},
{
name: "one prefixed",
params: map[string]string{prefixedSnapshotterSecretNameKey: "bar", "bim": "baz"},
expectedParams: map[string]string{"bim": "baz"},
},
{
name: "all known prefixed",
params: map[string]string{
prefixedSnapshotterSecretNameKey: "csiBar",
prefixedSnapshotterSecretNamespaceKey: "csiBar",
},
expectedParams: map[string]string{},
},
{
name: "all known deprecated params not stripped",
params: map[string]string{
snapshotterSecretNameKey: "csiBar",
snapshotterSecretNamespaceKey: "csiBar",
},
expectedParams: map[string]string{
snapshotterSecretNameKey: "csiBar",
snapshotterSecretNamespaceKey: "csiBar",
},
},
{
name: "unknown prefixed var",
params: map[string]string{csiParameterPrefix + "bim": "baz"},
expectErr: true,
},
{
name: "empty",
params: map[string]string{},
expectedParams: map[string]string{},
},
}
for _, tc := range testcases {
t.Logf("test: %v", tc.name)
newParams, err := removePrefixedParameters(tc.params)
if err != nil {
if tc.expectErr {
continue
} else {
t.Fatalf("Encountered unexpected error: %v", err)
}
} else {
if tc.expectErr {
t.Fatalf("Did not get error when one was expected")
}
}
eq := reflect.DeepEqual(newParams, tc.expectedParams)
if !eq {
t.Fatalf("Stripped paramaters: %v not equal to expected paramaters: %v", newParams, tc.expectedParams)
}
}
}