search

GalleyBytes / terraform-operator

A Kubernetes CRD to handle terraform operations
http://tf.galleybytes.com
Apache License 2.0
364 stars 47 forks source link

added gitSecret locks #142

Closed kepiukik closed 1 year ago

kepiukik commented 1 year ago

Added ability to lock k8s secrets referred in SCMAuthMethods error via controller's finalizer. It prevents terraform workflow from failing when secret(s) were deleted before setup/init-delete jobs have been fully completed.

isaaguilar commented 1 year ago

Thanks @kepiukik I want to acknowledge I got this and I hope to review it sometime next week.

isaaguilar commented 1 year ago

While testing, every now and then the finalizer code gets skipped. I'm still looking into this to find out if this is my cluster setup or if it is the code.

kepiukik commented 1 year ago

While testing, every now and then the finalizer code gets skipped. I'm still looking into this to find out if this is my cluster setup or if it is the code.

Hm, seems a bit strange. I've tested it in 3 ways according to different conditions and got those results: 1) spec.ignoreDelete: true ==> no finalizer on Terraform CR and any referred secret. 2) spec.ignoreDelete: false & lockSecretDeletion: true on some secrets ==> finalizer on Terraform CR & finalizers on referred secrets. 3) spec.ignoreDelete: false & lockSecretDeletion: false on whole secrets ==> only finalizer on Terraform CR

isaaguilar commented 1 year ago

This is a good change and I will merge soon, however, there's a few things in the logic that I finally figured out.

There is only one case that matters for locking, which is case !tf.Spec.IgnoreDelete: understandably. But the function is set to ONLY lock. So when tokenSecret.LockSecretDeletion is true, this is correct. However, in the case we're moving tokenSecret.LockSecretDeletion from true to false, this doesn't unlock.

I was expecting setting tokenSecret.LockSecretDeletion to false would trigger an unlock but that didn't happen.

It's probably only a < 20% of the time problem. Do you want to try to figure out how to get this to work or do you think we're good with the 80+% use case?

kepiukik commented 1 year ago

Apologies for the delay. I've just found another issue: when finalizer already presents on the tf CR, nothing happens even if we turn on tokenSecret.LockSecretDeletion. The root cause is the same as for your previous comment. All locking magic works only if something happens with CR finalizer. So if we want to cover all corner-cases updateSecretFinalizer method should be called before updateFinalizer, because it ends with return from main loop. I'll try to find solution and will be back soon.

kepiukik commented 1 year ago

Hello again! It seems, locking works now as expected. If you have free time please take a look and test cases mentioned above.