puppetlabs / puppetlabs-tomcat Goto Github PK

Use Case

Maybe I missed it, but I can't figure out how to use this module to configure a tomcat 10 with TLS support. Since Version 8.5 an SSLHostConfig within the connector in the server.xml is possible and how it seems to me since version 10 it is mandatory.

https://tomcat.apache.org/tomcat-10.1-doc/ssl-howto.html#Edit_the_Tomcat_Configuration_File

Describe the Solution You Would Like

Please add support for an SSLHostConfig to tomcat::config::server::connector

Best regards and many thanks in advance,
Dirk

Describe the Bug

When upgrading this module from 6.3.0 to 6.4.0 using Stdlib::Port data type as argument to tomcat::config::server::port fails with

Tomcat::Config::Server[am-server]: parameter 'port' expects a value of type Undef or String, got Integer

Expected Behavior

Minor version upgrades should never break like this.
Stdlib data types should always be allowed in puppetlabs modules.

metadata.json on current master has changes which are need to allow puppet/archive 6.x to be used within a puppet environment. This issue is requesting that a new forge release be made from the current master.

Since upgrade to Apache 8.5.84 nightly and PR tests are failing with:

Error: Working directory /opt/apache-tomcat/bin/commons-daemon-1.3.1-native-src/unix does not exist!
e.g: https://github.com/puppetlabs/puppetlabs-tomcat/actions/runs/3869636565/jobs/6595886291#step:13:354

Looks like commons-daemon was updated from 1.3.1 to 1.3.2 with Tomcat 8.5.84 so needs a bump of the daemon_version in spec/acceptance/acceptance_1b_spec.rb

Use Case

Previously i opened an issue with a type being wrong but i'm facing an other issue which can be resolved on my side but i think an overal review of the different new var types should be considered.

I'll try to describe some "wrong" types.

Describe the Solution You Would Like

To match var types better with their purpose

Additional Context

I'll add examples from where i would change types i will not include all params or go through all classes but just pasting some most used probably.

define tomcat::config::server (
Optional[String[1]] $address = undef, => since this has to be TCP/IP address check better instead of string
Optional[String[1]] $port = undef, => Change to integer or variant [string, integer]
Optional[String[1]] $server_config = undef, => Change to absolutepath since docs says "valid option string containing absolutepath"
) {

define tomcat::install (
String[1] $catalina_home = $name, => Change to absolutepath since docs says "valid option string containing absolutepath"
Optional[String[1]] $source_url = undef, => Change to enum or update docs this is not check but docs says "Valid options: a string containing a puppet://, http(s)://, or ftp://"
Optional[String[1]] $proxy_server = undef, => Could be checked better to since you have an enum on proxy_type
) {

This is ofc just a suggestion but stricter checking or using the correct types will help everyone in my opinion :)

Describe the Bug

In previous version 6.3.0 no type was given to for paramter $package_ensure in define tomcat::install
But with 6.4.0 a type is given but the type is boolean which i think is wrong because you pass it through to an other class and there the type is Optional[String[1]] which in my opinion is the correct one.

Expected Behavior

That i can use a string for package_ensure.

Steps to Reproduce

Steps to reproduce the behavior:
Trying to run this code
tomcat::install { $catalina_home:
install_from_source => false,
package_name => 'tomcat',
package_ensure => '10.1.5',
}

Environment

• Version [7.23.0] (puppet)
• Version [6.4.0] (module)
• Platform [Rhel 8]

Use Case

I am attempting to add the attribute crossContext="true" to the Context element in $CATALINA_BASE/conf/context.xml

On reading the documentation tomcat::config::context suggests it might be something it can do:

Configure attributes for the Context element in $CATALINA_BASE/conf/context.xml

However it appears that this define type is hard coded to just set Context/WatchedResource/#text "WEB-INF/web.xml"

Describe the Solution You Would Like

The ability to set attributes of the Context element. Potentially through adding $additional_attributes and $attributes_to_remove to tomcat::config::context define type

Describe Alternatives You've Considered

I've not found another way of achieving this result using this module.

Additional Context

Describe the Bug

Property "name" don't exist in tomcat documentation and generate a warning

• documentation tomcat 9 link > https://tomcat.apache.org/tomcat-9.0-doc/config/resources.html

Expected Behavior

define tomcat::config::context::resources add a unexisting property "name"

Steps to Reproduce

tomcat::config::context::resources { 'resources':
  ensure                    => 'present',
  catalina_base         => '/opt/tomcat',
  additional_attributes => { 'cachingAllowed' => 'false' },
  notify                     => Service['tomcat'],
}

context.xml

<Context>
….
<Resources name="resources" cachingAllowed="false"></Resources>
</Context>

catalina.out
WARNING property name (value resources) can't fixed

AVERTISSEMENT [main] org.apache.tomcat.util.digester.SetPropertiesRule.begin La correspondance [Context/Resources] n'a pu fixer la propriété [name] à [resources]

Environment

• Debian 11
• Module tomcat 6.3.0
• Tomcat 9.0.67

Describe the Bug

The way archive is used with the installation from source does not work in my setup. It seems curl doesn't download the archive.

Expected Behavior

curl should download the archive which in turn should be extracted in place.

Environment

• module version 7.2.0
• Rocky Linux 9
• puppet 7.28.0
• archive 7.1.0
• curl 7.76.1 (x86_64-redhat-linux-gnu) libcurl/7.76.1 OpenSSL/3.0.7 zlib/1.2.11 brotli/1.0.9 libidn2/2.3.0 libpsl/0.21.1 (+libidn2/2.3.0) libssh/0.10.4/openssl/zlib nghttp2/1.43.0
• tar 1.34

I provided PR #559 as a workaround

Describe the Bug

It appears that the following classes act on the server.xml file before that file is actually copied to the instance's config directory from the home config directory. The result is a mis-formatted server.xml file that includes the connector, host and valve sections only, preventing tomcat from starting.

• tomcat::config::server::connector
• tomcat::config::server::host
• tomcat::config::server::valve

The only way to successfully deploy is using

    require => File["${catalina_base}/conf/server.xml"], # Added to solve a dependency problem in tomcat module

This file resource comes from tomcat::instance::copy_from_home, which suggests that the dependency is missing in this module.

Expected Behavior

Manipulation of server.xml (as well as other config files) should occur only after the tomcat::instance class has completed copying these files from the home config directory.

Steps to Reproduce

1. create tomcat:;instance
2. attempt to manipulate server.xml using tomcat::config::server::connector
3. Result is a server.xml file that only contains this stanza, instead of editing a previously-existing server.xml file

Environment

• Puppet 8
• mod 'puppetlabs-tomcat', '7.1.0'
• Ubuntu 22

Additional Context

This excerpt from my tomcat server configuration demonstrates the workaround.

...  

  ### MINGLE INSTANCE
  tomcat::instance { 'mingle':
    catalina_home         => $catalina_home,
    catalina_base         => $catalina_base,
    require               => File['Mingle Unit File'],
    service_name          => 'mingle',
    use_init              => true,
    manage_service        => true,
    manage_copy_from_home => true, # Copy initial config files from tomcat to this instance
    user                  => $t['user'],
    group                 => $t['user'],
  }

...

  # Listen Socket
  # - removes 'redirectPort' because this instance only listens on http
  # - adds 'relaxed' attributes to allow mingle to use '[]' in urls, which is now bad practice
  tomcat::config::server::connector { "mingle-http-${port['http']}": 
    catalina_base         => $catalina_base,
    port                  => $port['http'],     # Server socket port
    protocol              => 'HTTP/1.1',
    purge_connectors      => true,
    attributes_to_remove  => ['redirectPort'],  # This instance is behind a reverse proxy and only listens on http
    additional_attributes => {
      'relaxedPathChars'  => '[ ]',             # Mingle requires "relaxed path" characters
      'relaxedQueryChars' => '[ ]',
    },
    require               => File["${catalina_base}/conf/server.xml"], # Added to solve a dependency problem in tomcat module
  }

...

Describe the Bug

The puppetlabs-tomcat module states it has Puppet 8 compatibility, however it has a dependency upon a version of the puppet-archive module which doesn't support Puppet 8. Currently puppetlabs-tomcat 7.1.0 requires a version of puppet-archive >= 1.0.0 < 7.0.0. However puppet-archive is not Puppet 8 compatible until version 7.1.0, which is outside of the puppet-tomcat dependency range.