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-3024 - Fixed Java finding issues #891

Merged
merged 1 commit into from
Mar 21, 2024
Merged

Conversation

smolnar82
Copy link
Contributor

What changes were proposed in this pull request?

I updated the existing login in knox-functions.sh to successfully check Java executables under /usr if no JAVA_HOME is set or Java is not available on the path.
In addition to fixing the issue, I added an option to display the Java with the --verbose option (when using KnoxCLI, --verbose true should be used.

How was this patch tested?

Checked the updated scripts on shellcheck.net and tested them manually:

  • gateway.sh
$ bin/gateway.sh restart --verbose
Found Java at /usr/local/opt/openjdk@8/bin/java
Stopping Gateway with PID 12173 succeeded.
Starting Gateway succeeded with PID 12346.

$ bin/gateway.sh restart
Stopping Gateway with PID 12346 succeeded.
Starting Gateway succeeded with PID 12419.
  • knoxcli.sh
$ bin/knoxcli.sh export-cert --type JKS --verbose true
Found Java at /usr/local/opt/openjdk@8/bin/java
Certificate gateway-identity has been successfully exported to: /Users/sandormolnar/test/knoxGateway/data/security/keystores/gateway-client-trust.jks

$ bin/knoxcli.sh export-cert --type JKS
Certificate gateway-identity has been successfully exported to: /Users/sandormolnar/test/knoxGateway/data/security/keystores/gateway-client-trust.jks
  • ldap.sh
$ bin/ldap.sh start --verbose
Found Java at /usr/local/opt/openjdk@8/bin/java
Starting LDAP succeeded with PID 12531.

$ bin/ldap.sh stop
Stopping LDAP with PID 12531 succeeded.

$ bin/ldap.sh start
Starting LDAP succeeded with PID 12573.

Copy link
Contributor

@moresandeep moresandeep left a comment

Choose a reason for hiding this comment

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

Looks good!

@@ -50,6 +50,17 @@ DEFAULT_APP_STATUS_TEST_RETRY_SLEEP=2
##### common functions #####
############################

function setVerbose() {
Copy link
Contributor

Choose a reason for hiding this comment

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

nitpick: May be accommodate shortform as well -v

Copy link
Contributor Author

Choose a reason for hiding this comment

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

KnoxCLI only supports --verbose, hence the long version only. Maybe later...

Copy link
Contributor

Choose a reason for hiding this comment

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

i see makes sense :)

@smolnar82 smolnar82 merged commit 1e9a39b into apache:master Mar 21, 2024
2 checks passed
@smolnar82 smolnar82 deleted the KNOX-3024 branch March 21, 2024 13:16
stoty pushed a commit to stoty/knox that referenced this pull request May 14, 2024
Change-Id: Ic1f1bc29ecd41d0eb2d64521d70c7edd00709a66
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.

2 participants