Identity Client always uses adminURL enpoint, so cannot test non-admin actions correctly

Bug #1473396 reported by Ievgeniia Zadorozhna
12
This bug affects 2 people
Affects Status Importance Assigned to Milestone
tempest
Fix Released
Medium
Ievgeniia Zadorozhna

Bug Description

The tempest Identity Client always is initialized with adminURL as the endpoint, it is hard-coded in tempest/tempest/clients.py in _set_identity_clients method.

This allows to run api/identity/admin tests with correct admin endpoint but this leads to the situation when we cannot run non-admin api/identity tests correctly, because non-admin user actions will try to use admin endpoint, when really publicURL endpoint is needed.

Now there is not many non-admin tests in api/identity and we (Refstack team) are working on creating new ones to expand the tempest coverage.
Since both admin and non-admin tests use the same identity_client we suggest that it's better to initialize identity_client with endpoint from tempest.conf ([identity]/enpoint_type) than the way to create two: public and admin identity clients with the only one difference - enpoint_type.

The way where enpoint_type will be taken from config allows to run admin tests and non-admin tests in a separate way.

Changed in tempest:
assignee: nobody → izadorozhna (izadorozhna)
summary: Identity Client always uses adminURL enpoint, so cannot test public user
- actions
+ actions correctly
summary: - Identity Client always uses adminURL enpoint, so cannot test public user
+ Identity Client always uses adminURL enpoint, so cannot test non-admin
actions correctly
description: updated
description: updated
Changed in tempest:
status: New → Confirmed
description: updated
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to tempest (master)

Fix proposed to branch: master
Review: https://review.openstack.org/200486

Changed in tempest:
status: Confirmed → In Progress
Changed in tempest:
importance: Undecided → Medium
Revision history for this message
Andrea Frittoli (andrea-frittoli) wrote :

I agree that having the endpoint_type hardcoded is not ideal, so this should be fixed.
However I disagree about the assumption that non-admin tests cannot be run against an admin URL.

Which API is exposed in which URL type is really a deployer decision, and access to the different URL is controlled via network configuration / firewall rather than OpenStack configuration.

When testing a public cloud it is possible that the admin URL is not accessible to the persons running the tests, so the identity client should honour the endpoint_type.

Revision history for this message
Sergey Slipushenko (sslypushenko) wrote :

andrea-frittoli:
Actually, if we want to non-admin tests pass against admin URL it requires to have too smart identity client.

Right now non-admin tests use non-admin identity client. It looks quite reasonable and it works for non-admin endpoint. But if we run same tests against admin endpoint we will get a failure (something like that - http://paste.openstack.org/show/361823/ ). It because non-admin identity client gets a token with primary credentials and tries to do some actions require admin rights. In public endpoint these actions don't require admin rights but in admin endpoint - they do.

So if we want to have non-admin tests passed against admin endpoint it leads us to have "smart" non_admin_but_admin_if_necessary identity client. It is not a good idea, I think.

Changed in tempest:
assignee: izadorozhna (izadorozhna) → Sergey Slipushenko (sslypushenko)
Changed in tempest:
assignee: Sergey Slipushenko (sslypushenko) → izadorozhna (izadorozhna)
Revision history for this message
Matthew Treinish (treinish) wrote :

So I think the confusion here is around the idea that all clouds have the same restrictions set on endpoint configurations and the catalog entries. The point Andrea is making here is that keystone (or any service for that matter) can be configured to restrict or to not restrict operations to any endpoint type. Just like the policy file is completely configurable so you can severely limit the rules for performing an operation or even disable it. Yes this makes things terrible, absolutely terrible, from an interoperability standpoint, but as it stands this how things work. We can't be making assumptions like what you have described in this bug about only being able to perform admin actions against the adminURL endpoint type. I can easily configure an OpenStack cloud to do the exact opposite and restrict admin actions to the publicURL endpoint type and reject all other user creds and non-admin actions, (I'm not sure why I'd do this though) the endpoint type is just a label and doesn't carry any extra weight.

This is exactly why tempest has a config option to specify which endpoint type to use for tests. The only solution that will fit any possible deployment is to limit ourselves to a single endpoint type because there are no standard rules for how they should be used. If using a single endpoint is too restrictive for your particular deployment to run all the tests then you'll have to leverage the test runner or config feature flags to select the set of tests which are supposed to work against that single endpoint. Then you can have a separate config/set of tests if you need and concatenate the results. (this is a nice feature of the subunit protocol if you use that for tracking results because this just works)

So I agree with Andrea, the part of the bug here which is valid is the hardcoded adminURL. In no way is that correct and we should fix it. As for taking an opinion on which tests should run with which endpoint type we can not do that, because there is not way to make that assumption compatible for every deployment out there. I also would like to change the default type to publicURL because that's really what we should be running against by default, but there is a backwards compat concern for changing the default. (a good compromise might be a detailed help comment and changing devstack over) We can discuss that in the review that honors the config flag.

FWIW, If you shared some more details about how your cloud is configured and why keystone is returning a 403 for your particular request I'd be willing to walk through logs with you to get to the bottom of a configuration which works.

Revision history for this message
Catherine Diep (cdiep) wrote :

I think we all agree that the issue to fix here is the hardcoded adminURL in the _set_identity_clients method in client.py. What we need is to have options to initialize this endpoint_type for non-admin user testing. I also agree with Andrea that different URL is controlled via network configuration / firewall. I have experienced testing on clouds where the endpoint_type needed to be set to internalURL.

What Refstack wants to test are a few basic Keystone functions using "non-amin user credential", such as get token, list user's own tenant(s) .. We expect that a cloud which is built with out-of-the-box default configuration should be able to pass these tests. Typically in these clouds, a non-admin user will have no admin role assigned to it and thus would not be able to access Keystone APIs through adminURL.

With hardcoded adminURL and non-admim user, the test_list_tenants_returns_authorized_tenants in tempest.api.identity.v2.test_tenants
(https://review.openstack.org/#/c/192709/) would fail for "code": 403, "title": "Forbidden" (stack trace: http://paste.openstack.org/show/378458/). This test result was collected on a RDO cloud (built based on installation instruction on https://www.rdoproject.org/Quickstart).

Does it make sense to add a non_admin_identity_client in which the endpoint_type is set to whatever value set in the Tempest configuration file?

Revision history for this message
Matthew Treinish (treinish) wrote :

@Cathrine no it doesn't make sense to do that as a separate thing. We just need to remove the hardcoded endpoint type and it should just use the same mechanisms that already exist. If we did anything where a client initialized with a credentials of a certain type had a different endpoint type then what was configured we'd be conflating the concept of roles assigned to a user and endpoint type. Just to make it clear there is no coupling between the concept of roles and endpoint type defined anywhere. Any restrictions where certain operations or roles only work on certain endpoint type is 100% a deployment specific and not something we can really account for in tempest because there are too many possibilities.

The fix here is literally just:

http://paste.openstack.org/show/378822/ (and the sample config update)

If you need to use publicURL for your testing, just change the config opt to that and you'll be good to go. The only bug here is that adminURL was hard coded.

For what you're trying to do in your test that's perfectly fine. The test just needs to be written so it allocates a regular user (which implies non-admin) from the credential provider and inits the clients with those creds, which it should do automatically if the inheritance is setup correctly. Nothing, about the endpoint type or the client needs to be special cased for the tests in that review.

Revision history for this message
Sergey Slipushenko (sslypushenko) wrote :

@Matthew:
I and izadorozhna, we fully understand your point - stick to the same mechanisms that already exist and support it.

We added skips for keystone admin tests against 'publicUrl' because we want to avoid false negative test results. I hope that it is obvious that keystone admin tests will fail against keystone 'publicUrl'. So we think that skipping these tests is more reasonable, than lots of failures.

Also, we added skips for keystone non-admin tests against 'adminUrl', because some tests for public Keystone API fail, if you point adminUrl in tempest config. For example:
test_tenants (list_tenants) FAIL: 403 error, log here : http://paste.openstack.org/show/380621/
reason: <admin-endpoint>/v2.0/tenants non reachable for non-admin user
test_users (change passwd) FAIL: 401, 404 for :35357/v2.0/OS-KSCRUD/users/<id>, log here: http://paste.openstack.org/show/380622/
reason: <admin-endpoint>/v2.0/OS-KSCRUD/users/<id> not exists on non-admin endpoint
Sure thing, we can skip only these two tests, but as for me, it is a bad idea to check that admin endpoint can act almost like a public endpoint.

It is clear, that our suggestion (skips) is not only one possible solution and definitely not perfect. But all other simple solutions look like dirty hacks(raw_request, one_more_identity_client, etc). In my humble opinion, the best solution here is allowing tests to choose requered endpoint, adminUrl for admin tests and publiclUrl for non-admin tests. Is definitely not the same mechanisms that already exist, because only keystone API requires such complicated logic. So, we decided to not propose such complicated solution.

Revision history for this message
Matthew Treinish (treinish) wrote :

Sergey, you're missing the point, your statement "I hope that it is obvious that keystone admin tests will fail against keystone 'publicUrl'." is 100% deployment specific and not a absolute truth. The way you've deployed your cloud, which is likely just the keystone default paste file, might make this statement true for when you're running tests. But, that doesn't make it true for every deployment, it's still completely configurable by deployers. I can easily setup a cloud using the knobs provided as deployer configuration and change this behavior.

The issue with all of those proposed solutions you mention is that the are treating adminURL and publicURL endpoint types as a defined categories that will mean the same thing on every single cloud you're deploying on. The problem is that's just not true is it, those are just labels, I will agree that keystone v2 does have a separate public and admin api route (v3 does not) which is different, but that doesn't have anything to do with which endpoint types you use to access the different routes. I agree it's easy to confuse things here they so I'll try to use examples. Let's look at the default paste.ini file in the keystone repo:

http://git.openstack.org/cgit/openstack/keystone/tree/etc/keystone-paste.ini

This is considered a configuration file for deployers, sure you can use the default one, but nothing stops anyone from editing a config file. Looking at it you can see exactly where the split your asserting is setup towards the bottom of the page. You can do a simple synthetic experiment: swap L96 and L102, then restart keystone, and then run your tests again you'll notice the exact opposite behavior of what you're asserting here. (that would probably be a strange way to deploy your keystone but a completely valid way) How would your proposed skips work in the case? They'd skip all the admin tests and fail with the non-admin tests right? That would be the exact opposite of your intent with adding the skip logic. This is exactly what I'm talking about though, the endpoint_type names don't actually match up to the api your touching it's just a configurable label. Yes, it's confusing because they're both called admin and the intent is you use them together, but there isn't anything dictating that.

Just as with your skip your proposed solutions based solely on the endpoint_type as a branch selector for some other behavior won't work because they rely on the endpoint_type as the switch. We can't programatically change how we operate based on the configured endpoint_type it just doesn't work. It'd be different if we just had to worry about a single deployment, we could easily code to the behavior of a single deployment.

Revision history for this message
Matthew Treinish (treinish) wrote :

So I've been thinking about this a bit more and the important distinction with keystone is that with v2 there 2 separate wsgi apps and your correct in that we don't differentiate between those 2. So the more I think about the more I reluctantly come to the conclusion that we really need 3 different endpoint_type options for keystone. 1 for keystone v3 (which is an independent app that can be configured completely independently from v2) 1 for the keystone v2 public api, and 1 for the keystone v2 admin api.

So we'd end up with 3 options, something like:

CONF.identity.v3_endpoint_type
CONF.identity.v2_public_endpoint_type
CONF.identity.v2_admin_endpoint_type

to replace the single opt we have now. Then we'd need a separate client init'd for each of the 3 endpoint types. It's ugly, but the only way I can come up with so that we can properly handle the differences here.

Revision history for this message
Sergey Slipushenko (sslypushenko) wrote :

@Matthew, thank you for paying so much attention to this discussion. I completely agree with you, that 3 clients - is not a perfect solution, but it looks like a pretty good compromise.
Also, I'm glad to see that you have stopped insisting on a deployment nature of this bug. Sure thing, we can deploy OpenStack in many different (and really weird) ways. But not all of them should be considered as valid. For example, we can easily deploy some OpenStack services without keystone middleware just removing it from paste config. It will break some tempest tests and it is great)

Changed in tempest:
assignee: izadorozhna (izadorozhna) → Sergey Slipushenko (sslypushenko)
Changed in tempest:
assignee: Sergey Slipushenko (sslypushenko) → izadorozhna (izadorozhna)
Changed in tempest:
assignee: izadorozhna (izadorozhna) → Brandon Palm (bapalm)
Changed in tempest:
assignee: Brandon Palm (bapalm) → izadorozhna (izadorozhna)
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to tempest (master)

Reviewed: https://review.openstack.org/200486
Committed: https://git.openstack.org/cgit/openstack/tempest/commit/?id=c786213f95132df27b439c397c88ca2cb0d69390
Submitter: Jenkins
Branch: master

commit c786213f95132df27b439c397c88ca2cb0d69390
Author: Jane Zadorozhna <email address hidden>
Date: Fri Jul 10 14:34:50 2015 +0300

    Added endpoint types for intialization of different IdentityClients

    There is a bug https://bugs.launchpad.net/tempest/+bug/1473396 which
    describes that IdentityClient always uses admin enpoint but this is
    not correct for non-admin api/identity tests. The latter need to use
    public endpoint_type instead.

    Added one more identity client ('identity_public_client') to two
    existent. Added to tempest.conf 3 enpoint_types to [identity] group
    instead of old one 'endpoint_type' for intialization of different
    IdentityClients:

    * CONF.identity.v3_endpoint_type (because Keystone api v3 can be
    configured independently from v2);
    * CONF.identity.v2_public_endpoint_type (for Keystone v2 public api);
    * CONF.identity.v2_admin_endpoint_type (for Keystone v2 admin api).

    Thus, non-admin tests in api/identity/v2 directory would use
    'identity_public_client'; admin tests in api/identity/admin would use
    'identity_client' and v3 tests in api/identity/v3 - identity_v3_client.

    Change-Id: Icd5d175f8de6ccdaa6c718f6d4f68677cd4d7008
    Closes-Bug: #1473396

Changed in tempest:
status: In Progress → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.