[Fixes #180] Allow for root-level arrays to be querystring-ed in Modem.prototype.buildQuerystring #181
Conversation
Clovel
changed the title
Signed-off-by: Clovis Durand <clovis.durand@loginline.com>
|
I tried running the GitHub Actions on my fork and there seems to be an issue with Node 18 & Node 22 because of sshfs. This seems unrelated to my changes. Node 14 passed. |
|
It seems that all GitHub Actions have passed over on this repo ! that's good news |
|
Hi @apocas, Now that this is merged, when can we expect a release tag to be published to NPM with this fix ? Is a release scheduled soon ? |
|
Published v5.0.6 |
| if ( | ||
| opts[key] && | ||
| typeof opts[key] === 'object' && | ||
| !Array.isArray(opts[key]) |
There was a problem hiding this comment.
@Clovel @apocas This change is quite aggressive, it goes for using query serialization for only t and extrahosts to using it for any Array option.
This doesn't seem to be universally true. At least I can see this breaks the cachefrom on Image.build that expects a JSON serialized array and otherwise it errors with
(HTTP code 400) unexpected - error reading cache-from: invalid character 'a' looking for beginning of value
I confirmed the issue does not happen with docker-modem v5.0.3, prior to this change.
There was a problem hiding this comment.
It would seem that the serialization needs to be more standard indeed. Not all routes behave the same (or at least I haven't found a common denominator).
It does fix the usage of ENV variables and other parameters of the /images/create route though, which was my issue.
Also unit tests pass, so a case seems to be missing for your issue.
There was a problem hiding this comment.
In the Moby code, t and extrahosts are read using Request.Form (see lines 55-56), while cachefrom uses Request.FormValue (see line 143). I don't know enough about Go to understand why the data is interpreted differently, but that could give us a clue.
There was a problem hiding this comment.
Yeah, the docker API is not very consistent with how it accepts array parameters.
cachefromin ImageBuild expects a stringified "JSON array of images"changesin ImageCreate expects an array of strings.
I'll open up an issue and possibly a PR
Docker [ImageBuild](https://docs.docker.com/reference/api/engine/version/v1.48/#tag/Image/operation/ImageBuild) endpoint requires that the `cachefrom` option is a "JSON serialized array". This requirement seems to be an exception to the way arrays are consumed by the API (usually querystring serialized). This change is needed after apocas/docker-modem#181 that makes querystring serialized arrays the default. This commit adds an exception for this option on `Docker.buildImage`. Change-type: patch
Docker [ImageBuild](https://docs.docker.com/reference/api/engine/version/v1.48/#tag/Image/operation/ImageBuild) endpoint requires that the `cachefrom` option is a "JSON serialized array". This requirement seems to be an exception to the way arrays are consumed by the API (usually querystring serialized). This change is needed after apocas/docker-modem#181 that makes querystring serialized arrays the default. This commit adds an exception for this option on `Docker.buildImage`. Change-type: patch Closes: apocas#792
Docker [ImageBuild](https://docs.docker.com/reference/api/engine/version/v1.48/#tag/Image/operation/ImageBuild) endpoint requires that the `cachefrom` option is a "JSON serialized array". This requirement seems to be an exception to the way arrays are consumed by the API (usually querystring serialized). This change is needed after apocas/docker-modem#181 that makes querystring serialized arrays the default. This commit adds an exception for this option on `Docker.buildImage`. Change-type: patch Closes: apocas#792
A change the way options are serialized in docker-modem causes an issue with builds using the `cachefrom` option and library tests to fail. This change can be reverted if apocas/dockerode#793 is merged. Change-type: patch Relates-to: apocas/docker-modem#181 Relates-to: apocas/dockerode#792
A change the way options are serialized in docker-modem causes an issue with builds using the `cachefrom` option and library tests to fail. This change can be reverted if apocas/dockerode#793 is merged. Change-type: patch Relates-to: apocas/docker-modem#181 Relates-to: apocas/dockerode#792
Relates-to: apocas/docker-modem#181 Change-type: patch
This fixes the behaviour of
Modem.prototype.buildQuerystringfor root-level arrays.It fixes these issues :
changesarray parameter for/create/imageisn't serialized correctly #180I have launched the tests on
docker-modemanddockerode(by setting my fork as the source for NPM) and they all pass. Versions are :Windows :
Let me know if anything more needs to be checked.