From c5bc09097447c4bc021003be448dfd84bfb4fdd0 Mon Sep 17 00:00:00 2001
From: tsullens <tsullens@mix.wvu.edu>
Date: Wed, 17 Oct 2018 23:01:31 -0400
Subject: [PATCH] nfs: update operator for server spec validation

adding validation of NFSServerSpec and unit tests
in accordance with issues #2036 and #2037

Signed-off-by: tsullens <tsullens@mix.wvu.edu>
---
 pkg/operator/nfs/controller.go      | 53 ++++++++++++++++++++++++++-
 pkg/operator/nfs/controller_test.go | 57 +++++++++++++++++++++++++++++
 2 files changed, 108 insertions(+), 2 deletions(-)

diff --git a/pkg/operator/nfs/controller.go b/pkg/operator/nfs/controller.go
index 84be1550a..4b64bbea1 100644
--- a/pkg/operator/nfs/controller.go
+++ b/pkg/operator/nfs/controller.go
@@ -184,9 +184,16 @@ func (c *Controller) createNFSService(nfsServer *nfsServer) error {
 
 func createGaneshaExport(id int, path string, access string, squash string) string {
 	var accessType string
-	if access == "ReadWrite" {
+	// 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"
 	}
+
 	idStr := fmt.Sprintf("%v", id)
 	nfsGaneshaConfig := `
 EXPORT {
@@ -197,7 +204,7 @@ EXPORT {
 	Transports = TCP;
 	Sectype = sys;
 	Access_Type = ` + accessType + `;
-	Squash = ` + squash + `;
+	Squash = ` + s.ToLower(squash) + `;
 	FSAL {
 		Name = VFS;
 	}
@@ -398,6 +405,12 @@ func (c *Controller) onAdd(obj interface{}) {
 
 	logger.Infof("new NFS server %s added to namespace %s", nfsObj.Name, nfsServer.namespace)
 
+	logger.Infof("validating nfs server spec in namespace %s", nfsServer.namespace)
+	if err := validateNFSServerSpec(nfsServer.spec); err != nil {
+		logger.Errorf("Invalid NFS Server spec: %+v", err)
+		return
+	}
+
 	logger.Infof("creating nfs server service in namespace %s", nfsServer.namespace)
 	if err := c.createNFSService(nfsServer); err != nil {
 		logger.Errorf("Unable to create NFS service %+v", err)
@@ -424,3 +437,39 @@ func (c *Controller) onDelete(obj interface{}) {
 	cluster := obj.(*nfsv1alpha1.NFSServer).DeepCopy()
 	logger.Infof("cluster %s deleted from namespace %s", cluster.Name, cluster.Namespace)
 }
+
+func validateNFSServerSpec(spec nfsv1alpha1.NFSServerSpec) error {
+	serverConfig := spec.Exports
+	for _, export := range serverConfig {
+		if err := validateAccessMode(export.Server.AccessMode); err != nil {
+			return err
+		}
+		if err := validateSquashMode(export.Server.Squash); err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
+func validateAccessMode(mode string) error {
+	switch s.ToLower(mode) {
+	case "readonly":
+	case "readwrite":
+	case "none":
+	default:
+		return fmt.Errorf("Invalid value (%s) for accessMode, valid values are (ReadOnly, ReadWrite, none)", mode)
+	}
+	return nil
+}
+
+func validateSquashMode(mode string) error {
+	switch s.ToLower(mode) {
+	case "none":
+	case "rootid":
+	case "root":
+	case "all":
+	default:
+		return fmt.Errorf("Invalid value (%s) for squash, valid values are (none, rootId, root, all)", mode)
+	}
+	return nil
+}
diff --git a/pkg/operator/nfs/controller_test.go b/pkg/operator/nfs/controller_test.go
index 974bfe485..497d337b9 100644
--- a/pkg/operator/nfs/controller_test.go
+++ b/pkg/operator/nfs/controller_test.go
@@ -17,6 +17,7 @@ package nfs
 
 import (
 	"fmt"
+	"strings"
 	"testing"
 
 	nfsv1alpha1 "github.com/rook/rook/pkg/apis/nfs.rook.io/v1alpha1"
@@ -29,6 +30,62 @@ 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",
+				},
+			},
+		},
+	}
+
+	err := validateNFSServerSpec(spec)
+	assert.Nil(t, err)
+
+	// test that AccessMode is invalid
+	spec = nfsv1alpha1.NFSServerSpec{
+		Replicas: 1,
+		Exports: []nfsv1alpha1.ExportsSpec{
+			{
+				Name: "test",
+				Server: nfsv1alpha1.ServerSpec{
+					AccessMode: "badValue",
+					Squash:     "none",
+				},
+			},
+		},
+	}
+
+	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",
+				},
+			},
+		},
+	}
+
+	err = validateNFSServerSpec(spec)
+	assert.NotNil(t, err)
+	assert.True(t, strings.Contains(err.Error(), "Invalid value (badValue) for squash"))
+}
+
 func TestOnAdd(t *testing.T) {
 	namespace := "rook-nfs-test"
 	nfsserver := &nfsv1alpha1.NFSServer{