diff --git a/staging/src/k8s.io/apiserver/pkg/admission/attributes.go b/staging/src/k8s.io/apiserver/pkg/admission/attributes.go index beea941fc30..1d291f6b22e 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/attributes.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/attributes.go @@ -24,6 +24,7 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/util/validation" + auditinternal "k8s.io/apiserver/pkg/apis/audit" "k8s.io/apiserver/pkg/authentication/user" ) @@ -42,12 +43,17 @@ type attributesRecord struct { // other elements are always accessed in single goroutine. // But ValidatingAdmissionWebhook add annotations concurrently. - annotations map[string]string + annotations map[string]annotation annotationsLock sync.RWMutex reinvocationContext ReinvocationContext } +type annotation struct { + level auditinternal.Level + value string +} + func NewAttributesRecord(object runtime.Object, oldObject runtime.Object, kind schema.GroupVersionKind, namespace, name string, resource schema.GroupVersionResource, subresource string, operation Operation, operationOptions runtime.Object, dryRun bool, userInfo user.Info) Attributes { return &attributesRecord{ kind: kind, @@ -111,7 +117,7 @@ func (record *attributesRecord) GetUserInfo() user.Info { // getAnnotations implements privateAnnotationsGetter.It's a private method used // by WithAudit decorator. -func (record *attributesRecord) getAnnotations() map[string]string { +func (record *attributesRecord) getAnnotations(maxLevel auditinternal.Level) map[string]string { record.annotationsLock.RLock() defer record.annotationsLock.RUnlock() @@ -120,26 +126,36 @@ func (record *attributesRecord) getAnnotations() map[string]string { } cp := make(map[string]string, len(record.annotations)) for key, value := range record.annotations { - cp[key] = value + if value.level.Less(maxLevel) || value.level == maxLevel { + cp[key] = value.value + } } return cp } +// AddAnnotation adds an annotation to attributesRecord with Metadata audit level func (record *attributesRecord) AddAnnotation(key, value string) error { + return record.AddAnnotationWithLevel(key, value, auditinternal.LevelMetadata) +} + +func (record *attributesRecord) AddAnnotationWithLevel(key, value string, level auditinternal.Level) error { if err := checkKeyFormat(key); err != nil { return err } - + if level.Less(auditinternal.LevelMetadata) { + return fmt.Errorf("admission annotations are not allowed to be set at audit level lower than Metadata, key: %q, level: %s", key, level) + } record.annotationsLock.Lock() defer record.annotationsLock.Unlock() if record.annotations == nil { - record.annotations = make(map[string]string) + record.annotations = make(map[string]annotation) } - if v, ok := record.annotations[key]; ok && v != value { - return fmt.Errorf("admission annotations are not allowd to be overwritten, key:%q, old value: %q, new value:%q", key, record.annotations[key], value) + annotation := annotation{level: level, value: value} + if v, ok := record.annotations[key]; ok && v != annotation { + return fmt.Errorf("admission annotations are not allowd to be overwritten, key:%q, old value: %v, new value: %v", key, record.annotations[key], annotation) } - record.annotations[key] = value + record.annotations[key] = annotation return nil } diff --git a/staging/src/k8s.io/apiserver/pkg/admission/attributes_test.go b/staging/src/k8s.io/apiserver/pkg/admission/attributes_test.go index d54780d9998..d4f886e30f9 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/attributes_test.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/attributes_test.go @@ -20,6 +20,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + auditinternal "k8s.io/apiserver/pkg/apis/audit" ) func TestAddAnnotation(t *testing.T) { @@ -28,13 +29,13 @@ func TestAddAnnotation(t *testing.T) { // test AddAnnotation attr.AddAnnotation("podsecuritypolicy.admission.k8s.io/validate-policy", "privileged") attr.AddAnnotation("podsecuritypolicy.admission.k8s.io/admit-policy", "privileged") - annotations := attr.getAnnotations() + annotations := attr.getAnnotations(auditinternal.LevelMetadata) assert.Equal(t, annotations["podsecuritypolicy.admission.k8s.io/validate-policy"], "privileged") // test overwrite assert.Error(t, attr.AddAnnotation("podsecuritypolicy.admission.k8s.io/validate-policy", "privileged-overwrite"), "admission annotations should not be allowd to be overwritten") - annotations = attr.getAnnotations() + annotations = attr.getAnnotations(auditinternal.LevelMetadata) assert.Equal(t, annotations["podsecuritypolicy.admission.k8s.io/validate-policy"], "privileged", "admission annotations should not be overwritten") // test invalid plugin names @@ -47,7 +48,7 @@ func TestAddAnnotation(t *testing.T) { for name, invalidKey := range testCases { err := attr.AddAnnotation(invalidKey, "value-foo") assert.Error(t, err) - annotations = attr.getAnnotations() + annotations = attr.getAnnotations(auditinternal.LevelMetadata) assert.Equal(t, annotations[invalidKey], "", name+": invalid pluginName is not allowed ") } diff --git a/staging/src/k8s.io/apiserver/pkg/admission/audit.go b/staging/src/k8s.io/apiserver/pkg/admission/audit.go index d81622b9217..5d1884da4d8 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/audit.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/audit.go @@ -84,11 +84,18 @@ func ensureAnnotationGetter(a Attributes) error { } func (handler auditHandler) logAnnotations(a Attributes) { + if handler.ae == nil { + return + } switch a := a.(type) { case privateAnnotationsGetter: - audit.LogAnnotations(handler.ae, a.getAnnotations()) + for key, value := range a.getAnnotations(handler.ae.Level) { + audit.LogAnnotation(handler.ae, key, value) + } case AnnotationsGetter: - audit.LogAnnotations(handler.ae, a.GetAnnotations()) + for key, value := range a.GetAnnotations(handler.ae.Level) { + audit.LogAnnotation(handler.ae, key, value) + } default: // this will never happen, because we have already checked it in ensureAnnotationGetter } diff --git a/staging/src/k8s.io/apiserver/pkg/admission/interfaces.go b/staging/src/k8s.io/apiserver/pkg/admission/interfaces.go index 5f6d703b216..869d1d28b1d 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/interfaces.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/interfaces.go @@ -21,6 +21,7 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" + auditinternal "k8s.io/apiserver/pkg/apis/audit" "k8s.io/apiserver/pkg/authentication/user" ) @@ -61,8 +62,15 @@ type Attributes interface { // "podsecuritypolicy" is the name of the plugin, "admission.k8s.io" is the name of the organization, "admit-policy" is the key name. // An error is returned if the format of key is invalid. When trying to overwrite annotation with a new value, an error is returned. // Both ValidationInterface and MutationInterface are allowed to add Annotations. + // By default, an annotation gets logged into audit event if the request's audit level is greater or + // equal to Metadata. AddAnnotation(key, value string) error + // AddAnnotationWithLevel sets annotation according to key-value pair with additional intended audit level. + // An Annotation gets logged into audit event if the request's audit level is greater or equal to the + // intended audit level. + AddAnnotationWithLevel(key, value string, level auditinternal.Level) error + // GetReinvocationContext tracks the admission request information relevant to the re-invocation policy. GetReinvocationContext() ReinvocationContext } @@ -85,13 +93,13 @@ type ObjectInterfaces interface { // privateAnnotationsGetter is a private interface which allows users to get annotations from Attributes. type privateAnnotationsGetter interface { - getAnnotations() map[string]string + getAnnotations(maxLevel auditinternal.Level) map[string]string } // AnnotationsGetter allows users to get annotations from Attributes. An alternate Attribute should implement // this interface. type AnnotationsGetter interface { - GetAnnotations() map[string]string + GetAnnotations(maxLevel auditinternal.Level) map[string]string } // ReinvocationContext provides access to the admission related state required to implement the re-invocation policy. diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/plugin_test.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/plugin_test.go index db178eeca23..e6c8aa12d44 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/plugin_test.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/plugin_test.go @@ -29,6 +29,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apiserver/pkg/admission" webhooktesting "k8s.io/apiserver/pkg/admission/plugin/webhook/testing" + auditinternal "k8s.io/apiserver/pkg/apis/audit" ) // TestAdmit tests that MutatingWebhook#Admit works as expected @@ -108,9 +109,9 @@ func TestAdmit(t *testing.T) { return } if len(tt.ExpectAnnotations) == 0 { - assert.Empty(t, fakeAttr.GetAnnotations(), tt.Name+": annotations not set as expected.") + assert.Empty(t, fakeAttr.GetAnnotations(auditinternal.LevelMetadata), tt.Name+": annotations not set as expected.") } else { - assert.Equal(t, tt.ExpectAnnotations, fakeAttr.GetAnnotations(), tt.Name+": annotations not set as expected.") + assert.Equal(t, tt.ExpectAnnotations, fakeAttr.GetAnnotations(auditinternal.LevelMetadata), tt.Name+": annotations not set as expected.") } reinvocationCtx := fakeAttr.Attributes.GetReinvocationContext() reinvocationCtx.SetIsReinvoke() diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/validating/plugin_test.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/validating/plugin_test.go index a2aec191e39..c62fc7b2d0b 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/validating/plugin_test.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/validating/plugin_test.go @@ -25,6 +25,7 @@ import ( "k8s.io/apimachinery/pkg/api/errors" webhooktesting "k8s.io/apiserver/pkg/admission/plugin/webhook/testing" + auditinternal "k8s.io/apiserver/pkg/apis/audit" ) // TestValidate tests that ValidatingWebhook#Validate works as expected @@ -86,9 +87,9 @@ func TestValidate(t *testing.T) { continue } if len(tt.ExpectAnnotations) == 0 { - assert.Empty(t, fakeAttr.GetAnnotations(), tt.Name+": annotations not set as expected.") + assert.Empty(t, fakeAttr.GetAnnotations(auditinternal.LevelMetadata), tt.Name+": annotations not set as expected.") } else { - assert.Equal(t, tt.ExpectAnnotations, fakeAttr.GetAnnotations(), tt.Name+": annotations not set as expected.") + assert.Equal(t, tt.ExpectAnnotations, fakeAttr.GetAnnotations(auditinternal.LevelMetadata), tt.Name+": annotations not set as expected.") } } } diff --git a/staging/src/k8s.io/apiserver/pkg/audit/request.go b/staging/src/k8s.io/apiserver/pkg/audit/request.go index 9cbbb90dada..7099e9622cd 100644 --- a/staging/src/k8s.io/apiserver/pkg/audit/request.go +++ b/staging/src/k8s.io/apiserver/pkg/audit/request.go @@ -230,16 +230,6 @@ func LogAnnotation(ae *auditinternal.Event, key, value string) { ae.Annotations[key] = value } -// LogAnnotations fills in the Annotations according to the annotations map. -func LogAnnotations(ae *auditinternal.Event, annotations map[string]string) { - if ae == nil || ae.Level.Less(auditinternal.LevelMetadata) { - return - } - for key, value := range annotations { - LogAnnotation(ae, key, value) - } -} - // truncate User-Agent if too long, otherwise return it directly. func maybeTruncateUserAgent(req *http.Request) string { ua := req.UserAgent()