why the hell did helm-release run here? 🤔

why the hell did helm-release run here? 🤔

I didn't do nothing, I swear! 🥲

A few quesitons:

• Do you want me to refactor this issue by sonarqube? I'd be happy to if necessary
• Any idea why tests I pointed out in the description fail locally?

@riccardomc Yes please, if you could take care of the sonar issue that would be fantastic. :) 🙇

@Skarlso sure, here you go! 😉

Thanks! :)

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

This looks incredible!
Will save me so much work and current headaches, thanks a lot for your work!

Nice. :) This has been released with 0.18.1. :) Enjoy!

@riccardomc Just wanted to say I finally had the chance to start working with this today and it's working perfectly!
Thanks again for your contribution 🙏

Quick update
While looking into migrating to this approach I stumbled into a bit of a soft blocker.
To quote #2987 (comment) - "it would be great to be able to specify priority for specific secret(s), if they exist"

Seems like this implementation is missing the "if they exist" part, as if I try to create an externalSecret with a "priority" configuration, but the key specified does not exist, it will result in an error.

For example, for this:

apiVersion: external-secrets.io/v1
kind: ExternalSecret
metadata:
  name: my-app-secrets
spec:
  refreshInterval: 1m
  secretStoreRef:
    kind: ClusterSecretStore
    name: hashicorp-vault-backend
  target:
    name: my-app-secrets
  dataFrom:
  - find:
      path: my-app
      name:
        regexp: ".*"
    rewrite:
    - merge:
        strategy: Extract
        conflictPolicy: Ignore
        priority:
          - my-app/secrets

If "my-app/secrets" does not exist, will result in:

Warning UpdateFailed 26s (x2 over 48s) external-secrets error processing spec.dataFrom[0].find, err: error applying rewrite to keys: failed rewrite operation[0]: merge failed with key "my-app/secrets" not found in input map

While I CAN see value in this causing an error if a secret is missing in some use-cases (for example if there's a secret you want to mandate), I think the "this key is prioritized if it exists" approach makes more sense, at least as the default.
Other solution for me would be requiring all apps create a blank "prioritized" secret (and I'm talking hundreds of apps 😵‍💫), so I think I'll hold off with this migration for now unfortunately

@riccardomc May I please request a follow up to either make prioritized secrets optional, or if you see fit, add a "priorityStrategy" field for either "Error" or "Ignore" 👉👈

Hey, thanks! I am very happy you're using it and thanks for reaching out, much appreciated.

The main reason I decided to keep this behavior is to minimize surprises for end users: if we just ignore the non existing remote secret, then the final content in the Secret might be different than expected without very little possibility of noticing.

I thought that this could lead to very unpleasant circumstances: it's quite easy to fail to override credentials that might cause significant issues at the application level. Imagine failing to override Production connection string because the Staging remote secret has a typo.

Maybe the priorityStrategy suggestion is the right thing to do, so at least there's a conscious opt-in into this behavior.

Would you mind opening a separate Issue for this as this is now merged?

@riccardomc sure, will do, thanks!