Skip to content

feat: Add access rule CRD to appcred provider#806

Open
mashawes wants to merge 3 commits into
openstack-experimental:mainfrom
mashawes:appcred-access-rule
Open

feat: Add access rule CRD to appcred provider#806
mashawes wants to merge 3 commits into
openstack-experimental:mainfrom
mashawes:appcred-access-rule

Conversation

@mashawes

Copy link
Copy Markdown
Collaborator

Extends the application credential provider with access rule CRD (create, read, delete, no update), following the same layering as application credentials.

  • Adds two error variants in core-types, AccessRuleNotFound and AccessRuleInUse. The AccessRule and AccessRuleCreate types already existed, so no new domain types were needed.
  • Adds create, get, list, and delete to the appcred-sql driver under application_credential/access_rule/. Access rules are user-scoped, and the user-facing id maps to the external_id column. Delete refuses to remove a rule that is still attached to an application credential (AccessRuleInUse) rather than cascade-deleting it, since that would silently strip a live credential's restriction; this matches Python Keystone's behavior.
  • Wires the new methods through the ApplicationCredentialApi provider including backend, provider_api, service, mock, and mod. The provider validates the request and generates the rule id when none is supplied, so the driver only has to persist the final value.
  • Adds integration tests under tests/integration/src/application_credential/access_rule.rs covering the full create, get, list, and delete cycle, plus the not-found and in-use error cases.

Closes #776

Note: this contribution was developed with AI assistance.

@gtema gtema left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please sort methods in all traits and impls that you modify alphabetically sorted. Otherwise looks fine

Comment thread crates/appcred-driver-sql/src/lib.rs Outdated
Comment thread crates/core/src/application_credential/backend.rs
@mashawes mashawes requested a review from gtema June 16, 2026 14:33
@mashawes mashawes self-assigned this Jun 16, 2026

@gtema gtema left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://github.com/openstack-experimental/keystone/blob/main/crates/identity-driver-sql/src/local_user/create.rs#L44, https://github.com/openstack-experimental/keystone/blob/main/crates/identity-driver-sql/src/password/set.rs#L44 show that in some cases we need to invoke backend functions in the transaction context. For access rules it is definitely the case where create/delete operations would be bound to the parenting application_credential create/delete. This is a small interface change that will not even require unittest changes. Otherwise fine

Comment thread crates/appcred-driver-sql/src/application_credential/access_rule/create.rs Outdated
Comment thread crates/core/src/application_credential/service.rs Outdated
@mashawes mashawes requested a review from gtema June 16, 2026 21:45

@gtema gtema left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one, hopefully final thing. Please add "user_id" into the AccessRule core-types and correspondingly expose it from the database. It would be populated during creation and is definitely necessary in output. The AccessRule struct already contain a comment that in the DB user_id must be made not optional, but we can't do this in the keystone-rs project

path: Set(rule.path),
service: Set(rule.service),
external_id: Set(Some(rule.id.unwrap_or(Uuid::new_v4().simple().to_string()))),
user_id: Set(Some(user_id.as_ref().to_string())),

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should extend the AccessRuleCreate to have user_id: String

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add application_credentials.access_rule CRUD support in the provider

2 participants