Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
107282: sql: do not audit internal executors r=yuzefovich a=yuzefovich

Previously, we were using `planner.isInternalPlanner` to check whether audit logging should be applied, but that field is not set for internal executors, and I believe IE should be excluded from audit too.

Epic: None

Release note: None

Co-authored-by: Yahor Yuzefovich <[email protected]>
  • Loading branch information
craig[bot] and yuzefovich committed Jul 21, 2023
2 parents be21b3b + a3a6dc5 commit 73f51aa
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 7 deletions.
18 changes: 12 additions & 6 deletions pkg/sql/audit_logging.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,9 @@ func (p *planner) maybeAuditSensitiveTableAccessEvent(
)
}

func (p *planner) maybeAuditRoleBasedAuditEvent(ctx context.Context) {
func (p *planner) maybeAuditRoleBasedAuditEvent(ctx context.Context, execType executorType) {
// Avoid doing audit work if not necessary.
if p.shouldNotRoleBasedAudit() {
if p.shouldNotRoleBasedAudit(execType) {
return
}

Expand Down Expand Up @@ -114,10 +114,16 @@ func (p *planner) initializeReducedAuditConfig(ctx context.Context) {
p.reducedAuditConfig.AuditSetting = p.AuditConfig().GetMatchingAuditSetting(userRoles, user)
}

// shouldNotRoleBasedAudit checks if we should do any auditing work for RoleBasedAuditEvents.
func (p *planner) shouldNotRoleBasedAudit() bool {
// shouldNotRoleBasedAudit checks if we should do any auditing work for
// RoleBasedAuditEvents.
func (p *planner) shouldNotRoleBasedAudit(execType executorType) bool {
// Do not do audit work if role-based auditing is not enabled.
// Do not emit audit events for reserved users/roles. This does not omit the root user.
// Do not emit audit events for reserved users/roles. This does not omit the
// root user.
// Do not emit audit events for internal planners.
return !auditlogging.UserAuditEnabled(p.execCfg.Settings, p.EvalContext().ClusterID) || p.User().IsReserved() || p.isInternalPlanner
// Do not emit audit events for internal executors.
return !auditlogging.UserAuditEnabled(p.execCfg.Settings, p.EvalContext().ClusterID) ||
p.User().IsReserved() ||
p.isInternalPlanner ||
execType == executorTypeInternal
}
2 changes: 1 addition & 1 deletion pkg/sql/exec_log.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ func (p *planner) maybeLogStatement(
queryStats *topLevelQueryStats,
statsCollector sqlstats.StatsCollector,
) {
p.maybeAuditRoleBasedAuditEvent(ctx)
p.maybeAuditRoleBasedAuditEvent(ctx, execType)
p.maybeLogStatementInternal(ctx, execType, isCopy, numRetries, txnCounter,
rows, bulkJobId, err, queryReceived, hasAdminRoleCache,
telemetryLoggingMetrics, stmtFingerprintID, queryStats, statsCollector,
Expand Down

0 comments on commit 73f51aa

Please sign in to comment.