Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NC | NSFS | IAM | Tech Debts (IAM Integration Tests, Username Validation Move module and Allow IAM User to Create Bucket) #8661

Merged
merged 1 commit into from
Jan 21, 2025

Conversation

shirady
Copy link
Contributor

@shirady shirady commented Jan 7, 2025

Explain the changes

IAM Tech Debts (from an agreed list to wrap up the feature for now) including:

  1. IAM Integration Tests: add the file test_nc_iam_basic_integration.js and make the needed changes in the files nc_coretest.js (add the IAM port), nc_index (add the new test in the CI) and test_utils.js (add the IAM client - like we have S3 client) - the IAM integration tests the APIs of IAM that we support today.
    Please Notice that it contains only the happy path, as the goal was to ensure nothing was broken between the IAM request to the server and our response (internal validation was already implemented in unit tests).
  2. Username Validation Move the Module: we have 2 flows noobaa-cli and API (S3, IAM), and don't want to import modules between the flows and only from an above level. Therefore, I moved the function validate_username from the iam_utils to validation_utils, since it used other functions I also had to move them and move the testing file.
  3. Allow IAM Users to Create Bucket - we temporarily didn't allow IAM users to create buckets (see comment).

Issues:

  1. Fix partially issue NC | NSFS | Organize Modules/Dependencies #8548.

This PR was part of MCGI-282 IAM Tech Debts and the GAPS mentioned in the comments here were added in the Jira ticket (for example: adding interrogation tests for IAM and S3 together).

Testing Instructions:

1) IAM Integration Tests:

Automatic Tests

Please run the test: sudo NC_CORETEST=true node ./node_modules/mocha/bin/mocha ./src/test/unit_tests/test_nc_iam_basic_integration.js
If you want to look at the logs in noobaa, you can: cat nsfs_integration_test_log.txt

Manual Tests

In this PR I didn’t run manual tests, but if you want you can use this guide

2) Username Validation Move the Module:

Automatic Tests

Please run the tests:

  • npx jest test_iam_utils.test.js (iam_utils is the previous file that the function was taken from)
  • npx jest test_nc_utils.test.js (validation_utils is the current file, where I moved the functions to)
  • sudo npx jest test_nc_nsfs_account_cli.test.js (contains the tests that were written in the past, search the test titles that include “invalid name”)

3) Allow IAM Users to Create Bucket

Automatic Tests

Please run the test: sudo NC_CORETEST=true node ./node_modules/mocha/bin/mocha ./src/test/unit_tests/test_bucketspace_fs.js

Manual Tests

  1. Create an account with the CLI: sudo node src/cmd/manage_nsfs account add --name <account-name> --new_buckets_path /Users/buckets/ --access_key <access-key> --secret_key <secret-key> --uid <uid> --gid <gid>
    Note: before creating the account need to give permission to the new_buckets_path: chmod 777 /Users/buckets/.
  2. Start the NSFS server with: sudo node src/cmd/nsfs --debug 5 --https_port_iam 7005
  3. Create the alias for S3 service:alias nc-user-1-s3=‘AWS_ACCESS_KEY_ID=<access-key> AWS_SECRET_ACCESS_KEY=<secret-key> aws --no-verify-ssl --endpoint-url https://localhost:6443’.
  4. Check the connection to the endpoint and try to list the buckets (should be empty): nc-user-1-s3 s3 ls; echo $?
  5. Create the alias for the IAM service: nc-user-1-iam='AWS_ACCESS_KEY_ID=<access-key-account> AWS_SECRET_ACCESS_KEY=<secret-key-account> aws --no-verify-ssl --endpoint-url https://localhost:7005/'
  6. Check the connection to the endoint and try to list the users (should be empty): nc-user-1-iam iam list-users
  7. Create a IAM user with access key: nc-user-1-iam iam create-user --user-name Bob and
    nc-user-1-iam iam create-access-key --user-name Bob
  8. Create the alias for the IAM user to S3 service: alias nc-user-1b-s3='AWS_ACCESS_KEY_ID=<access-key-user> AWS_SECRET_ACCESS_KEY=<secret-key-user> aws --no-verify-ssl --endpoint-url https://localhost:6443/'
  9. Check the the connection to the endpoint and try to list the buckets (should be empty): nc-user-1b-s3 s3 ls; echo $?
  10. Create a bucket by the IAM user: nc-user-1b-s3 s3 mb s3://iam-bucket
  11. Check that the owner_account property is the account’s ID (the owner of the user): sudo cat /etc/noobaa.conf.d/buckets/iam-bucket.json | jq .
  12. Delete the bucket: nc-user-1b-s3 s3 rb s3://iam-bucket
  • Doc added/updated
  • Tests added

@liranmauda
Copy link
Contributor

liranmauda commented Jan 13, 2025

@shirady could you create a new PR that only adds the @aws-sdk/client-iam

and if we are using it only for tests, make it a dev dependency

@shirady
Copy link
Contributor Author

shirady commented Jan 13, 2025

@shirady could you create a new PR that only adds the @aws-sdk/client-iam

and if we are using it only for tests, make it a dev dependency

@liranmauda Yes, thanks.

src/endpoint/iam/iam_utils.js Show resolved Hide resolved
src/nc/nc_utils.js Outdated Show resolved Hide resolved
src/sdk/bucketspace_fs.js Outdated Show resolved Hide resolved
src/test/unit_tests/nc_coretest.js Show resolved Hide resolved
src/test/unit_tests/test_nc_iam_basic_integration.js Outdated Show resolved Hide resolved
src/test/unit_tests/test_nc_iam_basic_integration.js Outdated Show resolved Hide resolved
src/test/unit_tests/test_nc_iam_basic_integration.js Outdated Show resolved Hide resolved
src/test/unit_tests/test_bucketspace_fs.js Show resolved Hide resolved
src/test/unit_tests/test_nc_iam_basic_integration.js Outdated Show resolved Hide resolved
@shirady shirady force-pushed the nsfs-nc-iam-past-tech-debts branch from d314801 to ae9710c Compare January 16, 2025 12:17
@shirady shirady requested a review from romayalon January 16, 2025 13:36
@shirady shirady force-pushed the nsfs-nc-iam-past-tech-debts branch from ae9710c to 1c0d5d5 Compare January 20, 2025 14:52
@shirady shirady force-pushed the nsfs-nc-iam-past-tech-debts branch from 1c0d5d5 to 9cfe133 Compare January 21, 2025 07:23
…ion Move module and Allow IAM User to Create Bucket)

1. IAM Integration Tests: add the file test_nc_iam_basic_integration.js and make the needed changes in the fiiles nc_coretest.js (add the IAM port), nc_index (add the new test in the CI) and test_utils.js (add the IAM client - like we have S3 client) - the IAM integration tests the APIs of IAM that we support today.
2. Username Validation Move the Module: we have 2 flows noobaa-cli and API (S3, IAM), and don't want to import modules between the flows and only from an above level. Therefore, I moved the function validate_username from the iam_utils to validation_utils, since it used other functions I also had to move them and move the testing file.
3. Allow IAM Users to Create Bucket - we temporarily didn't allow IAM users to create buckets.

Signed-off-by: shirady <[email protected]>
@shirady shirady changed the title NC | NSFS| IAM | Tech Debts (IAM Integration Tests, Username Validation Move module and Allow IAM User to Create Bucket) NC | NSFS | IAM | Tech Debts (IAM Integration Tests, Username Validation Move module and Allow IAM User to Create Bucket) Jan 21, 2025
@shirady shirady force-pushed the nsfs-nc-iam-past-tech-debts branch from 9515dff to d759a0e Compare January 21, 2025 08:22
@shirady shirady merged commit f6805ab into noobaa:master Jan 21, 2025
11 checks passed
@shirady shirady deleted the nsfs-nc-iam-past-tech-debts branch January 21, 2025 09:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants