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.

Sign up for GitHub

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

Jump to bottom

Aliasing allows two definitions of the same resource to be applied #11173

Closed
sfc-gh-jsirak opened this issue Oct 27, 2022 · 0 comments · Fixed by #11212
Closed

Aliasing allows two definitions of the same resource to be applied #11173

sfc-gh-jsirak opened this issue Oct 27, 2022 · 0 comments · Fixed by #11212
Assignees
@Frassle
Labels
area/engine Pulumi engine kind/bug Some behavior is incorrect or out of spec resolution/fixed This issue was fixed
Milestone
0.80

Comments

@sfc-gh-jsirak
Copy link

sfc-gh-jsirak commented Oct 27, 2022 •
edited

What happened?

As part of a refactoring, we attempted to rename all our pulumi resources with a consistent pattern using the Alias feature. Our changes previewed and applied with out an issue, but when we attempted to apply new changes, pulumi failed with error: post-step event returned an error: failed to verify snapshot: duplicate resource <urn>. As part of the renaming of our pulumi resources, we also have relocated them to a different place in the code, but apparently, we forgot to clean up some of the old resources with the old name. Typically, we would have gotten a duplicate resource error on pulumi preview. However, the use of aliasing seems to have prevented pulumi from erroring on the presence of duplicate resources.

Steps to reproduce

The change can be reproduced using a dev vault server:

  1. Create a vault dev server and grab the vault address and vault
  2. Create a new pulumi project and stack
  3. Create a vault provider and a vault policy like:
Pulumi Go code 
      
     package main
      
      import (
          "os"
          "github.com/pulumi/pulumi-vault/sdk/v5/go/vault"
          "github.com/pulumi/pulumi/sdk/v3/go/pulumi"
      )
      
      func main() {
          pulumi.Run(func(ctx *pulumi.Context) error {
      
  	        vaultAddr := os.Getenv("VAULT_ADDR")
  	        token := os.Getenv("VAULT_TOKEN")
      
  	        _, err := vault.NewProvider(ctx, "vault", &vault.ProviderArgs{
  		        Address: pulumi.String(vaultAddr),
  		        Token:   pulumi.ToSecret(pulumi.String(token)).(pulumi.StringOutput),
  	        })
  	        if err != nil {
  		        return err
  	        }
      
  	        _, err = vault.NewPolicy(ctx, "aws_policy_test", &vault.PolicyArgs{
  		        Name:   pulumi.String("test"),
  		        Policy: pulumi.String("path \"secret/data/test/*\" {\n  capabilities = [\"read\"]\n}\n"), 
                      })
  	        if err != nil {
  		        return err
  	        }
      
  	        return nil
          })
      }
  1. Run pulumi up and create the resources
  2. Add a new vault policy resource definition with aliasing pointing to the old resource and keep the old resource definition:
Pulumi Go code 
    package main

    import (
      "os"

      "github.com/pulumi/pulumi-vault/sdk/v5/go/vault"
      "github.com/pulumi/pulumi/sdk/v3/go/pulumi"
    )

    func main() {
      pulumi.Run(func(ctx *pulumi.Context) error {

        vaultAddr := os.Getenv("VAULT_ADDR")
        token := os.Getenv("VAULT_TOKEN")

        _, err := vault.NewProvider(ctx, "vault", &vault.ProviderArgs{
          Address: pulumi.String(vaultAddr),
          Token:   pulumi.ToSecret(pulumi.String(token)).(pulumi.StringOutput),
        })
        if err != nil {
          return err
        }

        _, err = vault.NewPolicy(ctx, "aws_policy_test", &vault.PolicyArgs{
          Name:   pulumi.String("test"),
          Policy: pulumi.String("path \"secret/data/test/*\" {\n  capabilities = [\"read\"]\n}\n"),
        })
        if err != nil {
          return err
        }

        _, err = vault.NewPolicy(ctx, "aws_policy_new_resource_name", &vault.PolicyArgs{
          Name:   pulumi.String("test"),
          Policy: pulumi.String("path \"secret/data/test/*\" {\n  capabilities = [\"read\"]\n}\n"),
        }, pulumi.Aliases([]pulumi.Alias{{Name: pulumi.String("aws_policy_test")}}))
        if err != nil {
          return err
        }

        return nil
      })
    }
  1. Run pulumi up and you will see that there are no changes, but apply it anyway.
Pulumi up with no change 
pulumi:pulumi:Stack: (same)
    [urn=urn:pulumi:dev::alias::pulumi:pulumi:Stack::alias-dev]
Resources:              
    4 unchanged
Do you want to perform this update? yes
Updating (dev)
 pulumi:pulumi:Stack: (same)
    [urn=urn:pulumi:dev::alias::pulumi:pulumi:Stack::alias-dev]
Resources:     
    4 unchanged
  1. Now run pulumi up again and you will see that there are no changes, but apply it anyway. This will result in error:
Pulumi up with no change 
      pulumi:pulumi:Stack: (same)
        [urn=urn:pulumi:dev::alias::pulumi:pulumi:Stack::alias-dev]
    Resources:              
        4 unchanged
    Do you want to perform this update? yes
    pulumi:pulumi:Stack: (same)
        [urn=urn:pulumi:dev::alias::pulumi:pulumi:Stack::alias-dev]
    error: post-step event returned an error: failed to verify snapshot: duplicate resource urn:pulumi:dev::alias::vault:index/policy:Policy::aws_policy_new_resource_name (not marked for deletion)
    error: update failed
    Resources:
        4 unchanged

Expected Behavior

We expected that pulumi preview would error out because of the duplicate resource definition wether one of the resources is an alias of the other or not.

Actual Behavior

Pulumi instead allowed two definitions of the same resource to be applied, which resulted in breaking the state. Also, because both the preview and apply were successful, we only discovered when trying to make new changes. This broke all our stacks including productions and it required manually deleting the duplicate resources from state in all our pulumi stacks.

Output of pulumi about

CLI          
Version      3.43.1
Go Version   go1.19.2
Go Compiler  gc

Plugins
NAME   VERSION
aws    5.13.0
go     unknown
vault  5.7.0

Host     
OS       darwin
Version  12.6
Arch     x86_64

This project is written in go: executable='/usr/local/bin/go' version='go version go1.19 darwin/amd64'

Current Stack: dev

TYPE                       URN
pulumi:pulumi:Stack        urn:pulumi:dev::alias::pulumi:pulumi:Stack::alias-dev
pulumi:providers:vault     urn:pulumi:dev::alias::pulumi:providers:vault::default
pulumi:providers:vault     urn:pulumi:dev::alias::pulumi:providers:vault::vault
vault:index/policy:Policy  urn:pulumi:dev::alias::vault:index/policy:Policy::aws_policy_test
vault:index/policy:Policy  urn:pulumi:dev::alias::vault:index/policy:Policy::aws_policy_new_resource_name
vault:index/policy:Policy  urn:pulumi:dev::alias::vault:index/policy:Policy::aws_policy_new_resource_name


Found no pending operations associated with dev

Backend        
Name         XXX
URL           XXXX
User           josephsirak
Organizations  josephsirak, XXXX

Dependencies:
NAME                                 VERSION
github.com/pulumi/pulumi-aws/sdk/v5  5.13.0
github.com/pulumi/pulumi/sdk/v3      3.39.1

Additional context

I have only tested this on vault pulumi resources. It quite likely that this is a larger issue with Alias implementation and not specific to vault resource definitions.

Contributing

Vote on this issue by adding a 👍 reaction.
To contribute a fix for this issue, leave a comment (and link to your pull request, if you've opened one already).
@sfc-gh-jsirak sfc-gh-jsirak added kind/bug Some behavior is incorrect or out of spec needs-triage Needs attention from the triage team labels Oct 27, 2022
@Frassle Frassle transferred this issue from pulumi/pulumi-vault Oct 27, 2022
@Frassle Frassle self-assigned this Oct 31, 2022
@Frassle Frassle added area/engine Pulumi engine and removed needs-triage Needs attention from the triage team labels Oct 31, 2022
Frassle added a commit that referenced this issue Nov 1, 2022
@Frassle
Check for duplicate aliases and plain URNs
9751b2e 
We need to check that for example that if a resource X is created that
we don't allow another resource Y to be alias against X. Likewise if our
old state has X and we then create Y aliased against X we should not
then allow X to be created later in the deployment.

Fixes #11173
@Frassle Frassle mentioned this issue Nov 1, 2022
Merged
3 tasks
Frassle added a commit that referenced this issue Nov 1, 2022
@Frassle
Check for duplicate aliases and plain URNs
6314f7c 
We need to check that for example that if a resource X is created that
we don't allow another resource Y to be alias against X. Likewise if our
old state has X and we then create Y aliased against X we should not
then allow X to be created later in the deployment.

Fixes #11173
bors bot added a commit that referenced this issue Nov 1, 2022
@bors @Frassle
Merge #11212
6754c38 
11212: Check for duplicate aliases and plain URNs r=Frassle a=Frassle



<!--- 
Thanks so much for your contribution! If this is your first time contributing, please ensure that you have read the [CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md) documentation.
-->

# Description

<!--- Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. -->

We need to check that for example that if a resource X is created that we don't allow another resource Y to be alias against X. Likewise if our old state has X and we then create Y aliased against X we should not then allow X to be created later in the deployment.

Fixes #11173

## Checklist

<!--- Please provide details if the checkbox below is to be left unchecked. -->
- [x] I have added tests that prove my fix is effective or that my feature works
<!--- 
User-facing changes require a CHANGELOG entry.
-->
- [x] I have run `make changelog` and committed the `changelog/pending/<file>` documenting my change
<!--
If the change(s) in this PR is a modification of an existing call to the Pulumi Service,
then the service should honor older versions of the CLI where this change would not exist.
You must then bump the API version in /pkg/backend/httpstate/client/api.go, as well as add
it to the service.
-->
- [ ] Yes, there are changes in this PR that warrants bumping the Pulumi Service API version
  <!-- `@Pulumi` employees: If yes, you must submit corresponding changes in the service repo. -->


Co-authored-by: Fraser Waters <fraser@pulumi.com>
@bors bors bot closed this as completed in 25bd778 Nov 1, 2022
@pulumi-bot pulumi-bot added the resolution/fixed This issue was fixed label Nov 1, 2022
@lukehoban lukehoban added this to the 0.80 milestone Nov 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Frassle Frassle

Labels
area/engine Pulumi engine kind/bug Some behavior is incorrect or out of spec resolution/fixed This issue was fixed
Projects
None yet
Milestone
0.80
Development

Successfully merging a pull request may close this issue.

Check for duplicate aliases and plain URNs pulumi/pulumi
4 participants
@lukehoban @Frassle @pulumi-bot @sfc-gh-jsirak