-
Notifications
You must be signed in to change notification settings - Fork 4
added the groupsio mailing list model #46
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Signed-off-by: Prabodh Chaudhari <pchaudhari@linuxfoundation.org>
|
Note Other AI code review bot(s) detectedCodeRabbit 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. WalkthroughHelm chart version incremented from 0.2.4 to 0.2.5. OpenFGA AuthorizationModelRequest major version bumped 3 → 4 and a new type Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Service
participant OpenFGA
User->>Service: Request access to mailing-list resource
Service->>OpenFGA: Check relation on groupsio_mailing_list (e.g., viewer)
rect rgba(200,235,255,0.25)
note right of OpenFGA: Evaluate relations and inheritance
OpenFGA->>OpenFGA: Traverse owner/writer/auditor/viewer
OpenFGA->>OpenFGA: Inherit via groupsio_service
OpenFGA->>OpenFGA: Inherit via committee (member→viewer)
end
OpenFGA-->>Service: Allow / Deny
Service-->>User: Response
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this 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 pull request adds support for GroupsIO mailing list authorization by introducing a new resource type to the OpenFGA authorization model and updating the Helm chart version accordingly.
- Added
groupsio_mailing_listtype to the OpenFGA authorization model with comprehensive permission relationships - Updated OpenFGA model version from 3.0.0 to 4.0.0 to reflect the breaking change
- Incremented Helm chart version from 0.2.4 to 0.2.5
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| charts/lfx-platform/templates/openfga/model.yaml | Added groupsio_mailing_list type with relations and bumped model major version |
| charts/lfx-platform/Chart.yaml | Updated Helm chart version to reflect the authorization model changes |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
There was a problem hiding this 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
🧹 Nitpick comments (1)
charts/lfx-platform/templates/openfga/model.yaml (1)
63-63: Confirm intentional global view access for mailing lists.
viewer: viewer from groupsio_serviceinherits[user:*]fromgroupsio_service.viewer, making all mailing lists public by default. If private lists are needed (acceptance criteria mentions supporting public/private), consider a toggleable pattern:
- Introduce
define public: [user:*]on bothgroupsio_serviceandgroupsio_mailing_list.- Change
viewertopublic or auditor from project …and manage visibility by writing/removing theuser:*tuple topublic.Otherwise, confirm that global visibility is intended for mailing lists.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
charts/lfx-platform/Chart.yaml(1 hunks)charts/lfx-platform/templates/openfga/model.yaml(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-08T21:57:43.678Z
Learnt from: andrest50
PR: linuxfoundation/lfx-v2-helm#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
🔇 Additional comments (2)
charts/lfx-platform/Chart.yaml (1)
8-8: Version bump looks good.Minor chart bump to 0.2.5 is appropriate given model changes shipped via templates.
Please update release notes/CHANGELOG to mention the OpenFGA model major bump and the new groupsio_mailing_list type.
charts/lfx-platform/templates/openfga/model.yaml (1)
15-17: Major model version bump: verify client compatibility and rollout plan.Moving to major: 4 will generate a new model ID. Ensure all services use “latest” model resolution or are updated to the new ID, and that tuple writes/reads are backward-compatible during rollout.
| define project: project from groupsio_service # Inherit project permissions | ||
| define committee: [committee] # Inherit committee permissions | ||
| define owner: owner from groupsio_service or owner from committee | ||
| define writer: writer from groupsio_service or writer from committee |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IInherit from the committee to address this ticket https://linuxfoundation.atlassian.net/browse/LFXV2-249. Thanks @prabodhcs
read/write on mailing lists as long as they are connected to the committee. Can create new mailing lists, update existing, subscribe people, and delete lists that are connected to the committee.
Signed-off-by: Prabodh Chaudhari <pchaudhari@linuxfoundation.org>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
charts/lfx-platform/templates/openfga/model.yaml (1)
63-63: Re: viewer via committee members — keep as-is but document rationaleYou intentionally grant viewing to all committee members (not just committee “viewer”). That’s fine given the stated requirement, but add a rationale to prevent future “tightening” PRs from breaking expected access.
- define viewer: viewer from groupsio_service or member from committee + # Rationale: any committee member should be able to view connected mailing lists. + define viewer: viewer from groupsio_service or member from committee
🧹 Nitpick comments (4)
charts/lfx-platform/templates/openfga/model.yaml (4)
15-17: Major version bump (4) — ensure coordinated rollout and model adoptionBumping the AuthorizationModel major version to 4 will create a new model in the OpenFGA store. Verify that:
- all services that read/write tuples fetch and use the latest model ID,
- any bootstrap tuple writes targeting new relations/types are deployed after this model is live,
- CI/CD for dependent services is ordered to avoid 404/validation errors during transition.
59-59: Comment is misleading; relation does not itself “inherit permissions”The
committee: [committee]relation only links to a committee. Inheritance happens in writer/auditor/viewer lines. Please clarify the inline comment.- define committee: [committee] # Inherit committee permissions + define committee: [committee] # Link to committee (permissions derived below)
57-57: Cardinality on parent relations should be enforced in business logicOpenFGA won’t enforce single-parent; if a mailing list must belong to exactly one groupsio_service (and optionally one committee), ensure backend validations prevent multiple tuples for these relations.
63-63: Optional: allow public lists independent of parent serviceIf you want the mailing list’s public/private to be decoupled from the parent service, add user:* directly here. Otherwise, skip.
- define viewer: viewer from groupsio_service or member from committee + define viewer: [user:*] or viewer from groupsio_service or member from committee
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
charts/lfx-platform/templates/openfga/model.yaml(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-08T21:57:43.678Z
Learnt from: andrest50
PR: linuxfoundation/lfx-v2-helm#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 (2)
charts/lfx-platform/templates/openfga/model.yaml (2)
55-64: New groupsio_mailing_list type is structurally soundAll referenced relations exist on their source types; removal of committee-derived ownership avoids the invalid reference noted earlier. Model should validate as-is.
61-62: Confirm write semantics vs. requirement “committee can read/write”Current write path is “writer from committee”, and on the committee type “writer” is “writer from project”. If the intent was “any committee member can write mailing lists connected to the committee,” this does not implement that. If only committee writers should write, this is correct. Please confirm intended scope before merging.
Issue - https://linuxfoundation.atlassian.net/browse/LFXV2-351
This pull request introduces a version update to the Helm chart and expands the OpenFGA authorization model to support a new resource type. The most significant changes are the addition of the
groupsio_mailing_listtype and the update of the OpenFGA model version.OpenFGA authorization model enhancements:
groupsio_mailing_listtype with relations forgroupsio_service,project,committee,owner,writer,auditor, andviewerto extend permission management capabilities for mailing lists.Helm chart maintenance:
0.2.4to0.2.5inChart.yamlto reflect these changes.