github_repository_file cant update/overwrite existing files.

[techdragon]

Terraform Version

Terraform v0.12.23

• provider.aws v2.57.0
• provider.github v2.6.1
• provider.helm v1.1.1
• provider.kubectl (unversioned)
• provider.kubernetes v1.11.1

Affected Resource(s)

github_repository_file

Terraform Configuration Files

resource "github_repository_file" "kubectl_container_github_actions" {
  repository = github_repository.kubectl_container.name
  file       = ".github/workflows/main.yml"
  content    = file("${path.module}/github-actions-config-files/kubectl-container.yaml")
}

( I unfortunately had to use a raw file like this due to hashicorp/terraform#23322, specifically that yamlencode doesnt support multiline block key-name: | type strings. But the content of the file should not matter for this particular bug. )

Debug Output

Complete terraform log not provided for security reasons. The relevant section of the log, with a small security redaction is here. This shows both the output sent to and the response from GitHub, so I hope this is sufficient.
https://gist.github.com/techdragon/ed73032aa5ab062674886a6c7478935e

Expected Behavior

github_repository_file should be able to update files that already exist.

Actual Behavior

Terraform fails to update an existing file in a github repository.

Error: PUT https://api.github.com/repos/<REDACTED>/contents/.github/workflows/main.yml: 422 Invalid request.

"sha" wasn't supplied. []

Steps to Reproduce

1. terraform apply using a github_repository_file for the first time on a file that is already in the git repo.

Important Factoids

• It is relatively easy to work around this by importing the file, but it would be good to provide a more user friendly experience than simply dropping the error message "sha" wasn't supplied directly from the upstream GitHub API. Either handling this, switching to update and "doing what the user wanted", or by emitting a more user friendly error message informing the user that they should import the resource first.
• This would also make it easier to use template repositories, since the files in a template repository will exist after its created by a github_repository resource. Which then requires importing files created by the template before you can use github_repository_file resources to override their contents.

References

[anGie44]

Hi @techdragon, thank you for submitting this issue! looking at the logs you've provided it does look like the provider is behaving as expected at apply time as information about the existing file is not known by terraform unless imported into state as you've suggested.

I do agree though that the error provided back to the user could be more detailed -- it looks like the Github API determines the file exists and attempts to do an update behind the scenes, but fails without a known sha and this error isn't reported back with better context. imo, I think providing better error messaging to a user is more feasible as the update-on-create behavior may conflict with the distinct CRUD operations expected in a resource.

[techdragon]

@anGie44 thanks for digging in and working out the extra details that are going on with the GitHub API behind the scenes. I think a full resolution to the issue as it stands, needs to deal with two major details.

• A better error response:
  • The default behaviour should definitely have a better error message. The heuristic (_ in pseudo code/flowchart form_) perform-create -> invoke error handling logic for perform-create -> check if the error response is for a version of the GitHub api where we trust this heuristic is valid -> check if the error response contains ' "sha" wasn't supplied. ' -> return better error message`... Should be sufficiently accurate with respect to catching only this particular error response case.
• Functionality improvements
  • While its arguable that for 'normal' existing repositories, the user should be forced to add and import them before adding any terraform managed files... I can just see arguments for both sides with respect to the "existing" repo behaviour so I wont really argue one way or the other on this part.
  • The use cases for "template repositories" are varied, in a number of cases I can see completely managing the a repo through terraform, the simplest example I have on hand is creating a "GitHub Action Repo" from one of the templates provided by GitHub. Which you want to use as part of your CI/CD pipeline. In this example, you likely know what you're adding to the repo before you create it, and want to automate things so that any webhook URLs or other IDs of systems managed by terraform are kept up to date. Having to create the repo with one terraform run, then add file resources for the required files that will be in every GitHub action repo, import these files, and add the remainder of your repo files, before applying the final configuration, seems like unnecessary complication.
    • To handle these kind of situations, it doesn't seem like a bad idea to add a specific flag that indicates the user knows and is opting-in to an "Upsert-Like" behaviour of "perform file update if file creation fails in such a way that it indicates the file already exists". Having explicit opt-in seems like a sensible way to both support the useful behaviour, and prevent conflict with the normal expected CRUD behaviour of resource objects.

[jcudit]

@techdragon thanks for sharing your use case and highlighting the incompatibility between template use and repository file management. I've been playing around with this locally and aim to push up a proposed fix that adds an overwrite flag.

[shoekstra]

Traditionally Terraform only manages what it knows about, so expecting it to update something it didn't create feels a bit unfair. I've seen a similar use case in other providers/resources and they will also generate an error that the resource already exists (we could catch the returned error and make it more user friendly though).

IMO the solution here is to import the file into the state using terraform import and then Terraform will manage the file correctly.