Skip to content

Conversation

@emsearcy
Copy link
Contributor

@emsearcy emsearcy commented Nov 4, 2025

These match the v2 relations unless otherwise noted.

Also includes same fix as #66 which I noticed independently while working on this.

🤖 Assisted with GitHub Copilot (via vim)

These match the v2 relations unless otherwise noted.

Also includes same fix as linuxfoundation#66 which I noticed independently while
working on this.

🤖 Assisted with [GitHub Copilot](https://github.com/features/copilot) (via vim)

Signed-off-by: Eric Searcy <[email protected]>
@emsearcy emsearcy requested a review from a team as a code owner November 4, 2025 21:19
Copilot AI review requested due to automatic review settings November 4, 2025 21:19
@coderabbitai
Copy link

coderabbitai bot commented Nov 4, 2025

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

Walkthrough

Helm chart version bumped (0.3.7 → 0.3.8). OpenFGA authorization model version advanced to 6.0.0 and adds five new v1_* read-only/indexer types; viewer semantics for past_meeting_recording, past_meeting_transcript, and past_meeting_summary broadened from [user:*] to [user, user:*].

Changes

Cohort / File(s) Summary
Helm Chart Metadata
charts/lfx-platform/Chart.yaml
Chart version bumped from 0.3.7 to 0.3.8.
OpenFGA Authorization Model
charts/lfx-platform/templates/openfga/model.yaml
Model version upgraded to 6.0.0; added types v1_meeting, v1_past_meeting, v1_past_meeting_recording, v1_past_meeting_transcript, v1_past_meeting_summary with read-only/indexer relation patterns; changed viewer for past_meeting_recording, past_meeting_transcript, and past_meeting_summary from [user:*] to [user, user:*].

Sequence Diagram(s)

sequenceDiagram
    autonumber
    participant User as Client (User)
    participant API as App/API
    participant OFGA as OpenFGA Model
    rect rgba(200,230,255,0.4)
      Note right of OFGA: Model v6.0.0 includes v1_* types\nand expanded viewer semantics
    end
    User->>API: Request access to past_meeting_recording
    API->>OFGA: Check relation `viewer` on past_meeting_recording
    alt user matches direct `user` or `user:*`
        OFGA-->>API: allow
    else no match
        OFGA-->>API: deny
    end
    API-->>User: grant/deny access
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Pay attention to correctness of the new v1_* relation definitions and cross-references.
  • Confirm the intended semantics of expanded viewer (from [user:*][user, user:*]) across policy checks and any existing derived rules.
  • Verify model version bump aligns with schema expectations in deployment tooling.

Possibly related PRs

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: adding v1 entity relations to the OpenFGA model configuration.
Description check ✅ Passed The description is related to the changeset, explaining that v1 relations match v2 relations and mentioning the fix from PR #66.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 1419f98 and 98fa733.

📒 Files selected for processing (2)
  • charts/lfx-platform/Chart.yaml (1 hunks)
  • charts/lfx-platform/templates/openfga/model.yaml (3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • charts/lfx-platform/Chart.yaml
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: mauriciozanettisalomao
Repo: linuxfoundation/lfx-v2-helm PR: 49
File: charts/lfx-platform/Chart.yaml:8-8
Timestamp: 2025-08-29T16:53:12.710Z
Learning: The maintainer mauriciozanettisalomao prefers to keep chart version increments conservative, choosing 0.2.7 over suggested 0.3.0 even for changes that could be considered behavior-altering (like new middleware and JSON Content-Type enforcement in Heimdall).
Learnt from: andrest50
Repo: linuxfoundation/lfx-v2-helm PR: 30
File: charts/lfx-platform/templates/openfga/model.yaml:66-69
Timestamp: 2025-08-08T21:57:43.678Z
Learning: In the lfx-platform OpenFGA authorization model for the meeting type, the viewer relation is intentionally defined as `[user:*] or participant or organizer or auditor from project`, which allows any user to view meetings by default. This is the correct and intended behavior for this system.
Learnt from: prabodhcs
Repo: linuxfoundation/lfx-v2-helm PR: 52
File: charts/lfx-platform/templates/openfga/model.yaml:73-80
Timestamp: 2025-09-09T15:09:56.135Z
Learning: In the OpenFGA authorization model for the lfx-platform Helm chart, the type for GroupsIO members is named "groupsio_member" rather than "groupsio_mailing_list_member", as preferred by the maintainers for brevity.
📚 Learning: 2025-08-08T21:57:43.678Z
Learnt from: andrest50
Repo: linuxfoundation/lfx-v2-helm PR: 30
File: charts/lfx-platform/templates/openfga/model.yaml:66-69
Timestamp: 2025-08-08T21:57:43.678Z
Learning: In the lfx-platform OpenFGA authorization model for the meeting type, the viewer relation is intentionally defined as `[user:*] or participant or organizer or auditor from project`, which allows any user to view meetings by default. This is the correct and intended behavior for this system.

Applied to files:

  • charts/lfx-platform/templates/openfga/model.yaml
📚 Learning: 2025-08-29T16:53:12.710Z
Learnt from: mauriciozanettisalomao
Repo: linuxfoundation/lfx-v2-helm PR: 49
File: charts/lfx-platform/Chart.yaml:8-8
Timestamp: 2025-08-29T16:53:12.710Z
Learning: The maintainer mauriciozanettisalomao prefers to keep chart version increments conservative, choosing 0.2.7 over suggested 0.3.0 even for changes that could be considered behavior-altering (like new middleware and JSON Content-Type enforcement in Heimdall).

Applied to files:

  • charts/lfx-platform/templates/openfga/model.yaml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: MegaLinter
🔇 Additional comments (1)
charts/lfx-platform/templates/openfga/model.yaml (1)

164-164: ✓ Viewer changes correctly broaden access semantics across artifact types.

The modification from [user:*] to [user, user:*] intentionally allows both explicit user tuple assignments and wildcard access. This aligns with the artifact_visibility settings: explicit tuples enable fine-grained per-user grants, while the wildcard preserves the "public" visibility mode. The change is consistent across past_meeting_recording, past_meeting_transcript, and past_meeting_summary.

Also applies to: 187-187, 210-210


Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
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

This PR updates the OpenFGA authorization model to version 6.0.1 and increments the Helm chart version to 0.3.6. The changes enhance the authorization model by adding support for individual user permissions alongside wildcard permissions and introducing new v1 meeting types for backward compatibility.

  • Updated authorization model version from 5.3.2 to 6.0.1
  • Added [user] alongside [user:*] in viewer relations for meeting artifacts to support both individual user grants and wildcard permissions
  • Introduced five new v1 meeting types (v1_meeting, v1_past_meeting, v1_past_meeting_recording, v1_past_meeting_transcript, v1_past_meeting_summary) for LFX v1 data synchronization

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
charts/lfx-platform/templates/openfga/model.yaml Updated authorization model to add individual user permission support and new v1 meeting types for backward compatibility
charts/lfx-platform/Chart.yaml Bumped chart version from 0.3.5 to 0.3.6

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between a056f68 and 2742d82.

📒 Files selected for processing (2)
  • charts/lfx-platform/Chart.yaml (1 hunks)
  • charts/lfx-platform/templates/openfga/model.yaml (4 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: mauriciozanettisalomao
Repo: linuxfoundation/lfx-v2-helm PR: 49
File: charts/lfx-platform/Chart.yaml:8-8
Timestamp: 2025-08-29T16:53:12.710Z
Learning: The maintainer mauriciozanettisalomao prefers to keep chart version increments conservative, choosing 0.2.7 over suggested 0.3.0 even for changes that could be considered behavior-altering (like new middleware and JSON Content-Type enforcement in Heimdall).
Learnt from: prabodhcs
Repo: linuxfoundation/lfx-v2-helm PR: 52
File: charts/lfx-platform/templates/openfga/model.yaml:73-80
Timestamp: 2025-09-09T15:09:56.135Z
Learning: In the OpenFGA authorization model for the lfx-platform Helm chart, the type for GroupsIO members is named "groupsio_member" rather than "groupsio_mailing_list_member", as preferred by the maintainers for brevity.
Learnt from: andrest50
Repo: linuxfoundation/lfx-v2-helm PR: 30
File: charts/lfx-platform/templates/openfga/model.yaml:66-69
Timestamp: 2025-08-08T21:57:43.678Z
Learning: In the lfx-platform OpenFGA authorization model for the meeting type, the viewer relation is intentionally defined as `[user:*] or participant or organizer or auditor from project`, which allows any user to view meetings by default. This is the correct and intended behavior for this system.
📚 Learning: 2025-08-29T16:53:12.710Z
Learnt from: mauriciozanettisalomao
Repo: linuxfoundation/lfx-v2-helm PR: 49
File: charts/lfx-platform/Chart.yaml:8-8
Timestamp: 2025-08-29T16:53:12.710Z
Learning: The maintainer mauriciozanettisalomao prefers to keep chart version increments conservative, choosing 0.2.7 over suggested 0.3.0 even for changes that could be considered behavior-altering (like new middleware and JSON Content-Type enforcement in Heimdall).

Applied to files:

  • charts/lfx-platform/Chart.yaml
📚 Learning: 2025-08-08T21:57:43.678Z
Learnt from: andrest50
Repo: linuxfoundation/lfx-v2-helm PR: 30
File: charts/lfx-platform/templates/openfga/model.yaml:66-69
Timestamp: 2025-08-08T21:57:43.678Z
Learning: In the lfx-platform OpenFGA authorization model for the meeting type, the viewer relation is intentionally defined as `[user:*] or participant or organizer or auditor from project`, which allows any user to view meetings by default. This is the correct and intended behavior for this system.

Applied to files:

  • charts/lfx-platform/templates/openfga/model.yaml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: MegaLinter
🔇 Additional comments (7)
charts/lfx-platform/Chart.yaml (1)

8-8: Version bump is appropriately conservative. Given the substantial OpenFGA model changes (major version bump to 6.0.1), keeping the Helm chart version at a patch increment (0.3.6) aligns well with your maintainers' preference for modest, deliberate version increments.

charts/lfx-platform/templates/openfga/model.yaml (6)

21-24: Version bump correctly follows stated guidelines. The major bump (5→6) is justified by the addition of five new v1_* types, and the patch component (6.0.1) accounts for the define modifications to past_meeting_recording/transcript/summary viewer relations.


146-146: Verify viewer definition changes match PR #66 intent. The additions of explicit [user] to the viewer definitions (now [user, user:*] instead of [user:*] only) represent a behavioral change to artifact access control. You note this aligns with PR #66; please confirm this is the intended fix and that all three artifact types should receive this treatment consistently.

Also applies to: 169-169, 192-192


204-213: v1_meeting structure is sound. The omission of explicit [user] for the organizer relation aligns with the read-only nature of v1 data, allowing organizers only through project/committee derivations. The comment explains this design choice clearly.


231-238: v1_past_meeting_recording structure is correct. References to v1_past_meeting are proper, and the viewer definition with [user, user:*] is consistent with the updates applied to the original past_meeting_recording.


242-249: v1_past_meeting_transcript structure is correct. References to v1_past_meeting are proper, and the viewer definition with [user, user:*] is consistent with the updates applied to the original past_meeting_transcript.


251-258: v1_past_meeting_summary structure is correct. References to v1_past_meeting are proper, and the viewer definition with [user, user:*] is consistent with the updates applied to the original past_meeting_summary.

Caught by both CoPilot and CodeRabbit in PR.

Signed-off-by: Eric Searcy <[email protected]>
bramwelt
bramwelt previously approved these changes Nov 7, 2025
@andrest50 andrest50 merged commit 9dc9fd7 into linuxfoundation:main Nov 14, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants