AzDevOpsProject: Azure DevOps, REST API Wrapper and AzDevOpsProject, DSC resource

[SphenicPaul]

Pull Request (PR) description

@johlju / @kilasuit - I couldn't see this had moved much in a number of months and...

• I had some 'lockdown', annual leave on my hands
• I've an eye on using this module in my workplace to configure the less-changable aspects of Azure DevOps server (via the REST API) in the coming months
• I wanted a bit more of a general work/use of PowerShell classes, class-based, DSC resources and Pester
• I wanted some basis/framework/experience for which to create other, API-facing, DSC Resource modules (for other tools, for example)

As a result, I've put the last few weeks into engineering something to start this module off and used the SqlServerDsc module as a comparison of sorts in getting to this point.

I had an existing set of PowerShell functions that I've used to form the basis of wrappers around the Azure DevOps REST API and I've tried to maintain the majority of the 'wrapper' functionality in a limited set of resources to encourage reuse (there's a little more information on module structure linked to from the 'contribution' document\readme).

This PR isn't completely ready to go (although there's possibly a lot in it - I'd like to think subsquent changes PRs would be much smaller), but I thought I'd put it out for review to get some feedback/improvements/suggestions etc.

Outstanding notes/points/actions:

• Still need to increase tests and code coverage on remaining items. I'd like to get this to 100% from the outset, if possible. Some of the class-based resource testing is a little more convoluted than I'd want it to be.

  • There are some 'using' references that I've had to hardcode to the 'output' directory path location. I was hitting problems where the classes referenced in tests were not being picked up by code coverage unless they were explicitly referencing the output directory files. Not sure what better option there might be for this as the path will change with each version increment as it stands. There might be a better way to setup classes and modules .. although community guidelines seemed to be limited on classes and class-based resources.

• I've updated the pipeline definition to:

  • Include Integration Tests (currently only from Windows OS, using PowerShell Core, but could be added to others if needed)
  • I run my own, Integration Tests against my own, test, Organization within Azure DevOps - I'm not sure if there is one setup for this within the 'dsccommunity'? - If not, one might have to be setup in order to run these.
  • Skip code coverage on Windows OS, using regular PowerShell - There seemed to be a performance hit relating to a Pester bug (although code coverage is still inplace on all the other OS test jobs and PowerShell Core on Windows).

• GitVersion is still currently referencing a 'master' branch, but the default in the repository seems to be 'main'. I'm not clear which branch name is wanting to be taken forward? - Either the default branch name needs to be changed or the 'GitVersion.yml' file needs updating to include the 'main' branch.

• I'm expecting to update $Pat variables/parameters to use a PSCredential, although unclear of most suitable option at present: 1) Have every Resource take a $Pat and $ApiUri parameter (where $Pat is updated to be a PSCredential) 2) Define a new Connection class which holds ApiUri and PAT information, and which is passed between functions 3) Use the existing Connection class (as part of current, Azure DevOps, client libraries)

(Note: I've steered away from depending directly on the client libraries as it stands)

• Add another resource, likely AzDevOpsTeam (which would relate within a project) to ensure it doesn't throw up any larger refactoring considerations.

• Wiki help on the resource is not present yet. Looks like the other DSCCommunity modules take this from the schema.mof files (which this module doesn't have/use). Is there an alternative guideline to coding something using reflection (or similar) that does this? ... and perhaps adding parameter description attributes within the DSC resources?

• Theres possible some tidy up of the testing, helper functions to do that generate/provide test cases etc.

This Pull Request (PR) fixes the following issues

Fixes #3 - Possibly? - This PR and it's changes might make this issue a non-issue

Task list

• [x] 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).
• [x] Resource documentation updated in the resource's README.md.
• [ ] Resource parameter descriptions updated in schema.mof. Not applicable? - Class-based resources
• [ ] Comment-based help updated, including parameter descriptions.
• [ ] Localization strings updated.
• [x] Examples updated.
• [ ] Unit tests updated. See DSC Community Testing Guidelines.
• [x] Integration tests updated (where possible). See DSC Community Testing Guidelines.
• [ ] Code changes adheres to DSC Community Style Guidelines.

---

This change is 

[gaelcolas]

Great work! I've had a look at the structure to start with, as it'll be easier to review. After that we'll also need to look at the CI, but it may not be setup on this very recent repo.

Putting all the comments here about the overall structure so it's easier to read:

Enums

enums should be single ps1 files with 1 enum definition per file. Compilation will put them in the main module psm1. No needs for subfolders in enums

DSC folders

remove DscResources and DscClassResources (folder and files). The DscClassResources' classes will be moved (see below).

DSC Classes

Instead of source/DscClassResources/AzDevOpsProject/AzDevOpsProject.psm1 you should have: source/Classes/AzDevOpsProject.ps1. It will be compiled within your module's psm1, solving problems with importing other files (no need to dotSource).

Submodules

In your source/Modules/AzureDevopsDsc.Common, I'd recommend only having public/private If it really gets too big, it's usually an indication that it should be further split out. I also think that should probably be a module of its own, your call. If you feel that's too many functions for a single helper module, probably best to create other helper modules. burrying them to deep do not help discoverability.

Final note on the submodules, you ought to be able to build the PSM1 by merging the Public/Private/Classes... folder the same way as the top module. You have to have the psd1, configure build.yml's NestedModules, and remove CopyOnly: true (or set to false)

NestedModule:
  AzDevOpsProject.Common:
    CopyOnly: false
    AddToManifest: false

This is not documented, so let me know if any issue and I'll help. I may also be wrong :)

[gaelcolas]

Oh, I forgot, don't put anything in the Module.psm1. You add this at the top or the bottom using a prefix.ps1 or suffix.ps1 and then letting build.yml know like this: https://github.com/gaelcolas/Sampler/blob/master/build.yaml#L18

[kilasuit]

@SphenicPaul Firstly, thanks for all the effort that you've gone to, I can see that you've been very busy indeed. Just to set proper expectations I do intend to get to this on Friday (I unfortunately don't have much time this week till then) and dig deeper into this PR and properly review it.

A few quick things though

• we'll be keeping main for the main branch so GitVersion.yml will need an update to that but there is also other work that will need to be done as you've highlighted prior as well, I will liaise with the others about the wider changes needed here.
• I had initially intended to make use of either VSTeam or PSDevOps as the base module for this and essentially build wrappers around either of them. However lets keep on track with what you've built so far here & discuss down the line about whether or not we go down differringpath.
• I notice you'd joined the PowerShell Slack I'll sort out catching you over there in future and we can perhaps discuss this PR
• I also notice you aren't all that far being Leeds based, I help co-ordinate a number of the UK User Groups for PowerShell which you can find us all at https://www.meetup.com/pro/uk-devopscollective/

Looking forward to working with you on this going forward 😄

[gaelcolas]

Simply put, main is the new default for GitHub. It's a nice change, but we're not just ready yet.

@kilasuit You can only keep main if and when the whole pipeline works with it, which is something @PlagueHO has worked on, but I don't think it's ready just yet (let's catch up with him). Yes we can switch back to master until then, either @kilasuit or myself will do, whoever's got time first (probably not me until next week).

[SphenicPaul]

I've merged my most recent changes back into this PR (to update the Enums, DSC Folders and DSC Classes points raised).

Test coverage is currently about 50% (taking into account Unit tests only), largely due the ommission and duplication of class-related code-paths - It seems the functionality/code-paths in the classes are not registering as being tested (likely due to test setup following recent changes) and, in addition, the classes are being duplicated within the code coverage omissions (once in the 'Classes' directory and once within the output '.psm1' file).

I doubt (hope) there is much work to get these tests registering as part of the code coverage (and remove the duplication - I know this can be excluded in the build.yml, but I've left it as it is for now).

I'm pretty sure there is coverage for all bar 5 or so functions/methods, and the examples (which I believe get tested ... up to the point of generating the MOF... as part of the suite of DSC Resource tests as part of the build).

I'll leave this as it is for now until you've had a chance to take a look. I'm unlikely to look at making any changes to this before Sunday (29/11) or Monday, but will pickup/respond to feedback before then if I can.

[johlju]

@kilasuit do you have time to review this one? We need to fix the pipeline too, to support the branch rename. See blog post https://dsccommunity.org/blog/convert-master-to-main/

[SphenicPaul]

FWIW ... I've not been back to this for a little while, but I'm inclined to think that making use of some of the modules suggested by @kilasuit does make sense - There's lots of test coverage in my PR (with the exception of some of the classes) so even if we can get an initial build working, refactoring to use other modules should become much easier/cleaner (and I'm guessing any initial builds don't have to be "published" from day one?).

Meanwhile, given a few items that need doing to get the pipeline up and working, I'd probably suggest an initial (smaller 😅 ) PR to get the build pipeline working off main, but I do still think that it's worth creating a "destructable", Azure DevOps Services collection and linking an API key (or API keys limited to functionality around specific resources) into that instance so the DSC build can perform integration tests against it - I got this working from my own builds and worked quite well (Note: the build template I used is in this PR and includes some modifications from the original one).

I'm happy to create a small PR with just the build-related changes in it if required (i.e. updated template and branch name) but you both may have more access/knowledge to tweak/debug and make any revisions to get the build working but it would be good to get to a point to know if a PR build from another fork/repository works against this repository ... it could also mean that we/I could trickle in smaller changes (possibly items already in this PR but in a set of smaller PRs that are easier to review/update).

Let me know if you need me to do anything - I'm only on Slack intermittently, so tag me here if needed.

[johlju]

@SphenicPaul We can merge this and build from it. I fixed the pipeline files in the repos so it should build against main branch correctly now. I kicked off the test in this PR to see if it looks good or if there is something easy to fix to get the pipeline to pass (if it does not pass).

[SphenicPaul]

@johlju - Looks like the unit tests in the PR were failing due to the lack of code coverage and the integration tests are failing due to a lack of an Integration environment (Azure DevOps Services instance) and related API key.

For now, I've lowered the Unit tests threshold (I intend to put this back up) to try and ensure we can get a build running to completion (even if it's not used initially).... looks like you've done this anyway 😄 .

In order to resolve the integration tests, the following variables need adding to the build/pipeline:

• AzureDevOps.Integration.ApiUri
• AzureDevOps.Integration.Pat (set as sensitive variable)

... and...

• the ApiUri value needs setting to a URI of an Azure DevOps organization (e.g. https://dev.azure.com/someOrganizationName/_apis/) where the organization will be torn down and recreated each time (i.e. don't use this organization for anything you want to keep! 😁) ... there also has to be some consideration to ensure that multiple sets of integration tests can't run similtaniously against the same instance (not sure how that would be handled, initially - not sure if you want to create a new organization for every build? ... I think there is a limit).
• the ApiKey value needs setting to a key with permissions to effectively do anything within the organization used so the DSC resources can be applied into it.

[SphenicPaul]

Also looks like there are now some items/errors relating to missing modules on the Linux boxes and errors with trying to use the Examples which probably need me to make some changes to the scripts.

[johlju]

I'm pushing some changes to see if we can get the unit tests to work at least. My thinking is that we merge this and then create issues for everything we find that needs fixing. At least this is a baseline to work from.

The problem is that the integration test cannot run for PR's since the secret pipeline values are not added to the build for PR's. That will make the build fail on main branch only, and then its "to late" to fix the build issue for the PR. So I'm thinking we should look at making the integration tests to run manually and each contributor can set there own "destructible" DevOps tenant.

I disabled the integration test in the pipeline for now. I can create an issue for the integration tests part.

[johlju]

Unit tests passing now. HQRM tests was running for all OS's an compiling examples does not work xplat yet, moved it to a separate job.

[johlju]

Merged. Let's continue the work form here.

[SphenicPaul]

OK. Thanks.

Your points make sense - I'll have a think on the Integration tests and post my initial thoughts in the issue (#9).