Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Admission server's logger is uninitialized #1954

Closed
1 task done
zackrobichaud opened this issue Oct 21, 2021 · 2 comments · Fixed by #1955
Closed
1 task done

Admission server's logger is uninitialized #1954

zackrobichaud opened this issue Oct 21, 2021 · 2 comments · Fixed by #1955
Assignees
Labels
bug Something isn't working priority/medium

Comments

@zackrobichaud
Copy link
Contributor

Is there an existing issue for this?

  • I have searched the existing issues

Current Behavior

When the KIC is using the admission server, invalid plugin configurations result in a panic and incorrect message back to the users since the RequestHandler's logger in nil.

Requesthandler is created here and the Logger is not set. When an invalid plugin is configured and ServeHTTP attempts to log the error , it panics as the Logger == nil.

Results in a panic in the logs

kong-6df778d747-47krv ingress-controller 2021/10/20 21:04:13 http: panic serving 10.33.238.7:43566: runtime error: invalid memory address or nil pointer dereference
kong-6df778d747-47krv ingress-controller goroutine 984 [running]:
kong-6df778d747-47krv ingress-controller net/http.(*conn).serve.func1()
kong-6df778d747-47krv ingress-controller 	/usr/local/go/src/net/http/server.go:1801 +0xb9
kong-6df778d747-47krv ingress-controller panic({0x161cb60, 0x2689ba0})
kong-6df778d747-47krv ingress-controller 	/usr/local/go/src/runtime/panic.go:1047 +0x266
kong-6df778d747-47krv ingress-controller github.com/kong/kubernetes-ingress-controller/v2/internal/admission.RequestHandler.ServeHTTP({{0x1a2e1e0, 0xc000726800}, {0x0, 0x0}}, {0x1a2dd30, 0xc000731260}, 0xc0004b9d00)
kong-6df778d747-47krv ingress-controller 	/workspace/internal/admission/server.go:134 +0x37c
kong-6df778d747-47krv ingress-controller net/http.serverHandler.ServeHTTP({0x1a21408}, {0x1a2dd30, 0xc000731260}, 0xc0004b9d00)
kong-6df778d747-47krv ingress-controller 	/usr/local/go/src/net/http/server.go:2878 +0x43b
kong-6df778d747-47krv ingress-controller net/http.(*conn).serve(0xc001012dc0, {0x1a34a38, 0xc0008b8a50})
kong-6df778d747-47krv ingress-controller 	/usr/local/go/src/net/http/server.go:1929 +0xb08
kong-6df778d747-47krv ingress-controller created by net/http.(*Server).Serve
kong-6df778d747-47krv ingress-controller 	/usr/local/go/src/net/http/server.go:3033 +0x4e8

The user (in this case client using kubectl) see's the following error

error: kongplugins.configuration.konghq.com "<plugin>" could not be patched: Internal error occurred: failed calling webhook "validations.kong.konghq.com": Post "https://kong-validation-webhook.kong.svc:443/?timeout=10s": EOF

Expected Behavior

KIC should not panic, log the proper error, and the user should see a proper error message back from the validating webhook

error: kongplugins.configuration.konghq.com "<plugin>" could not be patched: admission webhook "validations.kong.konghq.com" denied the request: <error message>

Steps To Reproduce

1. Run the KIC with admission webhook enabled
2. Submit an invalid KongPlugin CR
3. Tail logs to see panic

Kong Ingress Controller version

v2.0.3 I believe this happens in any version of the KIC > 2.0

Kubernetes version

No response

Anything else?

No response

@zackrobichaud zackrobichaud added the bug Something isn't working label Oct 21, 2021
@shaneutt shaneutt self-assigned this Oct 21, 2021
@shaneutt
Copy link
Contributor

shaneutt commented Oct 21, 2021

Fixed in 2.0.4 via #1955

Thanks for reporting and patching this @zackrobichaud 🖖

@zackrobichaud
Copy link
Contributor Author

Awesome, thanks for taking a look and merging so quickly!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working priority/medium
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants