Skip to content

fix: treat 404 as already-deleted in organization_custom_properties Read/Delete - #3642

Open
gilfthde wants to merge 1 commit into
integrations:mainfrom
gilfthde:fix-organization-custom-property-404-on-delete
Open

gilfthde wants to merge 1 commit into
integrations:mainfrom
gilfthde:fix-organization-custom-property-404-on-delete

Conversation

@gilfthde

Copy link
Copy Markdown

Description

resourceGithubCustomPropertiesRead propagated any error from GetCustomProperty verbatim, including a 404 for a property that no longer exists on GitHub. This breaks the standard Terraform "resource already gone" recovery convention: Read should call d.SetId("") and return nil when the remote resource is confirmed absent, not surface a hard error.

In practice this means: once the underlying custom property is deleted (whether by terraform destroy, an external actor, or any controller built on this provider that re-observes state after a delete), the next Read never converges - it keeps reporting a 404 as a reconcile error instead of confirming deletion.

resourceGithubCustomPropertiesDelete has the same gap: removing an already-removed property currently errors, instead of being treated as a no-op.

Fix

Mirrors the handling already used by the repo-scoped sibling, resource_github_repository_custom_property.go's Read/Delete (errors.AsType[*github.ErrorResponse](err) with StatusCode == 404).

Testing

  • go build ./github/... and go vet ./github/... pass.
  • No existing acceptance test in this repo exercises the disappears/404 case for either the org- or repo-scoped custom property resource, so none was added here, consistent with the existing test coverage for this resource; happy to add one if maintainers want it and can point me at credentials/CI conventions for it.

Fixes #3641

…ead/Delete

resourceGithubCustomPropertiesRead propagated any error from
GetCustomProperty verbatim, including a 404 for a property that no longer
exists. This breaks the standard Terraform "resource already gone" recovery
path: Read must call d.SetId("") and return nil when the remote resource
is confirmed absent, not return a hard error.

This left Read to poll a deleted resource and report the delete as still
failed until it was manually removed from state. The 404 also could be hit
right after a successful Delete call re-observes state, matching the pattern
already used in resource_github_repository_custom_property.go's Read (via
errors.AsType[*github.ErrorResponse] with StatusCode == 404).

Delete gets the same treatment: removing an already-removed property should
be a no-op, not an error, again mirroring the repository-scoped sibling.

Fixes integrations#3641
@github-actions

Copy link
Copy Markdown

👋 Hi, and thank you for this contribution!

This repo is maintained by GitHub and community members on a best-effort basis. We'll get to this as soon as we can.

You can help us prioritize by joining the discussion on open issues and PRs, sharing details on the changes you need, and reviewing other contributions.


🤖 This is an automated message.

@github-actions github-actions Bot added the Type: Bug Something isn't working as documented label Sep 14, 2026
@deiga

deiga commented Sep 14, 2026

Copy link
Copy Markdown
Collaborator

@gilfthde

Copy link
Copy Markdown
Author

Hello @deiga, the mentioned PR is open since February. I see that there is recent activity, but is there any chance to get this simple fix merged in advance? It is affecting us in production.

@deiga

deiga commented Sep 16, 2026

Copy link
Copy Markdown
Collaborator

@gilfthde there are so many bugs in custom properties currently, that I doubt there will be a release without #3234, so merging this would just slow that PR down due to merge conflicts

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Type: Bug Something isn't working as documented

Projects

None yet

Development

Successfully merging this pull request may close these issues.

github_organization_custom_properties Read doesn't handle 404, breaking recovery after deletion

2 participants