IAM | Basic Interrogation Tests Add User Inline Policy Test Cases

[shirady]

Describe the Problem

Currently, we do not have test cases for user inline policy in our basic IAM interrogation tests, as it is not implemented in NC yet.

Explain the Changes

1. Add test cases for the user inline policy (will not run in NC)
2. Add clarification in our doc, as it is something that I found during testing - Statement as an array in the policy document.
3. Fix mocha.after to run only in NC deployment.

Issues:

1. none

Testing Instructions:

Automatic test

Containerized

1. Use the guide "Run Tests From coretest Locally" (link) for running the core tests.
2. Run first tab: docker run -p 5432:5432 -e POSTGRESQL_ADMIN_PASSWORD=noobaa quay.io/sclorg/postgresql-15-c9s
3. Run second tab: NOOBAA_LOG_LEVEL=all ./node_modules/.bin/mocha src/test/integration_tests/api/iam/test_iam_basic_integration.js

NC:
sudo NC_CORETEST=true ./node_modules/.bin/mocha src/test/integration_tests/api/iam/test_iam_basic_integration.js

• Doc added/updated
• Tests added

Summary by CodeRabbit

• Documentation

  • Clarified IAM policy guidance: Statement must be an array, added canonical example, contrasted single-item vs array usage, and aligned behavior with AWS expectations.

• Tests

  • Added end-to-end IAM User Policy integration tests covering list/put/get/delete flows, validation of policy content, error handling for missing users, and conditional test execution for specific environments.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

Adds documentation enforcing that IAM inline policy Statement must be an array and expands IAM integration tests to cover user inline policy APIs (Put/Get/List/Delete), including setup/teardown and error cases with NC coretest conditional behavior.

Changes

Cohort / File(s)	Summary
Documentation docs/design/IamUserInlinePolicy.md	Adds guidance that Statement must be an array, provides AWS vs NooBaa examples, shows canonical form and describes enforcement context.
IAM User Policy Integration Tests src/test/integration_tests/api/iam/test_iam_basic_integration.js	Adds imports for ListUserPoliciesCommand, PutUserPolicyCommand, GetUserPolicyCommand, DeleteUserPolicyCommand; introduces new test suites for user policy CRUD, error cases for non-existent users, setup/teardown, and NC coretest conditional skips/teardown.

Sequence Diagram(s)

(omitted — changes are documentation and tests; no new runtime control flow to diagram)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

• Areas to focus on:
  • correctness and completeness of Put/Get/List/Delete test assertions and PolicyDocument validation
  • duplicate/parallel test blocks and any accidental test duplication
  • imports (removed duplicate import) and test setup/teardown ordering
  • NC coretest conditional skip/teardown logic

Possibly related PRs

• IAM | User Inline Policy Document Additional Validation  #9274: Adds/changes policy document validators enforcing Statement as an array — directly related to the doc change and tested behavior.
• IAM | User Inline Policy Implementation #9270: Implements or updates the IAM user policy API handlers (Put/Get/List/Delete) that these tests exercise.
• IAM | Doc | User Inline Policy Documentation #9285: Related documentation updates to IamUserInlinePolicy.md overlapping examples and wording.

Suggested reviewers

• aayushchouhan09
• naveenpaul1
• liranmauda

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: adding user inline policy test cases to IAM basic interrogation tests, which aligns with both the file-level changes (new test suite) and PR objectives.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

src/test/integration_tests/api/iam/test_iam_basic_integration.js (1)

76-80: Consider awaiting folder_delete in async after hook

mocha.after is async, but fs_utils.folder_delete(\${config_root}`)is not awaited. Iffolder_deleteis asynchronous (similar tocreate_fresh_pathandfile_must_exist` above), teardown could complete before the directory is actually removed, and any errors would be unobserved.

You can make cleanup more robust with:

    mocha.after(async () => {
        if (is_nc_coretest) {
-            fs_utils.folder_delete(`${config_root}`);
+            await fs_utils.folder_delete(`${config_root}`);
        }
    });

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 33924d7 and f2c88cf.

📒 Files selected for processing (2)

• docs/design/IamUserInlinePolicy.md (1 hunks)
• src/test/integration_tests/api/iam/test_iam_basic_integration.js (4 hunks)

🧰 Additional context used

📓 Path-based instructions (1)

src/test/**/*.*

⚙️ CodeRabbit configuration file

src/test/**/*.*: Ensure that the PR includes tests for the changes.

Files:

• src/test/integration_tests/api/iam/test_iam_basic_integration.js

🧬 Code graph analysis (1)

src/test/integration_tests/api/iam/test_iam_basic_integration.js (3)

src/test/system_tests/test_utils.js (1)

• is_nc_coretest (48-48)

src/test/utils/coretest/nc_coretest.js (1)

• mocha (7-7)

src/test/utils/coretest/coretest.js (2)

• mocha (7-7)
• assert (8-8)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)

• GitHub Check: Build Noobaa Image
• GitHub Check: run-jest-unit-tests

🔇 Additional comments (2)

docs/design/IamUserInlinePolicy.md (1)

96-102: Statement-as-array clarification looks good

The added note clearly explains the array requirement for Statement and contrasts AWS vs NooBaa behavior; this should prevent common policy-shape mistakes. No changes needed.

src/test/integration_tests/api/iam/test_iam_basic_integration.js (1)

14-27: IAM command imports align with new coverage

The added IAM v3 command imports (user policy, signing certificates, SSH public keys) are all used later in this file and keep the IAM API coverage consistent. No changes needed here.

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

src/test/integration_tests/api/iam/test_iam_basic_integration.js (1)

289-311: Consider extracting shared test logic to reduce duplication.

The "list user policies for non existing user" and "list user policies for user - should be empty" tests (lines 289-311) duplicate the same tests in the "other APIs" suite (lines 785-807). While this is intentional since suites run in different environments, you could consider extracting the error-case test logic into a shared helper function to reduce maintenance burden.

This is a minor nit given the skip logic ensures no runtime redundancy in NC coretest.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d8e7298 and 636b874.

📒 Files selected for processing (2)

• docs/design/IamUserInlinePolicy.md (1 hunks)
• src/test/integration_tests/api/iam/test_iam_basic_integration.js (3 hunks)

🚧 Files skipped from review as they are similar to previous changes (1)

• docs/design/IamUserInlinePolicy.md

🧰 Additional context used

📓 Path-based instructions (1)

src/test/**/*.*

⚙️ CodeRabbit configuration file

src/test/**/*.*: Ensure that the PR includes tests for the changes.

Files:

• src/test/integration_tests/api/iam/test_iam_basic_integration.js

🧬 Code graph analysis (1)

src/test/integration_tests/api/iam/test_iam_basic_integration.js (2)

src/test/system_tests/test_utils.js (1)

• is_nc_coretest (48-48)

src/endpoint/iam/iam_utils.js (15)

• IamError (109-109)
• IamError (282-282)
• IamError (508-508)
• IamError (537-537)
• IamError (565-565)
• IamError (572-572)
• IamError (600-600)
• IamError (622-622)
• IamError (651-651)
• IamError (686-686)
• IamError (703-703)
• IamError (718-718)
• IamError (725-725)
• IamError (738-738)
• IamError (742-742)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)

• GitHub Check: run-jest-unit-tests
• GitHub Check: Build Noobaa Image
• GitHub Check: run-package-lock-validation

🔇 Additional comments (3)

src/test/integration_tests/api/iam/test_iam_basic_integration.js (3)

14-27: LGTM!

The new IAM policy command imports are correctly added and consolidated. The imports are organized alongside related IAM commands and all are utilized in the new test suite.

---

261-266: LGTM!

The suite-level skip guard correctly prevents the entire suite (including before/after hooks) from executing in NC coretest environments where user inline policy is not implemented. The test constants are well-defined.

---

334-349: LGTM!

The get user policy test correctly validates the PolicyDocument structure. The Version assertion now properly uses assert.equal for value comparison, and assert.deepEqual appropriately validates the Statement object content.