Merge pull request #147 from michaeltlombardi/gh-145/main/add_validat…

…ion_mode

(GH-145) Add validation mode flag to puppetized DSC Resources

CHANGELOG.md

@@ -14,6 +14,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Long file path support information to the requirements section of autogenerated READMEs ([#149](https://github.com/puppetlabs/Puppet.Dsc/issues/149))
 - Notes on installation limitations to generated README files to clarify minitar limitations for `r10k` and serverless Puppet users ([#150](https://github.com/puppetlabs/Puppet.Dsc/issues/150))
 - Clarification added regarding configuring the DSC Local Configuration Manager to work with Puppetized DSC modules (LCM) ([#139](https://github.com/puppetlabs/Puppet.Dsc/issues/139))
+- Add `validation_mode` to allow choosing to rely on Puppet's property-by-property change reporting/insync testing or DSC's resource-level insync testing for cases where the DSC Resource does not correctly return current state as expected by its API surface. ([#145](https://github.com/puppetlabs/Puppet.Dsc/issues/145))
 
 ### Fixed
 

acceptance/Basic.Tests.ps1

@@ -167,6 +167,89 @@ Describe 'Acceptance Tests: Basic' -Tag @('Acceptance', 'Basic') {
           }
         )
       }
+      @{
+        Scenario                                    = 'puppetizing a module which cannot be idempotent in property mode'
+        # We need to know where the module will be built and what properties to build it with
+        expected_base                               = '../bar/securitypolicydsc'
+        PuppetModuleName                            = 'securitypolicydsc'
+        BuildParameters                             = @{
+          PowerShellModuleName    = 'SecurityPolicyDsc'
+          PowerShellModuleVersion = '2.10.0.0'
+          PuppetModuleAuthor      = 'testuser'
+          OutputDirectory         = '../bar'
+        }
+        # Any modified resources need to be cleaned up prior to each test run
+        DscResetInvocations                         = @(
+          @{
+            Name       = 'SecurityOption'
+            Method     = 'Set'
+            Property   = @{ Name = 'Enforce Anonymous SID Translation'; NetworkAccessAllowAnonymousSidNameTranslation = 'Enabled' }
+            ModuleName = @{
+              ModuleName      = 'C:/ProgramData/PuppetLabs/code/modules/securitypolicydsc/lib/puppet_x/securitypolicydsc/dsc_resources/SecurityPolicyDsc/SecurityPolicyDsc.psd1'
+              RequiredVersion = '2.10.0.0'
+            }
+          }
+        )
+        # The module should be uninstalled prior to each run if it exists
+        PdkModuleUninstallationInvocationParameters = @{
+          Path                = '../bar/securitypolicydsc'
+          Command             = 'pdk bundle exec puppet module uninstall testuser-securitypolicydsc'
+          SuccessFilterScript = { $true }
+        }
+        # Each of these types should be created and defined for puppet
+        TypesToValidateTestCases                    = @(
+          @{ Type = 'dsc_securityoption' }
+        )
+        # We need to validate that `puppet resource` works with the built module
+        TestResource                                = 'dsc_securityoption'
+        MinimalProperties                           = 'dsc_name="Enforce Anonymous SID Translation"'
+        MinimalExpectation                          = "dsc_name => 'Enforce Anonymous SID Translation'"
+        PropertyExpectation                         = "dsc_network_access_allow_anonymous_sid_name_translation => 'Enabled'"
+        # These are scenarios for child contexts that validate expected invocation behavior
+        ApplicationScenarios                        = @(
+          @{
+            ApplicationScenarioTitle     = 'when setting a security policy with "puppet apply"'
+            ApplicationScenarioTestCases = @(
+              @{
+                TestName               = 'works'
+                ManifestFileName       = 'security_option.pp'
+                ManifestFileValue      = @(
+                  'dsc_securityoption { "Enforce Anonymous SID Translation":'
+                  '  dsc_name => "Enforce Anonymous SID Translation",'
+                  '  dsc_network_access_allow_anonymous_sid_name_translation => "Disabled",'
+                  "}`n"
+                ) -join "`n"
+                PdkSuccessFilterScript = { $_ -match 'Updating: Finished' }
+                PdkErrorFilterScript   = { $_ -match 'Error' }
+              }
+              @{
+                TestName               = 'is not idempotent in property validation mode'
+                ManifestFileName       = 'non_idempotent_security_option.pp'
+                ManifestFileValue      = @(
+                  'dsc_securityoption { "Enforce Anonymous SID Translation":'
+                  '  dsc_name => "Enforce Anonymous SID Translation",'
+                  '  dsc_network_access_allow_anonymous_sid_name_translation => "Disabled",'
+                  "}`n"
+                ) -join "`n"
+                PdkSuccessFilterScript = { $_ -match 'Updating: Finished' }
+                PdkErrorFilterScript   = { $_ -match 'Error' }
+              }
+              @{
+                TestName             = 'is idempotent in resource validation mode'
+                ManifestFileName     = 'non_idempotent_security_option.pp'
+                ManifestFileValue    = @(
+                  'dsc_securityoption { "Enforce Anonymous SID Translation":'
+                  '  validation_mode        => "resource",'
+                  '  dsc_name               => "Enforce Anonymous SID Translation",'
+                  '  dsc_network_access_allow_anonymous_sid_name_translation => "Disabled",'
+                  "}`n"
+                ) -join "`n"
+                PdkErrorFilterScript = { $_ -match 'Updating: Finished' }
+              }
+            )
+          }
+        )
+      }
       # TODO: Class-based resource scenario
     )
     if ($null -ne $PwshLibSource) {

src/internal/functions/Get-ReadmeContent.ps1

@@ -212,6 +212,42 @@ Note that the only properties specified in a resource declaration which are pass
 If a property does _not_ start with `dsc_` it is used to control how Puppet interacts with DSC/other Puppet resources - for example,
 specifying a unique name for the resource for Puppet to distinguish between declarations or Puppet metaparameters (``notifies`, ``before`, etc).
 
+### Validation Mode
+
+By default, these resources use the property validation mode, which checks whether or not the resource is in the desired state the same way most Puppet resources are validated;
+by comparing the properties returned from the system with those specified in the manifest.
+Sometimes, however, this is insufficient;
+many DSC Resources return data that does not compare properly to the desired state (some are missing properties, others are malformed, some simply cannot be strictly compared).
+In these cases, you can set the validation mode to ``resource``, which falls back on calling ``Invoke-DscResource`` with the ``Test`` method and trusts that result.
+
+When using the ``resource`` validation mode, the resource is tested once and will then treat **all** properties of that resource as in sync (if the result returned ``true``) or not in sync.
+This loses the granularity of change reporting for the resource but prevents flapping and unexpected behavior.
+
+``````puppet
+# This will flap because the DSC resource never returns name in SecurityPolicyDsc v2.10.0.0
+dsc_securityoption { 'Enforce Anonoymous SID Translation':
+  dsc_name => 'Enforce Anonymous SID Translation',
+  dsc_network_access_allow_anonymous_sid_name_translation => 'Disabled',
+}
+
+# This will idempotently apply
+dsc_psrepository { 'PowerShell Gallery':
+  validation_mode => 'resource',
+  dsc_name        => 'Enforce Anonymous SID Translation',
+  dsc_network_access_allow_anonymous_sid_name_translation => 'Disabled',
+}
+``````
+
+It is important to note that this feature is only supported with a version of ``puppetlabs-pwshlib`` equal to or higher than ``0.9.0``, in which the supporting code for the DSC Base Provider to implement custom insync was shipped.
+
+Finally, while this module's metadata says that the supported Puppet versions are 6.0.0 and up, the implementation of the ``validation_mode`` parameter relies on the ``custom_insync`` feature of the Puppet Resource API.
+The ``custom_insync`` feature first shipped in the ``puppet-resource_api`` version ``1.8.14``, which itself is only included in Puppet versions equal to or newer than ``6.23.0`` and ``7.8.0`` for the 6x and 7x platforms respectively.
+Using this module against older Puppet versions will result in a warning (example below) and _only_ use the default property-by-property change reporting, regardless of the setting of ``validation_mode``.
+
+``````
+Warning: Unknown feature detected: ["custom_insync"]
+``````
+
 ## Troubleshooting
 
 In general, there are three broad categories of problems:

src/internal/functions/Get-TypeContent.Tests.ps1

@@ -29,7 +29,8 @@ Describe 'Get-TypeContent' -Tag 'Unit' {
         $Result | Should -MatchExactly "dscmeta_module_version: '2.12.0.0'"
         $Result | Should -MatchExactly 'The DSC Archive resource type.'
         $Result | Should -MatchExactly 'Automatically generated from version 2.12.0.0'
-        $Result | Should -MatchExactly "features: \['simple_get_filter', 'canonicalize'\]"
+        $Result | Should -MatchExactly "features: \['simple_get_filter', 'canonicalize', 'custom_insync'\]"
+        $Result | Should -MatchExactly 'validation_mode:'
       }
       It 'Attempts to interpolate the parameter information once' {
         Should -Invoke Get-TypeParameterContent -Times 1 -Scope Context

src/internal/functions/Get-TypeContent.ps1

@@ -53,13 +53,19 @@ Puppet::ResourceApi.register_type(
   dscmeta_module_name: '$($Resource.ModuleName)',
   dscmeta_module_version: '$($Resource.Version)',
   docs: $(ConvertTo-PuppetRubyString $ResourceDescription),
-  features: ['simple_get_filter', 'canonicalize'],
+  features: ['simple_get_filter', 'canonicalize', 'custom_insync'],
   attributes: {
     name: {
       type:      'String',
       desc:      'Description of the purpose for this resource declaration.',
       behaviour: :namevar,
     },
+    validation_mode: {
+      type:      'Enum[property, resource]',
+      desc:      'Whether to check if the resource is in the desired state by property (default) or using Invoke-DscResource in Test mode (resource).',
+      behaviour: :parameter,
+      default:   'property',
+    },
 $((Get-TypeParameterContent -ParameterInfo $Resource.ParameterInfo) -join "`n")
   },
 )

src/internal/functions/Update-PuppetModuleMetadata.Tests.ps1

@@ -85,7 +85,7 @@ Describe 'Update-PuppetModuleMetadata' -Tag 'Unit' {
         }
         It 'Updates the dependencies' {
           $Result.dependencies[0].Name | Should -Be 'puppetlabs/pwshlib'
-          $Result.dependencies[0].version_requirement | Should -Be '>= 0.7.0 < 2.0.0'
+          $Result.dependencies[0].version_requirement | Should -Be '>= 0.9.0 < 2.0.0'
         }
         It 'Updates the supported operating system list' {
           $Result.operatingsystem_support[0].operatingsystem | Should -Be 'windows'

src/internal/functions/Update-PuppetModuleMetadata.ps1

@@ -78,7 +78,7 @@ function Update-PuppetModuleMetadata {
     $PuppetMetadata.dependencies = @(
       @{
         name                = 'puppetlabs/pwshlib'
-        version_requirement = '>= 0.7.0 < 2.0.0'
+        version_requirement = '>= 0.9.0 < 2.0.0'
       }
     )
     # Update the operating sytem to only support windows *for now*.