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

APIServer crashes when a MutatingWebhook returns a faulty patch #64291

Closed
alvaroaleman opened this issue May 24, 2018 · 6 comments · Fixed by #64355
Closed

APIServer crashes when a MutatingWebhook returns a faulty patch #64291

alvaroaleman opened this issue May 24, 2018 · 6 comments · Fixed by #64355
Labels
area/admission-control kind/bug Categorizes issue or PR as related to a bug. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery.

Comments

@alvaroaleman
Copy link
Member

/kind bug

What happened:

Wrote a MutatingWebhook that returned a faulty json patch which specified an our of range index (e.G. "op":"add","path":"/spec/containers/2","value":"some-container-spec" when there is only one container). This resulted in the apiserver crashing.

Stacktrace:

panic: runtime error: index out of range
goroutine 11654 [running]:
k8s.io/kubernetes/vendor/github.com/evanphx/json-patch.(*partialArray).add(0xc42dfb09a0, 0xc423ed4d99, 0x1, 0xc42dfb0db0, 0xc42fb7f4e8, 0xc423ed4d01)
	/workspace/anago-v1.10.1-beta.0.68+d4ab47518836c7/src/k8s.io/kubernetes/_output/dockerized/go/src/k8s.io/kubernetes/vendor/github.com/evanphx/json-patch/patch.go:382 +0x4bd
k8s.io/kubernetes/vendor/github.com/evanphx/json-patch.Patch.add(0xc42679d540, 0x3, 0x4, 0xc42f743d00, 0xc42dfb07b0, 0x0, 0x0)
	/workspace/anago-v1.10.1-beta.0.68+d4ab47518836c7/src/k8s.io/kubernetes/_output/dockerized/go/src/k8s.io/kubernetes/vendor/github.com/evanphx/json-patch/patch.go:434 +0x108
k8s.io/kubernetes/vendor/github.com/evanphx/json-patch.Patch.ApplyIndent(0xc42679d540, 0x3, 0x4, 0xc42b781180, 0x54a, 0x54a, 0x0, 0x0, 0x98493c0, 0xc42dfbe620, ...)
	/workspace/anago-v1.10.1-beta.0.68+d4ab47518836c7/src/k8s.io/kubernetes/_output/dockerized/go/src/k8s.io/kubernetes/vendor/github.com/evanphx/json-patch/patch.go:603 +0x3d9
k8s.io/kubernetes/vendor/github.com/evanphx/json-patch.Patch.Apply(0xc42679d540, 0x3, 0x4, 0xc42b781180, 0x54a, 0x54a, 0x54a, 0x0, 0x0, 0xc4203f0f00, ...)
	/workspace/anago-v1.10.1-beta.0.68+d4ab47518836c7/src/k8s.io/kubernetes/_output/dockerized/go/src/k8s.io/kubernetes/vendor/github.com/evanphx/json-patch/patch.go:579 +0x82
k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating.(*MutatingWebhook).callAttrMutatingHook(0xc420b24780, 0x9883680, 0xc42001a030, 0xc42e3425a0, 0x988ffc0, 0xc42a6b9c70, 0x0, 0x0, 0x9865000, 0xc42f6f6a80, ...)
	/workspace/anago-v1.10.1-beta.0.68+d4ab47518836c7/src/k8s.io/kubernetes/_output/dockerized/go/src/k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/admission.go:302 +0x41f
k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating.(*MutatingWebhook).Admit(0xc420b24780, 0x988ffc0, 0xc42a6b9c70, 0x7ff67afa9788, 0xc420b24780)
	/workspace/anago-v1.10.1-beta.0.68+d4ab47518836c7/src/k8s.io/kubernetes/_output/dockerized/go/src/k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/admission.go:234 +0x43d
k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/admission/metrics.pluginHandlerWithMetrics.Admit(0x9855180, 0xc420b24780, 0xc4205d1a70, 0xc4205d1a80, 0x1, 0x1, 0x988ffc0, 0xc42a6b9c70, 0x1, 0x7ff67afa9608)
	/workspace/anago-v1.10.1-beta.0.68+d4ab47518836c7/src/k8s.io/kubernetes/_output/dockerized/go/src/k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/admission/metrics/metrics.go:85 +0xca
k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/admission/metrics.(*pluginHandlerWithMetrics).Admit(0xc420b81170, 0x988ffc0, 0xc42a6b9c70, 0x7ff67afa9608, 0xc420b81170)
	<autogenerated>:1 +0x5f
k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/admission.chainAdmissionHandler.Admit(0xc420b0a200, 0x9, 0x10, 0x988ffc0, 0xc42a6b9c70, 0x2067d33e, 0x72a2d8664)
	/workspace/anago-v1.10.1-beta.0.68+d4ab47518836c7/src/k8s.io/kubernetes/_output/dockerized/go/src/k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/admission/chain.go:35 +0xf5
k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/admission.(*chainAdmissionHandler).Admit(0xc420b40700, 0x988ffc0, 0xc42a6b9c70, 0x7ff67afa9668, 0xc420b40700)
	<autogenerated>:1 +0x62
k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/admission/metrics.pluginHandlerWithMetrics.Admit(0x985bc40, 0xc420b40700, 0xc4203c4140, 0x0, 0x0, 0x0, 0x988ffc0, 0xc42a6b9c70, 0x6, 0x7ff67afa9601)
	/workspace/anago-v1.10.1-beta.0.68+d4ab47518836c7/src/k8s.io/kubernetes/_output/dockerized/go/src/k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/admission/metrics/metrics.go:85 +0xca
k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/admission/metrics.(*pluginHandlerWithMetrics).Admit(0xc420b47da0, 0x988ffc0, 0xc42a6b9c70, 0x7ff67afa9601, 0xc420b47da0)
	<autogenerated>:1 +0x5f
k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/endpoints/handlers.createHandler.func1(0x987fe00, 0xc4260e1a90, 0xc42f530f00)
	/workspace/anago-v1.10.1-beta.0.68+d4ab47518836c7/src/k8s.io/kubernetes/_output/dockerized/go/src/k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/endpoints/handlers/create.go:99 +0x15f5
k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/endpoints.restfulCreateResource.func1(0xc42f6e8540, 0xc42f53c9c0)
	/workspace/anago-v1.10.1-beta.0.68+d4ab47518836c7/src/k8s.io/kubernetes/_output/dockerized/go/src/k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/endpoints/installer.go:1027 +0xd5
k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/endpoints/metrics.InstrumentRouteFunc.func1(0xc42f6e8540, 0xc42f53c9c0)
	/workspace/anago-v1.10.1-beta.0.68+d4ab47518836c7/src/k8s.io/kubernetes/_output/dockerized/go/src/k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/endpoints/metrics/metrics.go:199 +0x208
k8s.io/kubernetes/vendor/github.com/emicklei/go-restful.(*Container).dispatch(0xc4200f6e10, 0x7ff67b0fbe40, 0xc4260e1a78, 0xc42f530f00)
	/workspace/anago-v1.10.1-beta.0.68+d4ab47518836c7/src/k8s.io/kubernetes/_output/dockerized/go/src/k8s.io/kubernetes/vendor/github.com/emicklei/go-restful/container.go:277 +0xb18
k8s.io/kubernetes/vendor/github.com/emicklei/go-restful.(*Container).Dispatch(0xc4200f6e10, 0x7ff67b0fbe40, 0xc4260e1a78, 0xc42f530f00)
	/workspace/anago-v1.10.1-beta.0.68+d4ab47518836c7/src/k8s.io/kubernetes/_output/dockerized/go/src/k8s.io/kubernetes/vendor/github.com/emicklei/go-restful/container.go:199 +0x57
k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/server.director.ServeHTTP(0x3d2edf3, 0xe, 0xc4200f6e10, 0xc420393490, 0x7ff67b0fbe40, 0xc4260e1a78, 0xc42f530f00)
	/workspace/anago-v1.10.1-beta.0.68+d4ab47518836c7/src/k8s.io/kubernetes/_output/dockerized/go/src/k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/server/handler.go:152 +0x4e0
k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/server.(*director).ServeHTTP(0xc42042b200, 0x7ff67b0fbe40, 0xc4260e1a78, 0xc42f530f00)
	<autogenerated>:1 +0x75
k8s.io/kubernetes/vendor/k8s.io/kube-aggregator/pkg/apiserver.(*proxyHandler).ServeHTTP(0xc427b9db00, 0x7ff67b0fbe40, 0xc4260e1a78, 0xc42f530f00)
	/workspace/anago-v1.10.1-beta.0.68+d4ab47518836c7/src/k8s.io/kubernetes/_output/dockerized/go/src/k8s.io/kubernetes/vendor/k8s.io/kube-aggregator/pkg/apiserver/handler_proxy.go:93 +0x18a
k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/server/mux.(*pathHandler).ServeHTTP(0xc4298a8540, 0x7ff67b0fbe40, 0xc4260e1a78, 0xc42f530f00)
	/workspace/anago-v1.10.1-beta.0.68+d4ab47518836c7/src/k8s.io/kubernetes/_output/dockerized/go/src/k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/server/mux/pathrecorder.go:248 +0x26d
k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/server/mux.(*PathRecorderMux).ServeHTTP(0xc422503650, 0x7ff67b0fbe40, 0xc4260e1a78, 0xc42f530f00)
	/workspace/anago-v1.10.1-beta.0.68+d4ab47518836c7/src/k8s.io/kubernetes/_output/dockerized/go/src/k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/server/mux/pathrecorder.go:234 +0xa1
k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/server.director.ServeHTTP(0x3d31bb4, 0xf, 0xc421d1b320, 0xc422503650, 0x7ff67b0fbe40, 0xc4260e1a78, 0xc42f530f00)
	/workspace/anago-v1.10.1-beta.0.68+d4ab47518836c7/src/k8s.io/kubernetes/_output/dockerized/go/src/k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/server/handler.go:160 +0x6ad
k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/server.(*director).ServeHTTP(0xc4235217a0, 0x7ff67b0fbe40, 0xc4260e1a78, 0xc42f530f00)
	<autogenerated>:1 +0x75
k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/endpoints/filters.WithAuthorization.func1(0x7ff67b0fbe40, 0xc4260e1a78, 0xc42f530f00)
	/workspace/anago-v1.10.1-beta.0.68+d4ab47518836c7/src/k8s.io/kubernetes/_output/dockerized/go/src/k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/endpoints/filters/authorization.go:52 +0x37d
net/http.HandlerFunc.ServeHTTP(0xc42302dd10, 0x7ff67b0fbe40, 0xc4260e1a78, 0xc42f530f00)
	/usr/local/go/src/net/http/server.go:1918 +0x44
k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/server/filters.WithMaxInFlightLimit.func1(0x7ff67b0fbe40, 0xc4260e1a78, 0xc42f530f00)
	/workspace/anago-v1.10.1-beta.0.68+d4ab47518836c7/src/k8s.io/kubernetes/_output/dockerized/go/src/k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/server/filters/maxinflight.go:165 +0x42a
net/http.HandlerFunc.ServeHTTP(0xc4226f5300, 0x7ff67b0fbe40, 0xc4260e1a78, 0xc42f530f00)
	/usr/local/go/src/net/http/server.go:1918 +0x44
k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/endpoints/filters.WithImpersonation.func1(0x7ff67b0fbe40, 0xc4260e1a78, 0xc42f530f00)
	/workspace/anago-v1.10.1-beta.0.68+d4ab47518836c7/src/k8s.io/kubernetes/_output/dockerized/go/src/k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/endpoints/filters/impersonation.go:49 +0x203a
net/http.HandlerFunc.ServeHTTP(0xc42302dd60, 0x7ff67b0fbe40, 0xc4260e1a78, 0xc42f530f00)
	/usr/local/go/src/net/http/server.go:1918 +0x44
k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/endpoints/filters.WithAuthentication.func1(0x7ff67b0fbe40, 0xc4260e1a78, 0xc42f530f00)
	/workspace/anago-v1.10.1-beta.0.68+d4ab47518836c7/src/k8s.io/kubernetes/_output/dockerized/go/src/k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/endpoints/filters/authentication.go:78 +0x2b1
net/http.HandlerFunc.ServeHTTP(0xc42302ddb0, 0x7ff67b0fbe40, 0xc4260e1a78, 0xc42f530f00)
	/usr/local/go/src/net/http/server.go:1918 +0x44
k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/endpoints/request.WithRequestContext.func1(0x7ff67b0fbe40, 0xc4260e1a78, 0xc42f530f00)
	/workspace/anago-v1.10.1-beta.0.68+d4ab47518836c7/src/k8s.io/kubernetes/_output/dockerized/go/src/k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/endpoints/request/requestcontext.go:110 +0xcb
net/http.HandlerFunc.ServeHTTP(0xc4235217c0, 0x7ff67b0fbe40, 0xc4260e1a78, 0xc42f530f00)
	/usr/local/go/src/net/http/server.go:1918 +0x44
k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/server/filters.(*timeoutHandler).ServeHTTP.func1(0xc423521840, 0x9884740, 0xc4260e1a78, 0xc42f530f00, 0xc42f6b22a0)
	/workspace/anago-v1.10.1-beta.0.68+d4ab47518836c7/src/k8s.io/kubernetes/_output/dockerized/go/src/k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/server/filters/timeout.go:93 +0x8d
created by k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/server/filters.(*timeoutHandler).ServeHTTP
	/workspace/anago-v1.10.1-beta.0.68+d4ab47518836c7/src/k8s.io/kubernetes/_output/dockerized/go/src/k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/server/filters/timeout.go:92 +0x1ab

What you expected to happen: No crash

/sig-api-machinery

@alvaroaleman
Copy link
Member Author

/sig api-machinery

@k8s-ci-robot k8s-ci-robot added the sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. label May 24, 2018
@wgliang
Copy link
Contributor

wgliang commented May 25, 2018

This maybe a bug in github.com/evanphx/json-patch.
I will follow this question.

@wgliang
Copy link
Contributor

wgliang commented May 25, 2018

This bug had been fixed in PR:#49259
you can checkout it.

@alvaroaleman
Copy link
Member Author

@wgliang Thanks for your comment but I don't think the linked PR fixed the issue, as it got merged almost a year ago and I am running Kube 1.10.1 which does contain that patch but still panics.

Just from quickly looking at the code I the linked PR I believe there are two things wrong:

  • The check if the index is valid only happens when a negative index was given
  • if idx > len(ary) should be if idx >= len(ary)

@wgliang
Copy link
Contributor

wgliang commented May 25, 2018

@alvaroaleman Once the PR of evanphx/json-patch is merged I will update kubernetes.

@dims
Copy link
Member

dims commented May 26, 2018

/kind bug

@k8s-ci-robot k8s-ci-robot added the kind/bug Categorizes issue or PR as related to a bug. label May 26, 2018
k8s-github-robot pushed a commit that referenced this issue May 27, 2018
Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

bump(github.com/evanphx/json-patch): 94e38aa1586e8a6c8a75770bddf5ff84c48a106b

update package github.com/evanphx/json-patch

fixes #64291

/cc  wgliang cblecker

```release-note
fixes a panic applying json patches containing out of bounds operations
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/admission-control kind/bug Categorizes issue or PR as related to a bug. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery.
Projects
None yet
5 participants