diff --git a/CHANGELOG.md b/CHANGELOG.md index b3a38e7f..297752ca 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/acceptance/Basic.Tests.ps1 b/acceptance/Basic.Tests.ps1 index f186d63a..c3833e0f 100644 --- a/acceptance/Basic.Tests.ps1 +++ b/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) { diff --git a/src/internal/functions/Get-ReadmeContent.ps1 b/src/internal/functions/Get-ReadmeContent.ps1 index f8c2015a..4c7c03a0 100644 --- a/src/internal/functions/Get-ReadmeContent.ps1 +++ b/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: diff --git a/src/internal/functions/Get-TypeContent.Tests.ps1 b/src/internal/functions/Get-TypeContent.Tests.ps1 index cd0e4ee5..c2c5c62f 100644 --- a/src/internal/functions/Get-TypeContent.Tests.ps1 +++ b/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 diff --git a/src/internal/functions/Get-TypeContent.ps1 b/src/internal/functions/Get-TypeContent.ps1 index b154b339..b49619f8 100644 --- a/src/internal/functions/Get-TypeContent.ps1 +++ b/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") }, ) diff --git a/src/internal/functions/Update-PuppetModuleMetadata.Tests.ps1 b/src/internal/functions/Update-PuppetModuleMetadata.Tests.ps1 index 130bcafc..7f43420c 100644 --- a/src/internal/functions/Update-PuppetModuleMetadata.Tests.ps1 +++ b/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' diff --git a/src/internal/functions/Update-PuppetModuleMetadata.ps1 b/src/internal/functions/Update-PuppetModuleMetadata.ps1 index b96fa045..945cbda9 100644 --- a/src/internal/functions/Update-PuppetModuleMetadata.ps1 +++ b/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*.