Cache public tenant

[coderbydesign]

Link(s) to Jira

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

Description of Intent of Change(s)

QUESTIONS

• What should the default TTL be/do we need to expire it?
• Should we refactor the existing TenantCache to make it more generic, or keep it separate?

There are several places in the code we query the public tenant. We should DRY this process and also optimize the lookups by caching the tenant.

Add method to retrieve the public tenant

This adds a method on the Tenant model manager, get_public_tenant() which can
be called with Tenant.objects.get_public_tenant().

The public tenant record will be added to/retrieved from the cache. A new cache
inheriting from TenantCache was added to allow for a custom TTL and key to be
used for the public tenant (open to feedback here, I just didn't want to refactor
too much initially).

Add the PublicTenantCache and TTL config

This adds a configurable PUBLIC_TENANT_CACHE_LIFETIME defaulted to 24hrs to
be used on the new cache object, allowing for some inheritance from TenantCache
while also having a custom TTL and key for retrieving and setting based on the
name, since the public tenant has no org.

Use the model manager to query for the public tenant

Update instances of Tenant.objects.get(tenant_name="public") to use the new
method on the Tenant model manager.

Local Testing

Start the service, and after hitting an endpoint, check redis:

$ redis-cli -n 1
> TTL "rbac::tenant::tenant=public"

Note that this k/v should exist, and you should see a TTL of ~24 hours.

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

[astrozzc]

LGTM, not sure if the TTL is useful though, I can't think of a situation that we need to expire it

[coderbydesign]

@astrozzc yeah, that's fair and what I was thinking as well. Wasn't sure if there would be any situation where things might get into a weird state, etc., and wanted to get feedback on that (hence the question in the PR around TTL for this).

[lpichler]

@astrozzc yeah, that's fair and what I was thinking as well. Wasn't sure if there would be any situation where things might get into a weird state, etc., and wanted to get feedback on that (hence the question in the PR around TTL for this).

This PR looks good and we basically use only id of public tenant in queries - so it also doesn't look like that TTL is needed.

1. Bypass the Redis cache ?
   Given that the public tenant doesn't change at all, maybe it is possible to bypass the Redis cache entirely when retrieving the public tenant.
   If so, it will be enough to store public tenant in some dict at startup of rbac app or maybe as static variable on tenant model. Public tenant will be stored in memory in each pod but it is not too much data.

Example of using static variable(code was not run):

# In tenant model 

@staticmethod
 def _get_public_tenant() -> Tenant:
        if self._public_tenant is None:
            self._public_tenant = Tenant.objects.get(tenant_name="public")
        return self._public_tenant

2. Instantiate tenant model.
   How is tenant object instantiated from redis ? Is there some form of (de)serialization ?
   Maybe it is possible to just work with public tenant id in queries instead of passing whole tenant object in queries:

roles.filter(tenant_id=Tenant.public_tenant_id())

@coderbydesign @astrozzc What do you think about it ? Do you know about anything what would not work with this approach ?