Skip to content

Conversation

@pthirun
Copy link
Contributor

@pthirun pthirun commented Jan 28, 2026

Problem Statement

As part of the controller gRPC migration, the enableStore API needs to be available via gRPC while maintaining the existing HTTP endpoint during the migration period. This follows the same pattern established by other controller API migrations, where business logic is abstracted so both HTTP and gRPC can share the same implementation.

Solution

Added gRPC support for the enableStore API following the established migration pattern:

  1. Proto definitions (StoreGrpcService.proto): Added enableStore RPC method with EnableStoreGrpcRequest, EnableStoreGrpcResponse, and StoreEnableOperation enum for operation types (READ, WRITE, READ_WRITE)
  2. Request handler (StoreRequestHandler.java): Added enableStore() method that validates parameters and calls the appropriate admin method based on operation type
  3. gRPC service implementation (StoreGrpcServiceImpl.java): Added enableStore() override with ACL checks (matching HTTP endpoint behavior)
  4. HTTP route update (StoresRoutes.java): Updated to convert HTTP requests to gRPC format and delegate to the shared handler

The implementation reuses the existing business logic in Admin.setStoreReadability(), Admin.setStoreWriteability(), and Admin.setStoreReadWriteability(), ensuring consistency between HTTP and gRPC endpoints.

Code changes

  • Added new code behind a config. If so list the config names and their default values in the PR description.
  • Introduced new log lines.
    • Confirmed if logs need to be rate limited to avoid excessive logging. (Info/Debug level logging only, consistent with other gRPC endpoints)

Concurrency-Specific Checks

Both reviewer and PR author to verify

  • Code has no race conditions or thread safety issues. (Stateless request handling, delegates to existing thread-safe Admin methods)
  • Proper synchronization mechanisms (e.g., synchronized, RWLock) are used where needed. (N/A - no shared mutable state)
  • No blocking calls inside critical sections that could lead to deadlocks or performance degradation. (N/A - no critical sections)
  • Verified thread-safe collections are used (e.g., ConcurrentHashMap, CopyOnWriteArrayList). (N/A - no collections introduced)
  • Validated proper exception handling in multi-threaded code to avoid silent thread termination. (Uses handleRequest() utility for consistent error handling)

How was this PR tested?

  • New unit tests added.
    • testEnableStoreReadOperation - Verifies read operation
    • testEnableStoreWriteOperation - Verifies write operation
    • testEnableStoreReadWriteOperation - Verifies read-write operation
    • testEnableStoreUnspecifiedOperation - Verifies error for unspecified operation
    • testEnableStoreReturnsSuccessfulResponse - Verifies gRPC success
    • testEnableStoreReturnsErrorResponse - Verifies gRPC error handling
    • testEnableStoreReturnsPermissionDenied - Verifies ACL enforcement
  • New integration tests added.
    • testEnableStoreGrpcEndpoint - End-to-end test for all operation types
  • Modified or extended existing tests.
  • Code coverage verified.
  • Verified backward compatibility (if applicable). (HTTP endpoint unchanged, gRPC is additive)

Does this PR introduce any user-facing or breaking changes?

  • No. You can skip the rest of this section.
  • Yes. Clearly explain the behavior change and its impact.

@pthirun pthirun force-pushed the controller-grpc-migration-enable-store branch from b860c7a to d122b3d Compare January 28, 2026 22:47
@pthirun
Copy link
Contributor Author

pthirun commented Jan 29, 2026

/rerun-ci

Add gRPC support for the enableStore API while maintaining
backward compatibility with the existing HTTP endpoint.

Changes:
- Add enableStore RPC to StoreGrpcService.proto with StoreEnableOperation enum
- Add handler method to StoreRequestHandler
- Add gRPC service implementation with ACL checks
- Update HTTP route to delegate to shared handler
- Add comprehensive unit and integration tests

Co-Authored-By: Claude Opus 4.5 <[email protected]>
@pthirun pthirun force-pushed the controller-grpc-migration-enable-store branch from d122b3d to b1e1d09 Compare January 29, 2026 22:42
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.

1 participant