Skip to content

OpenStack authentication token cache#1557

Merged
asfgit merged 4 commits intoapache:trunkfrom
godaddy:token-cache
Nov 3, 2021
Merged

OpenStack authentication token cache#1557
asfgit merged 4 commits intoapache:trunkfrom
godaddy:token-cache

Conversation

@dpeschman
Copy link
Copy Markdown
Contributor

@dpeschman dpeschman commented Feb 26, 2021

OpenStack authentication token cache

Description

This change adds support to the OpenStack drivers for saving authentication tokens in an external cache that can be shared among multiple processes. This is to address the performance degradation in Keystone that can occur when tokens are allocated quickly and not reused (see #1460).

Parameter ex_auth_cache is added to the OpenStack drivers. If supplied, this cache will be queried for an active authentication token prior to requesting a new one from OpenStack. When a new token is acquired from OpenStack, it is inserted into the cache for later reuse.

In the event that a token is revoked, causing a 401 Unauthorized response while using it, that token is removed from the cache to prevent future uses.

Status

  • done, ready for review

Checklist (tick everything that applies)

  • Code linting (required, can be done after the PR checks)
  • Documentation
  • Tests
  • ICLA (required for bigger changes)

@Kami
Copy link
Copy Markdown
Member

Kami commented May 2, 2021

Sorry for not getting to this yet - it's slightly larger PR so it will take a bit more time.

Having said that - could you also add a short entry + example to the documentation on how to use this functionality?

Copy link
Copy Markdown
Member

@Kami Kami left a comment

Choose a reason for hiding this comment

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

Sorry for the delay.

Thanks for adding some docs and working on this change - LGTM.

@Kami
Copy link
Copy Markdown
Member

Kami commented Oct 22, 2021

@dpeschman I had a look and it appears you haven't filled an ICLA yet.

Could you please look into filling an ICLA when you get a chance - https://www.apache.org/licenses/icla.pdf? Thanks.

@dpeschman
Copy link
Copy Markdown
Contributor Author

dpeschman commented Oct 22, 2021

Thanks @Kami . Reviewing the ICLA now.

One thing I notice looking over the code once more - this change (need to expand openstack_identity.py) to the exception message seems like it might break backwards compatibility. The rest of the change is backwards compatible, and I did not need this exception text changed for any particular reason, was just a grammar fix. So I can revert that part if it is desirable.

@Kami
Copy link
Copy Markdown
Member

Kami commented Oct 22, 2021

@dpeschman I think you can leave it as is, but please add a note about this change in the upgrade notes document (docs/upgrade_notes.rst).

@dpeschman
Copy link
Copy Markdown
Contributor Author

@dpeschman I think you can leave it as is, but please add a note about this change in the upgrade notes document (docs/upgrade_notes.rst).

Done!

@Kami
Copy link
Copy Markdown
Member

Kami commented Oct 22, 2021

@dpeschman Thanks. I will go ahead and merge it as soon as ASF secretary notifies the project that an ICLA has been received.

@tarkatronic
Copy link
Copy Markdown

@dpeschman Thanks. I will go ahead and merge it as soon as ASF secretary notifies the project that an ICLA has been received.

Hi @Kami! We are working on getting a CCLA set up for GoDaddy, with @dpeschman as a designated employee. Is there anything specific we need to include, aside from his name, to make sure it goes through smoothly?

@Kami
Copy link
Copy Markdown
Member

Kami commented Oct 22, 2021

@tarkatronic Great.

For an ICLA, person can specify to notify a project, but I believe CCLA form has no such field. So probably the best thing to do is to ping us here once you receive a response from ASF secretary that a CCLA has been processed and filled.

Thanks.

@tarkatronic
Copy link
Copy Markdown

@Kami I just received a response from the secretary; the CCLA for GoDaddy.com, LLC has been officially filed! 🎉

@dpeschman Looks like you will need to update your branch and resolve conflicts.

@dpeschman
Copy link
Copy Markdown
Contributor Author

Thanks @tarkatronic! I won't have access to a printer until (for signing the ICLA) until Nov 5. Will sign and submit then.

@Kami
Copy link
Copy Markdown
Member

Kami commented Nov 3, 2021

@dpeschman Thanks. I can confirm ICLA has been received, I will go ahead and merge this PR.

asfgit pushed a commit that referenced this pull request Nov 3, 2021
@asfgit asfgit merged commit 46454a3 into apache:trunk Nov 3, 2021
d-mo pushed a commit to mistio/libcloud that referenced this pull request Nov 15, 2021
d-mo pushed a commit to mistio/libcloud that referenced this pull request Nov 19, 2021
Kami added a commit to Kami/libcloud that referenced this pull request Mar 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants