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

Upgrading provider version causes a terraform plan on merge commit message/title #1295

Closed
garrettheel opened this issue Sep 20, 2022 · 10 comments
Labels
hacktoberfest Issues for participation in Hacktoberfest Status: Stale Used by stalebot to clean house Status: Up for grabs Issues that are ready to be worked on by anyone Type: Bug Something isn't working as documented

Comments

@garrettheel
Copy link
Contributor

Opening an issue on behalf of @danielpops and @chaosaffe from discussion in #1263. This would have began as of v4.31.0.

This PR causes a plan like so to be generated on repositories:

  # github_repository.repo["the_repo"] will be updated in-place
  ~ resource "github_repository" "repo" {
        id                          = "the_repo"
      + merge_commit_message        = "PR_TITLE"
      + merge_commit_title          = "MERGE_MESSAGE"
        name                        = "the_repo"
      + squash_merge_commit_message = "COMMIT_MESSAGES"
      + squash_merge_commit_title   = "COMMIT_OR_PR_TITLE"
        # (28 unchanged attributes hidden)
}

Presumably this plan may confuse users since it says something is going to happen, however the changes are simply setting new defaults and they are safe to apply.

@garrettheel
Copy link
Contributor Author

garrettheel commented Sep 20, 2022

@danielpops I think you're saying that the impact is that a plan is shown even though there is no effective change (the "new values" are simply the defaults, they just didn't exist in the tf state previously).

I don't know of a way to avoid terraform showing a plan difference here. Setting no default values would still display a plan like:

  ~ repository = {
      - branches                                = [] -> null
        id                                      = "repo"
      + merge_commit_message                    = null
      + merge_commit_title                      = null
        name                                    = "repo"
      + squash_merge_commit_message             = null
      + squash_merge_commit_title               = null
        # (34 unchanged elements hidden)
    }

This seems like one of those unavoidable issues given how terraform currently tracks state, and I can't find a way to avoid it and still add properties to the repository object over time. If someone know of a better solution then I'm happy to take a look and implement it.

@scruplelesswizard
Copy link

scruplelesswizard commented Oct 12, 2022

If preferred I can open a separate issue, as this doesn't fully encompass the issues we are experiencing.

The challenge on my side is that our org doesn't allow for merge or squash commits. The GH API doesn't allow setting defaults for merge and squash titles and messages while those merge types are disabled, putting us in a state where we are unable to upgrade the Github provider.

While the addition in the plan is inconvenient, adding a requirement for merge and squash defaults breaks the provider for anyone with merge or squash disabled on a repo.

@garrettheel
Copy link
Contributor Author

@chaosaffe I can't repro your issue. Here's what I tried:

  1. Create a repo with an older version of the provider (4.30.0) with allow_merge_commit = false and allow_squash_merge = false
  2. Bump the provider version to latest (~> 5)
  3. Apply a plan that looks like below:
  ~ resource "github_repository" "private" {
        id                          = "test-repo"
      + merge_commit_message        = "PR_TITLE"
      + merge_commit_title          = "MERGE_MESSAGE"
        name                        = "test-repo"
      + squash_merge_commit_message = "COMMIT_MESSAGES"
      + squash_merge_commit_title   = "COMMIT_OR_PR_TITLE"
        # (25 unchanged attributes hidden)

For me, the above applies successfully. Is there something I'm missing as far as reproducing?

@foosinn
Copy link

foosinn commented Oct 14, 2022

hey @garrettheel,

i find it odd that updating changes the repositories.

Do the displayed changes cause changes on the github repos?

@isaacsanders
Copy link

https://docs.github.com/en/rest/repos/repos#create-an-organization-repository

@rsrchboy
Copy link

FWIW, I'm also seeing the behaviour described. This is reminding me of the schema/state upgrade functionality the SDK provides -- I don't have time to dig into it now, but it seems like if the state is changing to have a new default, then an upgrader might be in order.

https://pkg.go.dev/github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema#StateUpgrader

@kfcampbell kfcampbell added Type: Bug Something isn't working as documented Status: Up for grabs Issues that are ready to be worked on by anyone Priority: Normal labels Feb 17, 2023
@kfcampbell kfcampbell moved this to 🔥 Backlog in 🧰 Octokit Active Feb 17, 2023
@scruplelesswizard
Copy link

scruplelesswizard commented Mar 22, 2023

My apologies, I haven't had sufficient cycles to get back to this issue previously.

Unfortunately we encountered this bug merging a time-sensitive PR and had to pin to a previous version to avoid this issue. I'll make note to bump the provider and confirm the issue still exists.

@chemmi
Copy link

chemmi commented Mar 30, 2023

As a workaround I ignored changes in the corresponding fields.

  lifecycle {
    ignore_changes = [
      merge_commit_message,
      merge_commit_title,
      squash_merge_commit_message,
      squash_merge_commit_title,
    ]
  }

For me this is fine, since I do not want to configure PR merge strategies or commit defaults with terraform at all. This fixed my plan, but I do not know what changes in state the provider does under the hood.

Perhaps this also fixes the case where merge or squash are explicitly disabled.

@chaosaffe

@scruplelesswizard
Copy link

@chemmi thanks for that workaround!

Copy link

👋 Hey Friends, this issue has been automatically marked as stale because it has no recent activity. It will be closed if no further activity occurs. Please add the Status: Pinned label if you feel that this issue needs to remain open/active. Thank you for your contributions and help in keeping things tidy!

@github-actions github-actions bot added the Status: Stale Used by stalebot to clean house label Jun 19, 2024
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Jun 27, 2024
@github-project-automation github-project-automation bot moved this from 🔥 Backlog to ✅ Done in 🧰 Octokit Active Jun 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hacktoberfest Issues for participation in Hacktoberfest Status: Stale Used by stalebot to clean house Status: Up for grabs Issues that are ready to be worked on by anyone Type: Bug Something isn't working as documented
Projects
None yet
Development

No branches or pull requests

8 participants