[I] [Improvement][API] Associate worker group with tenant [dolphinscheduler]

["lancemao (via GitHub)" <gi...@apache.org>]

lancemao opened a new issue, #17167:
URL: https://github.com/apache/dolphinscheduler/issues/17167

   ### Search before asking
   
   - [x] I had searched in the [issues](https://github.com/apache/dolphinscheduler/issues?q=is%3Aissue) and found no similar feature requirement.
   
   
   ### Description
   
   We want to isolate worker resources based on different tenants. Currently in dolphinscheduler, when we create worker group, it is not associated with tenant (no such field in t_ds_worker_group)
   
   Also for frontend, after we associated worker group with tenant, we need to ensure that user can only select worker group in his tenant when he tries to start executing workflows, or setting worker group in tasks 
   
   ### Are you willing to submit a PR?
   
   - [ ] Yes I am willing to submit a PR!
   
   ### Code of Conduct
   
   - [x] I agree to follow this project's [Code of Conduct](https://www.apache.org/foundation/policies/conduct)
   


-- 
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: commits-unsubscribe@dolphinscheduler.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [I] [Feature][API] Associate worker group with tenant [dolphinscheduler]

["SargamPuram (via GitHub)" <gi...@apache.org>]

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: commits-unsubscribe@dolphinscheduler.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [I] [Feature][API] Associate worker group with tenant [dolphinscheduler]

["ruanwenjun (via GitHub)" <gi...@apache.org>]

ruanwenjun commented on issue #17167:
URL: https://github.com/apache/dolphinscheduler/issues/17167#issuecomment-2872863905

   +1, Good suggestion, in most of the cases, there will only one tenant in one worker.


-- 
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: commits-unsubscribe@dolphinscheduler.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [I] [Feature][API] Associate worker group with tenant [dolphinscheduler]

["goutamadwant (via GitHub)" <gi...@apache.org>]

goutamadwant commented on issue #17167:
URL: https://github.com/apache/dolphinscheduler/issues/17167#issuecomment-5675822705

   @ruanwenjun @davidzollo I reproduced the current behavior on 3.4.1 with two tenants and two ordinary users. Both users can see both worker groups and tenants, and all four tenant/group combinations execute successfully. Two additional checks confirmed that an explicit task group takes precedence over the workflow launch group, and that selecting another execution tenant works independently of the user's default tenant.
   
   The fixture used one physical worker, so this establishes logical placement behavior rather than physical resource separation.
   
   For a first version, I propose:
   
   - Add an optional `tenant_id` to administrator-managed database worker groups, with fresh-install and upgrade SQL for MySQL, PostgreSQL, and H2. Existing unbound groups remain shared.
   - Validate compatibility against each task's resolved worker group and effective workflow execution tenant. Include API preflight and an authoritative master check before dispatch, covering task overrides, schedules, backfill, subworkflows, and retries/restarts. Reject mismatches explicitly without silently falling back to `default`.
   - Let administrators manage all groups, while applying the same compatibility checks to administrator-triggered executions.
   - Add tenant selection to worker-group management. Refresh compatible project-scoped groups when the execution tenant changes in workflow-start and schedule configuration.
   - In task authoring, display each group's tenant requirement. Task definitions do not determine the eventual execution tenant, so filtering solely by the editor's default tenant could exclude valid workflows.
   - Prevent tenant deletion while groups remain bound. Keep reserved `default` and configuration-derived groups shared in this first version, and document that limitation.
   
   Existing project authorization and tenant-selection behavior would be preserved.
   
   Before implementation, could you clarify:
   
   1. Should compatibility follow the selected execution tenant, or should ordinary users also be restricted to their default tenant? The latter changes current tenant-selection behavior.
   2. Is optional single-tenant binding with shared legacy/default groups acceptable?
   3. Should binding changes require a drained group, or may incompatible undispatched work fail at subsequent validation?
   4. Is displaying tenant requirements during task authoring acceptable until an execution tenant is known?
   
   Regression coverage would include matching and mismatched bindings, task overrides, retained retry placement, administrator execution, existing permission denials, legacy groups, tenant deletion, and supported database upgrades.
   
   Let me know your thoughts. thanks! 


-- 
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: commits-unsubscribe@dolphinscheduler.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org