WIP: completely rework the way imports work for the native types

[damoxc]

When running under Puppet Server, attempting to load files from puppet_x
results in an autoload failure due to some stricter $LOAD_PATH rules. Whilst
certainly a bug upstream reworking the native types like this allows the module
to work under Puppet Server.

I haven't been able to test this when not running under Puppet Server as I don't have that available I'm afraid. The loading logic was shamelessly lifted from the archive module.

[damoxc]

Actually this still doesn't solve the problems when running on an agent¸ only when running via puppet master --compile NODE

[damoxc]

OK, found out my error is happening due to Puppet[:environment] returning the master's environment, not the agent's.

[jenkinsadmin]

Thank you for a pull request! Please check this document for how the Jenkins project handles pull requests

[jhoblitt]

Hi @damoxc - this looks reasonable but I want contemplate it for a bit. By any chance are you at puppetconf this week?

[jhoblitt]

Is the forward declaration causing an import problem?

[damoxc]

Not entirely, however it just cuts out the required number of bodged imports and number of files they need to be made in.

[damoxc]

Hi @jhoblitt, sadly I'm not at PuppetConf. But contemplate away, I think I'll be doing the same this evening and probably hacking around a bit more tomorrow. I think also, the changes in the groovy script need some rethinking too, I was getting errors due to the explicit inclusion of checking for the github-oauth providers.

[jhoblitt]

@damoxc Aww, I'm guessing you don't have github-oauth plugin installed?

[damoxc]

@jhoblitt I don't no, been working on sorting out getting Active Directory (:cry:) auth working.

I think this is probably the simplest solution to allow for more providers to be added that come in plugins that may or may not be present, what do you think? Lose a bit of performance but the flexibility is worth it IMO.

cloudbuy@e793f47

[jhoblitt]

@damoxc I suspect you are right; mental bandwidth is still a bit limited by puppetconf.

What are the comments for #14073 and #7788 referring to? They don't seem to be [public] PL PUP-XXXX or SERVER-XXXX ticket ids.

[jhoblitt]

@damoxc I think you missed a class name in get_security_realm() https://github.com/cloudbuy/puppet-jenkins/blob/e793f475b7e645dd212f939ded8738ee4625dbbb/files/puppet_helper.groovy#L700 Are you willing to open a PR for the stringification of class names that applies to current master? I'd like to get that merged.

I'm going to try to meet up with some of the puppet-server folks that are at puppetconf to discuss the loading issue.

[jhoblitt]

@damoxc Your quoting fix has been merged as #387

[damoxc]

@jhoblitt Sorry, missed this over the weekend. Thanks for merging that 😄

I lifted the code directly from nanliu/archive which uses code in puppet_x successfully, I should have updated that in the comments. I think that they may be referencing https://projects.puppetlabs.com/issues/14073 and https://projects.puppetlabs.com/issues/7788 on the old tracker.

Given this method seems to be broken if the module isn't present in the environment that the master is in, a better method will have to be thought of. Would using relative imports be an option?

[jhoblitt]

@damoxc After a ton of useful help from @hunner, I've managed to hack together a beaker acceptance test that reproduces the loading failure (still needs to be cleaned up before PRing it).

I'm going to wrestle with this a bit more in hopes of finding a less ugly fix.

[jhoblitt]

@damoxc What version of puppetserver are you using? I'm testing with 1.1.1.

[damoxc]

@jhoblitt I was using puppetserver 2.1.1 (I'm currently using a snapshot of 2.1.2) along with puppet-agent 1.2.6 from PC1.

[jhoblitt]

Thanks. I'm currently trying to get beaker to play nice with the new aio paths.

I've also opened a new Jira ticket for the path loading issue since I couldn't identify an existing on: https://tickets.puppetlabs.com/browse/SERVER-973

[jhoblitt]

@damoxc Thus far is looks like the problem isn't triggered unless the new types are in the manifest. That's good news as it means it won't blow-up for anyone not using them.

I also may have come across a short term fix until this is fixed in puppetserver. Could you try to modify your puppetserver.conf like this:

jruby-puppet: {
    ruby-load-path: [/opt/puppetlabs/puppet/lib/ruby/vendor_ruby, /opt/puppetlabs/puppet/cache/lib]
    ...
}

[jhoblitt]

@damoxc I've decided to document the workaround in my last comment and ship 1.6.0 without merging these changes. We'll definitely want to revisit this if a fixed version of puppetserver doesn't ship before we want to make a release that starts to merge the new types into the existing classes/defines.

[damoxc]

@jhoblitt sounds good to me, although I would also document that if you are in a situation where your machine running puppetserver is using an environment that doesn't contain the jenkins module (say master running production and the Jenkins machine running on jenkins-experiment, you'll still receive the failure. I think I'll mention this on the issue you opened against puppetserver.

[elconas]

I used this workaround here to work with directory environments:

begin
  require 'puppet_x/jenkins/type/cli'
rescue LoadError
  environment = Puppet.lookup(:current_environment).to_s
  if environment == '**root**'
    environment = Puppet[:environment].to_s
  end
  require 'pathname' # WORK_AROUND #14073 and #7788
  themodule = Puppet::Module.find('jenkins', environment)
  raise(LoadError, "Unable to find jenkins module in modulepath #{Puppet[:basemodulepath] || Puppet[:modulepath]}") unless themodule
  require File.join themodule.path, 'lib/puppet_x/jenkins/type/cli'
end