Uploaded image for project: 'Puppet Server'
  1. Puppet Server
  2. SERVER-715

`isExpired` method on environment registry implementation is not resilient

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Normal
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: SERVER 5.0.0
    • Component/s: None
    • Labels:
      None
    • Template:
    • Team:
      Systems Engineering
    • Sub-team:
    • Story Points:
      2
    • Sprint:
      Server 2017-06-14
    • Release Notes:
      Bug Fix
    • Release Notes Summary:
      Hide
      Fix for a bug which intermittently caused some HTTP API requests to fail with a 500 error status code, with a message like the following in the puppetserver.log file:

      2015-05-05 17:40:29,649 ERROR [puppet-server] Puppet
      puppetlabs.services.jruby.puppet_environments$environment_registry$reify__12408.isExpired(puppet_environments.clj:27)
      ...
      Show
      Fix for a bug which intermittently caused some HTTP API requests to fail with a 500 error status code, with a message like the following in the puppetserver.log file: 2015-05-05 17:40:29,649 ERROR [puppet-server] Puppet puppetlabs.services.jruby.puppet_environments$environment_registry$reify__12408.isExpired(puppet_environments.clj:27) ...

      Description

      We saw some errors in PE-10057 that looked like this:

      2015-05-05 17:40:29,649 ERROR [puppet-server] Puppet 
      puppetlabs.services.jruby.puppet_environments$environment_registry$reify__12408.isExpired(puppet_environments.clj:27)
      puppetlabs.services.jruby.puppet_environments$environment_registry$reify__12408.isExpired(puppetlabs/services/jruby/puppet_environments.clj:27)
      

      The line that's happening on is here:

      https://github.com/puppetlabs/puppet-server/blob/16c4f503b46d5103fda713c1e030731892deed29/src/clj/puppetlabs/services/jruby/puppet_environments.clj#L28

      That line could theoretically return a `nil`, which might cause some kind of exception since the Java interface specifies a boolean as the return type:

      https://github.com/puppetlabs/puppet-server/blob/47b9ad3cb20f576acf1333cfcd354a60c185089f/src/java/com/puppetlabs/puppetserver/EnvironmentRegistry.java#L5

      I thought that the code paths through Puppet would guarantee that we'd always registered an environment prior to calling isExpired, so it would be good to investigate that and make sure we understand what's going on, but in either case it's probably best to go ahead and fix the clojure code to explicitly return false if the value doesn't exist.

        Attachments

          Activity

            jsd-sla-details-panel

              People

              • Assignee:
                joe.pinsonault Joe Pinsonault
                Reporter:
                chris Chris Price
                QA Contact:
                Erik Dasher
              • Votes:
                0 Vote for this issue
                Watchers:
                6 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: