Skip to content
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

Dual Write: Generate replication events for Role endpoints #1172

Merged
merged 63 commits into from
Oct 11, 2024

Conversation

lpichler
Copy link
Contributor

@lpichler lpichler commented Aug 20, 2024

  • Class RelationApiDualWriteHandler was added. This class containers has methods which performs Replication Event generation
  • methods:
    • generate_relations_from_current_state_of_role - this generate relations from current stage of role only from PRBAC DB- it uses mappings tables to get v2 role and role bindings uuids
    • generate_relations_and_mappings_for_role - this generates new relations for role - it generates new UUID for v2 role and role bindings
    • regenerate_relations_and_mappings_for_role - generate_relations_and_mappings_for_role
    • build_replication_event - generates json representation of relation which are sent to outbox table - see replication event bellow
    • save_replication_event_to_outbox - store relation event into outbox table (content of this method is not added yet)
    • generate_replication_event_to_outbox - regenerate_relations_and_mappings_for_role and save_replication_event_to_outbox

Each role action has mix of those methods:
CREATE:

  • create role from input to DB
  • generate_relations_and_mappings_for_role
  • save_replication_event_to_outbox

UPDATE:

  • generate_relations_from_current_state_of_role
  • update role based on input in DB
  • generate_replication_event_to_outbox

DELETE:

  • generate_relations_from_current_state_of_role
  • delete role in DB
  • save_replication_event_to_outbox

  • Replication Event is not stored into outbox table, PR with table is not merged yet- PR
  • Migration tool and mappings update: Columns v2 role uuid and permissions where added into BindingMapping table ,
    this information are required to identification of role and role bindings
  • Control Variable REPLICATION_TO_RELATION_ENABLED to enabled and disable replication - Replication is disable by default except to unit tests

Next Steps: Missing Endpoints

Replication Event (which will be stored in outbox table as payload):

{
  "relations_to_add": [
    {
      "resource": {
        "type": "role_binding",
        "id": "049fd753-4fe2-4d70-bc56-c59a68d749df"
      },
      "relation": "granted",
      "subject": {
        "type": "role",
        "id": "a1cb1ab4-0443-4257-b576-2eec75079d3d"
      }
    },
    {
      "resource": {
        "type": "role",
        "id": "a1cb1ab4-0443-4257-b576-2eec75079d3d"
      },
      "relation": "app_all_read",
      "subject": {
        "type": "user",
        "id": "*"
      }
    },
  ],
  "relations_to_remove": [
    {
      "resource": {
        "type": "keya/id",
        "id": "valueA"
      },
      "relation": "user_grant",
      "subject": {
        "type": "role_binding",
        "id": "bf8e6f6b-19b6-446a-81e4-d48a9b694bd4"
      }
    }]
}

Links

https://issues.redhat.com/browse/RHCLOUD-34786
https://issues.redhat.com/browse/RHCLOUD-34509

@lpichler lpichler force-pushed the dual_write_for_role branch 12 times, most recently from 34abb11 to 9b5a83f Compare August 21, 2024 16:11
@lpichler lpichler changed the title [WIP] Dual Write: Generate replication event for Role endpoints Dual Write: Generate replication event for Role endpoints Aug 21, 2024
@lpichler lpichler changed the title Dual Write: Generate replication event for Role endpoints Dual Write: Generate replication events for Role endpoints Aug 21, 2024
@lpichler lpichler force-pushed the dual_write_for_role branch 10 times, most recently from a8185c5 to 88ba65f Compare August 22, 2024 12:20
@lpichler
Copy link
Contributor Author

/retest

@@ -371,7 +394,7 @@ def update(self, request, *args, **kwargs):
@apiParam (Path) {String} id Role unique identifier

@apiParam (Request Body) {String} name Role name
@apiParam (Request Body) {Array} access Access definition
@apiParam (Request Body) {ArRray} access Access definition
Copy link
Collaborator

Choose a reason for hiding this comment

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

Might be a small nitpick not sure if the docstrings affect things here but should ArRray just be Array?

Comment on lines 140 to 143
id = models.UUIDField(default=uuid4, primary_key=True)
v1_role = models.ForeignKey(Role, on_delete=models.CASCADE)
v2_role = models.ForeignKey(V2Role, on_delete=models.CASCADE)
permissions = ArrayField(models.CharField(max_length=200), blank=True, null=True)

This comment was marked as resolved.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Also note that since most operations are performed in their own isolated transaction (autocommit / not inside atomic()) then I think the perf overhead of repeatable read (or serializable for that matter) should be negligible (however you will also not see any of the benefits of the increased isolation level in those cases–it will only matter to atomic() blocks).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Option 2 was added.


def prepare_to_delete_group(self):
"""Generate relations to delete."""
roles = Role.objects.filter(policies__group=self.group)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Need to add a check if replication is enabled here

@lpichler
Copy link
Contributor Author

/retest

Copy link
Collaborator

@alechenninger alechenninger left a comment

Choose a reason for hiding this comment

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

IMO this is ready to ship

@alechenninger alechenninger merged commit cfea2f6 into master Oct 11, 2024
9 checks passed
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.

6 participants