Skip to content

[WIP] [AAP-48392] Models and APIs for tracking remote permissions in DAB RBAC #749

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 71 commits into
base: devel
Choose a base branch
from

Conversation

AlanCoding
Copy link
Member

@AlanCoding AlanCoding commented Jun 30, 2025

Description

This modifies the RBAC app so that the models can store permissions for remote objects - objects that don't actually exist in the local server. To know which are which, a service field is added to our type-tracking model, which is also new as of this work. Importantly, permission evaluations can be done for both local items and remote items.

Why? Just as we have synchronization to a "resource server" via the resource registry app, this allows you to appoint a single service to be the gatekeeper for RBAC. This still requires synchronization, making it different from other approaches. Several new endpoints under /service-index/ are introduced to help facilitate that synchronization.

EDITing some snapshots of the progress state

  • As of opening, the core code is not finished being written, just want to get CI output continuously.
  • As of July 14 - relatively stable with current AWX, but integration work for rest of components is still WIP

Fixes #80

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Test update
  • Refactoring (no functional changes)
  • Development environment change
  • Configuration change

Self-Review Checklist

  • I have performed a self-review of my code
  • I have added relevant comments to complex code sections
  • I have updated documentation where needed
  • I have considered the security impact of these changes
  • I have considered performance implications
  • I have thought about error handling and edge cases
  • I have tested the changes in my local environment

Testing Instructions

Prerequisites

Steps to Test

Expected Results

Additional Context

Required Actions

  • Requires documentation updates
  • Requires downstream repository changes
  • Requires infrastructure/deployment changes
  • Requires coordination with other teams
  • Blocked by PR/MR: #XXX

Screenshots/Logs

@AlanCoding AlanCoding changed the title [WIP] Model changes for tracking remote permissions in DAB RBAC AAP-48392 [WIP] Model changes for tracking remote permissions in DAB RBAC Jul 1, 2025
AlanCoding added a commit to ansible/eda-server that referenced this pull request Jul 2, 2025
…1355)

This is another version of
#1353, but with the dependency
changes reverted.

That linked PR (draft) shows that this is effective. I expect tests will
pass here (which current DAB) as well. But that PR shows that, if we
merge this, it will make downstream tests for
ansible/django-ansible-base#749 will pass.

----

Technical summary:

There are 2 structural changes in DAB RBAC that require adjustment by
the app:
- the `post_migrate` signal will now run _once_ as opposed to run _for
every app_ and to do that, it can only run when the post migrate signal
is called for its own app (the "dab_rbac" app), but doing this mucks
with the assumptions around what order post_migrate methods run in, so
this often requires other post-migrate methods to call the methods to
create DAB types and permissions to resolve the ordering problem
- A DAB RBAC-specific content type app is introduced, and this is
clearly not the same as the proper `ContentType` model, and this will
error any queries that pass a content type object as a python object. To
do that, we'll just use the variable from DAB RBAC for the content type,
which will give the correct model for whatever version of DAB we are
using.
@AlanCoding
Copy link
Member Author

AlanCoding commented Jul 2, 2025

Documenting the eda-server test failure I'm troubleshooting, log:

2025-07-02 19:16:07,056 WARNING  ansible_base.rbac.models.content_type Could not find content type for ('eda', 'core', 'awxtoken'), so creating new, out of:
dict_keys([('eda', 'core')])

failure:

FAILED tests/integration/api/test_activation_with_event_stream.py::test_create_activation_with_event_stream - django.db.utils.IntegrityError: duplicate key value violates unique constraint "dab_rbac_dabcontenttype_pkey"
DETAIL:  Key (id)=(1) already exists.

It tried to create at:

  File "/home/runner/work/django-ansible-base/django-ansible-base/ansible_base/rbac/models/role.py", line 82, in give_creator_permissions
    cts = DABContentType.objects.get_for_models(*model_and_children).values()
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/runner/work/django-ansible-base/django-ansible-base/ansible_base/rbac/models/content_type.py", line 145, in get_for_models
    ct = self.create(
         ^^^^^^^^^^^^

I might have ran into the indexing bug, because I set a pk value in migrations for the types, but the current pk index for new objects does not increment in some databases. This might be a good lead.

Apparently this is called "sequence desynchronization"

@AlanCoding
Copy link
Member Author

The higher-level error is that AwxToken is not a registered permission model

https://github.com/ansible/eda-server/blob/b6c03d41936fcce4a842760cf3f101c9903ca9df/src/aap_eda/core/models/__init__.py#L69

So that suggests that give_creator_permissions was, in some way, wrong to determine that it is a child model of something else. Or else the tests added it unexpectedly.

@AlanCoding AlanCoding force-pushed the remote_rbac branch 2 times, most recently from 12b210e to 47de239 Compare July 3, 2025 20:42
@AlanCoding AlanCoding requested a review from fosterseth July 8, 2025 00:34
@john-westcott-iv john-westcott-iv changed the title AAP-48392 [WIP] Model changes for tracking remote permissions in DAB RBAC [WIP] [AAP-48392] Model changes for tracking remote permissions in DAB RBAC Jul 10, 2025
@AlanCoding
Copy link
Member Author

Update on downstream tests:

FAILED awx/main/tests/functional/dab_rbac/test_dab_rbac_api.py::test_custom_read_role

with

ansible_base.resource_registry.models.resource.Resource.DoesNotExist: Resource matching query does not exist.

This is a novel failure, due to adding resource on to the RoleDefinition model. This needs a change in AWX which is not there yet. It is also extremely challenging to add that to AWX in a smooth way to keep tests passing. I am considering a new hack for that.

@AlanCoding AlanCoding changed the title [WIP] [AAP-48392] Model changes for tracking remote permissions in DAB RBAC [WIP] [AAP-48392] Models and APIs for tracking remote permissions in DAB RBAC Jul 15, 2025
@AlanCoding AlanCoding mentioned this pull request Jul 15, 2025
20 tasks
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
32 New issues
1 Security Hotspot
0.0% Coverage on New Code (required ≥ 80%)
B Reliability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

Copy link

DVCS PR Check Results:

PR appears valid (JIRA key(s) found)

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.

[RBAC] Feature to include permissions of external resources, with no corresponding local model
1 participant