fix: destroy the resource on drift
closes 749
Based on the request from @jcudit https://github.com/integrations/terraform-provider-github/issues/749#issuecomment-945144056 related to https://github.com/integrations/terraform-provider-github/issues/749#issuecomment-939018875
👋 Hey Friends, this pull request 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!
hopefully will figure out how to test this locally one day 😢
@yordis have you had a peek at our CONTRIBUTING.md yet? There's some environment configuration you need but testing changes locally is very doable.
@kfcampbell definitely helped a ton, my breakpoint is being hit now. Now trying to figure out how we suppose to setup the test case for it!
I am trying to figure out what function would allow me to check if a new id was assigned to the resource ... any feedback welcome
The thing is working! Just the testing environment is tricky! I am unsure how to create a scenario where the external resource change and whatnot ... 😢
https://github.com/integrations/terraform-provider-github/assets/4237280/f7204e80-8413-4d3f-a1e2-7608e847115f
@yordis can you build a version of the provider locally and use it to manually test?
@kfcampbell good idea!
Hey I gave it a try to test it by building the local plugin, but I couldn't figure out how to make it work:
I did the following:
cat ~/.terraformrc
provider_installation {
dev_overrides {
"integrations/github" = "/Users/ubi/Developer/github.com/integrations/terraform-provider-github/bin/"
}
direct {}
}
No luck, I tested manually as I showed before, so maybe try on your end if you do not mind.
@yordis can you try following the instructions here to build the provider locally? You should be using a dev.tfrc file in order to specify the override.
I tried that guideline multiple times, and it didn't work.
As shown in the video, I tested by running the code in the test environment. I would appreciate it if someone took the lead in helping to do an integration test out of the generated binary.
👋 Hey Friends, this pull request 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!
keep
Hi @yordis
We're running into the same problem just with regular action secrets. I would like to offer my help in getting this fixed.
I can see your approach is to add a new destroy_on_drift property, but could we not just respect the already available ignore_changes array? And then check for either plaintext_value or encrypted_value appearing in that array, and not destroy it on drift, instead of introducing a new variable/config option? LMK what you think.
Why wasn't this merged?
👋 Hey Friends, this pull request 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!
this is funny, because we can't
Thanks for updating the documentation in this PR :+1: