Skip to content

ensure that the environment variables collection is never null #1959

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
benPearce1 wants to merge 5 commits into
base: main
Choose a base branch
from
+73 −4
Open

ensure that the environment variables collection is never null #1959

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
03d19b0
ensure that the environment variables collection is never null
benPearce1 May 22, 2026
e87494d
fix dependency issue on test class
benPearce1 May 22, 2026
7af64c4
include capturing wrapper in script wrapper chain
benPearce1 May 25, 2026
79c89e2
test fix
benPearce1 May 25, 2026
579434f
Merge branch 'main' into benpearce/fd-392-sev-2-medium-helm-deploymen…
benPearce1 May 25, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions source/Calamari.Common/Features/Scripting/ScriptEngine.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
using Calamari.Common.Features.Scripting.Python;
using Calamari.Common.Features.Scripting.WindowsPowerShell;
using Calamari.Common.Features.Scripts;
using Calamari.Common.FeatureToggles;
using Calamari.Common.Plumbing.Extensions;
using Calamari.Common.Plumbing.Logging;
using Calamari.Common.Plumbing.Variables;
Expand All @@ -18,11 +17,15 @@ public interface IScriptEngine
{
ScriptSyntax[] GetSupportedTypes();

CommandResult Execute(Script script,
IVariables variables,
ICommandLineRunner commandLineRunner);

CommandResult Execute(
Script script,
IVariables variables,
ICommandLineRunner commandLineRunner,
Dictionary<string, string>? environmentVars = null);
Dictionary<string, string> environmentVars);
}

public class ScriptEngine : IScriptEngine
Expand All @@ -41,11 +44,13 @@ public ScriptSyntax[] GetSupportedTypes()
return ScriptSyntaxHelper.GetPreferenceOrderedScriptSyntaxesForEnvironment();
}

public CommandResult Execute(Script script, IVariables variables, ICommandLineRunner commandLineRunner) => Execute(script, variables, commandLineRunner, []);
Copy link
Copy Markdown
Contributor Author

@benPearce1 benPearce1 May 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Workaround the fact that you cannot set a default value to anything other than null


public CommandResult Execute(
Script script,
IVariables variables,
ICommandLineRunner commandLineRunner,
Dictionary<string, string>? environmentVars = null)
Dictionary<string, string> environmentVars)
{
var syntax = script.File.ToScriptType();
return BuildWrapperChain(syntax, variables, commandLineRunner)
Expand Down
66 changes: 65 additions & 1 deletion source/Calamari.Tests/Fixtures/Integration/Scripting/ScriptEngineFixture.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
using System;
using System.Collections.Generic;
using Calamari.Common.Features.Processes;
using Calamari.Common.Features.Scripting;
using Calamari.Common.Features.Scripting.DotnetScript;
using Calamari.Common.Features.Scripts;
using Calamari.Common.Plumbing.Logging;
using Calamari.Common.Plumbing.Variables;
using Calamari.Testing.Helpers;
using FluentAssertions;
using NSubstitute;
Expand All @@ -30,6 +31,13 @@ public class ScriptEngineFixture
ScriptSyntax.Bash
};

ICommandLineRunner runner;
public ScriptEngineFixture()
{
runner = Substitute.For<ICommandLineRunner>();
runner.Execute(Arg.Any<CommandLineInvocation>()).Returns(new CommandResult("foo", 0));
}

[Test]
[Category(TestCategory.CompatibleOS.OnlyWindows)]
public void DeterminesCorrectScriptTypePreferenceOrderWindows()
Expand All @@ -44,6 +52,62 @@ public void DeterminesCorrectScriptTypePreferencesOrderNonWindows()
DeterminesCorrectScriptTypePreferenceOrder(ScriptPreferencesNonWindows);
}

[Test]
public void ExecuteWithoutEnvironmentVarsPassesEmptyDictionaryToWrappers()
{
Dictionary<string, string> receivedEnvironmentVars = null;
var capturingWrapper = new CapturingScriptWrapper()
{
OnExecute = envVars => receivedEnvironmentVars = envVars
};

var engine = new ScriptEngine(new List<IScriptWrapper> { capturingWrapper }, Substitute.For<ILog>());
var variables = new CalamariVariables();

engine.Execute(new Script("test.ps1"), variables, runner);

receivedEnvironmentVars.Should().NotBeNull();
receivedEnvironmentVars.Should().BeEmpty();
}

[Test]
public void ExecuteWithEnvironmentVarsPassesDictionaryToWrappers()
{
Dictionary<string, string> receivedEnvironmentVars = null;
var capturingWrapper = new CapturingScriptWrapper()
{
OnExecute = envVars => receivedEnvironmentVars = envVars
};

var engine = new ScriptEngine(new List<IScriptWrapper> { capturingWrapper }, Substitute.For<ILog>());
var variables = new CalamariVariables();
var envVars = new Dictionary<string, string> { { "AWS_REGION", "us-east-1" } };

engine.Execute(new Script("test.ps1"), variables, runner, envVars);

receivedEnvironmentVars.Should().NotBeNull();
receivedEnvironmentVars.Should().ContainKey("AWS_REGION");
}

/// <summary>
/// A wrapper that captures the environmentVars it receives, to verify they are never null.
/// </summary>
class CapturingScriptWrapper : IScriptWrapper
{
public Action<Dictionary<string, string>> OnExecute { get; set; }

public int Priority => ScriptWrapperPriorities.ToolConfigPriority;
public IScriptWrapper NextWrapper { get; set; }

public bool IsEnabled(ScriptSyntax syntax) => true;

public CommandResult ExecuteScript(Script script, ScriptSyntax scriptSyntax, ICommandLineRunner commandLineRunner, Dictionary<string, string> environmentVars)
{
OnExecute?.Invoke(environmentVars);
return NextWrapper?.ExecuteScript(script, scriptSyntax, commandLineRunner, environmentVars) ?? new CommandResult("captured", 0);
}
}

void DeterminesCorrectScriptTypePreferenceOrder(IEnumerable<ScriptSyntax> expected)
{
var engine = new ScriptEngine(new List<IScriptWrapper>(), Substitute.For<ILog>());
Expand Down