Comment 2 for bug 1400366

Revision history for this message
Mike Fedosin (mfedosin) wrote :

Oh, sure :) I mean the following: in api there is a set of keys that we can sort the index output by ('name', 'status', 'container_format', 'disk_format', 'size', 'id', 'created_at', 'updated_at'), other keys are not supported.
In v1 there is a check on both client and server sides (https://github.com/openstack/python-glanceclient/blob/master/glanceclient/v1/images.py#L224 and https://github.com/openstack/glance/blob/master/glance/registry/api/v1/images.py#L287), but in v2 there are no checks at all. In glance client it's okay, because currently it doesn't support sort_key and sort_dir option (but there are my commits in gerrit which implement it). But glance server now can accept any sort key like 'blah' or 'foo', etc.
Then it calls 'list' in domain model with sort_key='foo', it successfully gone through this, reaches the db layer in sqlalchemy/api.py and in pagination method it performs the check that sort key is one of the db model fields (https://github.com/openstack/glance/blob/master/glance/db/sqlalchemy/api.py#L345). If sort key is 'foo' then it raises an exception, but if sort key is 'checksum' it goes further and returns full image list sorted by checksum.

So I see two problems here:
1. I think sort key validation should be performed on the api layer near the sort dir, not on the db - current implementation looks ugly.
2. Now it supports sorting by all model fields and it is violates api.

Just look at my commit https://review.openstack.org/#/c/139996/