feat: all-in-one nodes for cloud engines by r-birkner · Pull Request #11349 · dfinity/ic

r-birkner · 2026-08-27T10:10:41Z

Cloud engines nodes will be "all-in-one" nodes. This means they don't just run the replica, but also ic-gateway and potentially also ic-boundary. The orchestrator already supports running multiple processes, but we can't just start ic-gateway, we also need to provide it with the right, engine-specific config. That's what this PR is doing:

This PR adds an additional service to the orchestrator that fetches the HTTP Gateway config from the engine operator canister and uses it to run ic-gateway.

The config includes the domain names, DNS API URLs and API key (for the DNS-01 ACME challenge to obtain the certificates), and the ACME account. That's the bare minimum for ic-gateway to run. In the future, the config will be extended.

The config is stored in the engine operator canister, which resides on the engine itself. The orchestrator first has to discover this canister using the engine management canister. Then, it needs to use the node's identity to make the call as the config is protected and can only be accessed by the nodes making up the engine.

…operator

…erator

…aunch ic-gateway

…onfig

…-permissions.sh

zeropath-ai · 2026-08-27T12:57:36Z

✅ No security or compliance issues detected. Reviewed everything up to 918cb72.

Security Overview

🔎 Scanned files: 34 changed file(s)
🔗 Scan Link: https://zeropath.com/app/repositories/f5e9d2bb-29a4-4b64-902a-391d65fd8b2d?scanId=4f3a4a02-e2eb-488a-af2d-a32b6b4af9da&codeScanTypes=PrScan&tab=issues

Detected Code Changes

Change Type	Relevant files
Enhancement	► ic-os/components/guestos/share/ic-gateway.env Update gateway environment for TLS and ACME settings ► rs/config/src/cloud_engine.rs Add cloud engine configuration (MAINNET_ENGINE_MANAGEMENT_CANISTER_ID) and Cloud Engine Config struct ► rs/config/src/config.rs Integrate cloud_engine config into main Config ► rs/config/src/config_sample.rs Add sample cloud_engine config and test asserting engine management canister id ► rs/config/src/config_sample.rs Add unit tests referencing MAINNET_ENGINE_MANAGEMENT_CANISTER_ID
Enhancement	► rs/ic_os/config/tool/src/guestos/generate_ic_config.rs Include engine_management_canister_id in IcConfigTemplate generation (derived from GuestOSConfig or deployment environment)
Enhancement	► rs/ic_os/config/tool/templates/ic.json5.template Extend JSON5 template to include cloud_engine block with engine_management_canister_id and adjust related ACLs in IPv4/IPv6 rule formatting
Enhancement	► rs/ic_os/config/types/src/lib.rs Bump CONFIG_VERSION to 1.17.0 and document new engine_management_canister_id field in GuestOSSettings ► rs/ic_os/config/types/src/lib.rs Add engine_management_canister_id to GuestOSSettings (optional)
Enhancement	► rs/orchestrator/BUILD.bazel Add dependency on ic-registry-routing_table ► rs/orchestrator/Cargo.toml Include ic-registry-routing-table as dependency
Enhancement	► rs/orchestrator/src/cloud_engine/agent.rs Add implementation for building agents to reach engine management and operator canisters (new file)
Enhancement	► rs/orchestrator/src/cloud_engine/config.rs Add GatewayConfig, AcmeAccount, ConfigError, TryFrom for gateway config, env_overlay, tests, and Debug/Display implementations (new file)
Enhancement	► rs/orchestrator/src/cloud_engine/discovery.rs Add Discovery logic for resolving engine operator canister via memory or engine management canister (new file)
Enhancement	► rs/orchestrator/src/cloud_engine/mod.rs Add cloud_engine module with submodules and GatewayConfig export (new file)
Enhancement	► rs/orchestrator/src/cloud_engine/discovery.rs Add tests for discovery logic (new file)
Enhancement	► rs/orchestrator/src/cloud_engine/operator.rs (Not shown in diff; referenced in mod.rs)
Enhancement	► rs/orchestrator/src/cloud_engine/operator.rs Add operator client and related functionality (reference shown)
Bug Fix / Refactor	► rs/ic_os/config/types/compatibility_tests/fixtures/guestos_v1.17.0.json Add new fixture reflecting new engine_management_canister_id field (data change)
Other	► Cargo.lock: no explicit changes detected to dependencies here beyond addition of routing_table in registry dependencies (implied by build/system changes)

basvandijk

Approve for the files under rs/tests/driver/src/driver/.

A PR description would be nice for a >2k diff ;)

zeropath-ai · 2026-08-27T13:00:02Z

✅ No security or compliance issues detected. Reviewed everything up to 918cb72.

Security Overview

🔎 Scanned files: 34 changed file(s)
🔗 Scan Link: https://zeropath.com/app/repositories/6992424c-495e-4621-b441-efd6821a4be6?scanId=c57abf48-08ba-4b0c-b80c-ff89a22662b8&codeScanTypes=PrScan&tab=issues

Detected Code Changes

Change Type	Relevant files
Enhancement	► ic-os/components/guestos/share/ic-gateway.env Update gateway environment for TLS and ACME settings ► rs/config/src/cloud_engine.rs Add cloud engine configuration (MAINNET_ENGINE_MANAGEMENT_CANISTER_ID) and Cloud Engine Config struct ► rs/config/src/config.rs Integrate cloud_engine config into main Config ► rs/config/src/config_sample.rs Add sample cloud_engine config and test asserting engine management canister id ► rs/config/src/config_sample.rs Add unit tests referencing MAINNET_ENGINE_MANAGEMENT_CANISTER_ID
Enhancement	► rs/ic_os/config/tool/src/guestos/generate_ic_config.rs Include engine_management_canister_id in IcConfigTemplate generation (derived from GuestOSConfig or deployment environment)
Enhancement	► rs/ic_os/config/tool/templates/ic.json5.template Extend JSON5 template to include cloud_engine block with engine_management_canister_id and adjust related ACLs in IPv4/IPv6 rule formatting
Enhancement	► rs/ic_os/config/types/src/lib.rs Bump CONFIG_VERSION to 1.17.0 and document new engine_management_canister_id field in GuestOSSettings ► rs/ic_os/config/types/src/lib.rs Add engine_management_canister_id to GuestOSSettings (optional)
Enhancement	► rs/orchestrator/BUILD.bazel Add dependency on ic-registry-routing_table ► rs/orchestrator/Cargo.toml Include ic-registry-routing-table as dependency
Enhancement	► rs/orchestrator/src/cloud_engine/agent.rs Add implementation for building agents to reach engine management and operator canisters (new file)
Enhancement	► rs/orchestrator/src/cloud_engine/config.rs Add GatewayConfig, AcmeAccount, ConfigError, TryFrom for gateway config, env_overlay, tests, and Debug/Display implementations (new file)
Enhancement	► rs/orchestrator/src/cloud_engine/discovery.rs Add Discovery logic for resolving engine operator canister via memory or engine management canister (new file)
Enhancement	► rs/orchestrator/src/cloud_engine/mod.rs Add cloud_engine module with submodules and GatewayConfig export (new file)
Enhancement	► rs/orchestrator/src/cloud_engine/discovery.rs Add tests for discovery logic (new file)
Enhancement	► rs/orchestrator/src/cloud_engine/operator.rs (Not shown in diff; referenced in mod.rs)
Enhancement	► rs/orchestrator/src/cloud_engine/operator.rs Add operator client and related functionality (reference shown)
Bug Fix / Refactor	► rs/ic_os/config/types/compatibility_tests/fixtures/guestos_v1.17.0.json Add new fixture reflecting new engine_management_canister_id field (data change)
Other	► Cargo.lock: no explicit changes detected to dependencies here beyond addition of routing_table in registry dependencies (implied by build/system changes)

pierugo-dfinity

Are we confident launching this without a system test?

Would there be a way to test that the different arguments to different methods of different canisters do not drift away from production?

pierugo-dfinity · 2026-08-27T12:32:28Z

+pub struct Config {
+    /// Principal of the engine management canister, which an all-in-one node
+    /// needs to discover the operator canister of its own engine. When unset,
+    /// the node cannot find its operator and therefore does not run `ic-gateway`.
+    #[serde(default)]
+    pub engine_management_canister_id: Option<CanisterId>,
+}


Did we consider putting that information to the registry instead of the config? Personally, having a hard-coded canister ID inside the config feels weird + the registry can be modified if we need to + I think it would be easier to mock in tests.

And the orchestrator would know whether to launch the CloudEngineManager or not based on its node reward type, the same way it tells it is a cloud-engine node for the registry replicator & firewall.

But this is not a strong opinion, just interested in what you think

pierugo-dfinity · 2026-08-27T12:46:40Z

+    let engine_management_canister_id = match guestos_config
+        .guestos_settings
+        .engine_management_canister_id
+        .as_deref()
+        .or(matches!(
+            guestos_config.icos_settings.deployment_environment,
+            DeploymentEnvironment::Mainnet
+        )
+        .then_some(ic_config::cloud_engine::MAINNET_ENGINE_MANAGEMENT_CANISTER_ID))
+    {
+        Some(id) => serde_json::to_string(id)
+            .context("Failed to encode the engine management canister id")?,
+        None => "null".to_string(),
+    };


Suggested change

let engine_management_canister_id = match guestos_config

.guestos_settings

.engine_management_canister_id

.as_deref()

.or(matches!(

guestos_config.icos_settings.deployment_environment,

DeploymentEnvironment::Mainnet

)

.then_some(ic_config::cloud_engine::MAINNET_ENGINE_MANAGEMENT_CANISTER_ID))

{

Some(id) => serde_json::to_string(id)

.context("Failed to encode the engine management canister id")?,

None => "null".to_string(),

};

let engine_management_canister_id = match guestos_config

.guestos_settings

.engine_management_canister_id

.as_deref() {

Some(id) => id,

None if guestos_config.icos_settings.deployment_environment == DeploymentEnvironment::Mainnet => ic_config::cloud_engine::MAINNET_ENGINE_MANAGEMENT_CANISTER_ID,

None => "null",

}

.to_string();

Maybe something like this would clarify the control flow. I'm not sure why use serde_json::to_string either

pierugo-dfinity · 2026-08-27T12:50:11Z

    DomainNameMissingError(NodeId, RegistryVersion),
+
+    /// A step of the cloud engine configuration lookup failed.
+    CloudEngineError(String),


There are a lot of different Error enums (FetchError, OperatorError, ConfigError), yet not complete (AgentFactory and Discovery return OrchestratorErrors).

Would it be feasible to flatten the existing errors into one big CloudEngineManagementError enum holding every possible variant? Maybe in its own orchestrator/src/cloud_engine/error.rs

I don't think we need a new variant in OrchestratorError. Existing callers of cloud_engine_error can have their own enums (AgentFactory/Discovery) or use a different variant (in registry_helper.rs and in fn env_overlay.

pierugo-dfinity · 2026-08-27T12:56:53Z

+    registry_helper::RegistryHelper,
+};
+use config::ConfigError;
+pub(crate) use config::GatewayConfig;


No need to re-export, I think callers can import the submodule directly

pierugo-dfinity · 2026-08-27T13:03:44Z

+    /// needs to discover the operator canister of its own engine. When unset,
+    /// the node cannot find its operator and therefore does not run `ic-gateway`.
+    #[serde(default)]
+    pub engine_management_canister_id: Option<CanisterId>,


Note that despite the very misleading documentation, CanisterId's implementation of Deserialize does not check that it is a valid canister ID (it could be any principal) because CanisterId::try_from does not actually try, it just casts without validating.

ic/rs/types/base_types/src/canister_id.rs

Lines 351 to 363 in c2cebaf

impl<'de> Deserialize<'de> for CanisterId {

fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>

where

D: serde::de::Deserializer<'de>,

{

// Not all principals are valid inside a CanisterId.

// Therefore, deserialization must explicitly

// transform the PrincipalId into a CanisterId.

// A derived implementation of Deserialize would open

// the door to invariant violation.

CanisterId::try_from(PrincipalId::deserialize(deserializer)?).map_err(D::Error::custom)

}

}

pierugo-dfinity · 2026-08-27T15:55:40Z

+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        f.debug_struct("GatewayConfig")
+            .field("base_domains", &self.base_domains)
+            .field("dns_api_url", &self.dns_api_url.as_str())


Just to make sure: the DNS API URL doesn't contain anything sensitive (like query parameters or whatnot)?

This is all as intended: the DNS API config consists of the URL and the key (below). The key is sensitive, the URL not.

pierugo-dfinity · 2026-08-27T15:57:03Z

+    }
+
+    #[test]
+    fn every_missing_field_is_incomplete() {


style nit: we could use rstest here (but this is fine too)

pierugo-dfinity · 2026-08-27T16:00:20Z

+        assert_eq!(
+            discovery.validated_from_memory(SUBNET_1, VERSION),
+            Some(operator())
+        );


assert_eq!(discovery.resolved, Some(operator()));

pierugo-dfinity · 2026-08-27T16:00:31Z

+        assert_matches!(
+            discovery.validate(operator().get(), SUBNET_1, VERSION),
+            Ok(accepted) if accepted == operator()
+        );


assert_eq!(discovery.resolved, Some(operator()));

pierugo-dfinity · 2026-08-27T16:02:05Z

+    SystemTime::now()
+        .duration_since(SystemTime::UNIX_EPOCH)
+        .map_or(0, |since_epoch| since_epoch.as_secs() as i64)
+}


Some unit tests for this module would be more than welcome 🙏 (especially the invalidation logic)

frankdavid

👍🏻 for node

r-birkner added 3 commits August 27, 2026 08:00

feat(orchestrator): read the cloud engine ic-gateway config from its …

e95d010

…operator

feat(orchestrator): run ic-gateway with the config from the engine op…

0d90aaa

…erator

feat(orchestrator): harden the cloud engine config fetch and always l…

c33981d

…aunch ic-gateway

github-actions Bot added the feat label Aug 27, 2026

r-birkner and others added 2 commits August 27, 2026 12:55

Merge branch 'master' into rjb/make-orchestrator-fetch-cloud-engine-c…

ea6b8ec

…onfig

refactor(orchestrator): leave the ACME cache dir permissions to setup…

918cb72

…-permissions.sh

r-birkner marked this pull request as ready for review August 27, 2026 12:56

r-birkner requested review from a team as code owners August 27, 2026 12:56

github-actions Bot added @node @core-protocol @infra labels Aug 27, 2026

basvandijk approved these changes Aug 27, 2026

View reviewed changes

pierugo-dfinity reviewed Aug 27, 2026

View reviewed changes

frankdavid approved these changes Aug 27, 2026

View reviewed changes

	impl<'de> Deserialize<'de> for CanisterId {
	fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
	where
	D: serde::de::Deserializer<'de>,
	{
	// Not all principals are valid inside a CanisterId.
	// Therefore, deserialization must explicitly
	// transform the PrincipalId into a CanisterId.
	// A derived implementation of Deserialize would open
	// the door to invariant violation.
	CanisterId::try_from(PrincipalId::deserialize(deserializer)?).map_err(D::Error::custom)
	}
	}

Uh oh!

Conversation

r-birkner commented Aug 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zeropath-ai Bot commented Aug 27, 2026

Uh oh!

basvandijk left a comment

Choose a reason for hiding this comment

Uh oh!

zeropath-ai Bot commented Aug 27, 2026

Uh oh!

pierugo-dfinity left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

frankdavid left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

r-birkner commented Aug 27, 2026 •

edited

Loading