diff --git a/core/middleware/admission/admission.go b/core/middleware/admission/admission.go index 64dbf35c..63b3c41f 100644 --- a/core/middleware/admission/admission.go +++ b/core/middleware/admission/admission.go @@ -42,17 +42,21 @@ func Middleware(skippers ...middleware.Skipper) gin.HandlerFunc { rpcerror.ParamError.WithErrMsg(fmt.Sprintf("request body is invalid, err: %v", err))) return } + // restore the request body c.Request.Body = ioutil.NopCloser(bytes.NewBuffer(bodyBytes)) if len(bodyBytes) > 0 { contentType := c.ContentType() - if contentType == binding.MIMEJSON { + if contentType == binding.MIMEJSON || contentType == "" { if err := json.Unmarshal(bodyBytes, &object); err != nil { response.AbortWithRPCError(c, rpcerror.ParamError.WithErrMsg(fmt.Sprintf("unmarshal request body failed, err: %v", err))) return } } else { - log.Warningf(c, "unsupported content type: %s", contentType) + log.Errorf(c, "unsupported content type: %s", contentType) + response.AbortWithRPCError(c, + rpcerror.ParamError.WithErrMsg(fmt.Sprintf("unsupported content type: %s", contentType))) + return } } // fill in the request url query into admission request options @@ -62,13 +66,13 @@ func Middleware(skippers ...middleware.Skipper) gin.HandlerFunc { options[k] = v } admissionRequest := &admissionwebhook.Request{ - Operation: admissionmodels.Operation(attr.GetVerb()), - Resource: attr.GetResource(), - ResourceName: attr.GetName(), - SubResource: attr.GetSubResource(), - Version: attr.GetAPIVersion(), - Object: object, - Options: options, + Operation: admissionmodels.Operation(attr.GetVerb()), + Resource: attr.GetResource(), + Name: attr.GetName(), + SubResource: attr.GetSubResource(), + Version: attr.GetAPIVersion(), + Object: object, + Options: options, } if err := admissionwebhook.Validating(c, admissionRequest); err != nil { response.AbortWithRPCError(c, diff --git a/pkg/admission/http.go b/pkg/admission/http.go index fbec07d0..c1dcef7b 100644 --- a/pkg/admission/http.go +++ b/pkg/admission/http.go @@ -20,6 +20,8 @@ import ( "github.com/horizoncd/horizon/pkg/util/common" ) +const DefaultTimeout = 5 * time.Second + type HTTPAdmissionClient struct { config config.ClientConfig http.Client @@ -45,6 +47,9 @@ func NewHTTPAdmissionClient(config config.ClientConfig, timeout time.Duration) * }, } } + if timeout == 0 { + timeout = DefaultTimeout + } return &HTTPAdmissionClient{ config: config, Client: http.Client{ @@ -61,7 +66,7 @@ func (c *HTTPAdmissionClient) Get(ctx context.Context, admitData *Request) (*Res return nil, err } - req, err := http.NewRequest("POST", c.config.URL, bytes.NewReader(body)) + req, err := http.NewRequestWithContext(ctx, "POST", c.config.URL, bytes.NewReader(body)) if err != nil { return nil, err } @@ -183,7 +188,7 @@ func NewHTTPWebhooks(config config.Admission) { } func NewHTTPWebhook(config config.Webhook) Webhook { - client := NewHTTPAdmissionClient(config.ClientConfig, time.Duration(config.TimeoutSeconds)*time.Second) + client := NewHTTPAdmissionClient(config.ClientConfig, config.Timeout) matchers := NewResourceMatchers(config.Rules) return &HTTPAdmissionWebhook{ config: config, diff --git a/pkg/admission/webhook.go b/pkg/admission/webhook.go index 21fa4c02..b54a57b8 100644 --- a/pkg/admission/webhook.go +++ b/pkg/admission/webhook.go @@ -2,7 +2,6 @@ package admission import ( "context" - "time" "k8s.io/apimachinery/pkg/util/runtime" @@ -12,8 +11,6 @@ import ( "github.com/horizoncd/horizon/pkg/util/log" ) -const DefaultTimeout = 5 * time.Second - var ( validatingWebhooks []Webhook ) @@ -25,31 +22,26 @@ func Register(kind models.Kind, webhook Webhook) { } } -func Clear() { - validatingWebhooks = nil -} - type validateResult struct { + req Request err error resp *Response } type Request struct { - Operation models.Operation `json:"operation"` - Resource string `json:"resource"` - ResourceName string `json:"resourceName"` - SubResource string `json:"subResource"` - Version string `json:"version"` - Object interface{} `json:"object"` - OldObject interface{} `json:"oldObject"` - Options map[string]interface{} `json:"options,omitempty"` + Operation models.Operation `json:"operation"` + Resource string `json:"resource"` + Name string `json:"name"` + SubResource string `json:"subResource"` + Version string `json:"version"` + Object interface{} `json:"object"` + OldObject interface{} `json:"oldObject"` + Options map[string]interface{} `json:"options,omitempty"` } type Response struct { - Allowed *bool `json:"allowed"` - Result string `json:"result,omitempty"` - Patch []byte `json:"patch,omitempty"` - PatchType string `json:"patchType,omitempty"` + Allowed *bool `json:"allowed"` + Result string `json:"result,omitempty"` } type Webhook interface { @@ -67,29 +59,29 @@ func Validating(ctx context.Context, request *Request) error { go func(webhook Webhook) { defer runtime.HandleCrash() if !webhook.Interest(request) { - resCh <- validateResult{nil, nil} + resCh <- validateResult{*request, nil, nil} return } response, err := webhook.Handle(ctx, request) if err != nil { if webhook.IgnoreError() { log.Errorf(ctx, "failed to admit request: %v", err) - resCh <- validateResult{nil, nil} + resCh <- validateResult{*request, nil, nil} return } - resCh <- validateResult{err, nil} + resCh <- validateResult{*request, err, nil} return } if response == nil || response.Allowed == nil { if webhook.IgnoreError() { log.Errorf(ctx, "failed to admit request: response is nil or allowed is nil") - resCh <- validateResult{nil, nil} + resCh <- validateResult{*request, nil, nil} return } - resCh <- validateResult{perror.New("response is nil or allowed is nil"), nil} + resCh <- validateResult{*request, perror.New("response is nil or allowed is nil"), nil} return } - resCh <- validateResult{nil, response} + resCh <- validateResult{*request, nil, response} }(webhook) } @@ -99,7 +91,10 @@ func Validating(ctx context.Context, request *Request) error { return res.err } if res.resp != nil && res.resp.Allowed != nil && !*res.resp.Allowed { - log.Infof(ctx, "request denied by webhook: %s", res.resp.Result) + log.Infof(ctx, + "request (resource: %s, resourceName: %s, subresource: %s, operation: %s) denied by webhook: %s", + res.req.Resource, res.req.Name, res.req.SubResource, + res.req.Operation, res.resp.Result) return perror.Wrapf(herrors.ErrForbidden, "request denied by webhook: %s", res.resp.Result) } if finishedCount >= len(validatingWebhooks) { diff --git a/pkg/admission/webhook_test.go b/pkg/admission/webhook_test.go index 5022d909..4a43f76b 100644 --- a/pkg/admission/webhook_test.go +++ b/pkg/admission/webhook_test.go @@ -3,6 +3,7 @@ package admission import ( "context" "testing" + "time" clusterctrl "github.com/horizoncd/horizon/core/controller/cluster" "github.com/horizoncd/horizon/pkg/admission/models" @@ -22,10 +23,9 @@ func TestWebhook(t *testing.T) { config := admissionconfig.Admission{ Webhooks: []admissionconfig.Webhook{ { - Name: "test1", - Kind: models.KindValidating, - FailurePolicy: admissionconfig.FailurePolicyFail, - TimeoutSeconds: 5, + Kind: models.KindValidating, + FailurePolicy: admissionconfig.FailurePolicyFail, + Timeout: 5 * time.Second, Rules: []admissionconfig.Rule{ { Resources: []string{ @@ -42,10 +42,9 @@ func TestWebhook(t *testing.T) { }, }, { - Name: "test2", - Kind: models.KindValidating, - FailurePolicy: admissionconfig.FailurePolicyIgnore, - TimeoutSeconds: 5, + Kind: models.KindValidating, + FailurePolicy: admissionconfig.FailurePolicyIgnore, + Timeout: 5 * time.Second, Rules: []admissionconfig.Rule{ { Resources: []string{ @@ -80,13 +79,13 @@ func TestWebhook(t *testing.T) { } createRequest := &Request{ - Operation: models.OperationCreate, - Resource: "applications", - ResourceName: "1", - SubResource: "clusters", - Version: "v2", - Object: createBody, - OldObject: nil, + Operation: models.OperationCreate, + Resource: "applications", + Name: "1", + SubResource: "clusters", + Version: "v2", + Object: createBody, + OldObject: nil, Options: map[string]interface{}{ "scope": []string{"online/hz1"}, }, @@ -114,14 +113,14 @@ func TestWebhook(t *testing.T) { }, } updateRequest := &Request{ - Operation: models.OperationUpdate, - Resource: "clusters", - ResourceName: "1", - SubResource: "", - Version: "v2", - Object: updateBody, - OldObject: nil, - Options: nil, + Operation: models.OperationUpdate, + Resource: "clusters", + Name: "1", + SubResource: "", + Version: "v2", + Object: updateBody, + OldObject: nil, + Options: nil, } err = Validating(ctx, updateRequest) assert.NoError(t, err) diff --git a/pkg/config/admission/config.go b/pkg/config/admission/config.go index 2bb1832b..0971f926 100644 --- a/pkg/config/admission/config.go +++ b/pkg/config/admission/config.go @@ -2,6 +2,7 @@ package admission import ( "strings" + "time" "github.com/horizoncd/horizon/pkg/admission/models" ) @@ -30,12 +31,11 @@ type Rule struct { } type Webhook struct { - Name string `yaml:"name"` - Kind models.Kind `yaml:"kind"` - FailurePolicy FailurePolicy `yaml:"failurePolicy"` - TimeoutSeconds int32 `yaml:"timeoutSeconds"` - Rules []Rule `yaml:"rules"` - ClientConfig ClientConfig `yaml:"clientConfig"` + Kind models.Kind `yaml:"kind"` + FailurePolicy FailurePolicy `yaml:"failurePolicy"` + Timeout time.Duration `yaml:"timeout"` + Rules []Rule `yaml:"rules"` + ClientConfig ClientConfig `yaml:"clientConfig"` } type Admission struct {