Skip to content
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

Allow winget configure from https location and extend winget configure validate for winget resource units #3833

Merged
merged 22 commits into from
Dec 14, 2023
Merged
Show file tree
Hide file tree
Changes from 21 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
2 changes: 1 addition & 1 deletion azure-pipelines.yml
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ jobs:
- template: templates/e2e-setup.yml
parameters:
sourceDir: $(Build.SourcesDirectory)
localhostWebServerArgs: '-BuildRoot $(buildOutDir)\LocalhostWebServer -StaticFileRoot $(Agent.TempDirectory)\TestLocalIndex -LocalSourceJson $(Build.SourcesDirectory)\src\AppInstallerCLIE2ETests\TestData\localsource.json -SourceCert $(Build.SourcesDirectory)\src\AppInstallerCLIE2ETests\TestData\AppInstallerTest.cer'
localhostWebServerArgs: '-BuildRoot $(buildOutDir)\LocalhostWebServer -StaticFileRoot $(Agent.TempDirectory)\TestLocalIndex -LocalSourceJson $(Build.SourcesDirectory)\src\AppInstallerCLIE2ETests\TestData\localsource.json -TestDataPath $(Build.SourcesDirectory)\src\AppInstallerCLIE2ETests\TestData -SourceCert $(Build.SourcesDirectory)\src\AppInstallerCLIE2ETests\TestData\AppInstallerTest.cer'

- template: templates/e2e-test.template.yml
parameters:
Expand Down
1 change: 1 addition & 0 deletions src/AppInstallerCLI.sln
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@ Project("{8BC9CEB8-8B4A-11D0-8D11-00A0C91BC942}") = "Detours", "Xlang\UndockedRe
EndProject
Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "templates", "templates", "{8E43F982-40D5-4DF1-9044-C08047B5F43B}"
ProjectSection(SolutionItems) = preProject
..\templates\e2e-setup.yml = ..\templates\e2e-setup.yml
..\templates\e2e-test.template.yml = ..\templates\e2e-test.template.yml
EndProjectSection
EndProject
Expand Down
2 changes: 2 additions & 0 deletions src/AppInstallerCLICore/AppInstallerCLICore.vcxproj
Original file line number Diff line number Diff line change
Expand Up @@ -382,6 +382,7 @@
<ClInclude Include="CompletionData.h" />
<ClInclude Include="ConfigurationCommon.h" />
<ClInclude Include="ConfigurationContext.h" />
<ClInclude Include="ConfigurationWingetDscModuleUnitValidation.h" />
<ClInclude Include="ContextOrchestrator.h" />
<ClInclude Include="COMContext.h" />
<ClInclude Include="Public\ConfigurationSetProcessorFactoryRemoting.h" />
Expand Down Expand Up @@ -440,6 +441,7 @@
<ClCompile Include="ConfigurationCommon.cpp" />
<ClCompile Include="ConfigurationContext.cpp" />
<ClCompile Include="ConfigurationSetProcessorFactoryRemoting.cpp" />
<ClCompile Include="ConfigurationWingetDscModuleUnitValidation.cpp" />
<ClCompile Include="ContextOrchestrator.cpp" />
<ClCompile Include="Workflows\ConfigurationFlow.cpp" />
<ClCompile Include="Workflows\DependenciesFlow.cpp" />
Expand Down
2 changes: 1 addition & 1 deletion src/AppInstallerCLICore/Commands/ConfigureCommand.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ namespace AppInstaller::CLI
{
context <<
VerifyIsFullPackage <<
VerifyFile(Execution::Args::Type::ConfigurationFile) <<
VerifyFileOrUri(Execution::Args::Type::ConfigurationFile) <<
CreateConfigurationProcessor <<
OpenConfigurationSet <<
ShowConfigurationSet <<
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ namespace AppInstaller::CLI
void ConfigureShowCommand::ExecuteInternal(Execution::Context& context) const
{
context <<
VerifyFile(Execution::Args::Type::ConfigurationFile) <<
VerifyFileOrUri(Execution::Args::Type::ConfigurationFile) <<
CreateConfigurationProcessor <<
OpenConfigurationSet <<
ShowConfigurationSet;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ namespace AppInstaller::CLI
{
context <<
VerifyIsFullPackage <<
VerifyFile(Execution::Args::Type::ConfigurationFile) <<
VerifyFileOrUri(Execution::Args::Type::ConfigurationFile) <<
CreateConfigurationProcessor <<
OpenConfigurationSet <<
ShowConfigurationSet <<
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,12 @@ namespace AppInstaller::CLI
{
context <<
VerifyIsFullPackage <<
VerifyFile(Execution::Args::Type::ConfigurationFile) <<
VerifyFileOrUri(Execution::Args::Type::ConfigurationFile) <<
CreateConfigurationProcessor <<
OpenConfigurationSet <<
ValidateConfigurationSetSemantics <<
ValidateConfigurationSetUnitProcessors <<
ValidateConfigurationSetUnitContents <<
ValidateAllGoodMessage;
}

Expand Down

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.
#pragma once
#include <map>
#include <string>

namespace winrt::Microsoft::Management::Configuration
{
struct ConfigurationUnit;
}

namespace AppInstaller::CLI::Execution
{
struct Context;
}

namespace AppInstaller::CLI::Configuration
{
using namespace std::string_view_literals;

struct WingetDscModuleUnitValidator
{
bool ValidateConfigurationSetUnit(AppInstaller::CLI::Execution::Context& context, const winrt::Microsoft::Management::Configuration::ConfigurationUnit& unit);

std::string_view ModuleName() { return "Microsoft.WinGet.DSC"sv; };

private:
std::map<std::string, std::string> m_dependenciesSourceAndUnitIdMap;
};
}
16 changes: 16 additions & 0 deletions src/AppInstallerCLICore/Resources.h
Original file line number Diff line number Diff line change
Expand Up @@ -583,6 +583,7 @@ namespace AppInstaller::CLI::Resource
WINGET_DEFINE_RESOURCE_STRINGID(UpgradeRequireExplicitCount);
WINGET_DEFINE_RESOURCE_STRINGID(UpgradeUnknownVersionCount);
WINGET_DEFINE_RESOURCE_STRINGID(UpgradeUnknownVersionExplanation);
WINGET_DEFINE_RESOURCE_STRINGID(UriSchemeNotSupported);
WINGET_DEFINE_RESOURCE_STRINGID(Usage);
WINGET_DEFINE_RESOURCE_STRINGID(UserSettings);
WINGET_DEFINE_RESOURCE_STRINGID(ValidateCommandLongDescription);
Expand All @@ -603,6 +604,21 @@ namespace AppInstaller::CLI::Resource
WINGET_DEFINE_RESOURCE_STRINGID(WindowsPackageManager);
WINGET_DEFINE_RESOURCE_STRINGID(WindowsPackageManagerPreview);
WINGET_DEFINE_RESOURCE_STRINGID(WindowsStoreTerms);
WINGET_DEFINE_RESOURCE_STRINGID(WinGetResourceUnitBothPackageVersionAndUseLatest);
WINGET_DEFINE_RESOURCE_STRINGID(WinGetResourceUnitDependencySourceNotConfigured);
WINGET_DEFINE_RESOURCE_STRINGID(WinGetResourceUnitDependencySourceNotDeclaredAsDependency);
WINGET_DEFINE_RESOURCE_STRINGID(WinGetResourceUnitEmptyContent);
WINGET_DEFINE_RESOURCE_STRINGID(WinGetResourceUnitFailedToValidatePackage);
WINGET_DEFINE_RESOURCE_STRINGID(WinGetResourceUnitFailedToValidatePackageMultipleFound);
WINGET_DEFINE_RESOURCE_STRINGID(WinGetResourceUnitFailedToValidatePackageNotFound);
WINGET_DEFINE_RESOURCE_STRINGID(WinGetResourceUnitFailedToValidatePackageSourceOpenFailed);
WINGET_DEFINE_RESOURCE_STRINGID(WinGetResourceUnitFailedToValidatePackageVersionNotFound);
WINGET_DEFINE_RESOURCE_STRINGID(WinGetResourceUnitKnownSourceConfliction);
WINGET_DEFINE_RESOURCE_STRINGID(WinGetResourceUnitMissingRecommendedArg);
WINGET_DEFINE_RESOURCE_STRINGID(WinGetResourceUnitMissingRequiredArg);
WINGET_DEFINE_RESOURCE_STRINGID(WinGetResourceUnitPackageVersionSpecifiedWithOnlyOnePackageVersion);
WINGET_DEFINE_RESOURCE_STRINGID(WinGetResourceUnitThirdPartySourceAssertion);
WINGET_DEFINE_RESOURCE_STRINGID(WinGetResourceUnitThirdPartySourceAssertionForPackage);
WINGET_DEFINE_RESOURCE_STRINGID(WordArgumentDescription);
};

Expand Down
127 changes: 117 additions & 10 deletions src/AppInstallerCLICore/Workflows/ConfigurationFlow.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,13 @@
#include "ConfigurationFlow.h"
#include "PromptFlow.h"
#include "Public/ConfigurationSetProcessorFactoryRemoting.h"
#include <AppInstallerDownloader.h>
#include <AppInstallerErrors.h>
#include <AppInstallerStrings.h>
#include <winrt/Microsoft.Management.Configuration.h>
#include <winget/SelfManagement.h>
#include "ConfigurationCommon.h"
#include "ConfigurationWingetDscModuleUnitValidation.h"

using namespace AppInstaller::CLI::Execution;
using namespace winrt::Microsoft::Management::Configuration;
Expand Down Expand Up @@ -772,10 +775,52 @@ namespace AppInstaller::CLI::Workflow
bool m_isFirstProgress = true;
};

std::filesystem::path GetConfigurationFilePath(Execution::Context& context)
std::string GetNormalizedIdentifier(const winrt::hstring& identifier)
{
std::filesystem::path argPath = Utility::ConvertToUTF16(context.Args.GetArg(Args::Type::ConfigurationFile));
return std::filesystem::weakly_canonical(argPath);
return Utility::FoldCase(Utility::NormalizedString{ identifier });
}

// Get unit validation order. Make sure dependency units are before units depending on them.
std::vector<uint32_t> GetConfigurationSetUnitValidationOrder(winrt::Windows::Foundation::Collections::IVectorView<ConfigurationUnit> units)
{
// Create id to index map for easier processing.
std::map<std::string, uint32_t> idToUnitIndex;
for (uint32_t i = 0; i < units.Size(); ++i)
{
auto id = GetNormalizedIdentifier(units.GetAt(i).Identifier());
if (!id.empty())
{
idToUnitIndex.emplace(std::move(id), i);
}
}

// We do not need to worry about duplicate id, missing dependency or loops
// since dependency integrity is already validated in earlier semantic checks.

std::vector<uint32_t> validationOrder;

std::function<void(const ConfigurationUnit&, uint32_t)> addUnitToValidationOrder =
[&](const ConfigurationUnit& unit, uint32_t index)
{
if (std::find(validationOrder.begin(), validationOrder.end(), index) == validationOrder.end())
{
for (auto const& dependencyId : unit.Dependencies())
{
auto dependencyIndex = idToUnitIndex.find(GetNormalizedIdentifier(dependencyId))->second;
addUnitToValidationOrder(units.GetAt(dependencyIndex), dependencyIndex);
}
validationOrder.emplace_back(index);
}
};

for (uint32_t i = 0; i < units.Size(); ++i)
{
addUnitToValidationOrder(units.GetAt(i), i);
}

THROW_HR_IF(E_UNEXPECTED, units.Size() != validationOrder.size());

return validationOrder;
}
}

Expand Down Expand Up @@ -811,10 +856,32 @@ namespace AppInstaller::CLI::Workflow
auto progressScope = context.Reporter.BeginAsyncProgress(true);
progressScope->Callback().SetProgressMessage(Resource::String::ConfigurationReadingConfigFile());

std::filesystem::path absolutePath = GetConfigurationFilePath(context);

std::string argPath{ context.Args.GetArg(Args::Type::ConfigurationFile) };
std::wstring argPathWide = Utility::ConvertToUTF16(argPath);
bool isRemote = Utility::IsUrlRemote(argPath);
std::filesystem::path absolutePath;
Streams::IInputStream inputStream = nullptr;

if (isRemote)
{
std::ostringstream stringStream;
ProgressCallback emptyCallback;
Utility::DownloadToStream(argPath, stringStream, Utility::DownloadType::ConfigurationFile, emptyCallback);

auto strContent = stringStream.str();
std::vector<BYTE> byteContent{ strContent.begin(), strContent.end() };

Streams::InMemoryRandomAccessStream memoryStream;
Streams::DataWriter streamWriter{ memoryStream };
streamWriter.WriteBytes(byteContent);
streamWriter.StoreAsync().get();
streamWriter.DetachStream();
memoryStream.Seek(0);
inputStream = memoryStream;
}
else
{
absolutePath = std::filesystem::weakly_canonical(std::filesystem::path{ argPathWide });
auto openAction = Streams::FileRandomAccessStream::OpenAsync(absolutePath.wstring(), FileAccessMode::Read);
auto cancellationScope = progressScope->Callback().SetCancellationFunction([&]() { openAction.Cancel(); });
inputStream = openAction.get();
Expand All @@ -831,7 +898,7 @@ namespace AppInstaller::CLI::Workflow

if (FAILED_LOG(static_cast<HRESULT>(openResult.ResultCode().value)))
{
AICLI_LOG(Config, Error, << "Failed to open configuration set at " << absolutePath.u8string() << " with error 0x" << Logging::SetHRFormat << static_cast<HRESULT>(openResult.ResultCode().value));
AICLI_LOG(Config, Error, << "Failed to open configuration set at " << (isRemote ? argPath : absolutePath.u8string()) << " with error 0x" << Logging::SetHRFormat << static_cast<HRESULT>(openResult.ResultCode().value));

switch (openResult.ResultCode())
{
Expand Down Expand Up @@ -871,10 +938,20 @@ namespace AppInstaller::CLI::Workflow
}

// Fill out the information about the set based on it coming from a file.
// TODO: Consider how to properly determine a good value for name and origin.
result.Name(absolutePath.filename().wstring());
result.Origin(absolutePath.parent_path().wstring());
result.Path(absolutePath.wstring());
if (isRemote)
{
// TODO: Suggestions welcome on what values to pass in.
result.Name(argPathWide);
Copy link
Member

Choose a reason for hiding this comment

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

We do have code that extracts the file name portion from a URL that we use when downloading installers. I would use that to set Name similarly to what we set for the local filesystem paths.

For Origin I think the remote URL is fine.

I agree that we should not set Path.

result.Origin(argPathWide);
// Do not set path. This means ${WinGetConfigRoot} not supported in remote configs.
}
else
{
// TODO: Consider how to properly determine a good value for name and origin.
result.Name(absolutePath.filename().wstring());
result.Origin(absolutePath.parent_path().wstring());
result.Path(absolutePath.wstring());
}

context.Get<Data::ConfigurationContext>().Set(result);
}
Expand Down Expand Up @@ -1309,6 +1386,36 @@ namespace AppInstaller::CLI::Workflow
}
}

void ValidateConfigurationSetUnitContents(Execution::Context& context)
{
ConfigurationContext& configContext = context.Get<Data::ConfigurationContext>();
auto units = configContext.Set().Units();
auto validationOrder = GetConfigurationSetUnitValidationOrder(units.GetView());

Configuration::WingetDscModuleUnitValidator wingetUnitValidator;

bool foundIssues = false;
for (const auto index : validationOrder)
{
const ConfigurationUnit& unit = units.GetAt(index);
auto moduleName = Utility::ConvertToUTF8(unit.Details().ModuleName());
if (Utility::CaseInsensitiveEquals(wingetUnitValidator.ModuleName(), moduleName))
{
bool result = wingetUnitValidator.ValidateConfigurationSetUnit(context, unit);
if (!result)
{
foundIssues = true;
}
}
}

if (foundIssues)
{
// Indicate that it was not a total success
AICLI_TERMINATE_CONTEXT(S_FALSE);
}
}

void ValidateAllGoodMessage(Execution::Context& context)
{
context.Reporter.Info() << Resource::String::ConfigurationValidationFoundNoIssues << std::endl;
Expand Down
6 changes: 6 additions & 0 deletions src/AppInstallerCLICore/Workflows/ConfigurationFlow.h
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,12 @@ namespace AppInstaller::CLI::Workflow
// Outputs: None
void ValidateConfigurationSetUnitProcessors(Execution::Context& context);

// Validates that specific unit contents referenced by the set are valid/available/etc.
// Required Args: None
// Inputs: ConfigurationProcessor, ConfigurationSet
// Outputs: None
void ValidateConfigurationSetUnitContents(Execution::Context& context);

// Outputs the final message stating that no issues were found.
// Required Args: None
// Inputs: None
Expand Down
39 changes: 39 additions & 0 deletions src/AppInstallerCLICore/Workflows/WorkflowBase.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ using namespace AppInstaller::Utility::literals;
using namespace AppInstaller::Pinning;
using namespace AppInstaller::Repository;
using namespace AppInstaller::Settings;
using namespace winrt::Windows::Foundation;

namespace AppInstaller::CLI::Workflow
{
Expand Down Expand Up @@ -1093,6 +1094,44 @@ namespace AppInstaller::CLI::Workflow
}
}

void VerifyFileOrUri::operator()(Execution::Context& context) const
{
auto path = context.Args.GetArg(m_arg);

// try uri first
Uri pathAsUri = nullptr;
try
{
pathAsUri = Uri{ Utility::ConvertToUTF16(path) };
}
catch (...) {}

if (pathAsUri && !pathAsUri.Suspicious())
Copy link
Member

Choose a reason for hiding this comment

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

Should Suspicious be handled under the "parsed as a URI portion"? Or do some local file paths end up with that flag?

We could probably have an appropriately handled interactive prompt for suspicious URIs to still allow them through if the user agrees. Or we can just flat out block them until there are complaints.

{
// SchemeName() always returns lower case
if (L"file" == pathAsUri.SchemeName() && !Utility::CaseInsensitiveStartsWith(path, "file:"))
{
// Uri constructor is smart enough to parse an absolute local file path to file uri.
// In this case, we should continue with VerifyFile.
context << VerifyFile(m_arg);
}
else if (std::find(m_supportedSchemes.begin(), m_supportedSchemes.end(), pathAsUri.SchemeName()) != m_supportedSchemes.end())
{
// Scheme supported.
return;
}
else
{
context.Reporter.Error() << Resource::String::UriSchemeNotSupported(Utility::LocIndView{ path }) << std::endl;
AICLI_TERMINATE_CONTEXT(HRESULT_FROM_WIN32(ERROR_NOT_SUPPORTED));
}
}
else
{
context << VerifyFile(m_arg);
}
}

void GetManifestFromArg(Execution::Context& context)
{
Logging::Telemetry().LogIsManifestLocal(true);
Expand Down
16 changes: 16 additions & 0 deletions src/AppInstallerCLICore/Workflows/WorkflowBase.h
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,22 @@ namespace AppInstaller::CLI::Workflow
Execution::Args::Type m_arg;
};

// Ensures the local file exists and is not a directory. Or it's a Uri. Default only https is supported at the moment.
// Required Args: the one given
// Inputs: None
// Outputs: None
struct VerifyFileOrUri : public WorkflowTask
{
VerifyFileOrUri(Execution::Args::Type arg, std::vector<std::wstring> supportedSchemes = { L"https" }) :
WorkflowTask("VerifyFileOrUri"), m_arg(arg), m_supportedSchemes(std::move(supportedSchemes)) {}

void operator()(Execution::Context& context) const override;

private:
Execution::Args::Type m_arg;
std::vector<std::wstring> m_supportedSchemes;
};

// Opens the manifest file provided on the command line.
// Required Args: Manifest
// Inputs: None
Expand Down
Loading