Skip replicating empty relations when deleting role

[astrozzc]

Link(s) to Jira

• https://issues.redhat.com/browse/RHCLOUD-36672

Description of Intent of Change(s)

The what, why and how.

Local Testing

How can the feature be exercised?
How can the bug be exploited and fix confirmed?
Is any special local setup required?

Checklist

• if API spec changes are required, is the spec updated?
• are there any pre/post merge actions required? if so, document here.
• are theses changes covered by unit tests?
• if warranted, are documentation changes accounted for?
• does this require migration changes?
  • if yes, are they backwards compatible?
• is there known, direct impact to dependent teams/components?
  • if yes, how will this be handled?

Secure Coding Practices Checklist Link

• https://github.com/RedHatInsights/secure-coding-checklist

Secure Coding Practices Checklist

• Input Validation
• Output Encoding
• Authentication and Password Management
• Session Management
• Access Control
• Cryptographic Practices
• Error Handling and Logging
• Data Protection
• Communication Security
• System Configuration
• Database Security
• File Management
• Memory Management
• General Coding Practices

[lpichler]

@astrozzc can you describe what is reason that relations are empty ? is because that role doesn't have any assignments ?

[astrozzc]

@astrozzc can you describe what is reason that relations are empty ? is because that role doesn't have any assignments ?

Yea, the created role has empty access, so there is no relations created for it, and no bindingmapping. When deleting it, there is no relationships to delete too, which explains why the delete/create has almost the same number.

[alechenninger]

Didn't take too close a look but if possible please add logging any time we are not replicating (info level is probably appropriate, I think that's what we do in some other places. Possibly warning if it's unexpected.)

[astrozzc]

Didn't take too close a look but if possible please add logging any time we are not replicating (info level is probably appropriate, I think that's what we do in some other places. Possibly warning if it's unexpected.)

Updated with logging. Using info to distinguish with the unexpected skipping replication events

[lpichler]

Overall idea looks to me.

My concern is whether we should put this logic in to dual write handler or in view/perform_ methods, build _expected_empty_relation_reason here pass it in dual write handler and into replicator.
Replicator will output log info message about the reason.

Reason is that it will not complicate dual write handler methods.

example:

        # check emptiness
        if not role.access.all():
           expected_empty_relation_reason = <build reason>
        dual_write_handler = RelationApiDualWriteHandler(instance, ReplicationEventType.DELETE_CUSTOM_ROLE)
         if not role.access.all():
            dual_write_handler.set_expected_empty_relation_reason_to_replicator(expected_empty_relation_reason)
            or
            dual_write_handler.replicator.set_expected_empty_relation_reason(expected_empty_relation_reason)
       else:
             dual_write_handler.prepare_for_update()

        self.delete_policies_if_no_role_attached(instance)
        instance.delete()

        dual_write_handler.replicate_deleted_role() # this will output only log message as replicator will react on set expected_empty_relation_reason
        role_obj_change_notification_handler(instance, "deleted", self.request.user)

it is just suggestion please let me know what do you think about it.

[lpichler]

/retest