Microsoft.WinGet.DSC improvements (and breaking change) (#4795)

The goal of this PR is to get these resources to a state where we feel ready to release the module without the prerelease tag. The `WinGetUserSettings`, `WinGetAdminSettings`, and `WinGetPackageManager` resources were deemed acceptable in their current state. The `WinGetSources` resource was decided to be overly complicated and likely not heavily used yet (and containing some bugs), so it was removed in favor of a new `WinGetSource` resource for managing individual sources. The `WinGetPackage` resource was updated to be better in line with best practices but should still be compatible unless one was using `Get`. ## Change The `WinGetSources` resource was replaced with a `WinGetSource` resource that manages a single source. The `WinGetSources` resource can continue to be used by specifying one of the module versions that contains it, but once released, new configurations should prefer the `WinGetSource` resource from the latest module. While there is no way to include or discover help via the module, I documented the resources similarly to the cmdlet documentation format. These can be used as the basis for official documentation, or we can put a link to them in the PowerShell Gallery listing (maybe updating the `Project Site` to point to a new landing page for the DSC resources). The updates to `WinGetPackage` aim to have it follow best practices and make general improvements: - Mark `Source` as a `Key` - Validate with existing attributes where possible - Not updating `$this` in calls to `Get`/`Set`/`Test` - Using the configurable properties to report values with `Get` - `IsInstalled` is reported via `Ensure` (true => `Present`, false => `Absent`) - `InstalledVersion` is reported via `Version` - `IsUpdateAvailable` is reported via `UseLatest` (they will have the opposite boolean value from each other) Any existing (non-`Get`) uses of the resource should continue to work as they did before. Smaller updates worth mentioning: - Updated the `winget configure validate` functionality to handle the new `WinGetSource` resource instead of `WinGetSources`. - Removed the error when giving both a `Version` and `UseLatest` to the `WinGetPackage` resource. `UseLatest` will override any `Version` provided if it is true. - Fixed a bug with COM `PackageCatalogInfo::TrustLevel` property reporting `None` improperly if the internal trust level contained both `Trusted` and another flag. - Renamed all of the enums in the module as there is no namespacing for enums/classes in PowerShell.
microsoft · Sep 10, 2024 · 9116d5d · 9116d5d
1 parent 8691cb3
commit 9116d5d
Show file tree

Hide file tree

Showing 20 changed files with 554 additions and 528 deletions.
diff --git a/src/AppInstallerCLICore/ConfigurationWingetDscModuleUnitValidation.cpp b/src/AppInstallerCLICore/ConfigurationWingetDscModuleUnitValidation.cpp
@@ -14,18 +14,19 @@ namespace AppInstaller::CLI::Configuration
 {
     namespace
     {
-        constexpr static std::string_view UnitType_WinGetSources = "WinGetSources"sv;
+        constexpr static std::string_view UnitType_WinGetSource = "WinGetSource"sv;
         constexpr static std::string_view UnitType_WinGetPackage = "WinGetPackage"sv;
 
         constexpr static std::string_view WellKnownSourceName_WinGet = "winget"sv;
         constexpr static std::string_view WellKnownSourceName_MSStore = "msstore"sv;
 
         constexpr static std::string_view ValueSetKey_TreatAsArray = "treatAsArray"sv;
 
-        constexpr static std::string_view WinGetSourcesValueSetKey_Sources = "sources"sv;
-        constexpr static std::string_view WinGetSourcesValueSetKey_SourceName = "name"sv;
-        constexpr static std::string_view WinGetSourcesValueSetKey_SourceType = "type"sv;
-        constexpr static std::string_view WinGetSourcesValueSetKey_SourceArg = "arg"sv;
+        constexpr static std::string_view WinGetSourceValueSetKey_Name = "name"sv;
+        constexpr static std::string_view WinGetSourceValueSetKey_Type = "type"sv;
+        constexpr static std::string_view WinGetSourceValueSetKey_Arg = "argument"sv;
+        constexpr static std::string_view WinGetSourceValueSetKey_Ensure = "ensure"sv;
+        constexpr static std::string_view WinGetSourceValueSetKey_Ensure_Present = "present"sv;
 
         constexpr static std::string_view WinGetPackageValueSetKey_Id = "id"sv;
         constexpr static std::string_view WinGetPackageValueSetKey_Version = "version"sv;
@@ -37,6 +38,7 @@ namespace AppInstaller::CLI::Configuration
             std::string Name;
             std::string Type;
             std::string Arg;
+            bool Present = true;
 
             bool Empty()
             {
@@ -66,64 +68,30 @@ namespace AppInstaller::CLI::Configuration
             return defaultIfFailed;
         }
 
-        std::vector<WinGetSource> ParseWinGetSourcesFromSettings(const ValueSet& settings)
+        WinGetSource ParseWinGetSourceFromSettings(const ValueSet& settings)
         {
+            WinGetSource result;
+
             // Iterate through the value set as Powershell variables are case-insensitive.
-            std::vector<WinGetSource> result;
             for (auto const& settingsPair : settings)
             {
                 auto settingsKey = Utility::ConvertToUTF8(settingsPair.Key());
-                if (Utility::CaseInsensitiveEquals(WinGetSourcesValueSetKey_Sources, settingsKey))
-                {
-                    auto sources = settingsPair.Value().try_as<ValueSet>();
-                    if (!sources)
-                    {
-                        return {};
-                    }
-                    bool isArray = false;
-                    for (auto const& sourcesPair : sources)
-                    {
-                        if (Utility::CaseInsensitiveEquals(ValueSetKey_TreatAsArray, Utility::ConvertToUTF8(sourcesPair.Key())))
-                        {
-                            isArray = true;
-                        }
-                        else
-                        {
-                            auto source = sourcesPair.Value().try_as<ValueSet>();
-                            if (source)
-                            {
-                                WinGetSource wingetSource;
-                                for (auto const& sourcePair : source)
-                                {
-                                    auto sourceKey = Utility::ConvertToUTF8(sourcePair.Key());
-                                    if (Utility::CaseInsensitiveEquals(WinGetSourcesValueSetKey_SourceName, sourceKey))
-                                    {
-                                        wingetSource.Name = GetPropertyValueAsString(sourcePair.Value());
-                                    }
-                                    else if (Utility::CaseInsensitiveEquals(WinGetSourcesValueSetKey_SourceType, sourceKey))
-                                    {
-                                        wingetSource.Type = GetPropertyValueAsString(sourcePair.Value());
-                                    }
-                                    else if (Utility::CaseInsensitiveEquals(WinGetSourcesValueSetKey_SourceArg, sourceKey))
-                                    {
-                                        wingetSource.Arg = GetPropertyValueAsString(sourcePair.Value());
-                                    }
-                                }
-
-                                if (!wingetSource.Empty())
-                                {
-                                    result.emplace_back(std::move(wingetSource));
-                                }
-                            }
-                        }
-                    }
-
-                    if (!isArray)
-                    {
-                        return {};
-                    }
 
-                    break;
+                if (Utility::CaseInsensitiveEquals(WinGetSourceValueSetKey_Name, settingsKey))
+                {
+                    result.Name = GetPropertyValueAsString(settingsPair.Value());
+                }
+                else if (Utility::CaseInsensitiveEquals(WinGetSourceValueSetKey_Type, settingsKey))
+                {
+                    result.Type = GetPropertyValueAsString(settingsPair.Value());
+                }
+                else if (Utility::CaseInsensitiveEquals(WinGetSourceValueSetKey_Arg, settingsKey))
+                {
+                    result.Arg = GetPropertyValueAsString(settingsPair.Value());
+                }
+                else if (Utility::CaseInsensitiveEquals(WinGetSourceValueSetKey_Ensure, settingsKey))
+                {
+                    result.Present = Utility::CaseInsensitiveEquals(WinGetSourceValueSetKey_Ensure_Present, GetPropertyValueAsString(settingsPair.Value()));
                 }
             }
 
@@ -206,56 +174,48 @@ namespace AppInstaller::CLI::Configuration
        auto unitType = Utility::ConvertToUTF8(details.UnitType());
        auto unitIntent = unit.Intent();
 
-       if (Utility::CaseInsensitiveEquals(UnitType_WinGetSources, unitType))
+       if (Utility::CaseInsensitiveEquals(UnitType_WinGetSource, unitType))
        {
-           auto sources = ParseWinGetSourcesFromSettings(unit.Settings());
-           if (sources.size() == 0)
-           {
-               AICLI_LOG(Config, Warning, << "Failed to parse WinGetSources or empty content.");
-               context.Reporter.Warn() << Resource::String::WinGetResourceUnitEmptyContent(Utility::LocIndView{ UnitType_WinGetSources }) << std::endl;
-               foundIssues = true;
-           }
-           for (auto const& source : sources)
-           {
-               // Validate basic semantics.
-               if (source.Name.empty())
-               {
-                   AICLI_LOG(Config, Error, << "WinGetSource unit missing required arg: Name");
-                   context.Reporter.Error() << Resource::String::WinGetResourceUnitMissingRequiredArg(Utility::LocIndView{ UnitType_WinGetSources }, "Name"_liv) << std::endl;
-                   foundIssues = true;
-               }
-               if (source.Arg.empty())
-               {
-                   AICLI_LOG(Config, Error, << "WinGetSource unit missing required arg: Arg");
-                   context.Reporter.Error() << Resource::String::WinGetResourceUnitMissingRequiredArg(Utility::LocIndView{ UnitType_WinGetSources }, "Arg"_liv) << std::endl;
-                   foundIssues = true;
-               }
+           auto source = ParseWinGetSourceFromSettings(unit.Settings());
 
-               // Validate well known source or process 3rd party source.
-               if (IsWellKnownSourceName(source.Name))
-               {
-                   if (!ValidateWellKnownSource(source))
-                   {
-                       AICLI_LOG(Config, Warning, << "WinGetSource conflicts with a well known source. Source: " << source.Name);
-                       context.Reporter.Warn() << Resource::String::WinGetResourceUnitKnownSourceConfliction(Utility::LocIndView{ source.Name }) << std::endl;
-                       foundIssues = true;
-                   }
-               }
-               else
-               {
-                   if (unitIntent == ConfigurationUnitIntent::Assert)
-                   {
-                       AICLI_LOG(Config, Warning, << "Asserting on 3rd party source: " << source.Name);
-                       context.Reporter.Warn() << Resource::String::WinGetResourceUnitThirdPartySourceAssertion(Utility::LocIndView{ source.Name }) << std::endl;
-                       foundIssues = true;
-                   }
-                   else if (unitIntent == ConfigurationUnitIntent::Apply)
-                   {
-                       // Add to dependency source map so it can be validated with later WinGetPackage source
-                       m_dependenciesSourceAndUnitIdMap.emplace(Utility::FoldCase(std::string_view{ source.Name }), Utility::FoldCase(Utility::NormalizedString{ unit.Identifier() }));
-                   }
-               }
-           }
+            // Validate basic semantics.
+            if (source.Name.empty())
+            {
+                AICLI_LOG(Config, Error, << "WinGetSource unit missing required arg: Name");
+                context.Reporter.Error() << Resource::String::WinGetResourceUnitMissingRequiredArg(Utility::LocIndView{ UnitType_WinGetSource }, "Name"_liv) << std::endl;
+                foundIssues = true;
+            }
+            if (source.Arg.empty() && source.Present)
+            {
+                AICLI_LOG(Config, Error, << "WinGetSource unit missing required arg: Argument");
+                context.Reporter.Error() << Resource::String::WinGetResourceUnitMissingRequiredArg(Utility::LocIndView{ UnitType_WinGetSource }, "Argument"_liv) << std::endl;
+                foundIssues = true;
+            }
+
+            // Validate well known source or process 3rd party source.
+            if (IsWellKnownSourceName(source.Name))
+            {
+                if (!ValidateWellKnownSource(source))
+                {
+                    AICLI_LOG(Config, Warning, << "WinGetSource conflicts with a well known source. Source: " << source.Name);
+                    context.Reporter.Warn() << Resource::String::WinGetResourceUnitKnownSourceConfliction(Utility::LocIndView{ source.Name }) << std::endl;
+                    foundIssues = true;
+                }
+            }
+            else
+            {
+                if (unitIntent == ConfigurationUnitIntent::Assert)
+                {
+                    AICLI_LOG(Config, Warning, << "Asserting on 3rd party source: " << source.Name);
+                    context.Reporter.Warn() << Resource::String::WinGetResourceUnitThirdPartySourceAssertion(Utility::LocIndView{ source.Name }) << std::endl;
+                    foundIssues = true;
+                }
+                else if (unitIntent == ConfigurationUnitIntent::Apply)
+                {
+                    // Add to dependency source map so it can be validated with later WinGetPackage source
+                    m_dependenciesSourceAndUnitIdMap.emplace(Utility::FoldCase(std::string_view{ source.Name }), Utility::FoldCase(Utility::NormalizedString{ unit.Identifier() }));
+                }
+            }
        }
        else if (Utility::CaseInsensitiveEquals(UnitType_WinGetPackage, unitType))
        {
@@ -281,8 +241,8 @@ namespace AppInstaller::CLI::Configuration
            }
            if (package.UseLatest && !package.Version.empty())
            {
-               AICLI_LOG(Config, Error, << "WinGetPackage unit both UseLatest and Version declared. Package: " << package.Id);
-               context.Reporter.Error() << Resource::String::WinGetResourceUnitBothPackageVersionAndUseLatest(Utility::LocIndView{ package.Id }) << std::endl;
+               AICLI_LOG(Config, Warning, << "WinGetPackage unit both UseLatest and Version declared. Package: " << package.Id);
+               context.Reporter.Warn() << Resource::String::WinGetResourceUnitBothPackageVersionAndUseLatest(Utility::LocIndView{ package.Id }) << std::endl;
                foundIssues = true;
            }
            // Validate dependency source is configured.
@@ -395,4 +355,4 @@ namespace AppInstaller::CLI::Configuration
 
        return !foundIssues;
     }
-}
+}
diff --git a/src/AppInstallerCLIE2ETests/TestData/Configuration/WinGetDscResourceValidate_Good.yml b/src/AppInstallerCLIE2ETests/TestData/Configuration/WinGetDscResourceValidate_Good.yml
@@ -1,15 +1,14 @@
 properties:
   configurationVersion: 0.2.0
   resources:
-    - resource: Microsoft.WinGet.DSC/WinGetSources
+    - resource: Microsoft.WinGet.DSC/WinGetSource
       id: configureSource
       directives:
         description: Add test source
         allowPrerelease: true
       settings:
-        Sources:
-          - Name: TestSource
-            Arg: "https://localhost:5001/TestKit"
+        Name: TestSource
+        Argument: "https://localhost:5001/TestKit"
     - resource: Microsoft.WinGet.DSC/WinGetPackage
       id: testPackage
       dependsOn:
@@ -21,4 +20,3 @@ properties:
         id: AppInstallerTest.TestExeInstaller
         source: TestSource
         version: "1.0.1.0"
-
diff --git a/...InstallerCLIE2ETests/TestData/Configuration/WinGetDscResourceValidate_PackageNotFound.yml b/...InstallerCLIE2ETests/TestData/Configuration/WinGetDscResourceValidate_PackageNotFound.yml
@@ -1,15 +1,14 @@
 properties:
   configurationVersion: 0.2.0
   resources:
-    - resource: Microsoft.WinGet.DSC/WinGetSources
+    - resource: Microsoft.WinGet.DSC/WinGetSource
       id: configureSource
       directives:
         description: Add test source
         allowPrerelease: true
       settings:
-        Sources:
-          - Name: TestSource
-            Arg: "https://localhost:5001/TestKit"
+        Name: TestSource
+        Argument: "https://localhost:5001/TestKit"
     - resource: Microsoft.WinGet.DSC/WinGetPackage
       id: testPackage
       dependsOn:
@@ -21,4 +20,3 @@ properties:
         id: AppInstallerTest.DoesNotExist
         source: TestSource
         version: "1.0.1.0"
-
diff --git a/...erCLIE2ETests/TestData/Configuration/WinGetDscResourceValidate_PackageVersionNotFound.yml b/...erCLIE2ETests/TestData/Configuration/WinGetDscResourceValidate_PackageVersionNotFound.yml
@@ -1,15 +1,14 @@
 properties:
   configurationVersion: 0.2.0
   resources:
-    - resource: Microsoft.WinGet.DSC/WinGetSources
+    - resource: Microsoft.WinGet.DSC/WinGetSource
       id: configureSource
       directives:
         description: Add test source
         allowPrerelease: true
       settings:
-        Sources:
-          - Name: TestSource
-            Arg: "https://localhost:5001/TestKit"
+        Name: TestSource
+        Argument: "https://localhost:5001/TestKit"
     - resource: Microsoft.WinGet.DSC/WinGetPackage
       id: testPackage
       dependsOn:
@@ -21,4 +20,3 @@ properties:
         id: AppInstallerTest.TestExeInstaller
         source: TestSource
         version: "101.0.101.0"
-
diff --git a/...nstallerCLIE2ETests/TestData/Configuration/WinGetDscResourceValidate_SourceOpenFailed.yml b/...nstallerCLIE2ETests/TestData/Configuration/WinGetDscResourceValidate_SourceOpenFailed.yml
@@ -1,15 +1,14 @@
 properties:
   configurationVersion: 0.2.0
   resources:
-    - resource: Microsoft.WinGet.DSC/WinGetSources
+    - resource: Microsoft.WinGet.DSC/WinGetSource
       id: configureSource
       directives:
         description: Add test source
         allowPrerelease: true
       settings:
-        Sources:
-          - Name: TestSourceV2
-            Arg: "https://localhost:5001/TestKit"
+        Name: TestSourceV2
+        Argument: "https://localhost:5001/TestKit"
     - resource: Microsoft.WinGet.DSC/WinGetPackage
       id: testPackage
       dependsOn:
@@ -21,4 +20,3 @@ properties:
         id: AppInstallerTest.TestExeInstaller
         source: TestSourceV2
         version: "1.0.1.0"
-
diff --git a/...a/Configuration/WinGetDscResourceValidate_VersionSpecifiedWithOnlyOneVersionAvailable.yml b/...a/Configuration/WinGetDscResourceValidate_VersionSpecifiedWithOnlyOneVersionAvailable.yml
@@ -1,15 +1,14 @@
 properties:
   configurationVersion: 0.2.0
   resources:
-    - resource: Microsoft.WinGet.DSC/WinGetSources
+    - resource: Microsoft.WinGet.DSC/WinGetSource
       id: configureSource
       directives:
         description: Add test source
         allowPrerelease: true
       settings:
-        Sources:
-          - Name: TestSource
-            Arg: "https://localhost:5001/TestKit"
+        Name: TestSource
+        Argument: "https://localhost:5001/TestKit"
     - resource: Microsoft.WinGet.DSC/WinGetPackage
       id: testPackage
       dependsOn:
@@ -21,4 +20,3 @@ properties:
         id: AppInstallerTest.TestValidManifest
         source: TestSource
         version: "1.0.0.0"
-
diff --git a/...ETests/TestData/Configuration/WinGetDscResourceValidate_VersionSpecifiedWithUseLatest.yml b/...ETests/TestData/Configuration/WinGetDscResourceValidate_VersionSpecifiedWithUseLatest.yml
@@ -1,15 +1,14 @@
 properties:
   configurationVersion: 0.2.0
   resources:
-    - resource: Microsoft.WinGet.DSC/WinGetSources
+    - resource: Microsoft.WinGet.DSC/WinGetSource
       id: configureSource
       directives:
         description: Add test source
         allowPrerelease: true
       settings:
-        Sources:
-          - Name: TestSource
-            Arg: "https://localhost:5001/TestKit"
+        Name: TestSource
+        Argument: "https://localhost:5001/TestKit"
     - resource: Microsoft.WinGet.DSC/WinGetPackage
       id: testPackage
       dependsOn:
@@ -22,4 +21,3 @@ properties:
         source: TestSource
         version: "1.0.1.0"
         useLatest: true
-