Skip to content

Conversation

@bbonaby
Copy link
Contributor

@bbonaby bbonaby commented Sep 16, 2024

Pull Request (PR) description

Currently DSC resources can be ran outside of the LCM. For these cases users will need to run their configuration files with Administrator privileges. When users run their Dev Drive specific configurations without running as administrator, the configuration fails due to one of the PowerShell cmdlets failing due to not being ran elevated. This then returns an error to the user that is true for what we were attempting to do but doesn't tell the user that anything failed due to the user not running the configuration as admin.

To fix this I have added a check for elevation in the Test-TargetResource and the Set-TargetResource methods when the $DevDrive parameter is true. I have also updated the tests for this as well.

I also updated the Disk dsc's readme file with to tell users that running as admin is required.

This Pull Request (PR) fixes the following issues

Task list

  • Added an entry to the change log under the Unreleased section of the
    file CHANGELOG.md. Entry should say what was changed and how that
    affects users (if applicable), and reference the issue being resolved
    (if applicable).
  • Resource documentation added/updated in README.md.
  • Resource parameter descriptions added/updated in README.md, schema.mof
    and comment-based help.
  • Comment-based help added/updated.
  • Localization strings added/updated in all localization files as appropriate.
  • Examples appropriately added/updated.
  • Unit tests added/updated. See DSC Community Testing Guidelines.
  • Integration tests added/updated (where possible). See DSC Community Testing Guidelines.
  • New/changed code adheres to DSC Community Style Guidelines.

This change is Reviewable

@codecov
Copy link

codecov bot commented Sep 16, 2024

Codecov Report

❌ Patch coverage is 60.00000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 94%. Comparing base (215b284) to head (7a2c64e).
⚠️ Report is 5 commits behind head on main.

Files with missing lines Patch % Lines
...e/Modules/StorageDsc.Common/StorageDsc.Common.psm1 0% 2 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@         Coverage Diff         @@
##           main   #292   +/-   ##
===================================
- Coverage    95%    94%   -1%     
===================================
  Files         9      9           
  Lines      1236   1239    +3     
===================================
+ Hits       1176   1177    +1     
- Misses       60     62    +2     
Files with missing lines Coverage Δ
source/DSCResources/DSC_Disk/DSC_Disk.psm1 98% <100%> (+<1%) ⬆️
...urces/DSC_VirtualHardDisk/DSC_VirtualHardDisk.psm1 100% <100%> (ø)
...e/Modules/StorageDsc.Common/StorageDsc.Common.psm1 89% <0%> (-3%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Contributor

@dan-hughes dan-hughes left a comment

Choose a reason for hiding this comment

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

Reviewed 3 of 8 files at r1, all commit messages.
Reviewable status: 3 of 8 files reviewed, 6 unresolved discussions (waiting on @bbonaby)


CHANGELOG.md line 10 at r1 (raw file):

## [6.0.1] - 2024-06-11

### Changed

Also include the change to VirtualHardDisk


source/DSCResources/DSC_Disk/README.md line 39 at r1 (raw file):

This is a low-level concept that most users will never need to interact with but
for further reading, see the documentation [here](https://learn.microsoft.com/windows/dev-drive/#how-do-i-configure-additional-filters-on-dev-drive)
for further reading. In order to create a Dev Drive your configuration must run with

One of the for further reading's can be removed

Code quote:

for further reading.

source/DSCResources/DSC_Disk/en-US/DSC_Disk.strings.psd1 line 55 at r1 (raw file):

    TheVolumeIsNotConfiguredAsADevDriveVolume = The volume with path '{0}' and Drive letter '{1}' is not configured as a Dev Drive volume.
    TheVolumeIsCurrentlyConfiguredAsADevDriveVolume = The volume with path '{0}' and Drive letter '{1}' is currently configured as a Dev Drive volume.
    DevDriveAdminError = Creating a Dev Drive volume requires running local Administrator permissions. Please ensure this resource is being applied with an account with local Administrator permissions.

Can you add an id to the new entry Localization (dsccommunity.org).


source/Modules/StorageDsc.Common/StorageDsc.Common.psm1 line 638 at r1 (raw file):

        A user friendly error message specific to the caller.
#>
function Assert-ElevatedUserWithCustomErrorMessage

I wonder if it's worth extending Assert-ElevatedUser to accept a custom error message. @johlju thoughts?


source/Modules/StorageDsc.Common/StorageDsc.Common.psm1 line 656 at r1 (raw file):

    {
        # Use a user friendly error message specific to the caller
        throw $CustomErrorMessage

Need to not use throw and use ThrowTerminatingError e.g. in Assert-ElevatedUser.

That will depend on the overall strategy for this function though.


tests/Unit/DSC_Disk.Tests.ps1 line 713 at r1 (raw file):

        }

        function Assert-ElevatedUserWithCustomErrorMessageWithCustomErrorMessage

Adding a stub function here should not be required as it lives in this module.

@dan-hughes
Copy link
Contributor

@bbonaby,

Assert-ElevatedUser in DscResource.Common now has an ErrorMessage parameter which removes the need for the Assert-ElevatedUserWithCustomErrorMessageWithCustomErrorMessage function added in this PR.

Are you able to re-download the dependencies locally as this is included in a new release and use this instead of using the try/catch here. It will remove the majority of the comments for the PR, improve test coverage and the overall size of the PR.

Ping me when you've completed and I'll review and get this merged.

@bbonaby
Copy link
Contributor Author

bbonaby commented Oct 23, 2024

Hey @dan-hughes sorry, I've been busy with work but will get to this, this weekend. Thanks!

@johlju johlju added the needs review The pull request needs a code review. label Apr 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs review The pull request needs a code review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Dev Drive DSC error message when not run as admin

3 participants