fix: Stop storing ETags in state with the new client by benlangfeld · Pull Request #3693 · integrations/terraform-provider-github

benlangfeld · 2026-10-22T01:30:47Z

Resolves #3694

Before the change?

GitHub's ETag for an authenticated response depends on the credentials, not just the resource: two installation tokens for the same app see different ETags for the same unchanged repository. Every resource still writes that header into etag.
With app auth, each provider configuration mints a new token, so an unchanged resource's etag changes from run to run. Import and destroy plans print every attribute, so two plans of the same change never match. The org-mode acceptance tests hit this (e.g. TestAccGithubRepository/imports_repositories_without_error in run 39583616753), and it's why several tests pass ImportStateVerifyIgnore: []string{"etag"}.
Only the legacy client reads the stored value back (etagTransport turns it into If-None-Match). The new client revalidates through its transport cache, so with legacy_client = false the attribute has no consumer.

After the change?

Owner has a storeEtag field, set from LegacyClient in configureProviderMeta.
Every d.Set("etag", …) is wrapped in if meta.storeEtag { … } (or meta.(*Owner).storeEtag where the function has no local *Owner). The legacy client behaves exactly as before; the new client leaves etag unset. The schema is untouched.
The acceptance tests run with the new client (GITHUB_LEGACY_CLIENT: "false" in CI), so the etag entries in ImportStateVerifyIgnore are removed, as are the checks asserting etag is set, since with the new client it now isn't.
Unit tests (make test) pass. I haven't run the acceptance tests locally against this revision.

Pull request checklist

Schema migrations have been created if needed (example)
Tests for the changes have been added (for bug fixes / features)
Docs have been reviewed and added / updated if needed (for bug fixes / features)

Does this introduce a breaking change?

Please see our docs on breaking changes to help!

Yes
No

etag is unset with the new client, which is opt-in (legacy_client = false); nothing in the provider reads it in that mode.

🤖 Generated with Claude Code

github-actions · 2026-10-22T01:30:57Z

👋 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.}

benlangfeld · 2026-10-22T01:50:36Z

@stevehipwell @deiga @robert-crandall since you've all weighed in on the etag handling (#3103, #3291, #3474, #3637), would you mind taking a look at this one? It follows the direction from those threads: the schema is unchanged, the legacy client keeps its conditional requests, and with the new client etag is no longer filled from a token-dependent header. The cause is written up in #3694.

I'm leaving it as a draft for now because I can't mark it ready for review while I'm still waiting on reviews of my other open PRs (#3685 and #3658), but it's complete and I'd welcome feedback in the meantime.

stevehipwell

Please open an issue to request changes before opening a PR. There isn't a lot of maintainer bandwidth, and changes like this will be quicker to be implemented on our side than via an unsolicited PR.

benlangfeld · 2026-10-22T20:29:42Z

@stevehipwell This is a fundamental bug that's been amply documented in issues going back many months. I figured if someone else was going to fix it they would have done so by now.

I'll stop contributing then. Thanks.

stevehipwell · 2026-10-22T20:52:27Z

@benlangfeld which issue(s)? AFAIK all new client bugs have been fixed. Unfortunately we (the maintainers) are not free to merge code or cut releases without coordination with GitHub so there is a large backlog to get out. The whole process is highly time consuming and we're not getting any support outside of blocking reviews.

GitHub's ETag for an authenticated response depends on the credentials, so with app auth every new installation token sees a different ETag for the same unchanged resource, and import and destroy plans never match. Only the legacy client reads the stored etag back, to send If-None-Match; the new client revalidates through its own response cache. Owner now has a storeEtag flag, set only for the legacy client, and resources set etag only when it is true. Acceptance tests run with the new client, so they no longer ignore or assert etag. Resolves integrations#3694 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

stevehipwell

Thanks for making the changes @benlangfeld. I've added some comments, but the changes look good in principal.

Functions that read storeEtag through an inline meta.(*Owner) assertion now take m any and assert once with meta, _ := m.(*Owner), which the forcetypeassert linter accepts. The etag sets inside the storeEtag checks use the if err := d.Set(...); err != nil idiom. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…ping The repository read now returns an error from setting etag, like the other resources, rather than discarding it. The EMU group mapping reads the ETag header inside the storeEtag check instead of into a local beforehand, so it is only read and traced when it is stored. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Copilot

🟡 Changes recommended

Direct regression tests are needed to verify that ETag storage remains enabled for legacy clients and disabled for new clients.

1 open issue

Add regression coverage for client-dependent ETag storage · New

What changed in this PR

These provider review instructions are being used.

This PR addresses #3694 by preventing the new client's credential-dependent ETags from being written to Terraform state, reducing import and destroy plan differences under GitHub App authentication.

Changes:

Derives Owner.storeEtag from the legacy-client configuration.
Gates ETag writes across resources and data sources without changing schemas.
Removes ETag presence assertions and import-verification exclusions from affected acceptance tests.

File	Description
`github/resource_organization_block.go`	Gates organization-block ETag writes.
`github/resource_github_user_ssh_key.go`	Gates SSH-key ETag writes.
`github/resource_github_team.go`	Gates team lifecycle ETag writes.
`github/resource_github_team_test.go`	Removes ETag assertions and import exclusions.
`github/resource_github_team_sync_group_mapping.go`	Gates team-sync mapping ETag writes.
`github/resource_github_team_repository.go`	Gates team-repository ETag writes.
`github/resource_github_team_membership.go`	Gates team-membership ETag writes.
`github/resource_github_repository.go`	Gates repository ETag writes.
`github/resource_github_repository_ruleset.go`	Gates repository-ruleset ETag writes.
`github/resource_github_repository_ruleset_test.go`	Includes ETags in import verification.
`github/resource_github_repository_deployment_branch_policy.go`	Gates deployment-policy ETag writes.
`github/resource_github_repository_deployment_branch_policy_test.go`	Removes the ETag presence assertion.
`github/resource_github_repository_deploy_key.go`	Gates deploy-key ETag writes.
`github/resource_github_organization_webhook.go`	Gates organization-webhook ETag writes.
`github/resource_github_organization_webhook_test.go`	Includes ETags in import verification.
`github/resource_github_organization_ruleset.go`	Gates organization-ruleset ETag writes.
`github/resource_github_organization_ruleset_test.go`	Includes ETags in import verification.
`github/resource_github_membership.go`	Gates organization-membership ETag writes.
`github/resource_github_issue.go`	Gates issue ETag writes.
`github/resource_github_issue_test.go`	Includes ETags in import verification.
`github/resource_github_issue_label.go`	Gates issue-label ETag writes.
`github/resource_github_enterprise_actions_runner_group.go`	Gates enterprise runner-group ETag writes.
`github/resource_github_emu_group_mapping.go`	Gates EMU mapping ETag writes and associated logging.
`github/resource_github_emu_group_mapping_test.go`	Removes ETag assertions and import exclusions.
`github/resource_github_branch.go`	Gates branch ETag writes.
`github/resource_github_branch_protection_v3.go`	Gates branch-protection ETag writes.
`github/resource_github_branch_default.go`	Gates default-branch lifecycle and import ETag writes.
`github/resource_github_branch_default_test.go`	Removes ETag assertions and import exclusion.
`github/resource_github_actions_runner_group.go`	Gates organization runner-group ETag writes.
`github/provider.go`	Sets ETag storage mode from `LegacyClient`.
`github/data_source_github_ref.go`	Gates reference data-source ETag writes.
`github/data_source_github_membership.go`	Gates membership data-source ETag writes.
`github/data_source_github_membership_test.go`	Removes the ETag presence assertion.
`github/data_source_github_branch.go`	Gates branch data-source ETag writes.
`github/config.go`	Adds the `Owner.storeEtag` flag.

_{Review effort: Balanced}

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

stevehipwell

LGTM

robert-crandall

I checked the resource and data-source paths along with the Test_configureProviderMeta check and the ImportStateVerifyIgnore test cleanup, and everything looks good to me. Thank you for the contribution!

stevehipwell · 2026-10-28T18:40:36Z

@deiga do you want to review this or are you happy for me to merge it now?

benlangfeld mentioned this pull request Oct 22, 2026

[BUG]: etag in state changes with every installation token, so import and destroy plans never match under app auth #3694

Open

1 task done

benlangfeld changed the title ~~test: Show the repository etag changing with each installation token~~ fix: Stop storing ETags in state with the new client Oct 22, 2026

stevehipwell requested changes Oct 22, 2026

View reviewed changes

benlangfeld closed this Oct 22, 2026

benlangfeld reopened this Oct 23, 2026

benlangfeld force-pushed the etag-varies-per-token branch from d2d0ab8 to 882d74a Compare October 23, 2026 20:59

stevehipwell requested changes Oct 26, 2026

View reviewed changes

Comment thread github/data_source_github_branch.go Resolved

Comment thread github/data_source_github_branch.go Resolved

Comment thread github/resource_github_actions_runner_group.go Resolved

benlangfeld and others added 2 commits October 26, 2026 15:13

benlangfeld marked this pull request as ready for review October 26, 2026 18:29

stevehipwell added the Type: Bug Something isn't working as documented label Oct 26, 2026

stevehipwell added this to the v6.14.0 milestone Oct 26, 2026

stevehipwell requested a balanced review from Copilot October 26, 2026 22:14

Copilot started reviewing on behalf of stevehipwell October 26, 2026 22:15

View session

Copilot AI reviewed Oct 26, 2026

View reviewed changes

Comment thread github/provider.go Resolved

test: Check storeEtag follows the client in configureProviderMeta

2ffa6a7

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

stevehipwell approved these changes Oct 27, 2026

View reviewed changes

stevehipwell added the needs-github-review Request a review from GitHub label Oct 27, 2026

stevehipwell requested a review from deiga October 27, 2026 17:46

team-review-router Bot requested a review from a team October 27, 2026 17:46

robert-crandall approved these changes Oct 28, 2026

View reviewed changes

Navigation Menu

Repository navigation

fix: Stop storing ETags in state with the new client - #3693

fix: Stop storing ETags in state with the new client#3693
benlangfeld wants to merge 4 commits into
integrations:mainfrom
benlangfeld:etag-varies-per-token

benlangfeld commented Oct 22, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Oct 22, 2026

Uh oh!

benlangfeld commented Oct 22, 2026

Uh oh!

stevehipwell left a comment

Uh oh!

benlangfeld commented Oct 22, 2026

Uh oh!

stevehipwell commented Oct 22, 2026

Uh oh!

stevehipwell left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

stevehipwell left a comment

Uh oh!

robert-crandall left a comment

Uh oh!

stevehipwell commented Oct 28, 2026

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

benlangfeld commented Oct 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Before the change?

After the change?

Pull request checklist

Does this introduce a breaking change?

Uh oh!

github-actions Bot commented Oct 22, 2026

Uh oh!

benlangfeld commented Oct 22, 2026

Uh oh!

stevehipwell left a comment

Choose a reason for hiding this comment

Uh oh!

benlangfeld commented Oct 22, 2026

Uh oh!

stevehipwell commented Oct 22, 2026

Uh oh!

stevehipwell left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

stevehipwell left a comment

Choose a reason for hiding this comment

Uh oh!

robert-crandall left a comment

Choose a reason for hiding this comment

Uh oh!

stevehipwell commented Oct 28, 2026

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

benlangfeld commented Oct 22, 2026 •

edited

Loading