argo-helm icon indicating copy to clipboard operation
argo-helm copied to clipboard

fix(argo-cd): argo-cd notifications config map & secret names

Open Voziv opened this issue 2 years ago • 6 comments

The notifications controller is hard coded to look for argocd-notifications-secret and argocd-notifications-cm. Unless this changes I think the sane default should be argocd-notifications-cm & argocd-notifications-secret

fixes: #1187

Checklist:

  • [x] I have bumped the chart version according to versioning
  • [x] I have updated the documentation according to documentation
  • [x] I have updated the chart changelog with all the changes that come with this pull request according to changelog.
  • [x] Any new values are backwards compatible and/or have sensible default.
  • [x] I have signed off all my commits as required by DCO.
  • [ ] My build is green (troubleshooting builds).

Changes are automatically published when merged to main. They are not published on branches.

Voziv avatar May 27 '22 07:05 Voziv

We only speak about the bot container right?

All other things should be setup correctly. The name is injected as CLI argument already:

Ah I see. I think the bug may be in the argo-cd notifications code then if it's supposed to be taking the names of the configmap/secret as arguments.

I'm not super well versed in go yet, but I believe it's this line that causes the errors: https://github.com/argoproj/argo-cd/blob/4f155b539b925c2ad732a57fd03a5b5fb8605ae2/cmd/argocd/commands/admin/notifications.go#L33

settings.GetFactorySettings(argocdService, "argocd-notifications-secret", "argocd-notifications-cm"), func(clientConfig clientcmd.ClientConfig) {

That said I've noticed that we're still on argocd 2.3.3, I'll try updating our chart version tomorrow morning to see if it's still happening.

Voziv avatar Jun 02 '22 03:06 Voziv

Updating to the latest chart I still get the same error

time="2022-06-02T13:41:33Z" level=error msg="Failed to process: secret \"argocd-notifications-secret\" not found" resource=argocd/linkerd

Note: linkerd is just the name of an app, every single app name is listed with this error in the argocd-notifications-controller-bot pod

However I do agree with what @mkilchhofer is getting at. This is likely a bug in argo-cd so I'll file a report there.

In the meantime a fix I can apply myself is overriding the config and secret names using notifications.cm.name and notifications.secret.name values.

Edit: I also tested the latest chart + argocd v2.4.0-rc4 and the problem still occurs. Issue with argoproj/argo-cd is now open

Voziv avatar Jun 02 '22 13:06 Voziv

I also started a discussion in slack over there: https://cloud-native.slack.com/archives/C020XM04CUW/p1654109494458449

mkilchhofer avatar Jun 02 '22 23:06 mkilchhofer

Update in Slack:

Screenshot of slack conversation

xref: https://github.com/argoproj/argo-cd/issues/9869

mkilchhofer avatar Jun 23 '22 09:06 mkilchhofer

I'm realizing there's some missing context that I could have repeated in this PR. In my reply on #1187 I identified that in the argocd repository there is a hard coded secret name which is what I believe causes the errors.

I'll post this in slack as well

Voziv avatar Jun 23 '22 13:06 Voziv

xref: #1340 as it also fixed things regarding this context here - except for the bot

mkilchhofer avatar Jun 23 '22 14:06 mkilchhofer

Resolved via https://github.com/argoproj/argo-helm/pull/1419

pdrastil avatar Aug 25 '22 10:08 pdrastil