Merge pull request #38378 from obnoxxx/glusterfs-gid-checks

Automatic merge from submit-queue (batch tested with PRs 38284, 38403, 38265, 38378)

glusterfs: properly check gidMin and gidMax values from SC individually

<!--  Thanks for sending a pull request!  Here are some tips for you:
1. If this is your first time, read our contributor guidelines https://github.com/kubernetes/kubernetes/blob/master/CONTRIBUTING.md and developer guide https://github.com/kubernetes/kubernetes/blob/master/docs/devel/development.md
2. If you want *faster* PR reviews, read how: https://github.com/kubernetes/kubernetes/blob/master/docs/devel/faster_reviews.md
3. Follow the instructions for writing a release note: https://github.com/kubernetes/kubernetes/blob/master/docs/devel/pull-requests.md#release-notes
-->

**What this PR does / why we need it**:

This fixes a misleading debug message, and also prevents the glusterfs provisioner from adapting a misconfiguration of the gid-range in the storage class. Instead it will fail with proper error messages.

**Which issue this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close that issue when PR gets merged)*: fixes #

https://bugzilla.redhat.com/show_bug.cgi?id=1402286

**Special notes for your reviewer**:

**Release note**:
<!--  Steps to write your release note:
1. Use the release-note-* labels to set the release note state (if you have access) 
2. Enter your extended release note in the below block; leaving it blank means using the PR title as the release note. If no release note is required, just write `NONE`. 
-->
```release-note
```

Don't override explict out-of max-range configuration, but
fail with an error message instead.

Signed-off-by: Michael Adam <obnox@redhat.com>

pkg/volume/glusterfs/glusterfs.go

@@ -74,7 +74,12 @@ const (
 	gciGlusterMountBinariesPath = "/sbin/mount.glusterfs"
 	defaultGidMin               = 2000
 	defaultGidMax               = math.MaxInt32
-	absoluteGidMax              = math.MaxInt32
+	// absoluteGidMin/Max are currently the same as the
+	// default values, but they play a different role and
+	// could take a different value. Only thing we need is:
+	// absGidMin <= defGidMin <= defGidMax <= absGidMax
+	absoluteGidMin = 2000
+	absoluteGidMax = math.MaxInt32
 )
 
 func (plugin *glusterfsPlugin) Init(host volume.VolumeHost) error {
@@ -850,6 +855,9 @@ func parseClassParameters(params map[string]string, kubeClient clientset.Interfa
 	var cfg provisioningConfig
 	var err error
 
+	cfg.gidMin = defaultGidMin
+	cfg.gidMax = defaultGidMax
+
 	authEnabled := true
 	for k, v := range params {
 		switch dstrings.ToLower(k) {
@@ -874,12 +882,24 @@ func parseClassParameters(params map[string]string, kubeClient clientset.Interfa
 			if err != nil {
 				return nil, fmt.Errorf("glusterfs: invalid value %q for volume plugin %s", k, glusterfsPluginName)
 			}
+			if parseGidMin < absoluteGidMin {
+				return nil, fmt.Errorf("glusterfs: gidMin must be >= %v", absoluteGidMin)
+			}
+			if parseGidMin > absoluteGidMax {
+				return nil, fmt.Errorf("glusterfs: gidMin must be <= %v", absoluteGidMax)
+			}
 			cfg.gidMin = parseGidMin
 		case "gidmax":
 			parseGidMax, err := convertGid(v)
 			if err != nil {
 				return nil, fmt.Errorf("glusterfs: invalid value %q for volume plugin %s", k, glusterfsPluginName)
 			}
+			if parseGidMax < absoluteGidMin {
+				return nil, fmt.Errorf("glusterfs: gidMax must be >= %v", absoluteGidMin)
+			}
+			if parseGidMax > absoluteGidMax {
+				return nil, fmt.Errorf("glusterfs: gidMax must be <= %v", absoluteGidMax)
+			}
 			cfg.gidMax = parseGidMax
 		default:
 			return nil, fmt.Errorf("glusterfs: invalid option %q for volume plugin %s", k, glusterfsPluginName)
@@ -912,14 +932,6 @@ func parseClassParameters(params map[string]string, kubeClient clientset.Interfa
 		cfg.secretValue = cfg.userKey
 	}
 
-	if cfg.gidMin == 0 {
-		cfg.gidMin = defaultGidMin
-	}
-
-	if cfg.gidMax == 0 {
-		cfg.gidMax = defaultGidMax
-	}
-
 	if cfg.gidMin > cfg.gidMax {
 		return nil, fmt.Errorf("StorageClass for provisioner %q must have gidMax value >= gidMin", glusterfsPluginName)
 	}

pkg/volume/glusterfs/glusterfs_test.go

@@ -350,6 +350,130 @@ func TestParseClassParameters(t *testing.T) {
 			true, // expect error
 			nil,
 		},
+		{
+			"invalid gidMin #1",
+			map[string]string{
+				"resturl":         "https://localhost:8080",
+				"restauthenabled": "false",
+				"gidMin":          "0",
+			},
+			&secret,
+			true, // expect error
+			nil,
+		},
+		{
+			"invalid gidMin #2",
+			map[string]string{
+				"resturl":         "https://localhost:8080",
+				"restauthenabled": "false",
+				"gidMin":          "1999",
+			},
+			&secret,
+			true, // expect error
+			nil,
+		},
+		{
+			"invalid gidMin #3",
+			map[string]string{
+				"resturl":         "https://localhost:8080",
+				"restauthenabled": "false",
+				"gidMin":          "1999",
+			},
+			&secret,
+			true, // expect error
+			nil,
+		},
+		{
+			"invalid gidMax #1",
+			map[string]string{
+				"resturl":         "https://localhost:8080",
+				"restauthenabled": "false",
+				"gidMax":          "0",
+			},
+			&secret,
+			true, // expect error
+			nil,
+		},
+		{
+			"invalid gidMax #2",
+			map[string]string{
+				"resturl":         "https://localhost:8080",
+				"restauthenabled": "false",
+				"gidMax":          "1999",
+			},
+			&secret,
+			true, // expect error
+			nil,
+		},
+		{
+			"invalid gidMax #3",
+			map[string]string{
+				"resturl":         "https://localhost:8080",
+				"restauthenabled": "false",
+				"gidMax":          "1999",
+			},
+			&secret,
+			true, // expect error
+			nil,
+		},
+		{
+			"invalid gidMin:gidMax",
+			map[string]string{
+				"resturl":         "https://localhost:8080",
+				"restauthenabled": "false",
+				"gidMin":          "5001",
+				"gidMax":          "5000",
+			},
+			&secret,
+			true, // expect error
+			nil,
+		},
+		{
+			"valid gidMin",
+			map[string]string{
+				"resturl":         "https://localhost:8080",
+				"restauthenabled": "false",
+				"gidMin":          "4000",
+			},
+			&secret,
+			false, // expect error
+			&provisioningConfig{
+				url:    "https://localhost:8080",
+				gidMin: 4000,
+				gidMax: 2147483647,
+			},
+		},
+		{
+			"valid gidMax",
+			map[string]string{
+				"resturl":         "https://localhost:8080",
+				"restauthenabled": "false",
+				"gidMax":          "5000",
+			},
+			&secret,
+			false, // expect error
+			&provisioningConfig{
+				url:    "https://localhost:8080",
+				gidMin: 2000,
+				gidMax: 5000,
+			},
+		},
+		{
+			"valid gidMin:gidMax",
+			map[string]string{
+				"resturl":         "https://localhost:8080",
+				"restauthenabled": "false",
+				"gidMin":          "4000",
+				"gidMax":          "5000",
+			},
+			&secret,
+			false, // expect error
+			&provisioningConfig{
+				url:    "https://localhost:8080",
+				gidMin: 4000,
+				gidMax: 5000,
+			},
+		},
 	}
 
 	for _, test := range tests {