Support `X-Trino-Role` header #322

ssheikin · 2025-11-04T01:44:32Z

solves

Add X-Trino-Role Header Support #299

Summary by Sourcery

Support the X-Trino-Role header by allowing users to specify a role in the data source config and passing it through backend context into SQL requests.

New Features:

Add UI input for configuring a Trino role in the data source settings
Propagate the configured role as X-Trino-Role header in SQL queries

Enhancements:

Extend TypeScript types and Go settings model to include the role field

sourcery-ai · 2025-11-04T01:44:37Z

Reviewer's Guide

This PR implements support for the X-Trino-Role header by extending the frontend configuration UI and type definitions, adding a Role field in the backend settings model, propagating the role value through the datasource context, and injecting it into SQL query arguments.

Entity relationship diagram for updated TrinoDatasourceSettings model

erDiagram
  TrinoDatasourceSettings {
    string ClientId
    string ClientSecret
    string ImpersonationUser
    string Role
    string ClientTags
  }

Class diagram for updated TrinoDataSourceOptions and TrinoDatasourceSettings

classDiagram
  class TrinoDataSourceOptions {
    tokenUrl?: string
    clientId?: string
    impersonationUser?: string
    role?: string
    clientTags?: string
  }
  class TrinoDatasourceSettings {
    ClientId string
    ClientSecret string
    ImpersonationUser string
    Role string
    ClientTags string
  }

File-Level Changes

Change	Details	Files
Add Role field to configuration UI	Introduce onRoleChange handler Render new Role InlineField with Input component	`src/ConfigEditor.tsx`
Extend frontend type definitions to include Role	Add role property to TrinoDataSourceOptions interface	`src/types.ts`
Add Role field to backend settings model	Define Role in TrinoDatasourceSettings struct	`pkg/trino/models/settings.go`
Propagate Role through datasource context	Set context value for role header if provided Format role header as system=ROLE{role}	`pkg/trino/datasource-context.go`
Include Role in SQL query arguments	Retrieve role from context Append role named argument to SQL args	`pkg/trino/datasource.go`

Tips and commands

Interacting with Sourcery

Trigger a new review: Comment @sourcery-ai review on the pull request.
Continue discussions: Reply directly to Sourcery's review comments.
Generate a GitHub issue from a review comment: Ask Sourcery to create an
issue from a review comment by replying to it. You can also reply to a
review comment with @sourcery-ai issue to create an issue from it.
Generate a pull request title: Write @sourcery-ai anywhere in the pull
request title to generate a title at any time. You can also comment
@sourcery-ai title on the pull request to (re-)generate the title at any time.
Generate a pull request summary: Write @sourcery-ai summary anywhere in
the pull request body to generate a PR summary at any time exactly where you
want it. You can also comment @sourcery-ai summary on the pull request to
(re-)generate the summary at any time.
Generate reviewer's guide: Comment @sourcery-ai guide on the pull
request to (re-)generate the reviewer's guide at any time.
Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
pull request to resolve all Sourcery comments. Useful if you've already
addressed all the comments and don't want to see them anymore.
Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
request to dismiss all existing Sourcery reviews. Especially useful if you
want to start fresh with a new review - don't forget to comment
@sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

Enable or disable review features such as the Sourcery-generated pull request
summary, the reviewer's guide, and others.
Change the review language.
Add, remove or edit custom review instructions.
Adjust other review settings.

Getting Help

Contact our support team for questions or feedback.
Visit our documentation for detailed guides and information.
Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

sourcery-ai

Hey there - I've reviewed your changes and they look great!

Prompt for AI Agents

Please address the comments from this code review:

## Individual Comments

### Comment 1
<location> `pkg/trino/datasource-context.go:45-46` </location>
<code_context>
 		ctx = context.WithValue(ctx, trinoUserHeader, user)
 	}

+	if settings.Role != "" {
+		ctx = context.WithValue(ctx, trinoRoleHeader, "system=ROLE{"+settings.Role+"}")
+	}
+
</code_context>

<issue_to_address>
**🚨 suggestion (security):** Consider validating the Role value before formatting.

If settings.Role is user-supplied, ensure it is validated or sanitized to prevent malformed or unexpected header values.

Suggested implementation:

```golang
	if settings.Role != "" {
		role := sanitizeRole(settings.Role)
		if role != "" {
			ctx = context.WithValue(ctx, trinoRoleHeader, "system=ROLE{"+role+"}")
		}
	}

```

```golang
const (
	accessTokenKey     = "accessToken"
	trinoUserHeader    = "X-Trino-User"
	trinoRoleHeader    = "X-Trino-Role"
	trinoClientTagsKey = "X-Trino-Client-Tags"
	bearerPrefix       = "Bearer "
)

// sanitizeRole ensures the role value is safe for use in the header.
// Only allows alphanumeric and underscores, returns empty string if invalid.
func sanitizeRole(role string) string {
	for _, r := range role {
		if !(r >= 'a' && r <= 'z') && !(r >= 'A' && r <= 'Z') && !(r >= '0' && r <= '9') && r != '_' {
			return ""
		}
	}
	return role
}

```
</issue_to_address>

### Comment 2
<location> `pkg/trino/datasource.go:97-98` </location>
<code_context>
 		args = append(args, sql.Named(accessTokenKey, accessToken.(string)))
 	}

+	if role != nil {
+		args = append(args, sql.Named(trinoRoleHeader, role.(string)))
+	}
+
</code_context>

<issue_to_address>
**issue (bug_risk):** Type assertion on role could panic if not a string.

Use a type check or type switch before asserting role as a string to prevent runtime panics.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

pkg/trino/datasource-context.go

pkg/trino/datasource.go

nineinchnick · 2025-11-04T15:35:53Z

Can you add an e2e test that'd verify this works?

ssheikin · 2025-11-04T15:50:32Z

in progress.

pkg/trino/datasource-context.go

.github/workflows/ci.yml

src/ConfigEditor.tsx

src/e2e.test.ts

ssheikin · 2025-11-17T12:44:33Z

@nineinchnick Could you let me know when you might have time to review this?

kokosing · 2025-11-17T15:39:19Z

src/e2e.test.ts

+test('test with role', async ({ page }) => {
+    await login(page);
+    await goToTrinoSettings(page);
+    await setupDataSourceWithRole(page, '},system=ALL,hive=ROLE{admin');


how you can set a role like this?

kokosing · 2025-11-17T15:40:04Z

src/e2e.test.ts

+    await expect(page.getByText(/Access Denied: Cannot show roles/)).toBeVisible();
+});
+
+async function runRoleQuery(page: Page) {


why utility functions are on top and why some are on the bottom?

The good practice, at least in trino java code to put utility functions on the bottom.
I'd put setupDataSourceWithRole on the bottom too, but very similar functions were on the top, so it looks there more natural.
Please let me know if you have an idea how to improve that.

We can reorder functions in this file in a separate commit

That's not for free - the git history of the file will be overriden.

nineinchnick · 2025-11-17T16:26:32Z

pkg/trino/datasource-context.go

 	}

+	if settings.Role != "" {
+		ctx = context.WithValue(ctx, trinoRoleHeader, "system=ROLE{"+settings.Role+"}")


Note there's an open PR to support roles directly in the Go driver: trinodb/trino-go-client#144

It's not required here, but we had a discussion there about the format in which to specify roles. We tried being consistent with other drivers, like JDBC and Python.

Sounds good to me. What is the golden standard here we should follow? https://trino.io/docs/current/client/jdbc.html ?

Authorization roles to use for catalogs, specified as a list of key-value pairs for the catalog and role. For example, catalog1:roleA;catalog2:roleB sets roleA for catalog1 and roleB for catalog2.

That makes sense. As I understand for system role we simply say system:sysadmin. @ssheikin that sounds like good way to expose support to all kind of roles and makes it easy to test.

@nineinchnick @kokosing
This was my initial approach. In order to reduce scope and moving parts the PR was simplified.
The current PR allows to specify system role in a very simple format.
An improvement to set roles for catalogs in the jdbc compliant format may follow up.
The code for http client header is identical to #309

This was my initial approach.

Hm... I was under impression that initial approach was similar, but a bit different than in JDBC. That your approach followed the protocol implementation, not the JDBC format.

An improvement to set roles for catalogs in the jdbc compliant format may follow up.
I think this is part of the protocol (UX), changing semantics of the field seems like a breaking change. If it is not a big deal I would prefer to do it once and don't come if not needed.

Let's do it right and be consistent with other drivers.

Also note the releases are blocked until either #303 or #321 is merged.

cla-bot bot added the cla-signed label Nov 4, 2025

sourcery-ai bot reviewed Nov 4, 2025

View reviewed changes

pkg/trino/datasource-context.go Show resolved Hide resolved

pkg/trino/datasource.go Show resolved Hide resolved

ssheikin commented Nov 4, 2025

View reviewed changes

pkg/trino/datasource-context.go Show resolved Hide resolved

Move config.properties under trino folder

57c2f9c

ssheikin force-pushed the ssheikin/X-Trino-Role branch 2 times, most recently from a6f9187 to 599f44b Compare November 7, 2025 20:21

ssheikin commented Nov 7, 2025

View reviewed changes

pkg/trino/datasource-context.go Show resolved Hide resolved

ssheikin force-pushed the ssheikin/X-Trino-Role branch from 599f44b to 69759bd Compare November 7, 2025 20:33

ksobolew reviewed Nov 12, 2025

View reviewed changes

.github/workflows/ci.yml Show resolved Hide resolved

src/ConfigEditor.tsx Outdated Show resolved Hide resolved

src/e2e.test.ts Outdated Show resolved Hide resolved

src/e2e.test.ts Outdated Show resolved Hide resolved

Support X-Trino-Role header

91e6b3e

ssheikin force-pushed the ssheikin/X-Trino-Role branch from 69759bd to 91e6b3e Compare November 17, 2025 14:48

kokosing reviewed Nov 17, 2025

View reviewed changes

nineinchnick reviewed Nov 17, 2025

View reviewed changes

Support X-Trino-Role header #322

Are you sure you want to change the base?

Support X-Trino-Role header #322

Uh oh!

Conversation

ssheikin commented Nov 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by Sourcery

Uh oh!

sourcery-ai bot commented Nov 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviewer's Guide

Entity relationship diagram for updated TrinoDatasourceSettings model

Class diagram for updated TrinoDataSourceOptions and TrinoDatasourceSettings

File-Level Changes

Interacting with Sourcery

Customizing Your Experience

Getting Help

Uh oh!

sourcery-ai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

nineinchnick commented Nov 4, 2025

Uh oh!

ssheikin commented Nov 4, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ssheikin commented Nov 17, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ssheikin Nov 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

4 participants

Support `X-Trino-Role` header #322

Support `X-Trino-Role` header #322

ssheikin commented Nov 4, 2025 •

edited

Loading

sourcery-ai bot commented Nov 4, 2025 •

edited

Loading

ssheikin Nov 18, 2025 •

edited

Loading