Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix volumes and networks list/prune filters in http api #9758

Merged

Conversation

jmguzik
Copy link
Contributor

@jmguzik jmguzik commented Mar 18, 2021

This is the continuation of work started in #9711. It turns out
that list/prune commands for volumes in libpod/compat api have
very dangerous error handling when broken filter input is supplied.
Problem also affects network list/prune in libpod. This commit
unifies filter handling across libpod/compat api and adds sanity
apiv2 testcases.

Signed-off-by: Jakub Guzik [email protected]

/kind bug
/kind cleanup
Below applies to libpod HTTP network list/prune, volume prune and volume list (both libpod and compat http)

  1. Docker: scenario => when the user makes mistake when using filters, then no negative consequences
$ docker volume create v1
v1
$ docker volume create v2
v2
$ curl -gG -XPOST --unix-socket /var/run/docker.sock  http://localhost/v1.41/volumes/prune?filters=dssd21 | jq
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100    67  100    67    0     0  67000      0 --:--:-- --:--:-- --:--:-- 67000
{
  "message": "invalid character 'd' looking for beginning of value"
}


  1. Podman (compat API): scenario => mistake, then there are negative consequences
$ bin/podman volume create v1
v1
$ bin/podman volume create v2
v2
$ curl -gG -XPOST --unix-socket /run/user/1000/podman/podman.sock  http://d/v3.0.0/volumes/prune?filters=dssd21 | jq
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100    50  100    50    0     0   2173      0 --:--:-- --:--:-- --:--:--  2173
{
  "VolumesDeleted": [
    "v1",
    "v2"
  ],
  "SpaceReclaimed": 0
}
$ bin/podman volume create v1bis
v1bis
$ bin/podman volume create v2bis
v2bis
$ curl -gG -XPOST --unix-socket /run/user/1000/podman/podman.sock  http://d/v3.0.0/volumes/prune?filters={garba1ge | jq
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100    56  100    56    0     0   2666      0 --:--:-- --:--:-- --:--:--  2666
{
  "VolumesDeleted": [
    "v1bis",
    "v2bis"
  ],
  "SpaceReclaimed": 0
}

  1. After fix:
$ curl -gG -XPOST --unix-socket /run/user/1000/podman/podman.sock  http://d/v3.0.0/volumes/prune?filters=dssd21 | jq
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100   155  100   155    0     0   151k      0 --:--:-- --:--:-- --:--:--  151k
{
  "cause": "invalid character 'd' looking for beginning of value",
  "message": "Decode(): invalid character 'd' looking for beginning of value",
  "response": 500
}

This is the continuation work started in containers#9711. It turns out
that list/prune commands for volumes in libpod/compat api have
very dangerous error handling when broken filter input is supplied.
Problem also affects network list/prune in libpod. This commit
unifies filter handling across libpod/compat api and adds sanity
apiv2 testcases.

Signed-off-by: Jakub Guzik <[email protected]>
@openshift-ci-robot openshift-ci-robot added kind/bug Categorizes issue or PR as related to a bug. kind/cleanup Categorizes issue or PR as related to cleanup. labels Mar 18, 2021
Copy link
Member

@Luap99 Luap99 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 19, 2021
@rhatdan
Copy link
Member

rhatdan commented Mar 19, 2021

Thanks @jmguzik
/lgtm
/approve

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Mar 19, 2021
@openshift-ci-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jmguzik, Luap99, 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:

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

@openshift-merge-robot openshift-merge-robot merged commit 61e3b15 into containers:master Mar 19, 2021
jmguzik added a commit to jmguzik/podman that referenced this pull request Mar 20, 2021
The problem described in containers#9711 and followed by containers#9758 affects
containers as well. When user provides wrong filter input, error
message should occur, not fallback to full list/prune command.
This change fixes the issue. Additionally, there are error message
fixes for docker http api compat.

Signed-off-by: Jakub Guzik <[email protected]>
mheon pushed a commit to mheon/libpod that referenced this pull request Mar 29, 2021
The problem described in containers#9711 and followed by containers#9758 affects
containers as well. When user provides wrong filter input, error
message should occur, not fallback to full list/prune command.
This change fixes the issue. Additionally, there are error message
fixes for docker http api compat.

Signed-off-by: Jakub Guzik <[email protected]>
jmguzik added a commit to jmguzik/podman that referenced this pull request Apr 26, 2021
The problem described in containers#9711 and followed by containers#9758 affects
containers as well. When user provides wrong filter input, error
message should occur, not fallback to full list/prune command.
This change fixes the issue. Additionally, there are error message
fixes for docker http api compat.

Signed-off-by: Jakub Guzik <[email protected]>
@github-actions github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 23, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. kind/bug Categorizes issue or PR as related to a bug. kind/cleanup Categorizes issue or PR as related to cleanup. lgtm Indicates that a PR is ready to be merged. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants