Can push deploy keys

[Shocktrooper]

Continuation of #608 that got stale

[Shocktrooper]

@timofurrer After looking at the initial proposed changes the only thing I can think that might need changing from a cursory check is to implement some of the things from #1008 but that is up to you if you want to proceed with the current code or have me do those changes.

[Shocktrooper]

@timofurrer I might have to rewrite part of this as the docs seem off to what the capabilities of the resource are on top of I had import failures when trying to implement things from #1008 . Also is there a reason to use a custom importer function for the deploy_key/deploy_key_enable resource vs just using schema.ImportStatePassthroughContext with a 2 part ID like in resource_gitlab_project_environment.go?

[timofurrer]

@Shocktrooper I don't know what the initial reason to implement it like that was. What I can say is that we should whenever possible use the passthru importer - which would have been possible in this case. The reason for not changing it is to preserve backwards compatibility. (Which apparently has gone unnoticed for the slack resource, see #1012).

We could implement some logic to handle the break "gracefully", but since it's "only" a tech debt in this case I'd leave it as is and concentrate on other things and change it in the provider v4 :)

[pdecat]

The reason for not changing it is to preserve backwards compatibility. (Which apparently has gone unnoticed for the slack resource, see #1012).

I found a way to maintain backward compatibility when changing IDs, see #1013

[Shocktrooper]

@timofurrer & @pdecat & @armsnyder Is there a preferred ID we are going with going forward for integrations or even generally? Are we going straight for the project Id as also the integration ID or a 2 part ID like projectId:integrationId

[timofurrer]

Well, since we want to use the passthru importer the id must contain all necessary information to read the resource at minimum. Sometimes more information is valuable (can't point to any example since my connectivity is very limited ...)

[Shocktrooper]

@timofurrer and @pdecat I was looking at the resource and it looks a bit janky to me as I do not see a need for a custom import at all. Form what I can tell we can use the passthru importer without any changes to the ID because it contains all the necessary information already and does not need to be changed. Can I remove the custom importer functions or am I missing something?

[Shocktrooper]

Also if it seems appropriate whomever reviews this please add it to the v3.14.0 milestone

[timofurrer]

@timofurrer and @pdecat I was looking at the resource and it looks a bit janky to me as I do not see a need for a custom import at all. Form what I can tell we can use the passthru importer without any changes to the ID because it contains all the necessary information already and does not need to be changed. Can I remove the custom importer functions or am I missing something?

The currently used resource id only contains the deploy key id, but you need the project id, too. Thus, without breaking the id (or using some migration strategy) it's not possible to use the passthru importer ...

[Shocktrooper]

@timofurrer I made some refactors to move the deploy token to a 2 part ID which is consistent with how the resource is imported. One thing I want to test and not sure how to do it so I am open to suggestions is to make an acceptance test for testing the ID migration path where someone might have had a 1 part legacy ID, then the next terraform run migrates that to the 2 part ID.

[timofurrer]

I've played around a bit and turns out it's actually possible to use state migrations for resource ids 😮 I've submitted a draft PR of my scratch:

• Draft state migration for resource id #1033

Unless I'm missing something IMHO this should be the way to go - also for the slack integration resource we've done this ...

WDYT?

cc @armsnyder

[timofurrer]

However, I'm still not a fan of changing the id without releasing a new major.

It's also against the terraform provider development best practices, see: https://www.terraform.io/plugin/sdkv2/best-practices/versioning#:~:text=Changing%20resource%20import,resource%20ID%20format

IMHO: this should be part of the v4 milestone ...

[Shocktrooper]

@timofurrer Alright I can revert it and we can wait till v4

[timofurrer]

Yes, I suggest to focus here on the can_push attribute and do the id change later.

[Shocktrooper]

@timofurrer I reverted the deploy key id changes but left the changes I made to deploy key enable id because that already had a 2 part id but instead of using the passthrough importer it had a custom importer that I believe was not necessary. Let me know what you think

[timofurrer]

LGTM 🎉

[github-actions]

This functionality has been released in v3.14.0 of the Terraform GitLab Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue. Thank you!