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

Pulumi plan reports false positive changes of dependants resources #3972

Open
JiriKovar opened this issue Feb 20, 2025 · 4 comments
Open

Pulumi plan reports false positive changes of dependants resources #3972

JiriKovar opened this issue Feb 20, 2025 · 4 comments
Labels
area/providers kind/bug Some behavior is incorrect or out of spec

Comments

@JiriKovar
Copy link

JiriKovar commented Feb 20, 2025

Hello!

  • Vote on this issue by adding a 👍 reaction
  • If you want to implement this feature, comment to let us know (we'll work with you on design, scheduling, etc.)

Issue details

A slight infrastructure drift in the infrastructure causes the Pulumi to not only fix that drift (correct), but preview also reports that dependant resources are to be changed / recreated based on a property that hasnt been changed and wont be changed.

Specific example: Someone enabled extensive logging to investigate an issue on an azure-native.web.WebApp resource (i.e. via app configuration change, or flipping some of the switches in the App Service logs tab). The Azure App Service has a Managed Service Identity enabled, which is used as an input for the resources azuread.GroupMember and azure-native.documentdb.SqlResourceSqlRoleAssignment. There were no changes in the Pulumi code whatsoever.

With this setup, the Pulumi preview command reports that it plans to recreate both the of the dependants, while Pulumi up command does nothing to them. Running Pulumi refresh does not help & sice the ID of the Managed Service Identity is an output that the Pulumi pretends not to know, we cannot validate what will actually happen. In another words, there is nothing we can do except for pushing the button and hoping for the best. This behaviour is quite inconvenient, because recreating the permissions, if it really happened, would either fail (because the same assignment cannot be created twice), or in the worst case scenario, would cause a short term down time of the service (which is unacceptable due to our SLA).

We have seen this behaviour not only in case of an infrastructure drift, but also minor changes to irelevant properties that cause no change to the properties the dependants depend on.

Affected area/feature

I believe this is unfortunately by the current design of the Pulumi preview and not localized to the aforementioned resource types / providers. That being said, Terraform (which we used to use before Pulumi) could handle these situations by detecting whether a specific property that creates the dependency will change or not and we would very much appreciate this to be improved towards a similar behaviour for the sake of our sanity.

@JiriKovar JiriKovar added kind/enhancement Improvements or new features needs-triage Needs attention from the triage team labels Feb 20, 2025
@otahirs
Copy link
Contributor

otahirs commented Feb 20, 2025

Example:

  1. resource, be it WebApp or VirtualMachine has assigned identity and principalId can be obtained from the output:
const principalId = webApp.identity.principalId;
// in reality we need to cast the type here, as typescript complains the identity can be undefined, which we know is not the case as it is set to SystemAssigned, but that is another non-relevant issue
// const principalId = (webApp.identity as pulumi.Output<{ principalId: string }>).principalId;
  1. the principalId is used for azuread.GroupMember or azureNative.authorization.RoleAssignment
new azuread.GroupMember(`kv-reader`, {
      groupObjectId'00000000-0000-0000-0000-000000000000', // uuids redacted
      memberObjectIdprincipalId,
},{});
  1. when any property of WebApp is changed, the GroupMember resource is shown in preview as to be replaced
+-azuread:index/groupMember:GroupMember: (replace)
2025-02-18T13:37:53.9619215Z  [id=00000000-0000-0000-0000-000000000000/member/00000000-0000-0000-0000-000000000000]
2025-02-18T13:37:53.9619816Z  [urn=*redacted*$azuread:index/groupMember:GroupMember::kv-reader]
2025-02-18T13:37:53.9620451Z  [provider=*redacted*::pulumi:providers:azuread::default_6_2_0::ef9a45f2-4da5-4842-aece-99b04eb191a4]
2025-02-18T13:37:53.9620994Z   ~ memberObjectId: "00000000-0000-0000-0000-000000000000" => output<string>

tldr;
any minor change to properties (e.g. changes to tags) of resources with principalId in the output can cascade into a lot of false positive replaces down the dependency graph

@Frassle
Copy link
Member

Frassle commented Feb 20, 2025

I believe this is unfortunately by the current design of the Pulumi preview and not localized to the aforementioned resource types / providers. That being said, Terraform (which we used to use before Pulumi) could handle these situations by detecting whether a specific property that creates the dependency will change or not and we would very much appreciate this to be improved towards a similar behaviour for the sake of our sanity.

Pulumi should also detect if a specific property is going to change or not, however it depends on the resource provider to give it that information accurately.

In this case it looks like the change to webApp is causing the provider to return it's dentity.principalId as "unknown". When pulumi then asks the provider what could happen to the GroupMember if memberObjectId is unknown the provider (probably correctly) replies that it might need to replace (replace diffs are pessimistic, better to say we're going to replace and then don't rather than saying we don't need to replace and then we do).

So I think this is actually an azure-native bug that it can't tell the change to the WebApp should leave its other outputs as they are.

@Frassle Frassle transferred this issue from pulumi/pulumi Feb 20, 2025
@JiriKovar
Copy link
Author

So I think this is actually an azure-native bug that it can't tell the change to the WebApp should leave its other outputs as they are.

Thank you very much for the quick response. However this is not a good news - it means that we will probably need to report similar issues case by case as there is no "single point of failure".

@Frassle
Copy link
Member

Frassle commented Feb 20, 2025

it means that we will probably need to report similar issues case by case as there is no "single point of failure".

Unfortunately that is the case. I don't think the engine can do anything to help here. If a provider says a resource might need replacement if a property is changing we have to respect that, and if a provider says an output might change based on a input change we have to respect that as well.

If you think there's something we're missing here I'll listen to ideas but this feels like just a case of needing the right information, and its the various resource providers that have that information.

@thomas11 thomas11 added kind/bug Some behavior is incorrect or out of spec area/providers and removed kind/enhancement Improvements or new features needs-triage Needs attention from the triage team labels Feb 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/providers kind/bug Some behavior is incorrect or out of spec
Projects
None yet
Development

No branches or pull requests

4 participants