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

Changing value of an organization secrets deletes it #111

Closed
mastoj opened this issue Mar 19, 2021 · 7 comments
Closed

Changing value of an organization secrets deletes it #111

mastoj opened this issue Mar 19, 2021 · 7 comments
Labels
kind/question Questions about existing features resolution/fixed This issue was fixed

Comments

@mastoj
Copy link

mastoj commented Mar 19, 2021

I have an automation job where I want to create and also update organization secrets. Creating them works fine. When I do an update to the secrets it for some reason gets deleted. It is still visible in the resource list of the stack, but the secret has been deleted in github.

Pulumi version: 2.22.0
pulumi/github version: 3.1.0
Language: typescript

@mastoj mastoj added the kind/bug Some behavior is incorrect or out of spec label Mar 19, 2021
@pierskarsenbarg
Copy link
Member

Managed to reproduce this:

import * as pulumi from "@pulumi/pulumi";
import * as github from "@pulumi/github";
import * as random from "@pulumi/random";

const secret = new random.RandomString("secret", {
    length:  15
});
const orgSecret = new github.ActionsOrganizationSecret("orgSecret", {
    plaintextValue: secret.result,
    secretName: "mysecret",
    visibility: "all"
});

Then change the length of the secret, run pulumi up again. Pulumi says that the secret has been replaced (and the stack says that it's still there) but the secret has been deleted in GitHub.

Diff when changing the secret:

Previewing update (dev)

View Live: https://app.azureselfhosted.pulumi-ce.team/piers/github-secrets/dev/previews/5b769ca7-5e93-4936-b390-dc98cd19f7f8

  pulumi:pulumi:Stack: (same)
    [urn=urn:pulumi:dev::github-secrets::pulumi:pulumi:Stack::github-secrets-dev]
    +-random:index/randomString:RandomString: (replace)
        [id=2bHD]rjo@cGi9lH]
        [urn=urn:pulumi:dev::github-secrets::random:index/randomString:RandomString::secret]
        [provider=urn:pulumi:dev::github-secrets::pulumi:providers:random::default_3_1_0::8cb37cd7-509c-41c8-a866-73e8e0fb9f42]
      ~ length: 15 => 20
    +-github:index/actionsOrganizationSecret:ActionsOrganizationSecret: (replace)
        [id=mysecret]
        [urn=urn:pulumi:dev::github-secrets::github:index/actionsOrganizationSecret:ActionsOrganizationSecret::orgSecret]
        [provider=urn:pulumi:dev::github-secrets::pulumi:providers:github::default_3_3_0::0b71d0ea-275c-4084-8de1-d0304385bd08]
      ~ plaintextValue: "[secret]" => output<string>
Resources:
    +-2 to replace
    1 unchanged
Updating (dev)

View Live: https://app.azureselfhosted.pulumi-ce.team/piers/github-secrets/dev/updates/24

  pulumi:pulumi:Stack: (same)
    [urn=urn:pulumi:dev::github-secrets::pulumi:pulumi:Stack::github-secrets-dev]
    +-random:index/randomString:RandomString: (replace)
        [id=2bHD]rjo@cGi9lH]
        [urn=urn:pulumi:dev::github-secrets::random:index/randomString:RandomString::secret]
        [provider=urn:pulumi:dev::github-secrets::pulumi:providers:random::default_3_1_0::8cb37cd7-509c-41c8-a866-73e8e0fb9f42]
      ~ length: 15 => 20
        --outputs:--
      ~ id        : "2bHD]rjo@cGi9lH" => "@&F#@]iiE1vh3!dJniq>"
    +-github:index/actionsOrganizationSecret:ActionsOrganizationSecret: (replace)
        [id=mysecret]
        [urn=urn:pulumi:dev::github-secrets::github:index/actionsOrganizationSecret:ActionsOrganizationSecret::orgSecret]
        [provider=urn:pulumi:dev::github-secrets::pulumi:providers:github::default_3_3_0::0b71d0ea-275c-4084-8de1-d0304385bd08]
      ~ plaintextValue: "[secret]" => "[secret]"
        --outputs:--
      ~ updatedAt     : "2021-03-19 13:18:23 +0000 UTC" => "2021-03-19 13:19:18 +0000 UTC"
Resources:
    +-2 replaced
    1 unchanged

Duration: 5s

@leezen
Copy link

leezen commented Mar 19, 2021

I'm guessing this is an outcome of attempting to create before delete for the replacement. If you specify deleteBeforeReplace -- does this work as expected?

@mastoj
Copy link
Author

mastoj commented Mar 19, 2021

@leezen , that worked. I guess it is for scenarios like this that deleteBeforeReplace exists. If that is the case this can be close.

@leezen leezen added kind/question Questions about existing features resolution/fixed This issue was fixed and removed kind/bug Some behavior is incorrect or out of spec labels Mar 20, 2021
@leezen
Copy link

leezen commented Mar 20, 2021

Great!

@leezen leezen closed this as completed Mar 20, 2021
@alex-hunt-materialize
Copy link

This should be reopened. deleteBeforeReplace is a workaround, not a proper fix for this. It should not delete the object at all if it is operating on the same secret. The issue is not the order, but that pulumi thinks it needs to replace it in the first place. It should be an update operation, not a delete and create. Pulumi is operating on a single resource, not two instances of it.

deleteBeforeReplace has side effects beyond just the order of those operations for that one resource.

Be aware that this behavior has a cascading impact on dependencies so more resources may be replaced, which can lead to downtime.

It is also extremely unintuitive that a user would need to specify deleteBeforeReplace. From the user perspective, they shouldn't be deleting the resource at all, just updating the value.

@guineveresaenger
Copy link
Contributor

hi @alex-hunt-materialize! I did some digging for you and:

  1. you are correct, deleteBeforeReplace is a workaround in this case

  2. unfortunately, the reason we are relying on this workaround is that the upstream provider implementation for updating actions organization secrets (and, incidentally, action environment secrets) has some timestamp-related reconciliation logic built in that is being hit here.

More info here:

@alex-hunt-materialize
Copy link

Thank you so much for the quick response.

I get that your hands are tied by the upstream provider, but if I'm reading that code correctly, shouldn't it only hit that when the update timestamp changes outside of pulumi/terraform? In our case, we're seeing this behavior even though pulumi is the sole modifier of the secret.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/question Questions about existing features resolution/fixed This issue was fixed
Projects
None yet
Development

No branches or pull requests

5 participants