Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixing wrong filename in module.xml for datasource oracle #153

Merged
merged 7 commits into from
Oct 30, 2020

Conversation

zaeh
Copy link
Contributor

@zaeh zaeh commented Aug 21, 2020

The filename of the driver jar for datasource oracle is fixed to "oracle.jar" in class keycloak::datasource::oracle.
This is not working with the fixed filename ojdbc8.jar in module.xml.

@treydock
Copy link
Owner

@zaeh I have no way to test this, and am concerned this could break other people's installs. I think the best approach here is to make that file a template and make the filename a parameter so it can easily be changed. If you need help on how to do that, let me know.

@zaeh
Copy link
Contributor Author

zaeh commented Aug 24, 2020

This is the way I need to do this at the moment, because of the bug.
The datasource "oracle" can not work at all without an own file/template for the module.xml given to the keycloak class as:
datasource_module_source => 'puppet:///${path_of_fixed_module.xml}'

As an alternative you could parameterize the value for the oracle.jar to make this working.

@treydock
Copy link
Owner

Right, my suggestion was parameterize the value. The Oracle support was a community contribution and I don't want to break other people's code in case their jar name is somehow different. If you it a parameter that would solve the problem and avoid breaking changes.

@treydock
Copy link
Owner

I am looking at the code, are you using datasource_jar_source? If you are using that parameter then oracle.jar should exist. I think the only reliable way is to ensure datasource_jar_source is set.

@treydock
Copy link
Owner

Just FYI that this change would most definitely break anyone using datasource_jar_source because that parameter will enforce the name of the jar to be oracle.jar.

@zaeh
Copy link
Contributor Author

zaeh commented Aug 25, 2020

Exactly that ist the problem:

  1. If you are using datasource_jar_source the name of the jar is fixed to oracle.jar
  2. The module.xml that ist used contains the hardcoded filename ojdbc8.jar (files/database/oracle/module.xml)
  3. JBoss will not start because it is trying to load ojdbc8.jar but the driver is named oracle.jar

Ok, I will parameterize the name of the jar, but nevertheless the name ojdbc8.jar hardcoded in the module.xml is wrong in nearly any case (newest driver is named ojdbc83.jar even at Oracle, afaik).

…dule xml to a template if not given by datasource_module_source
…dule xml to a template if not given by datasource_module_source
@zaeh
Copy link
Contributor Author

zaeh commented Aug 25, 2020

Done.
What do you think?

Co-authored-by: treydock <treydock@gmail.com>
@treydock
Copy link
Owner

treydock commented Sep 4, 2020

@zaeh Looks good. Would you mind adding an example of how you are using Oracle support to README.md? There are already examples for MySQL and PostgreSQL so can just add one example after those.

@treydock
Copy link
Owner

@zaeh, this is ready to merge, just needs README update with example of using Oracle similar to examples for MySQL and PostgreSQL.

@zaeh
Copy link
Contributor Author

zaeh commented Sep 22, 2020

Sorry, I was in holidays... Will add it in the next days! :-)

@treydock
Copy link
Owner

@zaeh Wanted to check if had a chance to make those README updates for this pull request.

@zaeh
Copy link
Contributor Author

zaeh commented Oct 30, 2020

Sorry, that it took so long!
I was very busy and just forgot about it.
Hope you like the Oracle usage example in the Readme.

@treydock treydock merged commit 044986b into treydock:master Oct 30, 2020
@treydock
Copy link
Owner

This will be released with v6.21.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants