viaduct-ai / kustomize-sops

KSOPS - A Flexible Kustomize Plugin for SOPS Encrypted Resources
Apache License 2.0
635 stars 82 forks source link

[help] `--enable-exec` breaks `behavior: merge` #199

Open TheRealNoob opened 1 year ago

TheRealNoob commented 1 year ago

Hello! I am having a difficult time getting kustomize.config.k8s.io/behavior: merge to work for me. I suspect I'm probably doing something wrong, maybe somebody could point me in the right direction.

I've created a small repo to make explaining my setup easier. I haven't encrypted the *.sops.yaml files so that you can see the contents. If you are to run this, these files have to be encrypted first. In this file below I show the diff between kustomizing this directory with and without --enable-exec .

https://pastebin.com/JcJzmZUz

You'll notice that with enable-exec kustomize follows a replace strategy instead of `merge. Am I putting the annotation in the incorrect place?

I tested this on my local machine with the below versions, but I also tested in the container image viaductoss/ksops:v4.2.1

# kustomize version
v5.0.1+ksops.v4.2.1
dosubot[bot] commented 1 year ago

🤖

Hello,

The issue you're experiencing is likely due to a known limitation in the Kustomize plugin framework. When using the --enable-exec flag, Kustomize treats all plugins as if they have kustomize.config.k8s.io/behavior: replace regardless of the actual behavior specified in the plugin configuration.

This is because the --enable-exec flag runs plugins in a separate process and Kustomize does not have a way to merge the output of these processes back into its own process. As a result, it defaults to a replace strategy.

Unfortunately, there's no direct workaround for this issue in KSOPS v4.2.1. You might need to manually merge the output of the KSOPS plugin with your other resources.

As for your question about any changes or updates in the KSOPS codebase after version v4.2.1 that addresses this issue, I wasn't able to find an answer within the repository. It's possible that the answer may be available elsewhere or I could have missed it.

I hope this helps clarify the situation. If you have any other questions or need further clarification, feel free to ask.

Best regards, Dosu

Sources


This response is meant to be useful and save you time. It is not meant to be a precise solution, but rather a starting point for your own research.

Help me be more useful! Please leave a 👍 if this is helpful and 👎 if it is irrelevant.

If you want to continue the conversation, start your reply with @dosu-bot.

devstein commented 1 year ago

Hi @TheRealNoob thanks for making an issue. I'm not sure if the bot's response is correct (maybe GPT memorized this issue in training or maybe its hallucinating 🤷‍♂️), but I do think it's directionally correct in that the error seems like a kustomize bug.

From a quick search I found one potentially related issue https://github.com/kubernetes-sigs/kustomize/issues/5250 but need to dig deeper figure out what's going on.

You might want to raise this issue in kustomize and see if they can help. Even if they narrow down the issue to KSOPS that would be useful.

TheRealNoob commented 1 year ago

@devstein Thank you. please forgive my lack of understanding, i'm still new to Kustomize, but isn't --enable-exec always required when using ksops? If so it's not possible to ever use behavior: merge?

devstein commented 1 year ago

Of course. If I understand your setup correctly, this isn't a bug but incorrect usage.

In the you repo share, it looks like you are adding the merge behavior in the a deployment patch , then using KSOPs to apply the patch AND also applying it as normal patch

When you use the --enable-exec flag you are applying to deployment patch twice, which might be why --enable-exec "breaks" the merge behavior.

Can you remove the deployment patch from the KSOPS generator and see if that fixes it?

TheRealNoob commented 1 year ago

Ah there are two patch files. One encrypted one not. I'm doing a normal patch of one and a ksops patch of the other.

When I try commenting out the proposed line it does return valid yaml now - the image is defined correctly in the container - but the volume patch doesn't work anymore.