added config automation (JDBC resource, license key) and bug fixes.

n2ygk commented 7 years ago

Enables installing and configuring an external JDBC database resource.
Automates a little more startup configuration by installing the license key. See Skipping startup screens in the README. (Eventually I'd like to get the startup configuratuon fully automated but it's looking difficult).
Fixes a couple of small bugs caused by the fact that an empty string is no longer "false" in Puppet DSL.
Fixes confluence_server fact not to get tricked into returning the Java version (e.g. 1.8.0 from /usr/lib/jvm/jre-1.8.0-oracle.x86_64/bin/java .... This was causing a confluence server restart on every Puppet run.

n2ygk commented 7 years ago

I see I did a NOOB thing and didn't rebase properly. Let me know if you want me to resubmit this PR.

oranenj commented 7 years ago

@n2ygk you don't need to re-submit the PR, but you should rebase to resolve the conflicts anyway. :)

You can replace your old branch with a force push, and the PR will update.

n2ygk commented 7 years ago

@oranenj I merged master into my fork and made an ugly syntax change in manifests/config.pp in order to pass the rake syntax check (indentation -- beyond my capabilities to figure out why rake doesn't like the indentation I had for an array of strings) but there are now a lot of other travis errors. Are these caused by my not adding tests for the new params I added? Thanks.

oranenj commented 7 years ago

@n2ygk those test failures seem to be caused by various issues. For example:

Evaluation Error: Unknown variable: '_jdbc'. at /home/travis/build/voxpupuli/puppet-confluence/spec/fixtures/modules/confluence/manifests/config.pp:66:75

This looks like you maybe have a typo somewhere.

oranenj commented 7 years ago

@n2ygk you should make sure tests pass for you locally (run bundle exec rake test) before you add more commits to the PR. Your local machine will likely run the tests faster than Travis :)

If you need help making the tests pass, feel free to ask here, or join #voxpupuli on Freenode and chances are there will be people around to help.

EDIT: also make sure your bundle setup is up-to-date (run bundle update)

n2ygk commented 7 years ago

@oranenj Looks like I have a bit more learning to do (and I need to get a proper Rake, etc. environment set up locally on my RHEL 6.9 system or use a different distro since dumb things like this happen: rake requires Ruby version >= 1.9.3.

Should I clean up and submit a new PR when ready?

oranenj commented 7 years ago

@n2ygk Yeah, you'll want to do most of the development on a desktop machine. Once you have a modern enough ruby, bundler can be used to install the required dependencies.

You can keep this PR open, there's no need to get rid of it IMO unless you want to split your work into separate PRs. You can always completely rewrite your branch anyway.

oranenj commented 7 years ago

@n2ygk: The problem is mostly with the web.xml. I think it might come as a surprise to existing users if the module overwrites the file after a module upgrade.

n2ygk commented 7 years ago

@oranenj Is there a reason to retain this augeas vs. template thing? Augeas solves the problem cleanly. I guess removing template becomes a breaking change.... I'll try something with file_line.

n2ygk commented 7 years ago

@oranenj OK, so I was thinking a little more about how to do this and hit on doing a multiline wildcard match (using [[:space:].]+) that will replace my addition if it's already there but I just can't get file_line multiline to work right. Here's a snippet from the end of web.xml:

    <!-- redirect all 500 errors to confluence error page -->
    <error-page>
        <error-code>500</error-code>
        <location>/500page.jsp</location>
    </error-page>

    <error-page>
        <error-code>404</error-code>
        <location>/fourohfour.action</location>
    </error-page>
</web-app>

and I'm going to try and replace everything from <location>/fourohfour.action</location> to </web-app> with this code:

      $res_ref = @("RES")
              <location>/fourohfour.action</location>
          </error-page>

          <!-- added by Puppet -->
          <resource-ref>
              <description>Connection Pool</description>
              <res-ref-name>${_jdbc_name}</res-ref-name>
              <res-type>${_jdbc_type}</res-type>
              <res-auth>${_jdbc_auth}</res-auth>
          </resource-ref>
      </web-app>
      | RES

      file_line { 'web.xml jdbc resource-ref':
        path     => "${confluence::webappdir}/confluence/WEB-INF/web.xml",
        match    => '^[ \t]+<location>/fourohfour\.action</location>[[:space:].]+</web-app>',
        line     => $res_ref,
        require  => Class['confluence::install'],
        notify   => Class['confluence::service'],
      }

and no matter what I do, it just appends $res_ref to the end of the file! Even when I put in a blatantly non-matching expression in!

    <error-page>
        <error-code>404</error-code>
        <location>/fourohfour.action</location>
    </error-page>
</web-app>
        <location>/fourohfour.action</location>
    </error-page>

    <!-- added by Puppet -->
    <resource-ref>
        <description>Connection Pool</description>
        <res-ref-name>jdbc/confluence</res-ref-name>
        <res-type>javax.sql.DataSource</res-type>
        <res-auth>Container</res-auth>
    </resource-ref>
</web-app>

Should file_line at a minimum log an error if the match expression fails?!

This is becoming way too much time spent because someone might not like using augeas....

oranenj commented 7 years ago

@n2ygk: Hmmh. This really looks like there is no sane way to do this.

I'd be fine if this new feature only worked with the 'augeas' management option, and just gave an error otherwise, as long as the template continues to function for people who do not use this feature.

n2ygk commented 7 years ago

Agreed. I'll submit a PR shortly.

On Mon, May 8, 2017 at 2:24 PM, Jarkko Oranen notifications@github.com wrote:

@n2ygk https://github.com/n2ygk: Hmmh. This really looks like there is no sane way to do this.

I'd be fine if this new feature only worked with the 'augeas' management option, and just gave an error otherwise, as long as the template continues to function for people who do not use this feature.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/voxpupuli/puppet-confluence/pull/125#issuecomment-299949522, or mute the thread https://github.com/notifications/unsubscribe-auth/AEJ5d3uoLnPo5YTXu5U_-NFcVbRs8xDqks5r313NgaJpZM4M79hG .

n2ygk commented 7 years ago

pfff! AJP requires the opposite in init.pp:

if ! empty($ajp) {
    if $manage_server_xml != 'template' {
      fail('An AJP connector can only be configured with manage_server_xml = template.')
    }

That's an improvement for another day.

n2ygk commented 7 years ago

Looks like @JCotton1123 might want to take a look at fixing the AJP stuff to support augeas configuration.

ronech commented 6 years ago

This looks nifty, can it be merged yet?

bastelfreak commented 6 years ago

@ronech this needs to be rebased, we can't merge it this way. Are you interested in doing this?

ronech commented 6 years ago

I've never rebased anything in this repo but i wouldn't object to giving it a shot.

n2ygk commented 6 years ago

superseded by #154

voxpupuli / puppet-confluence

added config automation (JDBC resource, license key) and bug fixes. #125