diff --git a/gnocchi/indexer/alembic/versions/397987e38570_no_more_slash.py b/gnocchi/indexer/alembic/versions/397987e38570_no_more_slash_and_reencode.py similarity index 87% rename from gnocchi/indexer/alembic/versions/397987e38570_no_more_slash.py rename to gnocchi/indexer/alembic/versions/397987e38570_no_more_slash_and_reencode.py index 77d58404..34363257 100644 --- a/gnocchi/indexer/alembic/versions/397987e38570_no_more_slash.py +++ b/gnocchi/indexer/alembic/versions/397987e38570_no_more_slash_and_reencode.py @@ -13,7 +13,7 @@ # under the License. # -"""no-more-slash +"""Remove slashes from original resource IDs, recompute their id with creator Revision ID: 397987e38570 Revises: aba5a217ca9b @@ -49,7 +49,8 @@ resource_table = sa.Table( sqlalchemy_utils.types.uuid.UUIDType(), nullable=False), sa.Column('original_resource_id', sa.String(255)), - sa.Column('type', sa.String(255)) + sa.Column('type', sa.String(255)), + sa.Column('creator', sa.String(255)) ) resourcehistory_table = sa.Table( @@ -100,15 +101,28 @@ def upgrade(): nullable=False), ) - for resource in connection.execute(resource_table.select().where( - resource_table.c.original_resource_id.like('%/%'))): + for resource in connection.execute(resource_table.select()): + + if resource_table.c.original_resource_id is None: + # statsd resource has no original_resource_id and is NULL + continue + + try: + orig_as_uuid = uuid.UUID( + str(resource_table.c.original_resource_id)) + except ValueError: + pass + else: + if orig_as_uuid == resource_table.c.id: + continue + new_original_resource_id = resource.original_resource_id.replace( '/', '_') if six.PY2: new_original_resource_id = new_original_resource_id.encode('utf-8') new_id = sa.literal(uuidtype.process_bind_param( - str(uuid.uuid5(utils.RESOURCE_ID_NAMESPACE, - new_original_resource_id)), + str(utils.ResourceUUID( + new_original_resource_id, resource.creator)), connection.dialect)) # resource table diff --git a/gnocchi/rest/__init__.py b/gnocchi/rest/__init__.py index 41139969..6878b5ec 100644 --- a/gnocchi/rest/__init__.py +++ b/gnocchi/rest/__init__.py @@ -14,6 +14,7 @@ # WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the # License for the specific language governing permissions and limitations # under the License. +import functools import itertools import uuid @@ -848,8 +849,10 @@ class ResourceController(rest.RestController): def __init__(self, resource_type, id): self._resource_type = resource_type + creator = pecan.request.auth_helper.get_current_user( + pecan.request.headers) try: - self.id = utils.ResourceUUID(id) + self.id = utils.ResourceUUID(id, creator) except ValueError: abort(404, indexer.NoSuchResource(id)) self.metric = NamedMetricController(str(self.id), self._resource_type) @@ -929,8 +932,15 @@ def schema_for(resource_type): return ResourceSchema(resource_type.schema) -def ResourceID(value): - return (six.text_type(value), utils.ResourceUUID(value)) +def ResourceUUID(value, creator): + try: + return utils.ResourceUUID(value, creator) + except ValueError as e: + raise voluptuous.Invalid(e) + + +def ResourceID(value, creator): + return (six.text_type(value), ResourceUUID(value, creator)) class ResourcesController(rest.RestController): @@ -946,7 +956,9 @@ class ResourcesController(rest.RestController): # NOTE(sileht): we need to copy the dict because when change it # and we don't want that next patch call have the "id" schema = dict(schema_for(self._resource_type)) - schema["id"] = ResourceID + creator = pecan.request.auth_helper.get_current_user( + pecan.request.headers) + schema["id"] = functools.partial(ResourceID, creator=creator) body = deserialize_and_validate(schema) body["original_resource_id"], body["id"] = body["id"] @@ -956,8 +968,6 @@ class ResourcesController(rest.RestController): } target.update(body) enforce("create resource", target) - creator = pecan.request.auth_helper.get_current_user( - pecan.request.headers) rid = body['id'] del body['id'] try: @@ -1003,8 +1013,7 @@ class ResourcesController(rest.RestController): def delete(self, **kwargs): # NOTE(sileht): Don't allow empty filter, this is going to delete # the entire database. - attr_filter = deserialize_and_validate( - SearchResourceTypeController.ResourceSearchSchema) + attr_filter = deserialize_and_validate(ResourceSearchSchema) # the voluptuous checks everything, but it is better to # have this here. @@ -1129,16 +1138,16 @@ class QueryStringSearchAttrFilter(object): return cls._parsed_query2dict(parsed_query) -def _ResourceSearchSchema(v): - """Helper method to indirect the recursivity of the search schema""" - return SearchResourceTypeController.ResourceSearchSchema(v) +def ResourceSearchSchema(v): + return _ResourceSearchSchema()(v) -class SearchResourceTypeController(rest.RestController): - def __init__(self, resource_type): - self._resource_type = resource_type +def _ResourceSearchSchema(): + user = pecan.request.auth_helper.get_current_user( + pecan.request.headers) + _ResourceUUID = functools.partial(ResourceUUID, creator=user) - ResourceSearchSchema = voluptuous.Schema( + return voluptuous.Schema( voluptuous.All( voluptuous.Length(min=0, max=1), { @@ -1155,31 +1164,36 @@ class SearchResourceTypeController(rest.RestController): voluptuous.Length(min=1, max=1), voluptuous.Any( {"id": voluptuous.Any( - utils.ResourceUUID, [utils.ResourceUUID]), + [_ResourceUUID], _ResourceUUID), voluptuous.Extra: voluptuous.Extra})), voluptuous.Any( u"and", u"∨", u"or", u"∧", u"not", ): voluptuous.All( - [_ResourceSearchSchema], voluptuous.Length(min=1) + [ResourceSearchSchema], voluptuous.Length(min=1) ) } ) ) - @classmethod - def parse_and_validate_qs_filter(cls, query): + +class SearchResourceTypeController(rest.RestController): + def __init__(self, resource_type): + self._resource_type = resource_type + + @staticmethod + def parse_and_validate_qs_filter(query): try: attr_filter = QueryStringSearchAttrFilter.parse(query) except InvalidQueryStringSearchAttrFilter as e: raise abort(400, e) - return voluptuous.Schema(cls.ResourceSearchSchema, + return voluptuous.Schema(ResourceSearchSchema, required=True)(attr_filter) def _search(self, **kwargs): if pecan.request.body: - attr_filter = deserialize_and_validate(self.ResourceSearchSchema) + attr_filter = deserialize_and_validate(ResourceSearchSchema) elif kwargs.get("filter"): attr_filter = self.parse_and_validate_qs_filter(kwargs["filter"]) else: @@ -1328,13 +1342,16 @@ class SearchMetricController(rest.RestController): class ResourcesMetricsMeasuresBatchController(rest.RestController): - MeasuresBatchSchema = voluptuous.Schema( - {ResourceID: {six.text_type: MeasuresListSchema}} - ) - @pecan.expose('json') def post(self, create_metrics=False): - body = deserialize_and_validate(self.MeasuresBatchSchema) + creator = pecan.request.auth_helper.get_current_user( + pecan.request.headers) + MeasuresBatchSchema = voluptuous.Schema( + {functools.partial(ResourceID, creator=creator): + {six.text_type: MeasuresListSchema}} + ) + + body = deserialize_and_validate(MeasuresBatchSchema) known_metrics = [] unknown_metrics = [] @@ -1349,8 +1366,6 @@ class ResourcesMetricsMeasuresBatchController(rest.RestController): known_names = [m.name for m in metrics] if strutils.bool_from_string(create_metrics): - creator = pecan.request.auth_helper.get_current_user( - pecan.request.headers) already_exists_names = [] for name in names: if name not in known_names: diff --git a/gnocchi/tests/gabbi/gabbits/base.yaml b/gnocchi/tests/gabbi/gabbits/base.yaml index eeb3ed9f..ef097711 100644 --- a/gnocchi/tests/gabbi/gabbits/base.yaml +++ b/gnocchi/tests/gabbi/gabbits/base.yaml @@ -134,13 +134,13 @@ tests: project_id: f3d41b77-0cc1-4f0b-b94a-1d5be9c0e3ea status: 201 response_headers: - location: $SCHEME://$NETLOC/v1/resource/generic/8d835270-2834-5e55-a693-fd0cf91cba3d + location: $SCHEME://$NETLOC/v1/resource/generic/2d869568-70d4-5ed6-9891-7d7a3bbf572d response_json_paths: type: generic started_at: "2014-01-03T02:02:02+00:00" project_id: f3d41b77-0cc1-4f0b-b94a-1d5be9c0e3ea created_by_project_id: 99d13f22-3618-4288-82b8-6512ded77e4f - id: 8d835270-2834-5e55-a693-fd0cf91cba3d + id: 2d869568-70d4-5ed6-9891-7d7a3bbf572d original_resource_id: 1.2.3.4 - name: get status denied diff --git a/gnocchi/tests/gabbi/gabbits/batch-measures.yaml b/gnocchi/tests/gabbi/gabbits/batch-measures.yaml index 6cc3710c..ae3b454e 100644 --- a/gnocchi/tests/gabbi/gabbits/batch-measures.yaml +++ b/gnocchi/tests/gabbi/gabbits/batch-measures.yaml @@ -244,7 +244,7 @@ tests: response_json_paths: $.description.cause: "Unknown resources" $.description.detail: - - resource_id: "301dbf9a-4fce-52b6-9010-4484c469dcec" + - resource_id: "6b8e287d-c01a-538c-979b-a819ee49de5d" original_resource_id: "foobar" - name: push measurements to named metrics and resource with create_metrics with wrong measure objects diff --git a/gnocchi/tests/gabbi/gabbits/resource-type.yaml b/gnocchi/tests/gabbi/gabbits/resource-type.yaml index dae792f0..9ffd74e3 100644 --- a/gnocchi/tests/gabbi/gabbits/resource-type.yaml +++ b/gnocchi/tests/gabbi/gabbits/resource-type.yaml @@ -6,6 +6,11 @@ fixtures: - ConfigFixture +defaults: + request_headers: + x-user-id: 0fbb231484614b1a80131fc22f6afc9c + x-project-id: f3d41b770cc14f0bb94a1d5be9c0e3ea + tests: - name: list resource type @@ -222,8 +227,6 @@ tests: - name: post invalid resource POST: /v1/resource/my_custom_resource request_headers: - x-user-id: 0fbb2314-8461-4b1a-8013-1fc22f6afc9c - x-project-id: f3d41b77-0cc1-4f0b-b94a-1d5be9c0e3ea content-type: application/json data: id: d11edfca-4393-4fda-b94d-b05a3a1b3747 @@ -239,8 +242,6 @@ tests: - name: post invalid resource uuid POST: $LAST_URL request_headers: - x-user-id: 0fbb2314-8461-4b1a-8013-1fc22f6afc9c - x-project-id: f3d41b77-0cc1-4f0b-b94a-1d5be9c0e3ea content-type: application/json data: id: d11edfca-4393-4fda-b94d-b05a3a1b3747 @@ -258,8 +259,6 @@ tests: - name: post custom resource POST: $LAST_URL request_headers: - x-user-id: 0fbb2314-8461-4b1a-8013-1fc22f6afc9c - x-project-id: f3d41b77-0cc1-4f0b-b94a-1d5be9c0e3ea content-type: application/json data: id: d11edfca-4393-4fda-b94d-b05a3a1b3747 @@ -276,8 +275,6 @@ tests: - name: patch custom resource PATCH: /v1/resource/my_custom_resource/d11edfca-4393-4fda-b94d-b05a3a1b3747 request_headers: - x-user-id: 0fbb2314-8461-4b1a-8013-1fc22f6afc9c - x-project-id: f3d41b77-0cc1-4f0b-b94a-1d5be9c0e3ea content-type: application/json data: name: foo @@ -303,8 +300,6 @@ tests: - name: post resource with default POST: /v1/resource/my_custom_resource request_headers: - x-user-id: 0fbb2314-8461-4b1a-8013-1fc22f6afc9c - x-project-id: f3d41b77-0cc1-4f0b-b94a-1d5be9c0e3ea content-type: application/json data: id: c4110aec-6e5c-43fa-b8c5-ffdfbca3ce59 diff --git a/gnocchi/tests/gabbi/gabbits/transformedids.yaml b/gnocchi/tests/gabbi/gabbits/transformedids.yaml index b5ab2092..cc544f11 100644 --- a/gnocchi/tests/gabbi/gabbits/transformedids.yaml +++ b/gnocchi/tests/gabbi/gabbits/transformedids.yaml @@ -66,8 +66,26 @@ tests: project_id: f3d41b770cc14f0bb94a1d5be9c0e3ea status: 400 response_strings: - - "Invalid input: not a valid value for dictionary value @ data[" - - "'id'] " + - "'/' is not supported in resource id" + + + - name: post new resource non uuid again different user + POST: /v1/resource/generic + request_headers: + x-user-id: 0fbb231484614b1a80131fc22f6afc9b + x-project-id: f3d41b770cc14f0bb94a1d5be9c0e3ea + data: + id: generic zero + metrics: + cpu.util: + archive_policy_name: medium + status: 201 + response_json_paths: + created_by_user_id: 0fbb231484614b1a80131fc22f6afc9b + created_by_project_id: f3d41b770cc14f0bb94a1d5be9c0e3ea + response_headers: + # is a UUID + location: /v1/resource/generic/[a-f0-9-]{36}/ - name: post new resource non uuid POST: /v1/resource/generic @@ -151,7 +169,7 @@ tests: archive_policy_name: medium status: 400 response_strings: - - not a valid value for + - transformable resource id >255 max allowed characters for dictionary value - name: post long non uuid resource id POST: $LAST_URL diff --git a/gnocchi/utils.py b/gnocchi/utils.py index 50dc303b..1b3cd476 100644 --- a/gnocchi/utils.py +++ b/gnocchi/utils.py @@ -39,23 +39,24 @@ LOG = log.getLogger(__name__) RESOURCE_ID_NAMESPACE = uuid.UUID('0a7a15ff-aa13-4ac2-897c-9bdf30ce175b') -def ResourceUUID(value): +def ResourceUUID(value, creator): if isinstance(value, uuid.UUID): return value if '/' in value: raise ValueError("'/' is not supported in resource id") try: - try: - return uuid.UUID(value) - except ValueError: - if len(value) <= 255: - if six.PY2: - value = value.encode('utf-8') - return uuid.uuid5(RESOURCE_ID_NAMESPACE, value) - raise ValueError( - 'transformable resource id >255 max allowed characters') - except Exception as e: - raise ValueError(e) + return uuid.UUID(value) + except ValueError: + if len(value) <= 255: + # value/creator must be str (unicode) in Python 3 and str (bytes) + # in Python 2. It's not logical, I know. + if six.PY2: + value = value.encode('utf-8') + creator = creator.encode('utf-8') + return uuid.uuid5(RESOURCE_ID_NAMESPACE, + value + "\x00" + creator) + raise ValueError( + 'transformable resource id >255 max allowed characters') def UUID(value): diff --git a/releasenotes/notes/uuid5-change-8a8c467d2b2d4c85.yaml b/releasenotes/notes/uuid5-change-8a8c467d2b2d4c85.yaml new file mode 100644 index 00000000..ec6b6c51 --- /dev/null +++ b/releasenotes/notes/uuid5-change-8a8c467d2b2d4c85.yaml @@ -0,0 +1,12 @@ +--- +issues: + - >- + The conversion mechanism provided by the API to convert non-UUID resource + id to UUID is now also based on the user creating/accessing the resource. + This makes sure that the conversion generates a unique UUID for the user + and that several users can use the same string as `original_resource_id`. +upgrade: + - >- + Since `original_resource_id` is now unique per creator, that means users + cannot refer to resource by using the `original_resource_id` if the + resource was not created by them. diff --git a/run-upgrade-tests.sh b/run-upgrade-tests.sh index 4220e252..5bcdd73f 100755 --- a/run-upgrade-tests.sh +++ b/run-upgrade-tests.sh @@ -109,12 +109,16 @@ RESOURCE_IDS=( "5a301761-cccc-46e2-8900-8b4f6fe6675a" ) # NOTE(sileht): / are now _ -[ "$have_resource_type_post" ] && RESOURCE_ID_EXT="5a301761_dddd_46e2_8900_8b4f6fe6675a" +# NOTE(jdanjou): and we reencode for admin:admin, but we cannot authenticate as +# admin:admin in basic since ":" is forbidden in any username, so let's use the direct +# computed ID +[ "$have_resource_type_post" ] && RESOURCE_ID_EXT="517920a9-2e50-58b8-88e8-25fd7aae1d8f" + dump_data $GNOCCHI_DATA/new # NOTE(sileht): change the output of the old gnocchi to compare with the new without '/' $GSED -i -e "s,5a301761/dddd/46e2/8900/8b4f6fe6675a,5a301761_dddd_46e2_8900_8b4f6fe6675a,g" \ - -e "s,19235bb9-35ca-5f55-b7db-165cfb033c86,fe1bdabf-d94c-5b3a-af1e-06bdff53f228,g" $GNOCCHI_DATA/old/resources.list + -e "s,19235bb9-35ca-5f55-b7db-165cfb033c86,517920a9-2e50-58b8-88e8-25fd7aae1d8f,g" $GNOCCHI_DATA/old/resources.list echo "* Checking output difference between Gnocchi $old_version and $new_version" diff -uNr $GNOCCHI_DATA/old $GNOCCHI_DATA/new