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

(MAINT) Ensure is_same check works for nil manifest values #96

Merged
merged 1 commit into from
Feb 2, 2021
Merged

Conversation

bwilcox
Copy link
Contributor

@bwilcox bwilcox commented Feb 2, 2021

When working with networkingdsc and using dsc_firewall we encounted an issue where the code attempts to sort a nil value:

 C:\Users\Administrator\test\modules\networkingdsc> puppet apply .\examples\test.pp --modulepath .\spec\fixtures\modules\
Notice: Compiled catalog for lay-escalation.delivery.puppetlabs.net in environment production in 0.21 seconds
Error: undefined method `sort' for nil:NilClass
PS C:\Users\Administrator\test\modules\networkingdsc>

Using pry I can see the offending parameter:

From: C:/Users/Administrator/test/modules/networkingdsc/spec/fixtures/modules/pwshlib/lib/puppet/provider/dsc_base_provider/dsc_base_provider.rb @ line 66 Puppet::Provider::DscBaseProvider#canonicalize:

    61:           end
    62:           downcased_result = recursively_downcase(canonicalized)
    63:           downcased_resource = recursively_downcase(r)
    64:           downcased_result.each do |key, value|
    65:             #is_same = (value.is_a?(Enumerable) & !downcased_resource[key].nil?) ? downcased_resource[key].sort == value.sort : downcased_resource[key] == value
 => 66:             is_same = value.is_a?(Enumerable) ? downcased_resource[key].sort == value.sort : downcased_resource[key] == value
    67:                         canonicalized[key] = r[key] unless is_same
    68:             canonicalized.delete(key) unless downcased_resource.keys.include?(key)
    69:           end
    70:           # Cache the actually canonicalized resource separately
    71:           @@cached_canonicalized_resource << canonicalized.dup
[26] pry(#<Puppet::Provider::DscFirewall::DscFirewall>)> key
=> :dsc_localport
[27] pry(#<Puppet::Provider::DscFirewall::DscFirewall>)> value
=> ["any"]
[28] pry(#<Puppet::Provider::DscFirewall::DscFirewall>)> downcased_resource[key]
=> nil
[29] pry(#<Puppet::Provider::DscFirewall::DscFirewall>)> next

From: C:/Ruby25-x64/lib/ruby/gems/2.5.0/gems/puppet-7.3.0-x64-mingw32/lib/puppet/application/apply.rb @ line 264 Puppet::Application::Apply#main:

    259:           exit(exit_status)
    260:         else
    261:           exit(0)
    262:         end
    263:       rescue => detail
 => 264:         Puppet.log_exception(detail)
    265:         exit(1)
    266:       end
    267:     end
    268:   end
    269:
[29] pry(#<Puppet::Application::Apply>)>

This request adds a check for nil values. I'm not sure if this is the best way to solve the problem, but in my continued testing it does the job.

@bwilcox bwilcox requested a review from a team as a code owner February 2, 2021 18:19
@michaeltlombardi michaeltlombardi merged commit a360517 into puppetlabs:main Feb 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants