From b889fb3566bc553866a3be9bb14a8acf0f458276 Mon Sep 17 00:00:00 2001 From: Jacob Beacham Date: Tue, 21 Mar 2017 19:44:33 -0700 Subject: [PATCH] Better messaging when GKE certificate signing fails. On errors, the GKE signing API can respond with a JSON body that contains an error message explaining the failure. If we're able to extract it, use that message when reporting the error instead of the generic error returned by the webhook library. Also, always add an event to the CSR object on signing errors. --- cmd/gke-certificates-controller/app/BUILD | 6 +++- .../app/gke_certificates_controller.go | 5 +++- .../app/gke_signer.go | 30 ++++++++++++++++++- .../app/gke_signer_test.go | 3 +- 4 files changed, 40 insertions(+), 4 deletions(-) diff --git a/cmd/gke-certificates-controller/app/BUILD b/cmd/gke-certificates-controller/app/BUILD index 72207eeac94..ab8b36ff338 100644 --- a/cmd/gke-certificates-controller/app/BUILD +++ b/cmd/gke-certificates-controller/app/BUILD @@ -31,6 +31,7 @@ go_library( "//vendor:k8s.io/apimachinery/pkg/runtime/schema", "//vendor:k8s.io/apiserver/pkg/util/webhook", "//vendor:k8s.io/client-go/kubernetes/typed/core/v1", + "//vendor:k8s.io/client-go/pkg/api/v1", "//vendor:k8s.io/client-go/plugin/pkg/client/auth", "//vendor:k8s.io/client-go/rest", "//vendor:k8s.io/client-go/tools/clientcmd", @@ -56,5 +57,8 @@ go_test( srcs = ["gke_signer_test.go"], library = ":go_default_library", tags = ["automanaged"], - deps = ["//pkg/apis/certificates/v1beta1:go_default_library"], + deps = [ + "//pkg/apis/certificates/v1beta1:go_default_library", + "//vendor:k8s.io/client-go/tools/record", + ], ) diff --git a/cmd/gke-certificates-controller/app/gke_certificates_controller.go b/cmd/gke-certificates-controller/app/gke_certificates_controller.go index 71de2888b00..e078d2d95c8 100644 --- a/cmd/gke-certificates-controller/app/gke_certificates_controller.go +++ b/cmd/gke-certificates-controller/app/gke_certificates_controller.go @@ -22,9 +22,11 @@ import ( "time" v1core "k8s.io/client-go/kubernetes/typed/core/v1" + clientv1 "k8s.io/client-go/pkg/api/v1" restclient "k8s.io/client-go/rest" "k8s.io/client-go/tools/clientcmd" "k8s.io/client-go/tools/record" + "k8s.io/kubernetes/pkg/api" "k8s.io/kubernetes/pkg/client/clientset_generated/clientset" informers "k8s.io/kubernetes/pkg/client/informers/informers_generated/externalversions" "k8s.io/kubernetes/pkg/controller" @@ -63,13 +65,14 @@ func Run(s *GKECertificatesController) error { eventBroadcaster := record.NewBroadcaster() eventBroadcaster.StartLogging(glog.Infof) eventBroadcaster.StartRecordingToSink(&v1core.EventSinkImpl{Interface: v1core.New(kubeClient.Core().RESTClient()).Events("")}) + recorder := eventBroadcaster.NewRecorder(api.Scheme, clientv1.EventSource{Component: "gke-certificates-controller"}) clientBuilder := controller.SimpleControllerClientBuilder{ClientConfig: kubeconfig} client := clientBuilder.ClientOrDie("certificate-controller") sharedInformers := informers.NewSharedInformerFactory(client, time.Duration(12)*time.Hour) - signer, err := NewGKESigner(s.ClusterSigningGKEKubeconfig, s.ClusterSigningGKERetryBackoff.Duration) + signer, err := NewGKESigner(s.ClusterSigningGKEKubeconfig, s.ClusterSigningGKERetryBackoff.Duration, recorder) if err != nil { return err } diff --git a/cmd/gke-certificates-controller/app/gke_signer.go b/cmd/gke-certificates-controller/app/gke_signer.go index 4496eb477f3..180c0029171 100644 --- a/cmd/gke-certificates-controller/app/gke_signer.go +++ b/cmd/gke-certificates-controller/app/gke_signer.go @@ -17,6 +17,7 @@ limitations under the License. package app import ( + "encoding/json" "fmt" "time" @@ -25,6 +26,7 @@ import ( "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apiserver/pkg/util/webhook" "k8s.io/client-go/rest" + "k8s.io/client-go/tools/record" "k8s.io/kubernetes/pkg/api" _ "k8s.io/kubernetes/pkg/apis/certificates/install" certificates "k8s.io/kubernetes/pkg/apis/certificates/v1beta1" @@ -40,10 +42,11 @@ type GKESigner struct { webhook *webhook.GenericWebhook kubeConfigFile string retryBackoff time.Duration + recorder record.EventRecorder } // NewGKESigner will create a new instance of a GKESigner. -func NewGKESigner(kubeConfigFile string, retryBackoff time.Duration) (*GKESigner, error) { +func NewGKESigner(kubeConfigFile string, retryBackoff time.Duration, recorder record.EventRecorder) (*GKESigner, error) { webhook, err := webhook.NewGenericWebhook(api.Registry, api.Codecs, kubeConfigFile, groupVersions, retryBackoff) if err != nil { return nil, err @@ -53,6 +56,7 @@ func NewGKESigner(kubeConfigFile string, retryBackoff time.Duration) (*GKESigner webhook: webhook, kubeConfigFile: kubeConfigFile, retryBackoff: retryBackoff, + recorder: recorder, }, nil } @@ -65,6 +69,9 @@ func (s *GKESigner) Sign(csr *certificates.CertificateSigningRequest) (*certific }) if err := result.Error(); err != nil { + if bodyErr := s.resultBodyError(result); bodyErr != nil { + return nil, s.webhookError(csr, bodyErr) + } return nil, s.webhookError(csr, err) } @@ -86,5 +93,26 @@ func (s *GKESigner) Sign(csr *certificates.CertificateSigningRequest) (*certific func (s *GKESigner) webhookError(csr *certificates.CertificateSigningRequest, err error) error { glog.V(2).Infof("error contacting webhook backend: %s", err) + s.recorder.Eventf(csr, "Warning", "SigningError", "error while calling GKE: %v", err) return err } + +// signResultError represents the structured response body of a failed call to +// GKE's SignCertificate API. +type signResultError struct { + Error struct { + Code int + Message string + Status string + } +} + +// resultBodyError attempts to extract an error out of a response body. +func (s *GKESigner) resultBodyError(result rest.Result) error { + body, _ := result.Raw() + var sre signResultError + if err := json.Unmarshal(body, &sre); err == nil { + return fmt.Errorf("server responded with error: %s", sre.Error.Message) + } + return nil +} diff --git a/cmd/gke-certificates-controller/app/gke_signer_test.go b/cmd/gke-certificates-controller/app/gke_signer_test.go index a56653cf9f2..8d04baad9d6 100644 --- a/cmd/gke-certificates-controller/app/gke_signer_test.go +++ b/cmd/gke-certificates-controller/app/gke_signer_test.go @@ -26,6 +26,7 @@ import ( "text/template" "time" + "k8s.io/client-go/tools/record" certificates "k8s.io/kubernetes/pkg/apis/certificates/v1beta1" ) @@ -103,7 +104,7 @@ func TestGKESigner(t *testing.T) { t.Fatalf("error closing kubeconfig template: %v", err) } - signer, err := NewGKESigner(kubeConfig.Name(), time.Duration(500)*time.Millisecond) + signer, err := NewGKESigner(kubeConfig.Name(), time.Duration(500)*time.Millisecond, record.NewFakeRecorder(10)) if err != nil { t.Fatalf("error creating GKESigner: %v", err) }