[MTC-293] Add e2e test for Role and RoleBinding migration #267
Merged
midays merged 3 commits intomigtools:mainfrom Apr 16, 2026
Merged
[MTC-293] Add e2e test for Role and RoleBinding migration #267midays merged 3 commits intomigtools:mainfrom
midays merged 3 commits intomigtools:mainfrom
Conversation
Signed-off-by: midays@redhat.com <midays@redhat.com>
midays
added
the
do-not-merge/work-in-progress
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a new Ginkgo/Gomega E2E test that creates Role, RoleBinding, and ServiceAccount on a source cluster as a non-admin namespace-admin, runs the crane export/transform/apply pipeline, applies manifests to a target cluster (non-admin flows), and verifies migrated RBAC resources on the target. Changes
Sequence Diagram(s)sequenceDiagram
participant TestRunner as Test Runner
participant Source as Source Cluster
participant Crane as Crane (export/transform/apply)
participant Target as Target Cluster
TestRunner->>Source: create namespace, ServiceAccount(test-sa), Role(pod-reader), RoleBinding(pod-reader-binding)
TestRunner->>Crane: run export/transform/apply (non-admin crane runner, extraVars non_admin_user=true)
Crane->>TestRunner: produce rendered manifests (Role_*.yaml, RoleBinding_*.yaml, ServiceAccount_*.yaml)
TestRunner->>Target: ApplyDir (non-admin, skip ordering/dry-run)
TestRunner->>Target: ApplyOutputToTargetNonAdmin (complete apply, fix ordering)
TestRunner->>Target: scale deployment, wait for validation
TestRunner->>Target: verify Role, RoleBinding, ServiceAccount exist and correct
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
midays
changed the title
Test Coverage ReportTotal: 33.0% Per-package coverage
Full function-level detailsPosted by CI |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
e2e-tests/tests/mtc_293_role_migration_test.go (2)
159-190: Use non-admin context for target RBAC verification.Lines 159-190 read target RBAC objects via
scenario.KubectlTgt(admin). For a non-admin migration E2E, usekubectlTgtNonAdminto keep apply and verification under the same identity.Suggested patch
- roleOut, err := scenario.KubectlTgt.Run( + roleOut, err := kubectlTgtNonAdmin.Run( "get", "role", "pod-reader", "-n", namespace, "-o", "jsonpath={.rules[0].verbs}", ) @@ - rbRoleRef, err := scenario.KubectlTgt.Run( + rbRoleRef, err := kubectlTgtNonAdmin.Run( "get", "rolebinding", "pod-reader-binding", "-n", namespace, "-o", "jsonpath={.roleRef.name}", ) @@ - rbSubject, err := scenario.KubectlTgt.Run( + rbSubject, err := kubectlTgtNonAdmin.Run( "get", "rolebinding", "pod-reader-binding", "-n", namespace, "-o", "jsonpath={.subjects[0].name}", ) @@ - _, err = scenario.KubectlTgt.Run("get", "serviceaccount", "test-sa", "-n", namespace) + _, err = kubectlTgtNonAdmin.Run("get", "serviceaccount", "test-sa", "-n", namespace)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@e2e-tests/tests/mtc_293_role_migration_test.go` around lines 159 - 190, The test is using the admin client scenario.KubectlTgt to read target RBAC objects; switch the verification calls to the non-admin client (kubectlTgtNonAdmin) so reads are performed under the same non-admin identity used for apply/verification. Replace occurrences of scenario.KubectlTgt.Run(...) for getting the role, rolebinding and serviceaccount (the calls that fetch "role pod-reader", "rolebinding pod-reader-binding" and "serviceaccount test-sa") with kubectlTgtNonAdmin.Run(...), keeping the same args and Expect checks (rbRoleRef, rbSubject assertions) so behavior is unchanged except the client used.
141-144: Avoid swallowing first-pass apply failures.Line 143 ignores the error completely. Keep the workaround, but capture/log and assert the expected ordering-related failure so unrelated errors are not masked.
Suggested patch
By("Apply rendered manifests to target (first pass)") log.Printf("First apply pass — skipping dry-run, ordering failure for RoleBinding expected\n") - _ = kubectlTgtNonAdmin.ApplyDir(paths.OutputDir) + firstPassErr := kubectlTgtNonAdmin.ApplyDir(paths.OutputDir) + if firstPassErr != nil { + log.Printf("First pass apply returned expected/known error: %v\n", firstPassErr) + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@e2e-tests/tests/mtc_293_role_migration_test.go` around lines 141 - 144, The call to kubectlTgtNonAdmin.ApplyDir(paths.OutputDir) currently discards errors; change it to capture the returned error (err := kubectlTgtNonAdmin.ApplyDir(paths.OutputDir)), log the error (e.g., t.Logf or log.Printf) and add an assertion that the error is non-nil and contains the expected ordering-related failure message (use your test assertion helper/Expect to check substring), so the workaround remains but unrelated failures are surfaced; reference the existing ApplyDir call and paths.OutputDir in the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@e2e-tests/tests/mtc_293_role_migration_test.go`:
- Around line 159-190: The test is using the admin client scenario.KubectlTgt to
read target RBAC objects; switch the verification calls to the non-admin client
(kubectlTgtNonAdmin) so reads are performed under the same non-admin identity
used for apply/verification. Replace occurrences of scenario.KubectlTgt.Run(...)
for getting the role, rolebinding and serviceaccount (the calls that fetch "role
pod-reader", "rolebinding pod-reader-binding" and "serviceaccount test-sa") with
kubectlTgtNonAdmin.Run(...), keeping the same args and Expect checks (rbRoleRef,
rbSubject assertions) so behavior is unchanged except the client used.
- Around line 141-144: The call to kubectlTgtNonAdmin.ApplyDir(paths.OutputDir)
currently discards errors; change it to capture the returned error (err :=
kubectlTgtNonAdmin.ApplyDir(paths.OutputDir)), log the error (e.g., t.Logf or
log.Printf) and add an assertion that the error is non-nil and contains the
expected ordering-related failure message (use your test assertion helper/Expect
to check substring), so the workaround remains but unrelated failures are
surfaced; reference the existing ApplyDir call and paths.OutputDir in the
change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8c9daf74-87ad-44e9-b4e4-bd1dc81d7bdd
📒 Files selected for processing (1)
e2e-tests/tests/mtc_293_role_migration_test.go
midays
removed
the
do-not-merge/work-in-progress
msajidmansoori12
requested changes
Apr 16, 2026
Signed-off-by: midays@redhat.com <midays@redhat.com>
msajidmansoori12
approved these changes
Apr 16, 2026
Contributor
|
lgtm , thanks ! |
This was referenced Apr 20, 2026
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
2 participants
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 by CodeRabbit