Skip to content
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

Jump to bottom

DB Console UI shows "undefined" when role session variable is altered #124871

Open
osmes opened this issue May 30, 2024 · 2 comments · May be fixed by #126354
Open

DB Console UI shows "undefined" when role session variable is altered #124871

osmes opened this issue May 30, 2024 · 2 comments · May be fixed by #126354
Assignees
@dhartunian
Labels
branch-release-23.1 Used to mark GA and release blockers and technical advisories for 23.1 branch-release-23.2 Used to mark GA and release blockers and technical advisories for 23.2 branch-release-24.1 Used to mark GA and release blockers and technical advisories for 24.1 C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. O-support Would prevent or help troubleshoot a customer escalation - bugs, missing observability/tooling, docs P-2 Issues/test failures with a fix SLA of 3 months T-observability
Projects
SQL Foundations

Comments

@osmes
Copy link
Contributor

osmes commented May 30, 2024 •
edited by cockroach-jira-scripts
Loading

Describe the problem

DB Console UI shows message "Role Altered: User root altered role <role_name> with options undefined" when altering a role to change session variables like with ALTER ROLE test SET timezone = 'America/New_York';. The undefined keyword implies that there is a problem in the reporting even if the intention is to not convey specific details of a session variable change.

Screenshot 2024-05-30 at 12 16 26 PM

To Reproduce

  1. Started cockroach demo
  2. CREATE ROLE test;
  3. ALTER ROLE test WITH CONTROLCHANGEFEED;
  4. ALTER ROLE test SET timezone = 'America/New_York';
  5. Observe with options CONTROLCHANGEFEED for step 3, but with options undefined for step 4.

Expected behavior
The UI should either:

  • include the session variable change performed by the command
  • indicate that a session variable was set rather than report as with options undefined
  • omit undefined to leave the UI reporting for session variable changes looking like "Role Altered: User root altered role <role_name> with options".

Environment:

  • CockroachDB version 23.1, 23.2, 24.1

Jira issue: CRDB-39120
@osmes osmes added the C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. label May 30, 2024
@blathers-crl blathers[crl]
Copy link

blathers-crl bot commented May 30, 2024

Hi @osmes, please add branch-* labels to identify which branch(es) this C-bug affects.

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

@osmes osmes added the O-support Would prevent or help troubleshoot a customer escalation - bugs, missing observability/tooling, docs label May 30, 2024
@yuzefovich yuzefovich added the T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions) label May 30, 2024
@blathers-crl blathers-crl bot added this to Triage in SQL Foundations May 30, 2024
@rafiss
Copy link
Collaborator

rafiss commented May 31, 2024

This looks like it should be fixed in the frontend. The code here is assuming that info.Options will be set.

cockroach/pkg/ui/workspaces/db-console/src/util/events.ts

Lines 180 to 181 in 9e61527

case eventTypes.ALTER_ROLE:
return `Role Altered: User ${info.User} altered role ${info.RoleName} with options ${info.Options}`;

But for ALTER ROLE ... SET, we only populate info.SetInfo:

cockroach/pkg/sql/alter_role.go

Lines 501 to 506 in c4065c0

return params.p.logEvent(params.ctx,
0, /* no target */
&eventpb.AlterRole{
RoleName: roleName.Normalized(),
SetInfo: []string{"DEFAULTSETTINGS"},
})

This was done in 0a0271c.

Before that, this would get logged similar to other ALTER ROLE statements:

cockroach/pkg/sql/alter_role.go

Lines 270 to 275 in c4065c0

return params.p.logEvent(params.ctx,
0, /* no target */
&eventpb.AlterRole{
RoleName: n.roleName.Normalized(),
Options: optStrs,
})

Changing the frontend code to check if info.Options is defined sounds like the right fix.

@rafiss rafiss added T-observability and removed T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions) labels May 31, 2024
@exalate-issue-sync exalate-issue-sync bot added the P-2 Issues/test failures with a fix SLA of 3 months label Jun 18, 2024
@dhartunian dhartunian added branch-release-23.1 Used to mark GA and release blockers and technical advisories for 23.1 branch-release-24.1 Used to mark GA and release blockers and technical advisories for 24.1 branch-release-23.2 Used to mark GA and release blockers and technical advisories for 23.2 labels Jun 26, 2024
dhartunian added a commit to dhartunian/cockroach that referenced this issue Jun 27, 2024
@dhartunian
ui: alter role events render correctly
84cf77d 
Previously, ALTER ROLE events without role options would render with
an "undefined" option in the event log on the DB Console. This change
amends the rendering logic to correctly render events without any
options.

Resolves cockroachdb#124871
Epic: None

Release note (bug fix,ui change): ALTER ROLE events in the DB Console
event log now render correctly when the event does not contain any
role options.
@dhartunian dhartunian linked a pull request Jun 27, 2024 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@dhartunian dhartunian

Labels
branch-release-23.1 Used to mark GA and release blockers and technical advisories for 23.1 branch-release-23.2 Used to mark GA and release blockers and technical advisories for 23.2 branch-release-24.1 Used to mark GA and release blockers and technical advisories for 24.1 C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. O-support Would prevent or help troubleshoot a customer escalation - bugs, missing observability/tooling, docs P-2 Issues/test failures with a fix SLA of 3 months T-observability
Projects
SQL Foundations
  
Triage
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

ui: alter role events render correctly dhartunian/cockroach
4 participants
@dhartunian @rafiss @yuzefovich @osmes