Skip to content

test(maas): update maas sub enforcement tests to use API key#1199

Merged
threcc merged 2 commits intoopendatahub-io:mainfrom
SB159:feature/update-sub-tc-api-key
Mar 11, 2026
Merged

test(maas): update maas sub enforcement tests to use API key#1199
threcc merged 2 commits intoopendatahub-io:mainfrom
SB159:feature/update-sub-tc-api-key

Conversation

@SB159
Copy link
Copy Markdown
Contributor

@SB159 SB159 commented Mar 10, 2026

Pull Request

Summary

Update MaaS subscription enforcement tests to use API key

Related Issues

  • Fixes:
  • JIRA:

How it has been tested

  • Locally
  • Jenkins

Additional Requirements

  • If this PR introduces a new test image, did you create a PR to mirror it in disconnected environment?
  • If this PR introduces new marker(s)/adds a new component, was relevant ticket created to update relevant Jenkins job?

Summary by CodeRabbit

  • Tests
    • Refactored subscription enforcement test suite with improved header construction and test parameter handling.
    • Updated test markers for better test categorization and organization.

@github-actions
Copy link
Copy Markdown

The following are automatically added/executed:

  • PR size label.
  • Run pre-commit
  • Run tox
  • Add PR author as the PR assignee
  • Build image based on the PR

Available user actions:

  • To mark a PR as WIP, add /wip in a comment. To remove it from the PR comment /wip cancel to the PR.
  • To block merging of a PR, add /hold in a comment. To un-block merging of PR comment /hold cancel.
  • To mark a PR as approved, add /lgtm in a comment. To remove, add /lgtm cancel.
    lgtm label removed on each new commit push.
  • To mark PR as verified comment /verified to the PR, to un-verify comment /verified cancel to the PR.
    verified label removed on each new commit push.
  • To Cherry-pick a merged PR /cherry-pick <target_branch_name> to the PR. If <target_branch_name> is valid,
    and the current PR is merged, a cherry-picked PR would be created and linked to the current PR.
  • To build and push image to quay, add /build-push-pr-image in a comment. This would create an image with tag
    pr-<pr_number> to quay repository. This image tag, however would be deleted on PR merge or close action.
Supported labels

{'/wip', '/hold', '/cherry-pick', '/lgtm', '/verified', '/build-push-pr-image'}

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 10, 2026

📝 Walkthrough

Walkthrough

Test file refactoring replaces build_maas_headers() function calls with direct fixture usage, renames the test parameter from ocp_token_for_actor to maas_headers_for_actor_api_key, and updates pytest markers from sanity to smoke/tier1.

Changes

Cohort / File(s) Summary
Test Fixture Refactoring
tests/model_serving/model_server/maas_billing/maas_subscription/test_maas_sub_enforcement.py
Replaces build_maas_headers() function calls with direct maas_headers_for_actor_api_key fixture usage. Renames test parameter and updates pytest markers (sanity → smoke/tier1). Manual header injection in tests via dictionary copy operations.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes


Actionable Issues

  1. Validation Logic Bypass (CWE-345: Insufficient Verification of Data Authenticity) — If build_maas_headers() performed header validation or sanitization, replacing it with direct fixture usage circumvents that check. Verify the fixture maas_headers_for_actor_api_key applies equivalent validation before assertion in tests.

  2. Header Injection Without Security Context — Tests manually inject MAAS_SUBSCRIPTION_HEADER into headers copied from the fixture. Confirm these injected headers bypass no authentication/authorization checks that would apply in production request flow.

  3. Parameter Semantics Change — Renaming ocp_token_for_actor to maas_headers_for_actor_api_key obscures the original token source. Verify the fixture construction doesn't conflate different token sources (e.g., OCP vs API key) that should be handled distinctly for security.

🚥 Pre-merge checks | ✅ 2
✅ Passed checks (2 passed)
Check name Status Explanation
Description check ✅ Passed The description matches the required template structure with Summary, Related Issues, How it has been tested, and Additional Requirements sections, though Related Issues sections lack substantive content.
Title check ✅ Passed The PR title directly and accurately reflects the main change: updating MaaS subscription enforcement tests to use API key instead of the previous build_maas_headers function.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@SB159 SB159 changed the title Update MaaS subscription enforcement tests to use API key test(maas): update maas sub enforcement tests to use API key Mar 10, 2026
@threcc threcc enabled auto-merge (squash) March 11, 2026 13:47
@threcc threcc merged commit 83b8d99 into opendatahub-io:main Mar 11, 2026
8 checks passed
@github-actions
Copy link
Copy Markdown

Status of building tag latest: success.
Status of pushing tag latest to image registry: success.

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