diff --git a/cardano-node/src/Cardano/Node/Run.hs b/cardano-node/src/Cardano/Node/Run.hs index 5298d926c9d..09f770e9d28 100644 --- a/cardano-node/src/Cardano/Node/Run.hs +++ b/cardano-node/src/Cardano/Node/Run.hs @@ -281,7 +281,7 @@ handleNodeWithTracers cmdPc nc p@(SomeConsensusProtocol blockType runP) = do then DisabledBlockForging else EnabledBlockForging)) - handleSimpleNode blockType runP tracers nc networkMagic + handleSimpleNode blockType runP tracers nc cmdPc networkMagic (\nk -> do setNodeKernel nodeKernelData nk traceWith (nodeStateTracer tracers) NodeKernelOnline) @@ -327,7 +327,7 @@ handleNodeWithTracers cmdPc nc p@(SomeConsensusProtocol blockType runP) = do -- We ignore peer logging thread if it dies, but it will be killed -- when 'handleSimpleNode' terminates. - handleSimpleNode blockType runP tracers nc networkMagic + handleSimpleNode blockType runP tracers nc cmdPc networkMagic (\nk -> do setNodeKernel nodeKernelData nk traceWith (nodeStateTracer tracers) NodeKernelOnline) @@ -400,13 +400,16 @@ handleSimpleNode -> Api.ProtocolInfoArgs blk -> Tracers RemoteAddress LocalAddress blk IO -> NodeConfiguration + -> PartialNodeConfiguration + -- ^ Original CLI configuration, used for SIGHUP config reload so CLI + -- overrides are preserved when re-reading the YAML file. -> NetworkMagic -> (NodeKernel IO RemoteAddress LocalConnectionId blk -> IO ()) -- ^ Called on the 'NodeKernel' after creating it, but before the network -- layer is initialised. This implies this function must not block, -- otherwise the node won't actually start. -> IO () -handleSimpleNode blockType runP tracers nc networkMagic onKernel = do +handleSimpleNode blockType runP tracers nc cmdPc networkMagic onKernel = do logStartupWarnings logDeprecatedLedgerDBOptions @@ -534,7 +537,7 @@ handleSimpleNode blockType runP tracers nc networkMagic onKernel = do (readTVar ledgerPeerSnapshotPathVar) (readTVar useLedgerVar) (writeTVar ledgerPeerSnapshotVar) - updateRpcConfiguration (startupTracer tracers) (ncConfigFile nc) rpcConfigVar + updateRpcConfiguration (startupTracer tracers) cmdPc rpcConfigVar traceWith (startupTracer tracers) (BlockForgingUpdate NotEffective) ) Nothing @@ -579,7 +582,7 @@ handleSimpleNode blockType runP tracers nc networkMagic onKernel = do rnNodeKernelHook = \registry nodeKernel -> do -- reinstall `SIGHUP` handler installSigHUPHandler (startupTracer tracers) (Consensus.kesAgentTracer $ consensusTracers tracers) - blockType nc networkMagic nodeKernel localRootsVar publicRootsVar useLedgerVar + blockType nc cmdPc networkMagic nodeKernel localRootsVar publicRootsVar useLedgerVar useBootstrapVar ledgerPeerSnapshotPathVar ledgerPeerSnapshotVar rpcConfigVar rnNodeKernelHook nodeArgs registry nodeKernel @@ -674,6 +677,7 @@ installSigHUPHandler :: Tracer IO (StartupTrace blk) -> Tracer IO KESAgentClientTrace -> Api.BlockType blk -> NodeConfiguration + -> PartialNodeConfiguration -- ^ original CLI configuration -> NetworkMagic -> NodeKernel IO RemoteAddress (ConnectionId LocalAddress) blk -> StrictTVar IO [(HotValency, WarmValency, Map RelayAccessPoint (LocalRootConfig PeerTrustable))] @@ -685,9 +689,9 @@ installSigHUPHandler :: Tracer IO (StartupTrace blk) -> StrictTVar IO RpcConfig -> IO () #ifndef UNIX -installSigHUPHandler _ _ _ _ _ _ _ _ _ _ _ _ _ = return () +installSigHUPHandler _ _ _ _ _ _ _ _ _ _ _ _ _ _ = return () #else -installSigHUPHandler startupTracer kesAgentTracer blockType nc networkMagic nodeKernel localRootsVar +installSigHUPHandler startupTracer kesAgentTracer blockType nc cmdPc networkMagic nodeKernel localRootsVar publicRootsVar useLedgerVar useBootstrapPeersVar ledgerPeerSnapshotPathVar ledgerPeerSnapshotVar rpcConfigVar = void $ Signals.installHandler @@ -704,7 +708,7 @@ installSigHUPHandler startupTracer kesAgentTracer blockType nc networkMagic node (readTVar ledgerPeerSnapshotPathVar) (readTVar useLedgerVar) (writeTVar ledgerPeerSnapshotVar) - updateRpcConfiguration startupTracer (ncConfigFile nc) rpcConfigVar + updateRpcConfiguration startupTracer cmdPc rpcConfigVar ) Nothing #endif @@ -881,21 +885,22 @@ rpcServerLoop startupTracer rpcTracer rpcConfigVar networkMagic = go atomically . modifyTVar rpcConfigVar $ \config -> config{isEnabled = Identity False} #ifdef UNIX --- | Reload RPC configuration from the configuration file -updateRpcConfiguration :: Tracer IO (StartupTrace blk) -- ^ tracer tracing the configuration reload - -> ConfigYamlFilePath -- ^ node configuration file, to reload configuration from - -> StrictTVar IO RpcConfig -- ^ TVar storing RPC configuration +-- | Reload RPC configuration by re-reading the YAML config file and merging +-- with CLI overrides, exactly like startup does via 'buildNodeConfiguration'. +updateRpcConfiguration :: Tracer IO (StartupTrace blk) + -- ^ tracer for configuration reload events + -> PartialNodeConfiguration + -- ^ original CLI configuration, so CLI overrides are + -- preserved when re-reading the YAML file + -> StrictTVar IO RpcConfig + -- ^ TVar storing RPC configuration -> IO () -updateRpcConfiguration tracer configFilePath rpcConfigVar = do - result <- fmap (join . first Exception.displayException) - . try @Exception.SomeException - . fmap makeNodeConfiguration - . parseNodeConfigurationFP - $ Just configFilePath +updateRpcConfiguration tracer cmdPc rpcConfigVar = do + result <- try @Exception.SomeException $ buildNodeConfiguration cmdPc case result of Left err -> -- reload failure, we don't do anything this time - traceWith tracer (RpcConfigUpdateError $ pack err) + traceWith tracer (RpcConfigUpdateError $ pack (Exception.displayException err)) Right NodeConfiguration{ncRpcConfig=newConfig} -> join . atomically $ do oldConfig <- readTVar rpcConfigVar diff --git a/cardano-node/test/Test/Cardano/Node/POM.hs b/cardano-node/test/Test/Cardano/Node/POM.hs index ab86a6c0300..2fd6eaab82d 100644 --- a/cardano-node/test/Test/Cardano/Node/POM.hs +++ b/cardano-node/test/Test/Cardano/Node/POM.hs @@ -8,6 +8,8 @@ module Test.Cardano.Node.POM ) where +import Cardano.Api (File (..)) + import Cardano.Crypto.ProtocolMagic (RequiresNetworkMagic (..)) import Cardano.Network.ConsensusMode (ConsensusMode (..)) import Cardano.Network.Diffusion.Configuration (defaultNumberOfBigLedgerPeers) @@ -18,7 +20,7 @@ import Cardano.Node.Configuration.POM import Cardano.Node.Configuration.Socket import Cardano.Node.Handlers.Shutdown import Cardano.Node.Types -import Cardano.Rpc.Server.Config (makeRpcConfig) +import Cardano.Rpc.Server.Config (RpcConfigF (..), makeRpcConfig) import Cardano.Tracing.Config (PartialTraceOptions (..), defaultPartialTraceConfiguration, partialTraceSelectionToEither) import Ouroboros.Consensus.Node (NodeDatabasePaths (..)) @@ -31,6 +33,7 @@ import Ouroboros.Network.PeerSelection.PeerSharing (PeerSharing (..)) import Ouroboros.Network.TxSubmission.Inbound.V2.Types import Data.Bifunctor (first) +import Data.Functor.Identity (Identity (..)) import Data.Monoid (Last (..)) import Data.String import Data.Text (Text) @@ -303,6 +306,57 @@ eExpectedConfig = do , ncTxSubmissionInitDelay = defaultTxSubmissionInitDelay } +-- A socket config with a node socket path, needed for RPC-enabled tests +-- because makeRpcConfig validates that a node socket exists when RPC is enabled. +testSocketConfigWithPath :: Last SocketConfig +testSocketConfigWithPath = + Last . Just $ SocketConfig mempty mempty mempty (Last . Just $ File "node.socket") + +-- | CLI --grpc-enable + YAML silent -> RPC stays enabled. +-- Verifies the fix for issue #6589: a node started with --grpc-enable must not +-- have RPC silently disabled on SIGHUP. +prop_rpcReload_cliEnabledYamlSilent :: Property +prop_rpcReload_cliEnabledYamlSilent = + withTests 1 . Hedgehog.property $ do + let cliConfig = testPartialCliConfig + { pncRpcConfig = RpcConfig (Last (Just True)) mempty mempty + , pncSocketConfig = testSocketConfigWithPath + } + merged = defaultPartialNodeConfiguration <> testPartialYamlConfig <> cliConfig + NodeConfiguration{ncRpcConfig = RpcConfig{isEnabled}} <- evalEither $ makeNodeConfiguration merged + isEnabled === Identity True + +-- | CLI silent + YAML EnableRpc -> RPC enabled. +prop_rpcReload_cliSilentYamlEnabled :: Property +prop_rpcReload_cliSilentYamlEnabled = + withTests 1 . Hedgehog.property $ do + let yamlConfig = testPartialYamlConfig{pncRpcConfig = RpcConfig (Last (Just True)) mempty mempty} + cliConfig = testPartialCliConfig{pncSocketConfig = testSocketConfigWithPath} + merged = defaultPartialNodeConfiguration <> yamlConfig <> cliConfig + NodeConfiguration{ncRpcConfig = RpcConfig{isEnabled}} <- evalEither $ makeNodeConfiguration merged + isEnabled === Identity True + +-- | Both CLI and YAML silent -> RPC disabled (default). +prop_rpcReload_bothSilent :: Property +prop_rpcReload_bothSilent = + withTests 1 . Hedgehog.property $ do + let merged = defaultPartialNodeConfiguration <> testPartialYamlConfig <> testPartialCliConfig + NodeConfiguration{ncRpcConfig = RpcConfig{isEnabled}} <- evalEither $ makeNodeConfiguration merged + isEnabled === Identity False + +-- | CLI --grpc-enable wins over YAML EnableRpc: false. +prop_rpcReload_cliOverridesYaml :: Property +prop_rpcReload_cliOverridesYaml = + withTests 1 . Hedgehog.property $ do + let yamlConfig = testPartialYamlConfig{pncRpcConfig = RpcConfig (Last (Just False)) mempty mempty} + cliConfig = testPartialCliConfig + { pncRpcConfig = RpcConfig (Last (Just True)) mempty mempty + , pncSocketConfig = testSocketConfigWithPath + } + merged = defaultPartialNodeConfiguration <> yamlConfig <> cliConfig + NodeConfiguration{ncRpcConfig = RpcConfig{isEnabled}} <- evalEither $ makeNodeConfiguration merged + isEnabled === Identity True + -- ----------------------------------------------------------------------------- tests :: IO Bool