Skip to content

fix: Database_observability: grant check only require SELECT *.* on perf_schema #5294

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
matthewnolf merged 4 commits into from
Jan 22, 2026
Merged

fix: Database_observability: grant check only require SELECT *.* on perf_schema #5294

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f7d17b1
Fix grant permission check to only require SELECT on perf_schema
matthewnolf Jan 20, 2026
e774419
lint
matthewnolf Jan 20, 2026
b6c3a3b
Merge branch 'main' into mn/db-o11y-mysql/status-permissions-fix
matthewnolf Jan 22, 2026
d89ceee
update checkRequiredGrants to include which grants are missing
matthewnolf Jan 22, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 47 additions & 13 deletions internal/component/database_observability/mysql/collector/health_check.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,8 @@ func checkAlloyVersion(ctx context.Context, db *sql.DB) healthCheckResult {
}

// checkRequiredGrants verifies required privileges are present.
// Requires: PROCESS, REPLICATION CLIENT, SHOW VIEW on *.*
// and SELECT on performance_schema.*
func checkRequiredGrants(ctx context.Context, db *sql.DB) healthCheckResult {
r := healthCheckResult{name: "RequiredGrantsPresent"}
req := map[string]bool{
Expand All @@ -160,28 +162,60 @@ func checkRequiredGrants(ctx context.Context, db *sql.DB) healthCheckResult {
}
up := strings.ToUpper(grantLine)

// Mark individual privileges if present on *.* scope.
for k := range req {
if strings.Contains(up, " ON *.*") && strings.Contains(up, k) {
req[k] = true
if strings.Contains(up, "SELECT") {
if strings.Contains(up, " ON `PERFORMANCE_SCHEMA`.*") ||
strings.Contains(up, " ON PERFORMANCE_SCHEMA.*") ||
strings.Contains(up, " ON `PERFORMANCE_SCHEMA`.") ||
strings.Contains(up, " ON *.*") {

req["SELECT"] = true
}
}

if strings.Contains(up, "ALL PRIVILEGES") {
if strings.Contains(up, " ON `PERFORMANCE_SCHEMA`.*") ||
strings.Contains(up, " ON PERFORMANCE_SCHEMA.*") ||
strings.Contains(up, " ON `PERFORMANCE_SCHEMA`.") ||
strings.Contains(up, " ON *.*") {

req["SELECT"] = true
}
}

if strings.Contains(up, "SHOW VIEW") {
req["SHOW VIEW"] = true
}
Comment on lines +185 to +187
Copy link
Contributor

@cristiangreco cristiangreco Jan 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe for a followup: would be cool to show on which objects SHOW VIEW is granted (e.g. *.* vs payments.* or something else), as it helps debug why e.g. a specific schema is not reported.


if strings.Contains(up, "PROCESS") && strings.Contains(up, " ON *.*") {
req["PROCESS"] = true
}

if strings.Contains(up, "REPLICATION CLIENT") && strings.Contains(up, " ON *.*") {
req["REPLICATION CLIENT"] = true
}
}
if err := rows.Err(); err != nil {
r.err = fmt.Errorf("iterate SHOW GRANTS: %w", err)
return r
}

r.result = true
for k, found := range req {
if !found {
r.result = false
if r.value == "" {
r.value = "missing: " + k
} else {
r.value += "," + k
}
r.result = req["PROCESS"] && req["REPLICATION CLIENT"] && req["SELECT"] && req["SHOW VIEW"]
Copy link
Contributor

@cristiangreco cristiangreco Jan 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we add to r.result the list of expected vs missing grants?

Copy link
Contributor Author

@matthewnolf matthewnolf Jan 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

r.result is a boolean whether the check passed or failed.
r.value can be used to send details of the check.

This comment seems similar to #5294 (comment) in which we attach more information about which grant is missing?

Copy link
Contributor

@cristiangreco cristiangreco Jan 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

r.result is a boolean whether the check passed or failed.
r.value can be used to send details of the check.

Sorry I confused the two fields. But yeah, basically I was thinking of attaching more info like I mentioned in the other comment.


if !r.result {
var missing []string
if !req["PROCESS"] {
missing = append(missing, "PROCESS")
}
if !req["REPLICATION CLIENT"] {
missing = append(missing, "REPLICATION CLIENT")
}
if !req["SELECT"] {
missing = append(missing, "SELECT on performance_schema.*")
}
if !req["SHOW VIEW"] {
missing = append(missing, "SHOW VIEW")
}
r.value = fmt.Sprintf("missing grants: %s", strings.Join(missing, ", "))
}

return r
Expand Down
36 changes: 35 additions & 1 deletion internal/component/database_observability/mysql/collector/health_check_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,9 +75,10 @@ func TestHealthCheck(t *testing.T) {
failingCheckName string
customSetup func(mock sqlmock.Sqlmock)
expectedResult string
expectedValue string
}{
{
name: "missing grants",
name: "missing PROCESS and REPLICATION CLIENT grants",
failingCheckName: "RequiredGrantsPresent",
customSetup: func(mock sqlmock.Sqlmock) {
mock.ExpectQuery(`SHOW GRANTS`).
Expand All @@ -87,6 +88,35 @@ func TestHealthCheck(t *testing.T) {
)
},
expectedResult: `result="false"`,
expectedValue: `value="missing grants: PROCESS, REPLICATION CLIENT"`,
},
{
name: "missing SELECT on performance_schema",
failingCheckName: "RequiredGrantsPresent",
customSetup: func(mock sqlmock.Sqlmock) {
mock.ExpectQuery(`SHOW GRANTS`).
WillReturnRows(
sqlmock.NewRows([]string{"Grants"}).
AddRow("GRANT PROCESS, REPLICATION CLIENT, SHOW VIEW ON *.* TO 'user'@'host'").
AddRow("GRANT SELECT ON cars.* TO 'user'@'host'"),
)
},
expectedResult: `result="false"`,
expectedValue: `value="missing grants: SELECT on performance_schema.*"`,
},
{
name: "missing SELECT and SHOW VIEW grants",
failingCheckName: "RequiredGrantsPresent",
customSetup: func(mock sqlmock.Sqlmock) {
mock.ExpectQuery(`SHOW GRANTS`).
WillReturnRows(
sqlmock.NewRows([]string{"Grants"}).
AddRow("GRANT PROCESS, REPLICATION CLIENT ON *.* TO 'user'@'host'").
AddRow("GRANT SELECT ON cars.* TO 'user'@'host'"),
)
},
expectedResult: `result="false"`,
expectedValue: `value="missing grants: SELECT on performance_schema.*, SHOW VIEW"`,
},
{
name: "no rows in events statements digest",
Expand All @@ -99,6 +129,7 @@ func TestHealthCheck(t *testing.T) {
)
},
expectedResult: `result="false"`,
expectedValue: "",
},
}

Expand Down Expand Up @@ -150,6 +181,9 @@ func TestHealthCheck(t *testing.T) {
if strings.Contains(entry.Line, tc.failingCheckName) {
require.Equal(t, model.LabelSet{"op": OP_HEALTH_STATUS}, entry.Labels)
require.Contains(t, entry.Line, tc.expectedResult)
if tc.expectedValue != "" {
require.Contains(t, entry.Line, tc.expectedValue)
}
found = true
break
}
Expand Down
Loading