From d4b8f3e1c24cbab8ea3ddad0c867f80d84147628 Mon Sep 17 00:00:00 2001 From: Xavier Michelon Date: Fri, 25 Nov 2022 11:21:24 +0100 Subject: [PATCH] GODT-2153: use file socket for bridge gRPC on linux & macOS. Other: fix integration tests. --- .../bridgepp/bridgepp/BridgeUtils.cpp | 43 +++++++++++ .../bridgepp/bridgepp/BridgeUtils.h | 14 ++++ .../bridgepp/bridgepp/GRPC/GRPCClient.cpp | 23 +++++- .../bridgepp/bridgepp/GRPC/GRPCConfig.cpp | 3 + .../bridgepp/bridgepp/GRPC/GRPCConfig.h | 1 + internal/frontend/grpc/config.go | 7 +- internal/frontend/grpc/config_test.go | 8 +- internal/frontend/grpc/service.go | 77 ++++++++++++++----- internal/frontend/grpc/service_methods.go | 2 +- tests/ctx_bridge_test.go | 11 ++- 10 files changed, 156 insertions(+), 33 deletions(-) diff --git a/internal/frontend/bridge-gui/bridgepp/bridgepp/BridgeUtils.cpp b/internal/frontend/bridge-gui/bridgepp/bridgepp/BridgeUtils.cpp index ac5e4b36..8c75e9be 100644 --- a/internal/frontend/bridge-gui/bridgepp/bridgepp/BridgeUtils.cpp +++ b/internal/frontend/bridge-gui/bridgepp/bridgepp/BridgeUtils.cpp @@ -237,4 +237,47 @@ SPUser randomUser() } +//**************************************************************************************************************************************************** +/// \return The OS the application is running on. +//**************************************************************************************************************************************************** +OS os() +{ + QString const osStr = QSysInfo::productType(); + if ((osStr == "macos") || (osStr == "osx")) // Qt < 5 returns "osx", Qt6 returns "macos". + return OS::MacOS; + + if (osStr == "windows") + return OS::Windows; + + return OS::Linux; +} + + +//**************************************************************************************************************************************************** +/// \return true if and only if the application is currently running on Linux. +//**************************************************************************************************************************************************** +bool onLinux() +{ + return OS::Linux == os(); +} + + +//**************************************************************************************************************************************************** +/// \return true if and only if the application is currently running on MacOS. +//**************************************************************************************************************************************************** +bool onMacOS() +{ + return OS::MacOS == os(); +} + + +//**************************************************************************************************************************************************** +/// \return true if and only if the application is currently running on Windows. +//**************************************************************************************************************************************************** +bool onWindows() +{ + return OS::Windows == os(); +} + + } // namespace bridgepp diff --git a/internal/frontend/bridge-gui/bridgepp/bridgepp/BridgeUtils.h b/internal/frontend/bridge-gui/bridgepp/bridgepp/BridgeUtils.h index fddecf9a..f28652fb 100644 --- a/internal/frontend/bridge-gui/bridgepp/bridgepp/BridgeUtils.h +++ b/internal/frontend/bridge-gui/bridgepp/bridgepp/BridgeUtils.h @@ -27,6 +27,16 @@ namespace bridgepp { +//**************************************************************************************************************************************************** +/// \brief Enumeration for the operating system. +//**************************************************************************************************************************************************** +enum class OS { + Linux = 0, ///< The Linux OS. + MacOS = 1, ///< The macOS OS. + Windows = 2, ///< The Windows OS. +}; + + QString userConfigDir(); ///< Get the path of the user configuration folder. QString userCacheDir(); ///< Get the path of the user cache folder. QString userLogsDir(); ///< Get the path of the user logs folder. @@ -35,6 +45,10 @@ qint64 randN(qint64 n); ///< return a random integer in the half open range [0, QString randomFirstName(); ///< Get a random first name from a pre-determined list. QString randomLastName(); ///< Get a random first name from a pre-determined list. SPUser randomUser(); ///< Get a random user. +OS os(); ///< Return the operating system. +bool onLinux(); ///< Check if the OS is Linux. +bool onMacOS(); ///< Check if the OS is macOS. +bool onWindows(); ///< Check if the OS in Windows. } // namespace diff --git a/internal/frontend/bridge-gui/bridgepp/bridgepp/GRPC/GRPCClient.cpp b/internal/frontend/bridge-gui/bridgepp/bridgepp/GRPC/GRPCClient.cpp index a8a719ac..debd767d 100644 --- a/internal/frontend/bridge-gui/bridgepp/bridgepp/GRPC/GRPCClient.cpp +++ b/internal/frontend/bridge-gui/bridgepp/bridgepp/GRPC/GRPCClient.cpp @@ -18,6 +18,7 @@ #include "GRPCClient.h" #include "GRPCUtils.h" +#include "../BridgeUtils.h" #include "../Exception/Exception.h" #include "../ProcessMonitor.h" @@ -39,9 +40,17 @@ qint64 const grpcConnectionWaitTimeoutMs = 60000; ///< Timeout for the connectio qint64 const grpcConnectionRetryDelayMs = 10000; ///< Retry delay for the gRPC connection in milliseconds. +//**************************************************************************************************************************************************** +/// return true if gRPC connection should use file socket instead of TCP socket. +//**************************************************************************************************************************************************** +bool useFileSocket() { + return !onWindows(); } +} // anonymous namespace + + //**************************************************************************************************************************************************** // //**************************************************************************************************************************************************** @@ -113,11 +122,20 @@ bool GRPCClient::connectToServer(GRPCConfig const &config, ProcessMonitor *serve try { serverToken_ = config.token.toStdString(); + QString address; + grpc::ChannelArguments chanArgs; + if (useFileSocket()) + { + address = QString("unix://" + config.fileSocketPath); + chanArgs.SetSslTargetNameOverride("127.0.0.1"); // for file socket, we skip name verification to avoid a confusion localhost/127.0.0.1 + } else { + address = QString("127.0.0.1:%1").arg(config.port); + } + SslCredentialsOptions opts; opts.pem_root_certs += config.cert.toStdString(); - QString const address = QString("127.0.0.1:%1").arg(config.port); - channel_ = CreateChannel(address.toStdString(), grpc::SslCredentials(opts)); + channel_ = CreateCustomChannel(address.toStdString(), grpc::SslCredentials(opts),chanArgs); if (!channel_) throw Exception("Channel creation failed."); @@ -158,7 +176,6 @@ bool GRPCClient::connectToServer(GRPCConfig const &config, ProcessMonitor *serve if (clientToken != returnedClientToken) throw Exception("gRPC server returned an invalid token"); - if (!status.ok()) throw Exception(QString::fromStdString(status.error_message())); diff --git a/internal/frontend/bridge-gui/bridgepp/bridgepp/GRPC/GRPCConfig.cpp b/internal/frontend/bridge-gui/bridgepp/bridgepp/GRPC/GRPCConfig.cpp index 87ccf057..952a39e5 100644 --- a/internal/frontend/bridge-gui/bridgepp/bridgepp/GRPC/GRPCConfig.cpp +++ b/internal/frontend/bridge-gui/bridgepp/bridgepp/GRPC/GRPCConfig.cpp @@ -31,6 +31,7 @@ Exception const couldNotSaveException("The service configuration file could not QString const keyPort = "port"; ///< The JSON key for the port. QString const keyCert = "cert"; ///< The JSON key for the TLS certificate. QString const keyToken = "token"; ///< The JSON key for the identification token. +QString const keyFileSocketPath = "fileSocketPath"; ///< The JSON key for the file socket path. //**************************************************************************************************************************************************** @@ -88,6 +89,7 @@ bool GRPCConfig::load(QString const &path, QString *outError) port = jsonIntValue(object, keyPort); cert = jsonStringValue(object, keyCert); token = jsonStringValue(object, keyToken); + fileSocketPath = jsonStringValue(object, keyFileSocketPath); return true; } @@ -113,6 +115,7 @@ bool GRPCConfig::save(QString const &path, QString *outError) object.insert(keyPort, port); object.insert(keyCert, cert); object.insert(keyToken, token); + object.insert(keyFileSocketPath, fileSocketPath); QFile file(path); if (!file.open(QIODevice::WriteOnly | QIODevice::Text)) diff --git a/internal/frontend/bridge-gui/bridgepp/bridgepp/GRPC/GRPCConfig.h b/internal/frontend/bridge-gui/bridgepp/bridgepp/GRPC/GRPCConfig.h index 37972921..8f3245e8 100644 --- a/internal/frontend/bridge-gui/bridgepp/bridgepp/GRPC/GRPCConfig.h +++ b/internal/frontend/bridge-gui/bridgepp/bridgepp/GRPC/GRPCConfig.h @@ -29,6 +29,7 @@ public: // data members qint32 port; ///< The port. QString cert; ///< The server TLS certificate. QString token; ///< The identification token. + QString fileSocketPath; ///< The path of the file socket. bool load(QString const &path, QString *outError = nullptr); ///< Load the service config from file bool save(QString const &path, QString *outError = nullptr); ///< Save the service config to file diff --git a/internal/frontend/grpc/config.go b/internal/frontend/grpc/config.go index a16a5f4f..57f9fc4f 100644 --- a/internal/frontend/grpc/config.go +++ b/internal/frontend/grpc/config.go @@ -24,9 +24,10 @@ import ( // Config is a structure containing the service configuration data that are exchanged by the gRPC server and client. type Config struct { - Port int `json:"port"` - Cert string `json:"cert"` - Token string `json:"token"` + Port int `json:"port"` + Cert string `json:"cert"` + Token string `json:"token"` + FileSocketPath string `json:"fileSocketPath"` } // save saves a gRPC service configuration to file. diff --git a/internal/frontend/grpc/config_test.go b/internal/frontend/grpc/config_test.go index 4ebfb96e..6ec2c074 100644 --- a/internal/frontend/grpc/config_test.go +++ b/internal/frontend/grpc/config_test.go @@ -29,13 +29,15 @@ const ( dummyCert = "A dummy cert" dummyToken = "A dummy token" tempFileName = "test.json" + socketPath = "/a/socket/file/path" ) func TestConfig(t *testing.T) { conf1 := Config{ - Port: dummyPort, - Cert: dummyCert, - Token: dummyToken, + Port: dummyPort, + Cert: dummyCert, + Token: dummyToken, + FileSocketPath: socketPath, } // Read-back test diff --git a/internal/frontend/grpc/service.go b/internal/frontend/grpc/service.go index 55a7aa79..1c35e46b 100644 --- a/internal/frontend/grpc/service.go +++ b/internal/frontend/grpc/service.go @@ -24,8 +24,11 @@ import ( "crypto/tls" "errors" "fmt" + "io/fs" "net" + "os" "path/filepath" + "runtime" "sync" "time" @@ -107,14 +110,38 @@ func NewService( logrus.WithError(err).Panic("Could not generate gRPC TLS config") } - listener, err := net.Listen("tcp", "127.0.0.1:0") // Port should be provided by the OS. - if err != nil { - logrus.WithError(err).Panic("Could not create gRPC listener") + config := Config{ + Cert: string(certPEM), + Token: uuid.NewString(), } - token := uuid.NewString() + var listener net.Listener + if useFileSocket() { + var err error + if config.FileSocketPath, err = computeFileSocketPath(); err != nil { + logrus.WithError(err).WithError(err).Panic("Could not create gRPC file socket") + } - if path, err := saveGRPCServerConfigFile(locations, listener, token, certPEM); err != nil { + listener, err = net.Listen("unix", config.FileSocketPath) + if err != nil { + logrus.WithError(err).Panic("Could not create gRPC file socket listener") + } + } else { + var err error + listener, err = net.Listen("tcp", "127.0.0.1:0") // Port should be provided by the OS. + if err != nil { + logrus.WithError(err).Panic("Could not create gRPC listener") + } + + // retrieve the port assigned by the system, so that we can put it in the config file. + address, ok := listener.Addr().(*net.TCPAddr) + if !ok { + return nil, fmt.Errorf("could not retrieve gRPC service listener address") + } + config.Port = address.Port + } + + if path, err := saveGRPCServerConfigFile(locations, &config); err != nil { logrus.WithError(err).WithField("path", path).Panic("Could not write gRPC service config file") } else { logrus.WithField("path", path).Info("Successfully saved gRPC service config file") @@ -123,8 +150,8 @@ func NewService( s := &Service{ grpcServer: grpc.NewServer( grpc.Creds(credentials.NewTLS(tlsConfig)), - grpc.UnaryInterceptor(newUnaryTokenValidator(token)), - grpc.StreamInterceptor(newStreamTokenValidator(token)), + grpc.UnaryInterceptor(newUnaryTokenValidator(config.Token)), + grpc.StreamInterceptor(newStreamTokenValidator(config.Token)), ), listener: listener, @@ -195,7 +222,7 @@ func (s *Service) Loop() error { s.watchEvents() }() - s.log.Info("Starting gRPC server") + s.log.WithField("useFileSocket", useFileSocket()).Info("Starting gRPC server") doneCh := make(chan struct{}) defer close(doneCh) @@ -456,18 +483,7 @@ func newTLSConfig() (*tls.Config, []byte, error) { }, certPEM, nil } -func saveGRPCServerConfigFile(locations Locator, listener net.Listener, token string, certPEM []byte) (string, error) { - address, ok := listener.Addr().(*net.TCPAddr) - if !ok { - return "", fmt.Errorf("could not retrieve gRPC service listener address") - } - - sc := Config{ - Port: address.Port, - Cert: string(certPEM), - Token: token, - } - +func saveGRPCServerConfigFile(locations Locator, config *Config) (string, error) { settingsPath, err := locations.ProvideSettingsPath() if err != nil { return "", err @@ -475,7 +491,7 @@ func saveGRPCServerConfigFile(locations Locator, listener net.Listener, token st configPath := filepath.Join(settingsPath, serverConfigFileName) - return configPath, sc.save(configPath) + return configPath, config.save(configPath) } // validateServerToken verify that the server token provided by the client is valid. @@ -558,3 +574,22 @@ func (s *Service) monitorParentPID() { } } } + +// computeFileSocketPath Return an available path for a socket file in the temp folder. +func computeFileSocketPath() (string, error) { + tempPath := os.TempDir() + for i := 0; i < 1000; i++ { + path := filepath.Join(tempPath, fmt.Sprintf("bridge_%v.sock", uuid.NewString())) + if _, err := os.Stat(path); errors.Is(err, fs.ErrNotExist) { + return path, nil + } + } + + return "", errors.New("unable to find a suitable file socket in user config folder") +} + +// useFileSocket return true iff file socket should be used for the gRPC service. +func useFileSocket() bool { + //goland:noinspection GoBoolExpressions + return runtime.GOOS != "windows" +} diff --git a/internal/frontend/grpc/service_methods.go b/internal/frontend/grpc/service_methods.go index 627ecd12..c63203c6 100644 --- a/internal/frontend/grpc/service_methods.go +++ b/internal/frontend/grpc/service_methods.go @@ -117,7 +117,7 @@ func (s *Service) quit() error { } // The following call is launched as a goroutine, as it will wait for current calls to end, including this one. - s.grpcServer.GracefulStop() + s.grpcServer.GracefulStop() // gRPC does clean up and remove the file socket if used. }() return nil diff --git a/tests/ctx_bridge_test.go b/tests/ctx_bridge_test.go index a076580e..a37ccff5 100644 --- a/tests/ctx_bridge_test.go +++ b/tests/ctx_bridge_test.go @@ -272,10 +272,17 @@ func (t *testCtx) initFrontendClient() error { return fmt.Errorf("failed to append certificates to pool") } + var target string + if len(cfg.FileSocketPath) != 0 { + target = "unix://" + cfg.FileSocketPath + } else { + target = fmt.Sprintf("%v:%d", constants.Host, cfg.Port) + } + conn, err := grpc.DialContext( context.Background(), - fmt.Sprintf("%v:%d", constants.Host, cfg.Port), - grpc.WithTransportCredentials(credentials.NewTLS(&tls.Config{RootCAs: cp})), + target, + grpc.WithTransportCredentials(credentials.NewTLS(&tls.Config{RootCAs: cp, ServerName: "127.0.0.1"})), grpc.WithUnaryInterceptor(func(ctx context.Context, method string, req, reply interface{}, cc *grpc.ClientConn, invoker grpc.UnaryInvoker, opts ...grpc.CallOption) error { return invoker(metadata.AppendToOutgoingContext(ctx, "server-token", cfg.Token), method, req, reply, cc, opts...) }),