ulimit-adjuster: add validation for hard limits

samuelkarp · samuelkarp · commit 5ecea042d15b · 2023-09-11T12:04:25.000-07:00
hard limits should always be &gt;= soft limits

Signed-off-by: Samuel Karp &lt;samuelkarp@google.com&gt;
diff --git a/plugins/ulimit-adjuster/adjuster.go b/plugins/ulimit-adjuster/adjuster.go
@@ -123,10 +123,10 @@ func (p *plugin) CreateContainer(
 		log.G(ctx).WithError(err).Debug("failed to parse annotations")
 		return nil, nil, err
 	}
-	adjust := &api.ContainerAdjustment{}
-	for _, u := range ulimits {
-		log.G(ctx).WithField("type", u.Type).WithField("hard", u.Hard).WithField("soft", u.Soft).Debug("adjust rlimit")
-		adjust.AddRlimit(u.Type, u.Hard, u.Soft)
+
+	adjust, err := adjustUlimits(ctx, ulimits)
+	if err != nil {
+		return nil, nil, err
 	}
 	return adjust, nil, nil
 }
@@ -159,3 +159,17 @@ func parseUlimits(ctx context.Context, container string, annotations map[string]
 	}
 	return ulimits, nil
 }
+
+func adjustUlimits(ctx context.Context, ulimits []ulimit) (*api.ContainerAdjustment, error) {
+	adjust := &api.ContainerAdjustment{}
+	for _, u := range ulimits {
+		l := log.G(ctx).WithField("type", u.Type).WithField("hard", u.Hard).WithField("soft", u.Soft)
+		if u.Hard < u.Soft {
+			l.Debug("failed to apply ulimit with hard < soft")
+			return nil, fmt.Errorf("ulimit %q must have hard limit >= soft limit", u.Type)
+		}
+		log.G(ctx).WithField("type", u.Type).WithField("hard", u.Hard).WithField("soft", u.Soft).Debug("adjust rlimit")
+		adjust.AddRlimit(u.Type, u.Hard, u.Soft)
+	}
+	return adjust, nil
+}
diff --git a/plugins/ulimit-adjuster/adjuster_test.go b/plugins/ulimit-adjuster/adjuster_test.go
@@ -21,6 +21,8 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+
+	"github.com/containerd/nri/pkg/api"
 )
 
 func TestParseAnnotations(t *testing.T) {
@@ -161,3 +163,80 @@ func TestParseAnnotations(t *testing.T) {
 		})
 	}
 }
+
+func TestAdjustUlimits(t *testing.T) {
+	tests := map[string]struct {
+		ulimits  []ulimit
+		expected *api.ContainerAdjustment
+		errStr   string
+	}{
+		"empty": {
+			ulimits:  nil,
+			expected: &api.ContainerAdjustment{},
+		},
+		"invalid-hard": {
+			ulimits: []ulimit{{
+				Type: "RLIMIT_NOFILE",
+				Hard: 0,
+				Soft: 100,
+			}},
+			errStr: `ulimit "RLIMIT_NOFILE" must have hard limit >= soft limit`,
+		},
+		"one": {
+			ulimits: []ulimit{{
+				Type: "RLIMIT_MEMLOCK",
+				Hard: 100,
+				Soft: 99,
+			}},
+			expected: &api.ContainerAdjustment{Rlimits: []*api.POSIXRlimit{{
+				Type: "RLIMIT_MEMLOCK",
+				Hard: 100,
+				Soft: 99,
+			}}},
+		},
+		"one-invalid": {
+			ulimits: []ulimit{{
+				Type: "RLIMIT_MEMLOCK",
+				Hard: 100,
+				Soft: 99,
+			}, {
+				Type: "RLIMIT_NOFILE",
+				Hard: 0,
+				Soft: 100,
+			}},
+			errStr: `ulimit "RLIMIT_NOFILE" must have hard limit >= soft limit`,
+		},
+		"multiple-valid": {
+			ulimits: []ulimit{{
+				Type: "RLIMIT_MEMLOCK",
+				Hard: 100,
+				Soft: 99,
+			}, {
+				Type: "RLIMIT_AS",
+				Hard: 10,
+				Soft: 0,
+			}},
+			expected: &api.ContainerAdjustment{Rlimits: []*api.POSIXRlimit{{
+				Type: "RLIMIT_MEMLOCK",
+				Hard: 100,
+				Soft: 99,
+			}, {
+				Type: "RLIMIT_AS",
+				Hard: 10,
+				Soft: 0,
+			}}},
+		},
+	}
+	for name, tc := range tests {
+		t.Run(name, func(t *testing.T) {
+			adjust, err := adjustUlimits(context.Background(), tc.ulimits)
+			if tc.errStr != "" {
+				assert.EqualError(t, err, tc.errStr)
+				assert.Nil(t, adjust)
+			} else {
+				assert.NoError(t, err)
+				assert.Equal(t, tc.expected, adjust)
+			}
+		})
+	}
+}
