BREAKING CHANGE - CORE 'Add Azure Managed Identity Support to AzDO'

ZanattaMichael commented 5 months ago

Pull Request (PR) description

The Pull Request introduces Azure DevOps Managed Identity by enabling the module to retrieve an AzureDevops API Token. This eliminates the need for a PAT. The feature is only available within Invoke-DSCResource and can be used within an Azure VM. The module achieves this by querying GET 'http://169.254.169.254/metadata/identity/oauth2/token?api-version=2018-02-01&resource=https://management.azure.com/' HTTP/1.1 Metadata: true.

Please note that this module minimum version is PowerShell 7.

This Pull Request (PR) fixes the following issues

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

This change is

ZanattaMichael commented 4 months ago

@kilasuit So the module minimum version will be version 7. Is that going to be an issue? I figured that it wouldn't since the module resources are incomplete and it's easier to write for the new version.

kilasuit commented 2 months ago

Sorry on the delay in getting back to you on this, I know it's been a while.

@kilasuit So the module minimum version will be version 7. Is that going to be an issue? I figured that it wouldn't since the module resources are incomplete and it's easier to write for the new version.

As you'll have seen in the DSC Channel in the PowerShell Discord/slack DSC I've asked others for input on this generally with modules part of the DSCCommunity

Personally I'm ok with it, if we do can potentially have releases that are both v5 and v7 compatible (for those orgs not upgrading to v7 yet) side by side & to do that we'd need to

Bump the major version of the module with this release to show that v7 language features are being used
merge this into a v7 branch not the main branch so can leave main as is for now for v5 compatible changes
update docs about this

All of the above I've mentioned other is maintainer work, so let me get that done, if that is what others agree as a way forward.

kilasuit commented 2 months ago

The previous comment was made based on the comment made not from having relooked at the code in this PR as was an interesting discussion point.

However on the disccusion in the DSC Channel in the slack/discord and investigation on how to check for compatibility I'm going to ask for simplicity for maintainence and use going forward for the following

Revert to using 5.1 language features as part of this PR so can get this merged sooner rather than later & users can look to use this in their configurations.
As part of longer term plans, bring this discussion point to the next DSC Community call, and raise a discussion for this in the PowerShell discussions org as we in DSCCommunity don't currently have discussions enabled
Based on the above discussion, also discuss potentially using this module as a basis for a POC of maintaining v5 and v7 together using the method I mentioned in previous comment
- This is cleaner way forward than forking this into yet another repo that needs managed and maintained and then releasing a AzureDevOpsDscV7 module instead (which @johlju suggested in the discussion in the dsc channel) which from historics leads to causing much more more confusion than needed when we can totally avoid that using Major Versions for seperation which I feel is the better way, if not the most correct way to do this, especially as we should keep alignment to Semver and can be easily incorporated into changelogs too which again aids discoverability

ZanattaMichael commented 2 months ago

The previous comment was made based on the comment made not from having relooked at the code in this PR as was an interesting discussion point.

However on the disccusion in the DSC Channel in the slack/discord and investigation on how to check for compatibility I'm going to ask for simplicity for maintainence and use going forward for the following

Revert to using 5.1 language features as part of this PR so can get this merged sooner rather than later & users can look to use this in their configurations.

As part of longer term plans, bring this discussion point to the next DSC Community call, and raise a discussion for this in the PowerShell discussions org as we in DSCCommunity don't currently have discussions enabled

Based on the above discussion, also discuss potentially using this module as a basis for a POC of maintaining v5 and v7 together using the method I mentioned in previous comment

This is cleaner way forward than forking this into yet another repo that needs managed and maintained and then releasing a AzureDevOpsDscV7 module instead (which @johlju suggested in the discussion in the dsc channel) which from historics leads to causing much more more confusion than needed when we can totally avoid that using Major Versions for seperation which I feel is the better way, if not the most correct way to do this, especially as we should keep alignment to Semver and can be easily incorporated into changelogs too which again aids discoverability

No worries. My reasoning for pwsh 7.0 to force the use of Invoke-DSC Resource rather then using the MOF since this configuration is best run within a pipeline so pwsh 7.0 would be preferred. I need to sit down and do some more work on this module, since I've been writing resources for work and I needed to refactor some of the MI code. I also plan to abstract out the PAT token, and possibly add client ID/ secret support. This will require some major rework of the module.

Thanks for getting back to me. Sorry I would of responded on discord, however I've been busy with #DadLife.

Cheers,

Michael.

ZanattaMichael commented 4 weeks ago

Closing PR due to new branch with additional features.

dsccommunity / AzureDevOpsDsc