Fix #2 for compat commit handling of --changes

[rhatdan]

Signed-off-by: Daniel J Walsh dwalsh@redhat.com

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rhatdan

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [rhatdan]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[rhatdan]

@leahneukirchen PTAL
This works with Docker, but my test is failing. Ideas?
@jwhonce @edsantiago Ideas?

[rhatdan]

Tested with
DOCKER_HOST=unix://$XDG_RUNTIME_DIR/podman/podman.sock docker commit --change 'EXPOSE 9999' -c 'env dan=walsh' CONTAINERID

[leahneukirchen]

How are []string interpreted? Is it not just multiple occurrences of parameters?

I think you explicitly need to split on newline.

(Also typo in commit subject).

[edsantiago]

@rhatdan this is going to sound really stupid, but here's how I do most of my testing with the apiv2 tests:

-t GET images/$iid/json 200 \
+t GET images/$iid/json 200 sdf \

The sdf (or foo or whatever) forces a one-to-one string comparison, which will always of course fail, which will then give me a nice error dump:

not ok 244 [20-containers] GET images/94afe6a97e213cee44fabfc17a926ebe41f4889bfed0ca11e42f7944386b3526/json : output
#  expected: sdf
#    actual: {"Id":"sha256:94afe6a97e213cee44fabfc17a926ebe41f4889bfed0ca11e42f7944386b3526","RepoTags":["docker.io/library/newrepo:v3"],"RepoDigests":["docker.io/library/newrepo@sha256:1d55359f91c488bf9cea7b314923131418e9244ec5301e96471a73696afedd18"],"Parent":"","Comment":"abcd","Created":"2022-01-20T19:06:29.232929799Z","Container":"","ContainerConfig":{"Hostname":"94afe6a97e2","Domainname":"","User":"","AttachStdin":false,"AttachStdout":false,"AttachStderr":false,"Tty":false,"OpenStdin":false,"StdinOnce":false,"Env":null,"Cmd":null,"Image":"","Volumes":null,"WorkingDir":"","Entrypoint":null,"OnBuild":null,"Labels":null},"DockerVersion":"","Author":"eric","Config":{"Hostname":"","Domainname":"","User":"","AttachStdin":false,"AttachStdout":false,"AttachStderr":false,"Tty":false,"OpenStdin":false,"StdinOnce":false,"Env":["PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin","TERM=xterm","container=podman"],"Cmd":["/bin/sh","-c","/bin/bar
EXPOSE 9090"],"Image":"","Volumes":null,"WorkingDir":"/","Entrypoint":["echo"],"OnBuild":null,"Labels":{"PODMAN":"/usr/bin/podman run -it --name NAME -e NAME=NAME -e IMAGE=IMAGE IMAGE echo podman","install":"docker run -it --name NAME -e NAME=NAME -e IMAGE=IMAGE IMAGE echo install","options":"/usr/bin/podman run -it --name NAME -e NAME=NAME -e IMAGE=IMAGE IMAGE echo $OPT1 ","run":"docker run -it --name NAME -e NAME=NAME -e IMAGE=IMAGE IMAGE echo RUN","uninstall":"/usr/bin/docker run -it --name NAME -e NAME=NAME -e IMAGE=IMAGE IMAGE echo uninstall","useradds":"/usr/bin/docker run -it IMAGE echo"}},"Architecture":"amd64","Os":"linux","Size":4678207,"VirtualSize":4678207,"GraphDriver":{"Data":{"LowerDir":"/dev/shm/test-apiv2.tmp.4xOVYr/server_root/overlay/df64d3292fd6194b7865d7326af5255db6d81e9df29f48adde61a918fbd8c332/diff","UpperDir":"/dev/shm/test-apiv2.tmp.4xOVYr/server_root/overlay/50e2d905bd98426b4b0bd17ce051ad8861d3d557b1c5384b7ec6fdfd1fd702d3/diff","WorkDir":"/dev/shm/test-apiv2.tmp.4xOVYr/server_root/overlay/50e2d905bd98426b4b0bd17ce051ad8861d3d557b1c5384b7ec6fdfd1fd702d3/work"},"Name":"overlay"},"RootFS":{"Type":"layers","Layers":["sha256:df64d3292fd6194b7865d7326af5255db6d81e9df29f48adde61a918fbd8c332","sha256:5f70bf18a086007016e948b04aed3b82103a36bea41755b6cddfaf10ace3c6ef"]},"Metadata":{"LastTagTime":"0001-01-01T00:00:00Z"}}

And if you look really closely (or, even better, use tmux's in-page search) you will see:

..."Cmd":["/bin/sh","-c","/bin/bar
EXPOSE 9090"],"Image":"",

UPDATE: I agree with @leahneukirchen , it looks like you're assuming that someone/something will do the newline-splitting for you into multiple []string elements, but what it looks like to me is an array with one element, a string, and that string includes a newline.

[rhatdan]

My question to both @edsantiago and @leahneukirchen is when I run the docker client against what I have they come pre-split so perhaps I need to split individuals and/or docker has expanded the way it handles this.

[rhatdan]

Ok now I split and handle multiple changes which I get from Docker.

[edsantiago]

when I run the docker client against what I have they come pre-split

Just thinking out loud, and probably not very usefully because it's been a hell of a long day and week, but... could it be one of those CR vs NL vs CRNL things? maybe some combination of the docker client sending it one way, and the server interpreting those as "oh I need to split those suckahs"?

[edsantiago]

PS @rhatdan lint is griping

pkg/api/handlers/compat/images.go:142: unnecessary leading newline (whitespace)

[leahneukirchen]

My question to both @edsantiago and @leahneukirchen is when I run the docker client against what I have they come pre-split so perhaps I need to split individuals and/or docker has expanded the way it handles this.

The docker client will send one changes= parameter per --change. The API allows any number of Dockerfile statements per changes=. Try --changes $'WORKDIR /\nEXPOSE 9999' it works too.

There is code out there that uses the API with multiple newline-separated statements, so it should be supported too.

The patch LGTM now. Thanks!

[ashley-cui]

/lgtm