Skip to content

fix(ingestion-pipeline): inherit owners from service and authorize trigger#28109

Open
manerow wants to merge 6 commits into
mainfrom
fix/27962-ingestion-pipeline-owner-inheritance
Open

fix(ingestion-pipeline): inherit owners from service and authorize trigger#28109
manerow wants to merge 6 commits into
mainfrom
fix/27962-ingestion-pipeline-owner-inheritance

Conversation

@manerow
Copy link
Copy Markdown
Contributor

@manerow manerow commented May 14, 2026

Fixes #27962

Summary

Two coordinated platform fixes to make isOwner()-based authorization work correctly on IngestionPipeline and to close a long-standing gap on the trigger endpoint:

  1. Owner inheritanceIngestionPipeline.owners now inherits from the parent referenced by service (Service / TestSuite / App). Inherited owners are tagged inherited: true, indexed for search, and visible to the policy evaluator. Test-suite pipelines inherit transitively from the underlying Table via TestSuiteRepository.setInheritedFields.
  2. Trigger authorizationPOST /v1/services/ingestionPipelines/trigger/{id} previously skipped authorizer.authorize(...) entirely, so any authenticated user could trigger any pipeline. It now authorizes against MetadataOperation.TRIGGER (the existing enum value already declared in the method for limits.enforceLimits — we just wire it to the authorizer).

A coordinated set of seeded-policy updates + idempotent upgrade migration preserves pre-fix behavior for default roles that previously had broad capability, so no customer is surprised by the new authz check on the trigger endpoint.

What's changing

1. IngestionPipelineRepository.setInheritedFields override

@Override
public void setInheritedFields(IngestionPipeline ingestionPipeline, Fields fields) {
  EntityReference serviceRef = ingestionPipeline.getService();
  if (serviceRef == null) return;
  try {
    EntityInterface parent = Entity.getEntity(serviceRef, "owners,domains", ALL);
    inheritOwners(ingestionPipeline, fields, parent);
    inheritDomains(ingestionPipeline, fields, parent);
  } catch (EntityNotFoundException e) {
    LOG.debug("Parent service {} not found for ingestion pipeline {}; skipping inheritance",
        serviceRef.getFullyQualifiedName(), ingestionPipeline.getFullyQualifiedName());
  }
}
  • Mirrors the established pattern (TableRepository.setInheritedFields:311-321, TestSuiteRepository.setInheritedFields, etc.).
  • Uses the existing inheritOwners / inheritDomains helpers (EntityRepository.java:4510-4525), which already null-guard and tag refs with inherited=true.
  • try/catch EntityNotFoundException protects GET / list endpoints from 500s when a service is soft-deleted with orphan pipelines.
  • setFieldsInBulk already routes through super.setFieldsInBulk → base setInheritedFields(List) → per-entity call, so list endpoints get inheritance for free. No bulk override needed.

2. IngestionPipelineResource.triggerPipelineInternal authorize call

Adds the missing authorizer.authorize(...) call to the existing OperationContext(entityType, MetadataOperation.TRIGGER) that was already built in the method:

public PipelineServiceClientResponse triggerPipelineInternal(
    UUID id, UriInfo uriInfo, SecurityContext securityContext, String botName) {
  OperationContext operationContext = new OperationContext(entityType, MetadataOperation.TRIGGER);
  authorizer.authorize(securityContext, operationContext, getResourceContextById(id));
  if (pipelineServiceClient == null) { ... }
  // ... rest unchanged, including limits.enforceLimits(...)
}
  • Placed before the pipelineServiceClient == null early-return so authz fires even in environments without an Airflow client (tests, dev, customer instances without pipeline service).
  • Uses the existing TRIGGER operation value rather than EDIT_ALL, consistent with upstream AI #200 - Add TRIGGER permission to application bots #25113 (AI #200 - Add TRIGGER permission to application bots) which already established Trigger as a discrete, explicitly-granted operation.
  • getResourceContextById(id) builds a ResourceContext that loads owners for authz, so the inherited owners populated by Change 1 are visible to isOwner() evaluation.

3. Seeded policies updated for Trigger + upgrade migration

EditAll does not imply Trigger (per CompiledRule.matchOperation:221-224 — only Edit*-prefixed ops are covered by EditAll). To preserve pre-fix behavior for default roles that previously had broad capability, six seeded policies receive Trigger via an idempotent migration:

Policy file Rule Why
IngestionBotPolicy IngestionBotRule-Allow Broad bot — main automation identity
LineageBotPolicy LineageBotRule-Allow Broad bot on ["All"] resources
ProfilerBotPolicy ProfilerBotBotRule-Allow Broad bot on ["All"] resources
QualityBotPolicy QualityBotBotRule-Allow Broad bot on ["All"] resources
UsageBotPolicy UsageBotRule-Allow-Usage Broad bot on ["All"] resources
DataStewardPolicy DataStewardPolicy-EditRule Has EditOwners on all resources — stewards could already reach trigger via an ownership-rewrite escalation (verified empirically). Granting Trigger directly aligns the policy with the effective capability and improves audit clarity.

Migration (migration/utils/v1129/MigrationUtil.addTriggerOperationToDefaultPolicies) iterates the six (policy, rule) targets and calls the existing idempotent v160.MigrationUtil.addOperationsToPolicyRule helper. Wired into:

  • migration/{mysql,postgres}/v1129/Migration.java — runs on upgrade to 1.12.9 (companion empty SQL placeholders in bootstrap/sql/migrations/native/1.12.9/{mysql,postgres}/).
  • migration/{mysql,postgres}/v1130/Migration.java — runs again on upgrade to 1.13.0 as an idempotent safety net (existing runDataMigration body extended with the call).

Deliberately out of scope:

  • AutoClassificationBotPolicyEditAll is scoped to Table / Container only; Trigger doesn't apply to those resources, so adding it would be a no-op.
  • DataConsumerPolicy — no EditAll, no EditOwners; Data Consumers genuinely shouldn't trigger pipelines.
  • OrganizationPolicy-NoOwner-Rule — adding Trigger here would re-introduce trivial-trigger for any unowned entity via noOwner().
  • OrganizationPolicy-Owner-Rule already grants [All] (which includes Trigger) to isOwner(), so no change needed.
  • CompiledRule.matchOperationno platform-wide semantic change. EditAll → Trigger would have been a hard rule on the operator. We modify shipped policies, not the operator's behavior, so customer-built EditAll policies remain unchanged and customers retain control over which roles trigger.

4. Tests

New file: openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/IngestionPipelineOwnerInheritanceIT.java. Two tests in the new SDK-client framework (the old IngestionPipelineResourceTest.java was removed in #26204):

  1. testInheritedOwners_fromService — service owner appears on pipeline GET with inherited: true.
  2. testIsOwnerPolicy_appliesToEditAndTrigger — full Policy → Role → User chain with Operations: [EditAll, Trigger], Condition: isOwner(). Owner can PATCH displayName + POST /trigger; non-owner gets precise permissionNotAllowed(USER2, [EditDisplayName]) / permissionNotAllowed(USER2, [Trigger]) 403s.

Behavior change to call out

/trigger now requires Trigger. Three groups of identities are affected:

Can trigger after the fix:

  • Admins (unchanged — via All).
  • Owners of the parent Service / TestSuite / App, via the default OrganizationPolicy-Owner-Rule. Inheritance fix makes this fire automatically.
  • Users with the default DataSteward role (via the migration above).
  • All seeded bots that previously had broad EditAll: Ingestion, Application, Lineage, Profiler, Quality, Usage.
  • Anyone granted Trigger explicitly in a custom policy.

Cannot trigger after the fix (intentional tightening):

  • Default DataConsumer role.
  • AutoClassificationBot (its EditAll scope doesn't cover ingestionPipeline).
  • DefaultBot / ScimBot (no broad edit grant).
  • Generic authenticated users with no ownership and no Trigger grant — closes the pre-fix trivial-trigger hole.

Customers should audit (the narrow regression surface):

  • Custom automation that calls /trigger using a non-admin / non-default-bot identity needs an explicit Trigger grant.
  • Custom-built roles with broad EditAll on ingestionPipeline (not the default DataSteward, which is covered by the migration) need Trigger added to keep their pre-fix trigger capability.

Unaffected:

  • Scheduled Airflow runs — the scheduler fires DAGs directly; the metadata CLI only does GET /pipelineStatus and PUT /pipelineStatus, neither of which calls /trigger.

Why this shape

  • Trigger remains discrete — consistent with upstream AI #200 - Add TRIGGER permission to application bots #25113. EditAll is the "broad edit capability" operator and Trigger is an "action" operation (alongside Deploy, Kill). Conflating them would weaken the operation model and was rejected in favor of explicit seeded-policy grants.
  • DataStewardPolicy gets Trigger directly rather than relying on the ownership-edit escalation. The capability is functionally already there for stewards; making it explicit removes the indirection and gives a cleaner audit trail (one Trigger event vs. an EditOwners PATCH followed by a Trigger).
  • Seeded policies are modified via data, not via operator semantics so customer-built EditAll policies retain customer-controlled semantics. Customers who want broader trigger access add Trigger to their own roles; customers who want stricter trigger gates (like Orsted) get exactly what their isOwner() policy already expressed.

Test plan

  • IngestionPipelineOwnerInheritanceIT#testInheritedOwners_fromService passes.
  • IngestionPipelineOwnerInheritanceIT#testIsOwnerPolicy_appliesToEditAndTrigger passes.
  • Full mvn -pl openmetadata-integration-tests -am clean install -DskipTests builds cleanly.
  • mvn spotless:apply clean.
  • Local upgrade smoke test: brought up a fresh 1.12.9 stack, confirmed Trigger present in IngestionBotPolicy / LineageBotPolicy / ProfilerBotPolicy / QualityBotPolicy / UsageBotPolicy / DataStewardPolicy post-migration via GET /api/v1/policies/name/{name}?fields=rules.
  • Six API scenarios validated end-to-end against a deployed stack with metadata-ingested data (TestSuite pipeline, Metadata pipeline, Profiler pipeline, default-Owner-Rule, IngestionBot regression check, admin sanity).
  • Playwright UI specs cover owner-can / non-owner-denied / inherited-owner across the agents tab.
  • Reindex ingestion_pipeline_search_index after deploy so the UI owner filter sees inherited owners.

@github-actions github-actions Bot added backend safe to test Add this label to run secure Github workflows on PRs labels May 14, 2026
Comment thread ingestion/src/metadata/ingestion/source/dashboard/looker/utils.py Fixed
Comment thread ingestion/src/metadata/ingestion/source/dashboard/looker/utils.py Fixed
@manerow manerow force-pushed the fix/27962-ingestion-pipeline-owner-inheritance branch from a76c6e2 to a3d7299 Compare May 14, 2026 10:13
@manerow manerow force-pushed the fix/27962-ingestion-pipeline-owner-inheritance branch from a3d7299 to 057d7d0 Compare May 14, 2026 12:22
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 14, 2026

🔴 Playwright Results — 26 failure(s), 32 flaky

✅ 3915 passed · ❌ 26 failed · 🟡 32 flaky · ⏭️ 199 skipped

Shard Passed Failed Flaky Skipped
🔴 Shard 1 284 6 4 9
🔴 Shard 2 745 2 7 22
🔴 Shard 3 748 7 6 27
🟡 Shard 4 787 0 3 18
🔴 Shard 5 632 6 5 107
🔴 Shard 6 719 5 7 16

Genuine Failures (failed on all attempts)

Features/MultipleRename.spec.ts › GlossaryTerm - should handle multiple consecutive renames (shard 1)
�[31m"beforeAll" hook timeout of 60000ms exceeded.�[39m
Features/NavigationBlocker.spec.ts › should navigate to new page when "Leave" is clicked (shard 1)
�[31m"beforeAll" hook timeout of 60000ms exceeded.�[39m
Features/NavigationBlocker.spec.ts › should stay on current page and keep changes when X button is clicked (shard 1)
�[31m"beforeAll" hook timeout of 60000ms exceeded.�[39m
Features/OnlineUsers.spec.ts › Should show online users under Settings > Members > Online Users for admins (shard 1)
�[31m"beforeAll" hook timeout of 60000ms exceeded.�[39m
Features/OnlineUsers.spec.ts › Should filter users by time window (shard 1)
�[31m"beforeAll" hook timeout of 60000ms exceeded.�[39m
Features/Pagination.spec.ts › should test pagination on Table version page columns (shard 1)
�[31mTest timeout of 60000ms exceeded.�[39m
Features/AdvancedSearch.spec.ts › Description Contains filter returns matching tables (shard 2)
�[31m"beforeAll" hook timeout of 60000ms exceeded.�[39m
Features/Announcements/AnnouncementEntity.spec.ts › creates an announcement on a domain (shard 2)
TimeoutError: page.reload: Timeout 60000ms exceeded.
Call log:
�[2m  - waiting for navigation until "load"�[22m
�[2m    - navigated to "http://localhost:8585/domain/%22PW%25domain.bba00716%22"�[22m

Features/Permissions/EntityPermissions.spec.ts › Pipeline allow common operations permissions (shard 3)
�[31m"beforeAll" hook timeout of 60000ms exceeded.�[39m
Features/Permissions/EntityPermissions.spec.ts › Pipeline allow entity-specific permission operations (shard 3)
�[31m"beforeAll" hook timeout of 60000ms exceeded.�[39m
Features/Permissions/EntityPermissions.spec.ts › Container deny common operations permissions (shard 3)
�[31m"beforeAll" hook timeout of 60000ms exceeded.�[39m
Features/Permissions/EntityPermissions.spec.ts › SearchIndex allow common operations permissions (shard 3)
�[31m"beforeAll" hook timeout of 60000ms exceeded.�[39m
Features/Permissions/EntityPermissions.spec.ts › File allow common operations permissions (shard 3)
�[31m"beforeAll" hook timeout of 60000ms exceeded.�[39m
Features/Permissions/EntityPermissions.spec.ts › File deny common operations permissions (shard 3)
�[31m"beforeAll" hook timeout of 60000ms exceeded.�[39m
Features/Permissions/GlossaryPermissions.spec.ts › Glossary allow operations (shard 3)
�[31mTest timeout of 60000ms exceeded while setting up "page".�[39m
Pages/Entity.spec.ts › Delete Metric (shard 5)
�[31m"beforeAll" hook timeout of 60000ms exceeded.�[39m
Pages/Entity.spec.ts › Delete Spreadsheet (shard 5)
�[31m"beforeAll" hook timeout of 60000ms exceeded.�[39m
Pages/Entity.spec.ts › User as Owner Add, Update and Remove (shard 5)
�[31m"beforeAll" hook timeout of 60000ms exceeded.�[39m
Pages/Entity.spec.ts › Team as Owner Add, Update and Remove (shard 5)
�[31m"beforeAll" hook timeout of 60000ms exceeded.�[39m
Pages/Entity.spec.ts › User should be denied access to edit description when deny policy rule is applied on an entity (shard 5)
�[31m"beforeAll" hook timeout of 60000ms exceeded.�[39m
Pages/Entity.spec.ts › User should be denied access to edit description when deny policy rule is applied on an entity (shard 5)
�[31m"beforeAll" hook timeout of 60000ms exceeded.�[39m
Pages/Glossary.spec.ts › Change glossary term hierarchy using menu options (shard 6)
�[31mTest timeout of 60000ms exceeded.�[39m
Pages/Glossary.spec.ts › Change glossary term hierarchy using menu options across glossary (shard 6)
�[31m"beforeAll" hook timeout of 60000ms exceeded.�[39m
Pages/Glossary.spec.ts › Create term with synonyms during creation (shard 6)
�[31m"beforeAll" hook timeout of 60000ms exceeded.�[39m
Pages/Glossary.spec.ts › Create term with references during creation (shard 6)
�[31m"beforeAll" hook timeout of 60000ms exceeded.�[39m
Pages/GlossaryTermRightPanel.spec.ts › Should display entity name link in panel header in glossary term assets context (shard 6)
TimeoutError: page.goto: Timeout 60000ms exceeded.
Call log:
�[2m  - navigating to "http://localhost:8585/", waiting until "load"�[22m

🟡 32 flaky test(s) (passed on retry)
  • Features/OnlineUsers.spec.ts › Should show user displayName in online users table (shard 1, 2 retries)
  • Features/Pagination.spec.ts › should test pagination on Users page (shard 1, 2 retries)
  • Features/Pagination.spec.ts › should test pagination on Observability Alerts page (shard 1, 1 retry)
  • Pages/AuditLogs.spec.ts › should apply both User and EntityType filters simultaneously (shard 1, 1 retry)
  • Features/ActivityAPI.spec.ts › creates an activity event when tags are added (shard 2, 1 retry)
  • Features/AdvancedSearch.spec.ts › All entity status options are visible in the Status dropdown (shard 2, 1 retry)
  • Features/BulkEditEntity.spec.ts › Database Schema (shard 2, 1 retry)
  • Features/ColumnBulkOperations.spec.ts › should keep latest search results when responses arrive out of order (shard 2, 1 retry)
  • Features/Glossary/GlossaryWorkflow.spec.ts › should start term as Draft when glossary has reviewers (shard 2, 2 retries)
  • Features/KnowledgeCenterList.spec.ts › Knowledge Center List - Test infinite scroll/pagination (shard 2, 1 retry)
  • Features/KnowledgeCenterTextEditor.spec.ts › Rich Text Editor - Text Formatting (shard 2, 1 retry)
  • Features/Permissions/GlossaryPermissions.spec.ts › EditOwners only permission (shard 3, 1 retry)
  • Features/Permissions/GlossaryPermissions.spec.ts › Create only permission (shard 3, 2 retries)
  • Features/RestoreEntityInheritedFields.spec.ts › Validate restore with Inherited domain and data products assigned (shard 3, 1 retry)
  • Features/RTL.spec.ts › Verify Following widget functionality (shard 3, 1 retry)
  • Features/Table.spec.ts › Table pagination with sorting should works (shard 3, 1 retry)
  • Flow/PersonaFlow.spec.ts › Set default persona for team should work properly (shard 3, 1 retry)
  • Pages/CustomProperties.spec.ts › Should search custom properties for dashboard in right panel (shard 4, 1 retry)
  • Pages/CustomProperties.spec.ts › Should clear search and show all properties for database in right panel (shard 4, 1 retry)
  • Pages/CustomProperties.spec.ts › Should display custom properties for databaseSchema in right panel (shard 4, 1 retry)
  • Pages/Entity.spec.ts › Announcement create, edit & delete (shard 5, 1 retry)
  • Pages/EntityDataConsumer.spec.ts › User as Owner Add, Update and Remove (shard 5, 1 retry)
  • Pages/EntityDataConsumer.spec.ts › Tier Add, Update and Remove (shard 5, 1 retry)
  • Pages/EntityDataConsumer.spec.ts › User as Owner Add, Update and Remove (shard 5, 1 retry)
  • Pages/ExplorePageRightPanel_KnowledgeCenter.spec.ts › Should remove user owner for knowledgeCenter (shard 5, 2 retries)
  • Pages/GlossaryTermRightPanel.spec.ts › Should display overview tab content in glossary term assets context (shard 6, 2 retries)
  • Pages/GlossaryTermRightPanel.spec.ts › Should edit domain from glossary term assets context (shard 6, 1 retry)
  • Pages/InputOutputPorts.spec.ts › Input port button visible, output port button hidden when no assets (shard 6, 2 retries)
  • Pages/Lineage/LineageFilters.spec.ts › Verify lineage schema filter selection (shard 6, 1 retry)
  • Pages/Lineage/LineageRightPanel.spec.ts › Verify custom properties tab IS visible for supported type: searchIndex (shard 6, 1 retry)
  • ... and 2 more

📦 Download artifacts

How to debug locally
# Download playwright-test-results-<shard> artifact and unzip
npx playwright show-trace path/to/trace.zip    # view trace

@manerow manerow added the To release Will cherry-pick this PR into the release branch label May 15, 2026
@manerow manerow force-pushed the fix/27962-ingestion-pipeline-owner-inheritance branch from 5372307 to c75d5f8 Compare May 15, 2026 09:01
@gitar-bot
Copy link
Copy Markdown

gitar-bot Bot commented May 15, 2026

Code Review ✅ Approved 5 resolved / 5 findings

Inherits ingestion pipeline owners from parent services and mandates MetadataOperation.TRIGGER for the trigger endpoint. All previously identified issues, including integration test deprecations and migration safety concerns, have been resolved.

✅ 5 resolved
Performance: batchFetchServices filter misses non-service parents

📄 openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/IngestionPipelineRepository.java:248
In batchFetchServices, the filter fromEntity.endsWith("_service") will not match testSuite or app entity types that can also be the parent (container) of an IngestionPipeline. This causes test-suite and application pipelines to always fall through to the individual getContainer() fallback on line 200, defeating the purpose of the batch optimization for those pipeline types.

The impact is minor since the fallback is correct and these pipeline types are typically fewer in number, but it's worth noting for completeness of the batch optimization.

Quality: Integration test uses legacy Joda-Time instead of java.time

📄 openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/IngestionPipelineOwnerInheritanceIT.java:12 📄 openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/IngestionPipelineOwnerInheritanceIT.java:57
The new integration test imports org.joda.time.DateTime (line 12) to construct a Date object. Per project standards (Java 21 features), this should use java.time APIs instead. Joda-Time is a legacy library superseded by java.time since Java 8.

Quality: PR description contradicts code: LineageBot DOES get Trigger

📄 openmetadata-service/src/main/java/org/openmetadata/service/migration/utils/v1129/MigrationUtil.java:30-37 📄 openmetadata-service/src/main/resources/json/data/policy/LineageBotPolicy.json:21
The PR summary states "Other system bots (LineageBot, ProfilerBot, UsageBotBot, ...) do not have Trigger" but the code explicitly adds Trigger to LineageBotPolicy.json, ProfilerBotPolicy.json, QualityBotPolicy.json, and UsageBotPolicy.json — both in the seed files and in the migration utility (MigrationUtil.addTriggerOperationToDefaultPolicies). This documentation mismatch could confuse reviewers and future maintainers about the intended security posture. Please update the PR description's "Behavior change to call out" section to accurately reflect that all bot policies receive Trigger.

Quality: Generic catch swallows migration failures silently

📄 openmetadata-service/src/main/java/org/openmetadata/service/migration/utils/v1129/MigrationUtil.java:90-92
Line 90 catches Exception broadly and only logs the error. If the DAO update fails (e.g., serialization issue, constraint violation), the migration will report success while the DataStewardPolicy remains un-patched. The bot-policy counterpart (addTriggerOperationToDefaultBotPolicies via the v160 helper) propagates exceptions, causing the migration to fail visibly.

For a data migration, failing loudly is preferable so operators notice and retry, rather than silently leaving an incomplete policy state that's hard to diagnose later.

Quality: DataStewardPolicy.json missing trailing newline

📄 openmetadata-service/src/main/resources/json/data/policy/DataStewardPolicy.json:23
The file ends without a trailing newline (\ No newline at end of file in the diff). While cosmetic, this causes noisy diffs when a future change appends content and most linters/editors flag it.

Options

Display: compact → Showing less information.

Comment with these commands to change:

Compact
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar

@sonarqubecloud
Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend safe to test Add this label to run secure Github workflows on PRs To release Will cherry-pick this PR into the release branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Ownership-based access control is not working for Test Suite Ingestion Pipelines

4 participants