Skip to content

[patch] Catch SSLError and RequestException in validateIBMEntitlementKey, return None instead of raising - #497

Open
Jeel-Oza wants to merge 2 commits into
stablefrom
mascore-16263
Open

Jeel-Oza wants to merge 2 commits into
stablefrom
mascore-16263

Conversation

@Jeel-Oza

Copy link
Copy Markdown
Contributor

Summary

Fixes validateIBMEntitlementKey() re-raising network exceptions to its caller, which forced the CLI layer to handle low-level network exceptions directly.

Changes

src/mas/devops/utils.py

  • validateIBMEntitlementKey() previously caught requests.exceptions.RequestException and re-raised it — now catches requests.exceptions.SSLError specifically and returns None, and catches all other requests.exceptions.RequestException and returns None instead of re-raising
  • Updated return type from bool to True/False/None in docstring
  • Removed Raises: section from docstring — the function no longer raises

Return value contract:

Return Meaning
True Key is valid, access confirmed
False Authentication failed — key is wrong or expired
None SSL or network error — key status unknown, may still be valid

Testing

Reproduced both scenarios manually inside the MAS CLI container.

Case 1 — SSL error (corporate proxy / self-signed certificate)

# Generate a fake CA cert that Python will not trust for cp.icr.io
openssl req -x509 -newkey rsa:2048 -keyout /tmp/fake-ca.key \
  -out /tmp/fake-ca.crt -days 1 -nodes \
  -subj "/CN=FakeCA"

# Force requests to use the fake bundle
export REQUESTS_CA_BUNDLE=/tmp/fake-ca.crt

# Run the CLI and enter any entitlement key
mas install

Result: Warning: SSL certificate verification failed — could not reach cp.icr.io with explanation that the key may still be valid and options to try again / continue / quit

Screenshot 2026-09-18 at 11 11 31 PM Screenshot 2026-09-18 at 11 10 52 PM

Case 2 — Wrong entitlement key

Result: Warning: IBM entitlement key validation failed with options to try again / continue / quit

Screenshot 2026-09-18 at 11 14 23 PM

Linked issue

Fixes MASCORE-16263
ibm-mas/cli#2519

@Jeel-Oza
Jeel-Oza requested a review from terc1997 September 18, 2026 18:34
@Jeel-Oza
Jeel-Oza requested a review from a team as a code owner September 18, 2026 18:34

@terc1997 terc1997 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I liked the approach, however return None for both exceptions can hide bugs since everything else in Requests also returns None. Instead of returning None I have two proposals:

  • Unpack two returns. One to identify if it was valid or not and another to specify if the failure was due to SSL.
  • Return None for SSL and raise for everything else in Requests. We're saying that the SSL error is acceptable and can be a warning and everything else needs to be checked, which complies with what we had before and add just the warning layer around SSL.

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.

2 participants