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

KNOX-3051: Ability to extend classpath with configurable paths #971

Merged
merged 3 commits into from
Dec 5, 2024

Conversation

hanicz
Copy link
Contributor

@hanicz hanicz commented Dec 1, 2024

What changes were proposed in this pull request?

Classpath extension for patches

It would be convenient to have the ability to easily pre-pend classes/JARs to the Knox classpath for quickly applying/testing patches. This approach is completely configurable and multiple locations can be configured although their order has to be kept in mind to avoid overrides. Placing any class or JAR in these locations will take precedence on the classpath as it will come first. These locations can be within the Knox installation, external to it or even a mix of them.

The logic is checking the gateway-site.xml file for 'gateway.server.classpath.extension' property, if it is there it will extend the classpath with its value. The launcher only modifies the classpath for the GatewayServer. There is a check for the main class that's in the properties and if it is 'org.apache.knox.gateway.GatewayServer' only then the classpath is extended.

Example extension:

<property>
    <name>gateway.server.classpath.extension</name>
    <value>/new/classpath/jars/*.jar;../patch/*;/new/classpath/classes;</value>
</property>
  • The different locations should be separated by ',' or ';'.
  • '*.jar': picks up all jar files in the folder.
  • '*': picks up all files in the folder.
  • '/folder': class files are picked up from various folder hierarchies (eq. put GatewayServer.class in org/apache/knox/gateway folder)

How was this patch tested?

  • Unit tests
  • Tested locally with class and JAR files

Copy link
Contributor

@smolnar82 smolnar82 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@smolnar82
Copy link
Contributor

Hi @moresandeep , @pzampino - Could you please review this one? Thanks!

@smolnar82 smolnar82 requested a review from pzampino December 5, 2024 09:43
Copy link
Contributor

@pzampino pzampino left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@pzampino pzampino merged commit 4748771 into apache:master Dec 5, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants