Add CTST tests for storage usage reporting API#2340
Add CTST tests for storage usage reporting API#2340delthas wants to merge 13 commits intodevelopment/2.14from
Conversation
Hello delthas,My role is to assist you with the merge of this Available options
Available commands
Status report is not available. |
Waiting for approvalThe following approvals are needed before I can proceed with the merge:
|
3f908f3 to
eee9043
Compare
42df742 to
1ef0c43
Compare
54b4acc to
4e692d3
Compare
Waiting for approvalThe following approvals are needed before I can proceed with the merge:
The following reviewers are expecting changes from the author, or must review again: |
4e692d3 to
0885b8e
Compare
Waiting for approvalThe following approvals are needed before I can proceed with the merge:
The following reviewers are expecting changes from the author, or must review again: |
0885b8e to
02d9543
Compare
Issue: ZENKO-5202
02d9543 to
77e25de
Compare
|
|
||
| Examples: | ||
| | locationName | | ||
| | us-east-1 | |
There was a problem hiding this comment.
- missing tests for "AccountOwner" persona (and multiple accounts): to verify he can see only his own account only
- missing tests for "StorageReporter" persona
There was a problem hiding this comment.
- AccountOwner: The
/reporting/usageendpoint doesn't support per-account permissions — authorization is all-or-nothing (403 fails fast before any account filtering). An AccountOwner test would behave identically to StorageManager (200 with all accounts) or get 403, neither of which tests account-scoped visibility. We'd need per-account filtering in pensieve-api first. - StorageReporter: No
StorageReporterKeycloak role exists in the test environment yet. On it
There was a problem hiding this comment.
Actually, for StorageReporter this needs a new user, which is a bit cumbersome in the current version of Zenko. This will change in https://github.com/scality/cli-testing/pull/96 / #2341
I suggest keeping this aside for now, and adding the user + tests after that Zenko PR is merged. Will be easier. I tested it end to end on my end.
There was a problem hiding this comment.
we can work incrementally, but:
- we need these extra tests to complete the work; so if we don't do this yet, the topic (and EPIC) stays open for longer.... esp. since most of the work was done so that we could have non-admin users with this capability...
- what is the ETA for that PR/change ? If this is a couple of hours or maybe days, why not ; but if there is no ETA we should really consider an alternative, less optimal, approach (i.e what is the actual cost of this "cumbersome-ness") and/or put more focus on that prerequisite change
- in that case need to "split" the current ticket, so we have a followup (and put it in sprint)
There was a problem hiding this comment.
The PR #2341 already has 2 approves, just a few remaining comments. Planned to be merged by early next week.
Ticket created at https://scality.atlassian.net/browse/ZENKO-5219 , assigned, added to sprint.
Replace the three scalar World keys (identityNameForScenario,
identityTypeForScenario, accountNameForScenario) with a typed
SavedIdentity[] array. This makes multiple identities naturally
queryable and eliminates manual additionalAccountNames tracking.
- Add SavedIdentity interface and getSavedIdentities/getSavedIdentity helpers
- saveIdentityInformation now appends to the array
- useSavedIdentity uses getSavedIdentity() (defaults to last entry)
- Migrate direct getSaved reads in bucket-policies and iam-policies
- Simplify "{int} additional accounts" step
- Drop additional accounts After cleanup hook
Issue: ZENKO-5202
Use the existing data_consumer Keycloak user (which has the DataConsumer role, not StorageManager) instead of introducing a dedicated norights user. This removes the KeycloakUsernameNoRights world parameter and reuses the existing DataConsumerUsername parameter. Issue: ZENKO-5202
Call prepareMetricsScenarios directly in the Before hook instead of going through an unnecessary indirection layer. Issue: ZENKO-5202
Add optional size parameter to putObject and uploadSetup so callers can pass the object size directly rather than routing it through the World saved-state dictionary. Issue: ZENKO-5202
… groups Collapse permission scenarios into a single parameterized Scenario Outline with role/expectedStatus examples. Separate content tests (structure, multi-account, metrics) into their own group. Replace two hardcoded When steps with a single parameterized step using a role-to-username lookup. Issue: ZENKO-5202
Disable camelcase rule for role-to-username map keys (must match Keycloak usernames). Shorten step names and parameter names to stay within 120-char line limit. Issue: ZENKO-5202
DarkIsDude
left a comment
There was a problem hiding this comment.
Can you check the CI ?
The roleToWorldParam lookup resolved role names to ctst_-prefixed usernames (e.g. ctst_storage_manager) which are not valid for direct Keycloak token acquisition. Pass the role string directly instead, matching how managementAPIRequest works elsewhere. Issue: ZENKO-5202
| Examples: | ||
| | role | expectedStatus | | ||
| | storage_manager | 200 | | ||
| | data_consumer | 403 | |
There was a problem hiding this comment.
what are really these "values" : the names of keycloak users? the keycloak role? the IAM role?
overal, I think this may actually be the name of the keycloak user, as defined in keycloak_config.json file ?
- we should probably rename these to make it clear what this is this →
storage_manager_identity, etc... - the is a fundamental inconsistency here, as we have hard-coded references here while we pass a KeycloakUsername in the Zenko World... To make/keep the tests portable, we will need to either handle some mapping in the world, or just create "our own" identities in the test (and just require a keycloak user to do this setup). Not sure how far we can go in this PR (certainly not the whole cleanup), but worth reviewing and checking we don't introduce more implicit coupling than there already is...
There was a problem hiding this comment.
Addressed: the Examples values are now World parameter keys (StorageManagerUsername, DataConsumerUsername) rather than raw Keycloak usernames, and the column is labeled persona. The step resolves them via this.parameters[persona] with a fallback for local dev.
There was a problem hiding this comment.
Reverted to raw Keycloak usernames (storage_manager, data_consumer). Here's why using World parameters doesn't work:
Two separate user populations exist in Keycloak:
-
Base users (
storage_manager,data_consumer,storage_account_owner) — created by importingkeycloak_config.jsonduring cluster setup. These have theinstanceIdsattribute (set inline in the JSON), which the management API (pensieve) requires for token validation. -
CTST users (
ctst_storage_manager,ctst_data_consumer, etc.) — created byseedKeycloak.shduring test setup. These do not haveinstanceIdsset. They're used for ARWWI (AssumeRoleWithWebIdentity) flows in S3/IAM tests, whereinstanceIdsisn't needed.
World parameters point to the wrong population:
StorageManagerUsername→ctst_storage_manager(noinstanceIds→ 401 from management API)DataConsumerUsername→ctst_data_consumer(same issue)KeycloakUsername→storage_manager(works, but only exists for one user)
There's no World parameter for the base data_consumer username.
The base usernames have no env var source either:
storage_manageris hardcoded inkeycloak_config.jsonline 81.OIDC_USERNAMEinend2end.yamlis a separate duplicate of the same value — it's used forkeycloak_user.json(admin user template) and the-norightsuser, not forkeycloak_config.json.data_consumerandstorage_account_ownerare hardcoded inkeycloak_config.jsonwith no env var at all.
So there's already duplication between end2end.yaml (OIDC_USERNAME: 'storage_manager') and keycloak_config.json ("username": "storage_manager"). Adding World parameters would just be a third copy of the same hardcoded strings with no actual decoupling.
Properly fixing this would mean either: (a) templating all usernames in keycloak_config.json through env vars, or (b) setting instanceIds on the ctst_ users too. Both are out of scope for this PR.
There was a problem hiding this comment.
I think for users we need a proper refactoring, the env vars + json config + ctst thing + cli testing is a bit all over the place. Maybe in a separate PR later?
The SavedIdentity refactor changed useSavedIdentity to read from an array, but getSavedIdentity returns undefined when no identity was saved. This crashed CountItems which calls useSavedIdentity via 'the operation finished without error' without prior identity setup. Match old behavior by returning early when no identity exists. Issue: ZENKO-5202
- Rename "role" to "persona" in feature file and step definitions
- Replace hardcoded Keycloak usernames with World parameter keys
- Add Given step for persona setup, decoupled from When action
- Split into "tries to retrieve" (no status check) and "retrieves" (asserts 200)
- Add generic "Then the http response code is {int}" step in common
- Remove section comments from feature file
Issue: ZENKO-5202
Kubernetes events expire after 1 hour by default. In CI, kind export logs often runs 1-2 hours after the test window, by which time the relevant events have already been garbage-collected. This was a concrete gap during the Azure archive flakiness investigation (PR #2340): the failing tests ran around 06:36 but kind export logs didn't execute until ~08:22 — nearly 2 hours later. All Kubernetes events from the failure window (pod scheduling, restarts, liveness probe failures, OOM kills) had expired and were unavailable for diagnosis. Bump --event-ttl to 4h via a kubeadm ClusterConfiguration patch in the Kind cluster config, ensuring events survive for the entire duration of CI jobs. Issue: ZENKO-5217
World parameter keys (StorageManagerUsername, DataConsumerUsername) resolve to ctst_-prefixed usernames which lack the instanceIds Keycloak attribute required by the management API. Use the base Keycloak usernames directly. Issue: ZENKO-5202
Summary
GET /instance/{uuid}/reporting/usagePensieve routemanagementAPIRequestwith optionalusername/passwordparams for testing different Keycloak usersprepareMetricsScenarioswithobjectSize/objectCountoptions for uploading non-empty test objectsIssue: ZENKO-5202