Wait for bl encryption resource proposal

[luisgmsft]

It allows Waiting for an encryption process to complete on a MountUnit. Usefull for chained orchestrated processes. With it, they can wait before keep on with other dependent tasks.

---

This change is 

[codecov-io]

Codecov Report

Merging #19 into dev will increase coverage by 15.89%.
The diff coverage is 2.7%.

@@            Coverage Diff             @@
##             dev      #19       +/-   ##
==========================================
+ Coverage   3.48%   19.37%   +15.89%     
==========================================
  Files          3        4        +1     
  Lines         86      129       +43     
==========================================
+ Hits           3       25       +22     
- Misses        83      104       +21

Impacted Files	Coverage Δ
...WaitForBLEncryption/MSFT_xWaitForBLEncryption.psm1	2.7% <2.7%> (ø)
...s/MSFT_xBLAutoBitlocker/MSFT_xBLAutoBitlocker.psm1	42.3% <0%> (+40.13%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9b00d76...f05e2ed. Read the comment docs.

[johlju]

Thank you for this! Great work! Some review comments.

---

Reviewed 3 of 3 files at r1, 1 of 1 files at r2.
Review status: all files reviewed, 18 unresolved discussions (waiting on @luisgmsft)

---

a discussion (no related file):
@luisgmsft Would you mind adding unit tests for this change? https://github.com/PowerShell/xBitlocker/blob/dev/Tests/Unit/MSFT_xBLAutoBitlocker.tests.ps1

---

README.md, line 55 at r2 (raw file):

This DSC Module allows you to configure Bitlocker on a single disk, configure a TPM chip, or automatically enable Bitlocker on multiple disks.

## Resources

We should add the resource to this list, in alphabetical order

---

DSCResources/MSFT_xWaitForBLEncryption/MSFT_xWaitForBLEncryption.psm1, line 1 at r2 (raw file):

function Get-TargetResource

We should have comment-based help for all functions. At least with .SYNOPSIS and .PARAMETER. Throughout all functions.

---

DSCResources/MSFT_xWaitForBLEncryption/MSFT_xWaitForBLEncryption.psm1, line 1 at r2 (raw file):

function Get-TargetResource

Can we add verbose messages so the user know what's happening (at least one)?

---

DSCResources/MSFT_xWaitForBLEncryption/MSFT_xWaitForBLEncryption.psm1, line 7 at r2 (raw file):

[parameter(Mandatory = $true)]

[Parameter(Mandatory = $true)] (upper 'P'). Throughout.

---

DSCResources/MSFT_xWaitForBLEncryption/MSFT_xWaitForBLEncryption.psm1, line 11 at r2 (raw file):

        $MountPoint,

        [System.UInt32]

Non-mandatory parameters should be decorated with [Parameter()]. Throughout.

---

DSCResources/MSFT_xWaitForBLEncryption/MSFT_xWaitForBLEncryption.psm1, line 18 at r2 (raw file):

    )

    #Load helper module

Comments should use space after #, e.g # Text. Throughout.

---

DSCResources/MSFT_xWaitForBLEncryption/MSFT_xWaitForBLEncryption.psm1, line 27 at r2 (raw file):

    if ($status -ne $null)
    {
        $returnValue = @{

This should always return all properties in the schema.mof. If the value for a property cannot be determined we should provide $null or appropriate value.

---

DSCResources/MSFT_xWaitForBLEncryption/MSFT_xWaitForBLEncryption.psm1, line 35 at r2 (raw file):

}

function Set-TargetResource

Can we add verbose messages so the user know what's happening (at least one)?

---

DSCResources/MSFT_xWaitForBLEncryption/MSFT_xWaitForBLEncryption.psm1, line 56 at r2 (raw file):

    CheckForPreReqs

    #$PSBoundParameters.Remove("Identity") | Out-Null

Can we remove this row?

---

DSCResources/MSFT_xWaitForBLEncryption/MSFT_xWaitForBLEncryption.psm1, line 68 at r2 (raw file):

                break
            }
            else {

We should have open-brace on separate row.

---

DSCResources/MSFT_xWaitForBLEncryption/MSFT_xWaitForBLEncryption.psm1, line 75 at r2 (raw file):

}

function Test-TargetResource

Can we add verbose messages so the user know what's happening (at least one)?

---

DSCResources/MSFT_xWaitForBLEncryption/MSFT_xWaitForBLEncryption.psm1, line 100 at r2 (raw file):

}

function TestStatus([string] $unit)

Please use Verb-Noun for function names.

---

DSCResources/MSFT_xWaitForBLEncryption/MSFT_xWaitForBLEncryption.psm1, line 100 at r2 (raw file):

}

function TestStatus([string] $unit)

Parameters should use a param()-block (like *-TargetResource functions).

---

DSCResources/MSFT_xWaitForBLEncryption/MSFT_xWaitForBLEncryption.psm1, line 118 at r2 (raw file):

}

function IsFullyEncrypted([string]$unit)

Please use Verb-Noun for function names.

---

DSCResources/MSFT_xWaitForBLEncryption/MSFT_xWaitForBLEncryption.psm1, line 118 at r2 (raw file):

}

function IsFullyEncrypted([string]$unit)

Parameters should use a param()-block (like *-TargetResource functions).

---

DSCResources/MSFT_xWaitForBLEncryption/MSFT_xWaitForBLEncryption.schema.mof, line 4 at r2 (raw file):

class MSFT_xWaitForBLEncryption : OMI_BaseResource
{
    [Key, Description("Drive letter")] String MountPoint;

Can we make this description more descriptive?

---

DSCResources/MSFT_xWaitForBLEncryption/MSFT_xWaitForBLEncryption.schema.mof, line 5 at r2 (raw file):

{
    [Key, Description("Drive letter")] String MountPoint;
    [Write] UInt32 RetryIntervalSeconds;

Please add description to these two properties properties as well. Use the same as in the README.md and comment-based help.

---

Comments from Reviewable

[johlju]

@PlagueHO Would this resource fit in StorageDsc instead, or do you think this is to specific so it should remain here? We don't need to move it there now. Just a thought.

[PlagueHO]

@johlju - it would make sense I think. It does need a little bit of work bringing up to HQRM, but nothing that couldn't be done fairly easily. May also be able to use the techniques in the StorageDsc resource to create integration tests for the resources.

[johlju]

@luisgmsft What you think about adding this to StorageDsc instead?

[luisgmsft]

I can certainly do as you suggest @johlju and take this to StorageDsc.

xBitlocker seemed to be the right place for this resource to live, as we're strictly waiting for an Encryption (Bitlocker) operation to finish prior than doing any other stuff on top of the drive at hand.

Please let me know your thoughts.

[johlju]

@luisgmsft I think all these resource could be split up into several resource modules. Activating TMP (xBLTpm ) could be moved to ComputerManagementDsc, all the resources handling disks could be moved to StorageDsc. This is to minimize the number of resource modules the community need to administer. To not add to the backlog it would be great to move it today. But I'm good wither way, if you want to add it to StorageDsc or if you want to add it here (and we move all resources in one sweep at a later time).

[stale]

Labeling this pull request (PR) as abandoned since it has gone 14 days or more since the last update. An abandoned PR can be continued by another contributor. The abandoned label will be removed if work on this PR is taken up again.

[mhendric]

Bringing this conversation back up. It seems that this PR got stuck on the topic of moving some or all of the xBitlocker resources into other modules. I'm wondering if that should really be a blocker for this PR, or if that should just be a future goal to track. As the original author of xBitlocker, I'm personally fine moving these resources to other modules if it makes sense. I just don't want all new work on the module to halt in the meantime. Thoughts on how best to move this forward @luisgmsft, @johlju, and @PlagueHO?

[johlju]

I was fine adding it here, or it be moved to StorageDsc, as per my previous comment. But the review comments have not been addressed here, so that is why it hasn’t gone any further here. I haven’t seen a PR in StorageDsc either. If @luisgmsft resolves them I can acknowledge them as resolved and continue the review. 😄

[PlagueHO]

I'd definitely like to see this in StorageDsc, but because this is an HQRM module it would require any new resources to meet the same level. This can be a bit of a challenge depending on the module. But as I said, I'd love to see it included!

[mhendric]

I've recently started working towards knocking out some of the items preventing this from being a high quality module (see #27 and #28). I plan to add remaining Unit tests next to get this near 100% code coverage. Perhaps we can get xBitlocker up to high quality, and then talk about re-homing the various resources. What do you guys think?

[johlju]

This repository have now been updated to the new CI/CD pipeline so deploy is automatic on PR merge. This PR was re-targeted to the master branch (dev branch is not longer present). This PR need to be rebased with the changes from the master-branch.