Skip to content

[ANCM] Add switch to prefer env over web.config #19746

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.

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

Merged
merged 3 commits into from
Mar 11, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
#define ASPNETCORE_IIS_AUTH_BASIC L"basic;"
#define ASPNETCORE_IIS_AUTH_ANONYMOUS L"anonymous;"
#define ASPNETCORE_IIS_AUTH_NONE L"none"
#define ANCM_PREFER_ENVIRONMENT_VARIABLES_ENV_STR L"ANCM_PREFER_ENVIRONMENT_VARIABLES"
Copy link
Member

Choose a reason for hiding this comment

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

Why ANCM instead of ASPNETCORE_ANCM like in ASPNETCORE_ANCM_HTTPS_PORT?

Copy link
Member Author

Choose a reason for hiding this comment

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

🤷‍♂ @jkotalik @anurse
Let the naming bikeshed commence!

Copy link
Contributor

Choose a reason for hiding this comment

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

This environment variable is only used by ANCM. All other environment variables are eventually used by managed code, while this is exclusively used by ANCM.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm fine with this name for now.

Copy link
Contributor

@analogrelay analogrelay Mar 11, 2020

Choose a reason for hiding this comment

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

We use ANCM for other variables. For example ANCM_HTTPS_PORT.

Seems reasonable to keep this name consistent.

Copy link
Contributor

@analogrelay analogrelay Mar 11, 2020

Choose a reason for hiding this comment

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

Why ANCM instead of ASPNETCORE_ANCM like in ASPNETCORE_ANCM_HTTPS_PORT?

Ah, and the reason for the ASPNETCORE_ prefix in ANCM_HTTPS_PORT is the fact that it's read out of config. This is directly read as an env var. I could go either way I suppose 🤷‍♀


//
// The key used for hash-table lookups, consists of the port on which the http process is created.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,9 +78,27 @@ class ENVIRONMENT_VAR_HELPERS
environmentVariables.insert_or_assign(HOSTING_STARTUP_ASSEMBLIES_ENV_STR, hostingStartupValues);
}

auto preferEnvironmentVariablesSetting = Environment::GetEnvironmentVariableValue(ANCM_PREFER_ENVIRONMENT_VARIABLES_ENV_STR).value_or(L"false");
auto preferEnvironmentVariables = equals_ignore_case(L"1", preferEnvironmentVariablesSetting) || equals_ignore_case(L"true", preferEnvironmentVariablesSetting);

for (auto& environmentVariable : environmentVariables)
{
environmentVariable.second = Environment::ExpandEnvironmentVariables(environmentVariable.second);
if (preferEnvironmentVariables)
{
auto env = Environment::GetEnvironmentVariableValue(environmentVariable.first);
if (env.has_value())
{
environmentVariable.second = env.value();
}
else
{
environmentVariable.second = Environment::ExpandEnvironmentVariables(environmentVariable.second);
}
}
else
{
environmentVariable.second = Environment::ExpandEnvironmentVariables(environmentVariable.second);
}
}

return environmentVariables;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,5 +126,22 @@ private async Task WebConfigExpandsVariables(HostingModel hostingModel)
deploymentParameters.WebConfigBasedEnvironmentVariables["OtherVariable"] = "%TestVariable%;Hello";
Assert.Equal("World;Hello", await GetStringAsync(deploymentParameters, "/GetEnvironmentVariable?name=OtherVariable"));
}

[ConditionalTheory]
[RequiresIIS(IISCapability.PoolEnvironmentVariables)]
[RequiresNewHandler]
[RequiresNewShim]
[InlineData(HostingModel.InProcess)]
[InlineData(HostingModel.OutOfProcess)]
public async Task PreferEnvironmentVariablesOverWebConfigWhenConfigured(HostingModel hostingModel)
{
var deploymentParameters = Fixture.GetBaseDeploymentParameters(hostingModel);

var environment = "Development";
deploymentParameters.EnvironmentVariables["ANCM_PREFER_ENVIRONMENT_VARIABLES"] = "true";
deploymentParameters.EnvironmentVariables["ASPNETCORE_ENVIRONMENT"] = environment;
deploymentParameters.WebConfigBasedEnvironmentVariables.Add("ASPNETCORE_ENVIRONMENT", "Debug");
Assert.Equal(environment, await GetStringAsync(deploymentParameters, "/GetEnvironmentVariable?name=ASPNETCORE_ENVIRONMENT"));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,11 @@ static RequiresIISAttribute()

var ancmConfigPath = Path.Combine(Environment.SystemDirectory, "inetsrv", "config", "schema", "aspnetcore_schema.xml");

if (!File.Exists(ancmConfigPath))
{
ancmConfigPath = Path.Combine(Environment.SystemDirectory, "inetsrv", "config", "schema", "aspnetcore_schema_v2.xml");
}

if (!File.Exists(ancmConfigPath) && !SkipInVSTSAttribute.RunningInVSTS)
{
_skipReasonStatic = "IIS Schema is not installed.";
Expand Down