Skip to content

Use timestamp expiry for data access requests#30099

Merged
anuj-kumary merged 13 commits into
mainfrom
policy-agent-dar-expiration-timestamp
Jul 20, 2026
Merged

Use timestamp expiry for data access requests#30099
anuj-kumary merged 13 commits into
mainfrom
policy-agent-dar-expiration-timestamp

Conversation

@yan-3005

@yan-3005 yan-3005 commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Summary

  • require future epoch-ms expirationDate for Data Access Request payloads while keeping duration as a legacy fallback
  • update the seeded DataAccessRequest form schema to require expirationDate
  • update task validation/default schema tests for timestamp expiry

Companion Collate PR: https://github.com/open-metadata/openmetadata-collate/pull/5075

Test Plan

  • mvn -pl openmetadata-service -Dtest=TaskFieldValidatorTest,TaskWorkflowLifecycleResolverTest test
  • git -C OpenMetadata diff --check

Summary by Gitar

  • Workflow governance:
    • Added dateVariable support to expiryTimer in UserApprovalTask, allowing absolute timestamp-based boundary timers.
    • Updated userApprovalTask.json schema to enforce exactly one of durationVariable or dateVariable.
  • Testing:
    • Added UserApprovalTaskTest to verify that expiryTimer correctly maps dateVariable to the workflow process model.

This will update automatically on new commits.

Greptile Summary

This PR moves Data Access Request expiry to an absolute timestamp. The main changes are:

  • Require a future epoch-millisecond expirationDate in request payloads and form schemas.
  • Add absolute-date boundary timer support to user approval workflows.
  • Update generated client types and task validation tests.

Confidence Score: 5/5

No additional blocking issue was found in the latest changes.

  • The remaining expiration validation, timer binding, compatibility, and schema migration concerns are already covered by existing findings.
  • No separate production failure requiring another code change was identified.

Important Files Changed

Filename Overview
openmetadata-service/src/main/java/org/openmetadata/service/tasks/TaskFieldValidator.java Replaces duration validation with a future timestamp check for new Data Access Requests.
openmetadata-service/src/main/java/org/openmetadata/service/governance/workflows/elements/nodes/userTask/UserApprovalTask.java Adds mutually exclusive absolute-date and duration timer expressions.
openmetadata-service/src/main/resources/json/data/taskFormSchemas/DataAccessRequest.json Requires a numeric expiration date and uses the date-time picker.
openmetadata-service/src/main/java/org/openmetadata/service/tasks/TaskWorkflowLifecycleResolver.java Updates the fallback Data Access Request schema to use a numeric expiration date.
openmetadata-spec/src/main/resources/json/schema/governance/workflows/elements/nodes/userTask/userApprovalTask.json Extends timer configuration with an absolute-date variable option.

Reviews (13): Last reviewed commit: "Merge branch 'main' into policy-agent-da..." | Re-trigger Greptile

Context used (3)

  • Context used - CLAUDE.md (source)
  • Context used - openmetadata-ui-core-components/CLAUDE.md (source)
  • Context used - AGENTS.md (source)

Copilot AI review requested due to automatic review settings July 15, 2026 16:55
@yan-3005 yan-3005 added the safe to test Add this label to run secure Github workflows on PRs label Jul 15, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@github-actions

Copy link
Copy Markdown
Contributor

❌ PR checklist incomplete

This PR cannot be merged until the following are addressed on its linked issue:

  • No GitHub issue is linked. Link an issue in the Development section of the PR (or add Fixes #12345 to the description). For a same-org cross-repo issue, add Fixes open-metadata/<repo>#123 to the description.

The fields live on the linked issue in the Shipping project (open the issue → right sidebar → Projects). After you set them, re-run this check (or push a commit) — issue/project changes do not re-trigger it automatically.

Maintainers can bypass this check by adding the skip-pr-checks label.

@github-actions

github-actions Bot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

🟡 Playwright Results — all passed (38 flaky)

✅ 4533 passed · ❌ 0 failed · 🟡 38 flaky · ⏭️ 95 skipped

Shard Passed Failed Flaky Skipped
🟡 Shard 1 431 0 9 16
✅ Shard 2 11 0 0 0
🟡 Shard 3 823 0 9 8
🟡 Shard 4 820 0 3 18
🟡 Shard 5 837 0 4 5
🟡 Shard 6 783 0 5 46
🟡 Shard 7 828 0 8 2
🟡 38 flaky test(s) (passed on retry)
  • Features/CustomizeDetailPage.spec.ts › Stored Procedure - customization should work (shard 1, 1 retry)
  • Features/Glossary/GlossaryPagination.spec.ts › should check for nested glossary term search (shard 1, 1 retry)
  • Pages/Lineage/LineageRightPanel.spec.ts › Verify custom properties tab IS visible for supported type: metric (shard 1, 1 retry)
  • Pages/SearchSettings.spec.ts › Latest preview config wins when a superseded request resolves late (shard 1, 1 retry)
  • Flow/SearchRBAC.spec.ts › User without permission (shard 1, 1 retry)
  • Flow/SearchRBAC.spec.ts › User without permission (shard 1, 1 retry)
  • Flow/SearchRBAC.spec.ts › User without permission (shard 1, 1 retry)
  • Flow/SearchRBAC.spec.ts › a fully denied user sees neither asset type when browsing (shard 1, 2 retries)
  • Flow/SearchRBAC.spec.ts › the browse tree only shows the asset-type categories a user can access (shard 1, 2 retries)
  • Features/BulkEditEntity.spec.ts › Glossary Term (Nested) (shard 3, 1 retry)
  • Features/BulkImportWithDotInName.spec.ts › Import at database level with dot in service name (shard 3, 1 retry)
  • Features/ContextCenterArchive.spec.ts › archive page lazy-loads more rows on scroll within its own scroll container (shard 3, 1 retry)
  • Features/ContextCenterArticles.spec.ts › Article listing search filters, clears, and shows empty state (shard 3, 1 retry)
  • Features/ContextCenterArticles.spec.ts › Article list cards, recently viewed widget, and pagination work (shard 3, 1 retry)
  • Features/ContextCenterArticles.spec.ts › Article edit persistence and unsaved title behavior are correct (shard 3, 1 retry)
  • Features/ContextCenterArticles.spec.ts › description: switching articles does not bleed unsaved content into next article (shard 3, 1 retry)
  • Features/ContextCenterMemories.spec.ts › cancel button in edit mode closes the modal without saving (shard 3, 1 retry)
  • Features/DataQuality/TestCaseImportExportBasic.spec.ts › should show validation errors for invalid CSV (shard 3, 1 retry)
  • Features/Glossary/GlossaryTermRelationsGraphNested.spec.ts › viewing a child term: parent appears as a 1-hop neighbour via parentOf edge (shard 4, 1 retry)
  • Features/Glossary/LargeGlossaryPerformance.spec.ts › should handle drag and drop for term reordering (shard 4, 1 retry)
  • Features/RestoreEntityInheritedFields.spec.ts › Validate restore with Inherited domain and data products assigned (shard 4, 1 retry)
  • Flow/ExploreDiscovery.spec.ts › Should not display soft deleted assets in search suggestions (shard 5, 1 retry)
  • Flow/PersonaFlow.spec.ts › Set default persona for team should work properly (shard 5, 1 retry)
  • Pages/CustomProperties.spec.ts › Email (shard 5, 1 retry)
  • Pages/CustomProperties.spec.ts › Integer (shard 5, 1 retry)
  • Pages/Entity.spec.ts › Tag Add, Update and Remove for child entities (shard 6, 1 retry)
  • Pages/Entity.spec.ts › Tier Add, Update and Remove (shard 6, 1 retry)
  • Pages/EntityDataSteward.spec.ts › DisplayName Add, Update and Remove for child entities (shard 6, 1 retry)
  • Pages/ExploreBrowse.spec.ts › service type drill-down disables unrelated roots and query-panel Clear resets it (shard 6, 1 retry)
  • Pages/ExplorePageRightPanel_KnowledgeCenter.spec.ts › Should remove user owner for knowledgeCenter (shard 6, 1 retry)
  • ... and 8 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

Copilot AI review requested due to automatic review settings July 16, 2026 06:50

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Copilot AI review requested due to automatic review settings July 16, 2026 07:46

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Fixes 11 CI failures in openmetadata-integration-tests:
- DataAccessRequestValidationIT: 10 tests failing with
  "Invalid task payload: required property 'expirationDate' not found"
- TaskResourceIT#testTaskCountsBucket: same rejection

Both were still building payloads with the removed "duration" field.
The DAR TaskFormSchema now requires "expirationDate" (epoch millis) —
switch the shared helper + the bucket-count test payload to send a
14-day future timestamp instead.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 17, 2026 10:56

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Copilot AI review requested due to automatic review settings July 17, 2026 12:24

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 12 out of 14 changed files in this pull request and generated 2 comments.

Comment on lines +137 to 141
Long expirationDate = payload.getExpirationDate();
if (expirationDate == null) {
throw new IllegalArgumentException(
"A Data Access Request requires an access duration (ISO 8601, e.g. 'P14D').");
"A Data Access Request requires an access expirationDate timestamp.");
}
Comment on lines +68 to 70
"expirationDate": {
"ui:widget": "dateTimePicker"
},

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review is ineligible. To be eligible to request a review, you need a paid Copilot license, or your organization must enable Copilot code review.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review is ineligible. To be eligible to request a review, you need a paid Copilot license, or your organization must enable Copilot code review.

@sonarqubecloud

Copy link
Copy Markdown

@sonarqubecloud

Copy link
Copy Markdown

@gitar-bot

gitar-bot Bot commented Jul 20, 2026

Copy link
Copy Markdown
Code Review ✅ Approved 4 resolved / 4 findings

Transitions data access requests to absolute timestamp-based expiry and updates the form schema to require a future expirationDate. All previously identified issues regarding schema validation, timer scheduling, and method naming have been addressed.

✅ 4 resolved
Bug: Seeded schema requires expirationDate, breaking legacy duration fallback

📄 openmetadata-service/src/main/resources/json/data/taskFormSchemas/DataAccessRequest.json:10 📄 openmetadata-service/src/main/java/org/openmetadata/service/tasks/TaskFieldValidator.java:131-145
TaskRepository.prepare runs validatePayloadAgainstFormSchema (line 356) before validateDataAccessRequestDuration (line 360), and the seeded DataAccessRequest form schema now lists expirationDate in required while dropping duration. The networknt validator strictly enforces required, so any legacy client sending only duration (no expirationDate) is rejected with a 400 at the schema step and never reaches the duration fallback branch. This directly contradicts the new docstring/PR claim that ISO-8601 duration "remains valid for existing clients"; the fallback is effectively dead code for API-created tasks. Either make the schema accept either field (e.g. via anyOf/oneOf on expirationDate/duration) or drop the legacy-duration claim and require expirationDate consistently.

Edge Case: DAR expiry timer may be unschedulable when only expirationDate is set

📄 openmetadata-service/src/main/java/org/openmetadata/service/tasks/TaskFieldValidator.java:139-147
The expiryTimer boundary event is built from ${durationVariable} (UserApprovalTask.java:355), an ISO-8601 duration expression, and resolveEffectiveExpirationDate only derives a value from payload.duration when no numeric expirationDate exists. Accepting a DAR with only expirationDate (and no duration) removes the exact input the docstring says the boundary timer needs, so the timer variable may resolve empty and leave the task's expiry unschedulable — the latent failure this validation was meant to prevent. Confirm the companion workflow change populates the timer from expirationDate (e.g. a timeDate timer) before merging; otherwise timestamp-only requests will not auto-expire.

Quality: Method name validateDataAccessRequestDuration no longer validates duration

📄 openmetadata-service/src/main/java/org/openmetadata/service/tasks/TaskFieldValidator.java:129-143
After this commit the method no longer inspects duration at all — it only validates the expirationDate timestamp, and the DurationUtil import was removed. The name validateDataAccessRequestDuration is now misleading and hurts self-documenting readability. Rename it (e.g. validateDataAccessRequestExpiry) and update the callers/test references accordingly.

Quality: Seed refresh failure can abort service startup

📄 openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TaskFormSchemaRepository.java:77-91 📄 openmetadata-service/src/main/java/org/openmetadata/service/resources/feeds/TaskFormSchemaResource.java:78
refreshDataAccessRequestSeedSchema() runs during TaskFormSchemaResource.initialize() and lets any IOException or RuntimeException from createOrUpdate() propagate, unlike initSeedDataFromResources() which swallows per-entity failures. A transient DB error while migrating the DataAccessRequest schema would then fail application startup entirely. Consider wrapping the refresh in a try/catch that logs and continues, matching the existing seed-init resilience.

Options

Display: compact → Showing less information.

Comment with these commands to change the behavior for this request:

Compact
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar

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

Labels

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.

5 participants