Skip to content

Commit 48aba18

Browse files
jrhynessclaude
andauthored
feat: add comprehensive e2e subscription flow tests with group support (opendatahub-io#468)
## Description Add 11 new end-to-end tests in TestE2ESubscriptionFlow that validate the complete subscription flow for MaaS, including both user-based and group-based access patterns. New E2E Tests (TestE2ESubscriptionFlow): - User-based access (8 tests): * Both auth and subscription → 200 OK * Auth but no subscription → 403 Forbidden * Subscription but no auth → 403 Forbidden * Single subscription auto-select → 200 OK * Multiple subscriptions without header → 403 Forbidden * Multiple subscriptions with valid header → 200 OK * Multiple subscriptions with invalid header → 403 Forbidden * Multiple subscriptions with inaccessible header → 403 Forbidden - Group-based access (3 tests): * Group auth and subscription → 200 OK * Group auth but no subscription → 403 Forbidden * Group subscription but no auth → 403 Forbidden Test Infrastructure Improvements: - Add validation helpers (_get_auth_policies_for_model, _get_subscriptions_for_model) to discover what resources exist and improve debugging - Add setup_class validation that fails fast with clear error messages if prerequisites are missing (better CI feedback than skipping tests) - Add _wait_for_maas_model_ready helper that waits for model reconciliation and returns the actual endpoint URL - All tests are self-contained: create their own resources, temporarily remove shared resources when needed to isolate test conditions, and restore everything in cleanup Supporting Fixes: - Fix smoke.sh: export GATEWAY_HOST so tests can run standalone - Fix auth_utils.sh: correct DNS name extraction bug that caused "maas-api.null.svc.cluster.local" errors - Fix 6 pre-existing tests that incorrectly assumed admin user had pre-configured access to models - Fix test_delete_last_subscription: expect 403 (not 200) based on actual system behavior when no subscriptions exist - Update README: fix example test name and improve documentation All tests handle Authorino's OR logic correctly (multiple auth policies/ subscriptions) by temporarily removing shared resources when testing negative cases, ensuring proper test isolation. ## How Has This Been Tested? Run the test suite using a ROSA cluster ## Merge criteria: <!--- This PR will be merged by any repository approver when it meets all the points in the checklist --> <!--- Go over all the following points, and put an `x` in all the boxes that apply. --> - [x] The commits are squashed in a cohesive manner and have meaningful messages. - [x] Testing instructions have been added in the PR body (for PRs involving changes that are not immediately obvious). - [x] The developer has manually tested the changes and verified that the changes work <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Added a "Running Tests Locally" guide covering environment setup, variables, virtualenv, dependency install, pytest usage, CI notes, and smoke-test steps. * **Tests** * Greatly expanded end-to-end subscription and authorization test suite, covering header selection, multiple subscriptions, group/admin access, reconciliation, and error cases. * Added test helpers to create, wait for, query, and clean up test resources (model refs, auth policies, subscriptions) and token handling. * **Chores** * Improved test scripts: derive/export gateway host and simplified MAAS namespace handling. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
1 parent 7195c0e commit 48aba18

File tree

4 files changed

+1055
-41
lines changed

4 files changed

+1055
-41
lines changed

test/e2e/README.md

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,3 +21,49 @@ If MaaS is already deployed and you just want to run tests:
2121
```bash
2222
./test/e2e/smoke.sh
2323
```
24+
25+
## Running Tests Locally
26+
27+
### Run All Subscription Tests
28+
```bash
29+
export GATEWAY_HOST="maas.apps.your-cluster.example.com"
30+
export MAAS_NAMESPACE="opendatahub"
31+
32+
# Activate virtual environment
33+
cd test/e2e
34+
python3 -m venv .venv
35+
source .venv/bin/activate
36+
pip install -r requirements.txt
37+
38+
# Run all tests
39+
pytest tests/test_subscription.py -v
40+
41+
# Run specific test class (e2e subscription flow tests)
42+
pytest tests/test_subscription.py::TestE2ESubscriptionFlow -v
43+
44+
# Run specific test
45+
pytest tests/test_subscription.py::TestE2ESubscriptionFlow::test_e2e_with_both_access_and_subscription_gets_200 -v
46+
```
47+
48+
### Environment Variables
49+
50+
See `tests/test_subscription.py` docstring for all available environment variables. Key ones:
51+
52+
- `GATEWAY_HOST`: Gateway hostname (required)
53+
- `MAAS_NAMESPACE`: MaaS namespace (default: opendatahub)
54+
- `E2E_TEST_TOKEN_SA_NAMESPACE`, `E2E_TEST_TOKEN_SA_NAME`: Service account for token generation
55+
- `E2E_TIMEOUT`: Request timeout in seconds (default: 30)
56+
- `E2E_RECONCILE_WAIT`: Wait time for reconciliation in seconds (default: 8)
57+
58+
## CI Integration
59+
60+
These tests run automatically in CI via:
61+
- **Prow**: `./test/e2e/scripts/prow_run_smoke_test.sh` (includes subscription tests)
62+
- **GitHub Actions**: Can be integrated into `.github/workflows/` as needed
63+
64+
The `prow_run_smoke_test.sh` script:
65+
1. Deploys MaaS platform and dependencies
66+
2. Deploys test models (free + premium simulators)
67+
3. Runs subscription controller tests (`test_subscription.py`)
68+
4. Runs deployment validation and token metadata verification
69+
5. Collects artifacts (HTML/XML reports, logs) to `ARTIFACT_DIR`

test/e2e/scripts/auth_utils.sh

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ collect_cluster_state() {
116116
kubectl get tokenratelimitpolicies -A 2>/dev/null || true
117117
echo ""
118118
echo "--- MaaS CRs ---"
119-
kubectl get maasmodels,maasauthpolicies,maassubscriptions -n "$MAAS_NAMESPACE" 2>/dev/null || true
119+
kubectl get maasmodelrefs,maasauthpolicies,maassubscriptions -n "$MAAS_NAMESPACE" 2>/dev/null || true
120120
echo ""
121121
echo "--- HTTPRoutes ---"
122122
kubectl get httproutes -A 2>/dev/null | head -30 || true
@@ -210,7 +210,7 @@ run_auth_debug_report() {
210210
_section "MaaS CRs"
211211
_run "MaaSAuthPolicies" "kubectl get maasauthpolicies -n $MAAS_NAMESPACE -o wide 2>/dev/null || true"
212212
_run "MaaSSubscriptions" "kubectl get maassubscriptions -n $MAAS_NAMESPACE -o wide 2>/dev/null || true"
213-
_run "MaaSModels" "kubectl get maasmodels -n $MAAS_NAMESPACE -o wide 2>/dev/null || true"
213+
_run "MaaSModelRefs" "kubectl get maasmodelrefs -n $MAAS_NAMESPACE -o wide 2>/dev/null || true"
214214
_append ""
215215

216216
_section "Gateway / HTTPRoutes"
@@ -223,9 +223,8 @@ run_auth_debug_report() {
223223
_append ""
224224

225225
# Determine maas-api namespace
226-
local maas_api_ns
227-
maas_api_ns=$(kubectl get deployment maas-controller -n $MAAS_NAMESPACE -o jsonpath='{.spec.template.spec.containers[0].env}' 2>/dev/null | jq -r '.[] | select(.name=="MAAS_API_NAMESPACE") | .value' 2>/dev/null || echo "$MAAS_NAMESPACE")
228-
[[ -z "$maas_api_ns" ]] && maas_api_ns="$MAAS_NAMESPACE"
226+
# Note: MAAS_API_NAMESPACE uses valueFrom.fieldRef, so .value is null; use MAAS_NAMESPACE instead
227+
local maas_api_ns="$MAAS_NAMESPACE"
229228

230229
local sub_select_url="https://maas-api.${maas_api_ns}.svc.cluster.local:8443/v1/subscriptions/select"
231230
_section "Subscription Selector Endpoint Validation"

test/e2e/smoke.sh

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,13 @@ if [[ -z "${MAAS_API_BASE_URL}" ]]; then
6565
MAAS_API_BASE_URL="${SCHEME}://${HOST}/maas-api"
6666
fi
6767

68+
# Extract HOST from MAAS_API_BASE_URL if not already set
69+
if [[ -z "${HOST}" && -n "${MAAS_API_BASE_URL}" ]]; then
70+
HOST=$(echo "${MAAS_API_BASE_URL}" | sed -E 's|^[^:]+://([^/]+).*|\1|')
71+
fi
72+
6873
export HOST
74+
export GATEWAY_HOST="${HOST}" # Required by test_subscription.py
6975
export MAAS_API_BASE_URL
7076

7177
echo "[smoke] MAAS_API_BASE_URL=${MAAS_API_BASE_URL}"

0 commit comments

Comments
 (0)