Skip to content

Harden migrate tests for single-provider (no typeField) resources - #6454

Open
DarshitChanpura wants to merge 2 commits into
opensearch-project:mainfrom
DarshitChanpura:harden-migrate-notification-tests
Open

Harden migrate tests for single-provider (no typeField) resources#6454
DarshitChanpura wants to merge 2 commits into
opensearch-project:mainfrom
DarshitChanpura:harden-migrate-notification-tests

Conversation

@DarshitChanpura

Copy link
Copy Markdown
Member

Description

Adds regression coverage for the resource-sharing migrate path used by plugins that register a single ResourceProvider with no typeField (for example, notifications' notification_config). This behavior currently has no unit coverage; the tests lock it so a future change to classifyDocType or the request-path validation can't silently break migration for these plugins.

Two cases:

  • classifyNotificationConfigFallsBackToSingleAccessLevelKey — with no type paths, classifyDocType falls back to the sole default_access_level key, so a notification_config doc is classified (and migrated) rather than recorded under skippedResources. Uses a faithful .opensearch-notifications-config _source (NotificationConfigDoc.toXContent shape with the real NotificationConstants tag names).
  • nonResolvingUsernamePathYieldsNoOwnerSoMigrationUsesDefaultOwnerusername_path must be a valid, non-empty path (an empty "" is rejected by PATH_VALIDATOR#requireNonEmpty), and a valid non-resolving path yields no owner, so migration attributes ownership to default_owner while backend_roles_path (/metadata/access) still supplies the shared backend roles.

Test-only; no production change. The live write-path counterpart is already covered by MigrateApiTests (a resource with no owner migrates into resourcesWithDefaultOwner).

Testing

./gradlew test --tests "org.opensearch.security.resources.api.migrate.MigrateResourceSharingInfoApiActionTests" — 10 passed, 0 failed.

Check List

  • New functionality includes testing.
  • Commits are signed per the DCO using --signoff.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Adds regression coverage for the resource-sharing migrate path used by plugins that register a single provider with no typeField (for example, notifications' notification_config):

- classifyDocType falls back to the sole default_access_level key, so such docs are classified and migrated rather than skipped.

- username_path must be a valid non-empty path (empty is rejected by the path validator), and a non-resolving path yields no owner so migration attributes ownership to default_owner, while backend_roles_path still supplies the shared backend roles.

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>
@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

PR Reviewer Guide 🔍

(Review updated until commit d35bb2f)

Here are some key observations to aid the review process:

🧪 PR contains tests
🔒 No security concerns identified
✅ No TODO sections
🔀 No multiple PR themes
⚡ No major issues detected

@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Persistent review updated to latest commit d35bb2f

@codecov

codecov Bot commented Sep 2, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 75.75%. Comparing base (257a3c8) to head (d35bb2f).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #6454      +/-   ##
==========================================
+ Coverage   75.74%   75.75%   +0.01%     
==========================================
  Files         457      457              
  Lines       30512    30508       -4     
  Branches     4615     4615              
==========================================
+ Hits        23110    23111       +1     
+ Misses       5284     5277       -7     
- Partials     2118     2120       +2     

see 8 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant