From 0df3a0047af4d8d4fe7c0270be5859d39cf0fb11 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Mon, 10 Feb 2025 19:34:25 +0100 Subject: [PATCH 1/2] api/types/container: introduce CommitResponse type Move api/types.IDResponse to a "common" package (to prevent cyclic import issues), and introduce a container.CommitResponse type as alias. This allows consumers to use ContainerCommit without having to import the "types" package, and allows us to differentiate the response for container commit separate from other endpoints currently using IDResponse. Signed-off-by: Sebastiaan van Stijn --- api/server/router/container/container_routes.go | 2 +- api/swagger.yaml | 11 ++++++----- api/types/{ => common}/id_response.go | 4 ++-- api/types/container/commit.go | 7 +++++++ api/types/types_deprecated.go | 4 ++++ client/client_interfaces.go | 2 +- client/container_commit.go | 11 +++++------ client/container_commit_test.go | 3 +-- hack/generate-swagger-api.sh | 5 ++++- 9 files changed, 31 insertions(+), 18 deletions(-) rename api/types/{ => common}/id_response.go (87%) create mode 100644 api/types/container/commit.go diff --git a/api/server/router/container/container_routes.go b/api/server/router/container/container_routes.go index 5b07db5ebc..daa74afec8 100644 --- a/api/server/router/container/container_routes.go +++ b/api/server/router/container/container_routes.go @@ -71,7 +71,7 @@ func (c *containerRouter) postCommit(ctx context.Context, w http.ResponseWriter, return err } - return httputils.WriteJSON(w, http.StatusCreated, &types.IDResponse{ID: imgID}) + return httputils.WriteJSON(w, http.StatusCreated, &container.CommitResponse{ID: imgID}) } func (c *containerRouter) getContainersJSON(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error { diff --git a/api/swagger.yaml b/api/swagger.yaml index 21cdf288cb..322b5a664b 100644 --- a/api/swagger.yaml +++ b/api/swagger.yaml @@ -2908,9 +2908,10 @@ definitions: example: message: "Something went wrong." - IdResponse: + IDResponse: description: "Response to an API call that returns just an Id" type: "object" + x-go-name: "IDResponse" required: ["Id"] properties: Id: @@ -10220,7 +10221,7 @@ paths: 201: description: "no error" schema: - $ref: "#/definitions/IdResponse" + $ref: "#/definitions/IDResponse" 404: description: "no such container" schema: @@ -10614,7 +10615,7 @@ paths: 201: description: "no error" schema: - $ref: "#/definitions/IdResponse" + $ref: "#/definitions/IDResponse" 404: description: "no such container" schema: @@ -13094,7 +13095,7 @@ paths: 201: description: "no error" schema: - $ref: "#/definitions/IdResponse" + $ref: "#/definitions/IDResponse" 409: description: "name conflicts with an existing object" schema: @@ -13301,7 +13302,7 @@ paths: 201: description: "no error" schema: - $ref: "#/definitions/IdResponse" + $ref: "#/definitions/IDResponse" 409: description: "name conflicts with an existing object" schema: diff --git a/api/types/id_response.go b/api/types/common/id_response.go similarity index 87% rename from api/types/id_response.go rename to api/types/common/id_response.go index 7592d2f8b1..22e8c60a48 100644 --- a/api/types/id_response.go +++ b/api/types/common/id_response.go @@ -1,10 +1,10 @@ -package types +package common // This file was generated by the swagger tool. // Editing this file might prove futile when you re-run the swagger generate command // IDResponse Response to an API call that returns just an Id -// swagger:model IdResponse +// swagger:model IDResponse type IDResponse struct { // The id of the newly created object. diff --git a/api/types/container/commit.go b/api/types/container/commit.go new file mode 100644 index 0000000000..6fd1b0ead1 --- /dev/null +++ b/api/types/container/commit.go @@ -0,0 +1,7 @@ +package container + +import "github.com/docker/docker/api/types/common" + +// CommitResponse response for the commit API call, containing the ID of the +// image that was produced. +type CommitResponse = common.IDResponse diff --git a/api/types/types_deprecated.go b/api/types/types_deprecated.go index 170a65b8b9..76c04785cc 100644 --- a/api/types/types_deprecated.go +++ b/api/types/types_deprecated.go @@ -3,11 +3,15 @@ package types import ( "context" + "github.com/docker/docker/api/types/common" "github.com/docker/docker/api/types/container" "github.com/docker/docker/api/types/image" "github.com/docker/docker/api/types/storage" ) +// IDResponse Response to an API call that returns just an Id +type IDResponse = common.IDResponse + // ContainerJSONBase contains response of Engine API GET "/containers/{name:.*}/json" // for API version 1.18 and older. // diff --git a/client/client_interfaces.go b/client/client_interfaces.go index 804383739d..c06e113ba7 100644 --- a/client/client_interfaces.go +++ b/client/client_interfaces.go @@ -69,7 +69,7 @@ type HijackDialer interface { // ContainerAPIClient defines API client methods for the containers type ContainerAPIClient interface { ContainerAttach(ctx context.Context, container string, options container.AttachOptions) (types.HijackedResponse, error) - ContainerCommit(ctx context.Context, container string, options container.CommitOptions) (types.IDResponse, error) + ContainerCommit(ctx context.Context, container string, options container.CommitOptions) (container.CommitResponse, error) ContainerCreate(ctx context.Context, config *container.Config, hostConfig *container.HostConfig, networkingConfig *network.NetworkingConfig, platform *ocispec.Platform, containerName string) (container.CreateResponse, error) ContainerDiff(ctx context.Context, container string) ([]container.FilesystemChange, error) ContainerExecAttach(ctx context.Context, execID string, options container.ExecAttachOptions) (types.HijackedResponse, error) diff --git a/client/container_commit.go b/client/container_commit.go index 718465ce06..b78b6b33ec 100644 --- a/client/container_commit.go +++ b/client/container_commit.go @@ -7,26 +7,25 @@ import ( "net/url" "github.com/distribution/reference" - "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/container" ) // ContainerCommit applies changes to a container and creates a new tagged image. -func (cli *Client) ContainerCommit(ctx context.Context, containerID string, options container.CommitOptions) (types.IDResponse, error) { +func (cli *Client) ContainerCommit(ctx context.Context, containerID string, options container.CommitOptions) (container.CommitResponse, error) { containerID, err := trimID("container", containerID) if err != nil { - return types.IDResponse{}, err + return container.CommitResponse{}, err } var repository, tag string if options.Reference != "" { ref, err := reference.ParseNormalizedNamed(options.Reference) if err != nil { - return types.IDResponse{}, err + return container.CommitResponse{}, err } if _, isCanonical := ref.(reference.Canonical); isCanonical { - return types.IDResponse{}, errors.New("refusing to create a tag with a digest reference") + return container.CommitResponse{}, errors.New("refusing to create a tag with a digest reference") } ref = reference.TagNameOnly(ref) @@ -49,7 +48,7 @@ func (cli *Client) ContainerCommit(ctx context.Context, containerID string, opti query.Set("pause", "0") } - var response types.IDResponse + var response container.CommitResponse resp, err := cli.post(ctx, "/commit", query, options.Config, nil) defer ensureReaderClosed(resp) if err != nil { diff --git a/client/container_commit_test.go b/client/container_commit_test.go index c6cf94a74d..60027a6fd2 100644 --- a/client/container_commit_test.go +++ b/client/container_commit_test.go @@ -10,7 +10,6 @@ import ( "strings" "testing" - "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/container" "github.com/docker/docker/errdefs" "gotest.tools/v3/assert" @@ -77,7 +76,7 @@ func TestContainerCommit(t *testing.T) { if len(changes) != len(expectedChanges) { return nil, fmt.Errorf("expected container changes size to be '%d', got %d", len(expectedChanges), len(changes)) } - b, err := json.Marshal(types.IDResponse{ + b, err := json.Marshal(container.CommitResponse{ ID: "new_container_id", }) if err != nil { diff --git a/hack/generate-swagger-api.sh b/hack/generate-swagger-api.sh index 06f9bb3b29..3bf30e56c5 100755 --- a/hack/generate-swagger-api.sh +++ b/hack/generate-swagger-api.sh @@ -4,13 +4,16 @@ set -eu swagger generate model -f api/swagger.yaml \ -t api -m types --skip-validator -C api/swagger-gen.yaml \ -n ErrorResponse \ - -n IdResponse \ -n Plugin \ -n PluginDevice \ -n PluginMount \ -n PluginEnv \ -n PluginInterfaceType +swagger generate model -f api/swagger.yaml \ + -t api -m types/common --skip-validator -C api/swagger-gen.yaml \ + -n IDResponse + swagger generate model -f api/swagger.yaml \ -t api -m types/storage --skip-validator -C api/swagger-gen.yaml \ -n DriverData From 9a20edf7b60c4068c4be1c6575d94a699861d457 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Mon, 10 Feb 2025 19:44:33 +0100 Subject: [PATCH 2/2] api/types/container: introduce ExecCreateResponse type Introduce a container.ExecCreateResponse type as alias for IDResponse to allow consumers to use ContainerCommit without having to import the "types" package, and allows us to differentiate the response for container commit separate from other endpoints currently using IDResponse. Signed-off-by: Sebastiaan van Stijn --- api/server/router/container/exec.go | 2 +- api/types/container/exec.go | 8 ++++++++ client/client_interfaces.go | 2 +- client/container_exec.go | 12 ++++++------ client/container_exec_test.go | 3 +-- 5 files changed, 17 insertions(+), 10 deletions(-) diff --git a/api/server/router/container/exec.go b/api/server/router/container/exec.go index fd2e403106..39a2773bbf 100644 --- a/api/server/router/container/exec.go +++ b/api/server/router/container/exec.go @@ -61,7 +61,7 @@ func (c *containerRouter) postContainerExecCreate(ctx context.Context, w http.Re return err } - return httputils.WriteJSON(w, http.StatusCreated, &types.IDResponse{ + return httputils.WriteJSON(w, http.StatusCreated, &container.ExecCreateResponse{ ID: id, }) } diff --git a/api/types/container/exec.go b/api/types/container/exec.go index 96093eb5cd..f4b22376ef 100644 --- a/api/types/container/exec.go +++ b/api/types/container/exec.go @@ -1,5 +1,13 @@ package container +import "github.com/docker/docker/api/types/common" + +// ExecCreateResponse is the response for a successful exec-create request. +// It holds the ID of the exec that was created. +// +// TODO(thaJeztah): make this a distinct type. +type ExecCreateResponse = common.IDResponse + // ExecOptions is a small subset of the Config struct that holds the configuration // for the exec feature of docker. type ExecOptions struct { diff --git a/client/client_interfaces.go b/client/client_interfaces.go index c06e113ba7..b281cc36a8 100644 --- a/client/client_interfaces.go +++ b/client/client_interfaces.go @@ -73,7 +73,7 @@ type ContainerAPIClient interface { ContainerCreate(ctx context.Context, config *container.Config, hostConfig *container.HostConfig, networkingConfig *network.NetworkingConfig, platform *ocispec.Platform, containerName string) (container.CreateResponse, error) ContainerDiff(ctx context.Context, container string) ([]container.FilesystemChange, error) ContainerExecAttach(ctx context.Context, execID string, options container.ExecAttachOptions) (types.HijackedResponse, error) - ContainerExecCreate(ctx context.Context, container string, options container.ExecOptions) (types.IDResponse, error) + ContainerExecCreate(ctx context.Context, container string, options container.ExecOptions) (container.ExecCreateResponse, error) ContainerExecInspect(ctx context.Context, execID string) (container.ExecInspect, error) ContainerExecResize(ctx context.Context, execID string, options container.ResizeOptions) error ContainerExecStart(ctx context.Context, execID string, options container.ExecStartOptions) error diff --git a/client/container_exec.go b/client/container_exec.go index 4891b98e6e..393fd794f8 100644 --- a/client/container_exec.go +++ b/client/container_exec.go @@ -11,10 +11,10 @@ import ( ) // ContainerExecCreate creates a new exec configuration to run an exec process. -func (cli *Client) ContainerExecCreate(ctx context.Context, containerID string, options container.ExecOptions) (types.IDResponse, error) { +func (cli *Client) ContainerExecCreate(ctx context.Context, containerID string, options container.ExecOptions) (container.ExecCreateResponse, error) { containerID, err := trimID("container", containerID) if err != nil { - return types.IDResponse{}, err + return container.ExecCreateResponse{}, err } // Make sure we negotiated (if the client is configured to do so), @@ -23,11 +23,11 @@ func (cli *Client) ContainerExecCreate(ctx context.Context, containerID string, // Normally, version-negotiation (if enabled) would not happen until // the API request is made. if err := cli.checkVersion(ctx); err != nil { - return types.IDResponse{}, err + return container.ExecCreateResponse{}, err } if err := cli.NewVersionError(ctx, "1.25", "env"); len(options.Env) != 0 && err != nil { - return types.IDResponse{}, err + return container.ExecCreateResponse{}, err } if versions.LessThan(cli.ClientVersion(), "1.42") { options.ConsoleSize = nil @@ -36,10 +36,10 @@ func (cli *Client) ContainerExecCreate(ctx context.Context, containerID string, resp, err := cli.post(ctx, "/containers/"+containerID+"/exec", nil, options, nil) defer ensureReaderClosed(resp) if err != nil { - return types.IDResponse{}, err + return container.ExecCreateResponse{}, err } - var response types.IDResponse + var response container.ExecCreateResponse err = json.NewDecoder(resp.body).Decode(&response) return response, err } diff --git a/client/container_exec_test.go b/client/container_exec_test.go index f291bccf97..045cf92702 100644 --- a/client/container_exec_test.go +++ b/client/container_exec_test.go @@ -10,7 +10,6 @@ import ( "strings" "testing" - "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/container" "github.com/docker/docker/errdefs" "gotest.tools/v3/assert" @@ -67,7 +66,7 @@ func TestContainerExecCreate(t *testing.T) { if execConfig.User != "user" { return nil, fmt.Errorf("expected an execConfig with User == 'user', got %v", execConfig) } - b, err := json.Marshal(types.IDResponse{ + b, err := json.Marshal(container.ExecCreateResponse{ ID: "exec_id", }) if err != nil {