From 63eb27082dc854b4b3922633c2ac04b87d409ba5 Mon Sep 17 00:00:00 2001 From: "xu.zhu" Date: Wed, 6 Dec 2023 22:41:35 +0800 Subject: [PATCH 1/4] feat: admission validating webhook Signed-off-by: xu.zhu --- core/cmd/cmd.go | 12 +- core/config/config.go | 2 + core/controller/application/admission.go | 1 + core/controller/cluster/admission.go | 73 ++++++ core/errors/horizonerrors.go | 2 +- core/middleware/admission/admission.go | 54 +++++ core/middleware/tag/tag.go | 2 +- go.mod | 2 + go.sum | 4 + pkg/admission/http.go | 284 +++++++++++++++++++++++ pkg/admission/models/models.go | 25 ++ pkg/admission/webhook.go | 140 +++++++++++ pkg/config/admission/config.go | 43 ++++ 13 files changed, 641 insertions(+), 3 deletions(-) create mode 100644 core/controller/application/admission.go create mode 100644 core/controller/cluster/admission.go create mode 100644 core/middleware/admission/admission.go create mode 100644 pkg/admission/http.go create mode 100644 pkg/admission/models/models.go create mode 100644 pkg/admission/webhook.go create mode 100644 pkg/config/admission/config.go diff --git a/core/cmd/cmd.go b/core/cmd/cmd.go index d6c3f9d8f..65b778a38 100644 --- a/core/cmd/cmd.go +++ b/core/cmd/cmd.go @@ -107,8 +107,10 @@ import ( "github.com/horizoncd/horizon/core/middleware/auth" "github.com/horizoncd/horizon/core/middleware/requestid" gitlablib "github.com/horizoncd/horizon/lib/gitlab" + "github.com/horizoncd/horizon/pkg/admission" "github.com/horizoncd/horizon/pkg/cd" clustermetrcis "github.com/horizoncd/horizon/pkg/cluster/metrics" + admissionconfig "github.com/horizoncd/horizon/pkg/config/admission" "github.com/horizoncd/horizon/pkg/environment/service" eventservice "github.com/horizoncd/horizon/pkg/event/service" "github.com/horizoncd/horizon/pkg/grafana" @@ -137,6 +139,7 @@ import ( templatev2 "github.com/horizoncd/horizon/core/http/api/v2/template" "github.com/horizoncd/horizon/core/http/health" "github.com/horizoncd/horizon/core/http/metrics" + admissionmiddle "github.com/horizoncd/horizon/core/middleware/admission" ginlogmiddle "github.com/horizoncd/horizon/core/middleware/ginlog" logmiddle "github.com/horizoncd/horizon/core/middleware/log" metricsmiddle "github.com/horizoncd/horizon/core/middleware/metrics" @@ -227,6 +230,10 @@ func ParseFlags() *Flags { return &flags } +func InitAdmissionWebhook(config admissionconfig.Admission) { + admission.NewHTTPWebhooks(config) +} + func InitLog(flags *Flags) { if flags.Environment == "production" { logrus.SetFormatter(&logrus.JSONFormatter{}) @@ -624,7 +631,8 @@ func Init(ctx context.Context, flags *Flags, coreConfig *config.Config) { middleware.MethodAndPathSkipper(http.MethodPost, regexp.MustCompile("^/apis/core/v[12]/users/login"))), prehandlemiddle.Middleware(r, manager), auth.Middleware(rbacAuthorizer, authzSkippers...), - tagmiddle.Middleware(), // tag middleware, parse and attach tagSelector to context + tagmiddle.Middleware(), + admissionmiddle.Middleware(authzSkippers...), } r.Use(middlewares...) @@ -725,6 +733,8 @@ func Run(flags *Flags) { panic(err) } + InitAdmissionWebhook(configs.Admission) + // enable pprof runPProfServer(&configs.PProf) diff --git a/core/config/config.go b/core/config/config.go index 84f1ed714..d2e720dd4 100644 --- a/core/config/config.go +++ b/core/config/config.go @@ -18,6 +18,7 @@ import ( "io/ioutil" "strings" + "github.com/horizoncd/horizon/pkg/config/admission" "github.com/horizoncd/horizon/pkg/config/argocd" "github.com/horizoncd/horizon/pkg/config/authenticate" "github.com/horizoncd/horizon/pkg/config/autofree" @@ -67,6 +68,7 @@ type Config struct { TemplateUpgradeMapper template.UpgradeMapper `yaml:"templateUpgradeMapper"` KubernetesEvent k8sevent.Config `yaml:"kubernetesEvent"` Clean clean.Config `yaml:"clean"` + Admission admission.Admission `yaml:"admission"` } func LoadConfig(configFilePath string) (*Config, error) { diff --git a/core/controller/application/admission.go b/core/controller/application/admission.go new file mode 100644 index 000000000..b584a8a4b --- /dev/null +++ b/core/controller/application/admission.go @@ -0,0 +1 @@ +package application diff --git a/core/controller/cluster/admission.go b/core/controller/cluster/admission.go new file mode 100644 index 000000000..506e53b81 --- /dev/null +++ b/core/controller/cluster/admission.go @@ -0,0 +1,73 @@ +package cluster + +import ( + "github.com/horizoncd/horizon/core/common" + appmodels "github.com/horizoncd/horizon/pkg/application/models" + "github.com/horizoncd/horizon/pkg/cluster/code" + "github.com/horizoncd/horizon/pkg/cluster/models" + tagmodels "github.com/horizoncd/horizon/pkg/tag/models" +) + +type Cluster struct { + *models.Cluster `json:",inline"` + *TemplateInput `json:"templateInput,omitempty"` + Tags tagmodels.TagsBasic `json:"tags,omitempty"` +} + +type Clusterv2 struct { + *models.Cluster `json:",inline"` + *TemplateInput `json:"templateInput,omitempty"` + TemplateConfig map[string]interface{} `json:"templateConfig,omitempty"` + TemplateInfo *code.TemplateInfo `json:"-"` + MergePatch bool `json:"mergePatch"` + BuildConfig map[string]interface{} `json:"buildConfig"` + Tags tagmodels.TagsBasic `json:"tags,omitempty"` +} + +func (c *Clusterv2) toClusterModel(application *appmodels.Application) *models.Cluster { + cluster := &models.Cluster{ + ApplicationID: c.ApplicationID, + Name: c.Name, + EnvironmentName: c.EnvironmentName, + RegionName: c.RegionName, + Description: c.Description, + ExpireSeconds: c.ExpireSeconds, + Template: c.Template, + TemplateRelease: c.TemplateRelease, + Status: common.ClusterStatusCreating, + } + if cluster.Template == application.Template { + cluster.GitURL = func() string { + if c.GitURL == "" && application.GitURL != "" { + return application.GitURL + } + // if URL is empty string, this means this cluster not depends on build from git + return c.GitURL + }() + cluster.GitSubfolder = func() string { + if c.GitSubfolder == "" { + return application.GitSubfolder + } + return c.GitSubfolder + }() + cluster.GitRef = func() string { + if c.GitRef == "" { + return application.GitRef + } + return c.GitRef + }() + cluster.GitRefType = func() string { + if c.GitRefType == "" { + return application.GitRefType + } + return c.GitRefType + }() + cluster.Image = func() string { + if c.Image == "" { + return application.Image + } + return c.Image + }() + } + return cluster +} diff --git a/core/errors/horizonerrors.go b/core/errors/horizonerrors.go index 391d1592b..9f8c68f40 100644 --- a/core/errors/horizonerrors.go +++ b/core/errors/horizonerrors.go @@ -212,7 +212,7 @@ var ( ErrGenerateRandomID = errors.New("failed to generate random id") ErrDisabled = errors.New("entity is disabled") ErrDuplicatedKey = errors.New("duplicated keys") - // ErrInternal = errors.New("internal error") + ErrValidatingFailed = errors.New("validating failed") // http ErrHTTPRespNotAsExpected = errors.New("http response is not as expected") diff --git a/core/middleware/admission/admission.go b/core/middleware/admission/admission.go new file mode 100644 index 000000000..a6b26a2fa --- /dev/null +++ b/core/middleware/admission/admission.go @@ -0,0 +1,54 @@ +package admission + +import ( + "fmt" + + "github.com/gin-gonic/gin" + "github.com/horizoncd/horizon/core/common" + "github.com/horizoncd/horizon/core/middleware" + admissionwebhook "github.com/horizoncd/horizon/pkg/admission" + admissionmodels "github.com/horizoncd/horizon/pkg/admission/models" + "github.com/horizoncd/horizon/pkg/auth" + "github.com/horizoncd/horizon/pkg/server/response" + "github.com/horizoncd/horizon/pkg/server/rpcerror" +) + +// Middleware to validate and mutate admission request +func Middleware(skippers ...middleware.Skipper) gin.HandlerFunc { + return middleware.New(func(c *gin.Context) { + // get auth record + record, ok := c.Get(common.ContextAuthRecord) + if !ok { + response.AbortWithRPCError(c, + rpcerror.BadRequestError.WithErrMsg("request with no auth record")) + return + } + attr := record.(auth.AttributesRecord) + // non resource request or read only request should be ignored + if !attr.IsResourceRequest() || attr.IsReadOnly() { + c.Next() + return + } + var object interface{} + if err := c.ShouldBind(object); err != nil { + response.AbortWithRPCError(c, + rpcerror.ParamError.WithErrMsg(fmt.Sprintf("request body is invalid, err: %v", err))) + return + } + admissionRequest := &admissionwebhook.Request{ + Operation: admissionmodels.Operation(attr.GetVerb()), + Resource: attr.GetResource(), + ResourceName: attr.GetName(), + SubResource: attr.GetSubResource(), + Version: attr.GetAPIVersion(), + Object: object, + OldObject: nil, + } + if err := admissionwebhook.Validating(c, admissionRequest); err != nil { + response.AbortWithRPCError(c, + rpcerror.ParamError.WithErrMsg(fmt.Sprintf("admission validating failed: %v", err))) + return + } + c.Next() + }, skippers...) +} diff --git a/core/middleware/tag/tag.go b/core/middleware/tag/tag.go index d5a40132b..2464fe713 100644 --- a/core/middleware/tag/tag.go +++ b/core/middleware/tag/tag.go @@ -28,7 +28,7 @@ import ( "k8s.io/apimachinery/pkg/labels" ) -// Middleware to parse tagSelector params +// Middleware to parse and attach tagSelector to context func Middleware(skippers ...middleware.Skipper) gin.HandlerFunc { return middleware.New(func(c *gin.Context) { // parse tagSelector diff --git a/go.mod b/go.mod index 3a2a86603..0d207217a 100644 --- a/go.mod +++ b/go.mod @@ -21,6 +21,7 @@ require ( github.com/hashicorp/go-retryablehttp v0.6.8 github.com/igm/sockjs-go v3.0.2+incompatible // indirect github.com/johannesboyne/gofakes3 v0.0.0-20210819161434-5c8dfcfe5310 + github.com/mattbaird/jsonpatch v0.0.0-20230413205102-771768614e91 github.com/mozillazg/go-pinyin v0.18.0 github.com/pkg/errors v0.9.1 github.com/prometheus/client_golang v1.11.0 @@ -37,6 +38,7 @@ require ( golang.org/x/net v0.0.0-20220107192237-5cfca573fb4d golang.org/x/oauth2 v0.0.0-20200107190931-bf48bf16ab8d gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c // indirect + gopkg.in/evanphx/json-patch.v5 v5.6.0 gopkg.in/igm/sockjs-go.v3 v3.0.1 gopkg.in/natefinch/lumberjack.v2 v2.0.0 gopkg.in/yaml.v3 v3.0.1 diff --git a/go.sum b/go.sum index 8486f3319..c850a5674 100644 --- a/go.sum +++ b/go.sum @@ -1209,6 +1209,8 @@ github.com/marstr/guid v1.1.0/go.mod h1:74gB1z2wpxxInTG6yaqA7KrtM0NZ+RbrcqDvYHef github.com/marten-seemann/qtls v0.2.3/go.mod h1:xzjG7avBwGGbdZ8dTGxlBnLArsVKLvwmjgmPuiQEcYk= github.com/matoous/godox v0.0.0-20190911065817-5d6d842e92eb/go.mod h1:1BELzlh859Sh1c6+90blK8lbYy0kwQf1bYlBhBysy1s= github.com/mattbaird/jsonpatch v0.0.0-20171005235357-81af80346b1a/go.mod h1:M1qoD/MqPgTZIk0EWKB38wE28ACRfVcn+cU08jyArI0= +github.com/mattbaird/jsonpatch v0.0.0-20230413205102-771768614e91 h1:JnZSkFP1/GLwKCEuuWVhsacvbDQIVa5BRwAwd+9k2Vw= +github.com/mattbaird/jsonpatch v0.0.0-20230413205102-771768614e91/go.mod h1:M1qoD/MqPgTZIk0EWKB38wE28ACRfVcn+cU08jyArI0= github.com/mattn/go-colorable v0.0.9/go.mod h1:9vuHe8Xs5qXnSaW/c/ABM9alt+Vo+STaOChaDxuIBZU= github.com/mattn/go-colorable v0.1.1/go.mod h1:FuOcm+DKB9mbwrcAfNl7/TZVBZ6rcnceauSikq3lYCQ= github.com/mattn/go-colorable v0.1.2/go.mod h1:U0ppj6V5qS13XJ6of8GYAs25YV2eR4EVcfRqFIhoBtE= @@ -2473,6 +2475,8 @@ gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c h1:Hei/4ADfdWqJk1ZMxUNpqntN gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c/go.mod h1:JHkPIbrfpd72SG/EVd6muEfDQjcINNoR0C8j2r3qZ4Q= gopkg.in/cheggaaa/pb.v1 v1.0.25/go.mod h1:V/YB90LKu/1FcN3WVnfiiE5oMCibMjukxqG/qStrOgw= gopkg.in/errgo.v2 v2.1.0/go.mod h1:hNsd1EY+bozCKY1Ytp96fpM3vjJbqLJn88ws8XvfDNI= +gopkg.in/evanphx/json-patch.v5 v5.6.0 h1:BMT6KIwBD9CaU91PJCZIe46bDmBWa9ynTQgJIOpfQBk= +gopkg.in/evanphx/json-patch.v5 v5.6.0/go.mod h1:/kvTRh1TVm5wuM6OkHxqXtE/1nUZZpihg29RtuIyfvk= gopkg.in/fsnotify.v1 v1.4.7/go.mod h1:Tz8NjZHkW78fSQdbUxIjBTcgA1z1m8ZHf0WmKUhAMys= gopkg.in/gcfg.v1 v1.2.0/go.mod h1:yesOnuUOFQAhST5vPY4nbZsb/huCgGGXlipJsBn0b3o= gopkg.in/gcfg.v1 v1.2.3/go.mod h1:yesOnuUOFQAhST5vPY4nbZsb/huCgGGXlipJsBn0b3o= diff --git a/pkg/admission/http.go b/pkg/admission/http.go new file mode 100644 index 000000000..6de02892c --- /dev/null +++ b/pkg/admission/http.go @@ -0,0 +1,284 @@ +package admission + +import ( + "bytes" + "context" + "crypto/tls" + "crypto/x509" + "encoding/json" + "fmt" + "io/ioutil" + "net/http" + "net/http/httptest" + "strings" + "time" + + herrors "github.com/horizoncd/horizon/core/errors" + "github.com/horizoncd/horizon/pkg/admission/models" + config "github.com/horizoncd/horizon/pkg/config/admission" + perror "github.com/horizoncd/horizon/pkg/errors" +) + +type HTTPAdmissionClient struct { + config config.ClientConfig + http.Client +} + +func NewHTTPAdmissionClient(config config.ClientConfig, timeout time.Duration) *HTTPAdmissionClient { + var transport = &http.Transport{} + if config.CABundle != "" { + ca := config.CABundle + certPool := x509.NewCertPool() + certPool.AppendCertsFromPEM([]byte(ca)) + transport = &http.Transport{ + TLSClientConfig: &tls.Config{ + RootCAs: certPool, + }, + } + } + if config.Insecure { + transport = &http.Transport{ + TLSClientConfig: &tls.Config{ + InsecureSkipVerify: true, + }, + } + } + return &HTTPAdmissionClient{ + config: config, + Client: http.Client{ + Timeout: timeout, + Transport: transport, + }, + } +} + +func (c *HTTPAdmissionClient) Get(ctx context.Context, admitData *Request) (*Response, error) { + body, err := json.Marshal(admitData) + if err != nil { + return nil, err + } + + req, err := http.NewRequest("POST", c.config.URL, bytes.NewReader(body)) + if err != nil { + return nil, err + } + req.Header.Set("Content-Type", "application/json") + + resp, err := c.Client.Do(req) + if err != nil { + return nil, err + } + + if resp.StatusCode != http.StatusOK { + return nil, perror.Wrapf(herrors.ErrHTTPRespNotAsExpected, "status code: %d", resp.StatusCode) + } + defer resp.Body.Close() + + var response Response + if err := json.NewDecoder(resp.Body).Decode(&response); err != nil { + return nil, err + } + return &response, nil +} + +type ResourceMatcher struct { + resources map[string]struct{} + operations map[models.Operation]struct{} + versions map[string]struct{} +} + +func NewResourceMatcher(rule config.Rule) *ResourceMatcher { + matcher := &ResourceMatcher{ + resources: make(map[string]struct{}), + operations: make(map[models.Operation]struct{}), + versions: make(map[string]struct{}), + } + for _, resource := range rule.Resources { + if resource == models.MatchAll { + matcher.resources = nil + break + } + matcher.resources[resource] = struct{}{} + } + for _, operation := range rule.Operations { + if operation.Eq(models.Operation(models.MatchAll)) { + matcher.operations = nil + break + } + matcher.operations[operation] = struct{}{} + } + for _, version := range rule.Versions { + if version == models.MatchAll { + matcher.versions = nil + break + } + matcher.versions[version] = struct{}{} + } + return matcher +} + +func (m *ResourceMatcher) Match(req *Request) bool { + if m.resources != nil { + resource := req.Resource + if req.SubResource != "" { + resource = fmt.Sprintf("%s/%s", resource, req.SubResource) + } + if _, ok := m.resources[resource]; !ok { + return false + } + } + if m.operations != nil { + if _, ok := m.operations[req.Operation]; !ok { + return false + } + } + if m.versions != nil { + if _, ok := m.versions[req.Version]; !ok { + return false + } + } + return true +} + +type ResourceMatchers []*ResourceMatcher + +func NewResourceMatchers(rules []config.Rule) ResourceMatchers { + matchers := make(ResourceMatchers, len(rules)) + for i, rule := range rules { + matchers[i] = NewResourceMatcher(rule) + } + return matchers +} + +func (m ResourceMatchers) Match(req *Request) bool { + for _, matcher := range m { + if matcher.Match(req) { + return true + } + } + return false +} + +type HTTPAdmissionWebhook struct { + config config.Webhook + httpclient *HTTPAdmissionClient + matchers ResourceMatchers +} + +func NewHTTPWebhooks(config config.Admission) { + for _, webhook := range config.Webhooks { + switch webhook.Kind { + case models.KindValidating: + Register(models.KindValidating, NewHTTPWebhook(webhook)) + } + } +} + +func NewHTTPWebhook(config config.Webhook) Webhook { + client := NewHTTPAdmissionClient(config.ClientConfig, time.Duration(config.TimeoutSeconds)*time.Second) + matchers := NewResourceMatchers(config.Rules) + return &HTTPAdmissionWebhook{ + config: config, + httpclient: client, + matchers: matchers, + } +} + +func (m *HTTPAdmissionWebhook) Handle(ctx context.Context, req *Request) (*Response, error) { + resp, err := m.httpclient.Get(ctx, req) + if err != nil { + return nil, err + } + return resp, nil +} + +func (m *HTTPAdmissionWebhook) IgnoreError() bool { + return m.config.FailurePolicy.Eq(config.FailurePolicyIgnore) +} + +func (m *HTTPAdmissionWebhook) Interest(req *Request) bool { + return m.matchers.Match(req) +} + +type DummyMutatingWebhookServer struct { + server *httptest.Server +} + +func NewDummyWebhookServer() *DummyMutatingWebhookServer { + webhook := &DummyMutatingWebhookServer{} + + mux := http.NewServeMux() + mux.HandleFunc("/validate", webhook.Validating) + + server := httptest.NewServer(mux) + webhook.server = server + return webhook +} + +func (*DummyMutatingWebhookServer) ReadAndResponse(resp http.ResponseWriter, + req *http.Request, fn func(Request, *Response)) { + bodyBytes, _ := ioutil.ReadAll(req.Body) + + var admissionReq Request + _ = json.Unmarshal(bodyBytes, &admissionReq) + var admissionResp Response + + fn(admissionReq, &admissionResp) + + respBytes, _ := json.Marshal(admissionResp) + resp.WriteHeader(http.StatusOK) + _, _ = resp.Write(respBytes) +} + +type Tag struct { + Key string `json:"key"` + Value string `json:"value"` +} + +func (w *DummyMutatingWebhookServer) Validating(resp http.ResponseWriter, req *http.Request) { + w.ReadAndResponse(resp, req, w.validating) +} + +func (w *DummyMutatingWebhookServer) validating(req Request, resp *Response) { + obj := req.Object.(map[string]interface{}) + + allow := true + + name, ok := obj["name"].(string) + if !ok { + allow = false + resp.Result = "no name found" + } + + if strings.Contains(name, "invalid") { + allow = false + resp.Result = fmt.Sprintf("name contains invalid: %s", name) + } + + if obj["tags"] != nil { + tags := obj["tags"].([]interface{}) + for _, tag := range tags { + tag := tag.(map[string]interface{}) + tagKey := tag["key"].(string) + if strings.Contains(tagKey, "invalid") { + allow = false + resp.Result = fmt.Sprintf("tag key contains invalid: %s", tagKey) + break + } + } + } + if !allow { + allow = false + resp.Allowed = &allow + return + } + resp.Allowed = &allow +} + +func (w *DummyMutatingWebhookServer) ValidatingURL() string { + return w.server.URL + "/validate" +} + +func (w *DummyMutatingWebhookServer) Stop() { + w.server.Close() +} diff --git a/pkg/admission/models/models.go b/pkg/admission/models/models.go new file mode 100644 index 000000000..0cb3e5267 --- /dev/null +++ b/pkg/admission/models/models.go @@ -0,0 +1,25 @@ +package models + +import "strings" + +type Kind string + +func (k Kind) String() string { + return strings.ToLower(string(k)) +} + +func (k Kind) Eq(other Kind) bool { + return strings.EqualFold(string(k), string(other)) +} + +type Operation string + +func (o Operation) Eq(other Operation) bool { + return strings.EqualFold(string(o), string(other)) +} + +const ( + MatchAll string = "*" + + KindValidating Kind = "validating" +) diff --git a/pkg/admission/webhook.go b/pkg/admission/webhook.go new file mode 100644 index 000000000..c56b2fe0a --- /dev/null +++ b/pkg/admission/webhook.go @@ -0,0 +1,140 @@ +package admission + +import ( + "context" + "encoding/json" + "time" + + jsonpatch "gopkg.in/evanphx/json-patch.v5" + "k8s.io/apimachinery/pkg/util/runtime" + + herrors "github.com/horizoncd/horizon/core/errors" + "github.com/horizoncd/horizon/pkg/admission/models" + perror "github.com/horizoncd/horizon/pkg/errors" + "github.com/horizoncd/horizon/pkg/util/log" +) + +const DefaultTimeout = 5 * time.Second + +var ( + validatingWebhooks []Webhook +) + +func Register(kind models.Kind, webhook Webhook) { + switch kind { + case models.KindValidating: + validatingWebhooks = append(validatingWebhooks, webhook) + } +} + +func Clear() { + validatingWebhooks = nil +} + +type validateResult struct { + 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"` +} + +type Response struct { + Allowed *bool `json:"allowed,omitempty"` + Result string `json:"result,omitempty"` + Patch []byte `json:"patch,omitempty"` + PatchType string `json:"patchType,omitempty"` +} + +type Webhook interface { + Handle(context.Context, *Request) (*Response, error) + IgnoreError() bool + Interest(*Request) bool +} + +func Validating(ctx context.Context, request *Request) error { + ctx, cancelFunc := context.WithCancel(ctx) + defer cancelFunc() + finishedCount := 0 + resCh := make(chan validateResult) + for _, webhook := range validatingWebhooks { + go func(webhook Webhook) { + defer runtime.HandleCrash() + if !webhook.Interest(request) { + resCh <- validateResult{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} + return + } + resCh <- validateResult{err, nil} + return + } + if response.Allowed != nil { + resCh <- validateResult{nil, response} + return + } + }(webhook) + } + + for res := range resCh { + finishedCount++ + if res.err != nil { + 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) + return perror.Wrapf(herrors.ErrForbidden, "request denied by webhook: %s", res.resp.Result) + } + if finishedCount >= len(validatingWebhooks) { + close(resCh) + break + } + } + + return nil +} + +func jsonPatch(obj interface{}, patchJSON []byte) (interface{}, error) { + objJSON, err := json.Marshal(obj) + if err != nil { + return nil, err + } + patch, err := jsonpatch.DecodePatch(patchJSON) + if err != nil { + return nil, err + } + + objPatched, err := patch.Apply(objJSON) + if err != nil { + return nil, err + } + err = json.Unmarshal(objPatched, &obj) + if err != nil { + return nil, err + } + return obj, nil +} + +func loggingError(ctx context.Context, err error, webhook Webhook) error { + if err != nil { + if webhook.IgnoreError() { + log.Errorf(ctx, "failed to admit request: %v", err) + return nil + } + return err + } + return err +} diff --git a/pkg/config/admission/config.go b/pkg/config/admission/config.go new file mode 100644 index 000000000..2bb1832bc --- /dev/null +++ b/pkg/config/admission/config.go @@ -0,0 +1,43 @@ +package admission + +import ( + "strings" + + "github.com/horizoncd/horizon/pkg/admission/models" +) + +type FailurePolicy string + +func (f FailurePolicy) Eq(other FailurePolicy) bool { + return strings.EqualFold(string(f), string(other)) +} + +const ( + FailurePolicyIgnore FailurePolicy = "ignore" + FailurePolicyFail FailurePolicy = "fail" +) + +type ClientConfig struct { + URL string `yaml:"url"` + CABundle string `yaml:"caBundle"` + Insecure bool `yaml:"insecure"` +} + +type Rule struct { + Resources []string `yaml:"resources"` + Operations []models.Operation `yaml:"operations"` + Versions []string `yaml:"versions"` +} + +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"` +} + +type Admission struct { + Webhooks []Webhook `yaml:"webhooks"` +} From f23593bdf14cfaae0242d899570d0bbb2e5f14f2 Mon Sep 17 00:00:00 2001 From: "xu.zhu" Date: Fri, 15 Dec 2023 17:47:24 +0800 Subject: [PATCH 2/4] fix: bugs Signed-off-by: xu.zhu --- core/middleware/admission/admission.go | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/core/middleware/admission/admission.go b/core/middleware/admission/admission.go index a6b26a2fa..5d951568a 100644 --- a/core/middleware/admission/admission.go +++ b/core/middleware/admission/admission.go @@ -1,7 +1,10 @@ package admission import ( + "bytes" + "encoding/json" "fmt" + "io/ioutil" "github.com/gin-gonic/gin" "github.com/horizoncd/horizon/core/common" @@ -30,11 +33,25 @@ func Middleware(skippers ...middleware.Skipper) gin.HandlerFunc { return } var object interface{} - if err := c.ShouldBind(object); err != nil { + // read request body and avoid side-effects on c.Request.Body + bodyBytes, err := ioutil.ReadAll(c.Request.Body) + if err != nil { response.AbortWithRPCError(c, rpcerror.ParamError.WithErrMsg(fmt.Sprintf("request body is invalid, err: %v", err))) return } + c.Request.Body = ioutil.NopCloser(bytes.NewBuffer(bodyBytes)) + if err := json.Unmarshal(bodyBytes, &object); err != nil { + response.AbortWithRPCError(c, + rpcerror.ParamError.WithErrMsg(fmt.Sprintf("unmarshal request body failed, err: %v", err))) + return + } + // fill in the request url query into admission request options + queries := c.Request.URL.Query() + options := make(map[string]interface{}, len(queries)) + for k, v := range queries { + options[k] = v + } admissionRequest := &admissionwebhook.Request{ Operation: admissionmodels.Operation(attr.GetVerb()), Resource: attr.GetResource(), @@ -42,7 +59,7 @@ func Middleware(skippers ...middleware.Skipper) gin.HandlerFunc { SubResource: attr.GetSubResource(), Version: attr.GetAPIVersion(), Object: object, - OldObject: nil, + Options: options, } if err := admissionwebhook.Validating(c, admissionRequest); err != nil { response.AbortWithRPCError(c, From b2ec543dd3028f224416e9e440cb78d57c2b8944 Mon Sep 17 00:00:00 2001 From: "xu.zhu" Date: Mon, 18 Dec 2023 14:23:44 +0800 Subject: [PATCH 3/4] fix: lint and ut Signed-off-by: xu.zhu --- core/controller/application/admission.go | 1 - core/controller/cluster/admission.go | 73 ------------ core/middleware/admission/admission.go | 17 ++- pkg/admission/http.go | 91 +++++++++------ pkg/admission/models/models.go | 5 +- pkg/admission/webhook.go | 46 ++------ pkg/admission/webhook_test.go | 139 +++++++++++++++++++++++ 7 files changed, 219 insertions(+), 153 deletions(-) delete mode 100644 core/controller/application/admission.go delete mode 100644 core/controller/cluster/admission.go create mode 100644 pkg/admission/webhook_test.go diff --git a/core/controller/application/admission.go b/core/controller/application/admission.go deleted file mode 100644 index b584a8a4b..000000000 --- a/core/controller/application/admission.go +++ /dev/null @@ -1 +0,0 @@ -package application diff --git a/core/controller/cluster/admission.go b/core/controller/cluster/admission.go deleted file mode 100644 index 506e53b81..000000000 --- a/core/controller/cluster/admission.go +++ /dev/null @@ -1,73 +0,0 @@ -package cluster - -import ( - "github.com/horizoncd/horizon/core/common" - appmodels "github.com/horizoncd/horizon/pkg/application/models" - "github.com/horizoncd/horizon/pkg/cluster/code" - "github.com/horizoncd/horizon/pkg/cluster/models" - tagmodels "github.com/horizoncd/horizon/pkg/tag/models" -) - -type Cluster struct { - *models.Cluster `json:",inline"` - *TemplateInput `json:"templateInput,omitempty"` - Tags tagmodels.TagsBasic `json:"tags,omitempty"` -} - -type Clusterv2 struct { - *models.Cluster `json:",inline"` - *TemplateInput `json:"templateInput,omitempty"` - TemplateConfig map[string]interface{} `json:"templateConfig,omitempty"` - TemplateInfo *code.TemplateInfo `json:"-"` - MergePatch bool `json:"mergePatch"` - BuildConfig map[string]interface{} `json:"buildConfig"` - Tags tagmodels.TagsBasic `json:"tags,omitempty"` -} - -func (c *Clusterv2) toClusterModel(application *appmodels.Application) *models.Cluster { - cluster := &models.Cluster{ - ApplicationID: c.ApplicationID, - Name: c.Name, - EnvironmentName: c.EnvironmentName, - RegionName: c.RegionName, - Description: c.Description, - ExpireSeconds: c.ExpireSeconds, - Template: c.Template, - TemplateRelease: c.TemplateRelease, - Status: common.ClusterStatusCreating, - } - if cluster.Template == application.Template { - cluster.GitURL = func() string { - if c.GitURL == "" && application.GitURL != "" { - return application.GitURL - } - // if URL is empty string, this means this cluster not depends on build from git - return c.GitURL - }() - cluster.GitSubfolder = func() string { - if c.GitSubfolder == "" { - return application.GitSubfolder - } - return c.GitSubfolder - }() - cluster.GitRef = func() string { - if c.GitRef == "" { - return application.GitRef - } - return c.GitRef - }() - cluster.GitRefType = func() string { - if c.GitRefType == "" { - return application.GitRefType - } - return c.GitRefType - }() - cluster.Image = func() string { - if c.Image == "" { - return application.Image - } - return c.Image - }() - } - return cluster -} diff --git a/core/middleware/admission/admission.go b/core/middleware/admission/admission.go index 5d951568a..64dbf35c6 100644 --- a/core/middleware/admission/admission.go +++ b/core/middleware/admission/admission.go @@ -7,6 +7,7 @@ import ( "io/ioutil" "github.com/gin-gonic/gin" + "github.com/gin-gonic/gin/binding" "github.com/horizoncd/horizon/core/common" "github.com/horizoncd/horizon/core/middleware" admissionwebhook "github.com/horizoncd/horizon/pkg/admission" @@ -14,6 +15,7 @@ import ( "github.com/horizoncd/horizon/pkg/auth" "github.com/horizoncd/horizon/pkg/server/response" "github.com/horizoncd/horizon/pkg/server/rpcerror" + "github.com/horizoncd/horizon/pkg/util/log" ) // Middleware to validate and mutate admission request @@ -41,10 +43,17 @@ func Middleware(skippers ...middleware.Skipper) gin.HandlerFunc { return } c.Request.Body = ioutil.NopCloser(bytes.NewBuffer(bodyBytes)) - if err := json.Unmarshal(bodyBytes, &object); err != nil { - response.AbortWithRPCError(c, - rpcerror.ParamError.WithErrMsg(fmt.Sprintf("unmarshal request body failed, err: %v", err))) - return + if len(bodyBytes) > 0 { + contentType := c.ContentType() + if contentType == binding.MIMEJSON { + 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) + } } // fill in the request url query into admission request options queries := c.Request.URL.Query() diff --git a/pkg/admission/http.go b/pkg/admission/http.go index 6de02892c..fbec07d0e 100644 --- a/pkg/admission/http.go +++ b/pkg/admission/http.go @@ -17,6 +17,7 @@ import ( "github.com/horizoncd/horizon/pkg/admission/models" config "github.com/horizoncd/horizon/pkg/config/admission" perror "github.com/horizoncd/horizon/pkg/errors" + "github.com/horizoncd/horizon/pkg/util/common" ) type HTTPAdmissionClient struct { @@ -24,6 +25,7 @@ type HTTPAdmissionClient struct { http.Client } +// NewHTTPAdmissionClient creates a new HTTPAdmissionClient func NewHTTPAdmissionClient(config config.ClientConfig, timeout time.Duration) *HTTPAdmissionClient { var transport = &http.Transport{} if config.CABundle != "" { @@ -52,6 +54,7 @@ func NewHTTPAdmissionClient(config config.ClientConfig, timeout time.Duration) * } } +// Get sends the admission request to the webhook server and returns the response func (c *HTTPAdmissionClient) Get(ctx context.Context, admitData *Request) (*Response, error) { body, err := json.Marshal(admitData) if err != nil { @@ -87,6 +90,7 @@ type ResourceMatcher struct { versions map[string]struct{} } +// NewResourceMatcher creates a new ResourceMatcher func NewResourceMatcher(rule config.Rule) *ResourceMatcher { matcher := &ResourceMatcher{ resources: make(map[string]struct{}), @@ -117,6 +121,7 @@ func NewResourceMatcher(rule config.Rule) *ResourceMatcher { return matcher } +// Match returns true if the request matches the matcher func (m *ResourceMatcher) Match(req *Request) bool { if m.resources != nil { resource := req.Resource @@ -142,6 +147,7 @@ func (m *ResourceMatcher) Match(req *Request) bool { type ResourceMatchers []*ResourceMatcher +// NewResourceMatchers creates a new ResourceMatchers func NewResourceMatchers(rules []config.Rule) ResourceMatchers { matchers := make(ResourceMatchers, len(rules)) for i, rule := range rules { @@ -150,6 +156,7 @@ func NewResourceMatchers(rules []config.Rule) ResourceMatchers { return matchers } +// Match returns true if any matcher matches the request func (m ResourceMatchers) Match(req *Request) bool { for _, matcher := range m { if matcher.Match(req) { @@ -165,6 +172,7 @@ type HTTPAdmissionWebhook struct { matchers ResourceMatchers } +// NewHTTPWebhooks registers the webhooks func NewHTTPWebhooks(config config.Admission) { for _, webhook := range config.Webhooks { switch webhook.Kind { @@ -184,6 +192,7 @@ func NewHTTPWebhook(config config.Webhook) Webhook { } } +// Handle handles the admission request and returns the response func (m *HTTPAdmissionWebhook) Handle(ctx context.Context, req *Request) (*Response, error) { resp, err := m.httpclient.Get(ctx, req) if err != nil { @@ -192,20 +201,23 @@ func (m *HTTPAdmissionWebhook) Handle(ctx context.Context, req *Request) (*Respo return resp, nil } +// IgnoreError returns true if the webhook is allowed to ignore the error func (m *HTTPAdmissionWebhook) IgnoreError() bool { return m.config.FailurePolicy.Eq(config.FailurePolicyIgnore) } +// Interest returns true if the request matches the webhook func (m *HTTPAdmissionWebhook) Interest(req *Request) bool { return m.matchers.Match(req) } -type DummyMutatingWebhookServer struct { +type DummyValidatingWebhookServer struct { server *httptest.Server } -func NewDummyWebhookServer() *DummyMutatingWebhookServer { - webhook := &DummyMutatingWebhookServer{} +// NewDummyWebhookServer creates a dummy validating webhook server for testing +func NewDummyWebhookServer() *DummyValidatingWebhookServer { + webhook := &DummyValidatingWebhookServer{} mux := http.NewServeMux() mux.HandleFunc("/validate", webhook.Validating) @@ -215,7 +227,7 @@ func NewDummyWebhookServer() *DummyMutatingWebhookServer { return webhook } -func (*DummyMutatingWebhookServer) ReadAndResponse(resp http.ResponseWriter, +func (*DummyValidatingWebhookServer) ReadAndResponse(resp http.ResponseWriter, req *http.Request, fn func(Request, *Response)) { bodyBytes, _ := ioutil.ReadAll(req.Body) @@ -230,55 +242,60 @@ func (*DummyMutatingWebhookServer) ReadAndResponse(resp http.ResponseWriter, _, _ = resp.Write(respBytes) } -type Tag struct { - Key string `json:"key"` - Value string `json:"value"` -} - -func (w *DummyMutatingWebhookServer) Validating(resp http.ResponseWriter, req *http.Request) { +func (w *DummyValidatingWebhookServer) Validating(resp http.ResponseWriter, req *http.Request) { w.ReadAndResponse(resp, req, w.validating) } -func (w *DummyMutatingWebhookServer) validating(req Request, resp *Response) { +func (w *DummyValidatingWebhookServer) validating(req Request, resp *Response) { obj := req.Object.(map[string]interface{}) - allow := true - - name, ok := obj["name"].(string) - if !ok { - allow = false - resp.Result = "no name found" + if req.Operation.Eq(models.OperationCreate) { + // check name + name, ok := obj["name"].(string) + if !ok { + resp.Allowed = common.BoolPtr(false) + resp.Result = "no name found" + return + } + if strings.Contains(name, "invalid") { + resp.Allowed = common.BoolPtr(false) + resp.Result = fmt.Sprintf("name contains invalid: %s", name) + return + } } - if strings.Contains(name, "invalid") { - allow = false - resp.Result = fmt.Sprintf("name contains invalid: %s", name) + // check tags + tagsMap, ok := obj["tags"].([]interface{}) + if !ok { + // skip tag validation if no tags found + resp.Allowed = common.BoolPtr(true) + return } - - if obj["tags"] != nil { - tags := obj["tags"].([]interface{}) - for _, tag := range tags { - tag := tag.(map[string]interface{}) - tagKey := tag["key"].(string) - if strings.Contains(tagKey, "invalid") { - allow = false - resp.Result = fmt.Sprintf("tag key contains invalid: %s", tagKey) - break - } + targetKey := "scope" + exist := false + for _, tag := range tagsMap { + t, ok := tag.(map[string]interface{}) + if !ok { + continue + } + if t["key"] == targetKey { + exist = true + break } } - if !allow { - allow = false - resp.Allowed = &allow + if !exist { + resp.Allowed = common.BoolPtr(false) + resp.Result = fmt.Sprintf("no tag with key: %s", targetKey) return } - resp.Allowed = &allow + + resp.Allowed = common.BoolPtr(true) } -func (w *DummyMutatingWebhookServer) ValidatingURL() string { +func (w *DummyValidatingWebhookServer) ValidatingURL() string { return w.server.URL + "/validate" } -func (w *DummyMutatingWebhookServer) Stop() { +func (w *DummyValidatingWebhookServer) Stop() { w.server.Close() } diff --git a/pkg/admission/models/models.go b/pkg/admission/models/models.go index 0cb3e5267..5498d4997 100644 --- a/pkg/admission/models/models.go +++ b/pkg/admission/models/models.go @@ -19,7 +19,10 @@ func (o Operation) Eq(other Operation) bool { } const ( + KindValidating Kind = "validating" + MatchAll string = "*" - KindValidating Kind = "validating" + OperationCreate Operation = "create" + OperationUpdate Operation = "update" ) diff --git a/pkg/admission/webhook.go b/pkg/admission/webhook.go index c56b2fe0a..21fa4c025 100644 --- a/pkg/admission/webhook.go +++ b/pkg/admission/webhook.go @@ -2,10 +2,8 @@ package admission import ( "context" - "encoding/json" "time" - jsonpatch "gopkg.in/evanphx/json-patch.v5" "k8s.io/apimachinery/pkg/util/runtime" herrors "github.com/horizoncd/horizon/core/errors" @@ -48,7 +46,7 @@ type Request struct { } type Response struct { - Allowed *bool `json:"allowed,omitempty"` + Allowed *bool `json:"allowed"` Result string `json:"result,omitempty"` Patch []byte `json:"patch,omitempty"` PatchType string `json:"patchType,omitempty"` @@ -82,10 +80,16 @@ func Validating(ctx context.Context, request *Request) error { resCh <- validateResult{err, nil} return } - if response.Allowed != nil { - resCh <- validateResult{nil, response} + 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} + return + } + resCh <- validateResult{perror.New("response is nil or allowed is nil"), nil} return } + resCh <- validateResult{nil, response} }(webhook) } @@ -106,35 +110,3 @@ func Validating(ctx context.Context, request *Request) error { return nil } - -func jsonPatch(obj interface{}, patchJSON []byte) (interface{}, error) { - objJSON, err := json.Marshal(obj) - if err != nil { - return nil, err - } - patch, err := jsonpatch.DecodePatch(patchJSON) - if err != nil { - return nil, err - } - - objPatched, err := patch.Apply(objJSON) - if err != nil { - return nil, err - } - err = json.Unmarshal(objPatched, &obj) - if err != nil { - return nil, err - } - return obj, nil -} - -func loggingError(ctx context.Context, err error, webhook Webhook) error { - if err != nil { - if webhook.IgnoreError() { - log.Errorf(ctx, "failed to admit request: %v", err) - return nil - } - return err - } - return err -} diff --git a/pkg/admission/webhook_test.go b/pkg/admission/webhook_test.go new file mode 100644 index 000000000..5022d909a --- /dev/null +++ b/pkg/admission/webhook_test.go @@ -0,0 +1,139 @@ +package admission + +import ( + "context" + "testing" + + clusterctrl "github.com/horizoncd/horizon/core/controller/cluster" + "github.com/horizoncd/horizon/pkg/admission/models" + codemodels "github.com/horizoncd/horizon/pkg/cluster/code" + admissionconfig "github.com/horizoncd/horizon/pkg/config/admission" + tagmodels "github.com/horizoncd/horizon/pkg/tag/models" + "github.com/stretchr/testify/assert" +) + +func TestWebhook(t *testing.T) { + ctx := context.Background() + + server := NewDummyWebhookServer() + defer server.Stop() + validatingURL := server.ValidatingURL() + + config := admissionconfig.Admission{ + Webhooks: []admissionconfig.Webhook{ + { + Name: "test1", + Kind: models.KindValidating, + FailurePolicy: admissionconfig.FailurePolicyFail, + TimeoutSeconds: 5, + Rules: []admissionconfig.Rule{ + { + Resources: []string{ + "applications/clusters", + }, + Operations: []models.Operation{ + models.OperationCreate, + }, + Versions: []string{"v2"}, + }, + }, + ClientConfig: admissionconfig.ClientConfig{ + URL: validatingURL, + }, + }, + { + Name: "test2", + Kind: models.KindValidating, + FailurePolicy: admissionconfig.FailurePolicyIgnore, + TimeoutSeconds: 5, + Rules: []admissionconfig.Rule{ + { + Resources: []string{ + "clusters", + }, + Operations: []models.Operation{ + models.OperationUpdate, + }, + Versions: []string{"v2"}, + }, + }, + ClientConfig: admissionconfig.ClientConfig{ + URL: validatingURL, + }, + }, + }, + } + NewHTTPWebhooks(config) + + createBody := clusterctrl.CreateClusterRequestV2{ + Name: "cluster-1", + Description: "xxx", + Priority: "P0", + Git: &codemodels.Git{ + URL: "https://github.com/horizoncd/horizon.git", + Branch: "main", + }, + TemplateInfo: &codemodels.TemplateInfo{ + Name: "javaapp", + Release: "v1.0.0", + }, + } + + createRequest := &Request{ + Operation: models.OperationCreate, + Resource: "applications", + ResourceName: "1", + SubResource: "clusters", + Version: "v2", + Object: createBody, + OldObject: nil, + Options: map[string]interface{}{ + "scope": []string{"online/hz1"}, + }, + } + err := Validating(ctx, createRequest) + assert.NoError(t, err) + + createBody.Name = "cluster-invalid" + createRequest.Object = createBody + err = Validating(ctx, createRequest) + assert.Error(t, err) + t.Logf("error: %v", err) + + updateBody := clusterctrl.UpdateClusterRequestV2{ + Description: "yyy", + Tags: tagmodels.TagsBasic{ + { + Key: "k1", + Value: "v1", + }, + { + Key: "scope", + Value: "online/hz1", + }, + }, + } + updateRequest := &Request{ + Operation: models.OperationUpdate, + Resource: "clusters", + ResourceName: "1", + SubResource: "", + Version: "v2", + Object: updateBody, + OldObject: nil, + Options: nil, + } + err = Validating(ctx, updateRequest) + assert.NoError(t, err) + + updateBody.Tags = tagmodels.TagsBasic{ + { + Key: "k1", + Value: "v1", + }, + } + updateRequest.Object = updateBody + err = Validating(ctx, updateRequest) + assert.Error(t, err) + t.Logf("error: %v", err) +} From f8e5e61403935ab971c37b2a52721af44b9db0b4 Mon Sep 17 00:00:00 2001 From: "xu.zhu" Date: Fri, 5 Jan 2024 17:37:51 +0800 Subject: [PATCH 4/4] fix: review comments Signed-off-by: xu.zhu --- core/middleware/admission/admission.go | 22 +++++++----- pkg/admission/http.go | 9 +++-- pkg/admission/webhook.go | 47 ++++++++++++-------------- pkg/admission/webhook_test.go | 45 ++++++++++++------------ pkg/config/admission/config.go | 12 +++---- 5 files changed, 69 insertions(+), 66 deletions(-) diff --git a/core/middleware/admission/admission.go b/core/middleware/admission/admission.go index 64dbf35c6..63b3c41f6 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 fbec07d0e..c1dcef7b4 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 21fa4c025..b54a57b85 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 5022d909a..4a43f76bf 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 2bb1832bc..0971f9264 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 {