search

woodpecker-ci / helm

This repo contains the helm charts of the Woodpecker project.
Apache License 2.0
21 stars 16 forks source link

Secret generation causes issues for GitOps approach #154

Closed metawave closed 7 months ago

metawave commented 7 months ago

After the latest upgrade to 1.1 charts I got can't setup store: could not open datastore: no value matched key

I follow a GitOps approach (using ArgoCD) and never have secrets in a values.yaml by using SealedSecrets operator currently.

When I upgraded from the 1.0.x chart to the 1.1. Chart, the secret generated by the SealedSecrets operator was overwritten, caused by #144.

I now had to set the secrets for agent and server to both:

secrets: {}

so the SealedSecrets operator was able to create the decrypted secrets.

This secret generation wasn't as obvious as I thought because I missed or, to be more precise, dismissed it in the changelog because I had set createSecret: false previously, thinking that hadn't been changed.

I don't know, but the current solution will cause some issues for users using argocd or fluxcd with secret solutions like vault, sealed-secrets or sops (often in combination with fluxcd). Maybe this should be documented somehow?

pat-s commented 7 months ago

Thanks for reporting!

Hmm, I don't use ArgoCD actively, but I wonder why it regularly has issues with some chart defaults (have seen like 3-4 of such issues across different charts lately).

144 has been requested to auto-create the agent secret and make the chart work OOB for "normal" helm deployments with included k8s secrets.

When I upgraded from the 1.0.x chart to the 1.1. Chart, the secret generated by the SealedSecrets operator was overwritten, caused by https://github.com/woodpecker-ci/helm/pull/144.

Before #144 no secrets were autogenerated and secrets: didn't even exist. But yes, if you have a predefined secret coming from elsewhere, this then likely conflicts with the new one created from secrets:.

This secret generation wasn't as obvious as I thought because I missed or, to be more precise, dismissed it in the changelog because I had set createSecret: false previously, thinking that hadn't been changed.

While createSecret was technically doing something before, it had never been exposed in values.yaml and also was not true by default. So it shouldn't have had any effect previously. In general, no k8s secret has been provisioned by the chart.

I think the issue is unrelated to ArgoCD but comes down to the conflict of the new agent token secret autogeneration vs. existing secrets. So retrospectively, it would have been good to make this a breaking change then.

For now, I think you have two options:

  1. Disable the auto-generation of the secret and use your own one
  2. Switch to the chart secret and don't inject a secret for the token yourself

Or does SealedSecrets completely conflict with any preexisting k8s secret and won't do anything then? If that's the case that would be quite a bummer, I guess it should be possible for both approaches to co-exist.

pat-s commented 7 months ago

Please take a look at #155, I hope this will make this clearer. In your specific case, you might want to turn off the internal secret generation so you can use your own one.

Thanks for reporting and not just silently applying a fix yourself :)

vyskocilm commented 7 months ago

I've been bitten by this as I have the woodpecker-secret not managed by helm. Adding secret: {} for both agent and a server have solved the problem and restored the woodpecker setup.

pat-s commented 7 months ago

@metawave @vyskocilm Sorry again for the issues! I'll close this one here as it is more of a report than something we can actively fix, and people will also find it when it's closed.