Master fixes openshift - #43
Merged
Merged
Conversation
Adds URL change detection for cached client reuse and refetch, ensuring updated configuration is respected without stale connections. Refines token retrieval to handle redirects and error cases more robustly, improving authentication reliability. Updates caching keys to include connection parameters for better cache validity. Enhances related tests to cover new logic and edge cases.
… request connect() built a new requests.Session and performed a full OAuth round trip on every single API call, since do_request() reaches the session through a property that always called it. The _access_token and _token_expiry attributes were already declared but never used; they now hold the token for consts.TOKEN_VALIDITY (30 min). do_request() already clears _session on a 401, so an early expiry self-heals on the next request. The force argument now actually forces a refresh. Also drops the global urllib3.disable_warnings(InsecureRequestWarning) call, which silenced warnings process-wide for every other service and is no longer needed now that get_token() honors verify_ssl. Connection identity is now built in a single place, OpenshiftClient.cache_key(), and mirrored by OpenshiftProvider.connection_key(). Both the cache key helpers and the cached-client validity check use it, so they can no longer disagree: the previous check only compared the two URLs while the cache key covered five fields. Adds offline tests for token parsing, session/token reuse and refetch after invalidation, and for the client/provider connection keys staying in sync.
The merge with origin/master took master's version of the OpenShift client, provider and consts, which undid most of the session/token caching commit but kept part of connect(), leaving code that referenced things no longer present. connect() raised AttributeError on consts.TOKEN_VALIDITY and built the Authorization header with a second get_token() round trip. Restores TOKEN_VALIDITY, get_token() parsing the redirect Location with allow_redirects=False and verify_ssl honored, the provider connection_key() used both by the cached client check and the is_available key helper, and initialize() clearing the cached client on config change.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request improves the OpenShift client’s connection and token handling, enhances error management, and adds comprehensive tests to ensure reliability and correct caching behavior. The most important changes are grouped below.
OpenShift Client Improvements:
cache_key()method toOpenshiftClientto uniquely identify connection parameters, ensuring proper cache invalidation when configuration changes.connect(): sessions are now reused and tokens are refreshed only when necessary, with a fixed token validity period [1] [2].get_token()to raise specific exceptions and log errors more clearly.Testing Enhancements:
tests/services/openshift/fixtures.py, ensuring test consistency with the new cache key logic [1] [2].TestOpenshiftClientTokentest class for offline testing of token retrieval, session reuse, and token/session invalidation logic.Provider Integration Testing:
These changes collectively improve the reliability, maintainability, and test coverage of the OpenShift integration.