Skip to content

WSD API migration to gRPC#9

Open
atharvasaraf123 wants to merge 11 commits into
GoogleCloudPlatform:mainfrom
atharvasaraf123:grpc
Open

WSD API migration to gRPC#9
atharvasaraf123 wants to merge 11 commits into
GoogleCloudPlatform:mainfrom
atharvasaraf123:grpc

Conversation

@atharvasaraf123

@atharvasaraf123 atharvasaraf123 commented May 4, 2026

Copy link
Copy Markdown
Collaborator

Migrates the Key Manager Daemon (WSD) from a manual REST-only HTTP multiplexer to a fully typed, high-performance Dual-Socket Architecture supporting both gRPC and REST JSON clients simultaneously using grpc-gateway.

Changes Made:
Dual Unix Domain Socket Support: The daemon now securely listens on two distinct endpoints (-grpc.sock and standard .sock), allowing native gRPC traffic to process without overhead while routing conventional REST requests via a reverse proxy.
Backward-Compatible JSON: Configured runtime.JSONPb with UseProtoNames: true to guarantee pristine snake_case key serialization matching legacy API outputs.
Robust Error & Validation Handling: Upgraded endpoint errors to status.Errorf to correctly map bad requests to HTTP 400/404 REST codes.

Changes in the API:

  1. Successful post on destroy used to return 204(StatusNoContent), now returns 200(StatusOK).
  2. Valid endpoint using an unsupported HTTP method now returns 501 (StatusNotImplemented), instead of 405(StatusMethodNotAllowed).

New Autogenerated files:
api_grpc.pb.go - Provided with RegisterWorkloadServiceServer
api.pb.gw.go - Provided with RegisterWorkloadServiceHandler

Testing:
Added UTs for GRPC apis
All existing UTs along with workload passes

Comment thread workload_service/proto/api.proto Outdated
Comment thread workload_service/server.go Outdated
Comment thread workload_service/server.go

// Shutdown gracefully shuts down the server.
func (s *Server) Shutdown(ctx context.Context) error {
if s.grpcServer != nil {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should also close the HTTP endpoint.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are doing s.httpServer.Shutdown below in the function.

Comment thread workload_service/server.go Outdated
Comment thread workload_service/server.go Outdated
Comment thread workload_service/server_test.go Outdated
@atharvasaraf123 atharvasaraf123 force-pushed the grpc branch 2 times, most recently from 65e1bc0 to 72cacbd Compare May 13, 2026 12:10
@atharvasaraf123

Copy link
Copy Markdown
Collaborator Author

/gcbrun

@NilanjanDaw

Copy link
Copy Markdown
Collaborator

I see a few of the schema cloudbuild tests are failing, which means we are diverging from the current API behavior. Can you please check once.

@atharvasaraf123 atharvasaraf123 force-pushed the grpc branch 5 times, most recently from 705ebd2 to 0ef665d Compare May 21, 2026 09:44
Comment thread workload_service/server.go Outdated
return s, nil
}

func handleRoutingError(ctx context.Context, mux *runtime.ServeMux, marshaler runtime.Marshaler, w http.ResponseWriter, r *http.Request, httpStatus int) {

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@NilanjanDaw @atulpatildbz do we also want to remove this check? If we remove this, then we will get a 501 (StatusNotImplemented) instead of 405 (StatusMethodNotAllowed).

Comment on lines +598 to +599
if err := protovalidate.Validate(req); err != nil {
return nil, status.Errorf(codes.InvalidArgument, "invalid request: %v", err)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of a per-function validate call which will require us to manually add these checks, can we add a central validator to automatically validate everything?
Something like

// ValidationInterceptor validates incoming protobuf requests before they reach the handler.
func ValidationInterceptor(validator *protovalidate.Validator) grpc.UnaryServerInterceptor {
	return func(ctx context.Context, req interface{}, info *grpc.UnaryServerInfo, handler grpc.UnaryHandler) (interface{}, error) {
		if msg, ok := req.(proto.Message); ok {
			if err := validator.Validate(msg); err != nil {
				return nil, status.Errorf(codes.InvalidArgument, "validation failed: %v", err)
			}
		}
		return handler(ctx, req)
	}
}

and then wire it up in the newserver

// In NewServer:
validator, err := protovalidate.New()
if err != nil {
    return nil, fmt.Errorf("failed to initialize protovalidate: %w", err)
}
grpcServer := grpc.NewServer(
    grpc.UnaryInterceptor(ValidationInterceptor(validator)),
)

if err := protovalidate.Validate(req); err != nil {
return nil, status.Errorf(codes.InvalidArgument, "invalid request: %v", err)
}
keys, _, err := s.keyProtectionService.EnumerateKEMKeys(ctx, 100, 0)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@atulpatildbz how is the limit and offset working here? does this mean we only return the first 100 keys?

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.

2 participants