From 3d28603f59e5e46b4e75533baace005b28f74651 Mon Sep 17 00:00:00 2001 From: Tyler Sullens Date: Wed, 28 Nov 2018 22:53:01 -0500 Subject: [PATCH] nfs: support client access control for NFS volumes adding functionality and tests for issue #2283 Signed-off-by: Tyler Sullens --- pkg/operator/nfs/controller.go | 59 ++++++++---- pkg/operator/nfs/controller_test.go | 138 ++++++++++++++++++---------- 2 files changed, 131 insertions(+), 66 deletions(-) diff --git a/pkg/operator/nfs/controller.go b/pkg/operator/nfs/controller.go index 4b64bbea118c6..a84a07965442e 100644 --- a/pkg/operator/nfs/controller.go +++ b/pkg/operator/nfs/controller.go @@ -117,16 +117,13 @@ func nfsOwnerRef(namespace, nfsServerID string) metav1.OwnerReference { } } -func getServerConfig(exports []nfsv1alpha1.ExportsSpec) map[string]map[string]string { - claimConfigOpt := make(map[string]map[string]string) - configOpt := make(map[string]string) +func getServerConfig(exports []nfsv1alpha1.ExportsSpec) map[string]nfsv1alpha1.ServerSpec { + claimConfigOpt := make(map[string]nfsv1alpha1.ServerSpec) for _, export := range exports { claimName := export.PersistentVolumeClaim.ClaimName if claimName != "" { - configOpt["accessMode"] = export.Server.AccessMode - configOpt["squash"] = export.Server.Squash - claimConfigOpt[claimName] = configOpt + claimConfigOpt[claimName] = export.Server } } @@ -182,16 +179,17 @@ func (c *Controller) createNFSService(nfsServer *nfsServer) error { return nil } -func createGaneshaExport(id int, path string, access string, squash string) string { - var accessType string - // validateNFSServerSpec guarantees `access` will be one of these values at this point - switch s.ToLower(access) { - case "readwrite": - accessType = "RW" - case "readonly": - accessType = "RO" - case "none": - accessType = "None" +func createGaneshaExport(id int, path string, serverConfig nfsv1alpha1.ServerSpec) string { + + ganeshaClientConfigs := make([]string, 0) + for _, clientConfig := range serverConfig.AllowedClients { + ganeshaConf := ` + CLIENT { + Clients = ` + s.Join(clientConfig.Clients, ", ") + `; + Access_Type = ` + accessType(clientConfig.AccessMode) + `; + Squash = ` + clientConfig.Squash + `; + }` + ganeshaClientConfigs = append(ganeshaClientConfigs, ganeshaConf) } idStr := fmt.Sprintf("%v", id) @@ -203,23 +201,36 @@ EXPORT { Protocols = 4; Transports = TCP; Sectype = sys; - Access_Type = ` + accessType + `; - Squash = ` + s.ToLower(squash) + `; + Access_Type = ` + accessType(serverConfig.AccessMode) + `; + Squash = ` + s.ToLower(serverConfig.Squash) + `; FSAL { Name = VFS; - } + }` + s.Join(ganeshaClientConfigs, "") + ` }` return nfsGaneshaConfig } +func accessType(val string) string { + switch s.ToLower(val) { + case "readwrite": + return "RW" + case "readonly": + return "RO" + case "none": + return "None" + default: + return "" + } +} + func createGaneshaConfig(spec *nfsv1alpha1.NFSServerSpec) string { serverConfig := getServerConfig(spec.Exports) exportsList := make([]string, 0) id := 10 for claimName, claimConfig := range serverConfig { - exportsList = append(exportsList, createGaneshaExport(id, claimName, claimConfig["accessMode"], claimConfig["squash"])) + exportsList = append(exportsList, createGaneshaExport(id, claimName, claimConfig)) id++ } @@ -447,6 +458,14 @@ func validateNFSServerSpec(spec nfsv1alpha1.NFSServerSpec) error { if err := validateSquashMode(export.Server.Squash); err != nil { return err } + for _, client := range export.Server.AllowedClients { + if err := validateAccessMode(client.AccessMode); err != nil { + return err + } + if err := validateSquashMode(client.Squash); err != nil { + return err + } + } } return nil } diff --git a/pkg/operator/nfs/controller_test.go b/pkg/operator/nfs/controller_test.go index 497d337b95d70..59535b1e566cb 100644 --- a/pkg/operator/nfs/controller_test.go +++ b/pkg/operator/nfs/controller_test.go @@ -17,7 +17,6 @@ package nfs import ( "fmt" - "strings" "testing" nfsv1alpha1 "github.com/rook/rook/pkg/apis/nfs.rook.io/v1alpha1" @@ -30,63 +29,110 @@ import ( "k8s.io/client-go/kubernetes/fake" ) -func TestValidateNFSServerSpec(t *testing.T) { - - // first, test that a good NFSServerSpec is good - spec := nfsv1alpha1.NFSServerSpec{ - Replicas: 1, - Exports: []nfsv1alpha1.ExportsSpec{ - { - Name: "test", - Server: nfsv1alpha1.ServerSpec{ - AccessMode: "readwrite", - Squash: "none", +func TestOnAddComplexServer(t *testing.T) { + namespace := "rook-nfs-test" + nfsserver := &nfsv1alpha1.NFSServer{ + ObjectMeta: metav1.ObjectMeta{ + Name: "nfs-server-X", + Namespace: namespace, + }, + Spec: nfsv1alpha1.NFSServerSpec{ + Replicas: 1, + Exports: []nfsv1alpha1.ExportsSpec{ + { + Name: "export-test", + Server: nfsv1alpha1.ServerSpec{ + AccessMode: "ReadWrite", + Squash: "none", + AllowedClients: []nfsv1alpha1.AllowedClientsSpec{ + { + Name: "client-test-1", + Clients: []string{"172.17.0.5"}, + AccessMode: "ReadOnly", + Squash: "root", + }, + { + Name: "client-test-2", + Clients: []string{"172.17.0.0/16", "serverX"}, + AccessMode: "ReadWrite", + Squash: "none", + }, + }, + }, + PersistentVolumeClaim: v1.PersistentVolumeClaimVolumeSource{ + ClaimName: "test-claim", + }, }, }, }, } - err := validateNFSServerSpec(spec) + // initialize the controller and its dependencies + clientset := testop.New(3) + context := &clusterd.Context{Clientset: clientset} + controller := NewController(context, "rook/nfs:mockTag") + + // in a background thread, simulate the pods running (fake statefulsets don't automatically do that) + go simulatePodsRunning(clientset, namespace, nfsserver.Spec.Replicas) + + // call onAdd given the specified nfsserver + controller.onAdd(nfsserver) + + // verify client service + clientService, err := clientset.CoreV1().Services(namespace).Get(appName, metav1.GetOptions{}) assert.Nil(t, err) + assert.NotNil(t, clientService) + assert.Equal(t, v1.ServiceTypeClusterIP, clientService.Spec.Type) - // test that AccessMode is invalid - spec = nfsv1alpha1.NFSServerSpec{ - Replicas: 1, - Exports: []nfsv1alpha1.ExportsSpec{ - { - Name: "test", - Server: nfsv1alpha1.ServerSpec{ - AccessMode: "badValue", - Squash: "none", - }, - }, - }, + // verify nfs-ganesha config in the configmap + configMap, err := clientset.CoreV1().ConfigMaps(namespace).Get(nfsConfigMapName, metav1.GetOptions{}) + assert.Nil(t, err) + assert.NotNil(t, configMap) + nfsGaneshaConfig := ` +EXPORT { + Export_Id = 10; + Path = /test-claim; + Pseudo = /test-claim; + Protocols = 4; + Transports = TCP; + Sectype = sys; + Access_Type = RW; + Squash = none; + FSAL { + Name = VFS; } - - err = validateNFSServerSpec(spec) - assert.NotNil(t, err) - assert.True(t, strings.Contains(err.Error(), "Invalid value (badValue) for accessMode")) - - // test that Squash is invalid - spec = nfsv1alpha1.NFSServerSpec{ - Replicas: 1, - Exports: []nfsv1alpha1.ExportsSpec{ - { - Name: "test", - Server: nfsv1alpha1.ServerSpec{ - AccessMode: "ReadWrite", - Squash: "badValue", - }, - }, - }, + CLIENT { + Clients = 172.17.0.5; + Access_Type = RO; + Squash = root; + } + CLIENT { + Clients = 172.17.0.0/16, serverX; + Access_Type = RW; + Squash = none; } +} +NFS_Core_Param +{ + fsid_device = true; +}` + assert.Equal(t, nfsGaneshaConfig, configMap.Data[nfsConfigMapName]) + + // verify stateful set + ss, err := clientset.AppsV1beta1().StatefulSets(namespace).Get(appName, metav1.GetOptions{}) + assert.Nil(t, err) + assert.NotNil(t, ss) + assert.Equal(t, int32(1), *ss.Spec.Replicas) + assert.Equal(t, 1, len(ss.Spec.Template.Spec.Containers)) - err = validateNFSServerSpec(spec) - assert.NotNil(t, err) - assert.True(t, strings.Contains(err.Error(), "Invalid value (badValue) for squash")) + container := ss.Spec.Template.Spec.Containers[0] + assert.Equal(t, 2, len(container.VolumeMounts)) + + expectedVolumeMounts := []v1.VolumeMount{{Name: "test-claim", MountPath: "/test-claim"}, {Name: "nfs-ganesha-config", MountPath: "/nfs-ganesha/config"}} + assert.Equal(t, expectedVolumeMounts, container.VolumeMounts) } -func TestOnAdd(t *testing.T) { +func TestOnAddSimpleServer(t *testing.T) { namespace := "rook-nfs-test" nfsserver := &nfsv1alpha1.NFSServer{ ObjectMeta: metav1.ObjectMeta{