fix(cli): load config in Ignore mode for auto-config-simulate

src/Cli.Tests/AutoConfigSimulateTests.cs

@@ -24,14 +24,61 @@ public class AutoConfigSimulateTests
         "Server=tcp:127.0.0.1,1433;Persist Security Info=False;User ID=sa;" +
         "Password=@env('MSSQL_SA_PASSWORD');MultipleActiveResultSets=False;Connection Timeout=30;";
 
+    /// <summary>
+    /// A fully resolved connection string containing no @env()/@akv() references. It points at a port
+    /// nothing listens on with a short timeout, so the tests that reach the query stage fail fast
+    /// without requiring a database.
+    /// </summary>
+    private const string MSSQL_RESOLVED_CONNECTION_STRING =
+        "Server=tcp:127.0.0.1,1;Persist Security Info=False;User ID=sa;" +
+        "Password=placeholder;TrustServerCertificate=True;Connect Timeout=1;";
+
+    /// <summary>
+    /// Name of an environment variable that is deliberately never set, used to produce an
+    /// unresolved @env() reference in a connection string.
+    /// </summary>
+    private const string UNSET_ENV_VAR_NAME = "DAB_TEST_UNSET_CONNECTION_SECRET";
+
+    /// <summary>
+    /// The OpenTelemetry environment variables that `dab init` always references from the generated
+    /// config. They are normally unset, which is the scenario covered by issue #3791.
+    /// </summary>
+    private static readonly string[] _openTelemetryEnvVarNames = new[]
+    {
+        "OTEL_EXPORTER_OTLP_ENDPOINT",
+        "OTEL_EXPORTER_OTLP_HEADERS",
+        "OTEL_SERVICE_NAME"
+    };
+
+    /// <summary>
+    /// Every environment variable these tests unset. The OpenTelemetry names are the real ones an
+    /// init-generated config references, so they may legitimately be set in the host environment.
+    /// </summary>
+    private static readonly string[] _mutatedEnvVarNames =
+        _openTelemetryEnvVarNames.Append(UNSET_ENV_VAR_NAME).ToArray();
+
     private IFileSystem? _fileSystem;
     private FileSystemRuntimeConfigLoader? _runtimeConfigLoader;
 
+    /// <summary>
+    /// Host values of <see cref="_mutatedEnvVarNames"/>, captured before each test clears them and
+    /// restored in cleanup. Without this, a cleared variable leaks into every test that runs later in
+    /// the same process, making unrelated tests fail depending on ordering and host environment.
+    /// </summary>
+    private readonly Dictionary<string, string?> _originalEnvVarValues = new();
+
     [TestInitialize]
     public void TestInitialize()
     {
+        foreach (string name in _mutatedEnvVarNames)
+        {
+            _originalEnvVarValues[name] = Environment.GetEnvironmentVariable(name);
+        }
+
         _fileSystem = FileSystemUtils.ProvisionMockFileSystem();
-        _runtimeConfigLoader = new FileSystemRuntimeConfigLoader(_fileSystem);
+        // isCliLoader mirrors how the CLI builds its loader. Without it a successful load starts a
+        // hot-reload file watcher against the mock file system, whose retries add seconds per test.
+        _runtimeConfigLoader = new FileSystemRuntimeConfigLoader(_fileSystem, isCliLoader: true);
 
         ILoggerFactory loggerFactory = TestLoggerSupport.ProvisionLoggerFactory();
         ConfigGenerator.SetLoggerForCliConfigGenerator(loggerFactory.CreateLogger<ConfigGenerator>());
@@ -41,27 +88,139 @@ public void TestInitialize()
     [TestCleanup]
     public void TestCleanup()
     {
+        foreach (KeyValuePair<string, string?> original in _originalEnvVarValues)
+        {
+            Environment.SetEnvironmentVariable(original.Key, original.Value);
+        }
+
+        _originalEnvVarValues.Clear();
         _fileSystem = null;
         _runtimeConfigLoader = null;
     }
 
     /// <summary>
     /// Tests that the simulate command fails when no autoentities are defined in the config.
+    /// The config is produced by `dab init`, which always writes unset OpenTelemetry @env()
+    /// placeholders, so asserting on the specific error also proves the command reached the
+    /// autoentities check rather than aborting during the config load.
     /// </summary>
     [TestMethod]
     public void TestSimulateAutoentities_NoAutoentitiesDefined()
     {
         // Arrange: create an MSSQL config without autoentities
-        InitOptions initOptions = CreateBasicInitOptionsForMsSqlWithConfig(config: TEST_RUNTIME_CONFIG_FILE);
+        ClearOpenTelemetryEnvironmentVariables();
+        InitOptions initOptions = CreateInitOptionsForMsSql(MSSQL_RESOLVED_CONNECTION_STRING);
+        Assert.IsTrue(TryGenerateConfig(initOptions, _runtimeConfigLoader!, _fileSystem!));
+
+        Mock<ILogger<ConfigGenerator>> loggerMock = new();
+        SetLoggerForCliConfigGenerator(loggerMock.Object);
+
+        AutoConfigSimulateOptions options = new(config: TEST_RUNTIME_CONFIG_FILE);
+
+        // Act
+        bool success = TrySimulateAutoentities(options, _runtimeConfigLoader!, _fileSystem!);
+
+        // Assert
+        Assert.IsFalse(success);
+        AssertErrorLogged(loggerMock, "No autoentities definitions found in the config file.");
+    }
+
+    /// <summary>
+    /// Regression test for https://github.com/Azure/data-api-builder/issues/3791.
+    /// A config generated by `dab init` references OpenTelemetry environment variables that are
+    /// normally unset. Those unresolved @env() references must not abort the config load, so the
+    /// command proceeds all the way to the database query stage.
+    /// </summary>
+    [TestMethod]
+    public void TestSimulateAutoentities_UnsetTelemetryEnvVars_DoesNotBlockConfigLoad()
+    {
+        // Arrange: an init-generated config (unset OpenTelemetry @env() placeholders) with an autoentity.
+        ClearOpenTelemetryEnvironmentVariables();
+        InitOptions initOptions = CreateInitOptionsForMsSql(MSSQL_RESOLVED_CONNECTION_STRING);
+        Assert.IsTrue(TryGenerateConfig(initOptions, _runtimeConfigLoader!, _fileSystem!));
+
+        AutoConfigOptions autoConfigOptions = new(
+            definitionName: "books-filter",
+            patternsInclude: new[] { "dbo.books" },
+            config: TEST_RUNTIME_CONFIG_FILE);
+        Assert.IsTrue(ConfigGenerator.TryConfigureAutoentities(autoConfigOptions, _runtimeConfigLoader!, _fileSystem!));
+
+        Mock<ILogger<ConfigGenerator>> loggerMock = new();
+        SetLoggerForCliConfigGenerator(loggerMock.Object);
+
+        AutoConfigSimulateOptions options = new(config: TEST_RUNTIME_CONFIG_FILE);
+
+        // Act
+        bool success = TrySimulateAutoentities(options, _runtimeConfigLoader!, _fileSystem!);
+
+        // Assert: the run fails only because no database is listening, which means the config load,
+        // the database type check, the autoentities check and the connection string checks all passed.
+        Assert.IsFalse(success, "No database is listening, so the simulation cannot succeed.");
+        AssertErrorLogged(loggerMock, "Failed to query the database");
+        AssertErrorNotLogged(loggerMock, "Failed to read the config file");
+        AssertErrorNotLogged(loggerMock, "No autoentities definitions found");
+    }
+
+    /// <summary>
+    /// A user-provided config path is not checked for existence before the load (see
+    /// TryGetConfigFileBasedOnCliPrecedence), so a missing file reaches TryLoadConfig, which logs
+    /// "Unable to find config file". Draining the loader's buffer now delivers that error, so the
+    /// generic fallback must stay silent rather than reporting the same failure a second time.
+    /// </summary>
+    [TestMethod]
+    public void TestSimulateAutoentities_MissingConfigFile_DoesNotLogGenericError()
+    {
+        // Arrange: a config path that was never written to the mock file system.
+        const string MISSING_CONFIG_FILE = "dab-config.missing.json";
+        Assert.IsFalse(_fileSystem!.File.Exists(MISSING_CONFIG_FILE), "The test config file must not exist.");
+
+        Mock<ILogger<ConfigGenerator>> loggerMock = new();
+        SetLoggerForCliConfigGenerator(loggerMock.Object);
+
+        AutoConfigSimulateOptions options = new(config: MISSING_CONFIG_FILE);
+
+        // Act
+        bool success = TrySimulateAutoentities(options, _runtimeConfigLoader!, _fileSystem!);
+
+        // Assert: the loader already reported the missing file, so no duplicate generic error.
+        Assert.IsFalse(success);
+        AssertErrorNotLogged(loggerMock, "Failed to read the config file");
+    }
+
+    /// <summary>
+    /// Tests that an @env() reference which could not be resolved is rejected with an actionable
+    /// message instead of being sent to the database as a literal. Unresolved references survive the
+    /// config load because it runs in Ignore mode, so this check is what catches them.
+    /// </summary>
+    [TestMethod]
+    public void TestSimulateAutoentities_UnresolvedEnvVarInConnectionString_Fails()
+    {
+        // Arrange: a config whose connection string references an environment variable that is not set.
+        ClearOpenTelemetryEnvironmentVariables();
+        Environment.SetEnvironmentVariable(UNSET_ENV_VAR_NAME, null);
+
+        InitOptions initOptions = CreateInitOptionsForMsSql(
+            "Server=tcp:127.0.0.1,1;User ID=sa;Password=@env('" + UNSET_ENV_VAR_NAME + "');Connect Timeout=1;");
         Assert.IsTrue(TryGenerateConfig(initOptions, _runtimeConfigLoader!, _fileSystem!));
 
+        AutoConfigOptions autoConfigOptions = new(
+            definitionName: "books-filter",
+            patternsInclude: new[] { "dbo.books" },
+            config: TEST_RUNTIME_CONFIG_FILE);
+        Assert.IsTrue(ConfigGenerator.TryConfigureAutoentities(autoConfigOptions, _runtimeConfigLoader!, _fileSystem!));
+
+        Mock<ILogger<ConfigGenerator>> loggerMock = new();
+        SetLoggerForCliConfigGenerator(loggerMock.Object);
+
         AutoConfigSimulateOptions options = new(config: TEST_RUNTIME_CONFIG_FILE);
 
         // Act
         bool success = TrySimulateAutoentities(options, _runtimeConfigLoader!, _fileSystem!);
 
         // Assert
         Assert.IsFalse(success);
+        AssertErrorLogged(loggerMock, "unresolved @env() or @akv() reference");
+        AssertErrorNotLogged(loggerMock, "Failed to query the database");
     }
 
     /// <summary>
@@ -236,4 +395,71 @@ public void TestSimulateAutoentities_WithNonMatchingFilter_OutputsNoMatches()
         StringAssert.Contains(output, "Matches: 0", "Output should show zero matches.");
         StringAssert.Contains(output, "(no matches)", "Output should show the 'no matches' message.");
     }
+
+    /// <summary>
+    /// Creates the init options used to generate an MSSQL config with the given connection string.
+    /// </summary>
+    /// <param name="connectionString">The connection string written to the generated config.</param>
+    private static InitOptions CreateInitOptionsForMsSql(string connectionString)
+    {
+        return new(
+            databaseType: DatabaseType.MSSQL,
+            connectionString: connectionString,
+            cosmosNoSqlDatabase: null,
+            cosmosNoSqlContainer: null,
+            graphQLSchemaPath: null,
+            setSessionContext: false,
+            hostMode: HostMode.Development,
+            corsOrigin: new List<string>(),
+            authenticationProvider: EasyAuthType.AppService.ToString(),
+            config: TEST_RUNTIME_CONFIG_FILE);
+    }
+
+    /// <summary>
+    /// Unsets the OpenTelemetry environment variables referenced by an init-generated config so the
+    /// tests deterministically exercise the unresolved @env() scenario.
+    /// </summary>
+    private static void ClearOpenTelemetryEnvironmentVariables()
+    {
+        foreach (string name in _openTelemetryEnvVarNames)
+        {
+            Environment.SetEnvironmentVariable(name, null);
+        }
+    }
+
+    /// <summary>
+    /// Asserts that an error containing the given fragment was logged exactly once.
+    /// </summary>
+    /// <param name="loggerMock">The mocked logger the command wrote to.</param>
+    /// <param name="expectedMessageFragment">Fragment expected in the logged error message.</param>
+    private static void AssertErrorLogged(Mock<ILogger<ConfigGenerator>> loggerMock, string expectedMessageFragment)
+    {
+        loggerMock.Verify(
+            x => x.Log(
+                LogLevel.Error,
+                It.IsAny<EventId>(),
+                It.Is<It.IsAnyType>((o, t) => o.ToString()!.Contains(expectedMessageFragment)),
+                It.IsAny<Exception?>(),
+                (Func<It.IsAnyType, Exception?, string>)It.IsAny<object>()),
+            Times.Once,
+            $"Expected an error containing '{expectedMessageFragment}' to be logged.");
+    }
+
+    /// <summary>
+    /// Asserts that no error containing the given fragment was logged.
+    /// </summary>
+    /// <param name="loggerMock">The mocked logger the command wrote to.</param>
+    /// <param name="unexpectedMessageFragment">Fragment that must not appear in any logged error.</param>
+    private static void AssertErrorNotLogged(Mock<ILogger<ConfigGenerator>> loggerMock, string unexpectedMessageFragment)
+    {
+        loggerMock.Verify(
+            x => x.Log(
+                LogLevel.Error,
+                It.IsAny<EventId>(),
+                It.Is<It.IsAnyType>((o, t) => o.ToString()!.Contains(unexpectedMessageFragment)),
+                It.IsAny<Exception?>(),
+                (Func<It.IsAnyType, Exception?, string>)It.IsAny<object>()),
+            Times.Never,
+            $"Did not expect an error containing '{unexpectedMessageFragment}' to be logged.");
+    }
 }

src/Cli/ConfigGenerator.cs

@@ -3819,11 +3819,32 @@ public static bool TrySimulateAutoentities(AutoConfigSimulateOptions options, Fi
                 return false;
             }
 
-            // Load config with env var replacement so the connection string is fully resolved.
-            DeserializationVariableReplacementSettings replacementSettings = new(doReplaceEnvVar: true);
+            // Load config with env var replacement so the connection string is resolved where possible.
+            // Unresolved @env() references must NOT abort the load: this is deliberately Ignore, matching
+            // the engine, which only ever loads through TryLoadKnownConfig (also Ignore). A config produced
+            // by `dab init` always carries OpenTelemetry @env() placeholders that are typically unset, so
+            // Throw here would reject configs that `dab start` accepts and runs. Placeholders that survive
+            // into the connection string are caught by the explicit check below.
+            DeserializationVariableReplacementSettings replacementSettings = new(doReplaceEnvVar: true, envFailureMode: EnvironmentVariableReplacementFailureMode.Ignore);
             if (!loader.TryLoadConfig(runtimeConfigFile, out RuntimeConfig? runtimeConfig, replacementSettings: replacementSettings))
             {
-                _logger.LogError("Failed to read the config file: {runtimeConfigFile}.", runtimeConfigFile);
+                // The loader buffers its logs until a logger is attached, and only the `start` verb attaches
+                // one. Attach and drain here so the underlying parse failure reaches the user instead of
+                // being dropped along with the generic message below.
+                loader.SetLogger(LoggerFactoryForCli.CreateLogger<FileSystemRuntimeConfigLoader>());
+                loader.FlushLogBuffer();
+
+                // The loader explains the two failures it can attribute: a parse error (flagged by
+                // IsParseErrorEmitted) and a missing file (TryGetConfigFileBasedOnCliPrecedence does not
+                // check that a user-provided path exists, so it reaches the loader). The flush above has
+                // now delivered whichever it logged, so emit the generic fallback only for the case the
+                // loader stays silent on - a config file that exists but is empty - rather than reporting
+                // the same failure twice.
+                if (!loader.IsParseErrorEmitted && fileSystem.File.Exists(runtimeConfigFile))
+                {
+                    _logger.LogError("Failed to read the config file: {runtimeConfigFile}.", runtimeConfigFile);
+                }
+
                 return false;
             }
 
@@ -3846,6 +3867,17 @@ public static bool TrySimulateAutoentities(AutoConfigSimulateOptions options, Fi
                 return false;
             }
 
+            // The config is loaded in Ignore mode, so an @env()/@akv() reference that was not resolved is
+            // left in place verbatim rather than failing the load. Such a placeholder would be sent to the
+            // database as a literal, producing a confusing connection error, so reject it here instead.
+            if (connectionString.Contains("@env('", StringComparison.Ordinal) || connectionString.Contains("@akv('", StringComparison.Ordinal))
+            {
+                _logger.LogError(
+                    "The connection string in the config file contains an unresolved @env() or @akv() reference. " +
+                    "Set the referenced environment variable (or provide it in a .env file) before running the simulation.");
+                return false;
+            }
+
             MsSqlQueryBuilder queryBuilder = new();
             string query = queryBuilder.BuildGetAutoentitiesQuery();