-
Notifications
You must be signed in to change notification settings - Fork 176
APEXCORE-488: Issues in SSL communication with StrAM #357
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -68,15 +68,30 @@ public static String getRMUsername(Configuration conf) | |
return principal; | ||
} | ||
|
||
public static String getSchemePrefix(YarnConfiguration conf) | ||
public static boolean isSSLEnabled(Configuration conf) | ||
{ | ||
if (HttpConfig.Policy.HTTPS_ONLY == HttpConfig.Policy.fromString( | ||
conf.get(YarnConfiguration.YARN_HTTP_POLICY_KEY, YarnConfiguration.YARN_HTTP_POLICY_DEFAULT))) { | ||
return true; | ||
} | ||
return false; | ||
} | ||
|
||
public static String getSchemePrefix(Configuration conf) | ||
{ | ||
if (HttpConfig.Policy.HTTPS_ONLY == HttpConfig.Policy.fromString(conf.get(YarnConfiguration.YARN_HTTP_POLICY_KEY, YarnConfiguration.YARN_HTTP_POLICY_DEFAULT))) { | ||
if (isSSLEnabled(conf)) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think @vrozov also pointed out this earlier, but this function is not required.. I see all the usages passing Yarnconfiguration There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you mean the getSchemePrefix(YarnConfiguration...) method? Needed for binary compatibility. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are you good with that explanation @gauravgopi123 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @PramodSSImmaneni : I am talking about getSchemePrefix(Configuration) method... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @PramodSSImmaneni : In-fact I won't make any change in this file. I would just add following code in StreamingAppMasterService
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @PramodSSImmaneni : I missed one thing. New method is needed as AppMaster has Configuration and YarnConfiguration object.. Changes look good |
||
return "https://"; | ||
} else { | ||
return "http://"; | ||
} | ||
} | ||
|
||
@Deprecated | ||
public static String getSchemePrefix(YarnConfiguration conf) | ||
{ | ||
return getSchemePrefix((Configuration)conf); | ||
} | ||
|
||
public static String getYarnLogDir() | ||
{ | ||
if (yarnLogDir != null) { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should be logged, saying falling back to http
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If it is a secure cluster, then fallback should be https?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Its not falling back. When protocol scheme is not specified in Tracking URL, it shall use the default protocol scheme i.e. 'http://'