Bug 893609

Summary: remove temp code that works around non-support of expressions when AS7 supports that
Product: [Other] RHQ Project Reporter: John Mazzitelli <mazz>
Component: Build System, InstallerAssignee: John Mazzitelli <mazz>
Status: ON_QA --- QA Contact: Mike Foley <mfoley>
Severity: unspecified Docs Contact:
Priority: unspecified    
Version: 4.5CC: hrupp
Target Milestone: ---   
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugzilla.redhat.com/show_bug.cgi?id=958494
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:

Description John Mazzitelli 2013-01-09 10:11:27 EST
AS7 does not support ${prop} expressions in several places where we need it. See the AS7 JIRA and its sub-tasks here - https://issues.jboss.org/browse/AS7-6120

We had to write some code to work around this, but it makes things ugly (e.g. users now have to run rhq-installer.sh --reconfig everytime they change one of these properties in rhq-server.properties).

Once AS7 supports expressions in the properties we expect them to work on, we should remove this temporary workaround code.

-----

Here is the code I know of that can go away once expressions are fully supported in AS7:

1) Remove this method from org.rhq.enterprise.server.installer.InstallerServiceImpl:

* reconfigure

2) Remove these methods from org.rhq.enterprise.server.installer.ServerInstallUtil

* isSameMailServiceExisting
* isSameDatasourceSecurityDomainExisting
* isSameWebConnectorsExisting

3) Find all code that calls:

* ServerInstallUtil.buildExpression(String, HashMap<String, String>, boolean)

where the boolean parameter "supportsExpression" is passed in as false and change those calls so the value is "true".

4) Fix code in:

org.rhq.common.jbossas.client.controller.SecurityDomainJBossASClient.updateSecureIdentitySecurityDomainCredentials(String, String, String)

so it checks to see if username or password has "${" in it and if so, use addExpression.

-----

The AS7 configuration settings that we need expression support but don't have it due to this AS7 problem are the following:

1. security domain's username and password
2. mail-smtp outgoing port's hostname
3. web connector's max-connections and redirect-port
4. most of web connector's ssl configuration attributes

To be more specific, here's the more technical details to those attributes mentioned above where we need expression support:

1. security domain's username/password values for a domain's login-module module-options:

/subsystem=security/security-domain=RHQDSSecurityDomain/authentication=classic
{
    "result" => {"login-modules" => [{
        "code" => "SecureIdentity",
        "flag" => "required",
        "module-options" => [
            ("username" => "rhqadmin"),
            ("password" => "1eeb2f255e832171df8592078de921bc")
        ]
    }]}
}

2. socket binding group remote dest. outbound socket binding's "host" attribute (curiously, the port attribute already DOES support expressions!):

/socket-binding-group=standard-sockets/remote-destination-outbound-socket-binding=mail-smtp
{
    "result" => {
        "host" => "localhost",
        "port" => expression "${rhq.server.email.smtp-port:25}"
    }
}

3. Web connector's max-connections and redirect-port:

/subsystem=web/connector=http
{
    "result" => {
        "max-connections" => 200,
        "redirect-port" => 7443,
    }
}

4. Web connector's ssl configuration (some of these allow for expressions, others don't - we need all of these that are not undefined to allow for expressions):

/subsystem=web/connector=https/ssl=configuration
{
    "result" => {
        "ca-certificate-file" => "/jbossas/standalone/configuration/rhq.truststore",
        "ca-certificate-password" => "RHQManagement",
        "ca-revocation-url" => undefined,
        "certificate-file" => undefined,
        "certificate-key-file" => "/jbossas/standalone/configuration/rhq.keystore",
        "cipher-suite" => undefined,
        "key-alias" => "RHQ",
        "keystore-type" => "JKS",
        "name" => undefined,
        "password" => "RHQManagement",
        "protocol" => "TLS",
        "session-cache-size" => undefined,
        "session-timeout" => undefined,
        "truststore-type" => "JKS",
        "verify-client" => "false",
        "verify-depth" => undefined
    }
}
Comment 1 John Mazzitelli 2013-01-09 10:51:31 EST
Here are some error messages when trying to use expressions in web connector ssl configuration attributes:

keystore-type:

Caused by: LifecycleException:  Protocol handler initialization failed: java.io.IOException: Failed to load keystore type ${x:JKS} with path /home/mazz/source/rhq/dev-container/jbossas/standalone/configuration/rhq.keystore due to ${x:JKS} not found

key-alias:

Caused by: LifecycleException:  Protocol handler initialization failed: java.io.IOException: Alias name ${x:RHQ} does not identify a key entry

I don't believe verify-client works with expressions - when I see the read-resource() on the ssl resource, I see:

        "verify-client" => "${x:false}",

Same with protocol:

        "protocol" => "${x:TLS}",

With those two, it doesn't say the value is an expression (if it is, it would say: expression "${x:TLS}") so I don't think these two work.

As for the password attribs, I do see this:

        "ca-certificate-password" => expression "${x:RHQManagement}",
        "password" => expression "${x:RHQManagement}",

so its possible passwords are working as expressions (I don't see any errors at startup about not being able to read the stores, so this might be working).
Comment 2 John Mazzitelli 2013-01-09 12:00:16 EST
One problem with ca-certificate-file and certificate-key-file is if you set that to:

  ${x}

and x is a system prop whose value is

  ${jboss.server.config.dir}/my.keystore

the web connector fails to start with:

Caused by: LifecycleException:  Protocol handler initialization failed: java.io.FileNotFoundException: ${jboss.server.config.dir}/my.keystore (No such file or directory)

So the web connector fails to do further sysprop expansion if a sysprop value itself has a sysprop expression in it.

For now, I think RHQ will assume that you can't use expressions in our own sys prop values for these file settings. You'll have to use absolute paths.
Comment 3 John Mazzitelli 2013-01-09 12:20:02 EST
another thing to remove is in rhq-container.build.xml - the target "add-auto-reconfig"
Comment 4 John Mazzitelli 2013-01-09 12:33:32 EST
regarding comment #2 - I'll leave it the way it works now (we'll reconfigure the setting with an absolute path - if you just give a relative path, we assume its relative to the config dir).

But, we will need to change some code inside

org.rhq.enterprise.server.installer.ServerInstallUtil.buildSecureConnectorConfiguration

once AS7 is fixed. The keystore/truststore attributes has a problem. They actually do support expressions like "${jboss.server.config.dir}" but our problem is we use properties like "rhq.server.tomcat.security.keystore.file" and its value itself could have an expression in it like jboss.server.config.dir. This doesn't work in 7.1.1.Final and is supposed to be fixed in 7.1.2.Final, as per:

https://issues.jboss.org/browse/AS7-6127?focusedCommentId=12744492&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-12744492

   "what you are asking for is different issue but that was fixed in 7.1.2
see AS7-6233 and AS7-5665 for that

as goes for web subsystem you can see all attributes that support expression by looking at what we test for https://github.com/jbossas/jboss-as/blob/master/web/src/test/resources/org/jboss/as/web/test/subsystem.xml"
Comment 5 John Mazzitelli 2013-04-30 11:11:53 EDT
another place where expression support is needed is:

/subsystem=logging/logger=org.rhq
Comment 6 John Mazzitelli 2013-04-30 11:17:06 EDT
If we remove the installer --reconfig workaround, the following wiki pages need to be updated to remove references to that option:

https://docs.jboss.org/author/display/RHQ/Running+The+Installer
https://docs.jboss.org/author/display/RHQ/Securing+Communications
https://docs.jboss.org/author/display/RHQ/Startup+Properties
Comment 7 John Mazzitelli 2013-04-30 13:59:32 EDT
regarding comment #2 and comment #4, this is still not fixed in EAP 6.1 Alpha. There is a WildFly issue for what we need:

https://issues.jboss.org/browse/WFLY-1177
Comment 8 John Mazzitelli 2013-05-01 11:30:17 EDT
to test and see if there are expressions in the resources of interest, these are the CLI commands to run:

./jboss-cli.sh --connect --controller=localhost:6999 '/subsystem=security/security-domain=RHQDSSecurityDomain/authentication=classic:read-resource()'

./jboss-cli.sh --connect --controller=localhost:6999 '/socket-binding-group=standard-sockets/remote-destination-outbound-socket-binding=mail-smtp:read-resource()'

./jboss-cli.sh --connect --controller=localhost:6999 '/subsystem=web/connector=http:read-resource()'

./jboss-cli.sh --connect --controller=localhost:6999 '/subsystem=web/connector=https/ssl=configuration:read-resource()'

./jboss-cli.sh --connect --controller=localhost:6999 '/subsystem=logging/logger=org.rhq:read-resource()'
Comment 9 John Mazzitelli 2013-05-01 12:04:37 EDT
Here's the resource data after the fixes - the server runs fine which means "almost" everything has been fixed in EAP 6.1.alpha (see comment #7 for what wasn't fixed - you can see we still work around it here in the ssl=configuration resource below):

$ ./jboss-cli.sh --connect --controller=localhost:6999 '/subsystem=security/security-domain=RHQDSSecurityDomain/authentication=classic:read-resource()'

{
    "outcome" => "success",
    "result" => {
        "login-modules" => [{
            "code" => "SecureIdentity",
            "flag" => "required",
            "module" => undefined,
            "module-options" => {
                "username" => expression "${rhq.server.database.user-name:rhqadmin}",
                "password" => expression "${rhq.server.database.password:1eeb2f255e832171df8592078de921bc}"
            }
        }],
        "login-module" => {"SecureIdentity" => undefined}
    }
}

$ ./jboss-cli.sh --connect --controller=localhost:6999 '/socket-binding-group=standard-sockets/remote-destination-outbound-socket-binding=mail-smtp:read-resource()'
{
    "outcome" => "success",
    "result" => {
        "fixed-source-port" => false,
        "host" => expression "${rhq.server.email.smtp-host:localhost}",
        "port" => expression "${rhq.server.email.smtp-port:25}",
        "source-interface" => undefined,
        "source-port" => undefined
    }
}

$ ./jboss-cli.sh --connect --controller=localhost:6999 '/subsystem=web/connector=http:read-resource()'
{
    "outcome" => "success",
    "result" => {
        "configuration" => undefined,
        "enable-lookups" => false,
        "enabled" => true,
        "executor" => undefined,
        "max-connections" => expression "${rhq.server.startup.web.max-connections:200}",
        "max-post-size" => 2097152,
        "max-save-post-size" => 4096,
        "name" => "http",
        "protocol" => "HTTP/1.1",
        "proxy-name" => undefined,
        "proxy-port" => undefined,
        "redirect-port" => expression "${rhq.server.socket.binding.port.https:7443}",
        "scheme" => "http",
        "secure" => false,
        "socket-binding" => "http",
        "ssl" => undefined,
        "virtual-server" => undefined
    }
}

$ ./jboss-cli.sh --connect --controller=localhost:6999 '/subsystem=web/connector=https/ssl=configuration:read-resource()'
{
    "outcome" => "success",
    "result" => {
        "ca-certificate-file" => "/home/mazz/source/rhq/dev-container/jbossas/standalone/configuration/rhq.truststore",
        "ca-certificate-password" => expression "${rhq.server.tomcat.security.truststore.password:RHQManagement}",
        "ca-revocation-url" => undefined,
        "certificate-file" => undefined,
        "certificate-key-file" => "/home/mazz/source/rhq/dev-container/jbossas/standalone/configuration/rhq.keystore",
        "cipher-suite" => undefined,
        "key-alias" => expression "${rhq.server.tomcat.security.keystore.alias:RHQ}",
        "keystore-type" => expression "${rhq.server.tomcat.security.keystore.type:JKS}",
        "name" => undefined,
        "password" => expression "${rhq.server.tomcat.security.keystore.password:RHQManagement}",
        "protocol" => expression "${rhq.server.tomcat.security.secure-socket-protocol:TLS}",
        "session-cache-size" => undefined,
        "session-timeout" => undefined,
        "truststore-type" => expression "${rhq.server.tomcat.security.truststore.type:JKS}",
        "verify-client" => expression "${rhq.server.tomcat.security.client-auth-mode:false}",
        "verify-depth" => undefined
    }
}

$ ./jboss-cli.sh --connect --controller=localhost:6999 '/subsystem=logging/logger=org.rhq:read-resource()'
{
    "outcome" => "success",
    "result" => {
        "category" => undefined,
        "filter" => undefined,
        "filter-spec" => undefined,
        "handlers" => undefined,
        "level" => expression "${rhq.server.log-level:INFO}",
        "use-parent-handlers" => true
    }
}

Note that, as per comment #7, ca-certificate-file and certificate-key-file attributes in the ssl=configuration resource are evaluated and their actual values are stored, we cannot use ${x} notation here. So, if users want to change the name or location of their keystore/truststore, they will have to hand-edit standalone-full.xml to have the change take effect. I added a comment in rhq-server.properties to let users know this. This is a minor issue though because people will rarely, if ever, need to rename these files. And if they need to, there is an easy way to do it (just edit standalone-full.xml).
Comment 10 John Mazzitelli 2013-05-01 12:15:37 EDT
git commit to master: 4c50679d28c84c2d0057c2438bb985c562fc8ac5
Comment 11 Heiko W. Rupp 2013-05-01 13:04:02 EDT
I'd like to see some testing around this before RHQ 4.7 is released. 
While I am sure mazz did a good job here, install failures for users will bring us bad press, so the most common install scenarios need testing
(fresh install, upgrade, ??)
Comment 12 John Mazzitelli 2013-05-01 13:11:07 EDT
Updated all the relevant wiki pages as appropriate. However, since WildFly/EAP still does not support recursive evaluations of properties, we still have two settings that won't take effect by modifying rhq-server.properties after the initial install (see bug #958494 where I talk about this). In the wiki pages, I added a blurb about this.

This issue can now be considered ready for QA. All wiki docs updates and code changes are done.