Azure / deployment-stacks

Contains Deployment Stacks CLI scripts and releases
MIT License
87 stars 7 forks source link

Problem with purging certain resources #31

Open slavizh opened 2 years ago

slavizh commented 2 years ago

Describe the bug Certain resources cannot co-exist if they have the same configuration defined. Such resource is for example diagnosticSettings. For example if for that resource you have defined certain logs to be sent to certain Log Analytics workspace and your resource name is called service1 but you rename it to service2. Deployment stacks will think that it needs to create service2 diagnosticSettings and delete service1 diagnosticSettings. The problem is that service1 and service2 will have the same configuration settings thus if service1 already exists deployment for service2 will fail. service1 is only deleted at the end of the deployment. This means that via deployment stacks you can never change a name for diagnosticSettings by creating new one with same configuration. Same behavior can be seen with scheduledqueryrules. Probably there are others.

I am not sure how this problem can be fully solved.

To Reproduce described above

Expected behavior to be able to change the name of diagnostic settings resource

Additional context Add any other context about the problem here.

bmoore-msft commented 2 years ago

This sounds like it's more related to the resource than the stack? I.e. the same behavior would occur with a template deployment independent of stacks. roleAssignments are similar/identical if I follow the scenario correctly.

One challenge I see with purging beforehand would be if the resource fails to deploy (you would lose the old ones and didn't get the new ones)... My guess is that would be unexpected. Also the behavior would either be inconsistent (i.e. it only works this way for some resources not all) or destructive (everything was deleted and nothing was deployed).

TBH, this feels like a "flaw" in the RP design, though I get why some resource do it (so you don't end up with wasteful duplication)

slavizh commented 2 years ago

yeah somehow not an easy way to solve this but decided to flag it anyway. I also get why certain resources does not allow that. Id not being able to solve it at least could be documented that you can achieve something like this with deployment script but of course as you have mentioned you can still end up in situation where old resource is deleted and new one is not applied due to failed deployment.

How are competitors doing in this situation?

bmoore-msft commented 2 years ago

IDK of a [competitor's] resource that has a similar limitation - i.e. the "primary key" is not the name property but a group of properties... but this is a pretty granular detail or obscurity. For roleAssignments you can manage it up front if you name the resource accordingly - is that possible with diags or is the property bag way too big?

slavizh commented 2 years ago

@bmoore-msft you can name diag resource however you want. The diag resource case is a little bit different than role assignments as I believe role assignments needs GUID for name and this is also not visible in portal. Diag resource name is visible on portal and sometimes it might be needed to switch to a naming standard or provide the option for the end user to define it.

Let me know if I didn't understand the question.

sqlkabouter commented 1 year ago

I was hoping that the deployment stack solve issues with changing guids in roleAssignments but it seems the new role roleAssignments is created before the old one is removed. So the deployment fails with the error that the roleAssignment already exists: The role assignment already exists. (Code: RoleAssignmentExists)

Posting this here as it seems to be related to the issue being discussed here.

I would expect the roleAssignment to be deleted because the resource (guid) is not present in my ARM template anymore. If the deployment fails for whatever reason and the new roleAssignment is not created yet then this would be exactly what I expect.

slavizh commented 1 year ago

@sqlkabouter these are overall problems with the RPs as they just do not allow having the same configuration even though the resource name is different. Specifically for role assignments you can do conditions and provide option for end user enter the name for the role assignment. If it is provided you will use that name and if not you can generate guid() based on the input configuration. At the end if I remember correctly for role assignments you can only provide guids and you cannot provide other types of strings so naming is irrelevant in terms what the name would be but it is important for browfield scenarios due to not being able to have role assignment with same configuration but different name.

sqlkabouter commented 1 year ago

@slavizh in a perfect world we would write code once and then never change it so the way the guid() is generated never changes. We would deploy the stack with the option DenyWriteAndDelete so nobody can add conflicting roleAssignments outside of the stack deployment.

In the real world you might encounter situations where people create roleAssignments manually or though some other way outside of the stack. Or maybe you wrote some Bicep module that generates a guid() for a roleAssignment and you realize it's not unique enough so you want to change the way the guid() is generated.

It would help if the deployment stack handles these types of situations.