pteich / caddy-tlsconsul

🔒 Consul K/V storage for Caddy Web Server / Certmagic TLS data
Apache License 2.0
96 stars 17 forks source link

panic on `ConsulStorage{}.ConsulClient` being nil #16

Closed Gurpartap closed 3 years ago

Gurpartap commented 3 years ago

Getting this panic error after upgrading certmagic, dns provider, storage, etc. modules. Not sure if it matters, but I switched from dnsimple to cloudflare dns provider as well.

certmagic.DefaultACME.Agreed = true
certmagic.DefaultACME.Email = email
certmagic.DefaultACME.DisableHTTPChallenge = true
certmagic.DefaultACME.DisableTLSALPNChallenge = true
certmagic.DefaultACME.CA = certmagic.LetsEncryptProductionCA
// config := dnsimple.NewDefaultConfig()
// config.AccessToken = accessToken
// dnsimpleProvider, err := dnsimple.NewDNSProviderConfig(config)
certmagic.DefaultACME.DNS01Solver = &certmagic.DNS01Solver{
    DNSProvider: &cloudflare.Provider{
        APIToken: accessToken,
    },
}
certmagic.Default.Storage = storageconsul.New()
magic := certmagic.NewDefault()
err := magic.ManageSync(domains)
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0xa5e54d]

goroutine 25 [running]:

github.com/hashicorp/consul/api.(*Client).newRequest(0x0, 0x1024cdf, 0x3, 0xc000274000, 0x97, 0xc000274000)
    /Users/gurpartap/go/pkg/mod/github.com/hashicorp/consul/api@v1.8.1/api.go:848 +0x6d

github.com/hashicorp/consul/api.(*KV).getInternal(0xc000262ba0, 0xc00003e240, 0x90, 0x0, 0xc000262c10, 0xe87d60, 0xc000036690, 0xc000036690, 0x87)
    /Users/gurpartap/go/pkg/mod/github.com/hashicorp/consul/api@v1.8.1/kv.go:126 +0x105

github.com/hashicorp/consul/api.(*KV).Get(0xc000262ba0, 0xc00003e240, 0x90, 0xc000262c10, 0x0, 0x0, 0x0, 0x0)
    /Users/gurpartap/go/pkg/mod/github.com/hashicorp/consul/api@v1.8.1/kv.go:65 +0xa5

github.com/pteich/caddy-tlsconsul.ConsulStorage.Load(0x0, 0x0, 0x0, 0x0, 0xc000220c60, 0x0, 0x0, 0x0, 0x0, 0xa, ...)
    /Users/gurpartap/go/pkg/mod/github.com/pteich/caddy-tlsconsul@v1.3.2/storage.go:127 +0x12d

github.com/caddyserver/certmagic.(*Config).loadCertResource(0xc00020bd60, 0x1178428, 0xc000172400, 0x104dab6, 0x27, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
    /Users/gurpartap/go/pkg/mod/github.com/caddyserver/certmagic@v0.14.0/crypto.go:239 +0x296

github.com/caddyserver/certmagic.(*Config).loadCertResourceAnyIssuer(0xc00020bd60, 0x104dab6, 0x27, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
    /Users/gurpartap/go/pkg/mod/github.com/caddyserver/certmagic@v0.14.0/crypto.go:171 +0xc1d

github.com/caddyserver/certmagic.(*Config).loadManagedCertificate(0xc00020bd60, 0x104dab6, 0x27, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
    /Users/gurpartap/go/pkg/mod/github.com/caddyserver/certmagic@v0.14.0/certificates.go:123 +0xc9

github.com/caddyserver/certmagic.(*Config).CacheManagedCertificate(0xc00020bd60, 0x104dab6, 0x27, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
    /Users/gurpartap/go/pkg/mod/github.com/caddyserver/certmagic@v0.14.0/certificates.go:110 +0xc9

github.com/caddyserver/certmagic.(*Config).manageOne(0xc00020bd60, 0x1189ee8, 0xc000042030, 0x104dab6, 0x27, 0xc000263e00, 0xa8ee05, 0xc0000a00b0)
    /Users/gurpartap/go/pkg/mod/github.com/caddyserver/certmagic@v0.14.0/config.go:323 +0xbe

github.com/caddyserver/certmagic.(*Config).manageAll(0xc00020bd60, 0x1189ee8, 0xc000042030, 0xc00009d0a0, 0x2, 0x2, 0x0, 0x0, 0x0)
    /Users/gurpartap/go/pkg/mod/github.com/caddyserver/certmagic@v0.14.0/config.go:312 +0x1f1

github.com/caddyserver/certmagic.(*Config).ManageSync(...)
    /Users/gurpartap/go/pkg/mod/github.com/caddyserver/certmagic@v0.14.0/config.go:251
pteich commented 3 years ago

@Gurpartap Where does your code snippet come from or in other words how do you use Caddy and in specific this plugin? Maybe it is just another use-case I not really cover by now.

pteich commented 3 years ago

@Gurpartap After issuing storageconsul.New() it has to be provisioned. Caddy does this be calling storageInstanceVariable.Provision(ctx caddy.Context) The reason is that it needs the caddy.Context that carries a logger instance. This is all done automatically if you just include this plugin and activate it in your Caddy config. Normally there is no need to code anything.

Gurpartap commented 3 years ago

Thanks. I'll try that today, and let you know.

I'm not using Caddy. Only certmagic with plain http.Server{}.

pteich commented 3 years ago

Ok, I see. You can also use an approach like this:

   cs := storageconsul.New()
   cs.Prefix = "your-consul-kv-prefix"
   cs.AESKey = []byte("12345678901234567890123456789012")
   client, err := api.NewClient(api.DefaultConfig())
   if err != nil {
      panic(err)
   }

   cs.ConsulClient = client
   certmagic.Default.Storage = cs
Gurpartap commented 3 years ago

Thank you @pteich. That worked perfectly. Feel free to close the thread.

Might be worthwhile making func (cs *ConsulStorage) createConsulClient() error {} public.

pteich commented 3 years ago

Great @Gurpartap - I was not aware of that use-case by now. I consider your idea or maybe add a NewWithConsulClient() func that calls createConsulClient internally as this feels more natural to me.