From 81b48bea23d3cca84e03df9c6dee1e5fa6aa7d6d Mon Sep 17 00:00:00 2001 From: Mengqi Yu Date: Thu, 24 Jan 2019 15:45:22 -0800 Subject: [PATCH 1/2] :warning: move webhook self installer to CT as generator --- Gopkg.lock | 8 +- example/main.go | 46 +- pkg/webhook/admission/builder/builder.go | 140 +-- pkg/webhook/admission/builder/doc.go | 57 +- pkg/webhook/admission/webhook.go | 24 - pkg/webhook/admission/webhook_test.go | 55 +- pkg/webhook/bootstrap.go | 356 ------- pkg/webhook/doc.go | 20 - pkg/webhook/internal/cert/doc.go | 36 - .../internal/cert/generator/certgenerator.go | 38 - .../cert/generator/certgenerator_test.go | 24 - pkg/webhook/internal/cert/generator/doc.go | 30 - .../cert/generator/fake/certgenerator.go | 53 - .../internal/cert/generator/selfsigned.go | 117 --- .../cert/generator/selfsigned_test.go | 134 --- .../internal/cert/generator/suite_test.go | 38 - pkg/webhook/internal/cert/generator/util.go | 61 -- pkg/webhook/internal/cert/provisioner.go | 133 --- pkg/webhook/internal/cert/provisioner_test.go | 229 ---- pkg/webhook/internal/cert/suite_test.go | 38 - .../cert/writer/atomic/atomic_writer.go | 453 -------- .../cert/writer/atomic/atomic_writer_test.go | 987 ------------------ .../internal/cert/writer/certwriter.go | 137 --- .../internal/cert/writer/certwriter_test.go | 360 ------- pkg/webhook/internal/cert/writer/doc.go | 64 -- pkg/webhook/internal/cert/writer/error.go | 43 - pkg/webhook/internal/cert/writer/fs.go | 216 ---- pkg/webhook/internal/cert/writer/fs_test.go | 249 ----- pkg/webhook/internal/cert/writer/secret.go | 184 ---- .../internal/cert/writer/secret_test.go | 241 ----- .../internal/cert/writer/suite_test.go | 38 - pkg/webhook/server.go | 211 ++-- pkg/webhook/server_test.go | 163 --- pkg/webhook/util.go | 115 -- .../client-go/util/testing/fake_handler.go | 139 --- .../k8s.io/client-go/util/testing/tmpdir.go | 44 - 36 files changed, 95 insertions(+), 5186 deletions(-) delete mode 100644 pkg/webhook/bootstrap.go delete mode 100644 pkg/webhook/internal/cert/doc.go delete mode 100644 pkg/webhook/internal/cert/generator/certgenerator.go delete mode 100644 pkg/webhook/internal/cert/generator/certgenerator_test.go delete mode 100644 pkg/webhook/internal/cert/generator/doc.go delete mode 100644 pkg/webhook/internal/cert/generator/fake/certgenerator.go delete mode 100644 pkg/webhook/internal/cert/generator/selfsigned.go delete mode 100644 pkg/webhook/internal/cert/generator/selfsigned_test.go delete mode 100644 pkg/webhook/internal/cert/generator/suite_test.go delete mode 100644 pkg/webhook/internal/cert/generator/util.go delete mode 100644 pkg/webhook/internal/cert/provisioner.go delete mode 100644 pkg/webhook/internal/cert/provisioner_test.go delete mode 100644 pkg/webhook/internal/cert/suite_test.go delete mode 100644 pkg/webhook/internal/cert/writer/atomic/atomic_writer.go delete mode 100644 pkg/webhook/internal/cert/writer/atomic/atomic_writer_test.go delete mode 100644 pkg/webhook/internal/cert/writer/certwriter.go delete mode 100644 pkg/webhook/internal/cert/writer/certwriter_test.go delete mode 100644 pkg/webhook/internal/cert/writer/doc.go delete mode 100644 pkg/webhook/internal/cert/writer/error.go delete mode 100644 pkg/webhook/internal/cert/writer/fs.go delete mode 100644 pkg/webhook/internal/cert/writer/fs_test.go delete mode 100644 pkg/webhook/internal/cert/writer/secret.go delete mode 100644 pkg/webhook/internal/cert/writer/secret_test.go delete mode 100644 pkg/webhook/internal/cert/writer/suite_test.go delete mode 100644 pkg/webhook/server_test.go delete mode 100644 pkg/webhook/util.go delete mode 100644 vendor/k8s.io/client-go/util/testing/fake_handler.go delete mode 100644 vendor/k8s.io/client-go/util/testing/tmpdir.go diff --git a/Gopkg.lock b/Gopkg.lock index b39b7c3874..897580722e 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -728,7 +728,7 @@ version = "kubernetes-1.13.1" [[projects]] - digest = "1:1a0e15167c6d864afac0ccf99a04cda3132afd2a8a7b202e408508197f2cfe14" + digest = "1:2b9c47d30a4bd0491b138fc8b9eb016e5ab7eb785b86919aa879b30cb2919707" name = "k8s.io/client-go" packages = [ "discovery", @@ -877,7 +877,6 @@ "util/integer", "util/jsonpath", "util/retry", - "util/testing", "util/workqueue", ] pruneopts = "UT" @@ -946,7 +945,6 @@ "go.uber.org/zap/buffer", "go.uber.org/zap/zapcore", "k8s.io/api/admission/v1beta1", - "k8s.io/api/admissionregistration/v1beta1", "k8s.io/api/apps/v1", "k8s.io/api/autoscaling/v1", "k8s.io/api/core/v1", @@ -964,7 +962,6 @@ "k8s.io/apimachinery/pkg/runtime/serializer", "k8s.io/apimachinery/pkg/selection", "k8s.io/apimachinery/pkg/types", - "k8s.io/apimachinery/pkg/util/intstr", "k8s.io/apimachinery/pkg/util/runtime", "k8s.io/apimachinery/pkg/util/sets", "k8s.io/apimachinery/pkg/util/uuid", @@ -988,12 +985,9 @@ "k8s.io/client-go/tools/metrics", "k8s.io/client-go/tools/record", "k8s.io/client-go/tools/reference", - "k8s.io/client-go/util/cert", - "k8s.io/client-go/util/testing", "k8s.io/client-go/util/workqueue", "k8s.io/kube-openapi/pkg/common", "sigs.k8s.io/testing_frameworks/integration", - "sigs.k8s.io/testing_frameworks/integration/addr", ] solver-name = "gps-cdcl" solver-version = 1 diff --git a/example/main.go b/example/main.go index 89b488ab35..81bcdc1cce 100644 --- a/example/main.go +++ b/example/main.go @@ -20,17 +20,15 @@ import ( "flag" "os" - admissionregistrationv1beta1 "k8s.io/api/admissionregistration/v1beta1" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" - apitypes "k8s.io/apimachinery/pkg/types" _ "k8s.io/client-go/plugin/pkg/client/auth/gcp" "sigs.k8s.io/controller-runtime/pkg/client/config" "sigs.k8s.io/controller-runtime/pkg/controller" "sigs.k8s.io/controller-runtime/pkg/handler" - "sigs.k8s.io/controller-runtime/pkg/manager" logf "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/log/zap" + "sigs.k8s.io/controller-runtime/pkg/manager" "sigs.k8s.io/controller-runtime/pkg/manager/signals" "sigs.k8s.io/controller-runtime/pkg/source" "sigs.k8s.io/controller-runtime/pkg/webhook" @@ -81,52 +79,22 @@ func main() { // Setup webhooks entryLog.Info("setting up webhooks") - mutatingWebhook, err := builder.NewWebhookBuilder(). - Name("mutating.k8s.io"). + mutatingWebhook := builder.NewWebhookBuilder(). + Path("/mutate-pods"). Mutating(). - Operations(admissionregistrationv1beta1.Create, admissionregistrationv1beta1.Update). - WithManager(mgr). - ForType(&corev1.Pod{}). Handlers(&podAnnotator{}). Build() - if err != nil { - entryLog.Error(err, "unable to setup mutating webhook") - os.Exit(1) - } - validatingWebhook, err := builder.NewWebhookBuilder(). - Name("validating.k8s.io"). + validatingWebhook := builder.NewWebhookBuilder(). + Path("/validate-pods"). Validating(). - Operations(admissionregistrationv1beta1.Create, admissionregistrationv1beta1.Update). - WithManager(mgr). - ForType(&corev1.Pod{}). Handlers(&podValidator{}). Build() - if err != nil { - entryLog.Error(err, "unable to setup validating webhook") - os.Exit(1) - } entryLog.Info("setting up webhook server") as, err := webhook.NewServer("foo-admission-server", mgr, webhook.ServerOptions{ - Port: 9876, - CertDir: "/tmp/cert", - DisableWebhookConfigInstaller: &disableWebhookConfigInstaller, - BootstrapOptions: &webhook.BootstrapOptions{ - Secret: &apitypes.NamespacedName{ - Namespace: "default", - Name: "foo-admission-server-secret", - }, - - Service: &webhook.Service{ - Namespace: "default", - Name: "foo-admission-server-service", - // Selectors should select the pods that runs this webhook server. - Selectors: map[string]string{ - "app": "foo-admission-server", - }, - }, - }, + Port: 9876, + CertDir: "/tmp/cert", }) if err != nil { entryLog.Error(err, "unable to create a new webhook server") diff --git a/pkg/webhook/admission/builder/builder.go b/pkg/webhook/admission/builder/builder.go index 26f91ceba3..4f7e12e20a 100644 --- a/pkg/webhook/admission/builder/builder.go +++ b/pkg/webhook/admission/builder/builder.go @@ -17,14 +17,6 @@ limitations under the License. package builder import ( - "errors" - "fmt" - - admissionregistrationv1beta1 "k8s.io/api/admissionregistration/v1beta1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime" - "sigs.k8s.io/controller-runtime/pkg/client/apiutil" - "sigs.k8s.io/controller-runtime/pkg/manager" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" "sigs.k8s.io/controller-runtime/pkg/webhook/types" ) @@ -46,26 +38,6 @@ type WebhookBuilder struct { // t specifies the type of the webhook. // Currently, Mutating and Validating are supported. t *types.WebhookType - - // operations define the operations this webhook cares. - // only one of operations and Rules can be set. - operations []admissionregistrationv1beta1.OperationType - // apiType represents the resource that this webhook cares. - // Only one of apiType and Rules can be set. - apiType runtime.Object - // rules contain a list of admissionregistrationv1beta1.RuleWithOperations - // It overrides operations and apiType. - rules []admissionregistrationv1beta1.RuleWithOperations - - // failurePolicy maps to the FailurePolicy in the admissionregistrationv1beta1.Webhook - failurePolicy *admissionregistrationv1beta1.FailurePolicyType - - // namespaceSelector maps to the NamespaceSelector in the admissionregistrationv1beta1.Webhook - namespaceSelector *metav1.LabelSelector - - // manager is the manager for the webhook. - // It is used for provisioning various dependencies for the webhook. e.g. RESTMapper. - manager manager.Manager } // NewWebhookBuilder creates an empty WebhookBuilder. @@ -98,7 +70,7 @@ func (b *WebhookBuilder) Validating() *WebhookBuilder { // Path sets the path for the webhook. // Path needs to be unique among different webhooks. -// This is optional. If not set, it will be built from the type and resource name. +// This is required. If not set, it will be built from the type and resource name. // For example, a webhook that mutates pods has a default path of "/mutate-pods" // If the defaulting logic can't find a unique path for it, user need to set it manually. func (b *WebhookBuilder) Path(path string) *WebhookBuilder { @@ -106,118 +78,24 @@ func (b *WebhookBuilder) Path(path string) *WebhookBuilder { return b } -// Operations sets the operations that this webhook cares. -// It will be overridden by Rules if Rules are not empty. -// This is optional -func (b *WebhookBuilder) Operations(ops ...admissionregistrationv1beta1.OperationType) *WebhookBuilder { - b.operations = ops - return b -} - -// ForType sets the type of resources that the webhook will operate. -// It will be overridden by Rules if Rules are not empty. -func (b *WebhookBuilder) ForType(obj runtime.Object) *WebhookBuilder { - b.apiType = obj - return b -} - -// Rules sets the RuleWithOperations for the webhook. -// It overrides ForType and Operations. -// This is optional and for advanced user. -func (b *WebhookBuilder) Rules(rules ...admissionregistrationv1beta1.RuleWithOperations) *WebhookBuilder { - b.rules = rules - return b -} - -// FailurePolicy sets the FailurePolicy of the webhook. -// If not set, it will be defaulted by the server. -// This is optional -func (b *WebhookBuilder) FailurePolicy(policy admissionregistrationv1beta1.FailurePolicyType) *WebhookBuilder { - b.failurePolicy = &policy - return b -} - -// NamespaceSelector sets the NamespaceSelector for the webhook. -// This is optional -func (b *WebhookBuilder) NamespaceSelector(namespaceSelector *metav1.LabelSelector) *WebhookBuilder { - b.namespaceSelector = namespaceSelector - return b -} - -// WithManager set the manager for the webhook for provisioning various dependencies. e.g. client etc. -func (b *WebhookBuilder) WithManager(mgr manager.Manager) *WebhookBuilder { - b.manager = mgr - return b -} - // Handlers sets the handlers of the webhook. func (b *WebhookBuilder) Handlers(handlers ...admission.Handler) *WebhookBuilder { b.handlers = handlers return b } -func (b *WebhookBuilder) validate() error { - if b.t == nil { - return errors.New("webhook type cannot be nil") - } - if b.rules == nil && b.apiType == nil { - return fmt.Errorf("ForType should be set") - } - if b.rules != nil && b.apiType != nil { - return fmt.Errorf("at most one of ForType and Rules can be set") - } - return nil -} - // Build creates the Webhook based on the options provided. -func (b *WebhookBuilder) Build() (*admission.Webhook, error) { - err := b.validate() - if err != nil { - return nil, err +func (b *WebhookBuilder) Build() *admission.Webhook { + if b.t == nil { + b.Mutating() } w := &admission.Webhook{ - Name: b.name, - Type: *b.t, - Path: b.path, - FailurePolicy: b.failurePolicy, - NamespaceSelector: b.namespaceSelector, - Handlers: b.handlers, - } - - if b.rules != nil { - w.Rules = b.rules - } else { - if b.manager == nil { - return nil, errors.New("manager should be set using WithManager") - } - gvk, err := apiutil.GVKForObject(b.apiType, b.manager.GetScheme()) - if err != nil { - return nil, err - } - mapper := b.manager.GetRESTMapper() - mapping, err := mapper.RESTMapping(gvk.GroupKind(), gvk.Version) - if err != nil { - return nil, err - } - - if b.operations == nil { - b.operations = []admissionregistrationv1beta1.OperationType{ - admissionregistrationv1beta1.Create, - admissionregistrationv1beta1.Update, - } - } - w.Rules = []admissionregistrationv1beta1.RuleWithOperations{ - { - Operations: b.operations, - Rule: admissionregistrationv1beta1.Rule{ - APIGroups: []string{gvk.Group}, - APIVersions: []string{gvk.Version}, - Resources: []string{mapping.Resource.Resource}, - }, - }, - } + Name: b.name, + Type: *b.t, + Path: b.path, + Handlers: b.handlers, } - return w, nil + return w } diff --git a/pkg/webhook/admission/builder/doc.go b/pkg/webhook/admission/builder/doc.go index 411016ee7d..9ef8d65fa0 100644 --- a/pkg/webhook/admission/builder/doc.go +++ b/pkg/webhook/admission/builder/doc.go @@ -19,27 +19,19 @@ Package builder provides methods to build admission webhooks. The following are 2 examples for building mutating webhook and validating webhook. - webhook1, err := NewWebhookBuilder(). + webhook1 := NewWebhookBuilder(). Mutating(). - Operations(admissionregistrationv1beta1.Create). + Path("/mutatepods"). ForType(&corev1.Pod{}). - WithManager(mgr). Handlers(mutatingHandler11, mutatingHandler12). Build() - if err != nil { - // handle error - } - webhook2, err := NewWebhookBuilder(). + webhook2 := NewWebhookBuilder(). Validating(). - Operations(admissionregistrationv1beta1.Create, admissionregistrationv1beta1.Update). + Path("/validatepods"). ForType(&appsv1.Deployment{}). - WithManager(mgr). Handlers(validatingHandler21). Build() - if err != nil { - // handle error - } Note: To build a webhook for a CRD, you need to ensure the manager uses the scheme that understands your CRD. This is necessary, because if the scheme doesn't understand your CRD types, the decoder won't be able to decode @@ -62,46 +54,5 @@ The following snippet shows how to register CRD types with manager's scheme. if err != nil { // handle error } - -There are more options for configuring a webhook. e.g. Name, Path, FailurePolicy, NamespaceSelector. -Here is another example: - - webhook3, err := NewWebhookBuilder(). - Name("foo.example.com"). - Path("/mutatepods"). - Mutating(). - Operations(admissionregistrationv1beta1.Create). - ForType(&corev1.Pod{}). - FailurePolicy(admissionregistrationv1beta1.Fail). - WithManager(mgr). - Handlers(mutatingHandler31, mutatingHandler32). - Build() - if err != nil { - // handle error - } - -For most users, we recommend to use Operations and ForType instead of Rules to construct a webhook, -since it is more intuitive and easier to pass the target operations to Operations method and -a empty target object to ForType method than passing a complex RuleWithOperations struct to Rules method. - -Rules may be useful for some more advanced use cases like subresources, wildcard resources etc. -Here is an example: - - webhook4, err := NewWebhookBuilder(). - Validating(). - Rules(admissionregistrationv1beta1.RuleWithOperations{ - Operations: []admissionregistrationv1beta1.OperationType{admissionregistrationv1beta1.Create}, - Rule: admissionregistrationv1beta1.Rule{ - APIGroups: []string{"apps", "batch"}, - APIVersions: []string{"v1"}, - Resources: []string{"*"}, - }, - }). - WithManager(mgr). - Handlers(validatingHandler41). - Build() - if err != nil { - // handle error - } */ package builder diff --git a/pkg/webhook/admission/webhook.go b/pkg/webhook/admission/webhook.go index 6a14dd6350..75983b6751 100644 --- a/pkg/webhook/admission/webhook.go +++ b/pkg/webhook/admission/webhook.go @@ -29,7 +29,6 @@ import ( "github.com/appscode/jsonpatch" admissionv1beta1 "k8s.io/api/admission/v1beta1" - admissionregistrationv1beta1 "k8s.io/api/admissionregistration/v1beta1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/runtime/inject" @@ -60,15 +59,6 @@ type Webhook struct { Type types.WebhookType // Path is the path this webhook will serve. Path string - // Rules maps to the Rules field in admissionregistrationv1beta1.Webhook - Rules []admissionregistrationv1beta1.RuleWithOperations - // FailurePolicy maps to the FailurePolicy field in admissionregistrationv1beta1.Webhook - // This optional. If not set, will be defaulted to Ignore (fail-open) by the server. - // More details: https://github.com/kubernetes/api/blob/f5c295feaba2cbc946f0bbb8b535fc5f6a0345ee/admissionregistration/v1beta1/types.go#L144-L147 - FailurePolicy *admissionregistrationv1beta1.FailurePolicyType - // NamespaceSelector maps to the NamespaceSelector field in admissionregistrationv1beta1.Webhook - // This optional. - NamespaceSelector *metav1.LabelSelector // Handlers contains a list of handlers. Each handler may only contains the business logic for its own feature. // For example, feature foo and bar can be in the same webhook if all the other configurations are the same. // The handler will be invoked sequentially as the order in the list. @@ -80,17 +70,6 @@ type Webhook struct { } func (w *Webhook) setDefaults() { - if len(w.Path) == 0 { - if len(w.Rules) == 0 || len(w.Rules[0].Resources) == 0 { - // can't do defaulting, skip it. - return - } - if w.Type == types.WebhookTypeMutating { - w.Path = "/mutate-" + w.Rules[0].Resources[0] - } else if w.Type == types.WebhookTypeValidating { - w.Path = "/validate-" + w.Rules[0].Resources[0] - } - } if len(w.Name) == 0 { reg := regexp.MustCompile("[^a-zA-Z0-9]+") processedPath := strings.ToLower(reg.ReplaceAllString(w.Path, "")) @@ -216,9 +195,6 @@ func (w *Webhook) Handler() http.Handler { // Validate validates if the webhook is valid. func (w *Webhook) Validate() error { w.once.Do(w.setDefaults) - if len(w.Rules) == 0 { - return errors.New("field Rules should not be empty") - } if len(w.Name) == 0 { return errors.New("field Name should not be empty") } diff --git a/pkg/webhook/admission/webhook_test.go b/pkg/webhook/admission/webhook_test.go index c23e51d8a9..1f6bddbcb3 100644 --- a/pkg/webhook/admission/webhook_test.go +++ b/pkg/webhook/admission/webhook_test.go @@ -30,7 +30,6 @@ import ( . "github.com/onsi/gomega" admissionv1beta1 "k8s.io/api/admission/v1beta1" - admissionregistrationv1beta1 "k8s.io/api/admissionregistration/v1beta1" atypes "sigs.k8s.io/controller-runtime/pkg/webhook/admission/types" "sigs.k8s.io/controller-runtime/pkg/webhook/types" ) @@ -228,59 +227,33 @@ var _ = Describe("admission webhook", func() { Context("valid mutating webhook", func() { wh := &Webhook{ Type: types.WebhookTypeMutating, + Path: "/mutate-deployments", Handlers: []Handler{&fakeHandler{}}, - Rules: []admissionregistrationv1beta1.RuleWithOperations{ - { - Operations: []admissionregistrationv1beta1.OperationType{}, - Rule: admissionregistrationv1beta1.Rule{ - APIGroups: []string{"apps"}, - APIVersions: []string{"v1"}, - Resources: []string{"deployments"}}, - }, - }, } It("should pass validation", func() { err := wh.Validate() Expect(err).NotTo(HaveOccurred()) Expect(wh.Name).To(Equal("mutatedeployments.example.com")) - Expect(wh.Path).To(Equal("/mutate-deployments")) }) }) Context("valid validating webhook", func() { wh := &Webhook{ Type: types.WebhookTypeValidating, + Path: "/validate-deployments", Handlers: []Handler{&fakeHandler{}}, - Rules: []admissionregistrationv1beta1.RuleWithOperations{ - { - Operations: []admissionregistrationv1beta1.OperationType{}, - Rule: admissionregistrationv1beta1.Rule{ - APIGroups: []string{"apps"}, - APIVersions: []string{"v1"}, - Resources: []string{"deployments"}}, - }, - }, } It("should pass validation", func() { err := wh.Validate() Expect(err).NotTo(HaveOccurred()) Expect(wh.Name).To(Equal("validatedeployments.example.com")) - Expect(wh.Path).To(Equal("/validate-deployments")) }) }) Context("missing webhook type", func() { wh := &Webhook{ + Path: "/mutate-deployments", Handlers: []Handler{&fakeHandler{}}, - Rules: []admissionregistrationv1beta1.RuleWithOperations{ - { - Operations: []admissionregistrationv1beta1.OperationType{}, - Rule: admissionregistrationv1beta1.Rule{ - APIGroups: []string{"apps"}, - APIVersions: []string{"v1"}, - Resources: []string{"deployments"}}, - }, - }, } It("should fail validation", func() { err := wh.Validate() @@ -288,31 +261,11 @@ var _ = Describe("admission webhook", func() { }) }) - Context("missing Rules", func() { - wh := &Webhook{ - Type: types.WebhookTypeValidating, - Handlers: []Handler{&fakeHandler{}}, - } - It("should fail validation", func() { - err := wh.Validate() - Expect(err).To(Equal(errors.New("field Rules should not be empty"))) - - }) - }) - Context("missing Handlers", func() { wh := &Webhook{ Type: types.WebhookTypeValidating, + Path: "/validate-deployments", Handlers: []Handler{}, - Rules: []admissionregistrationv1beta1.RuleWithOperations{ - { - Operations: []admissionregistrationv1beta1.OperationType{}, - Rule: admissionregistrationv1beta1.Rule{ - APIGroups: []string{"apps"}, - APIVersions: []string{"v1"}, - Resources: []string{"deployments"}}, - }, - }, } It("should fail validation", func() { err := wh.Validate() diff --git a/pkg/webhook/bootstrap.go b/pkg/webhook/bootstrap.go deleted file mode 100644 index c14f8dafff..0000000000 --- a/pkg/webhook/bootstrap.go +++ /dev/null @@ -1,356 +0,0 @@ -/* -Copyright 2018 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package webhook - -import ( - "errors" - "fmt" - "net" - "net/http" - "net/url" - "path" - "sort" - "strconv" - - "k8s.io/api/admissionregistration/v1beta1" - admissionregistration "k8s.io/api/admissionregistration/v1beta1" - corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/util/intstr" - "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/client/config" - "sigs.k8s.io/controller-runtime/pkg/webhook/admission" - "sigs.k8s.io/controller-runtime/pkg/webhook/internal/cert" - "sigs.k8s.io/controller-runtime/pkg/webhook/internal/cert/writer" - "sigs.k8s.io/controller-runtime/pkg/webhook/types" -) - -// setDefault does defaulting for the Server. -func (s *Server) setDefault() { - s.setServerDefault() - s.setBootstrappingDefault() -} - -// setServerDefault does defaulting for the ServerOptions. -func (s *Server) setServerDefault() { - if len(s.Name) == 0 { - s.Name = "default-k8s-webhook-server" - } - if s.registry == nil { - s.registry = map[string]Webhook{} - } - if s.sMux == nil { - s.sMux = http.DefaultServeMux - } - if s.Port <= 0 { - s.Port = 443 - } - if len(s.CertDir) == 0 { - s.CertDir = path.Join("k8s-webhook-server", "cert") - } - if s.DisableWebhookConfigInstaller == nil { - diwc := false - s.DisableWebhookConfigInstaller = &diwc - } - - if s.Client == nil { - cfg, err := config.GetConfig() - if err != nil { - s.err = err - return - } - s.Client, err = client.New(cfg, client.Options{}) - if err != nil { - s.err = err - return - } - } -} - -// setBootstrappingDefault does defaulting for the Server bootstrapping. -func (s *Server) setBootstrappingDefault() { - if s.BootstrapOptions == nil { - s.BootstrapOptions = &BootstrapOptions{} - } - if len(s.MutatingWebhookConfigName) == 0 { - s.MutatingWebhookConfigName = "mutating-webhook-configuration" - } - if len(s.ValidatingWebhookConfigName) == 0 { - s.ValidatingWebhookConfigName = "validating-webhook-configuration" - } - if s.Host == nil && s.Service == nil { - varString := "localhost" - s.Host = &varString - } - - var certWriter writer.CertWriter - var err error - if s.Secret != nil { - certWriter, err = writer.NewSecretCertWriter( - writer.SecretCertWriterOptions{ - Secret: s.Secret, - Client: s.Client, - }) - } else { - certWriter, err = writer.NewFSCertWriter( - writer.FSCertWriterOptions{ - Path: s.CertDir, - }) - } - if err != nil { - s.err = err - return - } - s.certProvisioner = &cert.Provisioner{ - CertWriter: certWriter, - } -} - -// InstallWebhookManifests creates the admissionWebhookConfiguration objects and service if any. -// It also provisions the certificate for the admission server. -func (s *Server) InstallWebhookManifests() error { - // do defaulting if necessary - s.once.Do(s.setDefault) - if s.err != nil { - return s.err - } - - var err error - s.webhookConfigurations, err = s.whConfigs() - if err != nil { - return err - } - svc := s.service() - objects := append(s.webhookConfigurations, svc) - - cc, err := s.getClientConfig() - if err != nil { - return err - } - // Provision the cert by creating new one or refreshing existing one. - _, err = s.certProvisioner.Provision(cert.Options{ - ClientConfig: cc, - Objects: s.webhookConfigurations, - }) - if err != nil { - return err - } - - return batchCreateOrReplace(s.Client, objects...) -} - -func (s *Server) getClientConfig() (*admissionregistration.WebhookClientConfig, error) { - if s.Host != nil && s.Service != nil { - return nil, errors.New("URL and Service can't be set at the same time") - } - cc := &admissionregistration.WebhookClientConfig{ - CABundle: []byte{}, - } - if s.Host != nil { - u := url.URL{ - Scheme: "https", - Host: net.JoinHostPort(*s.Host, strconv.Itoa(int(s.Port))), - } - urlString := u.String() - cc.URL = &urlString - } - if s.Service != nil { - cc.Service = &admissionregistration.ServiceReference{ - Name: s.Service.Name, - Namespace: s.Service.Namespace, - // Path will be set later - } - } - return cc, nil -} - -// getClientConfigWithPath constructs a WebhookClientConfig based on the server options. -// It will use path to the set the path in WebhookClientConfig. -func (s *Server) getClientConfigWithPath(path string) (*admissionregistration.WebhookClientConfig, error) { - cc, err := s.getClientConfig() - if err != nil { - return nil, err - } - return cc, setPath(cc, path) -} - -// setPath sets the path in the WebhookClientConfig. -func setPath(cc *admissionregistration.WebhookClientConfig, path string) error { - if cc.URL != nil { - u, err := url.Parse(*cc.URL) - if err != nil { - return err - } - u.Path = path - urlString := u.String() - cc.URL = &urlString - } - if cc.Service != nil { - cc.Service.Path = &path - } - return nil -} - -// whConfigs creates a mutatingWebhookConfiguration and(or) a validatingWebhookConfiguration based on registry. -// For the same type of webhook configuration, it generates a webhook entry per endpoint. -func (s *Server) whConfigs() ([]runtime.Object, error) { - objs := []runtime.Object{} - mutatingWH, err := s.mutatingWHConfigs() - if err != nil { - return nil, err - } - if mutatingWH != nil { - objs = append(objs, mutatingWH) - } - validatingWH, err := s.validatingWHConfigs() - if err != nil { - return nil, err - } - if validatingWH != nil { - objs = append(objs, validatingWH) - } - return objs, nil -} - -func (s *Server) mutatingWHConfigs() (runtime.Object, error) { - mutatingWebhooks := []v1beta1.Webhook{} - for path, webhook := range s.registry { - if webhook.GetType() != types.WebhookTypeMutating { - continue - } - - admissionWebhook := webhook.(*admission.Webhook) - wh, err := s.admissionWebhook(path, admissionWebhook) - if err != nil { - return nil, err - } - mutatingWebhooks = append(mutatingWebhooks, *wh) - } - - sort.Slice(mutatingWebhooks, func(i, j int) bool { - return mutatingWebhooks[i].Name < mutatingWebhooks[j].Name - }) - - if len(mutatingWebhooks) > 0 { - return &admissionregistration.MutatingWebhookConfiguration{ - TypeMeta: metav1.TypeMeta{ - APIVersion: fmt.Sprintf("%s/%s", admissionregistration.GroupName, "v1beta1"), - Kind: "MutatingWebhookConfiguration", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: s.MutatingWebhookConfigName, - }, - Webhooks: mutatingWebhooks, - }, nil - } - return nil, nil -} - -func (s *Server) validatingWHConfigs() (runtime.Object, error) { - validatingWebhooks := []v1beta1.Webhook{} - for path, webhook := range s.registry { - var admissionWebhook *admission.Webhook - if webhook.GetType() != types.WebhookTypeValidating { - continue - } - - admissionWebhook = webhook.(*admission.Webhook) - wh, err := s.admissionWebhook(path, admissionWebhook) - if err != nil { - return nil, err - } - validatingWebhooks = append(validatingWebhooks, *wh) - } - - sort.Slice(validatingWebhooks, func(i, j int) bool { - return validatingWebhooks[i].Name < validatingWebhooks[j].Name - }) - - if len(validatingWebhooks) > 0 { - return &admissionregistration.ValidatingWebhookConfiguration{ - TypeMeta: metav1.TypeMeta{ - APIVersion: fmt.Sprintf("%s/%s", admissionregistration.GroupName, "v1beta1"), - Kind: "ValidatingWebhookConfiguration", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: s.ValidatingWebhookConfigName, - }, - Webhooks: validatingWebhooks, - }, nil - } - return nil, nil -} - -func (s *Server) admissionWebhook(path string, wh *admission.Webhook) (*admissionregistration.Webhook, error) { - if wh.NamespaceSelector == nil && s.Service != nil && len(s.Service.Namespace) > 0 { - wh.NamespaceSelector = &metav1.LabelSelector{ - MatchExpressions: []metav1.LabelSelectorRequirement{ - { - Key: "control-plane", - Operator: metav1.LabelSelectorOpDoesNotExist, - }, - }, - } - } - - webhook := &admissionregistration.Webhook{ - Name: wh.GetName(), - Rules: wh.Rules, - FailurePolicy: wh.FailurePolicy, - NamespaceSelector: wh.NamespaceSelector, - ClientConfig: admissionregistration.WebhookClientConfig{ - // The reason why we assign an empty byte array to CABundle is that - // CABundle field will be updated by the Provisioner. - CABundle: []byte{}, - }, - } - cc, err := s.getClientConfigWithPath(path) - if err != nil { - return nil, err - } - webhook.ClientConfig = *cc - return webhook, nil -} - -// service creates a corev1.service object fronting the admission server. -func (s *Server) service() runtime.Object { - if s.Service == nil { - return nil - } - svc := &corev1.Service{ - TypeMeta: metav1.TypeMeta{ - APIVersion: "v1", - Kind: "Service", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: s.Service.Name, - Namespace: s.Service.Namespace, - }, - Spec: corev1.ServiceSpec{ - Selector: s.Service.Selectors, - Ports: []corev1.ServicePort{ - { - // When using service, kube-apiserver will send admission request to port 443. - Port: 443, - TargetPort: intstr.IntOrString{Type: intstr.Int, IntVal: s.Port}, - }, - }, - }, - } - return svc -} diff --git a/pkg/webhook/doc.go b/pkg/webhook/doc.go index 930df1e436..6aafff524c 100644 --- a/pkg/webhook/doc.go +++ b/pkg/webhook/doc.go @@ -26,9 +26,6 @@ Build webhooks Name("foo.k8s.io"). Mutating(). Path("/mutating-pods"). - Operations(admissionregistrationv1beta1.Create). - ForType(&corev1.Pod{}). - WithManager(mgr). Handlers(mutatingHandler1, mutatingHandler2). Build() if err != nil { @@ -39,9 +36,6 @@ Build webhooks Name("bar.k8s.io"). Validating(). Path("/validating-deployment"). - Operations(admissionregistrationv1beta1.Create, admissionregistrationv1beta1.Update). - ForType(&appsv1.Deployment{}). - WithManager(mgr). Handlers(validatingHandler1). Build() if err != nil { @@ -52,20 +46,6 @@ Create a webhook server. as, err := NewServer("baz-admission-server", mgr, ServerOptions{ CertDir: "/tmp/cert", - BootstrapOptions: &BootstrapOptions{ - Secret: &apitypes.NamespacedName{ - Namespace: "default", - Name: "foo-admission-server-secret", - }, - Service: &Service{ - Namespace: "default", - Name: "foo-admission-server-service", - // Selectors should select the pods that runs this webhook server. - Selectors: map[string]string{ - "app": "foo-admission-server", - }, - }, - }, }) if err != nil { // handle error diff --git a/pkg/webhook/internal/cert/doc.go b/pkg/webhook/internal/cert/doc.go deleted file mode 100644 index 5929246f02..0000000000 --- a/pkg/webhook/internal/cert/doc.go +++ /dev/null @@ -1,36 +0,0 @@ -/* -Copyright 2018 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -/* -Package cert provides functions to manage certificates for webhookClientConfiguration. - -Create a Provisioner with a CertWriter. - - provisioner := Provisioner{ - CertWriter: admission.NewSecretCertWriter(admission.SecretCertWriterOptions{...}), - } - -Provision the certificates for the webhookClientConfig - - err := provisioner.Provision(Options{ - ClientConfig: webhookClientConfig, - Objects: []runtime.Object{mutatingWebhookConfiguration, validatingWebhookConfiguration} - }) - if err != nil { - // handle error - } -*/ -package cert diff --git a/pkg/webhook/internal/cert/generator/certgenerator.go b/pkg/webhook/internal/cert/generator/certgenerator.go deleted file mode 100644 index 633eefc73c..0000000000 --- a/pkg/webhook/internal/cert/generator/certgenerator.go +++ /dev/null @@ -1,38 +0,0 @@ -/* -Copyright 2018 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package generator - -// Artifacts hosts a private key, its corresponding serving certificate and -// the CA certificate that signs the serving certificate. -type Artifacts struct { - // PEM encoded private key - Key []byte - // PEM encoded serving certificate - Cert []byte - // PEM encoded CA private key - CAKey []byte - // PEM encoded CA certificate - CACert []byte -} - -// CertGenerator is an interface to provision the serving certificate. -type CertGenerator interface { - // Generate returns a Artifacts struct. - Generate(CommonName string) (*Artifacts, error) - // SetCA sets the PEM-encoded CA private key and CA cert for signing the generated serving cert. - SetCA(caKey, caCert []byte) -} diff --git a/pkg/webhook/internal/cert/generator/certgenerator_test.go b/pkg/webhook/internal/cert/generator/certgenerator_test.go deleted file mode 100644 index fccf7b9468..0000000000 --- a/pkg/webhook/internal/cert/generator/certgenerator_test.go +++ /dev/null @@ -1,24 +0,0 @@ -/* -Copyright 2018 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package generator - -import "fmt" - -func ExampleServiceToCommonName() { - fmt.Println(ServiceToCommonName("myservicenamespace", "myservicename")) - // Output: myservicename.myservicenamespace.svc -} diff --git a/pkg/webhook/internal/cert/generator/doc.go b/pkg/webhook/internal/cert/generator/doc.go deleted file mode 100644 index 9d814e4289..0000000000 --- a/pkg/webhook/internal/cert/generator/doc.go +++ /dev/null @@ -1,30 +0,0 @@ -/* -Copyright 2018 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -/* -Package generator provides an interface and implementation to provision certificates. - -Create an instance of certGenerator. - - cg := SelfSignedCertGenerator{} - -Generate the certificates. - certs, err := cg.Generate("foo.bar.com") - if err != nil { - // handle error - } -*/ -package generator diff --git a/pkg/webhook/internal/cert/generator/fake/certgenerator.go b/pkg/webhook/internal/cert/generator/fake/certgenerator.go deleted file mode 100644 index 473f052fb4..0000000000 --- a/pkg/webhook/internal/cert/generator/fake/certgenerator.go +++ /dev/null @@ -1,53 +0,0 @@ -/* -Copyright 2018 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package fake - -import ( - "bytes" - "fmt" - - "sigs.k8s.io/controller-runtime/pkg/webhook/internal/cert/generator" -) - -// CertGenerator is a certGenerator for testing. -type CertGenerator struct { - CAKey []byte - CACert []byte - DNSNameToCertArtifacts map[string]*generator.Artifacts -} - -var _ generator.CertGenerator = &CertGenerator{} - -// SetCA sets the PEM-encoded CA private key and CA cert for signing the generated serving cert. -func (cp *CertGenerator) SetCA(CAKey, CACert []byte) { - cp.CAKey = CAKey - cp.CACert = CACert -} - -// Generate generates certificates by matching a common name. -func (cp *CertGenerator) Generate(commonName string) (*generator.Artifacts, error) { - certs, found := cp.DNSNameToCertArtifacts[commonName] - if !found { - return nil, fmt.Errorf("failed to find common name %q in the certGenerator", commonName) - } - if cp.CAKey != nil && cp.CACert != nil && - !bytes.Contains(cp.CAKey, []byte("invalid")) && !bytes.Contains(cp.CACert, []byte("invalid")) { - certs.CAKey = cp.CAKey - certs.CACert = cp.CACert - } - return certs, nil -} diff --git a/pkg/webhook/internal/cert/generator/selfsigned.go b/pkg/webhook/internal/cert/generator/selfsigned.go deleted file mode 100644 index 733b674ef7..0000000000 --- a/pkg/webhook/internal/cert/generator/selfsigned.go +++ /dev/null @@ -1,117 +0,0 @@ -/* -Copyright 2018 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package generator - -import ( - "crypto/rsa" - "crypto/x509" - "fmt" - "time" - - "k8s.io/client-go/util/cert" -) - -// ServiceToCommonName generates the CommonName for the certificate when using a k8s service. -func ServiceToCommonName(serviceNamespace, serviceName string) string { - return fmt.Sprintf("%s.%s.svc", serviceName, serviceNamespace) -} - -// SelfSignedCertGenerator implements the certGenerator interface. -// It provisions self-signed certificates. -type SelfSignedCertGenerator struct { - caKey []byte - caCert []byte -} - -var _ CertGenerator = &SelfSignedCertGenerator{} - -// SetCA sets the PEM-encoded CA private key and CA cert for signing the generated serving cert. -func (cp *SelfSignedCertGenerator) SetCA(caKey, caCert []byte) { - cp.caKey = caKey - cp.caCert = caCert -} - -// Generate creates and returns a CA certificate, certificate and -// key for the server. serverKey and serverCert are used by the server -// to establish trust for clients, CA certificate is used by the -// client to verify the server authentication chain. -// The cert will be valid for 365 days. -func (cp *SelfSignedCertGenerator) Generate(commonName string) (*Artifacts, error) { - var signingKey *rsa.PrivateKey - var signingCert *x509.Certificate - var valid bool - var err error - - valid, signingKey, signingCert = cp.validCACert() - if !valid { - signingKey, err = cert.NewPrivateKey() - if err != nil { - return nil, fmt.Errorf("failed to create the CA private key: %v", err) - } - signingCert, err = cert.NewSelfSignedCACert(cert.Config{CommonName: "webhook-cert-ca"}, signingKey) - if err != nil { - return nil, fmt.Errorf("failed to create the CA cert: %v", err) - } - } - - key, err := cert.NewPrivateKey() - if err != nil { - return nil, fmt.Errorf("failed to create the private key: %v", err) - } - signedCert, err := cert.NewSignedCert( - cert.Config{ - CommonName: commonName, - Usages: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth}, - }, - key, signingCert, signingKey, - ) - if err != nil { - return nil, fmt.Errorf("failed to create the cert: %v", err) - } - return &Artifacts{ - Key: cert.EncodePrivateKeyPEM(key), - Cert: cert.EncodeCertPEM(signedCert), - CAKey: cert.EncodePrivateKeyPEM(signingKey), - CACert: cert.EncodeCertPEM(signingCert), - }, nil -} - -func (cp *SelfSignedCertGenerator) validCACert() (bool, *rsa.PrivateKey, *x509.Certificate) { - if !ValidCACert(cp.caKey, cp.caCert, cp.caCert, "", - time.Now().AddDate(1, 0, 0)) { - return false, nil, nil - } - - var ok bool - key, err := cert.ParsePrivateKeyPEM(cp.caKey) - if err != nil { - return false, nil, nil - } - privateKey, ok := key.(*rsa.PrivateKey) - if !ok { - return false, nil, nil - } - - certs, err := cert.ParseCertsPEM(cp.caCert) - if err != nil { - return false, nil, nil - } - if len(certs) != 1 { - return false, nil, nil - } - return true, privateKey, certs[0] -} diff --git a/pkg/webhook/internal/cert/generator/selfsigned_test.go b/pkg/webhook/internal/cert/generator/selfsigned_test.go deleted file mode 100644 index d12e4d88e4..0000000000 --- a/pkg/webhook/internal/cert/generator/selfsigned_test.go +++ /dev/null @@ -1,134 +0,0 @@ -/* -Copyright 2018 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package generator - -import ( - "crypto/x509" - "encoding/pem" - - . "github.com/onsi/ginkgo" - . "github.com/onsi/gomega" -) - -var _ = Describe("Cert Generator", func() { - cn := "mysvc.myns.svc" - Describe("CA doesn't exist", func() { - It("should generate CA", func() { - cp := SelfSignedCertGenerator{} - certs, err := cp.Generate(cn) - Expect(err).NotTo(HaveOccurred()) - - // First, create the set of root certificates. For this example we only - // have one. It's also possible to omit this in order to use the - // default root set of the current operating system. - roots := x509.NewCertPool() - ok := roots.AppendCertsFromPEM(certs.CACert) - Expect(ok).To(BeTrue()) - - block, _ := pem.Decode(certs.Cert) - Expect(block).NotTo(BeNil()) - - cert, err := x509.ParseCertificate(block.Bytes) - Expect(err).NotTo(HaveOccurred()) - - opts := x509.VerifyOptions{ - DNSName: cn, - Roots: roots, - } - - _, err = cert.Verify(opts) - Expect(err).NotTo(HaveOccurred()) - }) - }) - - Describe("CA doesn't exist", func() { - Context("CA is valid", func() { - It("should reuse existing CA", func() { - cp := SelfSignedCertGenerator{} - certs, err := cp.Generate("foo.example.com") - Expect(err).NotTo(HaveOccurred()) - - cp = SelfSignedCertGenerator{} - cp.SetCA(certs.CAKey, certs.CACert) - certs, err = cp.Generate(cn) - Expect(err).NotTo(HaveOccurred()) - - Expect(certs.CAKey).To(Equal(cp.caKey)) - Expect(certs.CACert).To(Equal(cp.caCert)) - - // First, create the set of root certificates. For this example we only - // have one. It's also possible to omit this in order to use the - // default root set of the current operating system. - roots := x509.NewCertPool() - ok := roots.AppendCertsFromPEM(certs.CACert) - Expect(ok).To(BeTrue()) - - block, _ := pem.Decode(certs.Cert) - Expect(block).NotTo(BeNil()) - - cert, err := x509.ParseCertificate(block.Bytes) - Expect(err).NotTo(HaveOccurred()) - - opts := x509.VerifyOptions{ - DNSName: cn, - Roots: roots, - } - - _, err = cert.Verify(opts) - Expect(err).NotTo(HaveOccurred()) - }) - }) - - Context("CA is invalid", func() { - It("should reuse existing CA", func() { - cp := SelfSignedCertGenerator{} - certs, err := cp.Generate("foo.example.com") - Expect(err).NotTo(HaveOccurred()) - - cp = SelfSignedCertGenerator{} - cp.SetCA([]byte("invalidCAKey"), []byte("invalidCACert")) - - certs, err = cp.Generate(cn) - Expect(err).NotTo(HaveOccurred()) - - Expect(certs.CAKey).NotTo(Equal(cp.caKey)) - Expect(certs.CACert).NotTo(Equal(cp.caCert)) - - // First, create the set of root certificates. For this example we only - // have one. It's also possible to omit this in order to use the - // default root set of the current operating system. - roots := x509.NewCertPool() - ok := roots.AppendCertsFromPEM(certs.CACert) - Expect(ok).To(BeTrue()) - - block, _ := pem.Decode(certs.Cert) - Expect(block).NotTo(BeNil()) - - cert, err := x509.ParseCertificate(block.Bytes) - Expect(err).NotTo(HaveOccurred()) - - opts := x509.VerifyOptions{ - DNSName: cn, - Roots: roots, - } - - _, err = cert.Verify(opts) - Expect(err).NotTo(HaveOccurred()) - }) - }) - }) -}) diff --git a/pkg/webhook/internal/cert/generator/suite_test.go b/pkg/webhook/internal/cert/generator/suite_test.go deleted file mode 100644 index 1b232a6525..0000000000 --- a/pkg/webhook/internal/cert/generator/suite_test.go +++ /dev/null @@ -1,38 +0,0 @@ -/* -Copyright 2018 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package generator - -import ( - "testing" - - . "github.com/onsi/ginkgo" - . "github.com/onsi/gomega" - - "sigs.k8s.io/controller-runtime/pkg/envtest" - logf "sigs.k8s.io/controller-runtime/pkg/log" - "sigs.k8s.io/controller-runtime/pkg/log/zap" -) - -func TestSource(t *testing.T) { - RegisterFailHandler(Fail) - RunSpecsWithDefaultAndCustomReporters(t, "Cert Generator Test Suite", []Reporter{envtest.NewlineReporter{}}) -} - -var _ = BeforeSuite(func(done Done) { - logf.SetLogger(zap.LoggerTo(GinkgoWriter, true)) - close(done) -}, 60) diff --git a/pkg/webhook/internal/cert/generator/util.go b/pkg/webhook/internal/cert/generator/util.go deleted file mode 100644 index fd459cf05b..0000000000 --- a/pkg/webhook/internal/cert/generator/util.go +++ /dev/null @@ -1,61 +0,0 @@ -/* -Copyright 2018 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package generator - -import ( - "crypto/tls" - "crypto/x509" - "encoding/pem" - "time" -) - -// ValidCACert think cert and key are valid if they meet the following requirements: -// - key and cert are valid pair -// - caCert is the root ca of cert -// - cert is for dnsName -// - cert won't expire before time -func ValidCACert(key, cert, caCert []byte, dnsName string, time time.Time) bool { - if len(key) == 0 || len(cert) == 0 || len(caCert) == 0 { - return false - } - // Verify key and cert are valid pair - _, err := tls.X509KeyPair(cert, key) - if err != nil { - return false - } - - // Verify cert is valid for at least 1 year. - pool := x509.NewCertPool() - if !pool.AppendCertsFromPEM(caCert) { - return false - } - block, _ := pem.Decode([]byte(cert)) - if block == nil { - return false - } - c, err := x509.ParseCertificate(block.Bytes) - if err != nil { - return false - } - ops := x509.VerifyOptions{ - DNSName: dnsName, - Roots: pool, - CurrentTime: time, - } - _, err = c.Verify(ops) - return err == nil -} diff --git a/pkg/webhook/internal/cert/provisioner.go b/pkg/webhook/internal/cert/provisioner.go deleted file mode 100644 index 992e9da939..0000000000 --- a/pkg/webhook/internal/cert/provisioner.go +++ /dev/null @@ -1,133 +0,0 @@ -/* -Copyright 2018 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package cert - -import ( - "bytes" - "errors" - "fmt" - "net" - "net/url" - - admissionregistrationv1beta1 "k8s.io/api/admissionregistration/v1beta1" - "k8s.io/apimachinery/pkg/runtime" - "sigs.k8s.io/controller-runtime/pkg/webhook/internal/cert/generator" - "sigs.k8s.io/controller-runtime/pkg/webhook/internal/cert/writer" -) - -// Provisioner provisions certificates for webhook configurations and writes them to an output -// destination - such as a Secret or local file. Provisioner can update the CA field of -// certain resources with the CA of the certs. -type Provisioner struct { - // CertWriter knows how to persist the certificate. - CertWriter writer.CertWriter -} - -// Options are options for provisioning the certificate. -type Options struct { - // ClientConfig is the WebhookClientCert that contains the information to generate - // the certificate. The CA Certificate will be updated in the ClientConfig. - // The updated ClientConfig will be used to inject into other runtime.Objects, - // e.g. MutatingWebhookConfiguration and ValidatingWebhookConfiguration. - ClientConfig *admissionregistrationv1beta1.WebhookClientConfig - // Objects are the objects that will use the ClientConfig above. - Objects []runtime.Object -} - -// Provision provisions certificates for the WebhookClientConfig. -// It ensures the cert and CA are valid and not expiring. -// It updates the CABundle in the webhookClientConfig if necessary. -// It inject the WebhookClientConfig into options.Objects. -func (cp *Provisioner) Provision(options Options) (bool, error) { - if cp.CertWriter == nil { - return false, errors.New("CertWriter need to be set") - } - - dnsName, err := dnsNameFromClientConfig(options.ClientConfig) - if err != nil { - return false, err - } - - certs, changed, err := cp.CertWriter.EnsureCert(dnsName) - if err != nil { - return false, err - } - - caBundle := options.ClientConfig.CABundle - caCert := certs.CACert - // TODO(mengqiy): limit the size of the CABundle by GC the old CA certificate - // this is important since the max record size in etcd is 1MB (latest version is 1.5MB). - if !bytes.Contains(caBundle, caCert) { - // Ensure the CA bundle in the webhook configuration has the signing CA. - options.ClientConfig.CABundle = append(caBundle, caCert...) - changed = true - } - return changed, cp.inject(options.ClientConfig, options.Objects) -} - -// Inject the ClientConfig to the objects. -// It supports MutatingWebhookConfiguration and ValidatingWebhookConfiguration. -func (cp *Provisioner) inject(cc *admissionregistrationv1beta1.WebhookClientConfig, objs []runtime.Object) error { - if cc == nil { - return nil - } - for i := range objs { - switch typed := objs[i].(type) { - case *admissionregistrationv1beta1.MutatingWebhookConfiguration: - injectForEachWebhook(cc, typed.Webhooks) - case *admissionregistrationv1beta1.ValidatingWebhookConfiguration: - injectForEachWebhook(cc, typed.Webhooks) - default: - return fmt.Errorf("%#v is not supported for injecting a webhookClientConfig", - objs[i].GetObjectKind().GroupVersionKind()) - } - } - return cp.CertWriter.Inject(objs...) -} - -func injectForEachWebhook( - cc *admissionregistrationv1beta1.WebhookClientConfig, - webhooks []admissionregistrationv1beta1.Webhook) { - for i := range webhooks { - // only replacing the CA bundle to preserve the path in the WebhookClientConfig - webhooks[i].ClientConfig.CABundle = cc.CABundle - } -} - -func dnsNameFromClientConfig(config *admissionregistrationv1beta1.WebhookClientConfig) (string, error) { - if config == nil { - return "", errors.New("clientConfig should not be empty") - } - if config.Service != nil && config.URL != nil { - return "", fmt.Errorf("service and URL can't be set at the same time in a webhook: %v", config) - } - if config.Service == nil && config.URL == nil { - return "", fmt.Errorf("one of service and URL need to be set in a webhook: %v", config) - } - if config.Service != nil { - return generator.ServiceToCommonName(config.Service.Namespace, config.Service.Name), nil - } - u, err := url.Parse(*config.URL) - if err != nil { - return "", err - } - host, _, err := net.SplitHostPort(u.Host) - if err != nil { - return u.Host, nil - } - return host, err -} diff --git a/pkg/webhook/internal/cert/provisioner_test.go b/pkg/webhook/internal/cert/provisioner_test.go deleted file mode 100644 index 4b9bfa6b85..0000000000 --- a/pkg/webhook/internal/cert/provisioner_test.go +++ /dev/null @@ -1,229 +0,0 @@ -/* -Copyright 2018 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package cert - -import ( - . "github.com/onsi/ginkgo" - . "github.com/onsi/gomega" - - admissionregistrationv1beta1 "k8s.io/api/admissionregistration/v1beta1" - corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/runtime" - "sigs.k8s.io/controller-runtime/pkg/webhook/internal/cert/generator" - "sigs.k8s.io/controller-runtime/pkg/webhook/internal/cert/writer" -) - -var _ = Describe("provisioner", func() { - Context("Invalid Provisioner", func() { - It("should return error", func() { - p := Provisioner{} - _, err := p.Provision(Options{}) - Expect(err).To(MatchError("CertWriter need to be set")) - }) - }) - - Context("No objects in the options", func() { - It("should return no error", func() { - fcw := &fakeCertWriter{} - p := Provisioner{CertWriter: fcw} - changed, err := p.Provision(Options{ - ClientConfig: &admissionregistrationv1beta1.WebhookClientConfig{ - Service: &admissionregistrationv1beta1.ServiceReference{ - Namespace: "test-svc-namespace", - Name: "test-service", - }, - }, - }) - Expect(err).NotTo(HaveOccurred()) - Expect(changed).To(BeTrue()) - Expect(fcw.invokedEnsureCert).To(BeTrue()) - Expect(fcw.invokedInject).To(BeTrue()) - }) - }) - - Context("WebhookClientConfig is missing in the options", func() { - It("should return error", func() { - p := Provisioner{CertWriter: &fakeCertWriter{}} - _, err := p.Provision(Options{ - Objects: []runtime.Object{ - &corev1.Pod{}, - }, - }) - Expect(err).To(MatchError("clientConfig should not be empty")) - }) - }) - - Context("object is not support for injecting webhookClientConfig", func() { - It("should return no error", func() { - p := Provisioner{CertWriter: &fakeCertWriter{}} - _, err := p.Provision(Options{ - ClientConfig: &admissionregistrationv1beta1.WebhookClientConfig{ - Service: &admissionregistrationv1beta1.ServiceReference{ - Namespace: "test-svc-namespace", - Name: "test-service", - }, - }, - Objects: []runtime.Object{ - &corev1.Pod{}, - }, - }) - Expect(err.Error()).To(ContainSubstring("not supported for injecting a webhookClientConfig")) - }) - }) - - Context("webhookConfig has 0 webhook", func() { - It("should return no error", func() { - fcw := &fakeCertWriter{} - p := Provisioner{CertWriter: fcw} - _, err := p.Provision(Options{ - ClientConfig: &admissionregistrationv1beta1.WebhookClientConfig{ - Service: &admissionregistrationv1beta1.ServiceReference{ - Namespace: "test-svc-namespace", - Name: "test-service", - }, - }, - Objects: []runtime.Object{ - &admissionregistrationv1beta1.MutatingWebhookConfiguration{}, - }, - }) - Expect(err).To(BeNil()) - Expect(fcw.invokedEnsureCert).To(BeTrue()) - Expect(fcw.invokedInject).To(BeTrue()) - }) - }) - - Context("happy path", func() { - It("should return no error", func() { - fcw := &fakeCertWriter{} - mwc := &admissionregistrationv1beta1.MutatingWebhookConfiguration{ - Webhooks: []admissionregistrationv1beta1.Webhook{ - { - Name: "foo-webhook", - }, - }, - } - vwc := &admissionregistrationv1beta1.ValidatingWebhookConfiguration{ - Webhooks: []admissionregistrationv1beta1.Webhook{ - { - Name: "foo-webhook", - }, - }, - } - p := Provisioner{CertWriter: fcw} - _, err := p.Provision(Options{ - ClientConfig: &admissionregistrationv1beta1.WebhookClientConfig{ - Service: &admissionregistrationv1beta1.ServiceReference{ - Namespace: "test-svc-namespace", - Name: "test-service", - }, - }, - Objects: []runtime.Object{mwc, vwc}, - }) - Expect(err).To(BeNil()) - Expect(fcw.invokedEnsureCert).To(BeTrue()) - Expect(fcw.invokedInject).To(BeTrue()) - }) - }) -}) - -var _ = Describe("dnsNameFromClientConfig", func() { - Context("Invalid WebhookClientConfig", func() { - It("should return error", func() { - _, err := dnsNameFromClientConfig(nil) - Expect(err).To(MatchError("clientConfig should not be empty")) - }) - }) - - Context("Neither Service nor URL is set", func() { - It("should return error", func() { - urlStr := "foo.example.com" - cc := &admissionregistrationv1beta1.WebhookClientConfig{ - Service: &admissionregistrationv1beta1.ServiceReference{}, - URL: &urlStr, - } - _, err := dnsNameFromClientConfig(cc) - Expect(err.Error()).To(ContainSubstring("service and URL can't be set at the same time in a webhook")) - }) - }) - - Context("Both Service and URL are set", func() { - It("should return error", func() { - urlStr := "https://foo.example.com" - cc := &admissionregistrationv1beta1.WebhookClientConfig{ - Service: &admissionregistrationv1beta1.ServiceReference{}, - URL: &urlStr, - } - _, err := dnsNameFromClientConfig(cc) - Expect(err.Error()).To(ContainSubstring("service and URL can't be set at the same time in a webhook")) - }) - }) - - Context("Only service is set", func() { - It("should return a DNS name", func() { - path := "somepath" - cc := &admissionregistrationv1beta1.WebhookClientConfig{ - Service: &admissionregistrationv1beta1.ServiceReference{ - Namespace: "test-svc-namespace", - Name: "test-service", - Path: &path, - }, - } - dnsName, err := dnsNameFromClientConfig(cc) - Expect(err).NotTo(HaveOccurred()) - Expect(dnsName).To(Equal("test-service.test-svc-namespace.svc")) - }) - }) - - Context("Only URL is set", func() { - It("should return a DNS name", func() { - urlStr := "https://foo.example.com/webhookendpoint" - cc := &admissionregistrationv1beta1.WebhookClientConfig{ - URL: &urlStr, - } - dnsName, err := dnsNameFromClientConfig(cc) - Expect(err).NotTo(HaveOccurred()) - Expect(dnsName).To(Equal("foo.example.com")) - }) - - It("should return a DNS name w/o port", func() { - urlStr := "https://foo.example.com:9876/webhookendpoint" - cc := &admissionregistrationv1beta1.WebhookClientConfig{ - URL: &urlStr, - } - dnsName, err := dnsNameFromClientConfig(cc) - Expect(err).NotTo(HaveOccurred()) - Expect(dnsName).To(Equal("foo.example.com")) - }) - }) -}) - -type fakeCertWriter struct { - invokedEnsureCert bool - invokedInject bool -} - -var _ writer.CertWriter = &fakeCertWriter{} - -func (f *fakeCertWriter) EnsureCert(dnsName string) (*generator.Artifacts, bool, error) { - f.invokedEnsureCert = true - return &generator.Artifacts{}, true, nil -} - -func (f *fakeCertWriter) Inject(objs ...runtime.Object) error { - f.invokedInject = true - return nil -} diff --git a/pkg/webhook/internal/cert/suite_test.go b/pkg/webhook/internal/cert/suite_test.go deleted file mode 100644 index 6762ada6a5..0000000000 --- a/pkg/webhook/internal/cert/suite_test.go +++ /dev/null @@ -1,38 +0,0 @@ -/* -Copyright 2018 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package cert - -import ( - "testing" - - . "github.com/onsi/ginkgo" - . "github.com/onsi/gomega" - - "sigs.k8s.io/controller-runtime/pkg/envtest" - logf "sigs.k8s.io/controller-runtime/pkg/log" - "sigs.k8s.io/controller-runtime/pkg/log/zap" -) - -func TestSource(t *testing.T) { - RegisterFailHandler(Fail) - RunSpecsWithDefaultAndCustomReporters(t, "Cert Provisioner Test Suite", []Reporter{envtest.NewlineReporter{}}) -} - -var _ = BeforeSuite(func(done Done) { - logf.SetLogger(zap.LoggerTo(GinkgoWriter, true)) - close(done) -}, 60) diff --git a/pkg/webhook/internal/cert/writer/atomic/atomic_writer.go b/pkg/webhook/internal/cert/writer/atomic/atomic_writer.go deleted file mode 100644 index ec2665c3ee..0000000000 --- a/pkg/webhook/internal/cert/writer/atomic/atomic_writer.go +++ /dev/null @@ -1,453 +0,0 @@ -/* -Copyright 2016 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package atomic - -import ( - "bytes" - "fmt" - "io/ioutil" - "os" - "path" - "path/filepath" - "runtime" - "strings" - "time" - - "github.com/go-logr/logr" - "k8s.io/apimachinery/pkg/util/sets" -) - -const ( - maxFileNameLength = 255 - maxPathLength = 4096 -) - -// AtomicWriter handles atomically projecting content for a set of files into -// a target directory. -// -// Note: -// -// 1. AtomicWriter reserves the set of pathnames starting with `..`. -// 2. AtomicWriter offers no concurrency guarantees and must be synchronized -// by the caller. -// -// The visible files in this volume are symlinks to files in the writer's data -// directory. Actual files are stored in a hidden timestamped directory which -// is symlinked to by the data directory. The timestamped directory and -// data directory symlink are created in the writer's target dir.  This scheme -// allows the files to be atomically updated by changing the target of the -// data directory symlink. -// -// Consumers of the target directory can monitor the ..data symlink using -// inotify or fanotify to receive events when the content in the volume is -// updated. -type AtomicWriter struct { - targetDir string - log logr.Logger -} - -type FileProjection struct { - Data []byte - Mode int32 -} - -// NewAtomicWriter creates a new AtomicWriter configured to write to the given -// target directory, or returns an error if the target directory does not exist. -func NewAtomicWriter(targetDir string, log logr.Logger) (*AtomicWriter, error) { - _, err := os.Stat(targetDir) - if os.IsNotExist(err) { - return nil, err - } - - return &AtomicWriter{targetDir: targetDir, log: log}, nil -} - -const ( - dataDirName = "..data" - newDataDirName = "..data_tmp" -) - -// Write does an atomic projection of the given payload into the writer's target -// directory. Input paths must not begin with '..'. -// -// The Write algorithm is: -// -// 1. The payload is validated; if the payload is invalid, the function returns -// 2.  The current timestamped directory is detected by reading the data directory -// symlink -// 3. The old version of the volume is walked to determine whether any -// portion of the payload was deleted and is still present on disk. -// 4. The data in the current timestamped directory is compared to the projected -// data to determine if an update is required. -// 5.  A new timestamped dir is created -// 6. The payload is written to the new timestamped directory -// 7.  Symlinks and directory for new user-visible files are created (if needed). -// -// For example, consider the files: -// /podName -// /user/labels -// /k8s/annotations -// -// The user visible files are symbolic links into the internal data directory: -// /podName -> ..data/podName -// /usr -> ..data/usr -// /k8s -> ..data/k8s -// -// The data directory itself is a link to a timestamped directory with -// the real data: -// /..data -> ..2016_02_01_15_04_05.12345678/ -// 8.  A symlink to the new timestamped directory ..data_tmp is created that will -// become the new data directory -// 9.  The new data directory symlink is renamed to the data directory; rename is atomic -// 10. Old paths are removed from the user-visible portion of the target directory -// 11.  The previous timestamped directory is removed, if it exists -func (w *AtomicWriter) Write(payload map[string]FileProjection) error { - // (1) - cleanPayload, err := validatePayload(payload) - if err != nil { - w.log.Error(err, "invalid payload") - return err - } - - // (2) - dataDirPath := path.Join(w.targetDir, dataDirName) - oldTsDir, err := os.Readlink(dataDirPath) - if err != nil { - if !os.IsNotExist(err) { - w.log.Error(err, "unable to read link for data directory") - return err - } - // although Readlink() returns "" on err, don't be fragile by relying on it (since it's not specified in docs) - // empty oldTsDir indicates that it didn't exist - oldTsDir = "" - } - oldTsPath := path.Join(w.targetDir, oldTsDir) - - var pathsToRemove sets.String - // if there was no old version, there's nothing to remove - if len(oldTsDir) != 0 { - // (3) - pathsToRemove, err = w.pathsToRemove(cleanPayload, oldTsPath) - if err != nil { - w.log.Error(err, "unable to determine user-visible files to remove") - return err - } - - // (4) - if should, err := shouldWritePayload(cleanPayload, oldTsPath); err != nil { - w.log.Error(err, "unable to determine whether payload should be written to disk") - return err - } else if !should && len(pathsToRemove) == 0 { - w.log.V(1).Info("no update required for target directory", "directory", w.targetDir) - return nil - } else { - w.log.V(1).Info("write required for target directory", "directory", w.targetDir) - } - } - - // (5) - tsDir, err := w.newTimestampDir() - if err != nil { - w.log.Error(err, "error creating new ts data directory") - return err - } - tsDirName := filepath.Base(tsDir) - - // (6) - if err = w.writePayloadToDir(cleanPayload, tsDir); err != nil { - w.log.Error(err, "unable to write payload to ts data directory", "ts directory", tsDir) - return err - } else { - w.log.V(1).Info("performed write of new data to ts data directory", "ts directory", tsDir) - } - - // (7) - if err = w.createUserVisibleFiles(cleanPayload); err != nil { - w.log.Error(err, "unable to create visible symlinks in target directory", "target directory", w.targetDir) - return err - } - - // (8) - newDataDirPath := path.Join(w.targetDir, newDataDirName) - if err = os.Symlink(tsDirName, newDataDirPath); err != nil { - os.RemoveAll(tsDir) - w.log.Error(err, "unable to create symbolic link for atomic update") - return err - } - - // (9) - if runtime.GOOS == "windows" { - os.Remove(dataDirPath) - err = os.Symlink(tsDirName, dataDirPath) - os.Remove(newDataDirPath) - } else { - err = os.Rename(newDataDirPath, dataDirPath) - } - if err != nil { - os.Remove(newDataDirPath) - os.RemoveAll(tsDir) - w.log.Error(err, "unable to rename symbolic link for data directory", "data directory", newDataDirPath) - return err - } - - // (10) - if err = w.removeUserVisiblePaths(pathsToRemove); err != nil { - w.log.Error(err, "unable to remove old visible symlinks") - return err - } - - // (11) - if len(oldTsDir) > 0 { - if err = os.RemoveAll(oldTsPath); err != nil { - w.log.Error(err, "unable to remove old data directory", "data directory", oldTsDir) - return err - } - } - - return nil -} - -// validatePayload returns an error if any path in the payload returns a copy of the payload with the paths cleaned. -func validatePayload(payload map[string]FileProjection) (map[string]FileProjection, error) { - cleanPayload := make(map[string]FileProjection) - for k, content := range payload { - if err := validatePath(k); err != nil { - return nil, err - } - - cleanPayload[filepath.Clean(k)] = content - } - - return cleanPayload, nil -} - -// validatePath validates a single path, returning an error if the path is -// invalid. paths may not: -// -// 1. be absolute -// 2. contain '..' as an element -// 3. start with '..' -// 4. contain filenames larger than 255 characters -// 5. be longer than 4096 characters -func validatePath(targetPath string) error { - // TODO: somehow unify this with the similar api validation, - // validateVolumeSourcePath; the error semantics are just different enough - // from this that it was time-prohibitive trying to find the right - // refactoring to re-use. - if targetPath == "" { - return fmt.Errorf("invalid path: must not be empty: %q", targetPath) - } - if path.IsAbs(targetPath) { - return fmt.Errorf("invalid path: must be relative path: %s", targetPath) - } - - if len(targetPath) > maxPathLength { - return fmt.Errorf("invalid path: must be less than or equal to %d characters", maxPathLength) - } - - items := strings.Split(targetPath, string(os.PathSeparator)) - for _, item := range items { - if item == ".." { - return fmt.Errorf("invalid path: must not contain '..': %s", targetPath) - } - if len(item) > maxFileNameLength { - return fmt.Errorf("invalid path: filenames must be less than or equal to %d characters", maxFileNameLength) - } - } - if strings.HasPrefix(items[0], "..") && len(items[0]) > 2 { - return fmt.Errorf("invalid path: must not start with '..': %s", targetPath) - } - - return nil -} - -// shouldWritePayload returns whether the payload should be written to disk. -func shouldWritePayload(payload map[string]FileProjection, oldTsDir string) (bool, error) { - for userVisiblePath, fileProjection := range payload { - shouldWrite, err := shouldWriteFile(path.Join(oldTsDir, userVisiblePath), fileProjection.Data) - if err != nil { - return false, err - } - - if shouldWrite { - return true, nil - } - } - - return false, nil -} - -// shouldWriteFile returns whether a new version of a file should be written to disk. -func shouldWriteFile(path string, content []byte) (bool, error) { - _, err := os.Lstat(path) - if os.IsNotExist(err) { - return true, nil - } - - contentOnFs, err := ioutil.ReadFile(path) - if err != nil { - return false, err - } - - return (bytes.Compare(content, contentOnFs) != 0), nil -} - -// pathsToRemove walks the current version of the data directory and -// determines which paths should be removed (if any) after the payload is -// written to the target directory. -func (w *AtomicWriter) pathsToRemove(payload map[string]FileProjection, oldTsDir string) (sets.String, error) { - paths := sets.NewString() - visitor := func(path string, info os.FileInfo, err error) error { - relativePath := strings.TrimPrefix(path, oldTsDir) - relativePath = strings.TrimPrefix(relativePath, string(os.PathSeparator)) - if relativePath == "" { - return nil - } - - paths.Insert(relativePath) - return nil - } - - err := filepath.Walk(oldTsDir, visitor) - if os.IsNotExist(err) { - return nil, nil - } else if err != nil { - return nil, err - } - w.log.V(1).Info("current paths", "target directory", w.targetDir, "paths", paths.List()) - - newPaths := sets.NewString() - for file := range payload { - // add all subpaths for the payload to the set of new paths - // to avoid attempting to remove non-empty dirs - for subPath := file; subPath != ""; { - newPaths.Insert(subPath) - subPath, _ = filepath.Split(subPath) - subPath = strings.TrimSuffix(subPath, string(os.PathSeparator)) - } - } - w.log.V(1).Info("new paths", "target directory", w.targetDir, "paths", newPaths.List()) - - result := paths.Difference(newPaths) - w.log.V(1).Info("paths to remove", "target directory", w.targetDir, "paths", result) - - return result, nil -} - -// newTimestampDir creates a new timestamp directory -func (w *AtomicWriter) newTimestampDir() (string, error) { - tsDir, err := ioutil.TempDir(w.targetDir, time.Now().UTC().Format("..2006_01_02_15_04_05.")) - if err != nil { - w.log.Error(err, "unable to create new temp directory") - return "", err - } - - // 0755 permissions are needed to allow 'group' and 'other' to recurse the - // directory tree. do a chmod here to ensure that permissions are set correctly - // regardless of the process' umask. - err = os.Chmod(tsDir, 0755) - if err != nil { - w.log.Error(err, "unable to set mode on new temp directory") - return "", err - } - - return tsDir, nil -} - -// writePayloadToDir writes the given payload to the given directory. The -// directory must exist. -func (w *AtomicWriter) writePayloadToDir(payload map[string]FileProjection, dir string) error { - for userVisiblePath, fileProjection := range payload { - content := fileProjection.Data - mode := os.FileMode(fileProjection.Mode) - fullPath := path.Join(dir, userVisiblePath) - baseDir, _ := filepath.Split(fullPath) - - err := os.MkdirAll(baseDir, os.ModePerm) - if err != nil { - w.log.Error(err, "unable to create directory", "directory", baseDir) - return err - } - - err = ioutil.WriteFile(fullPath, content, mode) - if err != nil { - w.log.Error(err, "unable to write file", "file", fullPath, "mode", mode) - return err - } - // Chmod is needed because ioutil.WriteFile() ends up calling - // open(2) to create the file, so the final mode used is "mode & - // ~umask". But we want to make sure the specified mode is used - // in the file no matter what the umask is. - err = os.Chmod(fullPath, mode) - if err != nil { - w.log.Error(err, "unable to write file", "file", fullPath, "mode", mode) - } - } - - return nil -} - -// createUserVisibleFiles creates the relative symlinks for all the -// files configured in the payload. If the directory in a file path does not -// exist, it is created. -// -// Viz: -// For files: "bar", "foo/bar", "baz/bar", "foo/baz/blah" -// the following symlinks are created: -// bar -> ..data/bar -// foo -> ..data/foo -// baz -> ..data/baz -func (w *AtomicWriter) createUserVisibleFiles(payload map[string]FileProjection) error { - for userVisiblePath := range payload { - slashpos := strings.Index(userVisiblePath, string(os.PathSeparator)) - if slashpos == -1 { - slashpos = len(userVisiblePath) - } - linkname := userVisiblePath[:slashpos] - _, err := os.Readlink(path.Join(w.targetDir, linkname)) - if err != nil && os.IsNotExist(err) { - // The link into the data directory for this path doesn't exist; create it - visibleFile := path.Join(w.targetDir, linkname) - dataDirFile := path.Join(dataDirName, linkname) - - err = os.Symlink(dataDirFile, visibleFile) - if err != nil { - return err - } - } - } - return nil -} - -// removeUserVisiblePaths removes the set of paths from the user-visible -// portion of the writer's target directory. -func (w *AtomicWriter) removeUserVisiblePaths(paths sets.String) error { - ps := string(os.PathSeparator) - var lasterr error - for p := range paths { - // only remove symlinks from the volume root directory (i.e. items that don't contain '/') - if strings.Contains(p, ps) { - continue - } - if err := os.Remove(path.Join(w.targetDir, p)); err != nil { - w.log.Error(err, "unable to prune old user-visible path", "path", p) - lasterr = err - } - } - - return lasterr -} diff --git a/pkg/webhook/internal/cert/writer/atomic/atomic_writer_test.go b/pkg/webhook/internal/cert/writer/atomic/atomic_writer_test.go deleted file mode 100644 index bc1e7285a6..0000000000 --- a/pkg/webhook/internal/cert/writer/atomic/atomic_writer_test.go +++ /dev/null @@ -1,987 +0,0 @@ -// +build linux - -/* -Copyright 2016 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package atomic - -import ( - "encoding/base64" - "io/ioutil" - "os" - "path" - "path/filepath" - "reflect" - "strings" - "testing" - - log "github.com/go-logr/logr/testing" - "k8s.io/apimachinery/pkg/util/sets" - utiltesting "k8s.io/client-go/util/testing" -) - -func TestNewAtomicWriter(t *testing.T) { - targetDir, err := utiltesting.MkTmpdir("atomic-write") - if err != nil { - t.Fatalf("unexpected error creating tmp dir: %v", err) - } - defer os.RemoveAll(targetDir) - - _, err = NewAtomicWriter(targetDir, log.TestLogger{T: t}) - if err != nil { - t.Fatalf("unexpected error creating writer for existing target dir: %v", err) - } - - nonExistentDir, err := utiltesting.MkTmpdir("atomic-write") - if err != nil { - t.Fatalf("unexpected error creating tmp dir: %v", err) - } - err = os.Remove(nonExistentDir) - if err != nil { - t.Fatalf("unexpected error ensuring dir %v does not exist: %v", nonExistentDir, err) - } - - _, err = NewAtomicWriter(nonExistentDir, log.TestLogger{T: t}) - if err == nil { - t.Fatalf("unexpected success creating writer for nonexistent target dir: %v", err) - } -} - -func TestValidatePath(t *testing.T) { - maxPath := strings.Repeat("a", maxPathLength+1) - maxFile := strings.Repeat("a", maxFileNameLength+1) - - cases := []struct { - name string - path string - valid bool - }{ - { - name: "valid 1", - path: "i/am/well/behaved.txt", - valid: true, - }, - { - name: "valid 2", - path: "keepyourheaddownandfollowtherules.txt", - valid: true, - }, - { - name: "max path length", - path: maxPath, - valid: false, - }, - { - name: "max file length", - path: maxFile, - valid: false, - }, - { - name: "absolute failure", - path: "/dev/null", - valid: false, - }, - { - name: "reserved path", - path: "..sneaky.txt", - valid: false, - }, - { - name: "contains doubledot 1", - path: "hello/there/../../../../../../etc/passwd", - valid: false, - }, - { - name: "contains doubledot 2", - path: "hello/../etc/somethingbad", - valid: false, - }, - { - name: "empty", - path: "", - valid: false, - }, - } - - for _, tc := range cases { - err := validatePath(tc.path) - if tc.valid && err != nil { - t.Errorf("%v: unexpected failure: %v", tc.name, err) - continue - } - - if !tc.valid && err == nil { - t.Errorf("%v: unexpected success", tc.name) - } - } -} - -func TestPathsToRemove(t *testing.T) { - cases := []struct { - name string - payload1 map[string]FileProjection - payload2 map[string]FileProjection - expected sets.String - }{ - { - name: "simple", - payload1: map[string]FileProjection{ - "foo.txt": {Mode: 0644, Data: []byte("foo")}, - "bar.txt": {Mode: 0644, Data: []byte("bar")}, - }, - payload2: map[string]FileProjection{ - "foo.txt": {Mode: 0644, Data: []byte("foo")}, - }, - expected: sets.NewString("bar.txt"), - }, - { - name: "simple 2", - payload1: map[string]FileProjection{ - "foo.txt": {Mode: 0644, Data: []byte("foo")}, - "zip/bar.txt": {Mode: 0644, Data: []byte("zip/b}ar")}, - }, - payload2: map[string]FileProjection{ - "foo.txt": {Mode: 0644, Data: []byte("foo")}, - }, - expected: sets.NewString("zip/bar.txt", "zip"), - }, - { - name: "subdirs 1", - payload1: map[string]FileProjection{ - "foo.txt": {Mode: 0644, Data: []byte("foo")}, - "zip/zap/bar.txt": {Mode: 0644, Data: []byte("zip/bar")}, - }, - payload2: map[string]FileProjection{ - "foo.txt": {Mode: 0644, Data: []byte("foo")}, - }, - expected: sets.NewString("zip/zap/bar.txt", "zip", "zip/zap"), - }, - { - name: "subdirs 2", - payload1: map[string]FileProjection{ - "foo.txt": {Mode: 0644, Data: []byte("foo")}, - "zip/1/2/3/4/bar.txt": {Mode: 0644, Data: []byte("zip/b}ar")}, - }, - payload2: map[string]FileProjection{ - "foo.txt": {Mode: 0644, Data: []byte("foo")}, - }, - expected: sets.NewString("zip/1/2/3/4/bar.txt", "zip", "zip/1", "zip/1/2", "zip/1/2/3", "zip/1/2/3/4"), - }, - { - name: "subdirs 3", - payload1: map[string]FileProjection{ - "foo.txt": {Mode: 0644, Data: []byte("foo")}, - "zip/1/2/3/4/bar.txt": {Mode: 0644, Data: []byte("zip/b}ar")}, - "zap/a/b/c/bar.txt": {Mode: 0644, Data: []byte("zap/bar")}, - }, - payload2: map[string]FileProjection{ - "foo.txt": {Mode: 0644, Data: []byte("foo")}, - }, - expected: sets.NewString("zip/1/2/3/4/bar.txt", "zip", "zip/1", "zip/1/2", "zip/1/2/3", "zip/1/2/3/4", "zap", "zap/a", "zap/a/b", "zap/a/b/c", "zap/a/b/c/bar.txt"), - }, - { - name: "subdirs 4", - payload1: map[string]FileProjection{ - "foo.txt": {Mode: 0644, Data: []byte("foo")}, - "zap/1/2/3/4/bar.txt": {Mode: 0644, Data: []byte("zip/bar")}, - "zap/1/2/c/bar.txt": {Mode: 0644, Data: []byte("zap/bar")}, - "zap/1/2/magic.txt": {Mode: 0644, Data: []byte("indigo")}, - }, - payload2: map[string]FileProjection{ - "foo.txt": {Mode: 0644, Data: []byte("foo")}, - "zap/1/2/magic.txt": {Mode: 0644, Data: []byte("indigo")}, - }, - expected: sets.NewString("zap/1/2/3/4/bar.txt", "zap/1/2/3", "zap/1/2/3/4", "zap/1/2/3/4/bar.txt", "zap/1/2/c", "zap/1/2/c/bar.txt"), - }, - { - name: "subdirs 5", - payload1: map[string]FileProjection{ - "foo.txt": {Mode: 0644, Data: []byte("foo")}, - "zap/1/2/3/4/bar.txt": {Mode: 0644, Data: []byte("zip/bar")}, - "zap/1/2/c/bar.txt": {Mode: 0644, Data: []byte("zap/bar")}, - }, - payload2: map[string]FileProjection{ - "foo.txt": {Mode: 0644, Data: []byte("foo")}, - "zap/1/2/magic.txt": {Mode: 0644, Data: []byte("indigo")}, - }, - expected: sets.NewString("zap/1/2/3/4/bar.txt", "zap/1/2/3", "zap/1/2/3/4", "zap/1/2/3/4/bar.txt", "zap/1/2/c", "zap/1/2/c/bar.txt"), - }, - } - - for _, tc := range cases { - targetDir, err := utiltesting.MkTmpdir("atomic-write") - if err != nil { - t.Errorf("%v: unexpected error creating tmp dir: %v", tc.name, err) - continue - } - defer os.RemoveAll(targetDir) - - writer := &AtomicWriter{targetDir: targetDir, log: log.TestLogger{T: t}} - err = writer.Write(tc.payload1) - if err != nil { - t.Errorf("%v: unexpected error writing: %v", tc.name, err) - continue - } - - dataDirPath := path.Join(targetDir, dataDirName) - oldTsDir, err := os.Readlink(dataDirPath) - if err != nil && os.IsNotExist(err) { - t.Errorf("Data symlink does not exist: %v", dataDirPath) - continue - } else if err != nil { - t.Errorf("Unable to read symlink %v: %v", dataDirPath, err) - continue - } - - actual, err := writer.pathsToRemove(tc.payload2, path.Join(targetDir, oldTsDir)) - if err != nil { - t.Errorf("%v: unexpected error determining paths to remove: %v", tc.name, err) - continue - } - - if e, a := tc.expected, actual; !e.Equal(a) { - t.Errorf("%v: unexpected paths to remove:\nexpected: %v\n got: %v", tc.name, e, a) - } - } -} - -func TestWriteOnce(t *testing.T) { - // $1 if you can tell me what this binary is - encodedMysteryBinary := `f0VMRgIBAQAAAAAAAAAAAAIAPgABAAAAeABAAAAAAABAAAAAAAAAAAAAAAAAAAAAAAAAAEAAOAAB -AAAAAAAAAAEAAAAFAAAAAAAAAAAAAAAAAEAAAAAAAAAAQAAAAAAAfQAAAAAAAAB9AAAAAAAAAAAA -IAAAAAAAsDyZDwU=` - - mysteryBinaryBytes := make([]byte, base64.StdEncoding.DecodedLen(len(encodedMysteryBinary))) - numBytes, err := base64.StdEncoding.Decode(mysteryBinaryBytes, []byte(encodedMysteryBinary)) - if err != nil { - t.Fatalf("Unexpected error decoding binary payload: %v", err) - } - - if numBytes != 125 { - t.Fatalf("Unexpected decoded binary size: expected 125, got %v", numBytes) - } - - cases := []struct { - name string - payload map[string]FileProjection - success bool - }{ - { - name: "invalid payload 1", - payload: map[string]FileProjection{ - "foo": {Mode: 0644, Data: []byte("foo")}, - "..bar": {Mode: 0644, Data: []byte("bar")}, - "binary.bin": {Mode: 0644, Data: mysteryBinaryBytes}, - }, - success: false, - }, - { - name: "invalid payload 2", - payload: map[string]FileProjection{ - "foo/../bar": {Mode: 0644, Data: []byte("foo")}, - }, - success: false, - }, - { - name: "basic 1", - payload: map[string]FileProjection{ - "foo": {Mode: 0644, Data: []byte("foo")}, - "bar": {Mode: 0644, Data: []byte("bar")}, - }, - success: true, - }, - { - name: "basic 2", - payload: map[string]FileProjection{ - "binary.bin": {Mode: 0644, Data: mysteryBinaryBytes}, - ".binary.bin": {Mode: 0644, Data: mysteryBinaryBytes}, - }, - success: true, - }, - { - name: "basic mode 1", - payload: map[string]FileProjection{ - "foo": {Mode: 0777, Data: []byte("foo")}, - "bar": {Mode: 0400, Data: []byte("bar")}, - }, - success: true, - }, - { - name: "dotfiles", - payload: map[string]FileProjection{ - "foo": {Mode: 0644, Data: []byte("foo")}, - "bar": {Mode: 0644, Data: []byte("bar")}, - ".dotfile": {Mode: 0644, Data: []byte("dotfile")}, - ".dotfile.file": {Mode: 0644, Data: []byte("dotfile.file")}, - }, - success: true, - }, - { - name: "dotfiles mode", - payload: map[string]FileProjection{ - "foo": {Mode: 0407, Data: []byte("foo")}, - "bar": {Mode: 0440, Data: []byte("bar")}, - ".dotfile": {Mode: 0777, Data: []byte("dotfile")}, - ".dotfile.file": {Mode: 0666, Data: []byte("dotfile.file")}, - }, - success: true, - }, - { - name: "subdirectories 1", - payload: map[string]FileProjection{ - "foo/bar.txt": {Mode: 0644, Data: []byte("foo/bar")}, - "bar/zab.txt": {Mode: 0644, Data: []byte("bar/zab.txt")}, - }, - success: true, - }, - { - name: "subdirectories mode 1", - payload: map[string]FileProjection{ - "foo/bar.txt": {Mode: 0400, Data: []byte("foo/bar")}, - "bar/zab.txt": {Mode: 0644, Data: []byte("bar/zab.txt")}, - }, - success: true, - }, - { - name: "subdirectories 2", - payload: map[string]FileProjection{ - "foo//bar.txt": {Mode: 0644, Data: []byte("foo//bar")}, - "bar///bar/zab.txt": {Mode: 0644, Data: []byte("bar/../bar/zab.txt")}, - }, - success: true, - }, - { - name: "subdirectories 3", - payload: map[string]FileProjection{ - "foo/bar.txt": {Mode: 0644, Data: []byte("foo/bar")}, - "bar/zab.txt": {Mode: 0644, Data: []byte("bar/zab.txt")}, - "foo/blaz/bar.txt": {Mode: 0644, Data: []byte("foo/blaz/bar")}, - "bar/zib/zab.txt": {Mode: 0644, Data: []byte("bar/zib/zab.txt")}, - }, - success: true, - }, - { - name: "kitchen sink", - payload: map[string]FileProjection{ - "foo.log": {Mode: 0644, Data: []byte("foo")}, - "bar.zap": {Mode: 0644, Data: []byte("bar")}, - ".dotfile": {Mode: 0644, Data: []byte("dotfile")}, - "foo/bar.txt": {Mode: 0644, Data: []byte("foo/bar")}, - "bar/zab.txt": {Mode: 0644, Data: []byte("bar/zab.txt")}, - "foo/blaz/bar.txt": {Mode: 0644, Data: []byte("foo/blaz/bar")}, - "bar/zib/zab.txt": {Mode: 0400, Data: []byte("bar/zib/zab.txt")}, - "1/2/3/4/5/6/7/8/9/10/.dotfile.lib": {Mode: 0777, Data: []byte("1-2-3-dotfile")}, - }, - success: true, - }, - } - - for _, tc := range cases { - targetDir, err := utiltesting.MkTmpdir("atomic-write") - if err != nil { - t.Errorf("%v: unexpected error creating tmp dir: %v", tc.name, err) - continue - } - defer os.RemoveAll(targetDir) - - writer := &AtomicWriter{targetDir: targetDir, log: log.TestLogger{T: t}} - err = writer.Write(tc.payload) - if err != nil && tc.success { - t.Errorf("%v: unexpected error writing payload: %v", tc.name, err) - continue - } else if err == nil && !tc.success { - t.Errorf("%v: unexpected success", tc.name) - continue - } else if err != nil { - continue - } - - checkVolumeContents(targetDir, tc.name, tc.payload, t) - } -} - -func TestUpdate(t *testing.T) { - cases := []struct { - name string - first map[string]FileProjection - next map[string]FileProjection - shouldWrite bool - }{ - { - name: "update", - first: map[string]FileProjection{ - "foo": {Mode: 0644, Data: []byte("foo")}, - "bar": {Mode: 0644, Data: []byte("bar")}, - }, - next: map[string]FileProjection{ - "foo": {Mode: 0644, Data: []byte("foo2")}, - "bar": {Mode: 0640, Data: []byte("bar2")}, - }, - shouldWrite: true, - }, - { - name: "no update", - first: map[string]FileProjection{ - "foo": {Mode: 0644, Data: []byte("foo")}, - "bar": {Mode: 0644, Data: []byte("bar")}, - }, - next: map[string]FileProjection{ - "foo": {Mode: 0644, Data: []byte("foo")}, - "bar": {Mode: 0644, Data: []byte("bar")}, - }, - shouldWrite: false, - }, - { - name: "no update 2", - first: map[string]FileProjection{ - "foo/bar.txt": {Mode: 0644, Data: []byte("foo")}, - "bar/zab.txt": {Mode: 0644, Data: []byte("bar")}, - }, - next: map[string]FileProjection{ - "foo/bar.txt": {Mode: 0644, Data: []byte("foo")}, - "bar/zab.txt": {Mode: 0644, Data: []byte("bar")}, - }, - shouldWrite: false, - }, - { - name: "add 1", - first: map[string]FileProjection{ - "foo/bar.txt": {Mode: 0644, Data: []byte("foo")}, - "bar/zab.txt": {Mode: 0644, Data: []byte("bar")}, - }, - next: map[string]FileProjection{ - "foo/bar.txt": {Mode: 0644, Data: []byte("foo")}, - "bar/zab.txt": {Mode: 0644, Data: []byte("bar")}, - "blu/zip.txt": {Mode: 0644, Data: []byte("zip")}, - }, - shouldWrite: true, - }, - { - name: "add 2", - first: map[string]FileProjection{ - "foo/bar.txt": {Mode: 0644, Data: []byte("foo")}, - "bar/zab.txt": {Mode: 0644, Data: []byte("bar")}, - }, - next: map[string]FileProjection{ - "foo/bar.txt": {Mode: 0644, Data: []byte("foo")}, - "bar/zab.txt": {Mode: 0644, Data: []byte("bar")}, - "blu/two/2/3/4/5/zip.txt": {Mode: 0644, Data: []byte("zip")}, - }, - shouldWrite: true, - }, - { - name: "add 3", - first: map[string]FileProjection{ - "foo/bar.txt": {Mode: 0644, Data: []byte("foo")}, - "bar/zab.txt": {Mode: 0644, Data: []byte("bar")}, - }, - next: map[string]FileProjection{ - "foo/bar.txt": {Mode: 0644, Data: []byte("foo")}, - "bar/zab.txt": {Mode: 0644, Data: []byte("bar")}, - "bar/2/3/4/5/zip.txt": {Mode: 0644, Data: []byte("zip")}, - }, - shouldWrite: true, - }, - { - name: "delete 1", - first: map[string]FileProjection{ - "foo/bar.txt": {Mode: 0644, Data: []byte("foo")}, - "bar/zab.txt": {Mode: 0644, Data: []byte("bar")}, - }, - next: map[string]FileProjection{ - "foo/bar.txt": {Mode: 0644, Data: []byte("foo")}, - }, - shouldWrite: true, - }, - { - name: "delete 2", - first: map[string]FileProjection{ - "foo/bar.txt": {Mode: 0644, Data: []byte("foo")}, - "bar/1/2/3/zab.txt": {Mode: 0644, Data: []byte("bar")}, - }, - next: map[string]FileProjection{ - "foo/bar.txt": {Mode: 0644, Data: []byte("foo")}, - }, - shouldWrite: true, - }, - { - name: "delete 3", - first: map[string]FileProjection{ - "foo/bar.txt": {Mode: 0644, Data: []byte("foo")}, - "bar/1/2/sip.txt": {Mode: 0644, Data: []byte("sip")}, - "bar/1/2/3/zab.txt": {Mode: 0644, Data: []byte("bar")}, - }, - next: map[string]FileProjection{ - "foo/bar.txt": {Mode: 0644, Data: []byte("foo")}, - "bar/1/2/sip.txt": {Mode: 0644, Data: []byte("sip")}, - }, - shouldWrite: true, - }, - { - name: "delete 4", - first: map[string]FileProjection{ - "foo/bar.txt": {Mode: 0644, Data: []byte("foo")}, - "bar/1/2/sip.txt": {Mode: 0644, Data: []byte("sip")}, - "bar/1/2/3/4/5/6zab.txt": {Mode: 0644, Data: []byte("bar")}, - }, - next: map[string]FileProjection{ - "foo/bar.txt": {Mode: 0644, Data: []byte("foo")}, - "bar/1/2/sip.txt": {Mode: 0644, Data: []byte("sip")}, - }, - shouldWrite: true, - }, - { - name: "delete all", - first: map[string]FileProjection{ - "foo/bar.txt": {Mode: 0644, Data: []byte("foo")}, - "bar/1/2/sip.txt": {Mode: 0644, Data: []byte("sip")}, - "bar/1/2/3/4/5/6zab.txt": {Mode: 0644, Data: []byte("bar")}, - }, - next: map[string]FileProjection{}, - shouldWrite: true, - }, - { - name: "add and delete 1", - first: map[string]FileProjection{ - "foo/bar.txt": {Mode: 0644, Data: []byte("foo")}, - }, - next: map[string]FileProjection{ - "bar/baz.txt": {Mode: 0644, Data: []byte("baz")}, - }, - shouldWrite: true, - }, - } - - for _, tc := range cases { - targetDir, err := utiltesting.MkTmpdir("atomic-write") - if err != nil { - t.Errorf("%v: unexpected error creating tmp dir: %v", tc.name, err) - continue - } - defer os.RemoveAll(targetDir) - - writer := &AtomicWriter{targetDir: targetDir, log: log.TestLogger{T: t}} - - err = writer.Write(tc.first) - if err != nil { - t.Errorf("%v: unexpected error writing: %v", tc.name, err) - continue - } - - checkVolumeContents(targetDir, tc.name, tc.first, t) - if !tc.shouldWrite { - continue - } - - err = writer.Write(tc.next) - if err != nil { - if tc.shouldWrite { - t.Errorf("%v: unexpected error writing: %v", tc.name, err) - continue - } - } else if !tc.shouldWrite { - t.Errorf("%v: unexpected success", tc.name) - continue - } - - checkVolumeContents(targetDir, tc.name, tc.next, t) - } -} - -func TestMultipleUpdates(t *testing.T) { - cases := []struct { - name string - payloads []map[string]FileProjection - }{ - { - name: "update 1", - payloads: []map[string]FileProjection{ - { - "foo": {Mode: 0644, Data: []byte("foo")}, - "bar": {Mode: 0644, Data: []byte("bar")}, - }, - { - "foo": {Mode: 0400, Data: []byte("foo2")}, - "bar": {Mode: 0400, Data: []byte("bar2")}, - }, - { - "foo": {Mode: 0600, Data: []byte("foo3")}, - "bar": {Mode: 0600, Data: []byte("bar3")}, - }, - }, - }, - { - name: "update 2", - payloads: []map[string]FileProjection{ - { - "foo/bar.txt": {Mode: 0644, Data: []byte("foo/bar")}, - "bar/zab.txt": {Mode: 0644, Data: []byte("bar/zab.txt")}, - }, - { - "foo/bar.txt": {Mode: 0644, Data: []byte("foo/bar2")}, - "bar/zab.txt": {Mode: 0400, Data: []byte("bar/zab.txt2")}, - }, - }, - }, - { - name: "clear sentinel", - payloads: []map[string]FileProjection{ - { - "foo": {Mode: 0644, Data: []byte("foo")}, - "bar": {Mode: 0644, Data: []byte("bar")}, - }, - { - "foo": {Mode: 0644, Data: []byte("foo2")}, - "bar": {Mode: 0644, Data: []byte("bar2")}, - }, - { - "foo": {Mode: 0644, Data: []byte("foo3")}, - "bar": {Mode: 0644, Data: []byte("bar3")}, - }, - { - "foo": {Mode: 0644, Data: []byte("foo4")}, - "bar": {Mode: 0644, Data: []byte("bar4")}, - }, - }, - }, - { - name: "subdirectories 2", - payloads: []map[string]FileProjection{ - { - "foo/bar.txt": {Mode: 0644, Data: []byte("foo/bar")}, - "bar/zab.txt": {Mode: 0644, Data: []byte("bar/zab.txt")}, - "foo/blaz/bar.txt": {Mode: 0644, Data: []byte("foo/blaz/bar")}, - "bar/zib/zab.txt": {Mode: 0644, Data: []byte("bar/zib/zab.txt")}, - }, - { - "foo/bar.txt": {Mode: 0644, Data: []byte("foo/bar2")}, - "bar/zab.txt": {Mode: 0644, Data: []byte("bar/zab.txt2")}, - "foo/blaz/bar.txt": {Mode: 0644, Data: []byte("foo/blaz/bar2")}, - "bar/zib/zab.txt": {Mode: 0644, Data: []byte("bar/zib/zab.txt2")}, - }, - }, - }, - { - name: "add 1", - payloads: []map[string]FileProjection{ - { - "foo/bar.txt": {Mode: 0644, Data: []byte("foo/bar")}, - "bar//zab.txt": {Mode: 0644, Data: []byte("bar/zab.txt")}, - "foo/blaz/bar.txt": {Mode: 0644, Data: []byte("foo/blaz/bar")}, - "bar/zib////zib/zab.txt": {Mode: 0644, Data: []byte("bar/zib/zab.txt")}, - }, - { - "foo/bar.txt": {Mode: 0644, Data: []byte("foo/bar2")}, - "bar/zab.txt": {Mode: 0644, Data: []byte("bar/zab.txt2")}, - "foo/blaz/bar.txt": {Mode: 0644, Data: []byte("foo/blaz/bar2")}, - "bar/zib/zab.txt": {Mode: 0644, Data: []byte("bar/zib/zab.txt2")}, - "add/new/keys.txt": {Mode: 0644, Data: []byte("addNewKeys")}, - }, - }, - }, - { - name: "add 2", - payloads: []map[string]FileProjection{ - { - "foo/bar.txt": {Mode: 0644, Data: []byte("foo/bar2")}, - "bar/zab.txt": {Mode: 0644, Data: []byte("bar/zab.txt2")}, - "foo/blaz/bar.txt": {Mode: 0644, Data: []byte("foo/blaz/bar2")}, - "bar/zib/zab.txt": {Mode: 0644, Data: []byte("bar/zib/zab.txt2")}, - "add/new/keys.txt": {Mode: 0644, Data: []byte("addNewKeys")}, - }, - { - "foo/bar.txt": {Mode: 0644, Data: []byte("foo/bar2")}, - "bar/zab.txt": {Mode: 0644, Data: []byte("bar/zab.txt2")}, - "foo/blaz/bar.txt": {Mode: 0644, Data: []byte("foo/blaz/bar2")}, - "bar/zib/zab.txt": {Mode: 0644, Data: []byte("bar/zib/zab.txt2")}, - "add/new/keys.txt": {Mode: 0644, Data: []byte("addNewKeys")}, - "add/new/keys2.txt": {Mode: 0644, Data: []byte("addNewKeys2")}, - "add/new/keys3.txt": {Mode: 0644, Data: []byte("addNewKeys3")}, - }, - }, - }, - { - name: "remove 1", - payloads: []map[string]FileProjection{ - { - "foo/bar.txt": {Mode: 0644, Data: []byte("foo/bar")}, - "bar//zab.txt": {Mode: 0644, Data: []byte("bar/zab.txt")}, - "foo/blaz/bar.txt": {Mode: 0644, Data: []byte("foo/blaz/bar")}, - "zip/zap/zup/fop.txt": {Mode: 0644, Data: []byte("zip/zap/zup/fop.txt")}, - }, - { - "foo/bar.txt": {Mode: 0644, Data: []byte("foo/bar2")}, - "bar/zab.txt": {Mode: 0644, Data: []byte("bar/zab.txt2")}, - }, - { - "foo/bar.txt": {Mode: 0644, Data: []byte("foo/bar")}, - }, - }, - }, - } - - for _, tc := range cases { - targetDir, err := utiltesting.MkTmpdir("atomic-write") - if err != nil { - t.Errorf("%v: unexpected error creating tmp dir: %v", tc.name, err) - continue - } - defer os.RemoveAll(targetDir) - - writer := &AtomicWriter{targetDir: targetDir, log: log.TestLogger{T: t}} - - for _, payload := range tc.payloads { - writer.Write(payload) - - checkVolumeContents(targetDir, tc.name, payload, t) - } - } -} - -func checkVolumeContents(targetDir, tcName string, payload map[string]FileProjection, t *testing.T) { - dataDirPath := path.Join(targetDir, dataDirName) - // use filepath.Walk to reconstruct the payload, then deep equal - observedPayload := make(map[string]FileProjection) - visitor := func(path string, info os.FileInfo, err error) error { - if info.IsDir() { - return nil - } - - relativePath := strings.TrimPrefix(path, dataDirPath) - relativePath = strings.TrimPrefix(relativePath, "/") - if strings.HasPrefix(relativePath, "..") { - return nil - } - - content, err := ioutil.ReadFile(path) - if err != nil { - return err - } - fileInfo, err := os.Stat(path) - if err != nil { - return err - } - mode := int32(fileInfo.Mode()) - - observedPayload[relativePath] = FileProjection{Data: content, Mode: mode} - - return nil - } - - d, err := ioutil.ReadDir(targetDir) - if err != nil { - t.Errorf("Unable to read dir %v: %v", targetDir, err) - return - } - for _, info := range d { - if strings.HasPrefix(info.Name(), "..") { - continue - } - if info.Mode()&os.ModeSymlink != 0 { - p := path.Join(targetDir, info.Name()) - actual, err := os.Readlink(p) - if err != nil { - t.Errorf("Unable to read symlink %v: %v", p, err) - continue - } - if err := filepath.Walk(path.Join(targetDir, actual), visitor); err != nil { - t.Errorf("%v: unexpected error walking directory: %v", tcName, err) - } - } - } - - cleanPathPayload := make(map[string]FileProjection, len(payload)) - for k, v := range payload { - cleanPathPayload[filepath.Clean(k)] = v - } - - if !reflect.DeepEqual(cleanPathPayload, observedPayload) { - t.Errorf("%v: payload and observed payload do not match.", tcName) - } -} - -func TestValidatePayload(t *testing.T) { - maxPath := strings.Repeat("a", maxPathLength+1) - - cases := []struct { - name string - payload map[string]FileProjection - expected sets.String - valid bool - }{ - { - name: "valid payload", - payload: map[string]FileProjection{ - "foo": {}, - "bar": {}, - }, - valid: true, - expected: sets.NewString("foo", "bar"), - }, - { - name: "payload with path length > 4096 is invalid", - payload: map[string]FileProjection{ - maxPath: {}, - }, - valid: false, - }, - { - name: "payload with absolute path is invalid", - payload: map[string]FileProjection{ - "/dev/null": {}, - }, - valid: false, - }, - { - name: "payload with reserved path is invalid", - payload: map[string]FileProjection{ - "..sneaky.txt": {}, - }, - valid: false, - }, - { - name: "payload with doubledot path is invalid", - payload: map[string]FileProjection{ - "foo/../etc/password": {}, - }, - valid: false, - }, - { - name: "payload with empty path is invalid", - payload: map[string]FileProjection{ - "": {}, - }, - valid: false, - }, - { - name: "payload with unclean path should be cleaned", - payload: map[string]FileProjection{ - "foo////bar": {}, - }, - valid: true, - expected: sets.NewString("foo/bar"), - }, - } - getPayloadPaths := func(payload map[string]FileProjection) sets.String { - paths := sets.NewString() - for path := range payload { - paths.Insert(path) - } - return paths - } - - for _, tc := range cases { - real, err := validatePayload(tc.payload) - if !tc.valid && err == nil { - t.Errorf("%v: unexpected success", tc.name) - } - - if tc.valid { - if err != nil { - t.Errorf("%v: unexpected failure: %v", tc.name, err) - continue - } - - realPaths := getPayloadPaths(real) - if !realPaths.Equal(tc.expected) { - t.Errorf("%v: unexpected payload paths: %v is not equal to %v", tc.name, realPaths, tc.expected) - } - } - - } -} - -func TestCreateUserVisibleFiles(t *testing.T) { - cases := []struct { - name string - payload map[string]FileProjection - expected map[string]string - }{ - { - name: "simple path", - payload: map[string]FileProjection{ - "foo": {}, - "bar": {}, - }, - expected: map[string]string{ - "foo": "..data/foo", - "bar": "..data/bar", - }, - }, - { - name: "simple nested path", - payload: map[string]FileProjection{ - "foo/bar": {}, - "foo/bar/txt": {}, - "bar/txt": {}, - }, - expected: map[string]string{ - "foo": "..data/foo", - "bar": "..data/bar", - }, - }, - { - name: "unclean nested path", - payload: map[string]FileProjection{ - "./bar": {}, - "foo///bar": {}, - }, - expected: map[string]string{ - "bar": "..data/bar", - "foo": "..data/foo", - }, - }, - } - - for _, tc := range cases { - targetDir, err := utiltesting.MkTmpdir("atomic-write") - if err != nil { - t.Errorf("%v: unexpected error creating tmp dir: %v", tc.name, err) - continue - } - defer os.RemoveAll(targetDir) - - dataDirPath := path.Join(targetDir, dataDirName) - err = os.MkdirAll(dataDirPath, 0755) - if err != nil { - t.Fatalf("%v: unexpected error creating data path: %v", tc.name, err) - } - - writer := &AtomicWriter{targetDir: targetDir, log: log.TestLogger{T: t}} - payload, err := validatePayload(tc.payload) - if err != nil { - t.Fatalf("%v: unexpected error validating payload: %v", tc.name, err) - } - err = writer.createUserVisibleFiles(payload) - if err != nil { - t.Fatalf("%v: unexpected error creating visible files: %v", tc.name, err) - } - - for subpath, expectedDest := range tc.expected { - visiblePath := path.Join(targetDir, subpath) - destination, err := os.Readlink(visiblePath) - if err != nil && os.IsNotExist(err) { - t.Fatalf("%v: visible symlink does not exist: %v", tc.name, visiblePath) - } else if err != nil { - t.Fatalf("%v: unable to read symlink %v: %v", tc.name, dataDirPath, err) - } - - if expectedDest != destination { - t.Fatalf("%v: symlink destination %q not same with expected data dir %q", tc.name, destination, expectedDest) - } - } - } -} diff --git a/pkg/webhook/internal/cert/writer/certwriter.go b/pkg/webhook/internal/cert/writer/certwriter.go deleted file mode 100644 index ed6b51107c..0000000000 --- a/pkg/webhook/internal/cert/writer/certwriter.go +++ /dev/null @@ -1,137 +0,0 @@ -/* -Copyright 2018 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package writer - -import ( - "crypto/tls" - "crypto/x509" - "encoding/pem" - "errors" - "time" - - "k8s.io/apimachinery/pkg/runtime" - "sigs.k8s.io/controller-runtime/pkg/webhook/internal/cert/generator" -) - -const ( - // CAKeyName is the name of the CA private key - CAKeyName = "ca-key.pem" - // CACertName is the name of the CA certificate - CACertName = "ca-cert.pem" - // ServerKeyName is the name of the server private key - ServerKeyName = "key.pem" - // ServerCertName is the name of the serving certificate - ServerCertName = "cert.pem" -) - -// CertWriter provides method to handle webhooks. -type CertWriter interface { - // EnsureCert provisions the cert for the webhookClientConfig. - EnsureCert(dnsName string) (*generator.Artifacts, bool, error) - // Inject injects the necessary information given the objects. - // It supports MutatingWebhookConfiguration and ValidatingWebhookConfiguration. - Inject(objs ...runtime.Object) error -} - -// handleCommon ensures the given webhook has a proper certificate. -// It uses the given certReadWriter to read and (or) write the certificate. -func handleCommon(dnsName string, ch certReadWriter) (*generator.Artifacts, bool, error) { - if len(dnsName) == 0 { - return nil, false, errors.New("dnsName should not be empty") - } - if ch == nil { - return nil, false, errors.New("certReaderWriter should not be nil") - } - - certs, changed, err := createIfNotExists(ch) - if err != nil { - return nil, changed, err - } - - // Recreate the cert if it's invalid. - valid := validCert(certs, dnsName) - if !valid { - log.Info("cert is invalid or expiring, regenerating a new one") - certs, err = ch.overwrite() - if err != nil { - return nil, false, err - } - changed = true - } - return certs, changed, nil -} - -func createIfNotExists(ch certReadWriter) (*generator.Artifacts, bool, error) { - // Try to read first - certs, err := ch.read() - if isNotFound(err) { - // Create if not exists - certs, err = ch.write() - switch { - // This may happen if there is another racer. - case isAlreadyExists(err): - certs, err = ch.read() - return certs, true, err - default: - return certs, true, err - } - } - return certs, false, err -} - -// certReadWriter provides methods for reading and writing certificates. -type certReadWriter interface { - // read reads a webhook name and returns the certs for it. - read() (*generator.Artifacts, error) - // write writes the certs and return the certs it wrote. - write() (*generator.Artifacts, error) - // overwrite overwrites the existing certs and return the certs it wrote. - overwrite() (*generator.Artifacts, error) -} - -func validCert(certs *generator.Artifacts, dnsName string) bool { - if certs == nil { - return false - } - - // Verify key and cert are valid pair - _, err := tls.X509KeyPair(certs.Cert, certs.Key) - if err != nil { - return false - } - - // Verify cert is good for desired DNS name and signed by CA and will be valid for desired period of time. - pool := x509.NewCertPool() - if !pool.AppendCertsFromPEM(certs.CACert) { - return false - } - block, _ := pem.Decode([]byte(certs.Cert)) - if block == nil { - return false - } - cert, err := x509.ParseCertificate(block.Bytes) - if err != nil { - return false - } - ops := x509.VerifyOptions{ - DNSName: dnsName, - Roots: pool, - CurrentTime: time.Now().AddDate(0, 6, 0), - } - _, err = cert.Verify(ops) - return err == nil -} diff --git a/pkg/webhook/internal/cert/writer/certwriter_test.go b/pkg/webhook/internal/cert/writer/certwriter_test.go deleted file mode 100644 index bfb2a65fad..0000000000 --- a/pkg/webhook/internal/cert/writer/certwriter_test.go +++ /dev/null @@ -1,360 +0,0 @@ -/* -Copyright 2018 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package writer - -import ( - goerrors "errors" - - . "github.com/onsi/ginkgo" - . "github.com/onsi/gomega" - - "k8s.io/apimachinery/pkg/api/errors" - "k8s.io/apimachinery/pkg/runtime/schema" - "sigs.k8s.io/controller-runtime/pkg/webhook/internal/cert/generator" -) - -var certs1, certs2 *generator.Artifacts - -func init() { - cn1 := "example.com" - cn2 := "test-service.test-svc-namespace.svc" - cp1 := generator.SelfSignedCertGenerator{} - cp2 := generator.SelfSignedCertGenerator{} - certs1, _ = cp1.Generate(cn1) - certs2, _ = cp2.Generate(cn2) -} - -type fakeCertReadWriter struct { - numReadCalled int - readCertAndErr []certAndErr - - numWriteCalled int - writeCertAndErr []certAndErr - - numOverwriteCalled int - overwriteCertAndErr []certAndErr -} - -type certAndErr struct { - cert *generator.Artifacts - err error -} - -var _ certReadWriter = &fakeCertReadWriter{} - -func (f *fakeCertReadWriter) read() (*generator.Artifacts, error) { - defer func() { f.numReadCalled++ }() - - if len(f.readCertAndErr) <= f.numReadCalled { - return &generator.Artifacts{}, nil - } - certAndErr := f.readCertAndErr[f.numReadCalled] - return certAndErr.cert, certAndErr.err -} - -func (f *fakeCertReadWriter) write() (*generator.Artifacts, error) { - defer func() { f.numWriteCalled++ }() - - if len(f.writeCertAndErr) <= f.numWriteCalled { - return &generator.Artifacts{}, nil - } - certAndErr := f.writeCertAndErr[f.numWriteCalled] - return certAndErr.cert, certAndErr.err -} - -func (f *fakeCertReadWriter) overwrite() (*generator.Artifacts, error) { - defer func() { f.numOverwriteCalled++ }() - - if len(f.overwriteCertAndErr) <= f.numOverwriteCalled { - return &generator.Artifacts{}, nil - } - certAndErr := f.overwriteCertAndErr[f.numOverwriteCalled] - return certAndErr.cert, certAndErr.err -} - -var _ = Describe("handleCommon", func() { - var cert *generator.Artifacts - var invalidCert *generator.Artifacts - dnsName := "example.com" - - BeforeEach(func(done Done) { - cert = &generator.Artifacts{ - CACert: []byte(certs1.CACert), - Cert: []byte(certs1.Cert), - Key: []byte(certs1.Key), - } - invalidCert = &generator.Artifacts{ - CACert: []byte(`CACertBytes`), - Cert: []byte(`CertBytes`), - Key: []byte(`KeyBytes`), - } - close(done) - }) - - Context("when DNS name is empty", func() { - It("should return an error", func() { - certrw := &fakeCertReadWriter{} - _, _, err := handleCommon("", certrw) - Expect(err).To(MatchError("dnsName should not be empty")) - }) - }) - - Context("when certReadWriter is nil", func() { - It("should return an error", func() { - _, _, err := handleCommon(dnsName, nil) - Expect(err).To(MatchError("certReaderWriter should not be nil")) - }) - }) - - Context("cert doesn't exist", func() { - It("should return no error on successful write", func() { - certrw := &fakeCertReadWriter{ - readCertAndErr: []certAndErr{ - { - err: notFoundError{errors.NewNotFound(schema.GroupResource{}, "foo")}, - }, - }, - writeCertAndErr: []certAndErr{ - { - cert: cert, - }, - }, - } - - certs, changed, err := handleCommon(dnsName, certrw) - Expect(err).NotTo(HaveOccurred()) - Expect(certrw.numReadCalled).To(Equal(1)) - Expect(certrw.numWriteCalled).To(Equal(1)) - Expect(certrw.numOverwriteCalled).To(Equal(0)) - Expect(changed).To(BeTrue()) - Expect(certs).To(Equal(cert)) - }) - - It("should return the error on failed write", func() { - certrw := &fakeCertReadWriter{ - readCertAndErr: []certAndErr{ - { - err: notFoundError{errors.NewNotFound(schema.GroupResource{}, "foo")}, - }, - }, - writeCertAndErr: []certAndErr{ - { - err: goerrors.New("failed to write"), - }, - }, - } - - _, _, err := handleCommon(dnsName, certrw) - Expect(err).To(MatchError("failed to write")) - Expect(certrw.numReadCalled).To(Equal(1)) - Expect(certrw.numWriteCalled).To(Equal(1)) - Expect(certrw.numOverwriteCalled).To(Equal(0)) - }) - }) - - Context("valid cert exist", func() { - It("should return no error on successful read", func() { - certrw := &fakeCertReadWriter{ - readCertAndErr: []certAndErr{ - { - cert: cert, - }, - }, - } - - certs, changed, err := handleCommon(dnsName, certrw) - Expect(err).NotTo(HaveOccurred()) - Expect(certrw.numReadCalled).To(Equal(1)) - Expect(certrw.numWriteCalled).To(Equal(0)) - Expect(certrw.numOverwriteCalled).To(Equal(0)) - Expect(changed).To(BeFalse()) - Expect(certs).To(Equal(cert)) - }) - - It("should return the error on failed read", func() { - certrw := &fakeCertReadWriter{ - readCertAndErr: []certAndErr{ - { - err: goerrors.New("failed to read"), - }, - }, - } - - _, _, err := handleCommon(dnsName, certrw) - Expect(err).To(MatchError("failed to read")) - Expect(certrw.numReadCalled).To(Equal(1)) - Expect(certrw.numWriteCalled).To(Equal(0)) - Expect(certrw.numOverwriteCalled).To(Equal(0)) - }) - }) - - Context("invalid cert exist", func() { - It("should replace the empty cert with a new one", func() { - certrw := &fakeCertReadWriter{ - readCertAndErr: []certAndErr{ - { - cert: nil, - }, - }, - overwriteCertAndErr: []certAndErr{ - { - cert: cert, - }, - }, - } - - certs, changed, err := handleCommon(dnsName, certrw) - Expect(err).NotTo(HaveOccurred()) - Expect(certrw.numReadCalled).To(Equal(1)) - Expect(certrw.numWriteCalled).To(Equal(0)) - Expect(certrw.numOverwriteCalled).To(Equal(1)) - Expect(changed).To(BeTrue()) - Expect(certs).To(Equal(cert)) - }) - - It("should return no error on successful overwrite", func() { - certrw := &fakeCertReadWriter{ - readCertAndErr: []certAndErr{ - { - cert: invalidCert, - }, - }, - overwriteCertAndErr: []certAndErr{ - { - cert: cert, - }, - }, - } - - certs, changed, err := handleCommon(dnsName, certrw) - Expect(err).NotTo(HaveOccurred()) - Expect(certrw.numReadCalled).To(Equal(1)) - Expect(certrw.numWriteCalled).To(Equal(0)) - Expect(certrw.numOverwriteCalled).To(Equal(1)) - Expect(changed).To(BeTrue()) - Expect(certs).To(Equal(cert)) - }) - - It("should return the error on failed overwrite", func() { - certrw := &fakeCertReadWriter{ - readCertAndErr: []certAndErr{ - { - cert: invalidCert, - }, - }, - overwriteCertAndErr: []certAndErr{ - { - err: goerrors.New("failed to overwrite"), - }, - }, - } - - _, _, err := handleCommon(dnsName, certrw) - Expect(err).To(MatchError("failed to overwrite")) - Expect(certrw.numReadCalled).To(Equal(1)) - Expect(certrw.numOverwriteCalled).To(Equal(1)) - }) - }) - - Context("racing", func() { - It("should return the valid cert created by the racing one", func() { - certrw := &fakeCertReadWriter{ - readCertAndErr: []certAndErr{ - { - err: notFoundError{errors.NewNotFound(schema.GroupResource{}, "foo")}, - }, - { - cert: cert, - }, - }, - writeCertAndErr: []certAndErr{ - { - err: alreadyExistError{errors.NewAlreadyExists(schema.GroupResource{}, "foo")}, - }, - }, - } - - certs, changed, err := handleCommon(dnsName, certrw) - Expect(err).NotTo(HaveOccurred()) - Expect(certrw.numReadCalled).To(Equal(2)) - Expect(certrw.numWriteCalled).To(Equal(1)) - Expect(changed).To(BeTrue()) - Expect(certs).To(Equal(cert)) - }) - - It("should return the error if failed to read the cert created by the racing one", func() { - certrw := &fakeCertReadWriter{ - readCertAndErr: []certAndErr{ - { - err: notFoundError{errors.NewNotFound(schema.GroupResource{}, "foo")}, - }, - { - err: goerrors.New("failed to read"), - }, - }, - writeCertAndErr: []certAndErr{ - { - err: alreadyExistError{errors.NewAlreadyExists(schema.GroupResource{}, "foo")}, - }, - }, - } - - _, _, err := handleCommon(dnsName, certrw) - Expect(err).To(MatchError("failed to read")) - Expect(certrw.numReadCalled).To(Equal(2)) - Expect(certrw.numWriteCalled).To(Equal(1)) - }) - }) -}) - -var _ = Describe("validate cert", func() { - Context("invalid pair", func() { - It("should detect it", func() { - certs := generator.Artifacts{ - CACert: certs1.CACert, - Cert: certs1.Cert, - Key: certs2.Key, - } - valid := validCert(&certs, "example.com") - Expect(valid).To(BeFalse()) - }) - }) - - Context("CA not matching", func() { - It("should detect it", func() { - certs := generator.Artifacts{ - CACert: certs2.CACert, - Cert: certs1.Cert, - Key: certs1.Key, - } - valid := validCert(&certs, "example.com") - Expect(valid).To(BeFalse()) - }) - }) - - Context("DNS name not matching", func() { - It("should detect it", func() { - certs := generator.Artifacts{ - CACert: certs1.CACert, - Cert: certs1.Cert, - Key: certs1.Key, - } - valid := validCert(&certs, "foo.com") - Expect(valid).To(BeFalse()) - }) - }) -}) diff --git a/pkg/webhook/internal/cert/writer/doc.go b/pkg/webhook/internal/cert/writer/doc.go deleted file mode 100644 index 425626606c..0000000000 --- a/pkg/webhook/internal/cert/writer/doc.go +++ /dev/null @@ -1,64 +0,0 @@ -/* -Copyright 2018 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -/* -Package writer provides method to provision and persist the certificates. - -It will create the certificates if they don't exist. -It will ensure the certificates are valid and not expiring. If not, it will recreate them. - -Create a CertWriter that can write the certificate to secret - - writer, err := NewSecretCertWriter(SecretCertWriterOptions{ - Secret: types.NamespacedName{Namespace: "foo", Name: "bar"}, - Client: client, - }) - if err != nil { - // handler error - } - -Create a CertWriter that can write the certificate to the filesystem. - - writer, err := NewFSCertWriter(FSCertWriterOptions{ - Path: "path/to/cert/", - }) - if err != nil { - // handler error - } - -Provision the certificates using the CertWriter. The certificate will be available in the desired secret or -the desired path. - - // writer can be either one of the CertWriters created above - certs, changed, err := writer.EnsureCerts("admissionwebhook.k8s.io", false) - if err != nil { - // handler error - } - -Inject necessary information given the objects. - - err = writer.Inject(objs...) - if err != nil { - // handler error - } -*/ -package writer - -import ( - logf "sigs.k8s.io/controller-runtime/pkg/internal/log" -) - -var log = logf.RuntimeLog.WithName("admission").WithName("cert").WithName("writer") diff --git a/pkg/webhook/internal/cert/writer/error.go b/pkg/webhook/internal/cert/writer/error.go deleted file mode 100644 index 4f98e8cd05..0000000000 --- a/pkg/webhook/internal/cert/writer/error.go +++ /dev/null @@ -1,43 +0,0 @@ -/* -Copyright 2018 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package writer - -type notFoundError struct { - err error -} - -func (e notFoundError) Error() string { - return e.err.Error() -} - -func isNotFound(err error) bool { - _, ok := err.(notFoundError) - return ok -} - -type alreadyExistError struct { - err error -} - -func (e alreadyExistError) Error() string { - return e.err.Error() -} - -func isAlreadyExists(err error) bool { - _, ok := err.(alreadyExistError) - return ok -} diff --git a/pkg/webhook/internal/cert/writer/fs.go b/pkg/webhook/internal/cert/writer/fs.go deleted file mode 100644 index 6a4ef16da8..0000000000 --- a/pkg/webhook/internal/cert/writer/fs.go +++ /dev/null @@ -1,216 +0,0 @@ -/* -Copyright 2018 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package writer - -import ( - "errors" - "fmt" - "io/ioutil" - "os" - "path" - - "k8s.io/apimachinery/pkg/runtime" - "sigs.k8s.io/controller-runtime/pkg/webhook/internal/cert/generator" - "sigs.k8s.io/controller-runtime/pkg/webhook/internal/cert/writer/atomic" -) - -// fsCertWriter provisions the certificate by reading and writing to the filesystem. -type fsCertWriter struct { - // dnsName is the DNS name that the certificate is for. - dnsName string - - *FSCertWriterOptions -} - -// FSCertWriterOptions are options for constructing a FSCertWriter. -type FSCertWriterOptions struct { - // certGenerator generates the certificates. - CertGenerator generator.CertGenerator - // path is the directory that the certificate and private key and CA certificate will be written. - Path string -} - -var _ CertWriter = &fsCertWriter{} - -func (ops *FSCertWriterOptions) setDefaults() { - if ops.CertGenerator == nil { - ops.CertGenerator = &generator.SelfSignedCertGenerator{} - } -} - -func (ops *FSCertWriterOptions) validate() error { - if len(ops.Path) == 0 { - return errors.New("path must be set in FSCertWriterOptions") - } - return nil -} - -// NewFSCertWriter constructs a CertWriter that persists the certificate on filesystem. -func NewFSCertWriter(ops FSCertWriterOptions) (CertWriter, error) { - ops.setDefaults() - err := ops.validate() - if err != nil { - return nil, err - } - return &fsCertWriter{ - FSCertWriterOptions: &ops, - }, nil -} - -// EnsureCert provisions certificates for a webhookClientConfig by writing the certificates in the filesystem. -func (f *fsCertWriter) EnsureCert(dnsName string) (*generator.Artifacts, bool, error) { - // create or refresh cert and write it to fs - f.dnsName = dnsName - return handleCommon(f.dnsName, f) -} - -func (f *fsCertWriter) write() (*generator.Artifacts, error) { - return f.doWrite() -} - -func (f *fsCertWriter) overwrite() (*generator.Artifacts, error) { - return f.doWrite() -} - -func (f *fsCertWriter) doWrite() (*generator.Artifacts, error) { - certs, err := f.CertGenerator.Generate(f.dnsName) - if err != nil { - return nil, err - } - - // AtomicWriter's algorithm only manages files using symbolic link. - // If a file is not a symbolic link, will ignore the update for it. - // We want to cleanup for AtomicWriter by removing old files that are not symbolic links. - err = prepareToWrite(f.Path) - if err != nil { - return nil, err - } - - aw, err := atomic.NewAtomicWriter(f.Path, log.WithName("atomic-writer"). - WithValues("task", "processing webhook")) - if err != nil { - return nil, err - } - err = aw.Write(certToProjectionMap(certs)) - return certs, err -} - -// prepareToWrite ensures it directory is compatible with the atomic.Writer library. -func prepareToWrite(dir string) error { - _, err := os.Stat(dir) - switch { - case os.IsNotExist(err): - log.Info("cert directory doesn't exist, creating", "directory", dir) - // TODO: figure out if we can reduce the permission. (Now it's 0777) - err = os.MkdirAll(dir, 0777) - if err != nil { - return fmt.Errorf("can't create dir: %v", dir) - } - case err != nil: - return err - } - - filenames := []string{CAKeyName, CACertName, ServerCertName, ServerKeyName} - for _, f := range filenames { - abspath := path.Join(dir, f) - _, err := os.Stat(abspath) - if os.IsNotExist(err) { - continue - } else if err != nil { - log.Error(err, "unable to stat file", "file", abspath) - } - _, err = os.Readlink(abspath) - // if it's not a symbolic link - if err != nil { - err = os.Remove(abspath) - if err != nil { - log.Error(err, "unable to remove old file", "file", abspath) - } - } - } - return nil -} - -func (f *fsCertWriter) read() (*generator.Artifacts, error) { - if err := ensureExist(f.Path); err != nil { - return nil, err - } - caKeyBytes, err := ioutil.ReadFile(path.Join(f.Path, CAKeyName)) - if err != nil { - return nil, err - } - caCertBytes, err := ioutil.ReadFile(path.Join(f.Path, CACertName)) - if err != nil { - return nil, err - } - certBytes, err := ioutil.ReadFile(path.Join(f.Path, ServerCertName)) - if err != nil { - return nil, err - } - keyBytes, err := ioutil.ReadFile(path.Join(f.Path, ServerKeyName)) - if err != nil { - return nil, err - } - return &generator.Artifacts{ - CAKey: caKeyBytes, - CACert: caCertBytes, - Cert: certBytes, - Key: keyBytes, - }, nil -} - -func ensureExist(dir string) error { - filenames := []string{CAKeyName, CACertName, ServerCertName, ServerKeyName} - for _, filename := range filenames { - _, err := os.Stat(path.Join(dir, filename)) - switch { - case err == nil: - continue - case os.IsNotExist(err): - return notFoundError{err} - default: - return err - } - } - return nil -} - -func certToProjectionMap(cert *generator.Artifacts) map[string]atomic.FileProjection { - // TODO: figure out if we can reduce the permission. (Now it's 0666) - return map[string]atomic.FileProjection{ - CAKeyName: { - Data: cert.CAKey, - Mode: 0666, - }, - CACertName: { - Data: cert.CACert, - Mode: 0666, - }, - ServerCertName: { - Data: cert.Cert, - Mode: 0666, - }, - ServerKeyName: { - Data: cert.Key, - Mode: 0666, - }, - } -} - -func (f *fsCertWriter) Inject(objs ...runtime.Object) error { - return nil -} diff --git a/pkg/webhook/internal/cert/writer/fs_test.go b/pkg/webhook/internal/cert/writer/fs_test.go deleted file mode 100644 index 4b39eb1246..0000000000 --- a/pkg/webhook/internal/cert/writer/fs_test.go +++ /dev/null @@ -1,249 +0,0 @@ -/* -Copyright 2018 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package writer - -import ( - "io/ioutil" - "os" - "path" - - . "github.com/onsi/ginkgo" - . "github.com/onsi/gomega" - - "sigs.k8s.io/controller-runtime/pkg/webhook/internal/cert/generator" - fakegenerator "sigs.k8s.io/controller-runtime/pkg/webhook/internal/cert/generator/fake" -) - -var _ = Describe("fsCertWriter", func() { - dnsName := "test-service.test-svc-namespace.svc" - - var certWriter CertWriter - var testingDir string - BeforeEach(func(done Done) { - var err error - testingDir, err = ioutil.TempDir("", "testdir") - Expect(err).NotTo(HaveOccurred()) - certWriter, err = NewFSCertWriter(FSCertWriterOptions{ - CertGenerator: &fakegenerator.CertGenerator{ - DNSNameToCertArtifacts: map[string]*generator.Artifacts{ - dnsName: { - CACert: []byte(certs2.CACert), - Cert: []byte(certs2.Cert), - Key: []byte(certs2.Key), - }, - }, - }, - Path: testingDir, - }) - Expect(err).NotTo(HaveOccurred()) - close(done) - }) - - AfterEach(func() { - os.RemoveAll(testingDir) - }) - - Context("Failed to EnsureCert", func() { - Describe("empty DNS name", func() { - It("should return error", func() { - _, _, err := certWriter.EnsureCert("") - Expect(err).To(MatchError("dnsName should not be empty")) - }) - }) - }) - - Context("Succeeded to EnsureCert", func() { - Context("CertGenerator is not set", func() { - It("should default it and return no error", func() { - _, _, err := certWriter.EnsureCert(dnsName) - Expect(err).NotTo(HaveOccurred()) - - }) - }) - - Context("no existing certificate files", func() { - It("should create new certificate files", func() { - _, _, err := certWriter.EnsureCert(dnsName) - Expect(err).NotTo(HaveOccurred()) - caBytes, err := ioutil.ReadFile(path.Join(testingDir, CACertName)) - Expect(err).NotTo(HaveOccurred()) - Expect(caBytes).To(Equal([]byte(certs2.CACert))) - certBytes, err := ioutil.ReadFile(path.Join(testingDir, ServerCertName)) - Expect(err).NotTo(HaveOccurred()) - Expect(certBytes).To(Equal([]byte(certs2.Cert))) - keyBytes, err := ioutil.ReadFile(path.Join(testingDir, ServerKeyName)) - Expect(err).NotTo(HaveOccurred()) - Expect(keyBytes).To(Equal([]byte(certs2.Key))) - }) - }) - - Context("old secret exists", func() { - Context("cert is invalid", func() { - Describe("cert in secret is incomplete", func() { - Context("cert file is not a symbolic link", func() { - BeforeEach(func(done Done) { - err := ioutil.WriteFile(path.Join(testingDir, CACertName), []byte(`oldCACertBytes`), 0600) - Expect(err).NotTo(HaveOccurred()) - close(done) - }) - - It("should replace with new certs", func() { - _, _, err := certWriter.EnsureCert(dnsName) - Expect(err).NotTo(HaveOccurred()) - caBytes, err := ioutil.ReadFile(path.Join(testingDir, CACertName)) - Expect(err).NotTo(HaveOccurred()) - Expect(caBytes).To(Equal([]byte(certs2.CACert))) - certBytes, err := ioutil.ReadFile(path.Join(testingDir, ServerCertName)) - Expect(err).NotTo(HaveOccurred()) - Expect(certBytes).To(Equal([]byte(certs2.Cert))) - keyBytes, err := ioutil.ReadFile(path.Join(testingDir, ServerKeyName)) - Expect(err).NotTo(HaveOccurred()) - Expect(keyBytes).To(Equal([]byte(certs2.Key))) - }) - }) - - Context("cert file is a symbolic link", func() { - BeforeEach(func(done Done) { - dataDir := path.Join(testingDir, "..data") - realDataDir := path.Join(testingDir, "..2018_06_01_15_04_05.12345678") - caFileName := path.Join(testingDir, "..2018_06_01_15_04_05.12345678", CACertName) - err := os.Mkdir(realDataDir, 0700) - Expect(err).NotTo(HaveOccurred()) - err = ioutil.WriteFile(caFileName, []byte(`oldCACertBytes`), 0600) - Expect(err).NotTo(HaveOccurred()) - err = os.Symlink(realDataDir, dataDir) - Expect(err).NotTo(HaveOccurred()) - close(done) - }) - - It("should replace with new certs", func() { - _, _, err := certWriter.EnsureCert(dnsName) - Expect(err).NotTo(HaveOccurred()) - caBytes, err := ioutil.ReadFile(path.Join(testingDir, CACertName)) - Expect(err).NotTo(HaveOccurred()) - Expect(caBytes).To(Equal([]byte(certs2.CACert))) - certBytes, err := ioutil.ReadFile(path.Join(testingDir, ServerCertName)) - Expect(err).NotTo(HaveOccurred()) - Expect(certBytes).To(Equal([]byte(certs2.Cert))) - keyBytes, err := ioutil.ReadFile(path.Join(testingDir, ServerKeyName)) - Expect(err).NotTo(HaveOccurred()) - Expect(keyBytes).To(Equal([]byte(certs2.Key))) - }) - }) - }) - - Describe("cert content is invalid", func() { - Context("cert files are not symbolic links", func() { - BeforeEach(func(done Done) { - ioutil.WriteFile(path.Join(testingDir, CACertName), []byte(`oldCACertBytes`), 0600) - ioutil.WriteFile(path.Join(testingDir, ServerCertName), []byte(`oldCertBytes`), 0600) - ioutil.WriteFile(path.Join(testingDir, ServerKeyName), []byte(`oldKeyBytes`), 0600) - close(done) - }) - - It("should replace with new certs", func() { - _, _, err := certWriter.EnsureCert(dnsName) - Expect(err).NotTo(HaveOccurred()) - caBytes, err := ioutil.ReadFile(path.Join(testingDir, CACertName)) - Expect(err).NotTo(HaveOccurred()) - Expect(caBytes).To(Equal([]byte(certs2.CACert))) - certBytes, err := ioutil.ReadFile(path.Join(testingDir, ServerCertName)) - Expect(err).NotTo(HaveOccurred()) - Expect(certBytes).To(Equal([]byte(certs2.Cert))) - keyBytes, err := ioutil.ReadFile(path.Join(testingDir, ServerKeyName)) - Expect(err).NotTo(HaveOccurred()) - Expect(keyBytes).To(Equal([]byte(certs2.Key))) - }) - }) - - Context("cert files are symbolic links", func() { - BeforeEach(func(done Done) { - dataDir := path.Join(testingDir, "..data") - realDataDir := path.Join(testingDir, "..2018_06_01_15_04_05.12345678") - caFileName := path.Join(testingDir, "..2018_06_01_15_04_05.12345678", CACertName) - certFileName := path.Join(testingDir, "..2018_06_01_15_04_05.12345678", ServerCertName) - keyFileName := path.Join(testingDir, "..2018_06_01_15_04_05.12345678", ServerKeyName) - err := os.Mkdir(realDataDir, 0700) - Expect(err).NotTo(HaveOccurred()) - err = ioutil.WriteFile(caFileName, []byte(`oldCACertBytes`), 0600) - Expect(err).NotTo(HaveOccurred()) - err = ioutil.WriteFile(certFileName, []byte(`oldCertBytes`), 0600) - Expect(err).NotTo(HaveOccurred()) - err = ioutil.WriteFile(keyFileName, []byte(`oldKeyBytes`), 0600) - Expect(err).NotTo(HaveOccurred()) - err = os.Symlink(realDataDir, dataDir) - Expect(err).NotTo(HaveOccurred()) - close(done) - }) - - It("should replace with new certs", func() { - _, _, err := certWriter.EnsureCert(dnsName) - Expect(err).NotTo(HaveOccurred()) - caBytes, err := ioutil.ReadFile(path.Join(testingDir, CACertName)) - Expect(err).NotTo(HaveOccurred()) - Expect(caBytes).To(Equal([]byte(certs2.CACert))) - certBytes, err := ioutil.ReadFile(path.Join(testingDir, ServerCertName)) - Expect(err).NotTo(HaveOccurred()) - Expect(certBytes).To(Equal([]byte(certs2.Cert))) - keyBytes, err := ioutil.ReadFile(path.Join(testingDir, ServerKeyName)) - Expect(err).NotTo(HaveOccurred()) - Expect(keyBytes).To(Equal([]byte(certs2.Key))) - }) - }) - }) - }) - }) - - Context("cert is valid", func() { - Context("when not expiring", func() { - BeforeEach(func(done Done) { - err := ioutil.WriteFile(path.Join(testingDir, CACertName), []byte(certs2.CACert), 0600) - Expect(err).NotTo(HaveOccurred()) - err = ioutil.WriteFile(path.Join(testingDir, ServerCertName), []byte(certs2.Cert), 0600) - Expect(err).NotTo(HaveOccurred()) - err = ioutil.WriteFile(path.Join(testingDir, ServerKeyName), []byte(certs2.Key), 0600) - Expect(err).NotTo(HaveOccurred()) - close(done) - }) - It("should keep the secret", func() { - _, _, err := certWriter.EnsureCert(dnsName) - Expect(err).NotTo(HaveOccurred()) - caBytes, err := ioutil.ReadFile(path.Join(testingDir, CACertName)) - Expect(err).NotTo(HaveOccurred()) - Expect(caBytes).To(Equal([]byte(certs2.CACert))) - certBytes, err := ioutil.ReadFile(path.Join(testingDir, ServerCertName)) - Expect(err).NotTo(HaveOccurred()) - Expect(certBytes).To(Equal([]byte(certs2.Cert))) - keyBytes, err := ioutil.ReadFile(path.Join(testingDir, ServerKeyName)) - Expect(err).NotTo(HaveOccurred()) - Expect(keyBytes).To(Equal([]byte(certs2.Key))) - }) - }) - - Context("when expiring", func() { - // TODO: implement this. - BeforeEach(func(done Done) { - close(done) - }) - - It("should replace the expiring cert", func() { - - }) - }) - }) - }) -}) diff --git a/pkg/webhook/internal/cert/writer/secret.go b/pkg/webhook/internal/cert/writer/secret.go deleted file mode 100644 index 04d7e90c7d..0000000000 --- a/pkg/webhook/internal/cert/writer/secret.go +++ /dev/null @@ -1,184 +0,0 @@ -/* -Copyright 2018 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package writer - -import ( - "errors" - - corev1 "k8s.io/api/core/v1" - apierrors "k8s.io/apimachinery/pkg/api/errors" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/types" - "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/webhook/internal/cert/generator" -) - -// secretCertWriter provisions the certificate by reading and writing to the k8s secrets. -type secretCertWriter struct { - *SecretCertWriterOptions - - // dnsName is the DNS name that the certificate is for. - dnsName string -} - -// SecretCertWriterOptions is options for constructing a secretCertWriter. -type SecretCertWriterOptions struct { - // client talks to a kubernetes cluster for creating the secret. - Client client.Client - // certGenerator generates the certificates. - CertGenerator generator.CertGenerator - // secret points the secret that contains certificates that written by the CertWriter. - Secret *types.NamespacedName -} - -var _ CertWriter = &secretCertWriter{} - -func (ops *SecretCertWriterOptions) setDefaults() { - if ops.CertGenerator == nil { - ops.CertGenerator = &generator.SelfSignedCertGenerator{} - } -} - -func (ops *SecretCertWriterOptions) validate() error { - if ops.Client == nil { - return errors.New("client must be set in SecretCertWriterOptions") - } - if ops.Secret == nil { - return errors.New("secret must be set in SecretCertWriterOptions") - } - return nil -} - -// NewSecretCertWriter constructs a CertWriter that persists the certificate in a k8s secret. -func NewSecretCertWriter(ops SecretCertWriterOptions) (CertWriter, error) { - ops.setDefaults() - err := ops.validate() - if err != nil { - return nil, err - } - return &secretCertWriter{ - SecretCertWriterOptions: &ops, - }, nil -} - -// EnsureCert provisions certificates for a webhookClientConfig by writing the certificates to a k8s secret. -func (s *secretCertWriter) EnsureCert(dnsName string) (*generator.Artifacts, bool, error) { - // Create or refresh the certs based on clientConfig - s.dnsName = dnsName - return handleCommon(s.dnsName, s) -} - -var _ certReadWriter = &secretCertWriter{} - -func (s *secretCertWriter) buildSecret() (*corev1.Secret, *generator.Artifacts, error) { - certs, err := s.CertGenerator.Generate(s.dnsName) - if err != nil { - return nil, nil, err - } - secret := certsToSecret(certs, *s.Secret) - return secret, certs, err -} - -func (s *secretCertWriter) write() (*generator.Artifacts, error) { - secret, certs, err := s.buildSecret() - if err != nil { - return nil, err - } - err = s.Client.Create(nil, secret) - if apierrors.IsAlreadyExists(err) { - return nil, alreadyExistError{err} - } - return certs, err -} - -func (s *secretCertWriter) overwrite() ( - *generator.Artifacts, error) { - secret, certs, err := s.buildSecret() - if err != nil { - return nil, err - } - err = s.Client.Update(nil, secret) - return certs, err -} - -func (s *secretCertWriter) read() (*generator.Artifacts, error) { - secret := &corev1.Secret{ - TypeMeta: metav1.TypeMeta{ - APIVersion: "v1", - Kind: "Secret", - }, - } - err := s.Client.Get(nil, *s.Secret, secret) - if apierrors.IsNotFound(err) { - return nil, notFoundError{err} - } - certs := secretToCerts(secret) - if certs != nil { - // Store the CA for next usage. - s.CertGenerator.SetCA(certs.CAKey, certs.CACert) - } - return certs, nil -} - -func secretToCerts(secret *corev1.Secret) *generator.Artifacts { - if secret.Data == nil { - return nil - } - return &generator.Artifacts{ - CAKey: secret.Data[CAKeyName], - CACert: secret.Data[CACertName], - Cert: secret.Data[ServerCertName], - Key: secret.Data[ServerKeyName], - } -} - -func certsToSecret(certs *generator.Artifacts, sec types.NamespacedName) *corev1.Secret { - return &corev1.Secret{ - TypeMeta: metav1.TypeMeta{ - APIVersion: "v1", - Kind: "Secret", - }, - ObjectMeta: metav1.ObjectMeta{ - Namespace: sec.Namespace, - Name: sec.Name, - }, - Data: map[string][]byte{ - CAKeyName: certs.CAKey, - CACertName: certs.CACert, - ServerKeyName: certs.Key, - ServerCertName: certs.Cert, - }, - } -} - -// Inject sets the ownerReference in the secret. -func (s *secretCertWriter) Inject(objs ...runtime.Object) error { - // TODO: figure out how to get the UID - //for i := range objs { - // accessor, err := meta.Accessor(objs[i]) - // if err != nil { - // return err - // } - // err = controllerutil.SetControllerReference(accessor, s.sec, scheme.Scheme) - // if err != nil { - // return err - // } - //} - //return s.client.Update(context.Background(), s.sec) - return nil -} diff --git a/pkg/webhook/internal/cert/writer/secret_test.go b/pkg/webhook/internal/cert/writer/secret_test.go deleted file mode 100644 index e8f413d74b..0000000000 --- a/pkg/webhook/internal/cert/writer/secret_test.go +++ /dev/null @@ -1,241 +0,0 @@ -/* -Copyright 2018 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package writer - -import ( - . "github.com/onsi/ginkgo" - . "github.com/onsi/gomega" - - corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/types" - "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/client/fake" - "sigs.k8s.io/controller-runtime/pkg/webhook/internal/cert/generator" - fakegenerator "sigs.k8s.io/controller-runtime/pkg/webhook/internal/cert/generator/fake" -) - -var _ = Describe("secretCertWriter", func() { - dnsName := "test-service.test-svc-namespace.svc" - - var certWriter CertWriter - var sCertWriter *secretCertWriter - var secret *corev1.Secret - - BeforeEach(func(done Done) { - var err error - certWriter, err = NewSecretCertWriter(SecretCertWriterOptions{ - Client: fake.NewFakeClient(), - Secret: &types.NamespacedName{ - Namespace: "namespace-bar", - Name: "secret-foo", - }, - CertGenerator: &fakegenerator.CertGenerator{ - DNSNameToCertArtifacts: map[string]*generator.Artifacts{ - dnsName: { - CAKey: []byte(`CAKeyBytes`), - CACert: []byte(`CACertBytes`), - Cert: []byte(`CertBytes`), - Key: []byte(`KeyBytes`), - }, - }, - }, - }) - Expect(err).NotTo(HaveOccurred()) - sCertWriter = certWriter.(*secretCertWriter) - close(done) - }) - - Context("Failed to EnsureCerts", func() { - Describe("empty DNS name", func() { - It("should return error", func() { - _, _, err := certWriter.EnsureCert("") - Expect(err).To(MatchError("dnsName should not be empty")) - }) - }) - - }) - - Context("Succeeded to EnsureCerts", func() { - BeforeEach(func(done Done) { - //isController := true - //blockOwnerDeletion := true - secret = &corev1.Secret{ - TypeMeta: metav1.TypeMeta{ - APIVersion: "v1", - Kind: "Secret", - }, - ObjectMeta: metav1.ObjectMeta{ - Namespace: "namespace-bar", - Name: "secret-foo", - //OwnerReferences: []metav1.OwnerReference{ - // { - // APIVersion: "admissionregistration.k8s.io/v1beta1", - // Kind: "MutatingWebhookConfiguration", - // Name: "test-mwc", - // UID: "123456", - // BlockOwnerDeletion: &blockOwnerDeletion, - // Controller: &isController, - // }, - //}, - }, - Data: map[string][]byte{ - CAKeyName: []byte(`CAKeyBytes`), - CACertName: []byte(`CACertBytes`), - ServerKeyName: []byte(`KeyBytes`), - ServerCertName: []byte(`CertBytes`), - }, - } - close(done) - }) - - Context("certGenerator is not set", func() { - It("should default it and return no error", func() { - _, _, err := certWriter.EnsureCert(dnsName) - Expect(err).NotTo(HaveOccurred()) - list := &corev1.SecretList{} - err = sCertWriter.Client.List(nil, list, client.InNamespace("namespace-bar")) - Expect(err).NotTo(HaveOccurred()) - Expect(list.Items).To(HaveLen(1)) - }) - }) - - Context("no existing secret", func() { - It("should create new secrets with certs", func() { - _, changed, err := certWriter.EnsureCert(dnsName) - Expect(err).NotTo(HaveOccurred()) - list := &corev1.SecretList{} - err = sCertWriter.Client.List(nil, list, client.InNamespace("namespace-bar")) - Expect(err).NotTo(HaveOccurred()) - Expect(list.Items).To(ConsistOf(*secret)) - Expect(list.Items).To(HaveLen(1)) - Expect(changed).To(BeTrue()) - }) - }) - - Context("old secret exists", func() { - var oldSecret *corev1.Secret - - Context("cert is invalid", func() { - Describe("cert in secret is incomplete", func() { - BeforeEach(func(done Done) { - oldSecret = secret.DeepCopy() - oldSecret.Data = nil - sCertWriter.Client = fake.NewFakeClient(oldSecret) - close(done) - }) - - It("should replace with new certs", func() { - _, changed, err := certWriter.EnsureCert(dnsName) - Expect(err).NotTo(HaveOccurred()) - list := &corev1.SecretList{} - err = sCertWriter.Client.List(nil, list, client.InNamespace("namespace-bar")) - Expect(err).NotTo(HaveOccurred()) - Expect(list.Items).To(ConsistOf(*secret)) - Expect(list.Items).To(HaveLen(1)) - Expect(changed).To(BeTrue()) - }) - }) - - Describe("cert content is invalid", func() { - BeforeEach(func(done Done) { - oldSecret = secret.DeepCopy() - oldSecret.Data = map[string][]byte{ - CAKeyName: []byte(`invalidCAKeyBytes`), - CACertName: []byte(`invalidCACertBytes`), - ServerKeyName: []byte(`oldKeyBytes`), - ServerCertName: []byte(`oldCertBytes`), - } - sCertWriter.Client = fake.NewFakeClient(oldSecret) - close(done) - }) - - It("should replace with new certs", func() { - _, changed, err := certWriter.EnsureCert(dnsName) - Expect(err).NotTo(HaveOccurred()) - list := &corev1.SecretList{} - err = sCertWriter.Client.List(nil, list, client.InNamespace("namespace-bar")) - Expect(err).NotTo(HaveOccurred()) - Expect(list.Items).To(ConsistOf(*secret)) - Expect(list.Items).To(HaveLen(1)) - Expect(changed).To(BeTrue()) - }) - }) - }) - - Context("cert is valid", func() { - BeforeEach(func(done Done) { - oldSecret.Data = map[string][]byte{ - CAKeyName: []byte(certs2.CAKey), - CACertName: []byte(certs2.CACert), - ServerKeyName: []byte(certs2.Key), - ServerCertName: []byte(certs2.Cert), - } - sCertWriter.Client = fake.NewFakeClient(oldSecret) - close(done) - }) - - Context("when not expiring", func() { - BeforeEach(func(done Done) { - oldSecret = secret.DeepCopy() - oldSecret.Data = map[string][]byte{ - CAKeyName: []byte(certs2.CAKey), - CACertName: []byte(certs2.CACert), - ServerKeyName: []byte(certs2.Key), - ServerCertName: []byte(certs2.Cert), - } - - sCertWriter.Client = fake.NewFakeClient(oldSecret) - close(done) - }) - It("should keep the secret", func() { - _, changed, err := certWriter.EnsureCert(dnsName) - Expect(err).NotTo(HaveOccurred()) - list := &corev1.SecretList{} - err = sCertWriter.Client.List(nil, list, client.InNamespace("namespace-bar")) - Expect(err).NotTo(HaveOccurred()) - Expect(list.Items).To(HaveLen(1)) - Expect(list.Items[0]).To(Equal(*oldSecret)) - Expect(changed).To(BeFalse()) - }) - }) - - Context("when expiring", func() { - // TODO: implement this. - BeforeEach(func(done Done) { - oldSecret = secret.DeepCopy() - oldSecret.Data = map[string][]byte{ - CAKeyName: []byte(`oldCAKeyBytes`), - CACertName: []byte(`oldCACertBytes`), - //ServerKeyName: []byte(expiringKeyPEM), - //ServerCertName: []byte(expiringCertPEM), - } - //j, _ := json.Marshal(someNewValidSecret) - //expectedSecret = runtime.RawExtension{Raw: j} - - sCertWriter.Client = fake.NewFakeClient(oldSecret) - close(done) - }) - - It("should replace the expiring cert", func() { - - }) - }) - }) - }) - }) -}) diff --git a/pkg/webhook/internal/cert/writer/suite_test.go b/pkg/webhook/internal/cert/writer/suite_test.go deleted file mode 100644 index 968ca8b67a..0000000000 --- a/pkg/webhook/internal/cert/writer/suite_test.go +++ /dev/null @@ -1,38 +0,0 @@ -/* -Copyright 2018 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package writer - -import ( - "testing" - - . "github.com/onsi/ginkgo" - . "github.com/onsi/gomega" - - "sigs.k8s.io/controller-runtime/pkg/envtest" - logf "sigs.k8s.io/controller-runtime/pkg/log" - "sigs.k8s.io/controller-runtime/pkg/log/zap" -) - -func TestSource(t *testing.T) { - RegisterFailHandler(Fail) - RunSpecsWithDefaultAndCustomReporters(t, "Cert Writer Test Suite", []Reporter{envtest.NewlineReporter{}}) -} - -var _ = BeforeSuite(func(done Done) { - logf.SetLogger(zap.LoggerTo(GinkgoWriter, true)) - close(done) -}, 60) diff --git a/pkg/webhook/server.go b/pkg/webhook/server.go index 180a2b538f..b6f1cea94f 100644 --- a/pkg/webhook/server.go +++ b/pkg/webhook/server.go @@ -18,27 +18,25 @@ package webhook import ( "context" + "crypto/tls" "fmt" - "io" + "net" "net/http" "path" + "strconv" "sync" - "time" - "k8s.io/apimachinery/pkg/runtime" - apitypes "k8s.io/apimachinery/pkg/types" - "k8s.io/apimachinery/pkg/util/wait" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/config" "sigs.k8s.io/controller-runtime/pkg/manager" "sigs.k8s.io/controller-runtime/pkg/runtime/inject" atypes "sigs.k8s.io/controller-runtime/pkg/webhook/admission/types" - "sigs.k8s.io/controller-runtime/pkg/webhook/internal/cert" - "sigs.k8s.io/controller-runtime/pkg/webhook/internal/cert/writer" - "sigs.k8s.io/controller-runtime/pkg/webhook/types" ) -// default interval for checking cert is 90 days (~3 months) -var defaultCertRefreshInterval = 3 * 30 * 24 * time.Hour +const ( + certName = "tls.crt" + keyName = "tls.key" +) // ServerOptions are options for configuring an admission webhook server. type ServerOptions struct { @@ -58,60 +56,10 @@ type ServerOptions struct { // Client will be injected by the manager if not set. Client client.Client - // DisableWebhookConfigInstaller controls if the server will automatically create webhook related objects - // during bootstrapping. e.g. webhookConfiguration, service and secret. - // If false, the server will install the webhook config objects. It is defaulted to false. - DisableWebhookConfigInstaller *bool - - // BootstrapOptions contains the options for bootstrapping the admission server. - *BootstrapOptions -} - -// BootstrapOptions are options for bootstrapping an admission webhook server. -type BootstrapOptions struct { - // MutatingWebhookConfigName is the name that used for creating the MutatingWebhookConfiguration object. - MutatingWebhookConfigName string - // ValidatingWebhookConfigName is the name that used for creating the ValidatingWebhookConfiguration object. - ValidatingWebhookConfigName string - - // Secret is the location for storing the certificate for the admission server. - // The server should have permission to create a secret in the namespace. - // This is optional. If unspecified, it will write to the filesystem. - // It the secret already exists and is different from the desired, it will be replaced. - Secret *apitypes.NamespacedName - - // Deprecated: Writer will not be used anywhere. - Writer io.Writer - - // Service is k8s service fronting the webhook server pod(s). - // This field is optional. But one and only one of Service and Host need to be set. - // This maps to field .webhooks.getClientConfig.service - // https://github.com/kubernetes/api/blob/183f3326a9353bd6d41430fc80f96259331d029c/admissionregistration/v1beta1/types.go#L260 - Service *Service - // Host is the host name of .webhooks.clientConfig.url - // https://github.com/kubernetes/api/blob/183f3326a9353bd6d41430fc80f96259331d029c/admissionregistration/v1beta1/types.go#L250 - // This field is optional. But one and only one of Service and Host need to be set. - // If neither Service nor Host is unspecified, Host will be defaulted to "localhost". - Host *string - - // certProvisioner is constructed using certGenerator and certWriter - certProvisioner *cert.Provisioner // nolint: structcheck - // err will be non-nil if there is an error occur during initialization. err error // nolint: structcheck } -// Service contains information for creating a service -type Service struct { - // Name of the service - Name string - // Namespace of the service - Namespace string - // Selectors is the selector of the service. - // This must select the pods that runs this webhook server. - Selectors map[string]string -} - // Server is an admission webhook server that can serve traffic and // generates related k8s resources for deploying. type Server struct { @@ -125,28 +73,16 @@ type Server struct { // registry maps a path to a http.Handler. registry map[string]Webhook - // mutatingWebhookConfiguration and validatingWebhookConfiguration are populated during server bootstrapping. - // They can be nil, if there is no webhook registered under it. - webhookConfigurations []runtime.Object - // manager is the manager that this webhook server will be registered. manager manager.Manager - // httpServer is the actual server that serves the traffic. - httpServer *http.Server - once sync.Once } // Webhook defines the basics that a webhook should support. type Webhook interface { - // GetName returns the name of the webhook. - GetName() string // GetPath returns the path that the webhook registered. GetPath() string - // GetType returns the Type of the webhook. - // e.g. mutating or validating - GetType() types.WebhookType // Handler returns a http.Handler for the webhook. Handler() http.Handler // Validate validates if the webhook itself is valid. @@ -167,6 +103,38 @@ func NewServer(name string, mgr manager.Manager, options ServerOptions) (*Server return as, nil } +// setDefault does defaulting for the Server. +func (s *Server) setDefault() { + if len(s.Name) == 0 { + s.Name = "default-k8s-webhook-server" + } + if s.registry == nil { + s.registry = map[string]Webhook{} + } + if s.sMux == nil { + s.sMux = http.DefaultServeMux + } + if s.Port <= 0 { + s.Port = 443 + } + if len(s.CertDir) == 0 { + s.CertDir = path.Join("k8s-webhook-server", "cert") + } + + if s.Client == nil { + cfg, err := config.GetConfig() + if err != nil { + s.err = err + return + } + s.Client, err = client.New(cfg, client.Options{}) + if err != nil { + s.err = err + return + } + } +} + // Register validates and registers webhook(s) in the server func (s *Server) Register(webhooks ...Webhook) error { for i, webhook := range webhooks { @@ -203,83 +171,44 @@ func (s *Server) Start(stop <-chan struct{}) error { return s.err } - if s.DisableWebhookConfigInstaller != nil && !*s.DisableWebhookConfigInstaller { - log.Info("installing webhook configuration in cluster") - err := s.InstallWebhookManifests() - if err != nil { - return err - } - } else { - log.Info("webhook installer is disabled") + // TODO: watch the cert dir. Reload the cert if it changes + cert, err := tls.LoadX509KeyPair(path.Join(s.CertDir, certName), path.Join(s.CertDir, keyName)) + if err != nil { + return err } - return s.run(stop) -} - -func (s *Server) run(stop <-chan struct{}) error { // nolint: gocyclo - errCh := make(chan error) - serveFn := func() { - s.httpServer = &http.Server{ - Addr: fmt.Sprintf(":%v", s.Port), - Handler: s.sMux, - } - log.Info("starting the webhook server.") - errCh <- s.httpServer.ListenAndServeTLS(path.Join(s.CertDir, writer.ServerCertName), path.Join(s.CertDir, writer.ServerKeyName)) + cfg := &tls.Config{ + Certificates: []tls.Certificate{cert}, } - shutdownHappend := false - timer := time.Tick(wait.Jitter(defaultCertRefreshInterval, 0.1)) - go serveFn() - for { - select { - case <-timer: - changed, err := s.RefreshCert() - if err != nil { - log.Error(err, "encountering error when refreshing the certificate") - return err - } - if !changed { - log.Info("no need to reload the certificates.") - continue - } - log.Info("server is shutting down to reload the certificates.") - shutdownHappend = true - err = s.httpServer.Shutdown(context.Background()) - if err != nil { - log.Error(err, "encountering error when shutting down") - return err - } - timer = time.Tick(wait.Jitter(defaultCertRefreshInterval, 0.1)) - go serveFn() - case <-stop: - return s.httpServer.Shutdown(context.Background()) - case e := <-errCh: - // Don't exit when getting an http.ErrServerClosed error due to restarting the server. - if shutdownHappend && e == http.ErrServerClosed { - shutdownHappend = false - } else if e != nil { - log.Error(e, "server returns an unexpected error") - return e - } - } + listener, err := tls.Listen("tcp", net.JoinHostPort("", strconv.Itoa(int(s.Port))), cfg) + if err != nil { + return err } -} -// RefreshCert refreshes the certificate using Server's Provisioner if the certificate is expiring. -func (s *Server) RefreshCert() (bool, error) { - cc, err := s.getClientConfig() - if err != nil { - return false, err + srv := &http.Server{ + Handler: s.sMux, } - changed, err := s.certProvisioner.Provision(cert.Options{ - ClientConfig: cc, - Objects: s.webhookConfigurations, - }) - if err != nil { - return false, err + + idleConnsClosed := make(chan struct{}) + go func() { + <-stop + + // TODO: use a context with reasonable timeout + if err := srv.Shutdown(context.Background()); err != nil { + // Error from closing listeners, or context timeout + log.Error(err, "error shutting down the HTTP server") + } + close(idleConnsClosed) + }() + + err = srv.Serve(listener) + if err != nil && err != http.ErrServerClosed { + return err } - return changed, batchCreateOrReplace(s.Client, s.webhookConfigurations...) + <-idleConnsClosed + return nil } var _ inject.Client = &Server{} @@ -297,7 +226,7 @@ func (s *Server) InjectClient(c client.Client) error { var _ inject.Decoder = &Server{} -// InjectDecoder injects the client into the server +// InjectDecoder injects the decoder into the server func (s *Server) InjectDecoder(d atypes.Decoder) error { for _, wh := range s.registry { if _, err := inject.DecoderInto(d, wh.Handler()); err != nil { diff --git a/pkg/webhook/server_test.go b/pkg/webhook/server_test.go deleted file mode 100644 index ff660cb3cd..0000000000 --- a/pkg/webhook/server_test.go +++ /dev/null @@ -1,163 +0,0 @@ -/* -Copyright 2018 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package webhook - -import ( - "context" - "io/ioutil" - "net/http" - "os" - "time" - - . "github.com/onsi/ginkgo" - . "github.com/onsi/gomega" - - "k8s.io/apimachinery/pkg/runtime" - "sigs.k8s.io/controller-runtime/pkg/webhook/internal/cert" - "sigs.k8s.io/controller-runtime/pkg/webhook/internal/cert/generator" - "sigs.k8s.io/controller-runtime/pkg/webhook/internal/cert/writer" - "sigs.k8s.io/testing_frameworks/integration/addr" -) - -type fakeCertWriter struct { - changed bool -} - -func (cw *fakeCertWriter) EnsureCert(dnsName string) (*generator.Artifacts, bool, error) { - return &generator.Artifacts{}, cw.changed, nil -} - -func (cw *fakeCertWriter) Inject(objs ...runtime.Object) error { - return nil -} - -var _ = Describe("webhook server", func() { - Describe("run", func() { - var stop chan struct{} - var s *Server - var cn = "example.com" - - BeforeEach(func() { - port, _, err := addr.Suggest() - Expect(err).NotTo(HaveOccurred()) - s = &Server{ - sMux: http.NewServeMux(), - ServerOptions: ServerOptions{ - Port: int32(port), - BootstrapOptions: &BootstrapOptions{ - Host: &cn, - }, - }, - } - - cg := &generator.SelfSignedCertGenerator{} - s.CertDir, err = ioutil.TempDir("/tmp", "controller-runtime-") - Expect(err).NotTo(HaveOccurred()) - certWriter, err := writer.NewFSCertWriter(writer.FSCertWriterOptions{CertGenerator: cg, Path: s.CertDir}) - Expect(err).NotTo(HaveOccurred()) - _, _, err = certWriter.EnsureCert(cn) - Expect(err).NotTo(HaveOccurred()) - - stop = make(chan struct{}) - }) - - It("should stop if the stop channel is closed", func() { - var e error - go func() { - defer GinkgoRecover() - e = s.run(stop) - }() - - Eventually(func() *http.Server { - return s.httpServer - }).ShouldNot(BeNil()) - - close(stop) - Expect(e).NotTo(HaveOccurred()) - }) - - It("should exit if the server encounter an unexpected error", func() { - var e error - go func() { - defer GinkgoRecover() - e = s.run(stop) - }() - - Eventually(func() *http.Server { - return s.httpServer - }).ShouldNot(BeNil()) - - err := s.httpServer.Shutdown(context.Background()) - Expect(err).NotTo(HaveOccurred()) - - Eventually(func() error { - return e - }).Should(Equal(http.ErrServerClosed)) - - close(stop) - }) - - It("should be able to keep existing valid cert when timer fires", func() { - var e error - defaultCertRefreshInterval = 500 * time.Millisecond - - s.certProvisioner = &cert.Provisioner{ - CertWriter: &fakeCertWriter{changed: false}, - } - - go func() { - defer GinkgoRecover() - e = s.run(stop) - }() - - // Wait for multiple cycles of timer firing - time.Sleep(2 * time.Second) - Expect(e).NotTo(HaveOccurred()) - - close(stop) - }) - - It("should be able to rotate the cert when timer fires", func() { - var e error - defaultCertRefreshInterval = 500 * time.Millisecond - s.certProvisioner = &cert.Provisioner{ - CertWriter: &fakeCertWriter{changed: true}, - } - - go func() { - defer GinkgoRecover() - e = s.run(stop) - }() - - Eventually(func() *http.Server { - return s.httpServer - }).ShouldNot(BeNil()) - - // Wait for multiple cycles of timer firing - time.Sleep(2 * time.Second) - Expect(e).NotTo(HaveOccurred()) - - close(stop) - }) - - AfterEach(func() { - defaultCertRefreshInterval = 3 * 30 * 24 * time.Hour - err := os.RemoveAll(s.CertDir) - Expect(err).NotTo(HaveOccurred()) - }, 60) - }) -}) diff --git a/pkg/webhook/util.go b/pkg/webhook/util.go deleted file mode 100644 index 47e2635ca5..0000000000 --- a/pkg/webhook/util.go +++ /dev/null @@ -1,115 +0,0 @@ -/* -Copyright 2018 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package webhook - -import ( - "context" - - admissionregistration "k8s.io/api/admissionregistration/v1beta1" - corev1 "k8s.io/api/core/v1" - apierrors "k8s.io/apimachinery/pkg/api/errors" - "k8s.io/apimachinery/pkg/runtime" - "sigs.k8s.io/controller-runtime/pkg/client" -) - -type mutateFn func(current, desired *runtime.Object) error - -var serviceFn = func(current, desired *runtime.Object) error { - typedC := (*current).(*corev1.Service) - typedD := (*desired).(*corev1.Service) - typedC.Spec.Selector = typedD.Spec.Selector - return nil -} - -var mutatingWebhookConfigFn = func(current, desired *runtime.Object) error { - typedC := (*current).(*admissionregistration.MutatingWebhookConfiguration) - typedD := (*desired).(*admissionregistration.MutatingWebhookConfiguration) - typedC.Webhooks = typedD.Webhooks - return nil -} - -var validatingWebhookConfigFn = func(current, desired *runtime.Object) error { - typedC := (*current).(*admissionregistration.ValidatingWebhookConfiguration) - typedD := (*desired).(*admissionregistration.ValidatingWebhookConfiguration) - typedC.Webhooks = typedD.Webhooks - return nil -} - -var genericFn = func(current, desired *runtime.Object) error { - *current = *desired - return nil -} - -// createOrReplaceHelper creates the object if it doesn't exist; -// otherwise, it will replace it. -// When replacing, fn should know how to preserve existing fields in the object GET from the APIServer. -// TODO: use the helper in #98 when it merges. -func createOrReplaceHelper(c client.Client, obj runtime.Object, fn mutateFn) error { - if obj == nil { - return nil - } - err := c.Create(context.Background(), obj) - if apierrors.IsAlreadyExists(err) { - // TODO: retry mutiple times with backoff if necessary. - existing := obj.DeepCopyObject() - objectKey, err := client.ObjectKeyFromObject(obj) - if err != nil { - return err - } - err = c.Get(context.Background(), objectKey, existing) - if err != nil { - return err - } - err = fn(&existing, &obj) - if err != nil { - return err - } - return c.Update(context.Background(), existing) - } - return err -} - -// createOrReplace creates the object if it doesn't exist; -// otherwise, it will replace it. -// When replacing, it knows how to preserve existing fields in the object GET from the APIServer. -// It currently only support MutatingWebhookConfiguration, ValidatingWebhookConfiguration and Service. -// For other kinds, it uses genericFn to replace the whole object. -func createOrReplace(c client.Client, obj runtime.Object) error { - if obj == nil { - return nil - } - switch obj.(type) { - case *admissionregistration.MutatingWebhookConfiguration: - return createOrReplaceHelper(c, obj, mutatingWebhookConfigFn) - case *admissionregistration.ValidatingWebhookConfiguration: - return createOrReplaceHelper(c, obj, validatingWebhookConfigFn) - case *corev1.Service: - return createOrReplaceHelper(c, obj, serviceFn) - default: - return createOrReplaceHelper(c, obj, genericFn) - } -} - -func batchCreateOrReplace(c client.Client, objs ...runtime.Object) error { - for i := range objs { - err := createOrReplace(c, objs[i]) - if err != nil { - return err - } - } - return nil -} diff --git a/vendor/k8s.io/client-go/util/testing/fake_handler.go b/vendor/k8s.io/client-go/util/testing/fake_handler.go deleted file mode 100644 index 6790cfd8ce..0000000000 --- a/vendor/k8s.io/client-go/util/testing/fake_handler.go +++ /dev/null @@ -1,139 +0,0 @@ -/* -Copyright 2014 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package testing - -import ( - "io/ioutil" - "net/http" - "net/url" - "reflect" - "sync" -) - -// TestInterface is a simple interface providing Errorf, to make injection for -// testing easier (insert 'yo dawg' meme here). -type TestInterface interface { - Errorf(format string, args ...interface{}) - Logf(format string, args ...interface{}) -} - -// LogInterface is a simple interface to allow injection of Logf to report serving errors. -type LogInterface interface { - Logf(format string, args ...interface{}) -} - -// FakeHandler is to assist in testing HTTP requests. Notice that FakeHandler is -// not thread safe and you must not direct traffic to except for the request -// you want to test. You can do this by hiding it in an http.ServeMux. -type FakeHandler struct { - RequestReceived *http.Request - RequestBody string - StatusCode int - ResponseBody string - // For logging - you can use a *testing.T - // This will keep log messages associated with the test. - T LogInterface - - // Enforce "only one use" constraint. - lock sync.Mutex - requestCount int - hasBeenChecked bool - - SkipRequestFn func(verb string, url url.URL) bool -} - -func (f *FakeHandler) SetResponseBody(responseBody string) { - f.lock.Lock() - defer f.lock.Unlock() - f.ResponseBody = responseBody -} - -func (f *FakeHandler) ServeHTTP(response http.ResponseWriter, request *http.Request) { - f.lock.Lock() - defer f.lock.Unlock() - - if f.SkipRequestFn != nil && f.SkipRequestFn(request.Method, *request.URL) { - response.Header().Set("Content-Type", "application/json") - response.WriteHeader(f.StatusCode) - response.Write([]byte(f.ResponseBody)) - return - } - - f.requestCount++ - if f.hasBeenChecked { - panic("got request after having been validated") - } - - f.RequestReceived = request - response.Header().Set("Content-Type", "application/json") - response.WriteHeader(f.StatusCode) - response.Write([]byte(f.ResponseBody)) - - bodyReceived, err := ioutil.ReadAll(request.Body) - if err != nil && f.T != nil { - f.T.Logf("Received read error: %v", err) - } - f.RequestBody = string(bodyReceived) - if f.T != nil { - f.T.Logf("request body: %s", f.RequestBody) - } -} - -func (f *FakeHandler) ValidateRequestCount(t TestInterface, count int) bool { - ok := true - f.lock.Lock() - defer f.lock.Unlock() - if f.requestCount != count { - ok = false - t.Errorf("Expected %d call, but got %d. Only the last call is recorded and checked.", count, f.requestCount) - } - f.hasBeenChecked = true - return ok -} - -// ValidateRequest verifies that FakeHandler received a request with expected path, method, and body. -func (f *FakeHandler) ValidateRequest(t TestInterface, expectedPath, expectedMethod string, body *string) { - f.lock.Lock() - defer f.lock.Unlock() - if f.requestCount != 1 { - t.Logf("Expected 1 call, but got %v. Only the last call is recorded and checked.", f.requestCount) - } - f.hasBeenChecked = true - - expectURL, err := url.Parse(expectedPath) - if err != nil { - t.Errorf("Couldn't parse %v as a URL.", expectedPath) - } - if f.RequestReceived == nil { - t.Errorf("Unexpected nil request received for %s", expectedPath) - return - } - if f.RequestReceived.URL.Path != expectURL.Path { - t.Errorf("Unexpected request path for request %#v, received: %q, expected: %q", f.RequestReceived, f.RequestReceived.URL.Path, expectURL.Path) - } - if e, a := expectURL.Query(), f.RequestReceived.URL.Query(); !reflect.DeepEqual(e, a) { - t.Errorf("Unexpected query for request %#v, received: %q, expected: %q", f.RequestReceived, a, e) - } - if f.RequestReceived.Method != expectedMethod { - t.Errorf("Unexpected method: %q, expected: %q", f.RequestReceived.Method, expectedMethod) - } - if body != nil { - if *body != f.RequestBody { - t.Errorf("Received body:\n%s\n Doesn't match expected body:\n%s", f.RequestBody, *body) - } - } -} diff --git a/vendor/k8s.io/client-go/util/testing/tmpdir.go b/vendor/k8s.io/client-go/util/testing/tmpdir.go deleted file mode 100644 index 3b2d885fce..0000000000 --- a/vendor/k8s.io/client-go/util/testing/tmpdir.go +++ /dev/null @@ -1,44 +0,0 @@ -/* -Copyright 2016 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package testing - -import ( - "io/ioutil" - "os" -) - -// MkTmpdir creates a temporary directory based upon the prefix passed in. -// If successful, it returns the temporary directory path. The directory can be -// deleted with a call to "os.RemoveAll(...)". -// In case of error, it'll return an empty string and the error. -func MkTmpdir(prefix string) (string, error) { - tmpDir, err := ioutil.TempDir(os.TempDir(), prefix) - if err != nil { - return "", err - } - return tmpDir, nil -} - -// MkTmpdir does the same work as "MkTmpdir", except in case of -// errors, it'll trigger a panic. -func MkTmpdirOrDie(prefix string) string { - tmpDir, err := MkTmpdir(prefix) - if err != nil { - panic(err) - } - return tmpDir -} From 29adcfceddbf73f8f4c140320b35ea1cee623ffd Mon Sep 17 00:00:00 2001 From: Mengqi Yu Date: Mon, 11 Feb 2019 16:07:14 -0800 Subject: [PATCH 2/2] :warning:fix webhook injector, drop name and client --- example/main.go | 4 +- pkg/webhook/admission/webhook.go | 21 ++------ pkg/webhook/server.go | 87 ++++++++------------------------ 3 files changed, 27 insertions(+), 85 deletions(-) diff --git a/example/main.go b/example/main.go index 81bcdc1cce..55188fc739 100644 --- a/example/main.go +++ b/example/main.go @@ -92,7 +92,7 @@ func main() { Build() entryLog.Info("setting up webhook server") - as, err := webhook.NewServer("foo-admission-server", mgr, webhook.ServerOptions{ + as, err := webhook.NewServer(mgr, webhook.ServerOptions{ Port: 9876, CertDir: "/tmp/cert", }) @@ -104,7 +104,7 @@ func main() { entryLog.Info("registering webhooks to the webhook server") err = as.Register(mutatingWebhook, validatingWebhook) if err != nil { - entryLog.Error(err, "unable to register webhooks in the admission server") + entryLog.Error(err, "unable to setup the admission server") os.Exit(1) } diff --git a/pkg/webhook/admission/webhook.go b/pkg/webhook/admission/webhook.go index 75983b6751..632160c0cb 100644 --- a/pkg/webhook/admission/webhook.go +++ b/pkg/webhook/admission/webhook.go @@ -30,7 +30,6 @@ import ( admissionv1beta1 "k8s.io/api/admission/v1beta1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/runtime/inject" atypes "sigs.k8s.io/controller-runtime/pkg/webhook/admission/types" "sigs.k8s.io/controller-runtime/pkg/webhook/types" @@ -210,24 +209,12 @@ func (w *Webhook) Validate() error { return nil } -var _ inject.Client = &Webhook{} +var _ inject.Injector = &Webhook{} -// InjectClient injects the client into the handlers -func (w *Webhook) InjectClient(c client.Client) error { +// InjectFunc injects dependencies into the handlers. +func (w *Webhook) InjectFunc(f inject.Func) error { for _, handler := range w.Handlers { - if _, err := inject.ClientInto(c, handler); err != nil { - return err - } - } - return nil -} - -var _ inject.Decoder = &Webhook{} - -// InjectDecoder injects the decoder into the handlers -func (w *Webhook) InjectDecoder(d atypes.Decoder) error { - for _, handler := range w.Handlers { - if _, err := inject.DecoderInto(d, handler); err != nil { + if err := f(handler); err != nil { return err } } diff --git a/pkg/webhook/server.go b/pkg/webhook/server.go index b6f1cea94f..3b9f3b14e3 100644 --- a/pkg/webhook/server.go +++ b/pkg/webhook/server.go @@ -19,18 +19,14 @@ package webhook import ( "context" "crypto/tls" - "fmt" "net" "net/http" "path" "strconv" "sync" - "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/client/config" "sigs.k8s.io/controller-runtime/pkg/manager" "sigs.k8s.io/controller-runtime/pkg/runtime/inject" - atypes "sigs.k8s.io/controller-runtime/pkg/webhook/admission/types" ) const ( @@ -50,28 +46,20 @@ type ServerOptions struct { // If using SecretCertWriter in Provisioner, the server will provision the certificate in a secret, // the user is responsible to mount the secret to the this location for the server to consume. CertDir string - - // Client is a client defined in controller-runtime instead of a client-go client. - // It knows how to talk to a kubernetes cluster. - // Client will be injected by the manager if not set. - Client client.Client - - // err will be non-nil if there is an error occur during initialization. - err error // nolint: structcheck } // Server is an admission webhook server that can serve traffic and // generates related k8s resources for deploying. type Server struct { - // Name is the name of server - Name string - // ServerOptions contains options for configuring the admission server. ServerOptions sMux *http.ServeMux // registry maps a path to a http.Handler. - registry map[string]Webhook + registry map[string]http.Handler + + // setFields is used to inject dependencies into webhooks + setFields func(i interface{}) error // manager is the manager that this webhook server will be registered. manager manager.Manager @@ -81,6 +69,8 @@ type Server struct { // Webhook defines the basics that a webhook should support. type Webhook interface { + http.Handler + // GetPath returns the path that the webhook registered. GetPath() string // Handler returns a http.Handler for the webhook. @@ -91,11 +81,10 @@ type Webhook interface { } // NewServer creates a new admission webhook server. -func NewServer(name string, mgr manager.Manager, options ServerOptions) (*Server, error) { +func NewServer(mgr manager.Manager, options ServerOptions) (*Server, error) { as := &Server{ - Name: name, sMux: http.NewServeMux(), - registry: map[string]Webhook{}, + registry: map[string]http.Handler{}, ServerOptions: options, manager: mgr, } @@ -105,14 +94,11 @@ func NewServer(name string, mgr manager.Manager, options ServerOptions) (*Server // setDefault does defaulting for the Server. func (s *Server) setDefault() { - if len(s.Name) == 0 { - s.Name = "default-k8s-webhook-server" - } if s.registry == nil { - s.registry = map[string]Webhook{} + s.registry = map[string]http.Handler{} } if s.sMux == nil { - s.sMux = http.DefaultServeMux + s.sMux = http.NewServeMux() } if s.Port <= 0 { s.Port = 443 @@ -120,19 +106,6 @@ func (s *Server) setDefault() { if len(s.CertDir) == 0 { s.CertDir = path.Join("k8s-webhook-server", "cert") } - - if s.Client == nil { - cfg, err := config.GetConfig() - if err != nil { - s.err = err - return - } - s.Client, err = client.New(cfg, client.Options{}) - if err != nil { - s.err = err - return - } - } } // Register validates and registers webhook(s) in the server @@ -143,12 +116,14 @@ func (s *Server) Register(webhooks ...Webhook) error { if err != nil { return err } - _, found := s.registry[webhook.GetPath()] - if found { - return fmt.Errorf("can't register duplicate path: %v", webhook.GetPath()) - } - s.registry[webhook.GetPath()] = webhooks[i] + // Handle actually ensures that no duplicate paths are registered. s.sMux.Handle(webhook.GetPath(), webhook.Handler()) + s.registry[webhook.GetPath()] = webhooks[i] + + // Inject dependencies to each webhook. + if err := s.setFields(webhooks[i]); err != nil { + return err + } } // Lazily add Server to manager. @@ -167,9 +142,6 @@ var _ manager.Runnable = &Server{} // It will install the webhook related resources depend on the server configuration. func (s *Server) Start(stop <-chan struct{}) error { s.once.Do(s.setDefault) - if s.err != nil { - return s.err - } // TODO: watch the cert dir. Reload the cert if it changes cert, err := tls.LoadX509KeyPair(path.Join(s.CertDir, certName), path.Join(s.CertDir, keyName)) @@ -211,27 +183,10 @@ func (s *Server) Start(stop <-chan struct{}) error { return nil } -var _ inject.Client = &Server{} - -// InjectClient injects the client into the server -func (s *Server) InjectClient(c client.Client) error { - s.Client = c - for _, wh := range s.registry { - if _, err := inject.ClientInto(c, wh.Handler()); err != nil { - return err - } - } - return nil -} - -var _ inject.Decoder = &Server{} +var _ inject.Injector = &Server{} -// InjectDecoder injects the decoder into the server -func (s *Server) InjectDecoder(d atypes.Decoder) error { - for _, wh := range s.registry { - if _, err := inject.DecoderInto(d, wh.Handler()); err != nil { - return err - } - } +// InjectFunc injects dependencies into the handlers. +func (s *Server) InjectFunc(f inject.Func) error { + s.setFields = f return nil }