diff --git a/cardano-node/src/Cardano/Node/Run.hs b/cardano-node/src/Cardano/Node/Run.hs index f4f6d43e365..f6fe4e745d9 100644 --- a/cardano-node/src/Cardano/Node/Run.hs +++ b/cardano-node/src/Cardano/Node/Run.hs @@ -265,7 +265,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) @@ -305,13 +305,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 @@ -439,7 +442,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 @@ -484,7 +487,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 @@ -579,6 +582,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))] @@ -590,9 +594,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 @@ -609,7 +613,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 @@ -786,21 +790,18 @@ 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 +-- | 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, merged with re-read YAML on reload -> 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 2d131f83235..6a1cf3d6c0c 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 Ouroboros.Consensus.Node (NodeDatabasePaths (..)) import Ouroboros.Consensus.Node.Genesis (disableGenesisConfig) import Ouroboros.Consensus.Storage.LedgerDB.Args @@ -29,12 +31,14 @@ 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) -import Hedgehog (Property, discover, withTests, (===)) +import Hedgehog (Property, discover, (===)) import qualified Hedgehog +import qualified Hedgehog.Extras as H import Hedgehog.Internal.Property (evalEither, failWith) @@ -45,7 +49,7 @@ import Hedgehog.Internal.Property (evalEither, failWith) prop_sanityCheck_POM :: Property prop_sanityCheck_POM = - withTests 1 . Hedgehog.property $ do + H.propertyOnce $ do let combinedPartials = defaultPartialNodeConfiguration <> testPartialYamlConfig <> testPartialCliConfig @@ -290,6 +294,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. +-- Documents the config precedence that issue #6589 depends on: CLI flags +-- must survive a YAML-only re-read. +prop_rpcReload_cliEnabledYamlSilent :: Property +prop_rpcReload_cliEnabledYamlSilent = + H.propertyOnce $ 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 = + H.propertyOnce $ 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 = + H.propertyOnce $ 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 = + H.propertyOnce $ 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