terraform-provider-gitlab
terraform-provider-gitlab copied to clipboard
gitlab_repository_file resource cant update/overwrite existing files
Feature Description
When trying to update a file which isn't already in TF state using the gitlab_repository_file resource the provider produces the below error:
gitlab_repository_file.foo: Creating...
Error: POST https://gitlab.com/api/v4/projects/[REDACTED]/repository/files/foo: 400 {message: A file with this name already exists}
on main.tf line 7, in resource "gitlab_repository_file" "foo":
7: resource gitlab_repository_file foo {
It may go against conventional Terraform wisdom to modify existing resources without first importing them to state, but this could be very handy for bringing a large number of files under management by Terraform. My use case is programmatically managing existing CODEOWNERS files for all repos under a group.
Looking at the API docs it looks like this would mean performing a PUT rather than a POST request. I propose adding an argument to the resource to allow overwriting files.
A similar issue was reported and implemented for the GitHub provider.
Do you want to implement this?
- [ ] I would like to implement this myself 👷
@switchboardOp The terraform provider actually performs a POST to the Gitlab endpoint.
2022-08-25T19:16:45.500Z [ERROR] vertex "module.prometheus.gitlab_repository_file.cert_manager_values_enable_prometheus" error: POST https://gitlab.example.com/api/v4/projects/85/repository/files/values/eks-rafael-dev.yaml: 400 {message: A file with this name already exists}
Said that, this is also happening with me where I can not overwrite the file already created using the tf resource gitlab_repository_file. I would say that this is a bug in the terraform provider. A fix for this would be very much appreciated!
In terms of the client library being used it looks like a difference between calling CreateFile and UpdateFile, which use POST and PUT respectively.
I remember that we've had this discussion before when I actually proposed such an attribute when initially implementing the resource (see https://github.com/gitlabhq/terraform-provider-gitlab/pull/724#discussion_r724117228). However, we've dropped it, since the proper terraform process is to import resources first (as you've correctly stated @switchboardOp ) (and that's also why it's not really a bug in the provider per se @triceras ).
However, the only reason for me to provide such an attribute for this resource is for the use case where a project is created with an import_url, which already contains that file and therefore, you couldn't terraform apply in a single run without such an attribute ...
While it may be convenient to have it, to do changes on existing files in a bunch of repositories (as @switchboardOp mentioned) it's not really how Terraform "should be used" (I'm not saying I wouldn't use it if it's available 😜 ) - so I won't further argue on that use case I guess 😊
I'm in favor of adding an overwrite_on_create attribute to properly support the project import use case I've mentioned above.
WDYT @PatrickRice-KSC @armsnyder ?
@timofurrer - I'm kinda with you that I don't love the possible use-case. Like you say, the only truly terraform-like case for using the resource like this would be using it on top of a templated project. However, I usually try and avoid judgement on whether something is terraform-like or not when talking about the provider, so let's set that aside.
Do you know if using the POST option (and therefore the update API) will succeed if the file doesn't exist already? If not, we'd have to do a "GET" first, which would potentially be retrieving a ton of data to include within the state file and feels like it could cause some pretty big issue if used en-masse. We'd like want to use just the HEAD information (and the scaffolding for this already exists in go-gitlab) to avoid that issue.
Also, we'd have to make sure that overwrite_on_create had some nice documentation and was ignored on diff, so that if someone changed it from true -> false it didn't break odd things.
So yeah, it seems like something we could support. I'm in, even though I don't love the use-case 🙂
This is a different issue but would be great to have -o ci.skip to disable CI run as well as overwrite_on_create working ;)