Cover custom org roles CRUD in org integration suite#181
Open
sdairs wants to merge 3 commits into
Open
Conversation
efa7803 to
4fbc39a
Compare
Adds a Custom Roles CRUD phase to `integration_org_test.rs` that
exercises `organization_roles_get_list`, `organization_role_post`,
`organization_role_get`, `organization_role_patch` and
`organization_role_delete` end-to-end:
- Pre-list captures existing role ids for a sanity assertion that the
created role is genuinely new.
- Create uses `clickhousectl-it-role-{run_id}` so concurrent CI runs
don't collide. The role is registered with `CleanupRegistry` before
any further interaction so teardown still reclaims it on mid-phase
failure.
- Get + list-after-create check the role surfaces with the expected
name, custom type, and permissions.
- Patch replaces the policy permissions; a follow-up GET asserts the
change is observable to real callers.
- Delete unregisters from `CleanupRegistry` on success and verifies a
follow-up GET returns 404. On failure the registration stays put as
best-effort fallback for teardown.
All steps are `NonBlocking`; no downstream phase depends on the role.
Closes #154
Parent: #151
Stacked on #173.
The API rejected `organization/*` with `BAD_REQUEST: Organization * must
match the role's organization` — the wildcard form is only valid for
instance-scoped resources (the spec example is `instance/*`). For
org-scoped resources the API requires the literal org id.
Replaces `organization/*` with `format!("organization/{org_id}")` in
both the create and patch policy bodies.
The API rejects mixed-scope permissions inside a single policy ("All
permissions in a policy must target the same resource scope. Found
mixed scopes: organization, service. Split into separate policies per
scope."). The patch step was bundling an org-scoped permission and a
service-scoped permission in one policy.
Restructure the patch body to send two policies: one org-scoped (the
permission carried over from create) targeting `organization/{org_id}`,
and one service-scoped (the newly added permission) targeting
`instance/*`. The verify-via-GET step flattens permissions across all
policies, so its assertion logic is unchanged.
This also exercises the multi-policy code path, which the
single-policy version did not.
d8fcc23 to
4ec3d92
Compare
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.
Summary
Adds a Custom Roles CRUD phase to
integration_org_test.rscovering the five client methods called out in #154:organization_roles_get_list,organization_role_post,organization_role_get,organization_role_patch,organization_role_delete.clickhousectl-it-role-{run_id}so concurrent CI runs don't collide. Registered withCleanupRegistry::register_roleimmediately, before any further interaction, so a mid-phase failure still reclaims it via teardown.RBACRoleType::Custom, and the permissions sent in the POST body.CleanupRegistryon success and asserts a follow-up GET returns 404. On failure the registration stays put so teardown can still try as best-effort fallback.All steps are
NonBlockingso a single live run reports every broken endpoint in the phase, not just the first. No downstream phase depends on the role existing.Closes #154
Parent: #151
This PR is stacked on #173 (which delivered the
integration_org_test.rsskeleton andCleanupRegistry::register_role). GitHub will auto-retarget tomainwhen #173 merges.Test plan
cargo build -p clickhouse-cloud-apicleancargo clippy -p clickhouse-cloud-api --test integration_org_test -- -D warningsclean. Pre-existing clippy errors inintegration_test.rs(too_many_arguments) andspec_coverage_test.rs(collapsible_if) are unrelated and present on the base branch.cargo test -p clickhouse-cloud-api— all suites green;integration_org_teststill registers exactly one ignored lifecycle test.--ignoredrun via thecloud-integrationworkflow once this PR re-targetsmain.