Skip to content

K
k3s
Commit 2b2508ba authored by Jan Safranek's avatar Jan Safranek
Browse files
Options

Remove PV annotations for Gluster provisioner.

Don't store Gluster SotrageClass parameters in annotations, it's insecure. Instead, expect that there is the StorageClass available at the time when it's needed by Gluster deleter.
parent a47ac1fa
Hide whitespace changes
Inline Side-by-side
Showing with 214 additions and 133 deletions
pkg/volume/glusterfs/glusterfs.go
View file @ 2b2508ba
......@@ -403,21 +403,16 @@ func (d *glusterfsVolumeDeleter) Delete() error {
volumeName := d.glusterfsMounter.path
volumeId := dstrings.TrimPrefix(volumeName, volprefix)
err = d.annotationsToParam(d.spec)
class, err := volutil.GetClassForVolume(d.plugin.host.GetKubeClient(), d.spec)
if err != nil {
return err
}
if len(d.secretName) > 0 {
d.secretValue, err = parseSecret(d.secretNamespace, d.secretName, d.plugin.host.GetKubeClient())
if err != nil {
glog.Errorf("glusterfs: failed to read secret: %v", err)
return err
}
} else if len(d.userKey) > 0 {
d.secretValue = d.userKey
} else {
d.secretValue = ""
cfg, err := parseClassParameters(class.Parameters, d.plugin.host.GetKubeClient())
if err != nil {
return err
}
d.provisioningConfig = *cfg
glog.V(4).Infof("glusterfs: deleting volume %q with configuration %+v", volumeId, d.provisioningConfig)
......@@ -444,56 +439,11 @@ func (r *glusterfsVolumeProvisioner) Provision() (*api.PersistentVolume, error)
}
glog.V(4).Infof("glusterfs: Provison VolumeOptions %v", r.options)
authEnabled := true
for k, v := range r.options.Parameters {
switch dstrings.ToLower(k) {
case "endpoint":
r.endpoint = v
case "resturl":
r.url = v
case "restuser":
r.user = v
case "restuserkey":
r.userKey = v
case "secretname":
r.secretName = v
case "secretnamespace":
r.secretNamespace = v
case "restauthenabled":
authEnabled = dstrings.ToLower(v) == "true"
default:
return nil, fmt.Errorf("glusterfs: invalid option %q for volume plugin %s", k, r.plugin.GetPluginName())
}
}
if len(r.url) == 0 {
return nil, fmt.Errorf("StorageClass for provisioner %q must contain 'resturl' parameter", r.plugin.GetPluginName())
}
if len(r.endpoint) == 0 {
return nil, fmt.Errorf("StorageClass for provisioner %q must contain 'endpoint' parameter", r.plugin.GetPluginName())
}
if !authEnabled {
r.user = ""
r.secretName = ""
r.secretNamespace = ""
r.userKey = ""
r.secretValue = ""
}
if len(r.secretName) != 0 || len(r.secretNamespace) != 0 {
// secretName + Namespace has precedence over userKey
if len(r.secretName) != 0 && len(r.secretNamespace) != 0 {
r.secretValue, err = parseSecret(r.secretNamespace, r.secretName, r.plugin.host.GetKubeClient())
if err != nil {
return nil, err
}
} else {
return nil, fmt.Errorf("StorageClass for provisioner %q must have secretNamespace and secretName either both set or both empty", r.plugin.GetPluginName())
}
} else {
r.secretValue = r.userKey
cfg, err := parseClassParameters(r.options.Parameters, r.plugin.host.GetKubeClient())
if err != nil {
return nil, err
}
r.provisioningConfig = *cfg
glog.V(4).Infof("glusterfs: creating volume with configuration %+v", r.provisioningConfig)
glusterfs, sizeGB, err := r.CreateVolume()
......@@ -511,7 +461,6 @@ func (r *glusterfsVolumeProvisioner) Provision() (*api.PersistentVolume, error)
pv.Spec.Capacity = api.ResourceList{
api.ResourceName(api.ResourceStorage): resource.MustParse(fmt.Sprintf("%dGi", sizeGB)),
}
r.paramToAnnotations(pv)
return pv, nil
}
......@@ -564,37 +513,60 @@ func parseSecret(namespace, secretName string, kubeClient clientset.Interface) (
return secret, nil
}
// paramToAnnotations stores parameters needed to delete the volume in the PV
// annotations.
func (p *glusterfsVolumeProvisioner) paramToAnnotations(pv *api.PersistentVolume) {
ann := map[string]string{
annGlusterURL: p.url,
annGlusterUser: p.user,
annGlusterSecretName: p.secretName,
annGlusterSecretNamespace: p.secretNamespace,
annGlusterUserKey: p.userKey,
// parseClassParameters parses StorageClass.Parameters
func parseClassParameters(params map[string]string, kubeClient clientset.Interface) (*provisioningConfig, error) {
var cfg provisioningConfig
var err error
authEnabled := true
for k, v := range params {
switch dstrings.ToLower(k) {
case "endpoint":
cfg.endpoint = v
case "resturl":
cfg.url = v
case "restuser":
cfg.user = v
case "restuserkey":
cfg.userKey = v
case "secretname":
cfg.secretName = v
case "secretnamespace":
cfg.secretNamespace = v
case "restauthenabled":
authEnabled = dstrings.ToLower(v) == "true"
default:
return nil, fmt.Errorf("glusterfs: invalid option %q for volume plugin %s", k, glusterfsPluginName)
}
}
volutil.AddVolumeAnnotations(pv, ann)
}
// annotationsToParam parses annotations stored by paramToAnnotations
func (d *glusterfsVolumeDeleter) annotationsToParam(pv *api.PersistentVolume) error {
annKeys := []string{
annGlusterSecretName,
annGlusterSecretNamespace,
annGlusterURL,
annGlusterUser,
annGlusterUserKey,
if len(cfg.url) == 0 {
return nil, fmt.Errorf("StorageClass for provisioner %s must contain 'resturl' parameter", glusterfsPluginName)
}
params, err := volutil.ParseVolumeAnnotations(pv, annKeys)
if err != nil {
return err
if len(cfg.endpoint) == 0 {
return nil, fmt.Errorf("StorageClass for provisioner %s must contain 'endpoint' parameter", glusterfsPluginName)
}
d.url = params[annGlusterURL]
d.user = params[annGlusterUser]
d.userKey = params[annGlusterUserKey]
d.secretName = params[annGlusterSecretName]
d.secretNamespace = params[annGlusterSecretNamespace]
return nil
if !authEnabled {
cfg.user = ""
cfg.secretName = ""
cfg.secretNamespace = ""
cfg.userKey = ""
cfg.secretValue = ""
}
if len(cfg.secretName) != 0 || len(cfg.secretNamespace) != 0 {
// secretName + Namespace has precedence over userKey
if len(cfg.secretName) != 0 && len(cfg.secretNamespace) != 0 {
cfg.secretValue, err = parseSecret(cfg.secretNamespace, cfg.secretName, kubeClient)
if err != nil {
return nil, err
}
} else {
return nil, fmt.Errorf("StorageClass for provisioner %q must have secretNamespace and secretName either both set or both empty", glusterfsPluginName)
}
} else {
cfg.secretValue = cfg.userKey
}
return &cfg, nil
}
pkg/volume/glusterfs/glusterfs_test.go
View file @ 2b2508ba
......@@ -19,10 +19,13 @@ package glusterfs
import (
"fmt"
"os"
"reflect"
"testing"
"k8s.io/kubernetes/pkg/api"
"k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/fake"
"k8s.io/kubernetes/pkg/client/testing/core"
"k8s.io/kubernetes/pkg/runtime"
"k8s.io/kubernetes/pkg/types"
"k8s.io/kubernetes/pkg/util/exec"
"k8s.io/kubernetes/pkg/util/mount"
......@@ -237,62 +240,153 @@ func TestPersistentClaimReadOnlyFlag(t *testing.T) {
}
}
func TestAnnotations(t *testing.T) {
// Pass a provisioningConfigs through paramToAnnotations and back through
// annotationsToParam and check it did not change in the process.
tests := []provisioningConfig{
func TestParseClassParameters(t *testing.T) {
secret := api.Secret{
Data: map[string][]byte{
"data": []byte("mypassword"),
},
}
tests := []struct {
name string
parameters map[string]string
secret *api.Secret
expectError bool
expectConfig *provisioningConfig
}{
{
"password",
map[string]string{
"endpoint": "endpointname",
"resturl": "https://localhost:8080",
"restuser": "admin",
"restuserkey": "password",
},
nil, // secret
false, // expect error
&provisioningConfig{
endpoint: "endpointname",
url: "https://localhost:8080",
user: "admin",
userKey: "password",
secretValue: "password",
},
},
{
"secret",
map[string]string{
"endpoint": "endpointname",
"resturl": "https://localhost:8080",
"restuser": "admin",
"secretname": "mysecret",
"secretnamespace": "default",
},
&secret,
false, // expect error
&provisioningConfig{
endpoint: "endpointname",
url: "https://localhost:8080",
user: "admin",
secretName: "mysecret",
secretNamespace: "default",
secretValue: "mypassword",
},
},
{
// Everything empty
"no authentication",
map[string]string{
"endpoint": "endpointname",
"resturl": "https://localhost:8080",
"restauthenabled": "false",
},
&secret,
false, // expect error
&provisioningConfig{
endpoint: "endpointname",
url: "https://localhost:8080",
},
},
{
// Everything with a value
url: "http://localhost",
user: "admin",
secretNamespace: "default",
secretName: "gluster-secret",
userKey: "mykey",
"missing secret",
map[string]string{
"endpoint": "endpointname",
"resturl": "https://localhost:8080",
"secretname": "mysecret",
"secretnamespace": "default",
},
nil, // secret
true, // expect error
nil,
},
{
// No secret
url: "http://localhost",
user: "admin",
secretNamespace: "",
secretName: "",
userKey: "",
"secret with no namespace",
map[string]string{
"endpoint": "endpointname",
"resturl": "https://localhost:8080",
"secretname": "mysecret",
},
&secret,
true, // expect error
nil,
},
{
"missing url",
map[string]string{
"endpoint": "endpointname",
"restuser": "admin",
"restuserkey": "password",
},
nil, // secret
true, // expect error
nil,
},
{
"missing endpoint",
map[string]string{
"resturl": "https://localhost:8080",
"restuser": "admin",
"restuserkey": "password",
},
nil, // secret
true, // expect error
nil,
},
{
"unknown parameter",
map[string]string{
"unknown": "yes",
"resturl": "https://localhost:8080",
"restuser": "admin",
"restuserkey": "password",
},
nil, // secret
true, // expect error
nil,
},
}
for i, test := range tests {
provisioner := &glusterfsVolumeProvisioner{
provisioningConfig: test,
}
deleter := &glusterfsVolumeDeleter{}
for _, test := range tests {
pv := &api.PersistentVolume{
ObjectMeta: api.ObjectMeta{
Name: "pv",
},
}
client := &fake.Clientset{}
client.AddReactor("get", "secrets", func(action core.Action) (handled bool, ret runtime.Object, err error) {
if test.secret != nil {
return true, test.secret, nil
}
return true, nil, fmt.Errorf("Test %s did not set a secret", test.name)
})
provisioner.paramToAnnotations(pv)
err := deleter.annotationsToParam(pv)
if err != nil {
t.Errorf("test %d failed: %v", i, err)
}
if test.url != deleter.url {
t.Errorf("test %d failed: expected url %q, got %q", i, test.url, deleter.url)
}
if test.user != deleter.user {
t.Errorf("test %d failed: expected user %q, got %q", i, test.user, deleter.user)
}
if test.userKey != deleter.userKey {
t.Errorf("test %d failed: expected userKey %q, got %q", i, test.userKey, deleter.userKey)
cfg, err := parseClassParameters(test.parameters, client)
if err != nil && !test.expectError {
t.Errorf("Test %s got unexpected error %v", test.name, err)
}
if test.secretNamespace != deleter.secretNamespace {
t.Errorf("test %d failed: expected secretNamespace %q, got %q", i, test.secretNamespace, deleter.secretNamespace)
if err == nil && test.expectError {
t.Errorf("test %s expected error and got none", test.name)
}
if test.secretName != deleter.secretName {
t.Errorf("test %d failed: expected secretName %q, got %q", i, test.secretName, deleter.secretName)
if test.expectConfig != nil {
if !reflect.DeepEqual(cfg, test.expectConfig) {
t.Errorf("Test %s returned unexpected data, expected: %+v, got: %+v", test.name, test.expectConfig, cfg)
}
}
}
}
pkg/volume/util/util.go
View file @ 2b2508ba
......@@ -23,6 +23,7 @@ import (
"github.com/golang/glog"
"k8s.io/kubernetes/pkg/api"
"k8s.io/kubernetes/pkg/apis/storage"
clientset "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset"
"k8s.io/kubernetes/pkg/util/mount"
)
......@@ -157,3 +158,17 @@ func ParseVolumeAnnotations(pv *api.PersistentVolume, parseAnnotations []string)
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"]
if !found {
return nil, fmt.Errorf("Volume has no class annotation")
}
class, err := kubeClient.Storage().StorageClasses().Get(className)
if err != nil {
return nil, err
}
return class, nil
}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment