Skip to content

ZEPPELIN-1319 Use absolute path for ssl truststore and keystore when available#1319

Closed
r-kamath wants to merge 2 commits intoapache:masterfrom
r-kamath:ZEPPELIN-1319
Closed

ZEPPELIN-1319 Use absolute path for ssl truststore and keystore when available#1319
r-kamath wants to merge 2 commits intoapache:masterfrom
r-kamath:ZEPPELIN-1319

Conversation

@r-kamath
Copy link
Member

What is this PR for?

Use absolute path for ssl truststore and keystore when available

What type of PR is it?

Improvement

Todos

  • - Task

What is the Jira issue?

https://issues.apache.org/jira/browse/ZEPPELIN-1319

How should this be tested?

Config zeppelin.ssl.truststore.path, zeppelin.ssl.keystore.path and verify whether the absolute path or the path relative to conf is used.

Screenshots (if appropriate)

Questions:

  • Does the licenses files need update? n/a
  • Is there breaking changes for older versions? n/a
  • Does this needs documentation? n/a

@prabhjyotsingh
Copy link
Contributor

Good catch, LGTM!

CI failure looks unrelated.

getConfDir(),
getString(ConfVars.ZEPPELIN_SSL_KEYSTORE_PATH)));
String path = getString(ConfVars.ZEPPELIN_SSL_KEYSTORE_PATH);
if (path != null && path.startsWith("/")) {
Copy link
Member

Choose a reason for hiding this comment

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

does it need to consider Windows?

Copy link
Member Author

Choose a reason for hiding this comment

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

@felixcheung check for windows path is handled in getRelativeDir

Copy link
Member

Choose a reason for hiding this comment

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

hmm, if path == C:\zeppelin\conf\ssh
isn't this turning it into a broken path like c:\zeppelin\conf/c:\zeppelin\conf\ssh?

getRelativeDir(
           String.format("%s/%s",
               getConfDir(),
               getString(path)));

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed in 5587d5a

@corneadoug
Copy link
Contributor

@prabhjyotsingh Should we merge this?

@prabhjyotsingh
Copy link
Contributor

@corneadoug Yes, sure. Will merge this soon if no more discussion.

@asfgit asfgit closed this in 922364f Sep 2, 2016
asfgit pushed a commit that referenced this pull request Sep 2, 2016
…available

### What is this PR for?
Use absolute path for ssl truststore and keystore when available

### What type of PR is it?
Improvement

### Todos
* [ ] - Task

### What is the Jira issue?
https://issues.apache.org/jira/browse/ZEPPELIN-1319

### How should this be tested?
Config `zeppelin.ssl.truststore.path`, `zeppelin.ssl.keystore.path` and verify whether the absolute path or the path relative to conf is used.

### Screenshots (if appropriate)

### Questions:
* Does the licenses files need update? n/a
* Is there breaking changes for older versions? n/a
* Does this needs documentation? n/a

Author: Renjith Kamath <renjith.kamath@gmail.com>

Closes #1319 from r-kamath/ZEPPELIN-1319 and squashes the following commits:

5587d5a [Renjith Kamath] ZEPPELIN-1319 add check for Windows path
fc2ac9f [Renjith Kamath] ZEPPELIN-1319 Use absolute path for ssl truststore and keystore when available

(cherry picked from commit 922364f)
Signed-off-by: Prabhjyot Singh <prabhjyotsingh@gmail.com>
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.

4 participants