It's possible to create duplicate trusts

[Gilles Dubreuil]

A name field in Keystone DB is needed for helping identifying trusts.

Effectively , there could be multiple trusts for a same project/trustor/trustee including the same expiry date and same impersonate flag. And the same combination could have multiple trusts assigned with different roles or not.

Having a name would help for implementing trust usage.

A use case scenario is currently with Puppet Keystone module while creating the trust provider:

When creating a resource, Puppet uses a name as a title for the resource, that name is unique in order to provide idem-potency. The trust ID (Keystone DB) doesn't exist until its creation and therefore cannot be used as a title for a Puppet resource. Without a name, puppet provider has to make up a name from the different fields, which doesn't guarantee uniqueness anyway. Worse when fetching resources, the provider would have to fetch all the fields to identify the resource and take the first one if many available.

So far, most other Keystone DBMS objects (tables) have a name, which Puppet has been able to use to identify resources.
The latter is why it made more sense to create this request as a bug instead of a blueprint, basically saying a name has been missing upfront rather than being a request for enhancement.

[Richard Megginson (rmeggins)]

> So far, most other Keystone DBMS objects (tables) have a name, which Puppet has been able to use to identify resources.

or, for example, keystone_user_role works like the following::

    keystone_user_role { 'username@projectname':
      roles => ['admin', 'manager']
    }

The name is easily constructed from the username and projectname, and provides a mapping back to the role assignment list where it uniquely identifies the role assignment.

I'm not sure how it is possible to do this with trusts.

[Gilles Dubreuil (gdubreui)]

We could have a unique name for each trust, constructed with a serial number added to the project/trustor/trustee name.
Although since a name is not unique that's not ideal either.
I'm not sure we want to bother with that, but that's more an Openstack Puppet-keystone project question.

At least, with a name field, we could have idem-potency and allowing at Puppet manifests to run:

keystone_trust { 'admin_heat_delegation1':
      trustor_user => 'admin',
      trustee_user => 'heat',
      project => 'services',
      roles => ['admin', 'manager']
    }

[Steve Martinelli (stevemar)]

I could see the benefit of giving trusts a name. But since this would impact existing APIs it would require a spec. Suggest keeping this bug open, but also proposing a spec to keystone-specs. I will bring this up at the mid-cycle tomorrow (pending my memory)

[Richard Megginson (rmeggins)]

Adding a name field would also impact python-openstackclient, since that is what puppet uses to interact with keystone.

[Richard Megginson (rmeggins)]

If a `name` field is not forthcoming, we need some guidance from the Keystone devs:

* Is there some combination of trust fields/properties that can be used to uniquely identify a trust? If so, what are they?

[Dolph Mathews (dolph)]

We don't name role assignments, so I don't know why you would want to name trusts, either.

The trust ID is what makes them truly unique today. If the use case here is to put the burden of idempotency on keystone, then we can do that by creating a set of SQL-enforced unique fields, which would basically be all of them except for id, remaining_uses and deleted_at, and extra (why do trusts have an extra column??), so:

  trustor_user_id
  trustee_user_id
  project_id
  impersonation
  expires_at

If, on trust creation, a trust already existed with a given set of unique attributes, then keystone could respond with 200 (instead of 201) with the existing trust in the response body.

[Richard Megginson (rmeggins)]

> We don't name role assignments, so I don't know why you would want to name trusts, either.

These are easier to name, as you just have to deal with user + project or user + domain::

    keystone_user_role { 'username::userdomain@projectname::projectdomain':
       roles => ['foo', 'bar']
    }

These map easily on to the output of openstack role assignment list

> The trust ID is what makes them truly unique today.

Of course, but we can't have users putting trust IDs into puppet manifests as the name, especially when creating new trusts as you would have to create the trust in Keystone first to get the Keystone generated UUID of the trust.

> If the use case here is to put the burden of idempotency on keystone

No, we are not suggesting for Keystone to enforce idempotency.

The way puppet works is that it first does a list of all trusts e.g. `openstack trust list [--long]`. Then, it looks at each trust resource described in its manifest::

   keystone_trust { 'admin_heat_delegation1':
      trustor_user => 'admin',
      trustee_user => 'heat',
      project => 'services',
      roles => ['admin', 'manager']
    }

It tries to match each trust returned by the `trust list` command with each trust described in the manifest. If it is already there, fine, otherwise, it needs to create it. (or, if you are trying to delete a trust, by setting the ensure => absent in the manifest so the puppet code knows to delete it). The point is that we need a way for a keystone_trust description in a puppet manifest to uniquely identify a trust returned by `trust list`.

[Gilles Dubreuil (gdubreui)]

> If the use case here is to put the burden of idempotency on keystone

Absolutely not, that's a puppet concern but do so we need a name field

[Jamie Lennox (jamielennox)]

So i was in favour of this on first hearing it but i've been thinking about the problems of a name.

In keystone things with a name are only unique within a domain. That's why we have to specify user_domain_[id|name] and project_domain_[id|name] when using username or project_name. I'm concerned that after adding a name field to a trust we are going to want to be able to specify a trust_name as a scope. To include that we are also going to need to include trust_domain_[id|name] on authentication. I'm not sure if it makes sense for a trust to be owned by a domain (especially with the hierarchical projects/project=domain stuff we're starting now) as a trust is a relationship between two objects which are each owned by a (possibly different) domain.

Trusts are difficult and i'm keen to help out the deployment tools, but a name field has larger repercussions.

[Gilles Dubreuil (gdubreui)]

Can we get a description field instead?
So we could side effect it as a name.

[Richard Megginson (rmeggins)]

> Can we get a description field instead?
> So we could side effect it as a name.

What if someone wants to use it as a description?

[Richard Megginson (rmeggins)]

Jamie, I'm not sure I understand your comment - can you give an example of you would need to give trust_domain_name if the trust is not owned by a domain?

[Gilles Dubreuil (gdubreui)]

Using ID for primary keys are internal DBMS soup.
Indeed very helpful to make things simply unique and faster than a combination of several columns.
But they can't replace a functional need (requirement).
I think this is exactly what we're hitting here.
We need a unique way to identify a trust besides its technical ID key.
That would be the way to support Openstack Puppet project so it supports trusts

Yes, using a description field for its side effect is bad.
Quite as bad as using a name field to make it unique and we aren't asking the name field to be unique anyway.
What we need is a field so puppet can store a value that matters from a puppet viewpoint.
You call it whatever you want, if name is causing trouble then let's dub it 'nickname'.

[Gilles Dubreuil (gdubreui)]

I believe we can do better and if not that's going to be really dodgy for keystone_trust provider.

If not we' ll have to fish out (best catch call) using the following as a name for Puppet resource:
`trustor_user,trustee_user,project,[roles]`
And we'll have to fetch/convert each sub-field to its ID equivalent.

A the moment, for puppet needs to create a trust it would have to be something like this
  keystone_trust { 'user1,user2,tenant1,['admin','manager']':
      trustor_user => 'user1',
      trustee_user => 'user2',
      project => 'tenant1',
      roles => ['admin', 'manager']
    }

And since many trusts can exist for a same trustor/trustee/project and roles set, we have to guess the first one, hoping they always come in the same order.

For instance:

# openstack trust list -f value
3c5fb6f4fc1c4163b55ae98051d9311c None False 78e22bb71862481dbe8335b4ce4551e8 ac994e5701d644b6a3ac78c9dd1ad04a 24b047f52ff94029923f7f0ea982f03f
cdf0aeb44173473d87ce13ab02e5400f None False 78e22bb71862481dbe8335b4ce4551e8 ac994e5701d644b6a3ac78c9dd1ad04a 24b047f52ff94029923f7f0ea982f03f

# openstack trust show 3c5fb6f4fc1c4163b55ae98051d9311c -f shell
deleted_at="None"
expires_at="None"
id="3c5fb6f4fc1c4163b55ae98051d9311c"
impersonation="False"
project_id="78e22bb71862481dbe8335b4ce4551e8"
redelegation_count="0"
remaining_uses="None"
roles="admin manager"
trustee_user_id="ac994e5701d644b6a3ac78c9dd1ad04a"
trustor_user_id="24b047f52ff94029923f7f0ea982f03f"

# openstack trust show cdf0aeb44173473d87ce13ab02e5400f -f shell
deleted_at="None"
expires_at="None"
id="cdf0aeb44173473d87ce13ab02e5400f"
impersonation="False"
project_id="78e22bb71862481dbe8335b4ce4551e8"
redelegation_count="0"
remaining_uses="None"
roles="admin manager"
trustee_user_id="ac994e5701d644b6a3ac78c9dd1ad04a"
trustor_user_id="24b047f52ff94029923f7f0ea982f03f"

[Adam Young (ayoung)]

It appears like this will do little to help the situation, and may in fact make things worse. There is the rist that the description field for a trust might be misleading, and I;d rather have people look at the actual trustee field when determining whay a trust exists, and if it is legitimate.

Since most trusts should be to service users, specific to the tasks, those names or dsecriptions belong on the service users, not on the delegations.

[Gilles Dubreuil (gdubreui)]

Puppet Openstack cannot use trusts unless something is done.

Because when Puppet runs as a daemon, which is mostly the case in production, then every time the puppet catalog would be executed, all the trusts declared in the catalog will created (added), an infinity of trusts is going to be there very quickly, this is not acceptable for puppet users. This is basically breaking idem-potency rules where the same state is guaranteed.

BTW, Puppet initial need for trusts is to configure heat properly.

A Puppet manifest example:

keystone_trust {'adminv3 trust for user1':
  trustor => 'adminv3::admin_domain',
  trustee => 'user1::admin_domain',
  project => 'openstackv3::admin_domain',
  roles => ['admin'],
  impersonate => true,
  ensure => present
}

Because a trust is unique only by its ID, everytime the latter is executed, a new trust to be created.
Effectively, several trusts with the same properties can exists:

  $ trust create adminv3 user1 --role admin --project openstackv3 -f shell
  deleted_at="None"
  expires_at="None"
  id="00cf1f149fd0463994ad62ec0939ec71"
  impersonation="False"
  project_id="cf31bbe1ab4e4135a83b1b923d733b0d"
  redelegation_count="0"
  remaining_uses="None"
  roles="admin"
  trustee_user_id="e057d0ac9c394f5c833c6a746d76bc17"
  trustor_user_id="b0fa819e150a4005a10ebdfc9d1b97d4"

  $ trust create adminv3 user1 --role admin --project openstackv3 -f shell
  deleted_at="None"
  expires_at="None"
  id="0ae2d20042d6418b8f3b8832ab43360e"
  impersonation="False"
  project_id="cf31bbe1ab4e4135a83b1b923d733b0d"
  redelegation_count="0"
  remaining_uses="None"
  roles="admin"
  trustee_user_id="e057d0ac9c394f5c833c6a746d76bc17"
  trustor_user_id="b0fa819e150a4005a10ebdfc9d1b97d4"

$ openstack trust list -f csv
"ID","Expires At","Impersonation","Project ID","Trustee User ID","Trustor User ID"
  "00cf1f149fd0463994ad62ec0939ec71","",False,"cf31bbe1ab4e4135a83b1b923d733b0d","e057d0ac9c394f5c833c6a746d76bc17","b0fa819e150a4005a10ebdfc9d1b97d4"
  "0ae2d20042d6418b8f3b8832ab43360e","",False,"cf31bbe1ab4e4135a83b1b923d733b0d","e057d0ac9c394f5c833c6a746d76bc17","b0fa819e150a4005a10ebdfc9d1b97d4"

$ openstack trust show 00cf1f149fd0463994ad62ec0939ec71 -f shell
deleted_at="None"
expires_at="None"
id="00cf1f149fd0463994ad62ec0939ec71"
impersonation="False"
project_id="cf31bbe1ab4e4135a83b1b923d733b0d"
redelegation_count="0"
remaining_uses="None"
roles="admin"
trustee_user_id="e057d0ac9c394f5c833c6a746d76bc17"
trustor_user_id="b0fa819e150a4005a10ebdfc9d1b97d4"

$ openstack trust show 0ae2d20042d6418b8f3b8832ab43360e -f shell
deleted_at="None"
expires_at="None"
id="0ae2d20042d6418b8f3b8832ab43360e"
impersonation="False"
project_id="cf31bbe1ab4e4135a83b1b923d733b0d"
redelegation_count="0"
remaining_uses="None"
roles="admin"
trustee_user_id="e057d0ac9c394f5c833c6a746d76bc17"
trustor_user_id="b0fa819e150a4005a10ebdfc9d1b97d4"

[Steve Martinelli (stevemar)]

is there no way of setting the value dynamically? and setting it to the ID value that the trust create call returns?

[Richard Megginson (rmeggins)]

According to the work done by Sofer: http://lists.openstack.org/pipermail/openstack-dev/2015-September/075873.html

We don't need to worry about the actual 'name' of the keystone trust resource in puppet, as long as it is unique.

What we really need to know is some combination of keystone trust properties that, taken together, uniquely identify a trust.

Dolph: https://bugs.launchpad.net/keystone/+bug/1475091/comments/6

Are you saying that if we have specified in puppet:

  keystone_trust {'some trust for some user':
    trustor => trustor_user_id,
    trustee => trustee_user_id,
    project => project_id,
    impersonation => impersonationvalue,
    expires_at => expires_at_timestamp,
  }

And we do an `openstack trusts list`, and we find a trust that exactly matches the values of trustor_user_id, trustee_user_id, project_id, impersonationvalue, and expires_at_timestamp, can we be 100% certain that these trusts are the same, that the values of trustor_user_id, trustee_user_id, project_id, impersonationvalue, and expires_at_timestamp taken together uniquely identify a trust?

Or, is it possible for there to be two different trusts in keystone that have the same values for trustor_user_id, trustee_user_id, project_id, impersonationvalue, and expires_at_timestamp?

If trustor_user_id, trustee_user_id, project_id, impersonationvalue, and expires_at_timestamp do not uniquely identify a trust, is there some other combination of fields that, taken together, will uniquely identify a trust?

If there is no such combination, then we are just going to have to create some artificial limitations in puppet. For example, we will just have to pick some combination of fields and say that these identify a trust. Then, if puppet detects two different trusts with the same values for these fields, puppet will just have to pick one, and issue a warning that there may be duplicate trusts in keystone.

Worst case, if the user is doing something really bizarre, she/he may not be able to use puppet to manage trusts.

[Gilles Dubreuil (gdubreui)]

@stevemar,

Not sure I'm following you, with that dynamic value.
I guess if there was a way to assign an ID in advance then we could use that but that's not possible.

[Steve Martinelli (stevemar)]

Rich & Gilles,

I think creating an ID based on: `trustor_user_id, trustee_user_id, project_id, impersonation, expires_at, and ROLES` might just work.

Yes, this does not guarantee uniqueness; but at the same time, this does indicate duplication. There is no reason for both of these trusts to exist at the same time, since they are doing the same thing. If the user were to create two of these, then grabbing the first one detected would be OK since they would effectively do the same thing.

Note that roles are also passed in, as per the spec:
http://specs.openstack.org/openstack/keystone-specs/api/v3/identity-api-v3-os-trust-ext.html#create-trust

[Steve Martinelli (stevemar)]

Morgan, Gilles and I spoke about this on IRC. I think we agreed that making the columns in the trust database have a unique constraint is the way to go. So if two requests came to create trusts with the same trustor/trustee/roles/expiration/scope/impersonation, then the second would bring up an exception saying there's a conflict.

[Dolph Mathews (dolph)]

I changed the bug title to match the new direction of this bug. The bug title was previously very much a wishlist request, but the new bug title should accurately reflect the Medium-impact problem without prescribing a solution.

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/239114
Committed: https://git.openstack.org/cgit/openstack/keystone/commit/?id=59b09b50ff15df9975832dbfba42e0c984591e48
Submitter: Jenkins
Branch: master

commit 59b09b50ff15df9975832dbfba42e0c984591e48
Author: Kent Wang <email address hidden>
Date: Fri Oct 23 05:58:13 2015 -0700

    Add Trusts unique constraint to remove duplicates

    For now, effectively there could be multiple trusts with the same
    project, trustor, trustee, expiry date, impersonation. The same
    combination can have multiple trusts assigned with different roles
    or not.

    Patch fixes this issue by adding unique constraint to the trusts
    database model. If two requests create trusts with the same
    trustor, trustee, project, expiry, impersonation, then the second
    request would bring up an exception saying there's a conflict.

    This can help to improve specific trusts identification and
    improve user experience.

    Change-Id: I1a681b13cfbef40bf6c21271fb80966517fb1ec5
    Closes-Bug: #1475091

[Thierry Carrez (ttx)]

This issue was fixed in the openstack/keystone 9.0.0.0b2 development milestone.