search

argoproj / argo-cd

Declarative Continuous Deployment for Kubernetes
https://argo-cd.readthedocs.io
Apache License 2.0
17.42k stars 5.29k forks source link

Argocd Panics with invalid cluster secret #19496

Open empath-nirvana opened 1 month ago

empath-nirvana commented 1 month ago

Checklist:

Describe the bug

If you declaratively create a cluster secret with a TLS config that has "insecure: true" as well as CA data, the application server panics. I think it should just error out the sync.

https://github.com/argoproj/argo-cd/blob/v2.12.0/pkg/apis/application/v1alpha1/types.go#L3066-L3073 relevant panic in the code is here

panic: Unable to apply K8s REST config defaults: specifying a root certificates file with the insecure flag is not allowed

goroutine 419 [running]: github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1.(Cluster).RESTConfig(0xc000be7810?) /go/src/github.com/argoproj/argo-cd/pkg/apis/application/v1alpha1/types.go:3063 +0x79 github.com/argoproj/argo-cd/v2/controller/cache.(liveStateCache).getCluster(0xc00066c7e0, {0xc002b460f0, 0x4e}) /go/src/github.com/argoproj/argo-cd/controller/cache/cache.go:465 +0x485 github.com/argoproj/argo-cd/v2/controller/cache.(liveStateCache).getSyncedCluster(0x0?, {0xc002b460f0?, 0x0?}) /go/src/github.com/argoproj/argo-cd/controller/cache/cache.go:581 +0x1c github.com/argoproj/argo-cd/v2/controller/cache.(liveStateCache).handleAddEvent.func1() /go/src/github.com/argoproj/argo-cd/controller/cache/cache.go:763 +0x25 created by github.com/argoproj/argo-cd/v2/controller/cache.(*liveStateCache).handleAddEvent in goroutine 417 /go/src/github.com/argoproj/argo-cd/controller/cache/cache.go:761 +0x20b

To Reproduce

Create a cluster secret directly (ie, a kubernetes secret, not through argocd apis or the UI) with insecure true and with caData set, and try to sync an application.

It's possible you may have to initially create a secret with insecure set to "false" first, try to sync the app, and then set it to "insecure: true" to reproduce, I haven't tested it from a clean slate.

Expected behavior

The app should just have a sync error and argo cd shouldn't crash.

Screenshots

Version I'm on version 2.11.3, but I believe it still exists in the latest version -- the panic is still there in the code, but I haven't tested.

Logs


goroutine 419 [running]:
github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1.(*Cluster).RESTConfig(0xc000be7810?)
        /go/src/github.com/argoproj/argo-cd/pkg/apis/application/v1alpha1/types.go:3063 +0x79
github.com/argoproj/argo-cd/v2/controller/cache.(*liveStateCache).getCluster(0xc00066c7e0, {0xc002b460f0, 0x4e})
        /go/src/github.com/argoproj/argo-cd/controller/cache/cache.go:465 +0x485
github.com/argoproj/argo-cd/v2/controller/cache.(*liveStateCache).getSyncedCluster(0x0?, {0xc002b460f0?, 0x0?})
        /go/src/github.com/argoproj/argo-cd/controller/cache/cache.go:581 +0x1c
github.com/argoproj/argo-cd/v2/controller/cache.(*liveStateCache).handleAddEvent.func1()
        /go/src/github.com/argoproj/argo-cd/controller/cache/cache.go:763 +0x25
created by github.com/argoproj/argo-cd/v2/controller/cache.(*liveStateCache).handleAddEvent in goroutine 417
        /go/src/github.com/argoproj/argo-cd/controller/cache/cache.go:761 +0x20b.
christianh814 commented 1 month ago

+1 we should handle errors like this more gracefully.

pasha-codefresh commented 1 month ago

@david-wu-octopus is going to take it