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

BREAKING CHANGE: Identity and Resource Refactor and Update - Squash Commit #37

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

ZanattaMichael
Copy link

@ZanattaMichael ZanattaMichael commented Oct 20, 2024

Pull Request (PR) description

The following pull request performs a major rework of the identity and resources of the module.

  1. Re-Working the caching mechanism to focus on enumeration and caching at the execution of the script.
  2. Removal of PAT Tokens from Resources.
  3. Removal of Api URL's from Resources.
  4. Rework of authentication and separating it into it's own command: New-AzDoAuthenticationProvider
  5. Introduction of Token retrieval protections to prevent accidental token pipeline leakage.
  6. Added the following resources:
    1. AzDevOpsProjectGroup
    2. AzDevOpsOrganizationGroup
    3. AzDevOpsGroupMember
    4. AzDoGroupPermission - Disabled
    5. AzDoProjectServices
    6. AzDoGitRepository
    7. AzDoGitPermission
  7. Renamed resource: AzDevOpsProject to xAzDevOpsProject

Task list

This Pull Request is currently in draft form and will be updated as more resources and features are added.

  • 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.
  • Comment-based help updated, including parameter descriptions.
  • Localization strings updated.
  • Examples updated.
  • Unit tests updated. See DSC Community Testing Guidelines.
  • Integration tests updated (where possible). See DSC Community Testing Guidelines.
  • Code changes adheres to DSC Community Style Guidelines.

This change is Reviewable

ZanattaMichael and others added 3 commits January 17, 2024 13:00
Co-authored-by: Michael Zanatta <[email protected]>
* Adding Initial Class

* Creating ManagedIdentityToken class.
Expanding ManagedIdentityClass
Updated Invoke-AZDevOpsAPIRestMethod with ManagedIdentity Handeler

* Refactoring 004.AzManagedIdentity into Functions
Added APIRateLimit Enum

* Renaming DataResources
Updating AzureDevOpsDsc.Common.psd1
Initial ompleted Managed Identity Cmdlets.

* Adding Initial Tests

* Fixing Documentation
Adding Unit Tests

* Moving Tests
Completed ManagedIdentityToken Test

* Completed APIRateLimit Unit Testing

* Bug Fixes on Azure VM to get Managed Identity Working

* Updating Tests

* Fixing Tests

* Test Updates

* Updating Paths

* Update Paths

* Bug fixes with build and testing process

* Fixing Headers

* Bug Fixes with the Token

* Bug Fixes

* More bug fixes

* Fixing Bugs

* Updated Documentation and ChangeLog to bring into line with guidelines.

---------

Co-authored-by: Michael Zanatta <[email protected]>
* Finished Testing Set Method

* Update Add Resource

* Adding Debugging for New-AzDoOrgGroup

* Added Remove-xAzDoOrganizationGroup
Added Add-xAzDoOrganizationGroup

* Adding AzDoProject* Group

* Fixing Bugs

* Renaming Files

* Daily Commit

* Revert Export Cache Object

* Prep work to finish LCM

* Refactor Logic for LCM

* Add DscPlaybook
Refactor Code within LCM into Playbook
Updated logic to include DependsOn

* Adding Datum Compiling Configuration and Modules

* Add Configuration

* Adding baseline

* Rename Fix Bugs with Datum Lookups

* Update Configuration

* Refactored Configuration and Updated Merge Config

* Start work on build

* Refactoring Logic
Adding Verbose Logging

* Code Refactor

* Update GitIgnore
Update Configuration

* Fixed Logic Bug with Ensure:Absent

* Fixed Logic Bug with Ensure Absent

* Removing Debugging Code

* Bug Fixes

* Update

* Removing Endpoint Configuration

* Renaming Caching Functions

* Rename Functions

* Update Module Manifest

* Adding Group Member

* Adding top level tests

* Archiving Tests
Updating Tests

* Adding Tests
Sorting Tests

* Adding Cachine Tests

* Add Cache

* Adding Cache Test Initialization

* Reorganizing Tests

* Commenced Work of Adding GroupMembers Resource
Refactor of existing caching resource to preference groups prior to membership

* Adding Usercache and GroupMembership Cache

* Adding Logic to dertmine if the group needs to be modified.

* Bug Fixes

* Bug fixes with Caching

* Rename Functions

* Clearing Work on AzDoGroupMember

* Renamed files

* Bug Fixes

* Adding Format-AzDoProjectName
Adding Unit Tests

* Bug Fix

* Perform Code Cleanup
Fixed code for xAzDoGroupMember resource

* Accidently removed CLIXML Export

* Renamed file
Added Add-xAxDoGroupMember
Renamed: Format-UserPrincipalName  to Format-AzDoGroup

* Added New-DevOpsGroupMember
Renamed: Format-UserPrincipalName to Format-AzDoGroup

* Daily Commit

* Bug Fixes
Adding Verbose Logging
Daily Commit

* Bug Fixes with Searching for Display Name

* Add Remove-xAzDoGroupMember

* Daily Commit
Refactoring Authentication Logic to include differnt auth types

* Updated ResourceObject to load caching settings.
Added SecureString support for PAT Tokens
Bug Fixes
Fixing Plumbing issues with functions

* Performing Code Cleanup

* Code Refactor
Bug Fixes

* Daily Commit
Bug Fixes

* Daily Commit
Bug Fixes

* Daily Commit

* Added Initial Logic

* Adding wrapping logic to complete the set function

* Bugs Fixes

* Bug Fixes

* Remove ValidateSet with ValidateScript removing duplicate arrays
Move API Functions into Resources directories rather then using Verb.

* Cache Cleanup
Adding GitRepositoryCache Enumeratoion

* Added GitRepository Resource

* Bug Fixes

* Bug Fixes

* Adding Permission Classes and descriptor types

* Adding Permission Classes

* Adding Class Method Files

* Daily Commit

* Daily Commit

* Daily Commit

* Daily Commit

* Daily Commit

* Daily Commit

* Completed Initial ACLS resolution

* Daily Commit

* Bug Fixes

* Daily Commit

* Daily Commit

* Daily Commit
Bug Fixes

* Daily Commit

* Code cleanup

* Daily Commit
Added Compare-ACL function
Commenced Initial Testing

* Bug Fixes

* Daily Commit

* Adding TODO

* Daily Commit

* Daily Commit

* Renamed Files
Updated Test-ACLListForChanges

* Renamed Functions and Performed Code Cleanup

* Bug Fixes

* Bug Fixes with Caching

* Daily Commit

* Bug Fixes

* Bug Fixes
File Movement

* Disabling Logging

* Bug Fixes

* Bug Fixes

* Bug Fixes

* Removing Enums - Not required

* File cleanup. Remove redundent files.

* File Cleanup

* Code Cleanup. Recreating Tests

* Adding Authentication Unit Testing

* Adding Tests

* Adding Additional Tests

* Removing Unused Enums
Removing Permissions.ps1

* Adding AI Tests

* Adding Unit Tests

* Formatting cleanup for tests.

* Renaming Files to make place for new resources

* Adding xAzDoProjectGroupPermission

* Daily Commit

* Daily Commit

* Refactored ACL logic

* Adding xAzDoProjectGroupPermission

* Bug Fixes

* Adding Documentation

* Bug Fixes

* Bug Fixes

* Daily Commit

* Bug Fixes
Adding New Resource: xAzDoGroupPermission

* Adding Tests

* Adding Automated Testing

* Bug Fixes and Renaming Functions
Correcting File Names

* Bug Fixes

* Removing unused tests.
Writing a parameterized Initialization Script for intergration testing.

* Adding xAzDevOpsProjectServices resource

* Renamed Files

* Bug Fixes with AZDOProjectServices.

* Refactor of DevOps Project to include new settings

* Adding documentation

* Bug Fixes with Project Resource. More work is required.

* Bug Fixes

* Disabling Logging

* Adding ProjectAbbreviation
Bug Fixes with xAzDoProject Resource

* Remove ProjectAbbreviation

* Bug fixes with Description

* Fixed Bug with inherited flat within ACL check
Added additional logging

* Bug Fixes with GroupMembers
Starting work fixing issues with GroupMembers.

* Fixed caching bug with Set-xAzDoGroupMember when running Local Configuration Manager (LCM) twice.

* Fixing Spelling Issue
Adding Resource Documentation

* Adding More Documentation

* Adding More Documentation

* Bug fixes with Test method within LCM

* Adding Todo's

* Remove Export Clixml
TypeCast ReferanceACLs as Array
Adding Refresh-Cache Identity to fix bug with creating and setting group permissions in the same LCM run.

* Adding Documentation

* Adding Documentation

* Adding Documentation

* Renaming Directories
Updating Documentation

* Updating Documentation

* Adding Documentation

* Updating Documentation

* Updating Documentation

* Daily Commit

* Adjusting Logic for Test Framework

* Adding Unit Tests

* Update tests

* Adding Intergration Tests
Removing ProcessPermission Resource (not needed)

* Adding Initial Intergration Tests

* Bug Fixes with Intergration Testing

* Bug Fixes

* Remove -Verbose logging
Fixed bug in Get-xAzDoProject with blank description

* Moving untested intergrating tests.
Bug fixes.
Enabled logging for write-verbose.

* Bug Fixes

* Bug Fixes

* Update Logging
Moving Files

* Updating Documentation

* Big Fixes

* Adding New Tests
Removing Bugs

* Bug Fixes
Adding Logging
Completed Intergration Testing

* Bug Fixes

* Finish Testing

* Bug Fixes with DSCGetSummary State
Fixed Bug with Invoke-Tests

* Bug Fixes
Disabling Group Permission Resource

* Enabled All Tests
Fixed bugs with teardown logic

* Bug Fixes

* Starting unit tests
Reworking unit tests

* Adding CommonTestFunctions Module
Updating Tests to support CommonTestFunctions Module

* Adding Tests
Daily Commit

* Adding -ParameterFilter

* Adding Unit Tests for Cache

* Adding GitRepo Tests

* Fixing More Tests

* Update Project Tests

* Project Services

* Adding Security Descriptor

* Updating Tests

* Adding Managed Identity Token Tests
More work is needed

* Bug Fixes
Adding Authentication Tests

* Bug Fixes with Tests

* Adding Cache Tests

* Adding Tests

* Initialize-CacheObject

* Adding Unit Tests

* Add ProjectCache

* Adding GroupCache

* Adding Cache

* Bug Fixes

* Bug Fixes

* Adding Tests

* Adding Tests

* Bug Fixes with Tests

* Reformat Tests

* Test Fixes

* Rename File

* Rename Files

* Bug Fixes
Adding Tests

* Finishing Tests
Adding Tags

* Adding Test Tags

* Bug Fixes
Updating Tests

* Bug Fixes
Adding Tests

* Bug Fixes
Adding Tests

* Bug Fixes with Test

* Bug Fixes

* Disabling Tests

* Disabling Archived Tests

* Renaming Files

* Bug Fixes
Fixing Spelling

* Rename File

* Daily Commit

* Completed initial tests for xAzDoGitPermission

* Bug Fixes
Adding xAzDoGitRepository

* Rename files
Bug Fixes

* Adding tests

* Adding Tests to xAzDoGroupPermission

* Adding xAzDoOrganizationGroup

* Remove Wait-Debugger

* Add AzDoProject Tests

* Bug Fixes

* Bug Fixes

* Bug Fixes

* Fixed bug within Find-Identity
Disabling Unused Tests
Adding additional Verbose logging

* Fixed bug with caching after creating git repository

* Adding: Refresh-AzDoCache
Adding Logging
Fixed bug where identities are being added to the wrong cache
Replaced AzDOAPI_0_ProjectCache / Refresh-AzDoCache

* Add MI support for Azure Arc

* Bug Fixes with Azure Arc Authentication

* Add MI support for Azure Arc

* Bug Fixes with Azure Arc Authentication

---------

Co-authored-by: Michael Zanatta <[email protected]>
Co-authored-by: david <[email protected]>
@ZanattaMichael
Copy link
Author

Based off #36

* Updated Classes to be in-line with DSC Coding Guidelines

Removed 'x' from xAzDoGitPermission

* Removed 'x' from xAzDoGitRepository.

* Remove 'x' from xAzDoGroupMember

* Removed 'x' from xAzDoGroupPermission

* Removed 'x' from xAzDoOrganizationGroup

* Removed 'x' from xAzDoProject

* Removed 'x' from Resource

* Removed 'x' from Resources

* Adding Documentation

* Performing Code Refactoring to Bring it into line with AZDO Coding Guidelines

* Please Implicit Mandatory Attribute with 'explicit'

* Adding Documentation
Fixing Issues

* Set Explicit Mandatory
Performing Code Refactoring

* Code Formatting

* Code Refactoring

* Update the Cache Initialization Scripts

* Code Cleanup of Cache

* Fixed String Formatting and Code Blocks in Private Functions

* Updating Codebase to support DSC Community Code Guidelines

* Code standard changes

---------

Co-authored-by: Michael Zanatta <[email protected]>
@kilasuit kilasuit self-requested a review October 22, 2024 02:00
@kilasuit kilasuit changed the base branch from main to prerelease October 24, 2024 08:07
* Remove 'x' from Files and Directories

* Added Common Testing to root of the repository
Updating Project Tasks

* Bug Fixes

* Bug Fixes within Tests
Updating Tests to be in-line with changes

* Code Cleanup of commontests script
Add New-AzDoAuthenticationProvider tests

* Big Fixes

* Bug Fixes with Tests
Adding Azure DevOps DSC Tests

* Adding Basline Mocking and Classes

* Adding Test Initalization Fixes
Fixes to Tests
More Work is required.

* Adding Tests
Renamed: Invoke-BeforeEachFunctions to: Get-FunctionItem

* Renamed: Find-Functions to Find-MockedFunctions

* Finishing Initial Testing
Removing Un-used Tests

* Bug Fixes to Bring in tests

* Fixed: AzDevOpsDscResourceBase tests

* Refactoring Tests

* Bug Fixes

* Bug Fixes with Tests

* Adding Tests

* Adding More tests
Bug Fixes

* Removing Archived Tests

* Bug Fixes
Updating Documentation

* Adding Documentation

* Adding excludedProjectsFromTeardown

* Running Intergration Tests
Fixing Bug with Test-isWindowsAdmin function
Fixing Intergration Supporting function bugs

* Removing Unsed Paths.
Fixing Tests

* Fixed Bug with Regex Matching
Added Unit Testing to provide code coverage

* Bug Fixes

* Bug Fixes with Intergration Tests
Fixed Bug with ResourceBase class
Fixed Bug with GetMembers using the wrong type

* Fixed Bug with Test-AzDoProject being the incorrect type
Fixed bug in Get-AzDoProject typecasting lookupresult
Fixed unchanged bug with Get-AzDoProject

* Bug Fixing Tests

---------

Co-authored-by: Michael Zanatta <[email protected]>
@ZanattaMichael
Copy link
Author

All sorry for the run around. I've reworked the module to be in line with the DSC Community standards and fixed a few bugs along the way. :-)

@ZanattaMichael ZanattaMichael marked this pull request as ready for review November 3, 2024 22:07
@kilasuit
Copy link
Contributor

kilasuit commented Nov 6, 2024

FYI - I've changed what branch this points to only due how much there is here that needs to be reviewed

Just to set expectations I am setting aside time to fully review on 11th November to look at this getting merged that day.

@ZanattaMichael
Copy link
Author

Sorry I thought I responded, but I didn't. That's fine. I'll be DMing that day, but I'll keep a close eye on this.

@kilasuit kilasuit changed the base branch from prerelease to refactor November 15, 2024 06:22
Copy link
Contributor

@kilasuit kilasuit left a comment

Choose a reason for hiding this comment

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

I've reviewed as much as i can with the motivation that I have available to me right now, without outright rejecting or abandoning this review, or the desire to keep on breathing.

Theres lots in this PR I like, lots i very much dislike too. I've likely missed things as I've been reviewing this on my energy reserves & slept for a day and half between starting and getting to this point & mostly without coffee, chocolate or basic nutriition.

I've changed this to point to a refactor branch so that we can attempt to clean all this up before it going into a prerelease or released module as I don't think it's in a fit state right now.

I also opened #38 as this needs more maintainers than just me, especially as I've not written any of the code here & I can't continue doing stuff like this for free, when I have been barely surviving for well over 2 & half years & started this with plans for it to be presented about at PSConfEU 22, however I didnt' get support to do so on part paid, part my own time, and then lost my dad to cancer & had a nightmare few years since.

I am close to walking away from all of this, however, this PR, as brutal is has been, has been somewhat enjoyable, I just dont have right now the energy to continue with this.

It perhaps should have been split into smaller PR's but for now I am done reviewing this for at least a week if not more.

USAGE.md Show resolved Hide resolved
Comment on lines +43 to +47
DscResourcesToExport = @(
'AzDevOpsProject',
'AzDoOrganizationGroup',
'AzDoProjectGroup'
)
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this not include all the DSCResources & not just these 3?

source/Classes/002.PersonalAccessToken.ps1 Outdated Show resolved Hide resolved
source/Enum/DescriptorType.ps1 Outdated Show resolved Hide resolved
source/Examples/Resources/AzDoGitPermission.md Outdated Show resolved Hide resolved
tests/Integration/Supporting/API/Invoke-APIRestMethod.ps1 Outdated Show resolved Hide resolved
tests/Integration/Supporting/Teardown.ps1 Outdated Show resolved Hide resolved
@ZanattaMichael
Copy link
Author

I've reviewed as much as i can with the motivation that I have available to me right now, without outright rejecting or abandoning this review, or the desire to keep on breathing.

Theres lots in this PR I like, lots i very much dislike too. I've likely missed things as I've been reviewing this on my energy reserves & slept for a day and half between starting and getting to this point & mostly without coffee, chocolate or basic nutriition.

I've changed this to point to a refactor branch so that we can attempt to clean all this up before it going into a prerelease or released module as I don't think it's in a fit state right now.

I also opened #38 as this needs more maintainers than just me, especially as I've not written any of the code here & I can't continue doing stuff like this for free, when I have been barely surviving for well over 2 & half years & started this with plans for it to be presented about at PSConfEU 22, however I didnt' get support to do so on part paid, part my own time, and then lost my dad to cancer & had a nightmare few years since.

I am close to walking away from all of this, however, this PR, as brutal is has been, has been somewhat enjoyable, I just dont have right now the energy to continue with this.

It perhaps should have been split into smaller PR's but for now I am done reviewing this for at least a week if not more.

Thank you for taking the time to perform the review. Right now, I'll work with other members within the DSC community to continue this review.

In terms of maintainers, going forward I would be more then happy to help out as a reviewer. It sounds that you have a lot on your plate at the moment and I'm more than happy to help out.

@johlju johlju changed the base branch from refactor to main November 17, 2024 08:50
@johlju
Copy link
Member

johlju commented Nov 17, 2024

FYI. I'm gonna switch the default branch back to main otherwise the pipeline wont work. Having another branch is gonna be a pain and just add even more maintenance.

@dan-hughes
Copy link
Contributor

@ZanattaMichael, some updates to the repository have been made to get it working and publishing.
For next steps, are you able to identify a single DSC resource (with tests, examples and dependencies) from this PR that we can move/copy into another PR to get it merged.
Or we could disable all of the current resources and just merge the AzureDevOpsDsc.Common module as it has the most code then work on adding the resources after that?

It's a way of getting the first contribution aligned and hopefully halve the size of what's remaining in this PR.

I'm happy to assist you on this to get it moving and merged.

Thoughts?

@ZanattaMichael
Copy link
Author

@ZanattaMichael, some updates to the repository have been made to get it working and publishing. For next steps, are you able to identify a single DSC resource (with tests, examples and dependencies) from this PR that we can move/copy into another PR to get it merged. Or we could disable all of the current resources and just merge the AzureDevOpsDsc.Common module as it has the most code then work on adding the resources after that?

It's a way of getting the first contribution aligned and hopefully halve the size of what's remaining in this PR.

I'm happy to assist you on this to get it moving and merged.

Thoughts?

I'm open to both approaches. Disabling the resources and subsequently reintroducing them seems simpler. Alternatively, I could strip all the features from the module altogether. The issue lies with the supporting functions that these resources depend on. Should they also be separated from the module?

@dan-hughes
Copy link
Contributor

Typically, if some functions are used across multiple modules then they would be moved into DscResource.Common. It's good to know what's available in DscResource.Common as it means tests do not need to be written twice.

It's not critical this stage, as the module is in its infancy and is likely to change.

If the functions are solely for this module then they would live in a Common module within this repo. But I could see a situation here where the API functions may be useful to someone else but again that could be a future change to extract it into its own module.

From a module perspective it would be good to get a single resource added/updated as then the module is in a working state.

There will be some work to enable Pester 5 in config but once everything is in place it should all become a lot easier.

* Code Review Fixes (#7)

* Initial Commit

* Code Review Adjustments

---------

Co-authored-by: Michael Zanatta <[email protected]>

* Replace comment with amended version

* Adding Install-Module and Install-PSResource commands.

* Bug Fixes

* Big Fixes with Unit Tests

---------

Co-authored-by: Michael Zanatta <[email protected]>
@ZanattaMichael
Copy link
Author

@dan-hughes @kilas I have implemented the recommended changes. To advance this project I will disconnect the resources from the project (but not from the dsccommon module) and then re-add them.

Please let me know your thoughts on the matter.

@dan-hughes
Copy link
Contributor

/azp run

Copy link

Commenter does not have sufficient privileges for PR 37 in repo dsccommunity/AzureDevOpsDsc

@dan-hughes
Copy link
Contributor

dan-hughes commented Jan 8, 2025

@ZanattaMichael can you sync main on your fork and merge or rebase this branch onto it please.

CI should then run and let's see the outcome.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants