Merge pull request #43493 from pipejakob/gke-failure-messaging

Automatic merge from submit-queue

Better messaging when GKE certificate signing fails.

**What this PR does / why we need it**:
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.

**Release note**:

```release-note
NONE
```

CC @mikedanese @jcbsmpsn

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",
+    ],
 )

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
 	}

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
+}

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)
 		}