Deprecate api/types/strslice.StrSlice and remove its use by thaJeztah · Pull Request #50292 · moby/moby

thaJeztah · 2025-07-01T08:46:10Z

relates to;

The strslice.StrSlice type is a string-slice with a custom JSON Unmarshal function to provide backward-compatibility with older API requests from before docker 1.7 (see moby@17d6f00 and moby@ea4a067), which used a string instead of an array of strings for some fields (Cmd, Entrypoint).

We no longer support those API versions, and we no longer support pulling v1 images that may contain such a config, so we can deprecate the type and remove its use.

- What I did

- How I did it

- How to verify it

- Human readable description for the release notes

api/types/strslice: deprecate StrSlice in favor of using a regular `[]string`.

- A picture of a cute animal (not mandatory but encouraged)

thaJeztah · 2025-07-01T08:48:18Z

api/types/strslice/strslice.go

-}
+//
+// Deprecated: this type was used for compatibility with deprecated API versions. Use []string instead.
+type StrSlice = []string


Changed this to an alias for []string so that any uses of it would just become a no-op.

thaJeztah · 2025-07-01T10:02:07Z

And some tests that use invalid requests (current API versions only document / support string-slice)

=== FAIL: amd64.integration-cli TestDockerAPISuite/TestContainerAPIPostCreateNull (0.01s)
    docker_api_containers_test.go:778: assertion failed: 400 (res.StatusCode int) != 201 (http.StatusCreated int)
    --- FAIL: TestDockerAPISuite/TestContainerAPIPostCreateNull (0.01s)

=== FAIL: amd64.integration-cli TestDockerAPISuite/TestCreateWithTooLowMemoryLimit (0.00s)
    docker_api_containers_test.go:815: assertion failed: string "{\"message\":\"invalid JSON: json: cannot unmarshal string into Go struct field CreateRequest.Config.Cmd of type []string\"}\n" does not contain "Minimum memory limit allowed is 6MB"
    --- FAIL: TestDockerAPISuite/TestCreateWithTooLowMemoryLimit (0.00s)

=== FAIL: amd64.integration-cli TestDockerAPISuite/TestPostContainerAPICreateWithStringOrSliceEntrypoint (0.28s)
    docker_api_containers_test.go:1106: assertion failed: 
        Command:  /usr/local/cli-integration/docker start -a echotest2
        ExitCode: 1
        Error:    exit status 1
        Stdout:   
        Stderr:   Error: No such container: echotest2
        
        
        Failures:
        ExitCode was 1 expected 0
        Expected no error
    --- FAIL: TestDockerAPISuite/TestPostContainerAPICreateWithStringOrSliceEntrypoint (0.28s)

=== FAIL: amd64.integration-cli TestDockerAPISuite/TestPostContainersCreateWithStringOrSliceCmd (0.26s)
    docker_api_containers_test.go:1133: assertion failed: 
        Command:  /usr/local/cli-integration/docker start -a echotest2
        ExitCode: 1
        Error:    exit status 1
        Stdout:   
        Stderr:   Error: No such container: echotest2
        
        
        Failures:
        ExitCode was 1 expected 0
        Expected no error
    --- FAIL: TestDockerAPISuite/TestPostContainersCreateWithStringOrSliceCmd (0.26s)

thaJeztah · 2025-07-01T10:03:14Z

api/types/container/hostconfig.go

-	CapAdd          strslice.StrSlice // List of kernel capabilities to add to the container
-	CapDrop         strslice.StrSlice // List of kernel capabilities to remove from the container
+	CapAdd          []string          // List of kernel capabilities to add to the container
+	CapDrop         []string          // List of kernel capabilities to remove from the container


Even if we do want to continue supporting the old format; these two definitely look wrong, as these fields never supported a string (instead of a slice) AFAIK.

Looks like I was wrong there; Hmm, so CapAdd/CapDrop;

switched to using StrSlice in 17999c7

had some fixes related to parsing "string" instead of "slice"; 10a3061

looks indeed like they did accept a string instead of a slice before docker 1.7; /containers/create no longer accepts a string for CapAdd or CapDrop #14318

Which would be API v1.19 and before;

moby/api/common.go

Line 20 in 10a3061

Version version.Version = "1.20"

integration-cli/docker_api_containers_test.go

austinvazquez · 2025-08-06T20:38:49Z

Rebased to bring in some of the changes that have been merged to master.

thaJeztah · 2025-08-06T22:46:57Z

Definitely some weird things happening; checksum failiures in various places. Not sure if these are issues with the fixture. Also worth noting (just in case) that docker/dockerfile:v1.7.1 was put to "latest" earlier Today; which COULD be relevant if there's something with building the dev-container?

=== FAIL: amd64.docker.docker.integration.container TestInspectImageManifestPlatform/amd64_image_on_non-amd64_host (0.06s)
    inspect_test.go:123: assertion failed: error is not nil: Error response from daemon: apply layer error for "": wrong diff id calculated on extraction "sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"

=== FAIL: amd64.docker.docker.integration.daemon TestLiveRestore/volume_references/image_mount (0.08s)
    daemon_test.go:679: assertion failed: error is not nil: Error response from daemon: apply layer error for "": wrong diff id calculated on extraction "sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"
        --- FAIL: TestLiveRestore/volume_references/image_mount (0.08s)

=== FAIL: amd64.docker.docker.integration.volume TestRunMountImageMultipleTimes (0.14s)
    mount_test.go:370: assertion failed: error is not nil: Error response from daemon: apply layer error for "": wrong diff id calculated on extraction "sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"

thaJeztah · 2025-08-06T22:51:43Z

Error looks to come from containerd vendor; not sure why the i.Name() is empty, but maybe that's expected

moby/vendor/github.com/containerd/containerd/v2/client/image.go

Lines 335 to 338 in 0aa8fe0

    
           for _, layer := range layers { 
        
           	unpacked, err = rootfs.ApplyLayerWithOpts(ctx, layer, chain, sn, a, config.SnapshotOpts, config.ApplyOpts) 
        
           	if err != nil { 
        
           		return fmt.Errorf("apply layer error for %q: %w", i.Name(), err)

The strslice.StrSlice type is a string-slice with a custom JSON Unmarshal function to provide backward-compatibility with older API requests from before docker 1.7 (see [moby/moby@17d6f00] and [moby/moby@ea4a067]), which used a string instead of an array of strings for some fields (Cmd, Entrypoint). We no longer support those API versions, and we no longer support pulling v1 images that may contain such a config, so we can deprecate the type and remove its use. [moby/moby@17d6f00]: moby@17d6f00 [moby/moby@ea4a067]: moby@ea4a067 Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

thaJeztah · 2025-08-08T12:04:16Z

I think this should be ready to go, but doing a quick check with the BuildKit folks. The "short" story is that;

Our API has not supported "string" for these fields since... forever, so it's good to remove that type from our API
Likewise, the API server SHOULD not have to be able to handle those (as the API spec doesn't define it to be valid)
☝️ if REALLY needed, we can add handling for those in the API server, i.e., add an ad-hoc struct to unmarshal or otherwise.
❓ the only concern (hence "ask BuildKit folks") is container / image configs. Would there be images that somehow contain a (otherwise "invalid") config having a string for these fields?
❓ does BuildKit still contain its own special handling for those, and if it encounters, does it preserve the invalid fields, or does it convert the invalid to become valid ([]string)?

This type is one of those poorly documented cases where we deprecated something, but (without proper versioning the API handling) just left the old in as well. Which makes sense for "post v1.0" releases, but in this case it was to bridge compatibility between early v0.x versions.

thaJeztah · 2025-08-08T12:43:10Z

Chatted with @crazy-max - and looks like BuildKit has no special handling for this; it uses the OCI types for these fields, which don't support the string format, so (famous last words) I think we should be fine.

As mentioned; all handling was on unmarshalling (daemon side), so it's good to not have that implementation detail leak into the API module.

I'm bringing this one in.

We can probably remove the "deprecated" alias (as it's a new module), but I'm sure we have more cleanup rounds to make in the client and api modules.

thaJeztah added this to the 29.0.0 milestone Jul 1, 2025

thaJeztah added area/api API status/2-code-review impact/deprecation area/go-sdk impact/go-sdk Noteworthy (compatibility changes) in the Go SDK labels Jul 1, 2025

thaJeztah commented Jul 1, 2025

View reviewed changes

thaJeztah marked this pull request as draft July 1, 2025 14:54

thaJeztah force-pushed the deprecate_strslice branch from 96a5500 to 8f58969 Compare July 3, 2025 13:06

thaJeztah force-pushed the deprecate_strslice branch 4 times, most recently from f521274 to 3cc0361 Compare July 10, 2025 15:04

thaJeztah commented Jul 10, 2025

View reviewed changes

integration-cli/docker_api_containers_test.go Show resolved Hide resolved

thaJeztah added this to 🔦 Maintainer spotlight Jul 10, 2025

github-project-automation bot moved this to New in 🔦 Maintainer spotlight Jul 10, 2025

thompson-shaun moved this from New to Complete in 🔦 Maintainer spotlight Jul 10, 2025

thaJeztah marked this pull request as ready for review July 10, 2025 18:30

thaJeztah requested a review from tonistiigi as a code owner July 10, 2025 18:30

thaJeztah force-pushed the deprecate_strslice branch 4 times, most recently from b65ffc0 to f04c4cc Compare July 23, 2025 13:36

thaJeztah mentioned this pull request Jul 23, 2025

remove, update tests for deprecated API versions #50487

Merged

thaJeztah force-pushed the deprecate_strslice branch from f04c4cc to d4fb340 Compare July 25, 2025 11:53

thaJeztah mentioned this pull request Jul 25, 2025

[28.x backport] remove, update obsolete tests #50514

Merged

thaJeztah force-pushed the deprecate_strslice branch 2 times, most recently from 0eb1cf2 to 0f813a4 Compare July 28, 2025 20:29

thaJeztah force-pushed the deprecate_strslice branch from 0f813a4 to 377f7b2 Compare August 6, 2025 10:59

thompson-shaun added the release-blocker PRs we want to block a release on label Aug 6, 2025

austinvazquez force-pushed the deprecate_strslice branch from 377f7b2 to a1ecf68 Compare August 6, 2025 20:35

austinvazquez mentioned this pull request Aug 6, 2025

Dockerfile: update containerd binary to v1.7.28 #50645

Merged

thaJeztah mentioned this pull request Aug 7, 2025

CI is broken on loading / using frozen images on x86 #50655

Closed

austinvazquez force-pushed the deprecate_strslice branch from a1ecf68 to 9879083 Compare August 7, 2025 22:16

austinvazquez approved these changes Aug 8, 2025

View reviewed changes

dmcgowan approved these changes Aug 8, 2025

View reviewed changes

thaJeztah merged commit 846cf4b into moby:master Aug 8, 2025
175 checks passed

thaJeztah deleted the deprecate_strslice branch August 8, 2025 12:43

This was referenced Sep 5, 2025

Prepare release notes for api/v1.52.0-beta.0 #50904

Merged

Prepare release notes for client/v0.1.0-beta.0 #50913

Merged

Prepare release notes for v2.0.0-beta.0 #50918

Merged

dmcgowan mentioned this pull request Sep 24, 2025

Prepare release notes for api/v1.52.0-rc.0 #51030

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Deprecate api/types/strslice.StrSlice and remove its use#50292

Deprecate api/types/strslice.StrSlice and remove its use#50292
thaJeztah merged 1 commit intomoby:masterfrom
thaJeztah:deprecate_strslice

thaJeztah commented Jul 1, 2025

Uh oh!

thaJeztah Jul 1, 2025

Uh oh!

thaJeztah commented Jul 1, 2025

Uh oh!

thaJeztah Jul 1, 2025

Uh oh!

thaJeztah Jul 1, 2025

Uh oh!

Uh oh!

austinvazquez commented Aug 6, 2025

Uh oh!

thaJeztah commented Aug 6, 2025

Uh oh!

thaJeztah commented Aug 6, 2025

Uh oh!

thaJeztah commented Aug 8, 2025

Uh oh!

thaJeztah commented Aug 8, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

thaJeztah commented Jul 1, 2025

Uh oh!

thaJeztah Jul 1, 2025

Choose a reason for hiding this comment

Uh oh!

thaJeztah commented Jul 1, 2025

Uh oh!

thaJeztah Jul 1, 2025

Choose a reason for hiding this comment

Uh oh!

thaJeztah Jul 1, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

austinvazquez commented Aug 6, 2025

Uh oh!

thaJeztah commented Aug 6, 2025

Uh oh!

thaJeztah commented Aug 6, 2025

Uh oh!

thaJeztah commented Aug 8, 2025

Uh oh!

thaJeztah commented Aug 8, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants