From e59b9dae8fd7d0a2a3f079b970c9f47909c11af9 Mon Sep 17 00:00:00 2001 From: Nicola Sella Date: Wed, 27 May 2026 12:53:38 +0200 Subject: [PATCH] api/libpod: fix r_limits cleared on update Returnin nil when input is empty, to skip rlimit replacement when r_limits is not present in the request. Fixes: https://issues.redhat.com/browse/RHEL-178653 Signed-off-by: Nicola Sella --- pkg/api/handlers/libpod/containers.go | 7 ++- pkg/api/handlers/libpod/containers_test.go | 58 ++++++++++++++++++++++ test/apiv2/29-containersUpdate.at | 34 +++++++++++++ 3 files changed, 98 insertions(+), 1 deletion(-) create mode 100644 pkg/api/handlers/libpod/containers_test.go create mode 100644 test/apiv2/29-containersUpdate.at diff --git a/pkg/api/handlers/libpod/containers.go b/pkg/api/handlers/libpod/containers.go index cb55704bc3..37cf19730c 100644 --- a/pkg/api/handlers/libpod/containers.go +++ b/pkg/api/handlers/libpod/containers.go @@ -533,8 +533,13 @@ func UpdateContainer(w http.ResponseWriter, r *http.Request) { } // parseRLimits parses slice of WirePOSIXRlimit to slice of specs.POSIXRlimit. +// Returns nil when rLimits is empty so that the caller can distinguish if +// limits are set. Otherwise rlimits will be wiped on every run. func parseRLimits(rLimits []WirePOSIXRlimit) ([]specs.POSIXRlimit, error) { - rl := []specs.POSIXRlimit{} + if len(rLimits) == 0 { + return nil, nil + } + rl := make([]specs.POSIXRlimit, 0, len(rLimits)) for _, rLimit := range rLimits { if rLimit.Type == "" { return nil, fmt.Errorf("invalid value for POSIXRlimit.type: empty") diff --git a/pkg/api/handlers/libpod/containers_test.go b/pkg/api/handlers/libpod/containers_test.go new file mode 100644 index 0000000000..768c689eea --- /dev/null +++ b/pkg/api/handlers/libpod/containers_test.go @@ -0,0 +1,58 @@ +//go:build !remote && (linux || freebsd) + +package libpod + +import ( + "testing" + + specs "github.com/opencontainers/runtime-spec/specs-go" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestParseRLimits(t *testing.T) { + t.Run("nil input returns nil", func(t *testing.T) { + result, err := parseRLimits(nil) + require.NoError(t, err) + assert.Nil(t, result, "nil input must return nil so container_internal.go skips rlimit replacement") + }) + + t.Run("empty slice returns nil", func(t *testing.T) { + result, err := parseRLimits([]WirePOSIXRlimit{}) + require.NoError(t, err) + assert.Nil(t, result, "empty input must return nil so container_internal.go skips rlimit replacement") + }) + + t.Run("valid rlimits are parsed correctly", func(t *testing.T) { + input := []WirePOSIXRlimit{ + {Type: "RLIMIT_NOFILE", Soft: 1024, Hard: 2048}, + {Type: "RLIMIT_NPROC", Soft: 512, Hard: 512}, + } + result, err := parseRLimits(input) + require.NoError(t, err) + require.Len(t, result, 2) + assert.Equal(t, specs.POSIXRlimit{Type: "RLIMIT_NOFILE", Soft: 1024, Hard: 2048}, result[0]) + assert.Equal(t, specs.POSIXRlimit{Type: "RLIMIT_NPROC", Soft: 512, Hard: 512}, result[1]) + }) + + t.Run("unlimited sentinel (uint64 max) is passed through correctly", func(t *testing.T) { + // UInt64OrMinusOne is a uint64; -1 in JSON unmarshals to ^uint64(0). + unlimited := UInt64OrMinusOne(^uint64(0)) + input := []WirePOSIXRlimit{ + {Type: "RLIMIT_NOFILE", Soft: unlimited, Hard: unlimited}, + } + result, err := parseRLimits(input) + require.NoError(t, err) + require.Len(t, result, 1) + assert.Equal(t, uint64(unlimited), result[0].Soft, "Soft unlimited should be uint64 max") + assert.Equal(t, uint64(unlimited), result[0].Hard, "Hard unlimited should be uint64 max") + }) + + t.Run("empty type returns error", func(t *testing.T) { + input := []WirePOSIXRlimit{ + {Type: "", Soft: 1024, Hard: 1024}, + } + _, err := parseRLimits(input) + assert.ErrorContains(t, err, "invalid value for POSIXRlimit.type: empty") + }) +} diff --git a/test/apiv2/29-containersUpdate.at b/test/apiv2/29-containersUpdate.at new file mode 100644 index 0000000000..580af1bedc --- /dev/null +++ b/test/apiv2/29-containersUpdate.at @@ -0,0 +1,34 @@ +# -*- sh -*- +# +# Tests for the container update endpoint +# + +podman pull $IMAGE &>/dev/null +podman rm -a -f &>/dev/null + +# test that r_limits are preserved when omitted from update body (RHEL-178653) +# The bug: parseRLimits() returned empty non-nil slice when r_limits was absent, +# which triggered full rlimit replacement in container_internal.go, wiping existing rlimits. +t POST libpod/containers/create \ + Image=$IMAGE \ + r_limits='[{"type":"nofile","soft":52191,"hard":127585}]' \ + 201 \ + .Id~[0-9a-f]\\{64\\} +cid=$(jq -r '.Id' <<<"$output") + +# Verify ulimit is set at create time +t GET libpod/containers/$cid/json 200 \ + .HostConfig.Ulimits[0].Soft=52191 + +# Update an unrelated field (health log destination) without mentioning r_limits +t POST libpod/containers/$cid/update \ + health_log_destination=local \ + 201 + +# Ulimits must survive the update +t GET libpod/containers/$cid/json 200 \ + .HostConfig.Ulimits[0].Soft=52191 + +t DELETE containers/$cid 204 + +podman container rm -fa