Skip to content

Conversation

@himani2411
Copy link
Contributor

Description of changes

Adding Integration Tests Cases for addition of Resource Prefix, where I am creating a different user role with a permission boundary which restricts a user from creating IAM resources without a specific path or name prefix.

  • Add Integration test cases and config file for Iam Resource Prefix in test_iam.py
  • Add user-role-rp.cfn.yaml to provide a User Role for creating test clusters
  • Add the test case in pcluster3.yaml config for Jenkins tests.

Tests

  • Manually tested the scenarios covered in integration Tests.

Checklist

  • Make sure you are pointing to the right branch and add a label in the PR title (i.e. 2.x vs 3.x)
  • Check all commits' messages are clear, describing what and why vs how.
  • Make sure to have added unit tests or integration tests to cover the new/modified code.
  • Check if documentation is impacted by this change.

Please review the guidelines for contributing and Pull Request Instructions.

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

@himani2411 himani2411 added skip-changelog-update Disables the check that enforces changelog updates in PRs 3.x labels Dec 5, 2022
@himani2411 himani2411 requested review from a team as code owners December 5, 2022 16:40
@codecov
Copy link

codecov bot commented Dec 5, 2022

Codecov Report

Merging #4652 (9457ef2) into develop (6fee365) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff            @@
##           develop    #4652   +/-   ##
========================================
  Coverage    89.05%   89.05%           
========================================
  Files          163      163           
  Lines        14361    14361           
========================================
  Hits         12789    12789           
  Misses        1572     1572           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@himani2411 himani2411 force-pushed the wip/iam_resource_prefix branch 3 times, most recently from 407a2a4 to c6119ec Compare December 5, 2022 18:04
himani2411 pushed a commit to himani2411/aws-parallelcluster that referenced this pull request Dec 14, 2022
Update the pcluster.config.yaml file to remove Iam section and use _inject_resource_in_config() to inject Iam and ResourcePrefix Section
Change scope of initialize and register_prefix_cli_credentials from class to default(function) level
Update _test_iam_resource_in_cluster() to add Cluster creation verification
Update test_iam_resource_prefix to remove duplication of user-role-rp for each value of iam_resource_prefix_list test and improve performance.
Change position of user-role-iam-resource-prefix.cfn.yaml to the tests folder directory
Remove update_config variable from test_iam_resource_prefix

PR Link: aws#4652
@himani2411 himani2411 force-pushed the wip/iam_resource_prefix branch from 2f7b5e3 to ef876e8 Compare December 14, 2022 01:14
himani2411 pushed a commit to himani2411/aws-parallelcluster that referenced this pull request Dec 14, 2022
Update the pcluster.config.yaml file to remove Iam section and use _inject_resource_in_config() to inject Iam and ResourcePrefix Section
Change scope of initialize and register_prefix_cli_credentials from class to default(function) level
Update _test_iam_resource_in_cluster() to add Cluster creation verification
Update test_iam_resource_prefix to remove duplication of user-role-rp for each value of iam_resource_prefix_list test and improve performance.
Change position of user-role-iam-resource-prefix.cfn.yaml to the tests folder directory
Remove update_config variable from test_iam_resource_prefix

PR Link: aws#4652
@himani2411 himani2411 force-pushed the wip/iam_resource_prefix branch from ef876e8 to 73b2d9b Compare December 14, 2022 20:01
himani2411 pushed a commit to himani2411/aws-parallelcluster that referenced this pull request Dec 15, 2022
Update the pcluster.config.yaml file to remove Iam section and use _inject_resource_in_config() to inject Iam and ResourcePrefix Section
Change scope of initialize and register_prefix_cli_credentials from class to default(function) level
Update _test_iam_resource_in_cluster() to add Cluster creation verification
Update test_iam_resource_prefix to remove duplication of user-role-rp for each value of iam_resource_prefix_list test and improve performance.
Change position of user-role-iam-resource-prefix.cfn.yaml to the tests folder directory
Remove update_config variable from test_iam_resource_prefix

PR Link: aws#4652
@himani2411 himani2411 force-pushed the wip/iam_resource_prefix branch from 3619782 to a04ec63 Compare December 15, 2022 17:15
himani2411 pushed a commit to himani2411/aws-parallelcluster that referenced this pull request Dec 15, 2022
Update the pcluster.config.yaml file to remove Iam section and use _inject_resource_in_config() to inject Iam and ResourcePrefix Section
Change scope of initialize and register_prefix_cli_credentials from class to default(function) level
Update _test_iam_resource_in_cluster() to add Cluster creation verification
Update test_iam_resource_prefix to remove duplication of user-role-rp for each value of iam_resource_prefix_list test and improve performance.
Change position of user-role-iam-resource-prefix.cfn.yaml to the tests folder directory
Remove update_config variable from test_iam_resource_prefix

PR Link: aws#4652
himani2411 pushed a commit to himani2411/aws-parallelcluster that referenced this pull request Dec 15, 2022
Remove _inject_resource_in_config() which is unused for this PR( part of another PR)
Revert run_command() to its original definition
Update run_pcluster_command() to handle switching and KeyError for credential_arn
Remove unnecessary key word arguments passed from create_cluster() to run_pcluster_command()

Reference PR:
aws#4652
@himani2411 himani2411 force-pushed the wip/iam_resource_prefix branch from a04ec63 to dfcb0ee Compare December 15, 2022 20:00
himani2411 pushed a commit to himani2411/aws-parallelcluster that referenced this pull request Dec 15, 2022
Removing custom_cli_credentials from Cluster Factory class member.

Reference PR:
aws#4652
Himani Deshpande added 3 commits December 16, 2022 14:15
Add Integration test cases and config file for Iam Resource Prefix in test_iam.py
Add user-role-rp.cfn.yaml to provide a User Role for creating test clusters
Add the test case in pcluster3.yaml config for Jenkins tests.
Update the pcluster.config.yaml file to remove Iam section and use _inject_resource_in_config() to inject Iam and ResourcePrefix Section
Change scope of initialize and register_prefix_cli_credentials from class to default(function) level
Update _test_iam_resource_in_cluster() to add Cluster creation verification
Update test_iam_resource_prefix to remove duplication of user-role-rp for each value of iam_resource_prefix_list test and improve performance.
Change position of user-role-iam-resource-prefix.cfn.yaml to the tests folder directory
Remove update_config variable from test_iam_resource_prefix

PR Link: aws#4652
Himani Deshpande added 5 commits December 16, 2022 14:15
Add Iam/ResourcePrefix in pcluster.config.yaml
Change user-role-iam-resource-prefix.cfn.yaml to handle /path-prefix/name-prefix- Iam Resource Prefix
Add /path-prefix/name-prefix as the only pytest parameter and remove use_default_iam_credentials parameter check condition from initialize_resource_prefix_cli_creds
Add test specific CLI credentials while creation of cluster and run pcluster commands.
Change ClusterFactory's create_cluster() to have test specific CLI credentials as arguments
Change user-role-iam-resource-prefix.cfn.yaml to handle /path-prefix/name-prefix- Iam Resource Prefix
Remove register_resource_prefix_cli_credentials as it will affect parallel tests running in same region.
Remove _inject_resource_in_config() which is unused for this PR( part of another PR)
Revert run_command() to its original definition
Update run_pcluster_command() to handle switching and KeyError for credential_arn
Remove unnecessary key word arguments passed from create_cluster() to run_pcluster_command()

Reference PR:
aws#4652
Removing custom_cli_credentials from Cluster Factory class member.

Reference PR:
aws#4652
Revert the changes in indentation and scope of if clause in run_pcluster_command()

Reference PR:
aws#4652
@himani2411 himani2411 force-pushed the wip/iam_resource_prefix branch from fc1fc0d to 9457ef2 Compare December 16, 2022 19:15
@himani2411 himani2411 merged commit 1250871 into aws:develop Dec 16, 2022
himani2411 added a commit to himani2411/aws-parallelcluster that referenced this pull request Dec 19, 2022
* Add Integration tests for Resource Prefix

Add Integration test cases and config file for Iam Resource Prefix in test_iam.py
Add user-role-rp.cfn.yaml to provide a User Role for creating test clusters
Add the test case in pcluster3.yaml config for Jenkins tests.

* Remove File deleteS3.py

* Change according to PR

Update the pcluster.config.yaml file to remove Iam section and use _inject_resource_in_config() to inject Iam and ResourcePrefix Section
Change scope of initialize and register_prefix_cli_credentials from class to default(function) level
Update _test_iam_resource_in_cluster() to add Cluster creation verification
Update test_iam_resource_prefix to remove duplication of user-role-rp for each value of iam_resource_prefix_list test and improve performance.
Change position of user-role-iam-resource-prefix.cfn.yaml to the tests folder directory
Remove update_config variable from test_iam_resource_prefix

PR Link: aws#4652

* Change Iam Resource Prefix tests to handle one test case

Add Iam/ResourcePrefix in pcluster.config.yaml
Change user-role-iam-resource-prefix.cfn.yaml to handle /path-prefix/name-prefix- Iam Resource Prefix
Add /path-prefix/name-prefix as the only pytest parameter and remove use_default_iam_credentials parameter check condition from initialize_resource_prefix_cli_creds

* Add Test specific CLI credentials in cluster creation

Add test specific CLI credentials while creation of cluster and run pcluster commands.
Change ClusterFactory's create_cluster() to have test specific CLI credentials as arguments
Change user-role-iam-resource-prefix.cfn.yaml to handle /path-prefix/name-prefix- Iam Resource Prefix
Remove register_resource_prefix_cli_credentials as it will affect parallel tests running in same region.

* Remove unnecessary methods and keyword Arguments

Remove _inject_resource_in_config() which is unused for this PR( part of another PR)
Revert run_command() to its original definition
Update run_pcluster_command() to handle switching and KeyError for credential_arn
Remove unnecessary key word arguments passed from create_cluster() to run_pcluster_command()

Reference PR:
aws#4652

* Remove custom_cli_credentials from ClusterFactory

Removing custom_cli_credentials from Cluster Factory class member.

Reference PR:
aws#4652

* Revert change in run_pcluster_command

Revert the changes in indentation and scope of if clause in run_pcluster_command()

Reference PR:
aws#4652

Co-authored-by: Himani Deshpande <himanidp@amazon.com>
himani2411 added a commit that referenced this pull request Dec 19, 2022
* Add Integration tests for Resource Prefix

Add Integration test cases and config file for Iam Resource Prefix in test_iam.py
Add user-role-rp.cfn.yaml to provide a User Role for creating test clusters
Add the test case in pcluster3.yaml config for Jenkins tests.

* Remove File deleteS3.py

* Change according to PR

Update the pcluster.config.yaml file to remove Iam section and use _inject_resource_in_config() to inject Iam and ResourcePrefix Section
Change scope of initialize and register_prefix_cli_credentials from class to default(function) level
Update _test_iam_resource_in_cluster() to add Cluster creation verification
Update test_iam_resource_prefix to remove duplication of user-role-rp for each value of iam_resource_prefix_list test and improve performance.
Change position of user-role-iam-resource-prefix.cfn.yaml to the tests folder directory
Remove update_config variable from test_iam_resource_prefix

PR Link: #4652

* Change Iam Resource Prefix tests to handle one test case

Add Iam/ResourcePrefix in pcluster.config.yaml
Change user-role-iam-resource-prefix.cfn.yaml to handle /path-prefix/name-prefix- Iam Resource Prefix
Add /path-prefix/name-prefix as the only pytest parameter and remove use_default_iam_credentials parameter check condition from initialize_resource_prefix_cli_creds

* Add Test specific CLI credentials in cluster creation

Add test specific CLI credentials while creation of cluster and run pcluster commands.
Change ClusterFactory's create_cluster() to have test specific CLI credentials as arguments
Change user-role-iam-resource-prefix.cfn.yaml to handle /path-prefix/name-prefix- Iam Resource Prefix
Remove register_resource_prefix_cli_credentials as it will affect parallel tests running in same region.

* Remove unnecessary methods and keyword Arguments

Remove _inject_resource_in_config() which is unused for this PR( part of another PR)
Revert run_command() to its original definition
Update run_pcluster_command() to handle switching and KeyError for credential_arn
Remove unnecessary key word arguments passed from create_cluster() to run_pcluster_command()

Reference PR:
#4652

* Remove custom_cli_credentials from ClusterFactory

Removing custom_cli_credentials from Cluster Factory class member.

Reference PR:
#4652

* Revert change in run_pcluster_command

Revert the changes in indentation and scope of if clause in run_pcluster_command()

Reference PR:
#4652

Co-authored-by: Himani Deshpande <himanidp@amazon.com>

Co-authored-by: Himani Deshpande <himanidp@amazon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3.x skip-changelog-update Disables the check that enforces changelog updates in PRs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants