Repository: containers/podman
Pull request #11224 (2021-08-13 07:53:13)

Add space trimming check in ValidateSysctls
This is to catch invalid sysctl configs with extra spacing.

See
https://github.com/containers/common/issues/723#issuecomment-897395506

Signed-off-by: xatier <xatierlike@gmail.com>

<!--
Thanks for sending a pull request!

Please make sure you've read our contributing guidelines and how to submit a pull request (https://github.com/containers/podman/blob/master/CONTRIBUTING.md#submitting-pull-requests).

In case you're only changing docs, make sure to prefix the pull-request title with "[CI:DOCS]".  That will prevent functional tests from running and save time and energy.
-->


openshift-ci[bot]: [APPROVALNOTIFIER] This PR is **NOT APPROVED**

This pull-request has been approved by: *<a href="https://github.com/containers/podman/pull/11224#" title="Author self-approved">xatier</a>*
To complete the [pull request process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process), please assign **baude** after the PR has been reviewed.
You can assign the PR to them by writing `/assign @baude` in a comment when ready.

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=containers%2Fpodman).

<details open>
Needs approval from an approver in each of these files:

- **[OWNERS](https://github.com/containers/podman/blob/main/OWNERS)**

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":["baude"]} -->
unknowndevQwQ: Cloud you consider adding something like this for ulimit?
xatier: @unknowndevQwQ, I didn't look into that code path too closely, sorry.

> Cloud you consider adding something like this for ulimit?



Patch:
diff --git a/pkg/util/utils.go b/pkg/util/utils.go
index 774590f44f..63fad0286a 100644
--- a/pkg/util/utils.go
+++ b/pkg/util/utils.go
@@ -618,6 +618,12 @@ func ValidateSysctls(strSlice []string) (map[string]string, error) {
 		if len(arr) < 2 {
 			return nil, errors.Errorf("%s is invalid, sysctl values must be in the form of KEY=VALUE", val)
 		}
+
+		trimmed := fmt.Sprintf("%s=%s", strings.TrimSpace(arr[0]), strings.TrimSpace(arr[1]))
+		if trimmed != val {
+			return nil, errors.Errorf("'%s' is invalid, extra spaces found", val)
+		}
+
 		if validSysctlMap[arr[0]] {
 			sysctl[arr[0]] = arr[1]
 			continue


Test patch:
diff --git a/pkg/util/utils_test.go b/pkg/util/utils_test.go
index 027acbdabc..62de7509fe 100644
--- a/pkg/util/utils_test.go
+++ b/pkg/util/utils_test.go
@@ -1,6 +1,7 @@
 package util
 
 import (
+	"fmt"
 	"testing"
 	"time"
 
@@ -259,6 +260,28 @@ func TestValidateSysctlBadSysctl(t *testing.T) {
 	assert.Error(t, err)
 }
 
+func TestValidateSysctlBadSysctlWithExtraSpaces(t *testing.T) {
+	expectedError := "'%s' is invalid, extra spaces found"
+
+	// should fail fast on first sysctl
+	strSlice1 := []string{
+		"net.ipv4.ping_group_range = 0 0",
+		"net.ipv4.ping_group_range=0 0 ",
+	}
+	_, err := ValidateSysctls(strSlice1)
+	assert.Error(t, err)
+	assert.Equal(t, err.Error(), fmt.Sprintf(expectedError, strSlice1[0]))
+
+	// should fail on second sysctl
+	strSlice2 := []string{
+		"net.ipv4.ping_group_range=0 0",
+		"net.ipv4.ping_group_range=0 0 ",
+	}
+	_, err = ValidateSysctls(strSlice2)
+	assert.Error(t, err)
+	assert.Equal(t, err.Error(), fmt.Sprintf(expectedError, strSlice2[1]))
+}
+
 func TestCoresToPeriodAndQuota(t *testing.T) {
 	cores := 1.0
 	expectedPeriod := DefaultCPUPeriod