From 5458173739ccfcf5ba6f06a35674932c2e2dcc52 Mon Sep 17 00:00:00 2001 From: Inteon <42113979+inteon@users.noreply.github.com> Date: Mon, 12 Jul 2021 16:54:31 +0200 Subject: [PATCH 1/3] Add kubectl 'cert-manager check api' command Signed-off-by: Inteon <42113979+inteon@users.noreply.github.com> --- cmd/ctl/BUILD.bazel | 1 + cmd/ctl/cmd/BUILD.bazel | 1 + cmd/ctl/cmd/cmd.go | 2 + cmd/ctl/pkg/check/BUILD.bazel | 31 ++++++ cmd/ctl/pkg/check/api/BUILD.bazel | 30 ++++++ cmd/ctl/pkg/check/api/api.go | 137 ++++++++++++++++++++++++++ cmd/ctl/pkg/check/check.go | 43 ++++++++ pkg/util/BUILD.bazel | 1 + pkg/util/cmapichecker/BUILD.bazel | 31 ++++++ pkg/util/cmapichecker/cmapichecker.go | 115 +++++++++++++++++++++ 10 files changed, 392 insertions(+) create mode 100644 cmd/ctl/pkg/check/BUILD.bazel create mode 100644 cmd/ctl/pkg/check/api/BUILD.bazel create mode 100644 cmd/ctl/pkg/check/api/api.go create mode 100644 cmd/ctl/pkg/check/check.go create mode 100644 pkg/util/cmapichecker/BUILD.bazel create mode 100644 pkg/util/cmapichecker/cmapichecker.go diff --git a/cmd/ctl/BUILD.bazel b/cmd/ctl/BUILD.bazel index bc00537bf..d332cde59 100644 --- a/cmd/ctl/BUILD.bazel +++ b/cmd/ctl/BUILD.bazel @@ -34,6 +34,7 @@ filegroup( ":package-srcs", "//cmd/ctl/cmd:all-srcs", "//cmd/ctl/pkg/approve:all-srcs", + "//cmd/ctl/pkg/check:all-srcs", "//cmd/ctl/pkg/convert:all-srcs", "//cmd/ctl/pkg/create:all-srcs", "//cmd/ctl/pkg/deny:all-srcs", diff --git a/cmd/ctl/cmd/BUILD.bazel b/cmd/ctl/cmd/BUILD.bazel index 20e04c990..5133b25c0 100644 --- a/cmd/ctl/cmd/BUILD.bazel +++ b/cmd/ctl/cmd/BUILD.bazel @@ -7,6 +7,7 @@ go_library( visibility = ["//visibility:public"], deps = [ "//cmd/ctl/pkg/approve:go_default_library", + "//cmd/ctl/pkg/check:go_default_library", "//cmd/ctl/pkg/convert:go_default_library", "//cmd/ctl/pkg/create:go_default_library", "//cmd/ctl/pkg/deny:go_default_library", diff --git a/cmd/ctl/cmd/cmd.go b/cmd/ctl/cmd/cmd.go index 4df1d7a95..9a45adc3e 100644 --- a/cmd/ctl/cmd/cmd.go +++ b/cmd/ctl/cmd/cmd.go @@ -31,6 +31,7 @@ import ( cmdutil "k8s.io/kubectl/pkg/cmd/util" "github.com/jetstack/cert-manager/cmd/ctl/pkg/approve" + "github.com/jetstack/cert-manager/cmd/ctl/pkg/check" "github.com/jetstack/cert-manager/cmd/ctl/pkg/convert" "github.com/jetstack/cert-manager/cmd/ctl/pkg/create" "github.com/jetstack/cert-manager/cmd/ctl/pkg/deny" @@ -74,6 +75,7 @@ kubectl cert-manager is a CLI tool manage and configure cert-manager resources f cmds.AddCommand(inspect.NewCmdInspect(ctx, ioStreams, factory)) cmds.AddCommand(approve.NewCmdApprove(ctx, ioStreams, factory)) cmds.AddCommand(deny.NewCmdDeny(ctx, ioStreams, factory)) + cmds.AddCommand(check.NewCmdCheck(ctx, ioStreams, factory)) // Experimental features cmds.AddCommand(experimental.NewCmdExperimental(ctx, ioStreams, factory)) diff --git a/cmd/ctl/pkg/check/BUILD.bazel b/cmd/ctl/pkg/check/BUILD.bazel new file mode 100644 index 000000000..40037553c --- /dev/null +++ b/cmd/ctl/pkg/check/BUILD.bazel @@ -0,0 +1,31 @@ +load("@io_bazel_rules_go//go:def.bzl", "go_library") + +go_library( + name = "go_default_library", + srcs = ["check.go"], + importpath = "github.com/jetstack/cert-manager/cmd/ctl/pkg/check", + visibility = ["//visibility:public"], + deps = [ + "//cmd/ctl/pkg/check/api:go_default_library", + "@com_github_spf13_cobra//:go_default_library", + "@io_k8s_cli_runtime//pkg/genericclioptions:go_default_library", + "@io_k8s_kubectl//pkg/cmd/util:go_default_library", + ], +) + +filegroup( + name = "package-srcs", + srcs = glob(["**"]), + tags = ["automanaged"], + visibility = ["//visibility:private"], +) + +filegroup( + name = "all-srcs", + srcs = [ + ":package-srcs", + "//cmd/ctl/pkg/check/api:all-srcs", + ], + tags = ["automanaged"], + visibility = ["//visibility:public"], +) diff --git a/cmd/ctl/pkg/check/api/BUILD.bazel b/cmd/ctl/pkg/check/api/BUILD.bazel new file mode 100644 index 000000000..e73033f49 --- /dev/null +++ b/cmd/ctl/pkg/check/api/BUILD.bazel @@ -0,0 +1,30 @@ +load("@io_bazel_rules_go//go:def.bzl", "go_library") + +go_library( + name = "go_default_library", + srcs = ["api.go"], + importpath = "github.com/jetstack/cert-manager/cmd/ctl/pkg/check/api", + visibility = ["//visibility:public"], + deps = [ + "//pkg/util/cmapichecker:go_default_library", + "@com_github_spf13_cobra//:go_default_library", + "@io_k8s_apimachinery//pkg/util/wait:go_default_library", + "@io_k8s_cli_runtime//pkg/genericclioptions:go_default_library", + "@io_k8s_client_go//rest:go_default_library", + "@io_k8s_kubectl//pkg/cmd/util:go_default_library", + ], +) + +filegroup( + name = "package-srcs", + srcs = glob(["**"]), + tags = ["automanaged"], + visibility = ["//visibility:private"], +) + +filegroup( + name = "all-srcs", + srcs = [":package-srcs"], + tags = ["automanaged"], + visibility = ["//visibility:public"], +) diff --git a/cmd/ctl/pkg/check/api/api.go b/cmd/ctl/pkg/check/api/api.go new file mode 100644 index 000000000..10c2a9324 --- /dev/null +++ b/cmd/ctl/pkg/check/api/api.go @@ -0,0 +1,137 @@ +/* +Copyright 2021 The cert-manager 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 api + +import ( + "context" + "log" + "time" + + "github.com/jetstack/cert-manager/pkg/util/cmapichecker" + "github.com/spf13/cobra" + "k8s.io/apimachinery/pkg/util/wait" + "k8s.io/cli-runtime/pkg/genericclioptions" + restclient "k8s.io/client-go/rest" + cmdutil "k8s.io/kubectl/pkg/cmd/util" +) + +// Options is a struct to support check api command +type Options struct { + RESTConfig *restclient.Config + + // APIChecker is used to check that the cert-manager CRDs have been installed on the K8S + // API server and that the cert-manager webhooks are all working + APIChecker cmapichecker.Interface + + // If set to true, command will wait until creating resources against the api is possible + Wait bool + + // Time before timeout when waiting + Timeout time.Duration + + // Time between checks when waiting + Interval time.Duration + + // Namespace that is used to dry-run create the certificate resource in + Namespace string + + genericclioptions.IOStreams +} + +// NewOptions returns initialized Options +func NewOptions(ioStreams genericclioptions.IOStreams) *Options { + return &Options{ + IOStreams: ioStreams, + } +} + +// Complete takes the command arguments and factory and infers any remaining options. +func (o *Options) Complete(factory cmdutil.Factory) error { + var err error + + o.Namespace, _, err = factory.ToRawKubeConfigLoader().Namespace() + if err != nil { + return err + } + + o.RESTConfig, err = factory.ToRESTConfig() + if err != nil { + return err + } + + o.APIChecker, err = cmapichecker.New(o.RESTConfig, o.Namespace) + if err != nil { + return err + } + + return nil +} + +// NewCmdCheckApi returns a cobra command for checking creating cert-manager resources against the K8S API server +func NewCmdCheckApi(ctx context.Context, ioStreams genericclioptions.IOStreams, factory cmdutil.Factory) *cobra.Command { + o := NewOptions(ioStreams) + + cmd := &cobra.Command{ + Use: "api", + Short: ` + This check attempts to perform a dry-run create of a cert-manager *v1alpha2* + Certificate resource in order to verify that CRDs are installed and all the + required webhooks are reachable by the K8S API server. + We use v1alpha2 API to ensure that the API server has also connected to the + cert-manager conversion webhook. + `, + RunE: func(cmd *cobra.Command, args []string) error { + if err := o.Complete(factory); err != nil { + return err + } + return o.Run(ctx) + }, + SilenceUsage: true, + SilenceErrors: true, + } + cmd.Flags().BoolVar(&o.Wait, "wait", true, "If set to true, command will wait until creating resources against the api is possible") + cmd.Flags().DurationVar(&o.Timeout, "timeout", 30*time.Second, "Time before timeout when waiting, must include unit, e.g. 5s or 10m") + cmd.Flags().DurationVar(&o.Interval, "interval", 5*time.Second, "Time between checks when waiting, must include unit, e.g. 5s or 10m") + + return cmd +} + +// Run executes check api command +func (o *Options) Run(ctx context.Context) error { + log.SetFlags(0) // Disable prefixing logs with timestamps. + + if !o.Wait { + if err := o.APIChecker.Check(ctx); err != nil { + return err + } + + log.Print("The Kubernetes Api is ready to created cert-manager resources against") + + return nil + } + + return wait.PollImmediate(o.Interval, o.Timeout, func() (done bool, err error) { + if err := o.APIChecker.Check(ctx); err != nil { + log.Printf("%v", err) + return false, nil + } + + log.Print("The Kubernetes Api is ready to created cert-manager resources against") + + return true, nil + }) +} diff --git a/cmd/ctl/pkg/check/check.go b/cmd/ctl/pkg/check/check.go new file mode 100644 index 000000000..a2c720f6c --- /dev/null +++ b/cmd/ctl/pkg/check/check.go @@ -0,0 +1,43 @@ +/* +Copyright 2021 The cert-manager 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 check + +import ( + "context" + + "github.com/spf13/cobra" + "k8s.io/cli-runtime/pkg/genericclioptions" + cmdutil "k8s.io/kubectl/pkg/cmd/util" + + "github.com/jetstack/cert-manager/cmd/ctl/pkg/check/api" +) + +func NewCmdCheck(ctx context.Context, ioStreams genericclioptions.IOStreams, factory cmdutil.Factory) *cobra.Command { + cmds := NewCmdCreateBare() + cmds.AddCommand(api.NewCmdCheckApi(ctx, ioStreams, factory)) + + return cmds +} + +// Create a bare Create Command, without any subcommands +func NewCmdCreateBare() *cobra.Command { + return &cobra.Command{ + Use: "check", + Short: "Check cert-manager components", + Long: `Check cert-manager components`, + } +} diff --git a/pkg/util/BUILD.bazel b/pkg/util/BUILD.bazel index af4917bb8..c4b848a82 100644 --- a/pkg/util/BUILD.bazel +++ b/pkg/util/BUILD.bazel @@ -33,6 +33,7 @@ filegroup( name = "all-srcs", srcs = [ ":package-srcs", + "//pkg/util/cmapichecker:all-srcs", "//pkg/util/cmd:all-srcs", "//pkg/util/coverage:all-srcs", "//pkg/util/errors:all-srcs", diff --git a/pkg/util/cmapichecker/BUILD.bazel b/pkg/util/cmapichecker/BUILD.bazel new file mode 100644 index 000000000..e7797e835 --- /dev/null +++ b/pkg/util/cmapichecker/BUILD.bazel @@ -0,0 +1,31 @@ +load("@io_bazel_rules_go//go:def.bzl", "go_library") + +go_library( + name = "go_default_library", + srcs = ["cmapichecker.go"], + importpath = "github.com/jetstack/cert-manager/pkg/util/cmapichecker", + visibility = ["//visibility:public"], + deps = [ + "//pkg/apis/certmanager/v1alpha2:go_default_library", + "//pkg/apis/meta/v1:go_default_library", + "@com_github_pkg_errors//:go_default_library", + "@io_k8s_apimachinery//pkg/apis/meta/v1:go_default_library", + "@io_k8s_apimachinery//pkg/runtime:go_default_library", + "@io_k8s_client_go//rest:go_default_library", + "@io_k8s_sigs_controller_runtime//pkg/client:go_default_library", + ], +) + +filegroup( + name = "package-srcs", + srcs = glob(["**"]), + tags = ["automanaged"], + visibility = ["//visibility:private"], +) + +filegroup( + name = "all-srcs", + srcs = [":package-srcs"], + tags = ["automanaged"], + visibility = ["//visibility:public"], +) diff --git a/pkg/util/cmapichecker/cmapichecker.go b/pkg/util/cmapichecker/cmapichecker.go new file mode 100644 index 000000000..0492caa77 --- /dev/null +++ b/pkg/util/cmapichecker/cmapichecker.go @@ -0,0 +1,115 @@ +/* +Copyright 2021 The cert-manager 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 cmapichecker + +import ( + "context" + + errors "github.com/pkg/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/rest" + "sigs.k8s.io/controller-runtime/pkg/client" + + // Use v1alpha2 API to ensure that the API server has also connected to the + // cert-manager conversion webhook. + // TODO(wallrj): Only change this when the old deprecated APIs are removed, + // at which point the conversion webhook may be removed anyway. + cmapi "github.com/jetstack/cert-manager/pkg/apis/certmanager/v1alpha2" + cmmeta "github.com/jetstack/cert-manager/pkg/apis/meta/v1" +) + +// Interface is used to check that the cert-manager CRDs have been installed and are usable. +type Interface interface { + Check(context.Context) error +} + +type cmapiChecker struct { + // The client controller-runtime client.New function fails if can't reach + // the API server, so we load it lazily, to avoid breaking integration tests + // which rely on being able to start the webhook server before the API + // server. + clientBuilder func() (client.Client, error) + + client client.Client +} + +// New returns a cert-manager API checker +func New(restcfg *rest.Config, namespace string) (Interface, error) { + scheme := runtime.NewScheme() + if err := cmapi.AddToScheme(scheme); err != nil { + return nil, errors.Wrap(err, "while configuring scheme") + } + return &cmapiChecker{ + clientBuilder: func() (client.Client, error) { + cl, err := client.New(restcfg, client.Options{ + Scheme: scheme, + }) + if err != nil { + return nil, errors.Wrap(err, "while creating client") + } + return client.NewNamespacedClient(client.NewDryRunClient(cl), namespace), nil + }, + }, nil +} + +func (o *cmapiChecker) Client() (client.Client, error) { + if o.client != nil { + return o.client, nil + } + + cl, err := o.clientBuilder() + if err != nil { + return nil, err + } + o.client = cl + + return o.client, nil +} + +// Check attempts to perform a dry-run create of a cert-manager *v1alpha2* +// Certificate resource in order to verify that CRDs are installed and all the +// required webhooks are reachable by the K8S API server. +// We use v1alpha2 API to ensure that the API server has also connected to the +// cert-manager conversion webhook. +func (o *cmapiChecker) Check(ctx context.Context) error { + cert := &cmapi.Certificate{ + TypeMeta: metav1.TypeMeta{ + Kind: "Certificate", + APIVersion: "cert-manager.io/v1alpha2", + }, + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "cmapichecker-", + }, + Spec: cmapi.CertificateSpec{ + DNSNames: []string{"cmapichecker.example"}, + SecretName: "cmapichecker", + IssuerRef: cmmeta.ObjectReference{ + Name: "cmapichecker", + }, + }, + } + cl, err := o.Client() + if err != nil { + return err + } + + if err := cl.Create(ctx, cert); err != nil { + return errors.Wrap(err, "while attempting dry-run creation of Certificate") + } + return nil +} From ac7775bdb4d0d11e7b25e24e0c5b1957973a0d6f Mon Sep 17 00:00:00 2001 From: Inteon <42113979+inteon@users.noreply.github.com> Date: Thu, 15 Jul 2021 14:32:54 +0200 Subject: [PATCH 2/3] made errors human readable, added unit tests, added check api to e2e, fixed os.Exit(1) Signed-off-by: Inteon <42113979+inteon@users.noreply.github.com> --- cmd/ctl/main.go | 1 + cmd/ctl/pkg/check/api/BUILD.bazel | 4 +- cmd/ctl/pkg/check/api/api.go | 90 +++++++------ devel/addon/certmanager/install.sh | 3 + pkg/util/cmapichecker/BUILD.bazel | 14 +- pkg/util/cmapichecker/cmapichecker.go | 71 ++++++++-- pkg/util/cmapichecker/cmapichecker_test.go | 144 +++++++++++++++++++++ 7 files changed, 275 insertions(+), 52 deletions(-) create mode 100644 pkg/util/cmapichecker/cmapichecker_test.go diff --git a/cmd/ctl/main.go b/cmd/ctl/main.go index eae38ee45..2bc2ef1e4 100644 --- a/cmd/ctl/main.go +++ b/cmd/ctl/main.go @@ -33,5 +33,6 @@ func main() { if err := cmd.Execute(); err != nil { fmt.Fprintf(os.Stderr, "%s\n", err) + os.Exit(1) } } diff --git a/cmd/ctl/pkg/check/api/BUILD.bazel b/cmd/ctl/pkg/check/api/BUILD.bazel index e73033f49..b23225aec 100644 --- a/cmd/ctl/pkg/check/api/BUILD.bazel +++ b/cmd/ctl/pkg/check/api/BUILD.bazel @@ -10,8 +10,10 @@ go_library( "@com_github_spf13_cobra//:go_default_library", "@io_k8s_apimachinery//pkg/util/wait:go_default_library", "@io_k8s_cli_runtime//pkg/genericclioptions:go_default_library", - "@io_k8s_client_go//rest:go_default_library", "@io_k8s_kubectl//pkg/cmd/util:go_default_library", + "@io_k8s_kubectl//pkg/scheme:go_default_library", + "@io_k8s_kubectl//pkg/util/i18n:go_default_library", + "@io_k8s_kubectl//pkg/util/templates:go_default_library", ], ) diff --git a/cmd/ctl/pkg/check/api/api.go b/cmd/ctl/pkg/check/api/api.go index 10c2a9324..67b503322 100644 --- a/cmd/ctl/pkg/check/api/api.go +++ b/cmd/ctl/pkg/check/api/api.go @@ -18,30 +18,28 @@ package api import ( "context" - "log" + "fmt" "time" - "github.com/jetstack/cert-manager/pkg/util/cmapichecker" "github.com/spf13/cobra" "k8s.io/apimachinery/pkg/util/wait" "k8s.io/cli-runtime/pkg/genericclioptions" - restclient "k8s.io/client-go/rest" cmdutil "k8s.io/kubectl/pkg/cmd/util" + "k8s.io/kubectl/pkg/scheme" + "k8s.io/kubectl/pkg/util/i18n" + "k8s.io/kubectl/pkg/util/templates" + + "github.com/jetstack/cert-manager/pkg/util/cmapichecker" ) // Options is a struct to support check api command type Options struct { - RESTConfig *restclient.Config - // APIChecker is used to check that the cert-manager CRDs have been installed on the K8S // API server and that the cert-manager webhooks are all working APIChecker cmapichecker.Interface - // If set to true, command will wait until creating resources against the api is possible - Wait bool - // Time before timeout when waiting - Timeout time.Duration + Wait time.Duration // Time between checks when waiting Interval time.Duration @@ -49,9 +47,19 @@ type Options struct { // Namespace that is used to dry-run create the certificate resource in Namespace string + // Print details regarding encountered errors + Verbose bool + genericclioptions.IOStreams } +var checkApiDesc = templates.LongDesc(i18n.T(` +This check attempts to perform a dry-run create of a cert-manager *v1alpha2* +Certificate resource in order to verify that CRDs are installed and all the +required webhooks are reachable by the K8S API server. +We use v1alpha2 API to ensure that the API server has also connected to the +cert-manager conversion webhook.`)) + // NewOptions returns initialized Options func NewOptions(ioStreams genericclioptions.IOStreams) *Options { return &Options{ @@ -65,17 +73,20 @@ func (o *Options) Complete(factory cmdutil.Factory) error { o.Namespace, _, err = factory.ToRawKubeConfigLoader().Namespace() if err != nil { - return err + return fmt.Errorf("Error: cannot get the namespace: %v", err) } - o.RESTConfig, err = factory.ToRESTConfig() + restConfig, err := factory.ToRESTConfig() if err != nil { - return err + return fmt.Errorf("Error: cannot create the REST config: %v", err) } - o.APIChecker, err = cmapichecker.New(o.RESTConfig, o.Namespace) + // We pass the scheme that is used in the RESTConfig's NegotiatedSerializer, + // this makes sure that the cmapi is also added to NegotiatedSerializer's scheme + // see: https://github.com/jetstack/cert-manager/pull/4205#discussion_r668660271 + o.APIChecker, err = cmapichecker.New(restConfig, scheme.Scheme, o.Namespace) if err != nil { - return err + return fmt.Errorf("Error: %v", err) } return nil @@ -86,14 +97,9 @@ func NewCmdCheckApi(ctx context.Context, ioStreams genericclioptions.IOStreams, o := NewOptions(ioStreams) cmd := &cobra.Command{ - Use: "api", - Short: ` - This check attempts to perform a dry-run create of a cert-manager *v1alpha2* - Certificate resource in order to verify that CRDs are installed and all the - required webhooks are reachable by the K8S API server. - We use v1alpha2 API to ensure that the API server has also connected to the - cert-manager conversion webhook. - `, + Use: "api", + Short: "This check attempts to perform a dry-run create of a cert-manager Certificate", + Long: checkApiDesc, RunE: func(cmd *cobra.Command, args []string) error { if err := o.Complete(factory); err != nil { return err @@ -103,35 +109,39 @@ func NewCmdCheckApi(ctx context.Context, ioStreams genericclioptions.IOStreams, SilenceUsage: true, SilenceErrors: true, } - cmd.Flags().BoolVar(&o.Wait, "wait", true, "If set to true, command will wait until creating resources against the api is possible") - cmd.Flags().DurationVar(&o.Timeout, "timeout", 30*time.Second, "Time before timeout when waiting, must include unit, e.g. 5s or 10m") - cmd.Flags().DurationVar(&o.Interval, "interval", 5*time.Second, "Time between checks when waiting, must include unit, e.g. 5s or 10m") + cmd.Flags().DurationVar(&o.Wait, "wait", 1*time.Minute, "Time before timeout when waiting, must include unit, e.g. 0s or 20s") + cmd.Flags().DurationVar(&o.Interval, "interval", 5*time.Second, "Time between checks when waiting, must include unit, e.g. 1m or 10m") + cmd.Flags().BoolVarP(&o.Verbose, "verbose", "v", false, "Print details regarding encountered errors") return cmd } // Run executes check api command func (o *Options) Run(ctx context.Context) error { - log.SetFlags(0) // Disable prefixing logs with timestamps. + pollContext, cancel := context.WithTimeout(ctx, o.Wait) + defer cancel() - if !o.Wait { + pollErr := wait.PollImmediateUntil(o.Interval, func() (done bool, err error) { if err := o.APIChecker.Check(ctx); err != nil { - return err - } - - log.Print("The Kubernetes Api is ready to created cert-manager resources against") - - return nil - } - - return wait.PollImmediate(o.Interval, o.Timeout, func() (done bool, err error) { - if err := o.APIChecker.Check(ctx); err != nil { - log.Printf("%v", err) + if o.Verbose { + fmt.Fprintf(o.ErrOut, "Not ready: %v (%v)\n", err, err.Cause()) + } else { + fmt.Fprintf(o.ErrOut, "Not ready: %v\n", err) + } return false, nil } - log.Print("The Kubernetes Api is ready to created cert-manager resources against") + fmt.Fprintln(o.Out, "The cert-manager API is ready") return true, nil - }) + }, pollContext.Done()) + + if pollErr != nil { + if ctx.Err() != nil { + return ctx.Err() + } + return pollErr + } + + return nil } diff --git a/devel/addon/certmanager/install.sh b/devel/addon/certmanager/install.sh index 8745bf1d6..1a151a30f 100755 --- a/devel/addon/certmanager/install.sh +++ b/devel/addon/certmanager/install.sh @@ -31,6 +31,7 @@ SCRIPT_ROOT=$(dirname "${BASH_SOURCE}") # Require kubectl & helm available on PATH check_tool kubectl +check_tool kubectl-cert_manager check_tool helm # Use the current timestamp as the APP_VERSION so a rolling update will be @@ -65,3 +66,5 @@ helm upgrade \ --set "extraArgs={--dns01-recursive-nameservers=${SERVICE_IP_PREFIX}.16:53,--dns01-recursive-nameservers-only=true}" \ "$RELEASE_NAME" \ "$REPO_ROOT/bazel-bin/deploy/charts/cert-manager/cert-manager.tgz" + +kubectl cert-manager check api diff --git a/pkg/util/cmapichecker/BUILD.bazel b/pkg/util/cmapichecker/BUILD.bazel index e7797e835..0b4ce65d6 100644 --- a/pkg/util/cmapichecker/BUILD.bazel +++ b/pkg/util/cmapichecker/BUILD.bazel @@ -1,4 +1,4 @@ -load("@io_bazel_rules_go//go:def.bzl", "go_library") +load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test") go_library( name = "go_default_library", @@ -29,3 +29,15 @@ filegroup( tags = ["automanaged"], visibility = ["//visibility:public"], ) + +go_test( + name = "go_default_test", + srcs = ["cmapichecker_test.go"], + embed = [":go_default_library"], + deps = [ + "//pkg/apis/certmanager/v1alpha2:go_default_library", + "@io_k8s_apimachinery//pkg/runtime:go_default_library", + "@io_k8s_sigs_controller_runtime//pkg/client:go_default_library", + "@io_k8s_sigs_controller_runtime//pkg/client/fake:go_default_library", + ], +) diff --git a/pkg/util/cmapichecker/cmapichecker.go b/pkg/util/cmapichecker/cmapichecker.go index 0492caa77..4a3fde241 100644 --- a/pkg/util/cmapichecker/cmapichecker.go +++ b/pkg/util/cmapichecker/cmapichecker.go @@ -18,6 +18,7 @@ package cmapichecker import ( "context" + "regexp" errors "github.com/pkg/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -33,9 +34,35 @@ import ( cmmeta "github.com/jetstack/cert-manager/pkg/apis/meta/v1" ) +var ( + ErrAPIServerUnreachable = errors.New("unable to connect to the Kubernetes API server") + ErrCertManagerCRDsNotFound = errors.New("the cert-manager CRDs are not yet installed on the Kubernetes API server") + ErrCertManagerAPIEndpointsNotEstablished = errors.New("the cert-manager API endpoints have not yet been published by the Kubernetes API server") + ErrWebhookConnectionFailure = errors.New("the cert-manager webhook server can't be reached yet") + ErrWebhookCertificateFailure = errors.New("the client CA bundle is not yet updated to the certificate of the cert-manager webhook") + + regexErrCertManagerCRDsNotFound = regexp.MustCompile(`^error finding the scope of the object: failed to get restmapping: no matches for kind "Certificate" in group "cert-manager.io"$`) + regexErrCertManagerAPIEndpointsNotEstablished = regexp.MustCompile(`failed calling webhook "(.*)\.cert-manager\.io": Post "(.*)\/mutate(.*)": service "(.*)-webhook" not found$`) + regexErrWebhookConnectionFailure = regexp.MustCompile(`failed calling webhook "(.*)\.cert-manager\.io": Post "(.*)\/mutate(.*)": (.*): connect: connection refused$`) + regexErrWebhookCertificateFailure = regexp.MustCompile(`Post "(.*)": x509: certificate signed by unknown authority`) +) + +type ApiCheckError struct { + SimpleError error + UnderlyingError error +} + +func (e *ApiCheckError) Error() string { + return e.SimpleError.Error() +} + +func (e *ApiCheckError) Cause() error { + return e.UnderlyingError +} + // Interface is used to check that the cert-manager CRDs have been installed and are usable. type Interface interface { - Check(context.Context) error + Check(context.Context) *ApiCheckError } type cmapiChecker struct { @@ -49,8 +76,7 @@ type cmapiChecker struct { } // New returns a cert-manager API checker -func New(restcfg *rest.Config, namespace string) (Interface, error) { - scheme := runtime.NewScheme() +func New(restcfg *rest.Config, scheme *runtime.Scheme, namespace string) (Interface, error) { if err := cmapi.AddToScheme(scheme); err != nil { return nil, errors.Wrap(err, "while configuring scheme") } @@ -86,12 +112,8 @@ func (o *cmapiChecker) Client() (client.Client, error) { // required webhooks are reachable by the K8S API server. // We use v1alpha2 API to ensure that the API server has also connected to the // cert-manager conversion webhook. -func (o *cmapiChecker) Check(ctx context.Context) error { +func (o *cmapiChecker) Check(ctx context.Context) *ApiCheckError { cert := &cmapi.Certificate{ - TypeMeta: metav1.TypeMeta{ - Kind: "Certificate", - APIVersion: "cert-manager.io/v1alpha2", - }, ObjectMeta: metav1.ObjectMeta{ GenerateName: "cmapichecker-", }, @@ -103,13 +125,42 @@ func (o *cmapiChecker) Check(ctx context.Context) error { }, }, } + + // while creating client: Get "http://localhost:8080/api?timeout=32s": dial tcp 127.0.0.1:8080: connect: connection refused cl, err := o.Client() if err != nil { - return err + return &ApiCheckError{ + SimpleError: ErrAPIServerUnreachable, + UnderlyingError: err, + } } + // error finding the scope of the object: failed to get restmapping: no matches for kind "Certificate" in group "cert-manager.io" + // Internal error occurred: failed calling webhook "webhook.cert-manager.io": Post "https://cert-manager-webhook.cert-manager.svc:443/mutate?timeout=10s": service "cert-manager-webhook" not found + // Internal error occurred: failed calling webhook "webhook.cert-manager.io": Post "https://cert-manager-webhook.cert-manager.svc:443/mutate?timeout=10s": dial tcp 10.96.38.90:443: connect: connection refused + // Internal error occurred: failed calling webhook "webhook.cert-manager.io": Post "https://cert-manager-webhook.cert-manager.svc:443/mutate?timeout=10s": x509: certificate signed by unknown authority (possibly because of "x509: ECDSA verification failure" while trying to verify candidate authority certificate "cert-manager-webhook-ca") + // conversion webhook for cert-manager.io/v1alpha2, Kind=Certificate failed: Post "https://cert-manager-webhook.cert-manager.svc:443/convert?timeout=30s": x509: certificate signed by unknown authority if err := cl.Create(ctx, cert); err != nil { - return errors.Wrap(err, "while attempting dry-run creation of Certificate") + return &ApiCheckError{ + SimpleError: translateToSimpleError(err), + UnderlyingError: err, + } } return nil } + +func translateToSimpleError(err error) error { + s := err.Error() + + if regexErrCertManagerCRDsNotFound.MatchString(s) { + return ErrCertManagerCRDsNotFound + } else if regexErrCertManagerAPIEndpointsNotEstablished.MatchString(s) { + return ErrCertManagerAPIEndpointsNotEstablished + } else if regexErrWebhookConnectionFailure.MatchString(s) { + return ErrWebhookConnectionFailure + } else if regexErrWebhookCertificateFailure.MatchString(s) { + return ErrWebhookCertificateFailure + } + + return err +} diff --git a/pkg/util/cmapichecker/cmapichecker_test.go b/pkg/util/cmapichecker/cmapichecker_test.go new file mode 100644 index 000000000..0d381fcdb --- /dev/null +++ b/pkg/util/cmapichecker/cmapichecker_test.go @@ -0,0 +1,144 @@ +/* +Copyright 2021 The cert-manager 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 cmapichecker + +import ( + "context" + "errors" + "testing" + + "k8s.io/apimachinery/pkg/runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + cmapi "github.com/jetstack/cert-manager/pkg/apis/certmanager/v1alpha2" +) + +type fakeErrorClient struct { + client.Client + + newError error + createError error +} + +func (cl *fakeErrorClient) Create(ctx context.Context, obj client.Object, opts ...client.CreateOption) error { + if cl.createError != nil { + return cl.createError + } + + return cl.Client.Create(ctx, obj, opts...) +} + +func newFakeCmapiChecker() (*fakeErrorClient, Interface, error) { + scheme := runtime.NewScheme() + if err := cmapi.AddToScheme(scheme); err != nil { + return nil, nil, err + } + cl := fake.NewClientBuilder().WithScheme(scheme).Build() + errorClient := &fakeErrorClient{ + Client: cl, + newError: nil, + createError: nil, + } + + return errorClient, &cmapiChecker{ + clientBuilder: func() (client.Client, error) { + if errorClient.newError != nil { + return nil, errorClient.newError + } + return errorClient, nil + }, + }, nil +} + +func TestCmapiChecker(t *testing.T) { + tests := map[string]testT{ + "check API without errors": { + newError: nil, + createError: nil, + + expectedError: "", + }, + "check API server unreachable": { + newError: errors.New("while creating client: Get \"http://localhost:8080/api?timeout=32s\": dial tcp 127.0.0.1:8080: connect: connection refused"), + createError: nil, + + expectedError: ErrAPIServerUnreachable.Error(), + }, + "check API without CRDs installed": { + newError: nil, + createError: errors.New("error finding the scope of the object: failed to get restmapping: no matches for kind \"Certificate\" in group \"cert-manager.io\""), + + expectedError: ErrCertManagerCRDsNotFound.Error(), + }, + "check API with webhook service not ready": { + newError: nil, + createError: errors.New("Internal error occurred: failed calling webhook \"webhook.cert-manager.io\": Post \"https://cert-manager-webhook.cert-manager.svc:443/mutate?timeout=10s\": service \"cert-manager-webhook\" not found"), + + expectedError: ErrCertManagerAPIEndpointsNotEstablished.Error(), + }, + "check API with webhook pod not accepting connections": { + newError: nil, + createError: errors.New("Internal error occurred: failed calling webhook \"webhook.cert-manager.io\": Post \"https://cert-manager-webhook.cert-manager.svc:443/mutate?timeout=10s\": dial tcp 10.96.38.90:443: connect: connection refused"), + + expectedError: ErrWebhookConnectionFailure.Error(), + }, + "check API with webhook certificate not updated in mutation webhook resource definitions": { + newError: nil, + createError: errors.New("Internal error occurred: failed calling webhook \"webhook.cert-manager.io\": Post \"https://cert-manager-webhook.cert-manager.svc:443/mutate?timeout=10s\": x509: certificate signed by unknown authority (possibly because of \"x509: ECDSA verification failure\" while trying to verify candidate authority certificate \"cert-manager-webhook-ca\""), + + expectedError: ErrWebhookCertificateFailure.Error(), + }, + "check API with webhook certificate not updated in conversion webhook resource definitions": { + newError: nil, + createError: errors.New("conversion webhook for cert-manager.io/v1alpha2, Kind=Certificate failed: Post \"https://cert-manager-webhook.cert-manager.svc:443/convert?timeout=30s\": x509: certificate signed by unknown authority"), + + expectedError: ErrWebhookCertificateFailure.Error(), + }, + } + + for n, test := range tests { + t.Run(n, func(t *testing.T) { + runTest(t, test) + }) + } +} + +type testT struct { + newError error + createError error + + expectedError string +} + +func runTest(t *testing.T, test testT) { + errorClient, checker, _ := newFakeCmapiChecker() + + errorClient.newError = test.newError + errorClient.createError = test.createError + + err := checker.Check(context.TODO()) + if err != nil { + if err.Error() != test.expectedError { + t.Errorf("error differs from expected error:\n%s\n vs \n%s", err.Error(), test.expectedError) + } + } else { + if test.expectedError != "" { + t.Errorf("expected error did not occure:\n%s", test.expectedError) + } + } +} From 21bc98979e2c21df4da34142248ca741b3ccf3da Mon Sep 17 00:00:00 2001 From: Inteon <42113979+inteon@users.noreply.github.com> Date: Fri, 16 Jul 2021 13:11:40 +0200 Subject: [PATCH 3/3] improved ux Signed-off-by: Inteon <42113979+inteon@users.noreply.github.com> --- cmd/ctl/pkg/check/api/api.go | 40 ++++--- devel/addon/certmanager/install.sh | 5 +- pkg/util/cmapichecker/cmapichecker.go | 133 ++++++++++----------- pkg/util/cmapichecker/cmapichecker_test.go | 127 +++++++++++++------- 4 files changed, 177 insertions(+), 128 deletions(-) diff --git a/cmd/ctl/pkg/check/api/api.go b/cmd/ctl/pkg/check/api/api.go index 67b503322..533138e23 100644 --- a/cmd/ctl/pkg/check/api/api.go +++ b/cmd/ctl/pkg/check/api/api.go @@ -18,7 +18,10 @@ package api import ( "context" + "errors" "fmt" + "log" + "os" "time" "github.com/spf13/cobra" @@ -98,50 +101,57 @@ func NewCmdCheckApi(ctx context.Context, ioStreams genericclioptions.IOStreams, cmd := &cobra.Command{ Use: "api", - Short: "This check attempts to perform a dry-run create of a cert-manager Certificate", + Short: "Check if the cert-manager API is ready", Long: checkApiDesc, RunE: func(cmd *cobra.Command, args []string) error { if err := o.Complete(factory); err != nil { return err } - return o.Run(ctx) + o.Run(ctx) + return nil }, SilenceUsage: true, SilenceErrors: true, } - cmd.Flags().DurationVar(&o.Wait, "wait", 1*time.Minute, "Time before timeout when waiting, must include unit, e.g. 0s or 20s") + cmd.Flags().DurationVar(&o.Wait, "wait", 0, "Wait until the cert-manager API is ready (default 0s)") cmd.Flags().DurationVar(&o.Interval, "interval", 5*time.Second, "Time between checks when waiting, must include unit, e.g. 1m or 10m") - cmd.Flags().BoolVarP(&o.Verbose, "verbose", "v", false, "Print details regarding encountered errors") + cmd.Flags().BoolVarP(&o.Verbose, "verbose", "v", false, "Print detailed error messages") return cmd } // Run executes check api command -func (o *Options) Run(ctx context.Context) error { +func (o *Options) Run(ctx context.Context) { + if !o.Verbose { + log.SetFlags(0) // Disable prefixing logs with timestamps. + } + log.SetOutput(o.ErrOut) // Log all intermediate errors to stderr + pollContext, cancel := context.WithTimeout(ctx, o.Wait) defer cancel() pollErr := wait.PollImmediateUntil(o.Interval, func() (done bool, err error) { if err := o.APIChecker.Check(ctx); err != nil { - if o.Verbose { - fmt.Fprintf(o.ErrOut, "Not ready: %v (%v)\n", err, err.Cause()) - } else { - fmt.Fprintf(o.ErrOut, "Not ready: %v\n", err) + if !o.Verbose && errors.Unwrap(err) != nil { + err = errors.Unwrap(err) } + + log.Printf("Not ready: %v", err) return false, nil } - fmt.Fprintln(o.Out, "The cert-manager API is ready") - return true, nil }, pollContext.Done()) + log.SetOutput(o.Out) // Log conclusion to stdout + if pollErr != nil { - if ctx.Err() != nil { - return ctx.Err() + if errors.Is(pollContext.Err(), context.DeadlineExceeded) && o.Wait > 0 { + log.Printf("Timed out after %s", o.Wait) } - return pollErr + + os.Exit(1) } - return nil + log.Printf("The cert-manager API is ready") } diff --git a/devel/addon/certmanager/install.sh b/devel/addon/certmanager/install.sh index 1a151a30f..aaa47f56b 100755 --- a/devel/addon/certmanager/install.sh +++ b/devel/addon/certmanager/install.sh @@ -53,6 +53,9 @@ kubectl get namespace "${NAMESPACE}" || kubectl create namespace "${NAMESPACE}" # Build the Helm chart package .tgz bazel build //deploy/charts/cert-manager +# Pre-compile the kubectl plugin, so it can quickly check the api status +bazel build //hack/bin:kubectl-cert_manager + # Upgrade or install cert-manager helm upgrade \ --install \ @@ -67,4 +70,4 @@ helm upgrade \ "$RELEASE_NAME" \ "$REPO_ROOT/bazel-bin/deploy/charts/cert-manager/cert-manager.tgz" -kubectl cert-manager check api +kubectl cert-manager check api --wait=1m -v diff --git a/pkg/util/cmapichecker/cmapichecker.go b/pkg/util/cmapichecker/cmapichecker.go index 4a3fde241..e9dbd6477 100644 --- a/pkg/util/cmapichecker/cmapichecker.go +++ b/pkg/util/cmapichecker/cmapichecker.go @@ -18,6 +18,7 @@ package cmapichecker import ( "context" + "fmt" "regexp" errors "github.com/pkg/errors" @@ -35,43 +36,30 @@ import ( ) var ( - ErrAPIServerUnreachable = errors.New("unable to connect to the Kubernetes API server") - ErrCertManagerCRDsNotFound = errors.New("the cert-manager CRDs are not yet installed on the Kubernetes API server") - ErrCertManagerAPIEndpointsNotEstablished = errors.New("the cert-manager API endpoints have not yet been published by the Kubernetes API server") - ErrWebhookConnectionFailure = errors.New("the cert-manager webhook server can't be reached yet") - ErrWebhookCertificateFailure = errors.New("the client CA bundle is not yet updated to the certificate of the cert-manager webhook") - - regexErrCertManagerCRDsNotFound = regexp.MustCompile(`^error finding the scope of the object: failed to get restmapping: no matches for kind "Certificate" in group "cert-manager.io"$`) - regexErrCertManagerAPIEndpointsNotEstablished = regexp.MustCompile(`failed calling webhook "(.*)\.cert-manager\.io": Post "(.*)\/mutate(.*)": service "(.*)-webhook" not found$`) - regexErrWebhookConnectionFailure = regexp.MustCompile(`failed calling webhook "(.*)\.cert-manager\.io": Post "(.*)\/mutate(.*)": (.*): connect: connection refused$`) - regexErrWebhookCertificateFailure = regexp.MustCompile(`Post "(.*)": x509: certificate signed by unknown authority`) + ErrCertManagerCRDsNotFound = errors.New("the cert-manager CRDs are not yet installed on the Kubernetes API server") + ErrWebhookServiceFailure = errors.New("the cert-manager webhook service is not created yet") + ErrWebhookDeploymentFailure = errors.New("the cert-manager webhook deployment is not ready yet") + ErrWebhookCertificateFailure = errors.New("the cert-manager webhook CA bundle is not injected yet") ) -type ApiCheckError struct { - SimpleError error - UnderlyingError error -} +const ( + crdsMappingError = `error finding the scope of the object: failed to get restmapping: no matches for kind "Certificate" in group "cert-manager.io"` + crdsNotFoundError = `the server could not find the requested resource (post certificates.cert-manager.io)` +) -func (e *ApiCheckError) Error() string { - return e.SimpleError.Error() -} - -func (e *ApiCheckError) Cause() error { - return e.UnderlyingError -} +var ( + regexErrCertManagerCRDsNotFound = regexp.MustCompile(`^(` + regexp.QuoteMeta(crdsMappingError) + `|` + regexp.QuoteMeta(crdsNotFoundError) + `)$`) + regexErrWebhookServiceFailure = regexp.MustCompile(`Post "(.*)": service "(.*)-webhook" not found`) + regexErrWebhookDeploymentFailure = regexp.MustCompile(`Post "(.*)": (.*): connect: connection refused`) + regexErrWebhookCertificateFailure = regexp.MustCompile(`Post "(.*)": x509: certificate signed by unknown authority`) +) // Interface is used to check that the cert-manager CRDs have been installed and are usable. type Interface interface { - Check(context.Context) *ApiCheckError + Check(context.Context) error } type cmapiChecker struct { - // The client controller-runtime client.New function fails if can't reach - // the API server, so we load it lazily, to avoid breaking integration tests - // which rely on being able to start the webhook server before the API - // server. - clientBuilder func() (client.Client, error) - client client.Client } @@ -80,31 +68,17 @@ func New(restcfg *rest.Config, scheme *runtime.Scheme, namespace string) (Interf if err := cmapi.AddToScheme(scheme); err != nil { return nil, errors.Wrap(err, "while configuring scheme") } - return &cmapiChecker{ - clientBuilder: func() (client.Client, error) { - cl, err := client.New(restcfg, client.Options{ - Scheme: scheme, - }) - if err != nil { - return nil, errors.Wrap(err, "while creating client") - } - return client.NewNamespacedClient(client.NewDryRunClient(cl), namespace), nil - }, - }, nil -} -func (o *cmapiChecker) Client() (client.Client, error) { - if o.client != nil { - return o.client, nil - } - - cl, err := o.clientBuilder() + cl, err := client.New(restcfg, client.Options{ + Scheme: scheme, + }) if err != nil { - return nil, err + return nil, errors.Wrap(err, "while creating client") } - o.client = cl - return o.client, nil + return &cmapiChecker{ + client: client.NewNamespacedClient(client.NewDryRunClient(cl), namespace), + }, nil } // Check attempts to perform a dry-run create of a cert-manager *v1alpha2* @@ -112,7 +86,7 @@ func (o *cmapiChecker) Client() (client.Client, error) { // required webhooks are reachable by the K8S API server. // We use v1alpha2 API to ensure that the API server has also connected to the // cert-manager conversion webhook. -func (o *cmapiChecker) Check(ctx context.Context) *ApiCheckError { +func (o *cmapiChecker) Check(ctx context.Context) error { cert := &cmapi.Certificate{ ObjectMeta: metav1.ObjectMeta{ GenerateName: "cmapichecker-", @@ -126,21 +100,7 @@ func (o *cmapiChecker) Check(ctx context.Context) *ApiCheckError { }, } - // while creating client: Get "http://localhost:8080/api?timeout=32s": dial tcp 127.0.0.1:8080: connect: connection refused - cl, err := o.Client() - if err != nil { - return &ApiCheckError{ - SimpleError: ErrAPIServerUnreachable, - UnderlyingError: err, - } - } - - // error finding the scope of the object: failed to get restmapping: no matches for kind "Certificate" in group "cert-manager.io" - // Internal error occurred: failed calling webhook "webhook.cert-manager.io": Post "https://cert-manager-webhook.cert-manager.svc:443/mutate?timeout=10s": service "cert-manager-webhook" not found - // Internal error occurred: failed calling webhook "webhook.cert-manager.io": Post "https://cert-manager-webhook.cert-manager.svc:443/mutate?timeout=10s": dial tcp 10.96.38.90:443: connect: connection refused - // Internal error occurred: failed calling webhook "webhook.cert-manager.io": Post "https://cert-manager-webhook.cert-manager.svc:443/mutate?timeout=10s": x509: certificate signed by unknown authority (possibly because of "x509: ECDSA verification failure" while trying to verify candidate authority certificate "cert-manager-webhook-ca") - // conversion webhook for cert-manager.io/v1alpha2, Kind=Certificate failed: Post "https://cert-manager-webhook.cert-manager.svc:443/convert?timeout=30s": x509: certificate signed by unknown authority - if err := cl.Create(ctx, cert); err != nil { + if err := o.client.Create(ctx, cert); err != nil { return &ApiCheckError{ SimpleError: translateToSimpleError(err), UnderlyingError: err, @@ -149,18 +109,51 @@ func (o *cmapiChecker) Check(ctx context.Context) *ApiCheckError { return nil } +type ApiCheckError struct { + SimpleError error + UnderlyingError error +} + +func (e *ApiCheckError) Error() string { + // If no simple error exists, print underlying error + if e.SimpleError == nil { + return e.UnderlyingError.Error() + } + return fmt.Sprintf("%v (%v)", e.SimpleError.Error(), e.UnderlyingError.Error()) +} + +// If no simple error exists, this function will return nil +// which indicates that the error is not unwrappable +func (e *ApiCheckError) Unwrap() error { + return e.SimpleError +} + +// This translateToSimpleError function detects errors based on the error message. +// It tries to map these error messages to a better understandable error message that +// explains what is wrong. If it cannot create a simple error, it will return nil. +// ErrCertManagerCRDsNotFound: +// - error finding the scope of the object: failed to get restmapping: no matches for kind "Certificate" in group "cert-manager.io" +// ErrWebhookServiceFailure: +// - Internal error occurred: failed calling webhook "webhook.cert-manager.io": Post "https://cert-manager-webhook.cert-manager.svc:443/mutate?timeout=10s": service "cert-manager-webhook" not found +// - conversion webhook for cert-manager.io/v1alpha2, Kind=Certificate failed: Post "https://cert-manager-webhook.cert-manager.svc:443/convert?timeout=30s": service "cert-manager-webhook" not found +// ErrWebhookDeploymentFailure: +// - Internal error occurred: failed calling webhook "webhook.cert-manager.io": Post "https://cert-manager-webhook.cert-manager.svc:443/mutate?timeout=10s": dial tcp 10.96.38.90:443: connect: connection refused +// - conversion webhook for cert-manager.io/v1alpha2, Kind=Certificate failed: Post "https://cert-manager-webhook.cert-manager.svc:443/convert?timeout=30s": dial tcp 10.96.38.90:443: connect: connection refused +// ErrWebhookCertificateFailure: +// - Internal error occurred: failed calling webhook "webhook.cert-manager.io": Post "https://cert-manager-webhook.cert-manager.svc:443/mutate?timeout=10s": x509: certificate signed by unknown authority (possibly because of "x509: ECDSA verification failure" while trying to verify candidate authority certificate "cert-manager-webhook-ca") +// - conversion webhook for cert-manager.io/v1alpha2, Kind=Certificate failed: Post "https://cert-manager-webhook.cert-manager.svc:443/convert?timeout=30s": x509: certificate signed by unknown authority func translateToSimpleError(err error) error { s := err.Error() if regexErrCertManagerCRDsNotFound.MatchString(s) { return ErrCertManagerCRDsNotFound - } else if regexErrCertManagerAPIEndpointsNotEstablished.MatchString(s) { - return ErrCertManagerAPIEndpointsNotEstablished - } else if regexErrWebhookConnectionFailure.MatchString(s) { - return ErrWebhookConnectionFailure + } else if regexErrWebhookServiceFailure.MatchString(s) { + return ErrWebhookServiceFailure + } else if regexErrWebhookDeploymentFailure.MatchString(s) { + return ErrWebhookDeploymentFailure } else if regexErrWebhookCertificateFailure.MatchString(s) { return ErrWebhookCertificateFailure } - return err + return nil } diff --git a/pkg/util/cmapichecker/cmapichecker_test.go b/pkg/util/cmapichecker/cmapichecker_test.go index 0d381fcdb..cd8f5638e 100644 --- a/pkg/util/cmapichecker/cmapichecker_test.go +++ b/pkg/util/cmapichecker/cmapichecker_test.go @@ -19,6 +19,7 @@ package cmapichecker import ( "context" "errors" + "fmt" "testing" "k8s.io/apimachinery/pkg/runtime" @@ -31,7 +32,6 @@ import ( type fakeErrorClient struct { client.Client - newError error createError error } @@ -51,63 +51,91 @@ func newFakeCmapiChecker() (*fakeErrorClient, Interface, error) { cl := fake.NewClientBuilder().WithScheme(scheme).Build() errorClient := &fakeErrorClient{ Client: cl, - newError: nil, createError: nil, } return errorClient, &cmapiChecker{ - clientBuilder: func() (client.Client, error) { - if errorClient.newError != nil { - return nil, errorClient.newError - } - return errorClient, nil - }, + client: errorClient, }, nil } +const ( + errCertManagerCRDsMapping = `error finding the scope of the object: failed to get restmapping: no matches for kind "Certificate" in group "cert-manager.io"` + errCertManagerCRDsNotFound = `the server could not find the requested resource (post certificates.cert-manager.io)` + + errMutatingWebhookServiceFailure = `Internal error occurred: failed calling webhook "webhook.cert-manager.io": Post "https://cert-manager-webhook.cert-manager.svc:443/mutate?timeout=10s": service "cert-manager-webhook" not found` + errMutatingWebhookDeploymentFailure = `Internal error occurred: failed calling webhook "webhook.cert-manager.io": Post "https://cert-manager-webhook.cert-manager.svc:443/mutate?timeout=10s": dial tcp 10.96.38.90:443: connect: connection refused` + errMutatingWebhookCertificateFailure = `Internal error occurred: failed calling webhook "webhook.cert-manager.io": Post "https://cert-manager-webhook.cert-manager.svc:443/mutate?timeout=10s": x509: certificate signed by unknown authority (possibly because of "x509: ECDSA verification failure" while trying to verify candidate authority certificate "cert-manager-webhook-ca"` + + errConversionWebhookServiceFailure = `conversion webhook for cert-manager.io/v1alpha2, Kind=Certificate failed: Post "https://cert-manager-webhook.cert-manager.svc:443/convert?timeout=30s": service "cert-manager-webhook" not found` + errConversionWebhookDeploymentFailure = `conversion webhook for cert-manager.io/v1alpha2, Kind=Certificate failed: Post "https://cert-manager-webhook.cert-manager.svc:443/convert?timeout=30s": dial tcp 10.96.38.90:443: connect: connection refused` + errConversionWebhookCertificateFailure = `conversion webhook for cert-manager.io/v1alpha2, Kind=Certificate failed: Post "https://cert-manager-webhook.cert-manager.svc:443/convert?timeout=30s": x509: certificate signed by unknown authority` +) + func TestCmapiChecker(t *testing.T) { tests := map[string]testT{ "check API without errors": { - newError: nil, createError: nil, - expectedError: "", + expectedSimpleError: "", + expectedVerboseError: "", }, - "check API server unreachable": { - newError: errors.New("while creating client: Get \"http://localhost:8080/api?timeout=32s\": dial tcp 127.0.0.1:8080: connect: connection refused"), - createError: nil, + "check API without CRDs installed 1": { + createError: errors.New(errCertManagerCRDsMapping), - expectedError: ErrAPIServerUnreachable.Error(), + expectedSimpleError: ErrCertManagerCRDsNotFound.Error(), + expectedVerboseError: fmt.Sprintf("%s (%s)", ErrCertManagerCRDsNotFound.Error(), errCertManagerCRDsMapping), }, - "check API without CRDs installed": { - newError: nil, - createError: errors.New("error finding the scope of the object: failed to get restmapping: no matches for kind \"Certificate\" in group \"cert-manager.io\""), + "check API without CRDs installed 2": { + createError: errors.New(errCertManagerCRDsNotFound), - expectedError: ErrCertManagerCRDsNotFound.Error(), + expectedSimpleError: ErrCertManagerCRDsNotFound.Error(), + expectedVerboseError: fmt.Sprintf("%s (%s)", ErrCertManagerCRDsNotFound.Error(), errCertManagerCRDsNotFound), }, - "check API with webhook service not ready": { - newError: nil, - createError: errors.New("Internal error occurred: failed calling webhook \"webhook.cert-manager.io\": Post \"https://cert-manager-webhook.cert-manager.svc:443/mutate?timeout=10s\": service \"cert-manager-webhook\" not found"), - expectedError: ErrCertManagerAPIEndpointsNotEstablished.Error(), - }, - "check API with webhook pod not accepting connections": { - newError: nil, - createError: errors.New("Internal error occurred: failed calling webhook \"webhook.cert-manager.io\": Post \"https://cert-manager-webhook.cert-manager.svc:443/mutate?timeout=10s\": dial tcp 10.96.38.90:443: connect: connection refused"), + "check API with mutating webhook service not ready": { + createError: errors.New(errMutatingWebhookServiceFailure), - expectedError: ErrWebhookConnectionFailure.Error(), + expectedSimpleError: ErrWebhookServiceFailure.Error(), + expectedVerboseError: fmt.Sprintf("%s (%s)", ErrWebhookServiceFailure.Error(), errMutatingWebhookServiceFailure), }, + "check API with conversion webhook service not ready": { + createError: errors.New(errConversionWebhookServiceFailure), + + expectedSimpleError: ErrWebhookServiceFailure.Error(), + expectedVerboseError: fmt.Sprintf("%s (%s)", ErrWebhookServiceFailure.Error(), errConversionWebhookServiceFailure), + }, + + "check API with mutating webhook pod not accepting connections": { + createError: errors.New(errMutatingWebhookDeploymentFailure), + + expectedSimpleError: ErrWebhookDeploymentFailure.Error(), + expectedVerboseError: fmt.Sprintf("%s (%s)", ErrWebhookDeploymentFailure.Error(), errMutatingWebhookDeploymentFailure), + }, + "check API with conversion webhook pod not accepting connections": { + createError: errors.New(errConversionWebhookDeploymentFailure), + + expectedSimpleError: ErrWebhookDeploymentFailure.Error(), + expectedVerboseError: fmt.Sprintf("%s (%s)", ErrWebhookDeploymentFailure.Error(), errConversionWebhookDeploymentFailure), + }, + "check API with webhook certificate not updated in mutation webhook resource definitions": { - newError: nil, - createError: errors.New("Internal error occurred: failed calling webhook \"webhook.cert-manager.io\": Post \"https://cert-manager-webhook.cert-manager.svc:443/mutate?timeout=10s\": x509: certificate signed by unknown authority (possibly because of \"x509: ECDSA verification failure\" while trying to verify candidate authority certificate \"cert-manager-webhook-ca\""), + createError: errors.New(errMutatingWebhookCertificateFailure), - expectedError: ErrWebhookCertificateFailure.Error(), + expectedSimpleError: ErrWebhookCertificateFailure.Error(), + expectedVerboseError: fmt.Sprintf("%s (%s)", ErrWebhookCertificateFailure.Error(), errMutatingWebhookCertificateFailure), }, "check API with webhook certificate not updated in conversion webhook resource definitions": { - newError: nil, - createError: errors.New("conversion webhook for cert-manager.io/v1alpha2, Kind=Certificate failed: Post \"https://cert-manager-webhook.cert-manager.svc:443/convert?timeout=30s\": x509: certificate signed by unknown authority"), + createError: errors.New(errConversionWebhookCertificateFailure), - expectedError: ErrWebhookCertificateFailure.Error(), + expectedSimpleError: ErrWebhookCertificateFailure.Error(), + expectedVerboseError: fmt.Sprintf("%s (%s)", ErrWebhookCertificateFailure.Error(), errConversionWebhookCertificateFailure), + }, + "unexpected error": { + createError: errors.New("unexpected error"), + + expectedSimpleError: "", + expectedVerboseError: "unexpected error", }, } @@ -119,26 +147,41 @@ func TestCmapiChecker(t *testing.T) { } type testT struct { - newError error createError error - expectedError string + expectedSimpleError string + expectedVerboseError string } func runTest(t *testing.T, test testT) { - errorClient, checker, _ := newFakeCmapiChecker() + errorClient, checker, err := newFakeCmapiChecker() + if err != nil { + t.Error(err) + } - errorClient.newError = test.newError errorClient.createError = test.createError - err := checker.Check(context.TODO()) + var unwrappedErr error + err = checker.Check(context.TODO()) if err != nil { - if err.Error() != test.expectedError { - t.Errorf("error differs from expected error:\n%s\n vs \n%s", err.Error(), test.expectedError) + if err.Error() != test.expectedVerboseError { + t.Errorf("error differs from expected error:\n%s\n vs \n%s", err.Error(), test.expectedVerboseError) + } + + unwrappedErr = errors.Unwrap(err) + } else { + if test.expectedVerboseError != "" { + t.Errorf("expected error did not occure:\n%s", test.expectedVerboseError) + } + } + + if unwrappedErr != nil { + if unwrappedErr.Error() != test.expectedSimpleError { + t.Errorf("simple error differs from expected error:\n%s\n vs \n%s", unwrappedErr.Error(), test.expectedSimpleError) } } else { - if test.expectedError != "" { - t.Errorf("expected error did not occure:\n%s", test.expectedError) + if test.expectedSimpleError != "" { + t.Errorf("expected simple error did not occure:\n%s", test.expectedSimpleError) } } }