refactor CephFS PV spec to use SecretReference

Signed-off-by: Huamin Chen <hchen@redhat.com>

pkg/api/persistentvolume/util.go

@@ -48,8 +48,16 @@ func VisitPVSecretNames(pv *api.PersistentVolume, visitor Visitor) bool {
 		}
 		return true
 	case source.CephFS != nil:
-		if source.CephFS.SecretRef != nil && !visitor(getClaimRefNamespace(pv), source.CephFS.SecretRef.Name) {
+		if source.CephFS.SecretRef != nil {
-			return false
+			// previously persisted PV objects use claimRef namespace
+			ns := getClaimRefNamespace(pv)
+			if len(source.CephFS.SecretRef.Namespace) > 0 {
+				// use the secret namespace if namespace is set
+				ns = source.CephFS.SecretRef.Namespace
+			}
+			if !visitor(ns, source.CephFS.SecretRef.Name) {
+				return false
+			}
 		}
 	case source.FlexVolume != nil:
 		if source.FlexVolume.SecretRef != nil && !visitor(getClaimRefNamespace(pv), source.FlexVolume.SecretRef.Name) {

pkg/api/persistentvolume/util_test.go

@@ -46,8 +46,15 @@ func TestPVSecrets(t *testing.T) {
 		{Spec: api.PersistentVolumeSpec{
 			ClaimRef: &api.ObjectReference{Namespace: "claimrefns", Name: "claimrefname"},
 			PersistentVolumeSource: api.PersistentVolumeSource{
-				CephFS: &api.CephFSVolumeSource{
+				CephFS: &api.CephFSPersistentVolumeSource{
-					SecretRef: &api.LocalObjectReference{
+					SecretRef: &api.SecretReference{
+						Name:      "Spec.PersistentVolumeSource.CephFS.SecretRef",
+						Namespace: "cephfs"}}}}},
+		{Spec: api.PersistentVolumeSpec{
+			ClaimRef: &api.ObjectReference{Namespace: "claimrefns", Name: "claimrefname"},
+			PersistentVolumeSource: api.PersistentVolumeSource{
+				CephFS: &api.CephFSPersistentVolumeSource{
+					SecretRef: &api.SecretReference{
 						Name: "Spec.PersistentVolumeSource.CephFS.SecretRef"}}}}},
 		{Spec: api.PersistentVolumeSpec{
 			ClaimRef: &api.ObjectReference{Namespace: "claimrefns", Name: "claimrefname"},
@@ -81,7 +88,6 @@ func TestPVSecrets(t *testing.T) {
 						Name:      "Spec.PersistentVolumeSource.StorageOS.SecretRef",
 						Namespace: "storageosns"}}}}},
 	}
-
 	extractedNames := sets.NewString()
 	extractedNamesWithNamespace := sets.NewString()
 	for _, pv := range pvs {
@@ -132,6 +138,7 @@ func TestPVSecrets(t *testing.T) {
 		"claimrefns/Spec.PersistentVolumeSource.AzureFile.SecretName",
 		"Spec.PersistentVolumeSource.AzureFile.SecretNamespace/Spec.PersistentVolumeSource.AzureFile.SecretName",
 		"claimrefns/Spec.PersistentVolumeSource.CephFS.SecretRef",
+		"cephfs/Spec.PersistentVolumeSource.CephFS.SecretRef",
 		"claimrefns/Spec.PersistentVolumeSource.FlexVolume.SecretRef",
 		"claimrefns/Spec.PersistentVolumeSource.RBD.SecretRef",
 		"claimrefns/Spec.PersistentVolumeSource.ScaleIO.SecretRef",

pkg/api/types.go

@@ -360,7 +360,7 @@ type PersistentVolumeSource struct {
 	Cinder *CinderVolumeSource
 	// CephFS represents a Ceph FS mount on the host that shares a pod's lifetime
 	// +optional
-	CephFS *CephFSVolumeSource
+	CephFS *CephFSPersistentVolumeSource
 	// FC represents a Fibre Channel resource that is attached to a kubelet's host machine and then exposed to the pod.
 	// +optional
 	FC *FCVolumeSource
@@ -1019,6 +1019,40 @@ type CephFSVolumeSource struct {
 	ReadOnly bool
 }
 
+// SecretReference represents a Secret Reference. It has enough information to retrieve secret
+// in any namespace
+type SecretReference struct {
+	// Name is unique within a namespace to reference a secret resource.
+	// +optional
+	Name string
+	// Namespace defines the space within which the secret name must be unique.
+	// +optional
+	Namespace string
+}
+
+// Represents a Ceph Filesystem mount that lasts the lifetime of a pod
+// Cephfs volumes do not support ownership management or SELinux relabeling.
+type CephFSPersistentVolumeSource struct {
+	// Required: Monitors is a collection of Ceph monitors
+	Monitors []string
+	// Optional: Used as the mounted root, rather than the full Ceph tree, default is /
+	// +optional
+	Path string
+	// Optional: User is the rados user name, default is admin
+	// +optional
+	User string
+	// Optional: SecretFile is the path to key ring for User, default is /etc/ceph/user.secret
+	// +optional
+	SecretFile string
+	// Optional: SecretRef is reference to the authentication secret for User, default is empty.
+	// +optional
+	SecretRef *SecretReference
+	// Optional: Defaults to false (read/write). ReadOnly here will force
+	// the ReadOnly setting in VolumeMounts.
+	// +optional
+	ReadOnly bool
+}
+
 // Represents a Flocker volume mounted by the Flocker agent.
 // One and only one of datasetName and datasetUUID should be set.
 // Flocker volumes do not support ownership management or SELinux relabeling.

pkg/api/validation/validation.go

@@ -1066,6 +1066,14 @@ func validateCephFSVolumeSource(cephfs *api.CephFSVolumeSource, fldPath *field.P
 	return allErrs
 }
 
+func validateCephFSPersistentVolumeSource(cephfs *api.CephFSPersistentVolumeSource, fldPath *field.Path) field.ErrorList {
+	allErrs := field.ErrorList{}
+	if len(cephfs.Monitors) == 0 {
+		allErrs = append(allErrs, field.Required(fldPath.Child("monitors"), ""))
+	}
+	return allErrs
+}
+
 func validateFlexVolumeSource(fv *api.FlexVolumeSource, fldPath *field.Path) field.ErrorList {
 	allErrs := field.ErrorList{}
 	if len(fv.Driver) == 0 {
@@ -1350,7 +1358,7 @@ func ValidatePersistentVolume(pv *api.PersistentVolume) field.ErrorList {
 			allErrs = append(allErrs, field.Forbidden(specPath.Child("cephFS"), "may not specify more than 1 volume type"))
 		} else {
 			numVolumes++
-			allErrs = append(allErrs, validateCephFSVolumeSource(pv.Spec.CephFS, specPath.Child("cephfs"))...)
+			allErrs = append(allErrs, validateCephFSPersistentVolumeSource(pv.Spec.CephFS, specPath.Child("cephfs"))...)
 		}
 	}
 	if pv.Spec.ISCSI != nil {

pkg/printers/internalversion/describe.go

@@ -954,6 +954,17 @@ func printCephFSVolumeSource(cephfs *api.CephFSVolumeSource, w PrefixWriter) {
 		cephfs.Monitors, cephfs.Path, cephfs.User, cephfs.SecretFile, cephfs.SecretRef, cephfs.ReadOnly)
 }
 
+func printCephFSPersistentVolumeSource(cephfs *api.CephFSPersistentVolumeSource, w PrefixWriter) {
+	w.Write(LEVEL_2, "Type:\tCephFS (a CephFS mount on the host that shares a pod's lifetime)\n"+
+		"    Monitors:\t%v\n"+
+		"    Path:\t%v\n"+
+		"    User:\t%v\n"+
+		"    SecretFile:\t%v\n"+
+		"    SecretRef:\t%v\n"+
+		"    ReadOnly:\t%v\n",
+		cephfs.Monitors, cephfs.Path, cephfs.User, cephfs.SecretFile, cephfs.SecretRef, cephfs.ReadOnly)
+}
+
 func printStorageOSVolumeSource(storageos *api.StorageOSVolumeSource, w PrefixWriter) {
 	w.Write(LEVEL_2, "Type:\tStorageOS (a StorageOS Persistent Disk resource)\n"+
 		"    VolumeName:\t%v\n"+
@@ -1095,7 +1106,7 @@ func describePersistentVolume(pv *api.PersistentVolume, events *api.EventList) (
 		case pv.Spec.Local != nil:
 			printLocalVolumeSource(pv.Spec.Local, w)
 		case pv.Spec.CephFS != nil:
-			printCephFSVolumeSource(pv.Spec.CephFS, w)
+			printCephFSPersistentVolumeSource(pv.Spec.CephFS, w)
 		case pv.Spec.StorageOS != nil:
 			printStorageOSPersistentVolumeSource(pv.Spec.StorageOS, w)
 		case pv.Spec.FC != nil:

pkg/volume/cephfs/cephfs.go

@@ -56,12 +56,12 @@ func (plugin *cephfsPlugin) GetPluginName() string {
 }
 
 func (plugin *cephfsPlugin) GetVolumeName(spec *volume.Spec) (string, error) {
-	volumeSource, _, err := getVolumeSource(spec)
+	mon, _, _, _, _, err := getVolumeSource(spec)
 	if err != nil {
 		return "", err
 	}
 
-	return fmt.Sprintf("%v", volumeSource.Monitors), nil
+	return fmt.Sprintf("%v", mon), nil
 }
 
 func (plugin *cephfsPlugin) CanSupport(spec *volume.Spec) bool {
@@ -89,23 +89,23 @@ func (plugin *cephfsPlugin) GetAccessModes() []v1.PersistentVolumeAccessMode {
 }
 
 func (plugin *cephfsPlugin) NewMounter(spec *volume.Spec, pod *v1.Pod, _ volume.VolumeOptions) (volume.Mounter, error) {
-	cephvs, _, err := getVolumeSource(spec)
+	secretName, secretNs, err := getSecretNameAndNamespace(spec, pod.Namespace)
 	if err != nil {
 		return nil, err
 	}
 	secret := ""
-	if cephvs.SecretRef != nil {
+	if len(secretName) > 0 && len(secretNs) > 0 {
+		// if secret is provideded, retrieve it
 		kubeClient := plugin.host.GetKubeClient()
 		if kubeClient == nil {
 			return nil, fmt.Errorf("Cannot get kube client")
 		}
-
+		secrets, err := kubeClient.Core().Secrets(secretNs).Get(secretName, metav1.GetOptions{})
-		secretName, err := kubeClient.Core().Secrets(pod.Namespace).Get(cephvs.SecretRef.Name, metav1.GetOptions{})
 		if err != nil {
-			err = fmt.Errorf("Couldn't get secret %v/%v err: %v", pod.Namespace, cephvs.SecretRef, err)
+			err = fmt.Errorf("Couldn't get secret %v/%v err: %v", secretNs, secretName, err)
 			return nil, err
 		}
-		for name, data := range secretName.Data {
+		for name, data := range secrets.Data {
 			secret = string(data)
 			glog.V(4).Infof("found ceph secret info: %s", name)
 		}
@@ -114,37 +114,35 @@ func (plugin *cephfsPlugin) NewMounter(spec *volume.Spec, pod *v1.Pod, _ volume.
 }
 
 func (plugin *cephfsPlugin) newMounterInternal(spec *volume.Spec, podUID types.UID, mounter mount.Interface, secret string) (volume.Mounter, error) {
-	cephvs, _, err := getVolumeSource(spec)
+	mon, path, id, secretFile, readOnly, err := getVolumeSource(spec)
 	if err != nil {
 		return nil, err
 	}
 
-	id := cephvs.User
 	if id == "" {
 		id = "admin"
 	}
-	path := cephvs.Path
 	if path == "" {
 		path = "/"
 	}
 	if !strings.HasPrefix(path, "/") {
 		path = "/" + path
 	}
-	secret_file := cephvs.SecretFile
+
-	if secret_file == "" {
+	if secretFile == "" {
-		secret_file = "/etc/ceph/" + id + ".secret"
+		secretFile = "/etc/ceph/" + id + ".secret"
 	}
 
 	return &cephfsMounter{
 		cephfs: &cephfs{
 			podUID:       podUID,
 			volName:      spec.Name(),
-			mon:          cephvs.Monitors,
+			mon:          mon,
 			path:         path,
 			secret:       secret,
 			id:           id,
-			secret_file:  secret_file,
+			secret_file:  secretFile,
-			readonly:     cephvs.ReadOnly,
+			readonly:     readOnly,
 			mounter:      mounter,
 			plugin:       plugin,
 			mountOptions: volume.MountOptionFromSpec(spec),
@@ -301,13 +299,46 @@ func (cephfsVolume *cephfs) execMount(mountpoint string) error {
 	return nil
 }
 
-func getVolumeSource(spec *volume.Spec) (*v1.CephFSVolumeSource, bool, error) {
+func getVolumeSource(spec *volume.Spec) ([]string, string, string, string, bool, error) {
 	if spec.Volume != nil && spec.Volume.CephFS != nil {
-		return spec.Volume.CephFS, spec.Volume.CephFS.ReadOnly, nil
+		mon := spec.Volume.CephFS.Monitors
+		path := spec.Volume.CephFS.Path
+		user := spec.Volume.CephFS.User
+		secretFile := spec.Volume.CephFS.SecretFile
+		readOnly := spec.Volume.CephFS.ReadOnly
+		return mon, path, user, secretFile, readOnly, nil
 	} else if spec.PersistentVolume != nil &&
 		spec.PersistentVolume.Spec.CephFS != nil {
-		return spec.PersistentVolume.Spec.CephFS, spec.ReadOnly, nil
+		mon := spec.PersistentVolume.Spec.CephFS.Monitors
+		path := spec.PersistentVolume.Spec.CephFS.Path
+		user := spec.PersistentVolume.Spec.CephFS.User
+		secretFile := spec.PersistentVolume.Spec.CephFS.SecretFile
+		readOnly := spec.PersistentVolume.Spec.CephFS.ReadOnly
+		return mon, path, user, secretFile, readOnly, nil
 	}
 
-	return nil, false, fmt.Errorf("Spec does not reference a CephFS volume type")
+	return nil, "", "", "", false, fmt.Errorf("Spec does not reference a CephFS volume type")
+}
+
+func getSecretNameAndNamespace(spec *volume.Spec, defaultNamespace string) (string, string, error) {
+	if spec.Volume != nil && spec.Volume.CephFS != nil {
+		localSecretRef := spec.Volume.CephFS.SecretRef
+		if localSecretRef != nil {
+			return localSecretRef.Name, defaultNamespace, nil
+		}
+		return "", "", nil
+
+	} else if spec.PersistentVolume != nil &&
+		spec.PersistentVolume.Spec.CephFS != nil {
+		secretRef := spec.PersistentVolume.Spec.CephFS.SecretRef
+		secretNs := defaultNamespace
+		if secretRef != nil {
+			if len(secretRef.Namespace) != 0 {
+				secretNs = secretRef.Namespace
+			}
+			return secretRef.Name, secretNs, nil
+		}
+		return "", "", nil
+	}
+	return "", "", fmt.Errorf("Spec does not reference an CephFS volume type")
 }

pkg/volume/cephfs/cephfs_test.go

@@ -135,3 +135,94 @@ func TestConstructVolumeSpec(t *testing.T) {
 		t.Errorf("Get wrong cephfs spec name, got: %s", cephfsSpec.Name())
 	}
 }
+
+type testcase struct {
+	name      string
+	defaultNs string
+	spec      *volume.Spec
+	// Expected return of the test
+	expectedName  string
+	expectedNs    string
+	expectedError error
+}
+
+func TestGetSecretNameAndNamespaceForPV(t *testing.T) {
+	tests := []testcase{
+		{
+			name:      "persistent volume source",
+			defaultNs: "default",
+			spec: &volume.Spec{
+				PersistentVolume: &v1.PersistentVolume{
+					Spec: v1.PersistentVolumeSpec{
+						PersistentVolumeSource: v1.PersistentVolumeSource{
+							CephFS: &v1.CephFSPersistentVolumeSource{
+								Monitors: []string{"a", "b"},
+								User:     "user",
+								SecretRef: &v1.SecretReference{
+									Name:      "name",
+									Namespace: "ns",
+								},
+								SecretFile: "/etc/ceph/user.secret",
+							},
+						},
+					},
+				},
+			},
+			expectedName:  "name",
+			expectedNs:    "ns",
+			expectedError: nil,
+		},
+		{
+			name:      "persistent volume source without namespace",
+			defaultNs: "default",
+			spec: &volume.Spec{
+				PersistentVolume: &v1.PersistentVolume{
+					Spec: v1.PersistentVolumeSpec{
+						PersistentVolumeSource: v1.PersistentVolumeSource{
+							CephFS: &v1.CephFSPersistentVolumeSource{
+								Monitors: []string{"a", "b"},
+								User:     "user",
+								SecretRef: &v1.SecretReference{
+									Name: "name",
+								},
+								SecretFile: "/etc/ceph/user.secret",
+							},
+						},
+					},
+				},
+			},
+			expectedName:  "name",
+			expectedNs:    "default",
+			expectedError: nil,
+		},
+		{
+			name:      "pod volume source",
+			defaultNs: "default",
+			spec: &volume.Spec{
+				Volume: &v1.Volume{
+					VolumeSource: v1.VolumeSource{
+						CephFS: &v1.CephFSVolumeSource{
+							Monitors: []string{"a", "b"},
+							User:     "user",
+							SecretRef: &v1.LocalObjectReference{
+								Name: "name",
+							},
+							SecretFile: "/etc/ceph/user.secret",
+						},
+					},
+				},
+			},
+			expectedName:  "name",
+			expectedNs:    "default",
+			expectedError: nil,
+		},
+	}
+	for _, testcase := range tests {
+		resultName, resultNs, err := getSecretNameAndNamespace(testcase.spec, testcase.defaultNs)
+		if err != testcase.expectedError || resultName != testcase.expectedName || resultNs != testcase.expectedNs {
+			t.Errorf("%s failed: expected err=%v ns=%q name=%q, got %v/%q/%q", testcase.name, testcase.expectedError, testcase.expectedNs, testcase.expectedName,
+				err, resultNs, resultName)
+		}
+	}
+
+}

staging/src/k8s.io/api/core/v1/types.go

@@ -409,7 +409,7 @@ type PersistentVolumeSource struct {
 	Cinder *CinderVolumeSource `json:"cinder,omitempty" protobuf:"bytes,8,opt,name=cinder"`
 	// CephFS represents a Ceph FS mount on the host that shares a pod's lifetime
 	// +optional
-	CephFS *CephFSVolumeSource `json:"cephfs,omitempty" protobuf:"bytes,9,opt,name=cephfs"`
+	CephFS *CephFSPersistentVolumeSource `json:"cephfs,omitempty" protobuf:"bytes,9,opt,name=cephfs"`
 	// FC represents a Fibre Channel resource that is attached to a kubelet's host machine and then exposed to the pod.
 	// +optional
 	FC *FCVolumeSource `json:"fc,omitempty" protobuf:"bytes,10,opt,name=fc"`
@@ -848,6 +848,45 @@ type CephFSVolumeSource struct {
 	ReadOnly bool `json:"readOnly,omitempty" protobuf:"varint,6,opt,name=readOnly"`
 }
 
+// SecretReference represents a Secret Reference. It has enough information to retrieve secret
+// in any namespace
+type SecretReference struct {
+	// Name is unique within a namespace to reference a secret resource.
+	// +optional
+	Name string `json:"name,omitempty" protobuf:"bytes,1,opt,name=name"`
+	// Namespace defines the space within which the secret name must be unique.
+	// +optional
+	Namespace string `json:"namespace,omitempty" protobuf:"bytes,2,opt,name=namespace"`
+}
+
+// Represents a Ceph Filesystem mount that lasts the lifetime of a pod
+// Cephfs volumes do not support ownership management or SELinux relabeling.
+type CephFSPersistentVolumeSource struct {
+	// Required: Monitors is a collection of Ceph monitors
+	// More info: https://releases.k8s.io/HEAD/examples/volumes/cephfs/README.md#how-to-use-it
+	Monitors []string `json:"monitors" protobuf:"bytes,1,rep,name=monitors"`
+	// Optional: Used as the mounted root, rather than the full Ceph tree, default is /
+	// +optional
+	Path string `json:"path,omitempty" protobuf:"bytes,2,opt,name=path"`
+	// Optional: User is the rados user name, default is admin
+	// More info: https://releases.k8s.io/HEAD/examples/volumes/cephfs/README.md#how-to-use-it
+	// +optional
+	User string `json:"user,omitempty" protobuf:"bytes,3,opt,name=user"`
+	// Optional: SecretFile is the path to key ring for User, default is /etc/ceph/user.secret
+	// More info: https://releases.k8s.io/HEAD/examples/volumes/cephfs/README.md#how-to-use-it
+	// +optional
+	SecretFile string `json:"secretFile,omitempty" protobuf:"bytes,4,opt,name=secretFile"`
+	// Optional: SecretRef is reference to the authentication secret for User, default is empty.
+	// More info: https://releases.k8s.io/HEAD/examples/volumes/cephfs/README.md#how-to-use-it
+	// +optional
+	SecretRef *SecretReference `json:"secretRef,omitempty" protobuf:"bytes,5,opt,name=secretRef"`
+	// Optional: Defaults to false (read/write). ReadOnly here will force
+	// the ReadOnly setting in VolumeMounts.
+	// More info: https://releases.k8s.io/HEAD/examples/volumes/cephfs/README.md#how-to-use-it
+	// +optional
+	ReadOnly bool `json:"readOnly,omitempty" protobuf:"varint,6,opt,name=readOnly"`
+}
+
 // Represents a Flocker volume mounted by the Flocker agent.
 // One and only one of datasetName and datasetUUID should be set.
 // Flocker volumes do not support ownership management or SELinux relabeling.