Commit Graph

273 Commits

Author SHA1 Message Date
Takashi Kajinami 211fdbab28 Require more specific exception
This is follow-up of 1e683483e7 and
replaces the generic Exception asserted in a unit tests by the specific
castellan exception, according to the 4.4.0 release.

This change does not bump minimum required version of castellan because
the version requirement only affects unit tests.

Depends-on: https://review.opendev.org/c/openstack/requirements/+/911059
Change-Id: Id1ed909f179038713d9da2fd72cf39e7fb7c8dfe
2024-03-06 03:20:39 +00:00
Takashi Kajinami 1e683483e7 Prepare for castellan 4.4.0
In castellan 4.4.0, we fixed the wrong exception MockKeyManager.get
raised in case the requested key does not exist. This change interferes
with the way one unit test case ensure a key is gone.

This replaces the asserted exception by generic Exception so that we
can unblock u-c bump now. We can later replace it by the specific and
correct exception (ManagedObjectNotFoundError) once castellan 4.4.0 is
pulled to u-c.

[1] 2cc410f56e7275d982bca95aa65cd11e22fc7c3c

Change-Id: I8cc1420e8b16ce0bc74314fd7b8aabf6e133abd8
2024-02-28 11:28:48 +09:00
Pranali Deore 8c04d19e88 Enabled new defaults and scope checks by default
Enabling the enforce scope and new defaults by default in glance

Related blueprint secure-rbac

Change-Id: I0808dc0b1b34b527e38aa137c1dd25e1fc06409f
2023-02-16 11:11:31 +00:00
Pierre-Samuel Le Stang 480ea3825f Implement glance-download internal plugin
Add a new import method called glance-download
that implements a glance to glance download in
a multi-region cloud with a federated Keystone.

This method will copy the image data and
selected metadata to the target glance, checking
that the downloaded size match the "size" image
attribute in the source glance.

Implements: blueprint glance-download-import
Co-Authored-By: Victor Coutellier <victor.coutellier@gmail.com>
Change-Id: Ic51c5fd87caf04d38aeaf758ad2d0e2f28098e4d
2022-08-23 08:26:52 -07:00
xuanyandong be997b53ab Remove unicode literal strings
Co-Authored-By: Cyril Roelandt <cyril@redhat.com>
Change-Id: Id9e1a5fb9c732c207ee08f0dbf387436a1783174
2022-06-30 19:37:03 +02:00
Pranali Deore 3790cfd4a1 Remove dead code of auth and policy layers
In Xena we have mangaed to move all policy checks to API layer,
now removing the dead code from policy and authorization layer

NOTE: Some of the code is still being used from policy layer,
hence keeping it there only at this moment.

Change-Id: Ibee749cde20687d8c243cf84ae80b4de67d8ef3d
2022-06-14 10:15:55 +00:00
Stephen Finucane f68e04f268 Remove six.assertRaisesRegex usage
Change-Id: I172d640b7155913ad20599fd99825e501ed82631
Signed-off-by: Stephen Finucane <sfinucan@redhat.com>
2022-01-27 16:37:20 +00:00
Stephen Finucane 9679ffc463 Remove six.moves.http_client usage
This is a rather beefy change due to the number of usages of this
import. The changes are trivial though.

Change-Id: I7badeeaca438b0291f4ed86670e7f217e6372c61
Signed-off-by: Stephen Finucane <sfinucan@redhat.com>
2022-01-27 14:54:05 +00:00
Stephen Finucane 39e667a145 Remove six.moves.range usage
This is the same as the 'range' keyword in Python 3

Change-Id: If3aa008522c24e870b7bf13de32b8ed1b27cb519
Signed-off-by: Stephen Finucane <sfinucan@redhat.com>
2022-01-27 14:54:05 +00:00
Stephen Finucane 6bd7c188ee Remove six.text_type, six.binary_type usage
Change-Id: I2ed464202f8b645aed11490e111c61d3c7423c11
Signed-off-by: Stephen Finucane <sfinucan@redhat.com>
2022-01-27 14:54:00 +00:00
Cyril Roelandt 0ca2f92161 Fix typos
Change-Id: I5e7776324c01e467799b1296c35b84dc0c10cce2
2021-10-13 03:02:52 +02:00
Zuul 5232f8249c Merge "'community' images need to be treated as public" 2021-09-03 00:42:03 +00:00
Erno Kuvaja f0d891a3ed 'community' images need to be treated as public
Even though 'community' images are not listed by default their
behaviour is like public images otherwise. This means that
the image data needs to be available for everyone and thus
the acls for the file/object should be like public too.

Change-Id: I79683c81233b35f2399119128a63d33d69c50eeb
Closes-bug: #1885928
2021-08-31 08:49:38 -07:00
Abhishek Kekane 6c87a18d4c Check policies for image import operation in API
This patch enforces policy checks required for importing/copying image
data to store in API layer.

Partially-Implements: blueprint policy-refactor

Change-Id: I18a5187d80bf76c0dc6f22dd8c96a8ffa0f46dc1
2021-08-24 20:09:24 +00:00
Ghanshyam Mann 4b094df5cb Suppress policy deprecation and default change warnings
As part of the new policy work, all of the defaults for
policies were changed and warnings about it are being emitted
constantly in the logs. We can suppress these for now until we are
enforcing new defaults.

This also suppresses policy deprecation warnings during tests to avoid
filling the console with warning messages.

Change-Id: Ib2a7a2ed8bcadbb6a002cfa4b34c70910faa4f00
2021-08-18 11:12:48 -05:00
Abhishek Kekane c87bfbbcd7 Check delete_image policy in the API
Note that this adds a clause to the delete exception handler which
returns 404 if the user can't delete the image via policy, but leaves
the existing internal forbidden->403 handler because of how image
property protections work.

Partially-Implements: blueprint policy-refactor

Change-Id: I5f2a406e31b706906b71ea5ecb4f1a53674c97fa
2021-08-10 09:13:52 -07:00
Dan Smith 8ddbdb9526 Add a member field to Image when appropriate
As noted by lbragstad, we need to make ImageTarget contain a member
field so that we can generically apply policies and be able to
properly include images for which we are a member. This was hacked
into place for ImageRepoProxy.get() but in order to apply it generally
to listing and other ops, we need to formalize that.

Partially-Implements: blueprint policy-refactor

Change-Id: I92d3792602a69922078d109095ad8ac9afc89d14
2021-08-04 07:56:05 -07:00
Dan Smith ba37ea3227 Check get_image(s) in the API
This includes a change to catch Forbidden and convert to NotFound.
The previous Forbidden handler was not only correct (it shoud hide
the permissions error with "not found") but it was actually dead code,
since the DB was performing its own checks and would never raise
Forbidden.

This also includes a change of the default policy for get_images
to include the other states, like get_image does. I think this was
just an oversight in the original RBAC patches, which didn't matter
because they weren't really being honored strictly.

Partially implements: blueprint policy-refactor

Change-Id: I70100cd7f01da803e9740cea1f7ce7ae18ad6919
2021-08-04 07:56:05 -07:00
Dan Smith 3825d2111a Make image update check policy at API layer
This implements the proposal at the PTG that makes the image update
API our first one to do all policy checks at the API layer instead
of the lower ones.

There are still some things that have to be resolved, like the image
repo will still check get_image even though we don't want it to.

However, this adds a new v2/policy module to encapsulate these checks
and adds them into the various places of the update call. It also
makes our test policy for modify_image match that of our actual
default, which is a major step in making sure our tests actually run
with the policy we expect at runtime.

NOTE: db.ImageRepo.save() was raising NotFound in cases where
Forbidden was raised by _check_mutate_authorization(). This means
that we could not tell the difference at the higher layers between
an actual NotFound and the 404 generated by an auth failure.

The ImageAPIPolicy module will raise Forbidden for cases where an
operation is forbidden, but the image would be otherwise visible to
the user, and NotFound otherwise to obscure the existence of images
that the user can not otherwise see.

Partially-Implements: blueprint policy-refactor

Change-Id: I43dbc88a9f3fd4c6b2a10c2534ccee9283663282
2021-08-04 07:52:27 -07:00
Dan Smith 76162a7222 Make property protection tests use member role
In order to avoid overriding the default policy in tests to be
completely unrestricted (as they are today), the property protection
tests need to add the member role into the roles being used to
test the additional permissions. This is because the actual default
policies require "admin or project member" permissions to do anything,
and just using one of the special roles will get blocked by the
regular policy before property protections are checked.

This could also be fixed by constructing a very complex policy for
these tests which allows those special roles to do their thing in
addition to member, but I do not believe this is how real deployments
would actually implement it.

These should definitely be reviewed with scrutiny to make sure that
all the assertions are still valid, and make sure we don't need more
than are already here.

Note that there are a couple assertions that were looking for CONFLICT
instead of FORBIDDEN on the special role attempt, that seems to be
a case of "test the current behavior not the desired one".

Change-Id: Ib6540223468421776f07745010e5d4afe5d58774
2021-07-16 07:58:46 -07:00
Dan Smith 76c3011a64 Enforce keystone limits for image upload
This adds enforcement of the image_size_total keystone limit for
image upload and import. We simply check the quota before either of
these operations and refuse to proceed further if the user is over
their quota.

Note that this disables checking of the global size quota if keystone
quotas are enabled.

Note this includes another fix to couple unit tests that do not
properly pass context to the get_flow() method.

Partially-implements: blueprint glance-unified-quotas
Change-Id: Idf5f004b72436df1f9c77bb32d60b9be5ae77a68
2021-06-29 08:53:18 -07:00
Dan Smith 7c1cd438a0 Fix broken test_update_queued_image_with_hidden
This fixes the test to behave the way we expect. It was failing to
do the update because it was using an image the requester did not
own, and asserting the found behavior of 403. However, the intent
here is to allow it to be updated. So, this uses the proper image and
asserts the proper behavior.

Change-Id: I71afe6a877485c8f92e67dcf32bb475c1a1a42a3
Closes-Bug: #1933360
2021-06-25 06:54:33 +00:00
Dan Smith 41e1cecbe6 Distributed image import
This implements distributed image import support, which addresses
the problem when one API worker has staged the image and another
receives the import request.

The general approach is that when a worker stages the image, it
records its self-reference URL in the image's extra_properties.  When
the import request comes in, any other host will proxy that HTTP
request direct to the original host instead of trying to do the import
itself.

Implements: blueprint distributed-image-import

Change-Id: I12daccb43c535b579c22f9d0742039b2ab42e929
2021-03-02 11:52:12 -08:00
Abhishek Kekane 281fadc15c New API /v2/images/{id}/tasks
Added new API /v2/images/{id}/tasks to show tasks associated with
image. This API will return list of tasks associated for valid image
else returns 404 not found if image is not present. This API also
initiates task scrubbing before returning tasks to user.

Implements: blueprint messages-api
Change-Id: Ib3cacb4dd4d75de32e539f8a3b48bdaa762e6d8e
2021-02-24 05:19:43 +00:00
Abhishek Kekane d54449af44 Utilize newly added tasks database fields
Made provision to pass image_id, request_id and user_id information
while creating new task.

Partially-Implements: blueprint messages-api
Change-Id: I299a222eeef81431143db3ba7fc08365c924326b
2021-02-24 05:17:43 +00:00
Dan Smith 2bfdc87a8c Stop raising 403 when image is not found
Glance has a vestigial policy override knob that allows an operator
to force a 403 response when attempting to get an image from the DB
that is not found. This runs contrary to the API documentation, and
creates a (potential) interoperability concern across clouds with
differing policies on this topic.

This removes that override and changes the test from validating this
override to validating that it can no longer happen.

APIImpact
Change-Id: Ie24e3eb2f31d10d2ab9af62a0b645e8bdd0c2ff2
Closes-Bug: #1915543
2021-02-22 17:35:50 +00:00
Dan Smith 314e93abe4 Exclude os_glance namespace from property quota
Now that glance is using properties in the os_glance namespace for
internal purposes, we should exclude the counting of these from the
enforced image property quota.

Change-Id: I5fbe5eb12fd34e054137732a02c4cc5b687e7c77
Related-Bug: #1912001
2021-01-25 12:30:50 -08:00
Dan Smith 0c45de3ed8 Make os_glance namespace reserved
This adds a general mechanism for reserving property names that start
with os_glance. This has been done informally already, but no
enforcement was performed, except for specific keys on update. As a
result, banning these keys from create, for example, was missed and
users are able to set these keys during an POST /images operation.

Depends-On: https://review.opendev.org/c/openstack/nova/+/771234
Change-Id: I31b4dae018d52ead773db25472013d783066ee17
Closes-Bug: #1912001
2021-01-25 12:30:50 -08:00
Dan Smith 552da84400 Cleanup import status information after busting a lock
When we bust a lock, we now own the image for that time period
and may exclude the other task (if still running) from updating
the import status information. If not still running, we should
take responsibility of that cleanup since we know what task we
stole the lock from. We should, however, only do that if we
succeed in grabbing the lock to avoid racing with another thread
which might be trying to do the same thing.

Change-Id: Iff3dfbfcbfb956a06d77a144e5456bdb556c5a2c
2020-08-24 06:41:13 -07:00
Dan Smith 3f6e349d08 Implement time-limited import locking
This attempts to provide a time-based import lock that is dependent
on the task actually making progress. While the task is copying
data, the task message is updated, which in turn touches the task
updated_at time. The API will break any lock after 30 minutes of
no activity on a stalled or dead task. The import taskflow will
check to see if it has lost the lock at any point, and/or if its
task status has changed and abort if so.

The logic in more detail:

1. API locks the image by task-id before we start the task thread, but
   before we return
2. Import thread will check the task-id lock on the image every time it
   tries to modify the image, and if it has changed, will abort
3. The data pipeline will heartbeat the task every minute by updating
   the task.message (bonus: we get some status)
4. If the data pipeline heartbeat ever finds the task state to be changed
   from the expected 'processing' it will abort
5. On task revert or completion, we drop the task-id lock from the image
6. If something ever gets stuck or dies, the heartbeating will stop
7. If the API gets a request for an import where the lock is held, it
   will grab the task by id (in the lock) and check the state and age.
   If the age is sufficiently old (no heartbeating) and the state is
   either 'processing' or terminal, it will mark the task as failed,
   steal the lock, and proceed.

Lots of logging throughout any time we encounter unexpected situations.

Closes-Bug: #1884596
Change-Id: Icb3c1d27e9a514d96fca7c1d824fd2183f69d8b3
2020-08-24 06:41:13 -07:00
Dan Smith 7c91a177ed Flesh out FakeImage for extra_properties
The FakeImage in test_images_resource.py is missing a lot of standard
properties. That becomes a problem as soon as you add extra_properties,
which gets delegated to for any missing property by ImageTarget. Thus,
if that fixture has extra_properties defined on it, any other base
property (i.e. checksum, size, etc) will be looked for inside that dict
if it is not defined on the fixture itself.

Thus, this adds fake values for all those properties and adds
extra_properties={} so that subsequent patches can put things into
extra_properties without a bunch of work.

Change-Id: I4b63706927b599eb343b614f0c396f699378e14b
2020-08-07 12:13:22 -07:00
Erno Kuvaja e1f0e94b90 Add "stores" to disallowed properties
Stores is image property which API uses to indicate which
stores (store IDs) contains the image. This also can be
set by user making it very confusing and potentially
catastrophic breaking for consumers.

This patch prevents that to happen.

Depends-on: https://review.opendev.org/#/c/744024/
Change-Id: I4eca092bd0a7cce1d6bbbd30685f4643cb4e7d1c
Closes-Bug: #1889676
2020-07-30 19:30:17 +00:00
Dan Smith 16a5431c66 Make glance-api able to do async tasks in WSGI mode
This teaches glance-api how to do async threading things when it is
running in pure-WSGI mode. In order to do that, a refactoring of things
that currently depend on eventlet is required.

It adds a [wsgi]/task_pool_threads configuration knob, which is used
in the case of pure-WSGI and native threads to constrain the number
of threads in that pool (and thus the task parallelism). This will
allow tuning by the operator, but also lets us default that to just
a single thread in the backport of these fixes so that we can avoid
introducing a new larger footprint in the backport unexpectedly.

Partial-Bug: #1888713
Depends-On: https://review.opendev.org/#/c/742047/
Change-Id: Ie15028b75fb8518ec2b0c0c0386d21782166f759
2020-07-24 11:13:45 -07:00
Dan Smith ee8a69d506 Add a policy knob for allowing non-owned image copying
This adds a copy_image policy knob which can be used to grant users
the ability to copy images other than their own using the new
functionality just added to the api_image_import task. By default,
only admins are allowed to do this.

A functional test modification is included to show that users can
be granted permission to do this based on something like the
"public visibility" attribute of an image.

Change-Id: Idebf66e2944bcddb7a5c76b81e47c654b401c2a8
2020-07-15 12:59:13 -07:00
Zuul 769d057c6b Merge "Switch from unittest2 compat methods to Python 3.x methods" 2020-07-15 16:15:55 +00:00
Zuul 9c66f46a14 Merge "Improve lazy loading mechanism for multiple stores" 2020-07-14 16:17:03 +00:00
Dan Smith 2fd0c25733 Make import task capable of running as admin on behalf of user
This makes the api_image_import task capable of running as an admin on
behalf of a user if so authorized by the API. It includes a new object
called ImportActionWrapper which provides a bundle of utility methods
which can be run either against the user-authorized or admin-authorized
ImageRepo passed in from the API. It encapsulates all the actions
we are able and willing to run as an admin for the user.

This is currently not drivable by the API because the policy check is
still statically defined as "admin or owner" but this change is
offered without any needed modification to the functional tests to
prove that it does not regress existing functionality. The following
patch will introduce a more robust knob for allowing users to do this,
and it brings the functional test changes with it.

Change-Id: Iac75956e314ec6f41db18430486bd8be9754e780
2020-07-10 08:59:54 -07:00
Abhishek Kekane ab0e5268a9 Improve lazy loading mechanism for multiple stores
Glance has a facility lazy loading for legacy images which will be called
on get/list api calls to add store information in image's location metadata
based on location URL of image. Even if admin decides to change the store
names in glance-api.conf same will also be updated in location metadata
for all images related to that particular store. Current implementation of
legacy image performs this operation on each get/list call as location metadata
is not getting updated in database or it doesn't handle to perform store name
check in glance-api.conf.

Improvements done:
1. Save updated location metadata information in database permenantly
2. Add logic to perform lazy loading only if store information is not present
in location metadata or store present in location metadata is not defined in
glance's enbaled_backends configuration option.

Change-Id: I789fa7adfb459e7861c90a51f418a635c0c22244
Closes-Bug: #1886374
2020-07-06 07:49:31 +00:00
Dan Smith c930638fcf Check authorization before import for image
Right now we only check to see if the user can see the image before
we kick off an import operation. However, that will never work unless
the user is the *owner* of the image (or an admin) which means we
return a 202 to the API caller and then the task fails immediately.

This change makes us check that authorization up front and return an
appropriate error to the user so they know it failed, and avoid
starting a task destined for failure.

Note that there was already a check for a Forbidden result when calling
the import API. However, that used a context.owner=None which could never
happen in reality. A more suitable check would have been to use a context
with a different real owner, but it turns out that the task creation
would have succeeded in that case as well. This test is changed to use
an alternate owner and ensure that we get the forbidden result from the
new check immediately.

Change-Id: I385f222c5e3b46978b40bdefdc28fcb20d9c67d3
Closes-Bug: #1884587
2020-06-30 10:24:45 -07:00
Dirk Mueller c636ad293c Switch from unittest2 compat methods to Python 3.x methods
With the removal of Python 2.x we can remove the unittest2 compat
wrappers and switch to assertCountEqual instead of assertItemsEqual

We have been able to use them since then, because
testtools required unittest2, which still included it. With testtools
removing Python 2.7 support [3][4], we will lose support for
assertItemsEqual, so we should switch to use assertCountEqual.

[1] - https://bugs.python.org/issue17866
[2] - https://hg.python.org/cpython/rev/d9921cb6e3cd
[3] - testing-cabal/testtools#286
[4] - testing-cabal/testtools#277^

Change-Id: I1641af07b21925402490f0b7dfee5314f416dde8
2020-06-23 14:17:30 +02:00
Sean McGinnis 94b0876429 Use unittest.mock instead of third party mock
Now that we no longer support py27, we can use the standard library
unittest.mock module instead of the third party mock lib.

Change-Id: I44e7b6f76e2d12f620ec602afc77ce11ba6b9d9a
Signed-off-by: Sean McGinnis <sean.mcginnis@gmail.com>
2020-04-20 15:07:00 +00:00
Abhishek Kekane 55b7c86ecf Fix multiple image imports if boolean input passed as string
If user passes 'all_stores_must_succeed' as 'false' in copy-image, multiple
image imports request body then it does not work as expected. Expected is
to skip the failure store and continue copying/importing image to other
stores but instead it stops execution of task and revert it and deletes
the image data copied/imported to previois stores.

Raised 400 BadRequest if 'all_stores_must_succeed' and 'all_stores' are
other thatn than boolean values.

NOTE: Documentation clearly suggest that we expect boolean values for
'all_stores_must_succeed' and 'all_stores'

Closes-Bug: #1871588
Change-Id: I5118489284fea007f8f29663581221b07a575cf3
2020-04-08 19:07:58 +00:00
Erno Kuvaja f267bd6cde Add possibility to delete image from single store
This change introduces new 'v2/stores/<store_id>/<image_id>'
endpoint that accepts 'DELETE' method request. Once successful
the request will delete the image <image_id>'s location that
matches the store <store_id>. If the store is not read-only
or return image in use exception the image data will be
deleted. In the case of read-only store, the location will
be removed and if the image in use is raised, the call will
fail.

bp: delete-from-store

Co-authored-by: Brian Rosmaita <rosmaita.fossdev@gmail.com>

Change-Id: I1cb45026489a96a283b82e8e7efc9975c181fceb
2020-03-13 14:46:13 +00:00
Abhishek Kekane 1754c9e2b0 Copy existing image in multiple stores
Added new import method 'copy-image' which will copy existing image into
specified list of stores. Introduced additional task which will serve
as internal plugin which will allow copying existing image into staging
area and then this data will be uploaded to specified stores via regula
import flow.

NOTE: This new import method 'copy-image' is only supported if multiple
stores are enabled in deployment.

APIImpact
Implements: blueprint copy-existing-image
Change-Id: I13eaab7ab013f44ce18465bdbdbe8052942570ff
2020-02-12 05:32:46 +00:00
Abhishek Kekane 5d15f07371 Staging area not cleared if image is deleted while importing
If multiple stores configured in glance and Image is deleted while import
operation is in progress then image data stays in staging area
(filesystem backend) and there is no other way than clearing it
manually.

Modified delete method to delete the data from staging area if image is
deleted while import operation is in progress.

Change-Id: Ib58accd6514e589dccde57fe063815b1ab1ce496
Closes-Bug: #1855417
2020-01-03 05:56:30 +00:00
Zuul d9f4451828 Merge "Make location API compatible with multiple store" 2019-09-25 14:22:28 +00:00
Zuul 986041635a Merge "MultiStore: Lazy update fails if image is not owned by owner" 2019-09-12 18:37:31 +00:00
Zuul e475581c72 Merge "Delete secret key on image deletion" 2019-09-06 13:49:25 +00:00
Cyril Roelandt b190a39a28 Delete secret key on image deletion
We add two extra properties for images:
- cinder_encryption_key_id, which stores the encryption key id;
- cinder_encryption_key_deletion_policy, which states whether the secret
  key should be deleted on image deletion.

This feature uses the Castellan key manager, and will therefore work
with all its supported backends.

Implements: blueprint barbican-secret-deletion-support
DocImpact

Change-Id: Iacd0b3785ad4cdd06961e6d11967775806e009ff
2019-09-05 03:16:39 +02:00
Zuul e9353d7861 Merge "Add 'compressed' option to container_format" 2019-08-28 23:00:10 +00:00