operator-framework / operator-controller

Apache License 2.0
28 stars 47 forks source link

Possible bug: Do we properly handle mutations of `spec.installNamespace`? #961

Closed joelanford closed 1 week ago

joelanford commented 1 week ago

I haven't had a chance to attempt a reproduction yet, but looking at our code, I think we have a few problems if installNamespace changes on a ClusterExtension that has a successful installation.

  1. We will not find the installed version in the previous install namespace, so our reconciler will treat it like a new install (assuming a helm release in the new install namespace doesn't already exist)
  2. We will not do anything with the helm release in the previous install namespace, which essentially means orphaning it.
  3. I don't know for sure, but there may be issues transferring any shared objects between the old and new release manifests.

Options include:

  1. Make the spec.installNamespace field immutable. This has implications on users that want to relocate a cluster extension. We would likely need spec.deletionPolicy: orphan to make this tolerable to those users.
  2. Stop using helm releases. Use helm template instead? This has implications on support for helm hooks, but perhaps we detect and reject manifests that use hooks. This option would eliminate the problem described in #923 as well.

Any other options folks can think of?