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

Allow Users to Specify Installing to AllUsers via new $Scope Variable in Initialize-SCuBA #1388

Merged
merged 2 commits into from
Dec 11, 2024

Conversation

buidav
Copy link
Collaborator

@buidav buidav commented Oct 30, 2024

🗣 Description

  • Add $Scope to Initialize-SCuBA to let users install to AllUsers, a place where all users can access the dependent PowerShell modules, rather than just the CurrentUser on their system.

💭 Motivation and context

Closes #882

🧪 Testing

  • Run Initalize-SCuBA by itself. Should be the normal behavior.
  • Then run Initialize-SCuBA -Scope AllUsers.

✅ Pre-approval checklist

  • This PR has an informative and human-readable title.
  • PR targets the correct parent branch (e.g., main or release-name) for merge.
  • Changes are limited to a single goal - eschew scope creep!
  • Changes are sized such that they do not touch excessive number of files.
  • These code changes follow the ScubaGear content style guide.
  • Related issues these changes resolve are linked preferably via closing keywords.
  • All relevant type-of-change labels added.
  • All relevant project fields are set.
  • All relevant repo and/or project documentation updated to reflect these changes.
  • All relevant functional tests passed.
  • All automated checks (e.g., linting, static analysis, unit/smoke tests) passed.

✅ Pre-merge checklist

  • PR passed smoke test check.

  • Feature branch has been rebased against changes from parent branch, as needed

    Use Rebase branch button below or use this reference to rebase from the command line.

  • Resolved all merge conflicts on branch

  • Notified merge coordinator that PR is ready for merge via comment mention

✅ Post-merge checklist

  • Feature branch deleted after merge to clean up repository.
  • Verified that all checks pass on parent branch (e.g., main or release-name) after merge.

@buidav buidav added the enhancement This issue or pull request will add new or improve existing functionality label Oct 30, 2024
@buidav buidav added this to the Kraken milestone Oct 30, 2024
@buidav buidav requested review from ehaines1 and tkol2022 October 30, 2024 02:47
@buidav buidav self-assigned this Oct 30, 2024
@buidav buidav linked an issue Oct 30, 2024 that may be closed by this pull request
@buidav buidav marked this pull request as ready for review October 30, 2024 02:48
@tkol2022
Copy link
Collaborator

@buidav Can you kindly provide some specific testing guidance on how to test the -Scope AllUsers option? For example, after I execute it, where do I check to ensure it completed as expected?

@buidav
Copy link
Collaborator Author

buidav commented Oct 31, 2024

@buidav Can you kindly provide some specific testing guidance on how to test the -Scope AllUsers option? For example, after I execute it, where do I check to ensure it completed as expected?

This is meant for systems with multiple users. User1 and User2.
Check if Initialize-SCuBA -Scope AllUsers installs all the ScubaGear's dependencies in a place where all users are able to access it.
i.e you can run Invoke-SCuBA from User1 and User2.

@tkol2022
Copy link
Collaborator

@buidav Can you kindly provide some specific testing guidance on how to test the -Scope AllUsers option? For example, after I execute it, where do I check to ensure it completed as expected?

This is meant for systems with multiple users. User1 and User2. Check if Initialize-SCuBA -Scope AllUsers installs all the ScubaGear's dependencies in a place where all users are able to access it. i.e you can run Invoke-SCuBA from User1 and User2.

Thanks. It sounds like we need to test on a virtual machine. Please review my testing steps below and update them if necessary.

Testing Steps

  1. Login as the user that will install Scuba: User1.

  2. First make sure that Scuba is not installed. Run the command below and make sure nothing comes back
    Get-Module -Listavailable ScubaGear

  3. You can also run Invoke-Scuba and it should tell you that the cmdlet cannot be found

  4. Logout

  5. Login as User2. Fire up Powershell and run Invoke-Scuba without installing anything and ensure that it works.

@buidav
Copy link
Collaborator Author

buidav commented Oct 31, 2024

@buidav Can you kindly provide some specific testing guidance on how to test the -Scope AllUsers option? For example, after I execute it, where do I check to ensure it completed as expected?

This is meant for systems with multiple users. User1 and User2. Check if Initialize-SCuBA -Scope AllUsers installs all the ScubaGear's dependencies in a place where all users are able to access it. i.e you can run Invoke-SCuBA from User1 and User2.

Thanks. It sounds like we need to test on a virtual machine. Please review my testing steps below and update them if necessary.

Testing Steps

1. Login as the user that will install Scuba: User1.

2. First make sure that Scuba is not installed. Run the command below and make sure nothing comes back
   `Get-Module -Listavailable ScubaGear`

3. You can also run Invoke-Scuba and it should tell you that the cmdlet cannot be found

4. Logout

5. Login as User2. Fire up Powershell and run Invoke-Scuba without installing anything and ensure that it works.

Looks good.

@buidav buidav requested review from adhilto and removed request for tkol2022 November 5, 2024 17:47
Copy link
Collaborator

@adhilto adhilto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works as expected. Testing I performed:

  • Create 3 users on a VM: Alice, Bob, and an admin account.
  • Run Initialize-Scuba as normal as Alice. Alice now has the required PowerShell modules, as expected.
  • Login as Bob. Bob still does not have the required PowerShell modules, as expected.
  • Login as the admin. Run Initialize-Scuba -Scope AllUsers. The admin now has the required PowerShell modules, as expected.
  • Login as Bob. Bob now has the required PowerShell modules, as expected.

One suggestion would be to document this feature in the README, perhaps near the top of the Dependencies page. Something like "Note: the Initialize-SCuBA cmdlet can optionally install the required modules system-wide, if run as an administrator with -Scope AllUsers." Not essential, approving either way.

@buidav buidav force-pushed the 882-should-be-possible-to-use-scope-allusers branch from 7b912b6 to 94de1d7 Compare December 9, 2024 17:41
Copy link
Collaborator

@ehaines1 ehaines1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the code looks good!

@buidav
Copy link
Collaborator Author

buidav commented Dec 10, 2024

@nanda-katikaneni this is ready to merge.

@nanda-katikaneni nanda-katikaneni merged commit 0a9a18f into main Dec 11, 2024
28 checks passed
@nanda-katikaneni nanda-katikaneni deleted the 882-should-be-possible-to-use-scope-allusers branch December 11, 2024 13:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement This issue or pull request will add new or improve existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Should be possible to use -Scope AllUsers when Install-Module is called.
5 participants