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

Rollback Changes Requiring ID as a Primary Key #3344

Closed
NikCharlebois opened this issue May 26, 2023 · 3 comments · Fixed by #3675 or #3682
Closed

Rollback Changes Requiring ID as a Primary Key #3344

NikCharlebois opened this issue May 26, 2023 · 3 comments · Fixed by #3675 or #3682

Comments

@NikCharlebois
Copy link
Collaborator

NikCharlebois commented May 26, 2023

@andikrueger & @ykuijs I would like to revive this thread and propose we roll back the changes that introduced ID parameters as a primary key:

#2968

I had concerns with these changes and am now starting to hear from customers that this is causing them headaches. In my opinion, ID should never be required. Even if user can pass a fake value to work around it on creation, they should never be forced to do so. The whole point of DSC is to manage the entire lifecycle of resources. In our case, when the DSC config is used to create a new instance, we will never know the ID of it until it gets created. While the export process should always capture the ID of resources when generating the snapshot config, the resources should always support the cloning scenario. This means that the GET method should always try to retrieve the resource by ID if provided, then if it can't find it should fall back to a secondary key such as DisplayName.

@andikrueger
Copy link
Collaborator

Thanks for reviving this discussion. It’s a valid point check on the decision made.

Microsoft365DSC is a DSC module which already breaks with “traditional PowerShell DSC“ methodology and the underlying workflow. The immutable state of a configuration and a system is already loosened as there is no LCM within Microsoft 365 services. This bound requires us to implement the uniqueness of one resource within one configuration.
With Microsoft 365 DSC there could be several configurations that are applied from different host systems.

Further more we have an additional challenge about the unique identifiers for a resource within a configuration. Which we tried to solve by introducing the ID as required parameter.

The immutable ID of an object in Microsoft 365 is sometimes available at prior to the creation and sometimes not (see #2006 ). If not it is initiated during the first creation. All resources which are not unique prior to the creation of an object in the cloud would be the ones to consider for a change - or we might need to change how we handle/leverage DSC at all.

Supporting the cloning scenario requires to have compilable configurations. The first requirement for this are unique resources across the entire configuration.

If we would export the ID without it being a key parameter of the resource, we will still face issues with configuration, that can not be compiled. As long as the ID would not be the key parameter, we might face issues with not uniquely identifiable resources.

where as this ID key parameter makes it more challenging to create new objects in a tenant, it solve the clone and export/change/apply scenario.
For cloning it has one drawback - it will not be possible to run the same configuration twice, as the IDs would change after the initial setup.

overview of the current state

So right now, we can say, we fully support:

export/modify/apply

we solved:

export / create new

overview of rolling back

The ID change made is harder to

create/apply/monitor a configuration.

And I can absolutely understand why this will cause headaches.

Rolling back to the display name as key would result in:

Export/modify/apply will no longer work due to duplicates

Export and clone: failing due to duplicates

create apply monitor: might be failing due to the restriction of just having one resource with a given display name. The big question, should there be any manageable object more than once with the same display name.

yes/no/maybe

At the moment, I would say, we made it easier for those who clone a tenant or change a tenant based on the configuration.

It is more difficult for those who use DSC in the “on-premises” way. One configuration manager to setup, monitor and configure the tenant.

I’m still not convinced that the rollback would be solution. We are missing the immutable bound between a system and its configuration.

@NikCharlebois NikCharlebois pinned this issue Jul 21, 2023
@NikCharlebois
Copy link
Collaborator Author

I would like to propose that we move forward with the rollback and that we also establish a new standard in developing resources. I don't believe rolling this change back would result in a breaking change, but I could be wrong and further investigation will be required. I am hearing about lot of customers that are frustrated that we introduced this change which is casing them a lot of headaches when editing files manually.

  • The ID parameter should never be made required (even less a key), unless the resource type doesn't have a textual identifier such as a DisplayName, Name or Identity parameter.

  • Resources that expose an ID property as a GUID should always return it when doing an export.

  • The Get-TargetResource of resources having an ID parameter should ALWAYS start by trying to retrieve the resource instance by its ID field and then fall back to the key parameter if not provided.

  • If the Get-TargetResource function needs to fall back to the key parameter to get the instance (because it couldn't find it by ID) and that query results in more than 1 instances with the same key parameter, then we print out a warning (e.g., 2 or more AADApplication instances with the same display name). Logically, it makes no sense to have two instances of the same component with the same display name. Yes, it could happen like it does today with the AADApplication, but it DOESN'T make sense for us to force an arbitrary ID to be key just to get around these "edge" scenarios. AADApplication apart, things like policies and rules just wouldn't make sense if there were multiple instances with the same name? How could one then tell which is which without having to open each one up and scanning their differences?

@andikrueger
Copy link
Collaborator

I see the challenges of my customers and can anticipate what it must be like for those who have fully managed environments. Rolling back to a state with ID not being a key/requirement could be done without introducing a breaking change. I don’t see a reason, why it would be a breaking one.

The solution to modify the Get method is good and would be suitable in any scenario.

I have two more things in mind on how we could deal with ambiguous references between resources and enhance the processes:

  1. we could introduce a better dependency mapping between resources. E.g. any resource that references other objects should return the display name (the key parameter of the other object) and the ID. By having this string bound we will have a better mapping of objects.

  2. After the initial creation of an object we should write a log entry into the event log and make this information accessible under the name of the resources configuration within the configuration file. With a new function (e.g. Update-M365DSCIdentifierInConfiguration) that would extract the log information about the new IDs and the resource’s name within the configuration. The new function would then update the configuration file accordingly.

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

Successfully merging a pull request may close this issue.

2 participants