apiserver: refactor PolicyRuleEvaluator to return a struct

cluster/gce/gci/audit_policy_test.go

@@ -231,10 +231,10 @@ func (t *auditTester) expectLevel(expected audit.Level, attrs authorizer.Attribu
 	name := fmt.Sprintf("%s.%s.%s", attrs.GetUser().GetName(), attrs.GetVerb(), obj)
 	evaluator := t.evaluator
 	t.Run(name, func(t *testing.T) {
-		level, stages := evaluator.LevelAndStages(attrs)
-		assert.Equal(t, expected, level)
-		if level != audit.LevelNone {
-			assert.ElementsMatch(t, stages, []audit.Stage{audit.StageRequestReceived})
+		auditConfig := evaluator.EvaluatePolicyRule(attrs)
+		assert.Equal(t, expected, auditConfig.Level)
+		if auditConfig.Level != audit.LevelNone {
+			assert.ElementsMatch(t, auditConfig.OmitStages, []audit.Stage{audit.StageRequestReceived})
 		}
 	})
 }

staging/src/k8s.io/apiserver/pkg/audit/evaluator.go

@@ -36,10 +36,26 @@ type AuditContext struct {
 // RequestAuditConfig is the evaluated audit configuration that is applicable to
 // a given request. PolicyRuleEvaluator evaluates the audit policy against the
 // authorizer attributes and returns a RequestAuditConfig that applies to the request.
-type RequestAuditConfig struct{}
+type RequestAuditConfig struct {
+	// OmitStages is the stages that need to be omitted from being audited.
+	OmitStages []audit.Stage
+}
+
+// RequestAuditConfigWithLevel includes Level at which the request is being audited.
+// PolicyRuleEvaluator evaluates the audit configuration for a request
+// against the authorizer attributes and returns an RequestAuditConfigWithLevel
+// that applies to the request.
+type RequestAuditConfigWithLevel struct {
+	RequestAuditConfig
+
+	// Level at which the request is being audited at
+	Level audit.Level
+}
 
 // PolicyRuleEvaluator exposes methods for evaluating the policy rules.
 type PolicyRuleEvaluator interface {
-	// Check the audit level for a request with the given authorizer attributes.
-	LevelAndStages(authorizer.Attributes) (audit.Level, []audit.Stage)
+	// EvaluatePolicyRule evaluates the audit policy of the apiserver against
+	// the given authorizer attributes and returns the audit configuration that
+	// is applicable to the given equest.
+	EvaluatePolicyRule(authorizer.Attributes) RequestAuditConfigWithLevel
 }

staging/src/k8s.io/apiserver/pkg/audit/policy/checker.go

@@ -61,13 +61,24 @@ type policyRuleEvaluator struct {
 	audit.Policy
 }
 
-func (p *policyRuleEvaluator) LevelAndStages(attrs authorizer.Attributes) (audit.Level, []audit.Stage) {
+func (p *policyRuleEvaluator) EvaluatePolicyRule(attrs authorizer.Attributes) auditinternal.RequestAuditConfigWithLevel {
 	for _, rule := range p.Rules {
 		if ruleMatches(&rule, attrs) {
-			return rule.Level, rule.OmitStages
+			return auditinternal.RequestAuditConfigWithLevel{
+				Level: rule.Level,
+				RequestAuditConfig: auditinternal.RequestAuditConfig{
+					OmitStages: rule.OmitStages,
+				},
+			}
 		}
 	}
-	return DefaultAuditLevel, p.OmitStages
+
+	return auditinternal.RequestAuditConfigWithLevel{
+		Level: DefaultAuditLevel,
+		RequestAuditConfig: auditinternal.RequestAuditConfig{
+			OmitStages: p.OmitStages,
+		},
+	}
 }
 
 // Check whether the rule matches the request attrs.
@@ -210,6 +221,11 @@ type fakePolicyRuleEvaluator struct {
 	stage []audit.Stage
 }
 
-func (f *fakePolicyRuleEvaluator) LevelAndStages(_ authorizer.Attributes) (audit.Level, []audit.Stage) {
-	return f.level, f.stage
+func (f *fakePolicyRuleEvaluator) EvaluatePolicyRule(_ authorizer.Attributes) auditinternal.RequestAuditConfigWithLevel {
+	return auditinternal.RequestAuditConfigWithLevel{
+		Level: f.level,
+		RequestAuditConfig: auditinternal.RequestAuditConfig{
+			OmitStages: f.stage,
+		},
+	}
 }

staging/src/k8s.io/apiserver/pkg/audit/policy/checker_test.go

@@ -185,10 +185,10 @@ func test(t *testing.T, req string, expLevel audit.Level, policyStages, expOmitS
 		policy.Rules = append(policy.Rules, rules[rule])
 	}
 	require.Contains(t, attrs, req)
-	actualLevel, actualOmitStages := NewPolicyRuleEvaluator(&policy).LevelAndStages(attrs[req])
-	assert.Equal(t, expLevel, actualLevel, "request:%s rules:%s", req, strings.Join(ruleNames, ","))
-	assert.True(t, stageEqual(expOmitStages, actualOmitStages), "request:%s rules:%s, expected stages: %v, actual stages: %v",
-		req, strings.Join(ruleNames, ","), expOmitStages, actualOmitStages)
+	auditConfig := NewPolicyRuleEvaluator(&policy).EvaluatePolicyRule(attrs[req])
+	assert.Equal(t, expLevel, auditConfig.Level, "request:%s rules:%s", req, strings.Join(ruleNames, ","))
+	assert.True(t, stageEqual(expOmitStages, auditConfig.OmitStages), "request:%s rules:%s, expected stages: %v, actual stages: %v",
+		req, strings.Join(ruleNames, ","), expOmitStages, auditConfig.OmitStages)
 }
 
 func testAuditLevel(t *testing.T, stages []audit.Stage) {

staging/src/k8s.io/apiserver/pkg/endpoints/filters/audit.go

@@ -43,18 +43,24 @@ func WithAudit(handler http.Handler, sink audit.Sink, policy audit.PolicyRuleEva
 		return handler
 	}
 	return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
-		req, ev, omitStages, err := createAuditEventAndAttachToContext(req, policy)
+		auditContext, err := evaluatePolicyAndCreateAuditEvent(req, policy)
 		if err != nil {
 			utilruntime.HandleError(fmt.Errorf("failed to create audit event: %v", err))
 			responsewriters.InternalError(w, req, errors.New("failed to create audit event"))
 			return
 		}
-		ctx := req.Context()
-		if ev == nil || ctx == nil {
+
+		ev := auditContext.Event
+		if ev == nil || req.Context() == nil {
 			handler.ServeHTTP(w, req)
 			return
 		}
 
+		req = req.WithContext(audit.WithAuditContext(req.Context(), auditContext))
+
+		ctx := req.Context()
+		omitStages := auditContext.RequestAuditConfig.OmitStages
+
 		ev.Stage = auditinternal.StageRequestReceived
 		if processed := processAuditEvent(ctx, sink, ev, omitStages); !processed {
 			audit.ApiserverAuditDroppedCounter.WithContext(ctx).Inc()
@@ -111,41 +117,40 @@ func WithAudit(handler http.Handler, sink audit.Sink, policy audit.PolicyRuleEva
 	})
 }
 
-// createAuditEventAndAttachToContext is responsible for creating the audit event
-// and attaching it to the appropriate request context. It returns:
-// - context with audit event attached to it
-// - created audit event
+// evaluatePolicyAndCreateAuditEvent is responsible for evaluating the audit
+// policy configuration applicable to the request and create a new audit
+// event that will be written to the API audit log.
 // - error if anything bad happened
-func createAuditEventAndAttachToContext(req *http.Request, policy audit.PolicyRuleEvaluator) (*http.Request, *auditinternal.Event, []auditinternal.Stage, error) {
+func evaluatePolicyAndCreateAuditEvent(req *http.Request, policy audit.PolicyRuleEvaluator) (*audit.AuditContext, error) {
 	ctx := req.Context()
 
 	attribs, err := GetAuthorizerAttributes(ctx)
 	if err != nil {
-		return req, nil, nil, fmt.Errorf("failed to GetAuthorizerAttributes: %v", err)
+		return nil, fmt.Errorf("failed to GetAuthorizerAttributes: %v", err)
 	}
 
-	level, omitStages := policy.LevelAndStages(attribs)
-	audit.ObservePolicyLevel(ctx, level)
-	if level == auditinternal.LevelNone {
+	ls := policy.EvaluatePolicyRule(attribs)
+	audit.ObservePolicyLevel(ctx, ls.Level)
+	if ls.Level == auditinternal.LevelNone {
 		// Don't audit.
-		return req, nil, nil, nil
+		return &audit.AuditContext{
+			RequestAuditConfig: ls.RequestAuditConfig,
+		}, nil
 	}
 
 	requestReceivedTimestamp, ok := request.ReceivedTimestampFrom(ctx)
 	if !ok {
 		requestReceivedTimestamp = time.Now()
 	}
-	ev, err := audit.NewEventFromRequest(req, requestReceivedTimestamp, level, attribs)
+	ev, err := audit.NewEventFromRequest(req, requestReceivedTimestamp, ls.Level, attribs)
 	if err != nil {
-		return req, nil, nil, fmt.Errorf("failed to complete audit event from request: %v", err)
+		return nil, fmt.Errorf("failed to complete audit event from request: %v", err)
 	}
 
-	req = req.WithContext(audit.WithAuditContext(ctx, &audit.AuditContext{
-		RequestAuditConfig: audit.RequestAuditConfig{},
+	return &audit.AuditContext{
+		RequestAuditConfig: ls.RequestAuditConfig,
 		Event:              ev,
-	}))
-
-	return req, ev, omitStages, nil
+	}, nil
 }
 
 func processAuditEvent(ctx context.Context, sink audit.Sink, ev *auditinternal.Event, omitStages []auditinternal.Stage) bool {

staging/src/k8s.io/apiserver/pkg/endpoints/filters/authn_audit.go

@@ -36,22 +36,26 @@ func WithFailedAuthenticationAudit(failedHandler http.Handler, sink audit.Sink,
 		return failedHandler
 	}
 	return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
-		req, ev, omitStages, err := createAuditEventAndAttachToContext(req, policy)
+		a, err := evaluatePolicyAndCreateAuditEvent(req, policy)
 		if err != nil {
 			utilruntime.HandleError(fmt.Errorf("failed to create audit event: %v", err))
 			responsewriters.InternalError(w, req, errors.New("failed to create audit event"))
 			return
 		}
+
+		ev := a.Event
 		if ev == nil {
 			failedHandler.ServeHTTP(w, req)
 			return
 		}
 
+		req = req.WithContext(audit.WithAuditContext(req.Context(), a))
+
 		ev.ResponseStatus = &metav1.Status{}
 		ev.ResponseStatus.Message = getAuthMethods(req)
 		ev.Stage = auditinternal.StageResponseStarted
 
-		rw := decorateResponseWriter(req.Context(), w, ev, sink, omitStages)
+		rw := decorateResponseWriter(req.Context(), w, ev, sink, a.RequestAuditConfig.OmitStages)
 		failedHandler.ServeHTTP(rw, req)
 	})
 }

staging/src/k8s.io/apiserver/pkg/endpoints/filters/request_deadline.go

@@ -108,24 +108,28 @@ func withFailedRequestAudit(failedHandler http.Handler, statusErr *apierrors.Sta
 		return failedHandler
 	}
 	return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
-		req, ev, omitStages, err := createAuditEventAndAttachToContext(req, policy)
+		a, err := evaluatePolicyAndCreateAuditEvent(req, policy)
 		if err != nil {
 			utilruntime.HandleError(fmt.Errorf("failed to create audit event: %v", err))
 			responsewriters.InternalError(w, req, errors.New("failed to create audit event"))
 			return
 		}
+
+		ev := a.Event
 		if ev == nil {
 			failedHandler.ServeHTTP(w, req)
 			return
 		}
 
+		req = req.WithContext(audit.WithAuditContext(req.Context(), a))
+
 		ev.ResponseStatus = &metav1.Status{}
 		ev.Stage = auditinternal.StageResponseStarted
 		if statusErr != nil {
 			ev.ResponseStatus.Message = statusErr.Error()
 		}
 
-		rw := decorateResponseWriter(req.Context(), w, ev, sink, omitStages)
+		rw := decorateResponseWriter(req.Context(), w, ev, sink, a.RequestAuditConfig.OmitStages)
 		failedHandler.ServeHTTP(rw, req)
 	})
 }

test/integration/examples/webhook_test.go

@@ -28,6 +28,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/wait"
 	auditinternal "k8s.io/apiserver/pkg/apis/audit"
+	"k8s.io/apiserver/pkg/audit"
 	"k8s.io/apiserver/pkg/authorization/authorizer"
 	"k8s.io/kubernetes/cmd/kube-apiserver/app/options"
 	"k8s.io/kubernetes/pkg/controlplane"
@@ -52,14 +53,16 @@ func TestWebhookLoopback(t *testing.T) {
 
 			// Hook into audit to watch requests
 			config.GenericConfig.AuditBackend = auditSinkFunc(func(events ...*auditinternal.Event) {})
-			config.GenericConfig.AuditPolicyRuleEvaluator = auditPolicyRuleEvaluator(func(attrs authorizer.Attributes) (auditinternal.Level, []auditinternal.Stage) {
+			config.GenericConfig.AuditPolicyRuleEvaluator = auditPolicyRuleEvaluator(func(attrs authorizer.Attributes) audit.RequestAuditConfigWithLevel {
 				if attrs.GetPath() == webhookPath {
 					if attrs.GetUser().GetName() != "system:apiserver" {
 						t.Errorf("expected user %q, got %q", "system:apiserver", attrs.GetUser().GetName())
 					}
 					atomic.AddInt32(&called, 1)
 				}
-				return auditinternal.LevelNone, nil
+				return audit.RequestAuditConfigWithLevel{
+					Level: auditinternal.LevelNone,
+				}
 			})
 		},
 	})
@@ -106,9 +109,9 @@ func TestWebhookLoopback(t *testing.T) {
 	}
 }
 
-type auditPolicyRuleEvaluator func(authorizer.Attributes) (auditinternal.Level, []auditinternal.Stage)
+type auditPolicyRuleEvaluator func(authorizer.Attributes) audit.RequestAuditConfigWithLevel
 
-func (f auditPolicyRuleEvaluator) LevelAndStages(attrs authorizer.Attributes) (auditinternal.Level, []auditinternal.Stage) {
+func (f auditPolicyRuleEvaluator) EvaluatePolicyRule(attrs authorizer.Attributes) audit.RequestAuditConfigWithLevel {
 	return f(attrs)
 }