Replace user and organization secrets on repository selection changes

[cschramm]

Due to the terraform provider's drift detection mechanism (e.g. ActionsOrganizationSecret) and seemingly a mismatch between the updated_at values that Pulumi passes down and those that the provider gets from GitHub, secrets cannot get updated as they are always considered removed, so any property change requires a replacement and selectedRepositoryIds is no exception.

[github-actions[bot]]

PR is now waiting for a maintainer to run the acceptance tests. Note for the maintainer: To run the acceptance tests, please comment /run-acceptance-tests on the PR

[iwahbe]

Hey @cschramm. Thanks for opening a PR. I'm trying to understand the underlying problem.

You are setting to willReplaceOnChanges:

github:index/actionsOrganizationSecret:ActionsOrganizationSecret.selectedRepositoryIds
github:index/codespacesOrganizationSecret:CodespacesOrganizationSecret.properties.selectedRepositoryIds
github:index/dependabotOrganizationSecret:DependabotOrganizationSecret.selectedRepositoryIds

Is this because pulumi-github is behaving differently from terraform-provider-github, or for consistency in the docs? I ask because I'm trying to figure out if the fix should live in this repo or in terraform-provider-github.

[cschramm]

We're using ActionsOrganizationSecret and DependabotOrganizationSecret like

    github.ActionsOrganizationSecret(
        name,
        secret_name=name,
        visibility="selected",
        encrypted_value=encrypted_value,
        selected_repository_ids=internal_repo_ids,
        opts=pulumi.ResourceOptions(delete_before_replace=True),
    )

Now if we change (add to) internal_repo_ids, pulumi does update the secret correctly, but then fails with Resource provider reported that the resource did not exist while updating leading to an incorrect stack state.

If I'm not mistaken, that is due to the terraform provider 's drift detection mechanism: When it reads the resource, it checks its updated_at value to detect external updates and mark the resource for deletion if it has been externally updated. Of course that should not be the case right after an update, but it seems that the check will actually always find a mismatch as GitHub 's updated_at is in format YYYY-MM-DDThh:mm:ssZ (and not expected for new resources) while the one handed in by pulumi is formatted like YYYY-MM-DD hh:mm:ss +00:00 UTC (and always set?).

The fact that the terraform provider has that check in its read logic and pulumi reads back resources after updating them, effectively means that updates can never fully work for those resources as they are always considered removed afterward. Avoiding updates by forcing replacements helps with that and willReplaceOnChanges is actually already set for all properties anyway, but for some reason not for selectedRepositoryIds.

[guineveresaenger]

Hi @cschramm - thank you for the explanation. You're correct that we're working around this in other resources in this provider, and I think we should make these changes.

update: I see you're already setting delete_before_replace. I'm not sure why willReplaceOnChanges isn't set on this resource, but it sounds like it would be an upstream fix then.

[cschramm]

-        opts=pulumi.ResourceOptions(delete_before_replace=True),
+        opts=pulumi.ResourceOptions(
+            delete_before_replace=True, replace_on_changes=["selected_repository_ids"]
+        ),

is our workaround since we found the issue.

I think those resources cannot possibly work correctly without delete_before_replace either and not having to specify it (after running into issues :see_no_evil:) would definitely be another improvement, but that seems like a different topic (possibly connected to the drift detection mechanism as well?) and I did not look into it close enough to know which resources are affected (and why).

[guineveresaenger]

Hi @cschramm - thank you for your patience here, and thank you for providing a workaround.

Here's the way to fix this for good on the pulumi side. Let me know if you're up to the fix, and if not, we can take it from here.

1. Our schema changes are generated via https://github.com/pulumi/pulumi-github/blob/master/provider/resources.go.

2. You'll want to add ForceNew to the selected_repository_ids field of each resource with this problem, and also set DeleteBeforeReplace on the resource.

   "github_dependabot_organization_secret": {
   Tok:  makeResource(mainMod, "DependabotOrganizationSecret"),
   Docs: &tfbridge.DocInfo{AllowMissing: true},
   Fields: map[string]*tfbridge.SchemaInfo{
     "selected_repository_ids": {
     ForceNew: tfbridge.True(),
     },
   },
   DeleteBeforeReplace: true,
   },

3. Generate the schema: make tfgen
4. Generate the SDKs: make build_sdks

[cschramm]

Let me know if you're up to the fix, and if not, we can take it from here.

You're welcome to take it. Thanks!