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

Explicitly disable dry run for connect #66083

Merged
merged 1 commit into from
Aug 7, 2018
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,11 @@ func (scope *RequestScope) AllowsStreamSchema(s string) bool {
// ConnectResource returns a function that handles a connect request on a rest.Storage object.
func ConnectResource(connecter rest.Connecter, scope RequestScope, admit admission.Interface, restPath string, isSubresource bool) http.HandlerFunc {
return func(w http.ResponseWriter, req *http.Request) {
if isDryRun(req.URL) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @jennybuckley , if we were to forbid dryRun requests for all verbs, why not do this via an HTTP handler filter. 😃

Copy link
Author

@jennybuckley jennybuckley Jul 12, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for reviewing this! The reason is that we plan on enabling it for some verbs in the future, depending on whether or not the dry run feature is enabled. Also, some verbs, like GET, don't make any change in the cluster, so dry running a get is equivalent to just running the get normally, so we don't need to fail if the client sends the dry run flag.

scope.err(errors.NewBadRequest("dryRun is not supported"), w, req)
return
}

namespace, name, err := scope.Namer.Name(req)
if err != nil {
scope.err(err, w, req)
Expand Down