diff --git a/cmd/webhook/app/BUILD.bazel b/cmd/webhook/app/BUILD.bazel index bb78b9ec1..e9759108d 100644 --- a/cmd/webhook/app/BUILD.bazel +++ b/cmd/webhook/app/BUILD.bazel @@ -9,20 +9,15 @@ go_library( "//cmd/util:go_default_library", "//cmd/webhook/app/options:go_default_library", "//internal/apis/config/webhook:go_default_library", + "//internal/webhook:go_default_library", "//pkg/logs:go_default_library", "//pkg/util:go_default_library", "//pkg/util/feature:go_default_library", "//pkg/webhook:go_default_library", "//pkg/webhook/authority:go_default_library", "//pkg/webhook/configfile:go_default_library", - "//pkg/webhook/handlers:go_default_library", - "//pkg/webhook/server:go_default_library", - "//pkg/webhook/server/tls:go_default_library", - "@com_github_go_logr_logr//:go_default_library", "@com_github_spf13_cobra//:go_default_library", "@com_github_spf13_pflag//:go_default_library", - "@io_k8s_client_go//kubernetes:go_default_library", - "@io_k8s_client_go//tools/clientcmd:go_default_library", "@io_k8s_component_base//cli/flag:go_default_library", ], ) diff --git a/cmd/webhook/app/testing/BUILD.bazel b/cmd/webhook/app/testing/BUILD.bazel index 52c67e689..4becacd83 100644 --- a/cmd/webhook/app/testing/BUILD.bazel +++ b/cmd/webhook/app/testing/BUILD.bazel @@ -6,8 +6,8 @@ go_library( importpath = "github.com/jetstack/cert-manager/cmd/webhook/app/testing", visibility = ["//visibility:public"], deps = [ - "//cmd/webhook/app:go_default_library", "//cmd/webhook/app/options:go_default_library", + "//internal/webhook:go_default_library", "//pkg/util/pki:go_default_library", "//pkg/webhook/server:go_default_library", "@com_github_go_logr_logr//testing:go_default_library", diff --git a/cmd/webhook/app/testing/testwebhook.go b/cmd/webhook/app/testing/testwebhook.go index 26cc1ca78..4667f5011 100644 --- a/cmd/webhook/app/testing/testwebhook.go +++ b/cmd/webhook/app/testing/testwebhook.go @@ -36,8 +36,8 @@ import ( "k8s.io/apimachinery/pkg/util/wait" "k8s.io/utils/pointer" - "github.com/jetstack/cert-manager/cmd/webhook/app" "github.com/jetstack/cert-manager/cmd/webhook/app/options" + "github.com/jetstack/cert-manager/internal/webhook" "github.com/jetstack/cert-manager/pkg/util/pki" "github.com/jetstack/cert-manager/pkg/webhook/server" ) @@ -56,7 +56,7 @@ type ServerOptions struct { CAPEM []byte } -func StartWebhookServer(t *testing.T, ctx context.Context, args []string, argumentsForNewServerWithOptions ...app.ServerOption) (ServerOptions, StopFunc) { +func StartWebhookServer(t *testing.T, ctx context.Context, args []string, argumentsForNewServerWithOptions ...webhook.ServerOption) (ServerOptions, StopFunc) { log := logtesting.NewTestLogger(t) fs := pflag.NewFlagSet("testset", pflag.ExitOnError) @@ -99,7 +99,7 @@ func StartWebhookServer(t *testing.T, ctx context.Context, args []string, argume webhookConfig.HealthzPort = pointer.Int(0) errCh := make(chan error) - srv, err := app.NewServerWithOptions(log, *webhookFlags, *webhookConfig, argumentsForNewServerWithOptions...) + srv, err := webhook.NewCertManagerWebhookServer(log, *webhookFlags, *webhookConfig, argumentsForNewServerWithOptions...) if err != nil { t.Fatal(err) } diff --git a/cmd/webhook/app/webhook.go b/cmd/webhook/app/webhook.go index b62bdeddf..dafd36087 100644 --- a/cmd/webhook/app/webhook.go +++ b/cmd/webhook/app/webhook.go @@ -22,104 +22,28 @@ import ( "os" "path/filepath" - "github.com/go-logr/logr" "github.com/spf13/cobra" "github.com/spf13/pflag" - "k8s.io/client-go/kubernetes" - "k8s.io/client-go/tools/clientcmd" cliflag "k8s.io/component-base/cli/flag" cmdutil "github.com/jetstack/cert-manager/cmd/util" "github.com/jetstack/cert-manager/cmd/webhook/app/options" config "github.com/jetstack/cert-manager/internal/apis/config/webhook" + cmwebhook "github.com/jetstack/cert-manager/internal/webhook" logf "github.com/jetstack/cert-manager/pkg/logs" "github.com/jetstack/cert-manager/pkg/util" utilfeature "github.com/jetstack/cert-manager/pkg/util/feature" "github.com/jetstack/cert-manager/pkg/webhook" "github.com/jetstack/cert-manager/pkg/webhook/authority" "github.com/jetstack/cert-manager/pkg/webhook/configfile" - "github.com/jetstack/cert-manager/pkg/webhook/handlers" - "github.com/jetstack/cert-manager/pkg/webhook/server" - "github.com/jetstack/cert-manager/pkg/webhook/server/tls" ) -var validationHook handlers.ValidatingAdmissionHook = handlers.NewRegistryBackedValidator(logf.Log, webhook.Scheme, webhook.ValidationRegistry) -var mutationHook handlers.MutatingAdmissionHook = handlers.NewRegistryBackedMutator(logf.Log, webhook.Scheme, webhook.MutationRegistry) -var conversionHook handlers.ConversionHook = handlers.NewSchemeBackedConverter(logf.Log, webhook.Scheme) - -type ServerOption func(*server.Server) - -// WithConversionHandler allows you to override the handler for the `/convert` -// endpoint in tests. -func WithConversionHandler(handler handlers.ConversionHook) ServerOption { - return func(s *server.Server) { - s.ConversionWebhook = handler - } -} - -func NewServerWithOptions(log logr.Logger, _ options.WebhookFlags, opts config.WebhookConfiguration, optionFunctions ...ServerOption) (*server.Server, error) { - restcfg, err := clientcmd.BuildConfigFromFlags(opts.APIServerHost, opts.KubeConfig) - if err != nil { - return nil, err - } - - cl, err := kubernetes.NewForConfig(restcfg) - if err != nil { - return nil, fmt.Errorf("error creating kubernetes client: %s", err) - } - validationHook.InitPlugins(cl) - - var source tls.CertificateSource - switch { - case opts.TLSConfig.FilesystemConfigProvided(): - log.V(logf.InfoLevel).Info("using TLS certificate from local filesystem", "private_key_path", opts.TLSConfig.Filesystem.KeyFile, "certificate", opts.TLSConfig.Filesystem.CertFile) - source = &tls.FileCertificateSource{ - CertPath: opts.TLSConfig.Filesystem.CertFile, - KeyPath: opts.TLSConfig.Filesystem.KeyFile, - } - case opts.TLSConfig.DynamicConfigProvided(): - restcfg, err := clientcmd.BuildConfigFromFlags("", opts.KubeConfig) - if err != nil { - return nil, err - } - - log.V(logf.InfoLevel).Info("using dynamic certificate generating using CA stored in Secret resource", "secret_namespace", opts.TLSConfig.Dynamic.SecretNamespace, "secret_name", opts.TLSConfig.Dynamic.SecretName) - source = &tls.DynamicSource{ - DNSNames: opts.TLSConfig.Dynamic.DNSNames, - Authority: &authority.DynamicAuthority{ - SecretNamespace: opts.TLSConfig.Dynamic.SecretNamespace, - SecretName: opts.TLSConfig.Dynamic.SecretName, - RESTConfig: restcfg, - }, - } - default: - log.V(logf.WarnLevel).Info("serving insecurely as tls certificate data not provided") - } - - s := &server.Server{ - ListenAddr: fmt.Sprintf(":%d", *opts.SecurePort), - HealthzAddr: fmt.Sprintf(":%d", *opts.HealthzPort), - EnablePprof: opts.EnablePprof, - PprofAddr: opts.PprofAddress, - CertificateSource: source, - CipherSuites: opts.TLSConfig.CipherSuites, - MinTLSVersion: opts.TLSConfig.MinTLSVersion, - ValidationWebhook: validationHook, - MutationWebhook: mutationHook, - ConversionWebhook: conversionHook, - } - for _, f := range optionFunctions { - f(s) - } - return s, nil -} - const componentWebhook = "webhook" func NewServerCommand(stopCh <-chan struct{}) *cobra.Command { ctx := cmdutil.ContextWithStopCh(context.Background(), stopCh) log := logf.Log - ctx = logf.NewContext(ctx, log, "webhook") + ctx = logf.NewContext(ctx, log, componentWebhook) cleanFlagSet := pflag.NewFlagSet(componentWebhook, pflag.ContinueOnError) // Replaces all instances of `_` in flag names with `-` @@ -195,7 +119,7 @@ func NewServerCommand(stopCh <-chan struct{}) *cobra.Command { } } - srv, err := NewServerWithOptions(log, *webhookFlags, *webhookConfig) + srv, err := cmwebhook.NewCertManagerWebhookServer(log, *webhookFlags, *webhookConfig) if err != nil { log.Error(err, "Failed initialising server") os.Exit(1) diff --git a/internal/webhook/BUILD.bazel b/internal/webhook/BUILD.bazel index 9f730af59..8dc17c90f 100644 --- a/internal/webhook/BUILD.bazel +++ b/internal/webhook/BUILD.bazel @@ -14,6 +14,7 @@ go_library( "//internal/apis/certmanager/install:go_default_library", "//internal/apis/config/webhook:go_default_library", "//internal/apis/meta/install:go_default_library", + "//internal/plugin:go_default_library", "//pkg/logs:go_default_library", "//pkg/webhook/admission:go_default_library", "//pkg/webhook/admission/initializer:go_default_library", diff --git a/internal/webhook/webhook.go b/internal/webhook/webhook.go index 9efe998b8..d22e31aa5 100644 --- a/internal/webhook/webhook.go +++ b/internal/webhook/webhook.go @@ -45,9 +45,19 @@ import ( var conversionHook handlers.ConversionHook = handlers.NewSchemeBackedConverter(logf.Log, Scheme) +type ServerOption func(*server.Server) + +// WithConversionHandler allows you to override the handler for the `/convert` +// endpoint in tests. +func WithConversionHandler(handler handlers.ConversionHook) ServerOption { + return func(s *server.Server) { + s.ConversionWebhook = handler + } +} + // NewCertManagerWebhookServer creates a new webhook server configured with all cert-manager // resource types, validation, defaulting and conversion functions. -func NewCertManagerWebhookServer(log logr.Logger, _ options.WebhookFlags, opts config.WebhookConfiguration) (*server.Server, error) { +func NewCertManagerWebhookServer(log logr.Logger, _ options.WebhookFlags, opts config.WebhookConfiguration, optionFunctions ...ServerOption) (*server.Server, error) { restcfg, err := clientcmd.BuildConfigFromFlags(opts.APIServerHost, opts.KubeConfig) if err != nil { return nil, err @@ -59,12 +69,12 @@ func NewCertManagerWebhookServer(log logr.Logger, _ options.WebhookFlags, opts c } // Set up the admission chain - admissionHandler, err := buildAdmissionChain(log, cl) + admissionHandler, err := buildAdmissionChain(cl) if err != nil { return nil, err } - return &server.Server{ + s := &server.Server{ ListenAddr: fmt.Sprintf(":%d", *opts.SecurePort), HealthzAddr: fmt.Sprintf(":%d", *opts.HealthzPort), EnablePprof: opts.EnablePprof, @@ -75,13 +85,16 @@ func NewCertManagerWebhookServer(log logr.Logger, _ options.WebhookFlags, opts c ValidationWebhook: admissionHandler, MutationWebhook: admissionHandler, ConversionWebhook: conversionHook, - Log: log, - }, nil + } + for _, fn := range optionFunctions { + fn(s) + } + return s, nil } -func buildAdmissionChain(log logr.Logger, client kubernetes.Interface) (*admission.RequestHandler, error) { +func buildAdmissionChain(client kubernetes.Interface) (*admission.RequestHandler, error) { // Set up the admission chain - pluginHandler := admission.NewPlugins(log, Scheme) + pluginHandler := admission.NewPlugins(Scheme) plugin.RegisterAllPlugins(pluginHandler) authorizer, err := authorizerfactory.DelegatingAuthorizerConfig{ SubjectAccessReviewClient: client.AuthorizationV1(), @@ -115,7 +128,6 @@ func buildCertificateSource(log logr.Logger, tlsConfig config.TLSConfig, restCfg return &tls.FileCertificateSource{ CertPath: tlsConfig.Filesystem.CertFile, KeyPath: tlsConfig.Filesystem.KeyFile, - Log: log, } case tlsConfig.DynamicConfigProvided(): log.V(logf.InfoLevel).Info("using dynamic certificate generating using CA stored in Secret resource", "secret_namespace", tlsConfig.Dynamic.SecretNamespace, "secret_name", tlsConfig.Dynamic.SecretName) @@ -125,9 +137,7 @@ func buildCertificateSource(log logr.Logger, tlsConfig config.TLSConfig, restCfg SecretNamespace: tlsConfig.Dynamic.SecretNamespace, SecretName: tlsConfig.Dynamic.SecretName, RESTConfig: restCfg, - Log: log, }, - Log: log, } default: log.V(logf.WarnLevel).Info("serving insecurely as tls certificate data not provided") diff --git a/pkg/webhook/admission/BUILD.bazel b/pkg/webhook/admission/BUILD.bazel index 4279bb9b3..854d838b0 100644 --- a/pkg/webhook/admission/BUILD.bazel +++ b/pkg/webhook/admission/BUILD.bazel @@ -13,7 +13,6 @@ go_library( visibility = ["//:__subpackages__"], deps = [ "//pkg/webhook/handlers:go_default_library", - "@com_github_go_logr_logr//:go_default_library", "@io_k8s_api//admission/v1:go_default_library", "@io_k8s_apimachinery//pkg/apis/meta/v1:go_default_library", "@io_k8s_apimachinery//pkg/runtime:go_default_library", @@ -55,7 +54,6 @@ go_test( ], embed = [":go_default_library"], deps = [ - "//pkg/logs/testing:go_default_library", "//pkg/webhook/admission/initializer:go_default_library", "//pkg/webhook/handlers/testdata/apis/testgroup:go_default_library", "//pkg/webhook/handlers/testdata/apis/testgroup/install:go_default_library", diff --git a/pkg/webhook/admission/plugins.go b/pkg/webhook/admission/plugins.go index e49349bb5..4e0b8270b 100644 --- a/pkg/webhook/admission/plugins.go +++ b/pkg/webhook/admission/plugins.go @@ -19,7 +19,6 @@ package admission import ( "fmt" - "github.com/go-logr/logr" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/serializer" ) @@ -27,15 +26,13 @@ import ( // Plugins manages initialising, registering and executing admission plugins // for both validation and mutation. type Plugins struct { - log logr.Logger decoder runtime.Decoder pluginFactory map[string]Factory } -func NewPlugins(log logr.Logger, scheme *runtime.Scheme) *Plugins { +func NewPlugins(scheme *runtime.Scheme) *Plugins { return &Plugins{ - log: log, decoder: serializer.NewCodecFactory(scheme).UniversalDecoder(), pluginFactory: make(map[string]Factory), } diff --git a/pkg/webhook/admission/plugins_test.go b/pkg/webhook/admission/plugins_test.go index e4223c6c0..180ba2fa2 100644 --- a/pkg/webhook/admission/plugins_test.go +++ b/pkg/webhook/admission/plugins_test.go @@ -25,14 +25,13 @@ import ( "k8s.io/client-go/kubernetes" "k8s.io/client-go/kubernetes/fake" - log "github.com/jetstack/cert-manager/pkg/logs/testing" "github.com/jetstack/cert-manager/pkg/webhook/admission" "github.com/jetstack/cert-manager/pkg/webhook/admission/initializer" ) func TestPlugins_InitializesNamedOnly(t *testing.T) { scheme := runtime.NewScheme() - p := admission.NewPlugins(&log.TestLogger{T: t}, scheme) + p := admission.NewPlugins(scheme) testPlugin1 := &testPlugin{} p.Register("TestPlugin1", func() (admission.Interface, error) { @@ -61,7 +60,7 @@ func TestPlugins_InitializesNamedOnly(t *testing.T) { func TestPlugins_FailsIfAnyPluginFails(t *testing.T) { scheme := runtime.NewScheme() - p := admission.NewPlugins(&log.TestLogger{T: t}, scheme) + p := admission.NewPlugins(scheme) testPlugin1 := &testPlugin{} p.Register("TestPlugin1", func() (admission.Interface, error) { @@ -90,7 +89,7 @@ func TestPlugins_FailsIfAnyPluginFails(t *testing.T) { func TestPlugins_FailsNonExistingPlugin(t *testing.T) { scheme := runtime.NewScheme() - p := admission.NewPlugins(&log.TestLogger{T: t}, scheme) + p := admission.NewPlugins(scheme) testPlugin1 := &testPlugin{} p.Register("TestPlugin1", func() (admission.Interface, error) { @@ -109,7 +108,7 @@ func TestPlugins_FailsNonExistingPlugin(t *testing.T) { func TestPlugins_FailsIfPluginFailsToBuild(t *testing.T) { scheme := runtime.NewScheme() - p := admission.NewPlugins(&log.TestLogger{T: t}, scheme) + p := admission.NewPlugins(scheme) testPlugin1 := &testPlugin{} p.Register("TestPlugin1", func() (admission.Interface, error) { diff --git a/test/integration/framework/BUILD.bazel b/test/integration/framework/BUILD.bazel index 9ebf92b05..6894ff279 100644 --- a/test/integration/framework/BUILD.bazel +++ b/test/integration/framework/BUILD.bazel @@ -9,9 +9,9 @@ go_library( importpath = "github.com/jetstack/cert-manager/test/integration/framework", visibility = ["//visibility:public"], deps = [ - "//cmd/webhook/app:go_default_library", "//cmd/webhook/app/testing:go_default_library", "//internal/test/paths:go_default_library", + "//internal/webhook:go_default_library", "//pkg/api:go_default_library", "//pkg/client/clientset/versioned:go_default_library", "//pkg/client/informers/externalversions:go_default_library", diff --git a/test/integration/framework/apiserver.go b/test/integration/framework/apiserver.go index 752db3b4a..a6fe9031f 100644 --- a/test/integration/framework/apiserver.go +++ b/test/integration/framework/apiserver.go @@ -38,9 +38,9 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/envtest" - "github.com/jetstack/cert-manager/cmd/webhook/app" webhooktesting "github.com/jetstack/cert-manager/cmd/webhook/app/testing" "github.com/jetstack/cert-manager/internal/test/paths" + "github.com/jetstack/cert-manager/internal/webhook" "github.com/jetstack/cert-manager/pkg/api" "github.com/jetstack/cert-manager/pkg/webhook/handlers" "github.com/jetstack/cert-manager/test/internal/apiserver" @@ -87,7 +87,7 @@ func RunControlPlane(t *testing.T, ctx context.Context, optionFunctions ...RunCo webhookOpts, stopWebhook := webhooktesting.StartWebhookServer( t, ctx, []string{"--api-server-host=" + config.Host}, - app.WithConversionHandler(options.webhookConversionHandler), + webhook.WithConversionHandler(options.webhookConversionHandler), ) crds := readCustomResourcesAtPath(t, *options.crdsDir)