SargamPuram commented on issue #17167:
URL:
https://github.com/apache/dolphinscheduler/issues/17167#issuecomment-2910513632
Hi @lancemao @ruanwenjun ,
I'm interested in contributing to this feature as part of my OSPP 2025
preparation. I've analyzed the current worker group implementation and
understand the need for tenant-based isolation. I've read the contributing
guidelines and plan to use the branch name feature-17167.
Before I begin working and raising a PR, I wanted to ask: Should I wait to
be officially assigned this issue, or can I proceed with the implementation?
## Current Architecture Analysis
I've reviewed the existing codebase and identified the key components:
**Database Layer:**
- `t_ds_worker_group` table currently has no tenant association
- `t_ds_tenant` table exists with proper structure (`id`, `tenant_code`,
`queue_id`)
- Both have standard audit fields (`create_time`, `update_time`)
**API Layer:**
- `WorkerGroupController` provides CRUD operations without tenant filtering
- `WorkerGroupService` methods like `queryAllGroup()` and
`queryAllGroupPaging()` return all worker groups regardless of user's tenant
- `WorkerGroupMapper` queries don't include tenant-based filtering
## Proposed Technical Implementation
### 1. Database Schema Changes
```sql
-- Add tenant_id column to t_ds_worker_group
ALTER TABLE t_ds_worker_group
ADD COLUMN tenant_id INT(11) DEFAULT NULL COMMENT 'tenant id',
ADD INDEX idx_tenant_id (tenant_id),
ADD CONSTRAINT fk_worker_group_tenant
FOREIGN KEY (tenant_id) REFERENCES t_ds_tenant(id);
```
### 2. Entity Model Updates
```java
// Add to WorkerGroup.java
private Integer tenantId;
@TableField(exist = false)
private String tenantCode; // For display purposes
```
### 3. Service Layer Changes
- Modify `queryAllGroup()` and `queryAllGroupPaging()` to filter by user's
tenant
- Update `saveWorkerGroup()` to associate worker group with user's tenant
- Add validation to prevent cross-tenant worker group access
### 4. Mapper/Query Updates
- Add tenant-based filtering to `queryAllWorkerGroup()`
- Create new method `queryWorkerGroupByTenant(Integer tenantId)`
- Update existing queries to respect tenant boundaries
### 5. Frontend Considerations
- Worker group selection dropdowns should only show tenant-specific groups
- Admin interface for worker group management should include tenant
association
- Validation to prevent selecting worker groups from different tenants
## Questions & Considerations
1. **Backward Compatibility:** How should we handle existing worker groups
without tenant association?
- Option A: Assign them to a default tenant
- Option B: Allow them to be visible to all tenants until manually
assigned
2. **Admin vs User Access:** Should admin users see all worker groups across
tenants, or should tenant isolation be absolute?
3. **Migration Strategy:** Should we provide a migration script to associate
existing worker groups with tenants based on usage patterns?
## Implementation Approach
I'd like to start with:
1. Database schema changes and entity updates
2. Service layer modifications with proper tenant filtering
3. Unit tests for tenant isolation logic
4. Integration tests to ensure no cross-tenant data leakage
This would align with DolphinScheduler's existing patterns while providing
the tenant isolation needed for multi-tenant deployments.
I'm also working on the gRPC task plugin proposal for OSPP and this would
help me better understand the DolphinScheduler architecture.
**Technical Background:** Java/Spring Boot, database design, Vue.js frontend
development
**GitHub:** https://github.com/SargamPuram
Best regards,
Sargam
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
