fix(cfg)!: make sure we use correct relays (#2778)

## Description

This takes us back to only relying on the env var for forcing staging
relays. However for tests it should still remain active with the
test/feature combo by default.

## Breaking Changes

- `TEST_DNS_NODE_ORIGIN` is removed
- iroh now defaults to using prod relays unless
`IROH_FORCE_STAGING_RELAYS` is set to a non empty value

## Notes & open questions

<!-- Any notes, remarks or open questions you have to make about the PR.
-->

## Change checklist

- [ ] Self-review.
- [ ] Documentation updates following the [style
guide](https://rust-lang.github.io/rfcs/1574-more-api-documentation-conventions.html#appendix-a-full-conventions-text),
if relevant.
- [ ] Tests if relevant.
- [ ] All breaking changes documented.

.github/workflows/release.yml

@@ -40,7 +40,6 @@ env:
   SCCACHE_CACHE_SIZE: "50G"
   BIN_NAMES: "iroh,iroh-relay,iroh-dns-server"
   RELEASE_VERSION: ${{ github.event.inputs.release_version }}
-  IROH_FORCE_STAGING_RELAYS: "1"
 
 jobs:
     create-release:

iroh-net/src/discovery/dns.rs

@@ -6,17 +6,14 @@ use futures_lite::stream::Boxed as BoxStream;
 use crate::{
     discovery::{Discovery, DiscoveryItem},
     dns::ResolverExt,
+    relay::force_staging_infra,
     Endpoint, NodeId,
 };
 
 /// The n0 testing DNS node origin, for production.
 pub const N0_DNS_NODE_ORIGIN_PROD: &str = "dns.iroh.link";
 /// The n0 testing DNS node origin, for testing.
 pub const N0_DNS_NODE_ORIGIN_STAGING: &str = "staging-dns.iroh.link";
-/// Testing DNS node origin, must run server from [`crate::test_utils::DnsPkarrServer`].
-#[cfg(any(test, feature = "test-utils"))]
-#[cfg_attr(iroh_docsrs, doc(cfg(any(test, feature = "test-utils"))))]
-pub const TEST_DNS_NODE_ORIGIN: &str = "dns.iroh.test";
 
 const DNS_STAGGERING_MS: &[u64] = &[200, 300];
 
@@ -57,26 +54,15 @@ impl DnsDiscovery {
     ///
     /// # Usage during tests
     ///
-    /// When `cfg(test)` is enabled or when using the `test-utils` cargo feature the
-    /// [`TEST_DNS_NODE_ORIGIN`] is used.
-    ///
-    /// Note that the `iroh.test` domain is not integrated with the global DNS network and
-    /// thus node discovery is effectively disabled.  To use node discovery in a test use
-    /// the [`crate::test_utils::DnsPkarrServer`] in the test and configure it as a
-    /// custom discovery mechanism.
-    ///
-    /// For testing it is also possible to use the [`N0_DNS_NODE_ORIGIN_STAGING`] domain
-    /// with [`DnsDiscovery::new`].  This would then use a hosted discovery service again,
-    /// but for testing purposes.
+    /// For testing it is possible to use the [`N0_DNS_NODE_ORIGIN_STAGING`] domain
+    /// with [`DnsDiscovery::new`].  This would then use a hosted staging discovery
+    /// service for testing purposes.
     pub fn n0_dns() -> Self {
-        #[cfg(not(any(test, feature = "test-utils")))]
-        {
+        if force_staging_infra() {
+            Self::new(N0_DNS_NODE_ORIGIN_STAGING.to_string())
+        } else {
             Self::new(N0_DNS_NODE_ORIGIN_PROD.to_string())
         }
-        #[cfg(any(test, feature = "test-utils"))]
-        {
-            Self::new(TEST_DNS_NODE_ORIGIN.to_string())
-        }
     }
 }
 

iroh-net/src/discovery/pkarr.rs

@@ -61,6 +61,7 @@ use crate::{
     discovery::{Discovery, DiscoveryItem},
     dns::node_info::NodeInfo,
     key::SecretKey,
+    relay::force_staging_infra,
     AddrInfo, Endpoint, NodeId,
 };
 
@@ -177,15 +178,16 @@ impl PkarrPublisher {
     /// This uses the pkarr relay server operated by [number 0], at
     /// [`N0_DNS_PKARR_RELAY_PROD`].
     ///
-    /// When compiling for tests, i.e. when `cfg(test)` is true, or when the `test-utils`
-    /// crate feature is enabled the [`N0_DNS_PKARR_RELAY_STAGING`] server is used instead.
+    /// When running with the environment variable
+    /// `IROH_FORCE_STAGING_RELAYS` set to any non empty value [`N0_DNS_PKARR_RELAY_STAGING`]
+    /// server is used instead.
     ///
     /// [number 0]: https://n0.computer
     pub fn n0_dns(secret_key: SecretKey) -> Self {
-        #[cfg(not(any(test, feature = "test-utils")))]
-        let pkarr_relay = N0_DNS_PKARR_RELAY_PROD;
-        #[cfg(any(test, feature = "test-utils"))]
-        let pkarr_relay = N0_DNS_PKARR_RELAY_STAGING;
+        let pkarr_relay = match force_staging_infra() {
+            true => N0_DNS_PKARR_RELAY_STAGING,
+            false => N0_DNS_PKARR_RELAY_PROD,
+        };
 
         let pkarr_relay: Url = pkarr_relay.parse().expect("url is valid");
         Self::new(secret_key, pkarr_relay)
@@ -315,16 +317,16 @@ impl PkarrResolver {
     /// This uses the pkarr relay server operated by [number 0] at
     /// [`N0_DNS_PKARR_RELAY_PROD`].
     ///
-    /// When compiling for tests, i.e. when `cfg(test)` is true, or when the
-    /// `test-utils` crate feature is enabled the [`N0_DNS_PKARR_RELAY_STAGING`] server is
-    /// used instead.
+    /// When running with the environment variable `IROH_FORCE_STAGING_RELAYS`
+    /// set to any non empty value [`N0_DNS_PKARR_RELAY_STAGING`]
+    /// server is used instead.
     ///
     /// [number 0]: https://n0.computer
     pub fn n0_dns() -> Self {
-        #[cfg(not(any(test, feature = "test-utils")))]
-        let pkarr_relay = N0_DNS_PKARR_RELAY_PROD;
-        #[cfg(any(test, feature = "test-utils"))]
-        let pkarr_relay = N0_DNS_PKARR_RELAY_STAGING;
+        let pkarr_relay = match force_staging_infra() {
+            true => N0_DNS_PKARR_RELAY_STAGING,
+            false => N0_DNS_PKARR_RELAY_PROD,
+        };
 
         let pkarr_relay: Url = pkarr_relay.parse().expect("url is valid");
         Self::new(pkarr_relay)

iroh-net/src/endpoint.rs

@@ -36,7 +36,7 @@ use crate::{
     dns::{default_resolver, DnsResolver},
     key::{PublicKey, SecretKey},
     magicsock::{self, Handle, QuicMappedAddr},
-    relay::{RelayMode, RelayUrl},
+    relay::{force_staging_infra, RelayMode, RelayUrl},
     tls, NodeId,
 };
 
@@ -61,11 +61,6 @@ pub use super::magicsock::{
 /// is still no connection the configured [`Discovery`] will be used however.
 const DISCOVERY_WAIT_PERIOD: Duration = Duration::from_millis(500);
 
-/// Environment variable to force the use of staging relays.
-#[cfg(not(any(test, feature = "test-utils")))]
-#[cfg_attr(iroh_docsrs, doc(cfg(not(any(test, feature = "test-utils")))))]
-const ENV_FORCE_STAGING_RELAYS: &str = "IROH_FORCE_STAGING_RELAYS";
-
 type DiscoveryBuilder = Box<dyn FnOnce(&SecretKey) -> Option<Box<dyn Discovery>> + Send + Sync>;
 
 /// Builder for [`Endpoint`].
@@ -1358,19 +1353,11 @@ fn proxy_url_from_env() -> Option<Url> {
 
 /// Returns the default relay mode.
 ///
-/// If the `IROH_FORCE_STAGING_RELAYS` environment variable is set to `1`, it will return `RelayMode::Staging`.
+/// If the `IROH_FORCE_STAGING_RELAYS` environment variable is non empty, it will return `RelayMode::Staging`.
 /// Otherwise, it will return `RelayMode::Default`.
 pub fn default_relay_mode() -> RelayMode {
     // Use staging in testing
-    #[cfg(not(any(test, feature = "test-utils")))]
-    let force_staging_relays = match std::env::var(ENV_FORCE_STAGING_RELAYS) {
-        Ok(value) => value == "1",
-        Err(_) => false,
-    };
-    #[cfg(any(test, feature = "test-utils"))]
-    let force_staging_relays = true;
-
-    match force_staging_relays {
+    match force_staging_infra() {
         true => RelayMode::Staging,
         false => RelayMode::Default,
     }

iroh-net/src/relay.rs

@@ -20,6 +20,15 @@ pub mod server;
 
 pub use iroh_base::node_addr::RelayUrl;
 
+/// Environment variable to force the use of staging relays.
+#[cfg_attr(iroh_docsrs, doc(cfg(not(test))))]
+pub const ENV_FORCE_STAGING_RELAYS: &str = "IROH_FORCE_STAGING_RELAYS";
+
+/// Returns `true` if the use of staging relays is forced.
+pub fn force_staging_infra() -> bool {
+    matches!(std::env::var(ENV_FORCE_STAGING_RELAYS), Ok(value) if !value.is_empty())
+}
+
 pub use self::{
     client::{
         conn::{Conn as RelayConn, ReceivedMessage},

iroh/src/node/builder.rs

@@ -23,7 +23,7 @@ use iroh_net::{
     discovery::{dns::DnsDiscovery, pkarr::PkarrPublisher, ConcurrentDiscovery, Discovery},
     dns::DnsResolver,
     endpoint::TransportConfig,
-    relay::RelayMode,
+    relay::{force_staging_infra, RelayMode},
     Endpoint,
 };
 use quic_rpc::transport::{boxed::BoxableServerEndpoint, quinn::QuinnServerEndpoint};
@@ -118,7 +118,6 @@ where
     node_discovery: DiscoveryConfig,
     docs_storage: DocsStorage,
     #[cfg(any(test, feature = "test-utils"))]
-    #[cfg_attr(iroh_docsrs, doc(cfg(any(test, feature = "test-utils"))))]
     insecure_skip_relay_cert_verify: bool,
     /// Callback to register when a gc loop is done
     #[debug("callback")]
@@ -229,10 +228,10 @@ fn mk_external_rpc() -> IrohServerEndpoint {
 impl Default for Builder<iroh_blobs::store::mem::Store> {
     fn default() -> Self {
         // Use staging in testing
-        #[cfg(not(any(test, feature = "test-utils")))]
-        let relay_mode = RelayMode::Default;
-        #[cfg(any(test, feature = "test-utils"))]
-        let relay_mode = RelayMode::Staging;
+        let relay_mode = match force_staging_infra() {
+            true => RelayMode::Staging,
+            false => RelayMode::Default,
+        };
 
         Self {
             storage: StorageConfig::Mem,
@@ -265,10 +264,10 @@ impl<D: Map> Builder<D> {
         storage: StorageConfig,
     ) -> Self {
         // Use staging in testing
-        #[cfg(not(any(test, feature = "test-utils")))]
-        let relay_mode = RelayMode::Default;
-        #[cfg(any(test, feature = "test-utils"))]
-        let relay_mode = RelayMode::Staging;
+        let relay_mode = match force_staging_infra() {
+            true => RelayMode::Staging,
+            false => RelayMode::Default,
+        };
 
         Self {
             storage,