Skip to content

MINOR - Allow app definition to pass the impersonation rules for bots#25909

Merged
pmbrull merged 11 commits intomainfrom
bot-impersonation
Feb 17, 2026
Merged

MINOR - Allow app definition to pass the impersonation rules for bots#25909
pmbrull merged 11 commits intomainfrom
bot-impersonation

Conversation

@pmbrull
Copy link
Copy Markdown
Collaborator

@pmbrull pmbrull commented Feb 16, 2026

Describe your changes:

Also add the necessary changes in the UI to show impersonation

Type of change:

  • Bug fix
  • Improvement
  • New feature
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation

Checklist:

  • I have read the CONTRIBUTING document.
  • My PR title is Fixes <issue-number>: <short explanation>
  • I have commented on my code, particularly in hard-to-understand areas.
  • For JSON Schema changes: I updated the migration scripts or explained why it is not needed.

Summary by Gitar

  • New schema field:
    • allowBotImpersonation in createAppRequest.json enables per-application bot impersonation control (defaults to false)
  • Backend implementation:
    • Method overload in AppRepository.createNewAppBot() sets user-level allowImpersonation flag with backward compatibility
  • UI enhancement:
    • Audit logs display "Impersonated By: {username}" when actions performed via impersonation in AuditLogList.component.tsx
  • Policy update:
    • Added Impersonate operation to ApplicationBotPolicy.json allowed operations

This will update automatically on new commits.


@pmbrull pmbrull changed the title Bot impersonation MINOR - Allow app definition to pass the impersonation rules for bots Feb 16, 2026
@github-actions
Copy link
Copy Markdown
Contributor

TypeScript types have been updated based on the JSON schema changes in the PR

IceS2
IceS2 previously approved these changes Feb 17, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 29 out of 30 changed files in this pull request and generated 3 comments.

@gitar-bot
Copy link
Copy Markdown

gitar-bot Bot commented Feb 17, 2026

🔍 CI failure analysis for e10e179: CI failures appear unrelated to bot impersonation changes. Java integration tests (both retried, still failing) show persistent infrastructure issues. Playwright E2E tests across 2 shards show 2 failed + 19 flaky (all flaky passed on retry) involving login, permissions, search, and version pages - all unrelated to audit log changes.

Issue

CI failures occurred across multiple test environments:

Java Integration Test Failures:

  • integration-tests-postgres-opensearch (job ids: 63883965375, 63887643501) - Retry still failed
  • integration-tests-mysql-elasticsearch (job ids: 63883965543, 63887643516) - Retry still failed

Playwright E2E Test Failures (PostgreSQL):

Shard 6/6:

  • 1 failed: Login flow refresh test
  • 8 flaky (all passed on retry): Tag restricted entity, Users with Data Steward permissions, SearchIndex version pages, Spreadsheet version pages

Shard 3/6:

  • 1 failed: Element visibility assertion
  • 11 flaky (all passed on retry): Glossary term permissions, Service permissions common operations, TableSearch functionality, and others

Test Report Jobs (cascading failures):

  • Test Report (job ids: 63887063207, 63887213916, 63890592290, 63890853628)

Test Results:

  • Playwright (shard 6/6): 1 failed, 8 flaky (all flaky passed on retry)
  • Playwright (shard 3/6): 1 failed, 11 flaky (all flaky passed on retry)

Root Cause

All failures are unrelated to the bot impersonation changes in this PR and are pre-existing infrastructure and test flakiness issues.

Java Integration Test Failures - Infrastructure Issues

Connection Pool Shutdown:

  • Connection pool shut down errors in OpenSearch/Elasticsearch vector service
  • Error: java.lang.IllegalStateException: Connection pool shut down in Apache HttpClient

Service Lifecycle Issues:

  • NDManager has been closed already in DJL embedding client
  • pipelineServiceClient is null when trying to delete pipelines
  • processEngine is null when workflow failure listener tries to store failures

Typical Failing Tests:

  • TestSuiteResourceIT.test_deleteLogicalTestSuiteWithPipeline
  • WorkflowDefinitionResourceIT timeout issues
  • SearchResourceIT.testEntityTypeCountsWithEmptyResults

Playwright E2E Test Failures - Timing and Flakiness

Shard 6/6 (1 failed, 8 flaky):

  • Login flow refresh test (failed)
  • Tag restricted entity, user permissions, version pages (flaky - all passed on retry)

Shard 3/6 (1 failed, 11 flaky):

  • Element visibility assertions (expect(locator).toBeVisible failed)
  • Glossary term permissions
  • Service permissions common operations
  • TableSearch functionality

Details

Why these failures are unrelated to this PR:

This PR changes:

  • Application bot creation with role-based impersonation (AppRepository.java, AppMapper.java, AppResource.java)
  • New role and policy files (ApplicationBotImpersonationRole.json, ApplicationBotImpersonationPolicy.json)
  • UI audit log display (AuditLogList.component.tsx)
  • Schema definitions, localization files, migration scripts
  • New integration test: AppsResourceIT.java

Java integration test failures involve:

  • OpenSearch/Elasticsearch vector service connection pool
  • Pipeline service client initialization
  • Workflow engine timing
  • DJL NDManager lifecycle

Playwright test failures involve:

  • Login flow and refresh tokens
  • Tag, user, glossary, and service permissions
  • Version pages (SearchIndex, Spreadsheet)
  • Table search functionality

None of the modified files affect:

  • Vector embedding service lifecycle
  • Pipeline service client initialization
  • Workflow engine infrastructure
  • Login flow or refresh tokens
  • Permissions logic (tags, users, glossary, services)
  • Version pages
  • Table search functionality

Failure Pattern:

  • Java integration tests: Both jobs retried but still failed with same infrastructure issues
  • Playwright tests: Total 19 flaky tests across 2 shards (all passed on retry)
  • Playwright tests: 2 failed tests across 2 shards
  • These are the same types of failures seen in previous commits

Evidence:

java.lang.IllegalStateException: Connection pool shut down
ERROR: Cannot invoke "...PipelineServiceClientInterface.deletePipeline(...)" because "this.pipelineServiceClient" is null
Error: expect(locator).toBeVisible failed
Error: expect(locator).not.toBeVisible failed

The failures show persistent test infrastructure issues in Java integration tests and widespread test flakiness in Playwright E2E tests - none related to application bot impersonation functionality.

Code Review 👍 Approved with suggestions 1 resolved / 1 findings

Well-structured implementation of per-app bot impersonation with proper admin-only enforcement, clean role/policy separation, and good test coverage. The main outstanding concern (already posted) is that allowImpersonation is silently ignored when the bot user already exists during createOrUpdate.

✅ 1 resolved
Security: allowBotImpersonation silently ignored when botName is provided

📄 openmetadata-service/src/main/java/org/openmetadata/service/resources/apps/AppMapper.java:89
In AppMapper.validateAndAddBot(), when a botName is provided in the CreateApp request (line 89), the code just looks up the existing bot by name and the allowBotImpersonation flag is completely disregarded. This creates two problems:

  1. Silent misconfiguration: An admin sets allowBotImpersonation=true and provides a botName, expecting the bot to get impersonation capabilities, but nothing changes on the referenced bot.

  2. Potential bypass: Conversely, a non-impersonation bot could be referenced when allowBotImpersonation=false, but if that bot already has impersonation privileges from a previous setup, the app silently inherits those privileges.

Consider either:

  • Throwing a BadRequestException if both botName and allowBotImpersonation=true are provided (since impersonation config cannot be applied to a pre-existing bot reference)
  • Or validating that the referenced bot's user has allowImpersonation matching the requested flag

Tip

Comment Gitar fix CI or enable auto-apply: gitar auto-apply:on

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change:

Auto-apply Compact
gitar auto-apply:on         
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar

@sonarqubecloud
Copy link
Copy Markdown

@sonarqubecloud
Copy link
Copy Markdown

@pmbrull pmbrull merged commit a1e3a49 into main Feb 17, 2026
36 of 45 checks passed
@pmbrull pmbrull deleted the bot-impersonation branch February 17, 2026 18:52
pmbrull added a commit that referenced this pull request Feb 17, 2026
…#25909)

* MINOR - Streamline bot impersonation from apps

* MINOR - Streamline bot impersonation from apps

* MINOR - Streamline bot impersonation from apps

* MINOR - Streamline bot impersonation from apps

* Update generated TypeScript types

* policy flag

* policy flag

* policy flag

* policy flag

* fix feedback

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Ingestion safe to test Add this label to run secure Github workflows on PRs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants