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

restrict filters server side in docker images #10128

Merged
merged 2 commits into from
Jan 19, 2015
Merged

Conversation

vieux
Copy link
Contributor

@vieux vieux commented Jan 16, 2015

Unlike docker ps, docker images filters are restricted.

Filters are a great way to extend the filtering of the cli.
This change would be extremely useful in Swarm.

Signed-off-by: Victor Vieux <vieux@docker.com>
@vieux
Copy link
Contributor Author

vieux commented Jan 16, 2015

See docker-archive/classicswarm#249 for an example

@SvenDowideit
Copy link
Contributor

you just changed some restrictions to constraints on the cli - where is the documentation telling users of this? :)

@thaJeztah
Copy link
Member

I understand the use case, but want to point to a reason that accepting any filter could be troublesome #8777.

For that reason; perhaps this should be given another thought #928 Implement a 'clean' command so that uses can have a "safer" way to cleanup images and don't need to use the filters for that?

@thaJeztah
Copy link
Member

Reading docker-archive/classicswarm#249; wouldn't that be something to use "meta-data" for? #9882, i.e. docker images -f label=node=fedora-1?

@vieux
Copy link
Contributor Author

vieux commented Jan 16, 2015

@SvenDowideit @thaJeztha I'll add the check back server side, so there will be no change for the user

@vieux
Copy link
Contributor Author

vieux commented Jan 16, 2015

Filters are already sent to the daemon, so it doesn't require any doc change

@thaJeztah
Copy link
Member

I'll add the check back server side, so there will be no change for the user

👍 thanks

Signed-off-by: Victor Vieux <vieux@docker.com>
@vieux
Copy link
Contributor Author

vieux commented Jan 16, 2015

@thaJeztah @SvenDowideit updated, I added back the test

@crosbymichael @jfrazelle can you please take a look ?

@jessfraz
Copy link
Contributor

LGTM

@thaJeztah
Copy link
Member

I like this; having this check server-side makes more sense, so a nice improvement overall as well.

@jessfraz
Copy link
Contributor

@thaJeztah I agree :D

@crosbymichael
Copy link
Contributor

@vieux why is this more useful as a server side check? Are all filters done on the client or server?

@vieux
Copy link
Contributor Author

vieux commented Jan 19, 2015

@crosbymichael filters are done server side for docker images

@vieux vieux changed the title don't restrict filters in docker images restrict filters server side in docker images Jan 19, 2015
@icecrime icecrime added this to the 1.5.0 milestone Jan 19, 2015
@icecrime
Copy link
Contributor

LGTM: server-side should be the source of truth for what's allowed.

icecrime pushed a commit that referenced this pull request Jan 19, 2015
Server-side restriction of allowed image filters
@icecrime icecrime merged commit cb9db04 into moby:master Jan 19, 2015
@vieux vieux deleted the filters_image branch January 19, 2015 21:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants