Implement .metadata.ownerReferences handling

[jkroepke]

Fixes #120

Please advise me about common mistakes for golang. I do not have much experience into it.

[hensur]

Hi @jkroepke!

Thank you for taking the first step and opening this PR :) It looks good so far and will certainly fix the reported errors. Thanks for adding docs and tests as well.

I have to say, I am not sure whether globally en-/disabling this behavior is the best solution to this problem.

I'd like to propose a different approach.
I think it would be most helpful to strip ownerReferences from replicated resources by default. This will prevent the unexpected behavior when replicating resources with an ownerReference.
This will result in a breaking change. But I think the current handling of ownerReferences might be more "breaking" for most users.

I guess there are some edge cases where it might be desired to keep the ownerReferences (namespaced dependant with a clusterscoped owner). In this case we could provide an annotation (e.g. replicator.v1.mittwald.de/keep-owner-references) which allows one to decide this for each secret to be replicated.

What do you think about this approach? I can understand if you don't want to put in the additional work, I will take a look at this in the next few days then.

[jkroepke]

Hi all,

thats fine for me. I will switch from CLI flag to an annotations. That covers all cases.

[jkroepke]

I'm using an annotation as toggle now. Newly tests are green. Please review. Thanks

[jkroepke]

@hensur

I hope, Is the current state in aligned with you. Do you know if you could find some time for this?

[martin-helmich]

Hey all; I've just been reading up on the discussion and the proposed change and am just going to throw in my 2 cents.

From my understanding, the original suggestions by @jkmw and @hensur suggested that ownerReferences should be dropped by default (ref. #140 (comment) and #140 (comment)), with an annotation than can be set if the user explicitly wants the reference to persist.
That suggestion, even if technically a breaking change, is fine with me, 👍 since the current behaviour is unexpected and presumably not the desired behaviour in most cases anyway (expect in the most edgy of edge cases).

If I read the code correctly, the proposed change as it stands now does exactly the opposite of the suggested solution (suggested: "drop ownerReferences by default, keep if explicitly desired by anntation", actual: "keep ownerReferences by default, and only drop if explicitly desired").

[jkroepke]

Implemented, please review.

• Fun Fact for push based replication, ownerReference was not copy by default. I found this behavior while add a test.

[hensur]

Hey! Sorry for my late response.

The change looks good so far! :) Just one small nitpick.

[hensur]

Thank you for the contribution! I will try to respond faster next time :)

This should be available soon as v2.7.0