Merge pull request #25986 from Honny1/fix-unlimited-ulimits

Fix handling of "r_limits" in Podman REST API /libpod/containers/create

Author: openshift-merge-bot[bot]

pkg/api/handlers/libpod/containers_create.go

@@ -6,6 +6,7 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
+	"math"
 	"net/http"
 	"strconv"
 
@@ -18,8 +19,28 @@ import (
 	"github.com/containers/podman/v5/pkg/specgen/generate"
 	"github.com/containers/podman/v5/pkg/specgenutil"
 	"github.com/containers/storage"
+	"github.com/opencontainers/runtime-spec/specs-go"
 )
 
+// The JSON decoder correctly cannot decode (overflow) negative values (e.g., `-1`) for fields of type `uint64`,
+// as `-1` is used to represent `max` in `POSIXRlimit`. To address this, we use `tmpSpecGenerator` to decode the request body.
+// The `tmpSpecGenerator` overrides the `POSIXRlimit` type with a `tmpRlimit` type that uses the `json.Number` type for decoding values.
+// The `tmpRlimit` is then parsed into the `POSIXRlimit` type and assigned to the `SpecGenerator`.
+// This serves as a workaround for the issue (https://github.com/containers/podman/issues/24886).
+type tmpSpecGenerator struct {
+	specgen.SpecGenerator
+	Rlimits []tmpRlimit `json:"r_limits,omitempty"`
+}
+
+type tmpRlimit struct {
+	// Type of the rlimit to set
+	Type string `json:"type"`
+	// Hard is the hard limit for the specified type
+	Hard json.Number `json:"hard"`
+	// Soft is the soft limit for the specified type
+	Soft json.Number `json:"soft"`
+}
+
 // CreateContainer takes a specgenerator and makes a container. It returns
 // the new container ID on success along with any warnings.
 func CreateContainer(w http.ResponseWriter, r *http.Request) {
@@ -35,25 +56,36 @@ func CreateContainer(w http.ResponseWriter, r *http.Request) {
 	privileged := conf.Containers.Privileged
 
 	// we have to set the default before we decode to make sure the correct default is set when the field is unset
-	sg := specgen.SpecGenerator{
-		ContainerNetworkConfig: specgen.ContainerNetworkConfig{
-			UseImageHosts: &noHosts,
-		},
-		ContainerSecurityConfig: specgen.ContainerSecurityConfig{
-			Umask:      conf.Containers.Umask,
-			Privileged: &privileged,
-		},
-		ContainerHealthCheckConfig: specgen.ContainerHealthCheckConfig{
-			HealthLogDestination: define.DefaultHealthCheckLocalDestination,
-			HealthMaxLogCount:    define.DefaultHealthMaxLogCount,
-			HealthMaxLogSize:     define.DefaultHealthMaxLogSize,
+	tmpSg := tmpSpecGenerator{
+		SpecGenerator: specgen.SpecGenerator{
+			ContainerNetworkConfig: specgen.ContainerNetworkConfig{
+				UseImageHosts: &noHosts,
+			},
+			ContainerSecurityConfig: specgen.ContainerSecurityConfig{
+				Umask:      conf.Containers.Umask,
+				Privileged: &privileged,
+			},
+			ContainerHealthCheckConfig: specgen.ContainerHealthCheckConfig{
+				HealthLogDestination: define.DefaultHealthCheckLocalDestination,
+				HealthMaxLogCount:    define.DefaultHealthMaxLogCount,
+				HealthMaxLogSize:     define.DefaultHealthMaxLogSize,
+			},
 		},
 	}
 
-	if err := json.NewDecoder(r.Body).Decode(&sg); err != nil {
+	if err := json.NewDecoder(r.Body).Decode(&tmpSg); err != nil {
 		utils.Error(w, http.StatusInternalServerError, fmt.Errorf("decode(): %w", err))
 		return
 	}
+
+	sg := tmpSg.SpecGenerator
+	rLimits, err := parseRLimits(tmpSg.Rlimits)
+	if err != nil {
+		utils.Error(w, http.StatusBadRequest, fmt.Errorf("invalid rlimit: %w", err))
+		return
+	}
+	sg.Rlimits = rLimits
+
 	if sg.Passwd == nil {
 		t := true
 		sg.Passwd = &t
@@ -100,3 +132,42 @@ func CreateContainer(w http.ResponseWriter, r *http.Request) {
 	response := entities.ContainerCreateResponse{ID: ctr.ID(), Warnings: warn}
 	utils.WriteJSON(w, http.StatusCreated, response)
 }
+
+// parseRLimits parses slice of tmpLimit to slice of specs.POSIXRlimit.
+func parseRLimits(rLimits []tmpRlimit) ([]specs.POSIXRlimit, error) {
+	rl := []specs.POSIXRlimit{}
+
+	// The "soft" and "hard" values are expected to be of type uint64.
+	// The JSON decoder cannot cast -1 as to uint64.
+	// We need to convert them to uint64, and handle the special case of -1
+	// which indicates an max value.
+	parseLimitNumber := func(limit json.Number) (uint64, error) {
+		limitString := limit.String()
+		if limitString == "-1" {
+			// uint64(-1) overflow to max uint64 value
+			return math.MaxUint64, nil
+		}
+		return strconv.ParseUint(limitString, 10, 64)
+	}
+
+	for _, rLimit := range rLimits {
+		soft, err := parseLimitNumber(rLimit.Soft)
+		if err != nil {
+			return nil, fmt.Errorf("invalid value for POSIXRlimit.soft: %w", err)
+		}
+		hard, err := parseLimitNumber(rLimit.Hard)
+		if err != nil {
+			return nil, fmt.Errorf("invalid value for POSIXRlimit.hard: %w", err)
+		}
+		if rLimit.Type == "" {
+			return nil, fmt.Errorf("invalid value for POSIXRlimit.type: %w", err)
+		}
+
+		rl = append(rl, specs.POSIXRlimit{
+			Type: rLimit.Type,
+			Soft: soft,
+			Hard: hard,
+		})
+	}
+	return rl, nil
+}

test/apiv2/20-containers.at

@@ -772,6 +772,29 @@ if root; then
   podman rm -f updateCtr
 fi
 
+
+# test if API support -1 for ulimits https://github.com/containers/podman/issues/24886
+
+# Compat API
+t POST containers/create \
+  Image=$IMAGE           \
+  HostConfig='{"Ulimits":[{"Name":"memlock","Soft":-1,"Hard":-1}]}' \
+  201                    \
+  .Id~[0-9a-f]\\{64\\}
+cid=$(jq -r '.Id' <<<"$output")
+
+t DELETE containers/$cid 204
+
+# Libpod API
+t POST libpod/containers/create \
+  Image=$IMAGE           \
+  r_limits='[{"type":"memlock","soft":-1,"hard":-1}]' \
+  201                    \
+  .Id~[0-9a-f]\\{64\\}
+cid=$(jq -r '.Id' <<<"$output")
+
+t DELETE containers/$cid 204
+
 rm -rf $TMPD
 
 podman container rm -fa

test/system/030-run.bats

@@ -1309,6 +1309,14 @@ EOF
         fi
     fi
 
+    ctr="c-h-$(safename)"
+    run_podman run -d --name $ctr --ulimit core=-1:-1 $IMAGE /home/podman/pause
+
+    run_podman inspect $ctr --format "{{.HostConfig.Ulimits}}"
+    assert "$output" =~ "RLIMIT_CORE -1 -1" "ulimit core is not set to unlimited"
+
+    run_podman rm -f $ctr
+
     run_podman run --ulimit core=-1:-1 --rm $IMAGE grep core /proc/self/limits
     assert "$output" =~ " ${max}  * ${max}  * bytes"