gitops-engine icon indicating copy to clipboard operation
gitops-engine copied to clipboard

feat: add ExternalSecret to sync wave

Open ckav370 opened this issue 1 year ago • 5 comments

I want to be able to use a preSync hook to run some database migrations. This has been working fine, until such a time as updating the container secrets which are using the External Secret Operator and CRD to create a k8s secret which the job created by the preSync hook creates mounts.

As per this Github issue #9891, the recommended approach is to use Argo sync waves. However ExternalSecret is not included in one of the predefined kinds and therefore cannot be part of a different "wave" meaning that any secrets defined by ExternalSecrets cannot be synced before the preSync hook as part of the pre Sync wave.

This would allow the ExternalSecret to have an annotation such as the following

argocd.argoproj.io/sync-wave: "-2"

And a preSync hook to have a greater weighted sync wave such as

argocd.argoproj.io/sync-wave: "-1"

And all other resources which are part of the deployment would default to weight of 0 without the need for an annotation

ckav370 avatar Jul 16 '24 12:07 ckav370

Isn't this already possible?

apiVersion: external-secrets.io/v1beta1
kind: ExternalSecret
metadata:
  annotations:
    argocd.argoproj.io/hook: PreSync
    argocd.argoproj.io/sync-wave: "-1000"

We already use ExternalSecrets in this fashion. One of our PreSync jobs requires a secret present and this was our strategy to prevent the catch 22 of initial sync not having that secret defined yet.

schlags avatar Aug 01 '24 21:08 schlags

Isn't this already possible?

apiVersion: external-secrets.io/v1beta1
kind: ExternalSecret
metadata:
  annotations:
    argocd.argoproj.io/hook: PreSync
    argocd.argoproj.io/sync-wave: "-1000"

We already use ExternalSecrets in this fashion. One of our PreSync jobs requires a secret present and this was our strategy to prevent the catch 22 of initial sync not having that secret defined yet.

Using a PreSync jon in this way means that when the secret is updated, the secret will be recreated. In the case where some PreSync job and the main application uses this secret, this would not be a very robust- especially in the case where the secret refresh failed to pull from the ExternalSecret backend.

Granted this is probably more likely in legacy code bases, but it would not be a solution for my current use case :)

ckav370 avatar Aug 02 '24 11:08 ckav370

Hello

First of all, looking at the commit on its own I have two comments

  1. The list you modify contains a list of built-in Kubernetes resources. What you are adding is not a built-in resource. Even if that would fix your use case, the next person would then add their own custom CRD and so on. This is not sustainable in the long run (for everybody to add their own resource there)
  2. As the comment says this list is designed on purpose to mimic the default ordering that Helm follows. By editing this list, you break this convention.

However just to take a step backwards. Can you explain what exactly is your use case and why it is not covered by sync waves? And I mean JUST sync waves and not phases/hooks.

However ExternalSecret is not included in one of the predefined kinds and therefore cannot be part of a different "wave" meaning that any secrets defined by ExternalSecrets cannot be synced before the preSync hook as part of the pre Sync wave.

I don't understand this sentence. Could you please attach a minimal example of two resources with just sync waves and explain what Argo CD does versus what you expected to happen?

kostis-codefresh avatar Sep 09 '24 17:09 kostis-codefresh

I think they are trying to achieve this by forcing the ordering as a default.

https://github.com/argoproj/argo-cd/issues/9891#issuecomment-2090435031

It would be great to be able to set some default ordering sometimes through a configmap that apply globally to solve the use case here.

i.e., "In my cluster, every ExternalSecret has to be synced in sync wave X before continuing"

On top of that, currently if I use kustomize to generate a configmap, and then use that configmap on my pre-sync job (like a database migration, knowing the URL of the server)... you have to mark that configmap as a pre-sync resource as well, but now I need to tell ArgoCD to not delete it with some other annotation... But if I do that, a bunch of those start to accumulate :')

alexrecuenco avatar Dec 26 '24 19:12 alexrecuenco

I would treat this issue more as a bug than a feature: if we use ExternalSecret, there is no way to keep proper order and prevent deletion.

piotrplenik avatar Feb 28 '25 10:02 piotrplenik