Allow ImageReview backend to add audit annotations.

pkg/apis/imagepolicy/types.go

@@ -56,7 +56,7 @@ type ImageReviewContainerSpec struct {
 	// In future, we may add command line overrides, exec health check command lines, and so on.
 }
 
-// ImageReviewStatus is the result of the token authentication request.
+// ImageReviewStatus is the result of the review for the pod creation request.
 type ImageReviewStatus struct {
 	// Allowed indicates that all images were allowed to be run.
 	Allowed bool
@@ -64,4 +64,9 @@ type ImageReviewStatus struct {
 	// may contain a short description of what is wrong.  Kubernetes
 	// may truncate excessively long errors when displaying to the user.
 	Reason string
+	// AuditAnnotations will be added to the attributes object of the
+	// admission controller request using 'AddAnnotation'.  The keys should
+	// be prefix-less (i.e., the admission controller will add an
+	// appropriate prefix).
+	AuditAnnotations map[string]string
 }

pkg/apis/imagepolicy/v1alpha1/zz_generated.conversion.go

@@ -158,6 +158,7 @@ func Convert_imagepolicy_ImageReviewSpec_To_v1alpha1_ImageReviewSpec(in *imagepo
 func autoConvert_v1alpha1_ImageReviewStatus_To_imagepolicy_ImageReviewStatus(in *v1alpha1.ImageReviewStatus, out *imagepolicy.ImageReviewStatus, s conversion.Scope) error {
 	out.Allowed = in.Allowed
 	out.Reason = in.Reason
+	out.AuditAnnotations = *(*map[string]string)(unsafe.Pointer(&in.AuditAnnotations))
 	return nil
 }
 
@@ -169,6 +170,7 @@ func Convert_v1alpha1_ImageReviewStatus_To_imagepolicy_ImageReviewStatus(in *v1a
 func autoConvert_imagepolicy_ImageReviewStatus_To_v1alpha1_ImageReviewStatus(in *imagepolicy.ImageReviewStatus, out *v1alpha1.ImageReviewStatus, s conversion.Scope) error {
 	out.Allowed = in.Allowed
 	out.Reason = in.Reason
+	out.AuditAnnotations = *(*map[string]string)(unsafe.Pointer(&in.AuditAnnotations))
 	return nil
 }
 

pkg/apis/imagepolicy/zz_generated.deepcopy.go

@@ -30,7 +30,7 @@ func (in *ImageReview) DeepCopyInto(out *ImageReview) {
 	out.TypeMeta = in.TypeMeta
 	in.ObjectMeta.DeepCopyInto(&out.ObjectMeta)
 	in.Spec.DeepCopyInto(&out.Spec)
-	out.Status = in.Status
+	in.Status.DeepCopyInto(&out.Status)
 	return
 }
 
@@ -99,6 +99,13 @@ func (in *ImageReviewSpec) DeepCopy() *ImageReviewSpec {
 // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
 func (in *ImageReviewStatus) DeepCopyInto(out *ImageReviewStatus) {
 	*out = *in
+	if in.AuditAnnotations != nil {
+		in, out := &in.AuditAnnotations, &out.AuditAnnotations
+		*out = make(map[string]string, len(*in))
+		for key, val := range *in {
+			(*out)[key] = val
+		}
+	}
 	return
 }
 

plugin/pkg/admission/imagepolicy/admission.go

@@ -46,6 +46,21 @@ import (
 // PluginName indicates name of admission plugin.
 const PluginName = "ImagePolicyWebhook"
 
+// AuditKeyPrefix is used as the prefix for all audit keys handled by this
+// pluggin. Some well known suffixes are listed below.
+var AuditKeyPrefix = strings.ToLower(PluginName) + ".image-policy.k8s.io/"
+
+const (
+	// ImagePolicyFailedOpenKeySuffix in an annotation indicates the image
+	// review failed open when the image policy webhook backend connection
+	// failed.
+	ImagePolicyFailedOpenKeySuffix string = "failed-open"
+
+	// ImagePolicyAuditRequiredKeySuffix in an annotation indicates the pod
+	// should be audited.
+	ImagePolicyAuditRequiredKeySuffix string = "audit-required"
+)
+
 var (
 	groupVersions = []schema.GroupVersion{v1alpha1.SchemeGroupVersion}
 )
@@ -97,12 +112,15 @@ func (a *Plugin) webhookError(pod *api.Pod, attributes admission.Attributes, err
 	if err != nil {
 		glog.V(2).Infof("error contacting webhook backend: %s", err)
 		if a.defaultAllow {
+			attributes.AddAnnotation(AuditKeyPrefix+ImagePolicyFailedOpenKeySuffix, "true")
+			// TODO(wteiken): Remove the annotation code for the 1.13 release
 			annotations := pod.GetAnnotations()
 			if annotations == nil {
 				annotations = make(map[string]string)
 			}
 			annotations[api.ImagePolicyFailedOpenKey] = "true"
 			pod.ObjectMeta.SetAnnotations(annotations)
+
 			glog.V(2).Infof("resource allowed in spite of webhook backend failure")
 			return nil
 		}
@@ -174,13 +192,17 @@ func (a *Plugin) admitPod(pod *api.Pod, attributes admission.Attributes, review
 		a.responseCache.Add(string(cacheKey), review.Status, a.statusTTL(review.Status))
 	}
 
+	for k, v := range review.Status.AuditAnnotations {
+		if err := attributes.AddAnnotation(AuditKeyPrefix+k, v); err != nil {
+			glog.Warningf("failed to set admission audit annotation %s to %s: %v", AuditKeyPrefix+k, v, err)
+		}
+	}
 	if !review.Status.Allowed {
 		if len(review.Status.Reason) > 0 {
 			return fmt.Errorf("image policy webhook backend denied one or more images: %s", review.Status.Reason)
 		}
 		return errors.New("one or more images rejected by webhook backend")
 	}
-
 	return nil
 }
 

plugin/pkg/admission/imagepolicy/admission_test.go

@@ -192,6 +192,7 @@ current-context: default
 
 	for _, tt := range tests {
 		// Use a closure so defer statements trigger between loop iterations.
+		t.Run(tt.msg, func(t *testing.T) {
 			err := func() error {
 				tempfile, err := ioutil.TempFile("", "")
 				if err != nil {
@@ -252,6 +253,7 @@ current-context: default
 			if err == nil && tt.wantErr {
 				t.Errorf("wanted an error when loading config, did not get one: %q", tt.msg)
 			}
+		})
 	}
 }
 
@@ -296,6 +298,7 @@ func NewTestServer(s Service, cert, key, caCert []byte) (*httptest.Server, error
 		type status struct {
 			Allowed          bool              `json:"allowed"`
 			Reason           string            `json:"reason"`
+			AuditAnnotations map[string]string `json:"auditAnnotations"`
 		}
 		resp := struct {
 			APIVersion string `json:"apiVersion"`
@@ -304,7 +307,11 @@ func NewTestServer(s Service, cert, key, caCert []byte) (*httptest.Server, error
 		}{
 			APIVersion: v1alpha1.SchemeGroupVersion.String(),
 			Kind:       "ImageReview",
-			Status:     status{review.Status.Allowed, review.Status.Reason},
+			Status: status{
+				review.Status.Allowed,
+				review.Status.Reason,
+				review.Status.AuditAnnotations,
+			},
 		}
 		w.Header().Set("Content-Type", "application/json")
 		json.NewEncoder(w).Encode(resp)
@@ -320,6 +327,7 @@ func NewTestServer(s Service, cert, key, caCert []byte) (*httptest.Server, error
 type mockService struct {
 	allow          bool
 	statusCode     int
+	outAnnotations map[string]string
 }
 
 func (m *mockService) Review(r *v1alpha1.ImageReview) {
@@ -339,6 +347,8 @@ func (m *mockService) Review(r *v1alpha1.ImageReview) {
 	if !r.Status.Allowed {
 		r.Status.Reason = "not allowed"
 	}
+
+	r.Status.AuditAnnotations = m.outAnnotations
 }
 func (m *mockService) Allow()              { m.allow = true }
 func (m *mockService) Deny()               { m.allow = false }
@@ -455,7 +465,7 @@ func TestTLSConfig(t *testing.T) {
 	}
 	for _, tt := range tests {
 		// Use a closure so defer statements trigger between loop iterations.
-		func() {
+		t.Run(tt.test, func(t *testing.T) {
 			service := new(mockService)
 			service.statusCode = 200
 
@@ -502,7 +512,7 @@ func TestTLSConfig(t *testing.T) {
 			if err := wh.Validate(attr); err == nil {
 				t.Errorf("%s: incorrectly admitted with DenyAll policy", tt.test)
 			}
-		}()
+		})
 	}
 }
 
@@ -730,7 +740,7 @@ func TestContainerCombinations(t *testing.T) {
 	}
 	for _, tt := range tests {
 		// Use a closure so defer statements trigger between loop iterations.
-		func() {
+		t.Run(tt.test, func(t *testing.T) {
 			service := new(mockService)
 			service.statusCode = 200
 
@@ -769,27 +779,41 @@ func TestContainerCombinations(t *testing.T) {
 				t.Errorf("%s: failed to admit: %v", tt.test, err)
 				return
 			}
-		}()
+		})
 	}
 }
 
+// fakeAttributes decorate kadmission.Attributes. It's used to trace the added annotations.
+type fakeAttributes struct {
+	admission.Attributes
+	annotations map[string]string
+}
+
+func (f fakeAttributes) AddAnnotation(k, v string) error {
+	f.annotations[k] = v
+	return f.Attributes.AddAnnotation(k, v)
+}
+
 func TestDefaultAllow(t *testing.T) {
 	tests := []struct {
 		test                               string
 		pod                                *api.Pod
-		wantAllowed, wantErr, defaultAllow bool
+		defaultAllow                       bool
+		wantAllowed, wantErr, wantFailOpen bool
 	}{
 		{
 			test:         "DefaultAllow = true, backend unreachable, bad image",
 			pod:          goodPod("bad"),
 			defaultAllow: true,
 			wantAllowed:  true,
+			wantFailOpen: true,
 		},
 		{
 			test:         "DefaultAllow = true, backend unreachable, good image",
 			pod:          goodPod("good"),
 			defaultAllow: true,
 			wantAllowed:  true,
+			wantFailOpen: true,
 		},
 		{
 			test:         "DefaultAllow = false, backend unreachable, good image",
@@ -797,6 +821,7 @@ func TestDefaultAllow(t *testing.T) {
 			defaultAllow: false,
 			wantAllowed:  false,
 			wantErr:      true,
+			wantFailOpen: false,
 		},
 		{
 			test:         "DefaultAllow = false, backend unreachable, bad image",
@@ -804,11 +829,12 @@ func TestDefaultAllow(t *testing.T) {
 			defaultAllow: false,
 			wantAllowed:  false,
 			wantErr:      true,
+			wantFailOpen: false,
 		},
 	}
 	for _, tt := range tests {
 		// Use a closure so defer statements trigger between loop iterations.
-		func() {
+		t.Run(tt.test, func(t *testing.T) {
 			service := new(mockService)
 			service.statusCode = 500
 
@@ -826,6 +852,8 @@ func TestDefaultAllow(t *testing.T) {
 			}
 
 			attr := admission.NewAttributesRecord(tt.pod, nil, api.Kind("Pod").WithVersion("version"), "namespace", "", api.Resource("pods").WithVersion("version"), "", admission.Create, false, &user.DefaultInfo{})
+			annotations := make(map[string]string)
+			attr = &fakeAttributes{attr, annotations}
 
 			err = wh.Validate(attr)
 			if tt.wantAllowed {
@@ -847,7 +875,23 @@ func TestDefaultAllow(t *testing.T) {
 				t.Errorf("%s: failed to admit: %v", tt.test, err)
 				return
 			}
-		}()
+			podAnnotations := tt.pod.GetAnnotations()
+			if tt.wantFailOpen {
+				if podAnnotations == nil || podAnnotations[api.ImagePolicyFailedOpenKey] != "true" {
+					t.Errorf("missing expected fail open pod annotation")
+				}
+				if annotations[AuditKeyPrefix+ImagePolicyFailedOpenKeySuffix] != "true" {
+					t.Errorf("missing expected fail open attributes annotation")
+				}
+			} else {
+				if podAnnotations != nil && podAnnotations[api.ImagePolicyFailedOpenKey] == "true" {
+					t.Errorf("found unexpected fail open pod annotation")
+				}
+				if annotations[AuditKeyPrefix+ImagePolicyFailedOpenKeySuffix] == "true" {
+					t.Errorf("found unexpected fail open attributes annotation")
+				}
+			}
+		})
 	}
 }
 
@@ -898,7 +942,7 @@ func TestAnnotationFiltering(t *testing.T) {
 	}
 	for _, tt := range tests {
 		// Use a closure so defer statements trigger between loop iterations.
-		func() {
+		t.Run(tt.test, func(t *testing.T) {
 			service := new(annotationService)
 
 			server, err := NewTestServer(service, serverCert, serverKey, caCert)
@@ -928,7 +972,94 @@ func TestAnnotationFiltering(t *testing.T) {
 				t.Errorf("expected annotations sent to webhook: %v to match expected: %v", service.Annotations(), tt.outAnnotations)
 			}
 
-		}()
+		})
+	}
+}
+
+func TestReturnedAnnotationAdd(t *testing.T) {
+	tests := []struct {
+		test                string
+		pod                 *api.Pod
+		verifierAnnotations map[string]string
+		expectedAnnotations map[string]string
+	}{
+		{
+			test: "Add valid response annotations",
+			pod:  goodPod("good"),
+			verifierAnnotations: map[string]string{
+				"foo-test": "true",
+				"bar-test": "false",
+			},
+			expectedAnnotations: map[string]string{
+				"imagepolicywebhook.image-policy.k8s.io/foo-test": "true",
+				"imagepolicywebhook.image-policy.k8s.io/bar-test": "false",
+			},
+		},
+		{
+			test:                "No returned annotations are ignored",
+			pod:                 goodPod("good"),
+			verifierAnnotations: map[string]string{},
+			expectedAnnotations: map[string]string{},
+		},
+		{
+			test:                "Handles nil annotations",
+			pod:                 goodPod("good"),
+			verifierAnnotations: nil,
+			expectedAnnotations: map[string]string{},
+		},
+		{
+			test: "Adds annotations for bad request",
+			pod: &api.Pod{
+				Spec: api.PodSpec{
+					ServiceAccountName: "default",
+					SecurityContext:    &api.PodSecurityContext{},
+					Containers: []api.Container{
+						{
+							Image:           "bad",
+							SecurityContext: &api.SecurityContext{},
+						},
+					},
+				},
+			},
+			verifierAnnotations: map[string]string{
+				"foo-test": "false",
+			},
+			expectedAnnotations: map[string]string{
+				"imagepolicywebhook.image-policy.k8s.io/foo-test": "false",
+			},
+		},
+	}
+	for _, tt := range tests {
+		// Use a closure so defer statements trigger between loop iterations.
+		t.Run(tt.test, func(t *testing.T) {
+			service := new(mockService)
+			service.statusCode = 200
+			service.outAnnotations = tt.verifierAnnotations
+
+			server, err := NewTestServer(service, serverCert, serverKey, caCert)
+			if err != nil {
+				t.Errorf("%s: failed to create server: %v", tt.test, err)
+				return
+			}
+			defer server.Close()
+
+			wh, err := newImagePolicyWebhook(server.URL, clientCert, clientKey, caCert, 0, true)
+			if err != nil {
+				t.Errorf("%s: failed to create client: %v", tt.test, err)
+				return
+			}
+
+			pod := tt.pod
+
+			attr := admission.NewAttributesRecord(pod, nil, api.Kind("Pod").WithVersion("version"), "namespace", "", api.Resource("pods").WithVersion("version"), "", admission.Create, false, &user.DefaultInfo{})
+			annotations := make(map[string]string)
+			attr = &fakeAttributes{attr, annotations}
+
+			err = wh.Validate(attr)
+			if !reflect.DeepEqual(annotations, tt.expectedAnnotations) {
+				t.Errorf("got audit annotations: %v; want: %v", annotations, tt.expectedAnnotations)
+			}
+		})
 	}
 }
 

staging/src/k8s.io/api/imagepolicy/v1alpha1/generated.proto

@@ -65,7 +65,7 @@ message ImageReviewSpec {
   optional string namespace = 3;
 }
 
-// ImageReviewStatus is the result of the token authentication request.
+// ImageReviewStatus is the result of the review for the pod creation request.
 message ImageReviewStatus {
   // Allowed indicates that all images were allowed to be run.
   optional bool allowed = 1;
@@ -75,5 +75,12 @@ message ImageReviewStatus {
   // may truncate excessively long errors when displaying to the user.
   // +optional
   optional string reason = 2;
+
+  // AuditAnnotations will be added to the attributes object of the
+  // admission controller request using 'AddAnnotation'.  The keys should
+  // be prefix-less (i.e., the admission controller will add an
+  // appropriate prefix).
+  // +optional
+  map<string, string> auditAnnotations = 3;
 }
 

staging/src/k8s.io/api/imagepolicy/v1alpha1/types.go

@@ -62,7 +62,7 @@ type ImageReviewContainerSpec struct {
 	// In future, we may add command line overrides, exec health check command lines, and so on.
 }
 
-// ImageReviewStatus is the result of the token authentication request.
+// ImageReviewStatus is the result of the review for the pod creation request.
 type ImageReviewStatus struct {
 	// Allowed indicates that all images were allowed to be run.
 	Allowed bool `json:"allowed" protobuf:"varint,1,opt,name=allowed"`
@@ -71,4 +71,10 @@ type ImageReviewStatus struct {
 	// may truncate excessively long errors when displaying to the user.
 	// +optional
 	Reason string `json:"reason,omitempty" protobuf:"bytes,2,opt,name=reason"`
+	// AuditAnnotations will be added to the attributes object of the
+	// admission controller request using 'AddAnnotation'.  The keys should
+	// be prefix-less (i.e., the admission controller will add an
+	// appropriate prefix).
+	// +optional
+	AuditAnnotations map[string]string `json:"auditAnnotations,omitempty" protobuf:"bytes,3,rep,name=auditAnnotations"`
 }

staging/src/k8s.io/api/imagepolicy/v1alpha1/types_swagger_doc_generated.go

@@ -58,9 +58,10 @@ func (ImageReviewSpec) SwaggerDoc() map[string]string {
 }
 
 var map_ImageReviewStatus = map[string]string{
-	"":        "ImageReviewStatus is the result of the token authentication request.",
+	"":                 "ImageReviewStatus is the result of the review for the pod creation request.",
 	"allowed":          "Allowed indicates that all images were allowed to be run.",
 	"reason":           "Reason should be empty unless Allowed is false in which case it may contain a short description of what is wrong.  Kubernetes may truncate excessively long errors when displaying to the user.",
+	"auditAnnotations": "AuditAnnotations will be added to the attributes object of the admission controller request using 'AddAnnotation'.  The keys should be prefix-less (i.e., the admission controller will add an appropriate prefix).",
 }
 
 func (ImageReviewStatus) SwaggerDoc() map[string]string {

staging/src/k8s.io/api/imagepolicy/v1alpha1/zz_generated.deepcopy.go

@@ -30,7 +30,7 @@ func (in *ImageReview) DeepCopyInto(out *ImageReview) {
 	out.TypeMeta = in.TypeMeta
 	in.ObjectMeta.DeepCopyInto(&out.ObjectMeta)
 	in.Spec.DeepCopyInto(&out.Spec)
-	out.Status = in.Status
+	in.Status.DeepCopyInto(&out.Status)
 	return
 }
 
@@ -99,6 +99,13 @@ func (in *ImageReviewSpec) DeepCopy() *ImageReviewSpec {
 // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
 func (in *ImageReviewStatus) DeepCopyInto(out *ImageReviewStatus) {
 	*out = *in
+	if in.AuditAnnotations != nil {
+		in, out := &in.AuditAnnotations, &out.AuditAnnotations
+		*out = make(map[string]string, len(*in))
+		for key, val := range *in {
+			(*out)[key] = val
+		}
+	}
 	return
 }