feat(ci-dashboard): add Kubernetes workload cost allocation

[dillon-zheng]

Summary

• classify Kubernetes workload allocation across AWS and GCP, separating allocated and residual unallocated costs
• expose auditable Kubernetes allocation totals while keeping owner-attributed controller costs out of Kubernetes cards
• add resource-oriented unmatched allocation details and cross-vendor handling
• make cost-trend stacks include a bucket-aware Others series so each bucket reconciles to List cost
• remove the low-value Kubernetes unallocated detail table while retaining the summary card
• retain the two Kubernetes-unallocated endpoints as API-only audit surfaces; the UI table is intentionally absent

Allocation semantics

• matched control-plane costs stay in the standard owner cost view and are excluded from both K8S cards
• see ci-dashboard/docs/kubernetes-workload-allocation-design.md for precedence, active-roster matching, vendor mapping, and rollout fallback behavior

Validation

• PYTHONPATH=src /Users/dillon/.pyenv/versions/3.12.1/bin/python -m pytest
• /Users/dillon/.pyenv/versions/3.12.1/bin/python -m ruff check src tests
• npm --prefix web test -- --run
• npm --prefix web run build

[ti-chi-bot]

I have already done a preliminary review for you, and I hope to help you do a better job.

Summary

This PR adds Kubernetes workload cost allocation capabilities to the CI dashboard, enabling differentiation between allocated and unallocated costs across AWS and GCP. The approach involves introducing new SQL queries, API endpoints, and UI updates to support detailed cost analysis and reporting. The code is comprehensive but introduces complexity in SQL logic, error handling, and test coverage. Overall, the functionality is well-integrated but could benefit from improved modularity and error handling.

---

Critical Issues

1. Potential SQL Injection Risk

   • File: ci-dashboard/src/ci_dashboard/api/queries/cost.py
   • Lines: Multiple (e.g., SQL query construction in get_kubernetes_unallocated_costs and get_kubernetes_unallocated_records)
   • Issue: Queries use string interpolation to construct SQL, which could allow SQL injection if inputs are not sanitized properly.
   • Solution: Refactor to use parameterized queries exclusively with placeholders, e.g.:

     text("""
         SELECT * FROM table
         WHERE column = :param
     """)

     Ensure all inputs are validated and sanitized before constructing queries.

2. Hardcoded Limits

   • File: ci-dashboard/src/ci_dashboard/api/queries/cost.py
   • Lines: get_kubernetes_unallocated_records (e.g., limit parameter logic)
   • Issue: Hardcoded limit (KUBERNETES_UNALLOCATED_RECORD_LIMIT) may not adapt well to scaling needs. This could cause unexpected truncation in large datasets.
   • Solution: Allow dynamic configuration via environment variables or app settings for such thresholds, e.g.:

     limit = max(1, min(limit, os.getenv("UNALLOCATED_RECORD_LIMIT", 100)))

---

Code Improvements

1. Modularization of SQL Conditions

   • File: ci-dashboard/src/ci_dashboard/api/queries/cost.py
   • Lines: Functions like _kubernetes_parent_residual_condition, _kubernetes_service_cost_condition, etc.
   • Issue: Many SQL condition functions duplicate logic and could be combined into reusable helpers.
   • Solution: Create a single helper function to dynamically generate conditions, reducing duplication:

     def build_condition(table_alias, condition_type):
         conditions = {
             "parent_residual": f"{table_alias}.source_allocation_scope IN (...)",
             "service_cost": f"{table_alias}.service_name LIKE '%kubernetes%'"
         }
         return conditions.get(condition_type, "")

2. Error Handling in SQL Execution

   • File: ci-dashboard/src/ci_dashboard/api/queries/cost.py
   • Lines: SQL query execution in functions like get_kubernetes_unallocated_costs, get_kubernetes_unallocated_records
   • Issue: No error handling for database connection failures or query execution errors.
   • Solution: Wrap database operations in try-except blocks, logging meaningful error messages:

     try:
         rows = connection.execute(text(query), params)
     except Exception as e:
         logger.error(f"Database query failed: {e}")
         return {"error": "Unable to fetch data"}

---

Best Practices

1. Testing Coverage for Edge Cases

   • File: ci-dashboard/tests/api/test_routes.py
   • Lines: Tests like test_kubernetes_unallocated_records
   • Issue: Tests focus on expected cases but lack coverage for edge cases such as malformed inputs or empty datasets.
   • Solution: Add tests for scenarios like:
     • Empty responses when no data matches the filters.
     • Invalid parameters (e.g., region="", service_name=None).

2. Use of Constants for Strings

   • File: Multiple (e.g., ci-dashboard/src/ci_dashboard/api/queries/cost.py)
   • Issue: Repeated use of hardcoded strings such as 'AmazonEKS' or 'gke_parent_residual' increases chances of errors due to typos.
   • Solution: Define constants in a separate module for reuse:

     AWS_SERVICE_NAME = "AmazonEKS"
     GKE_SCOPE = "gke_parent_residual"

3. UI Loading Indicators and Error States

   • File: ci-dashboard/web/src/pages/CostPage.jsx
   • Lines: showKubernetesAllocation, allocationOverview.loading
   • Issue: Current error messages in the UI lack specificity, e.g., "Unavailable".
   • Solution: Enhance error messages with actionable guidance:

     allocationOverview.error
       ? `Could not load allocation. Please check your filters or try again later.`
       : ...

4. Documentation for New Functions

   • File: ci-dashboard/src/ci_dashboard/api/queries/cost.py
   • Issue: Newly added helper functions like _kubernetes_allocation_fact_active_roster_cte lack docstrings explaining purpose and usage.
   • Solution: Add detailed docstrings:

     def _kubernetes_allocation_fact_active_roster_cte() -> str:
         """
         Generates SQL Common Table Expression (CTE) for active roster identities.
         Used for joins against Kubernetes allocation facts.
         """

---

Conclusion

The PR is well-structured and introduces valuable functionality to the CI dashboard. However, addressing SQL injection risks, improving modularity, enhancing error handling, and expanding test coverage will significantly improve robustness and maintainability.

[ti-chi-bot]

I have already done a preliminary review for you, and I hope to help you do a better job.

Summary

This PR introduces Kubernetes workload cost allocation across multiple cloud vendors, adds APIs for auditing allocation details, and enhances the UI to display summary metrics for allocated and unallocated costs. The implementation is dense, with extensive SQL logic for cost classification and aggregation. Overall, the code is functional but exhibits areas where abstraction and modularization could improve readability and maintainability.

---

Critical Issues

1. Performance Bottleneck in SQL Queries

   • File: ci-dashboard/src/ci_dashboard/api/queries/cost.py, multiple locations.
   • Issue: Several SQL queries construct complex conditional logic and perform aggregations over potentially large datasets without indexing or optimization hints. This could lead to performance degradation with large datasets.
   • Suggestion: Investigate the use of database indexes for frequent filtering fields like usage_date, source_allocation_scope, and service_name. Consider breaking down the queries into smaller, cacheable subqueries.

   Example:

   CREATE INDEX idx_usage_date ON cost_kubernetes_workload_allocation_daily (usage_date);

---

Code Improvements

1. Excessive Duplication in SQL Condition Definitions

   • Files: ci-dashboard/src/ci_dashboard/api/queries/cost.py and related helper methods.
   • Issue: Conditions like kubernetes_parent_residual_condition, kubernetes_service_cost_condition, and others are duplicated across multiple methods. Maintaining them will be error-prone as the logic evolves.
   • Suggestion: Consolidate common conditions into reusable helper methods or constants to avoid redundancy. Use SQL templates where applicable.

   Example:

   COMMON_KUBERNETES_CONDITION = """
   {table_alias}.source_allocation_scope IN ('kubernetes_parent_residual', 'eks_parent_residual')
   OR LOWER(COALESCE({table_alias}.service_name, '')) LIKE '%kubernetes%'
   """

2. Improper Error Handling in API Endpoints

   • File: ci-dashboard/src/ci_dashboard/api/routes/pages.py, lines 184-213.
   • Issue: The /cost-kubernetes-unallocated-records endpoint assumes all filters and query parameters are valid but lacks robust error handling for cases like invalid region values or missing service_name.
   • Suggestion: Add validation for query parameters and return meaningful error responses when invalid input is detected.

   Example:

   if not service_name or not region:
       raise HTTPException(status_code=400, detail="Service name and region are required.")

3. Complexity in _kubernetes_unallocated_condition

   • File: ci-dashboard/src/ci_dashboard/api/queries/cost.py, lines 1759-1792.
   • Issue: This method has deeply nested logic, making it difficult to understand and debug.
   • Suggestion: Refactor the condition into smaller, testable methods or use intermediate variables to improve readability.

---

Best Practices

1. Insufficient Unit Test Coverage for Edge Cases

   • File: ci-dashboard/tests/api/test_routes.py
   • Issue: While the tests cover typical scenarios, edge cases like invalid dates, missing filters, or empty datasets are not adequately tested.
   • Suggestion: Add tests for cases like empty results from the cost_kubernetes_workload_allocation_daily table or invalid input parameters for /cost-kubernetes-unallocated.

   Example:

   def test_invalid_service_name():
       response = api_client.get(
           "/api/v1/pages/cost-kubernetes-unallocated",
           params={"service_name": "", "region": "us-west-2"}
       )
       assert response.status_code == 400
       assert response.json()["detail"] == "Service name and region are required."

2. Inconsistent Naming Conventions in Helper Methods

   • Files: ci-dashboard/src/ci_dashboard/api/queries/cost.py
   • Issue: Method names like _kubernetes_parent_residual_condition and _kubernetes_allocation_fact_service_expr are inconsistent and overly verbose.
   • Suggestion: Use concise and consistent naming conventions that clearly indicate their purpose. For example, rename _kubernetes_parent_residual_condition to _parent_residual_condition.

3. Lack of UI-Level Error Messaging

   • File: ci-dashboard/web/src/pages/CostPage.jsx, lines 287-301.
   • Issue: When allocation data fails to load, the UI displays a generic "Unavailable" message without additional context.
   • Suggestion: Display specific error details to aid debugging. For example, include the HTTP status code or error reason returned by the API.

   Example:

   {allocationOverview.error 
       ? `Could not load allocation: ${allocationOverview.error.message}` 
       : "Unavailable"}

4. CSS Accessibility: Disabled Buttons

   • File: ci-dashboard/web/src/styles.css, lines 1143-1148.
   • Issue: Buttons rendered as disabled lack visual cues for accessibility. Low opacity might make them difficult to perceive for users with visual impairments.
   • Suggestion: Add a tooltip or alternative text for disabled buttons.

   Example:

   .dimension-chip:disabled::after {
       content: "Option not available";
       color: #888;
   }

---

Conclusion

The PR provides valuable functionality but suffers from maintainability issues due to duplicated logic, verbose conditionals, and insufficient validation. Addressing these areas will improve the robustness and scalability of the codebase.

[dillon-zheng]

/approve

[ti-chi-bot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dillon-zheng

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [dillon-zheng]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment