[ https://issues.apache.org/jira/browse/YARN-2277?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14093199#comment-14093199 ]
Zhijie Shen commented on YARN-2277: ----------------------------------- bq. I have provided a minimal CORS filter that will give us an idea if this is the direction to go. Based on the direction of this patch, the scope has widened to create a general CrossOriginFilter for use within all Hadoop REST APIs. Probably, we will want to split the different pieces us across JIRAs, umbrella, Filter and FilterInitializer, additional configuration, and individual REST servers. This way we can focus on the end goal of getting Tez UI done in a timely manner without forgetting completeness of CORS support. [~jeagles], thanks for your contribution! +1 for making the minimal CORS filter. Another concern is that if we upgrade jetty sometime in the future, we can reuse the cross-origin filter provided by it, rebase this on top of it. One additional suggestion is that we can start the CORS filter even in smaller scope: the timeline server only, which means we should move the filter/filter initializer to this sub module. Once it is made robust enough and proved to be reliable, we can promote it to hadoop-yarn-common or even hadoop-common. How do you think? Bellow are some detailed comments for the patch: 1. The prefix changes to "yarn.timeline-service.http.cross-origin"? {code} + public static final String PREFIX = "hadoop.http.filter.cross.origin."; {code} 2. ALLOWED_ORIGINS -> "allowed-origins"? Not to make the config name too long. {code} + // Filter configuration + public static final String ALLOWED_ORIGINS = "access.control.allowed.origins"; {code} 3. Should most of the methods in CrossOriginFilter be private? 4. Is it better to make it configurable as well? Why allowedMethods doesn't have PUT? In the doc: https://developer.mozilla.org/en-US/docs/Web/HTTP/Access_control_CORS, it seems that the headers can go beyond the following set. {code} + void initializeAllowedMethods(FilterConfig filterConfig) { + allowedMethods.add("GET"); + allowedMethods.add("POST"); + allowedMethods.add("HEAD"); + LOG.info("Allowed Methods: " + getAllowedMethodsHeader()); + } + + void initializeAllowedHeaders(FilterConfig filterConfig) { + allowedHeaders.add("X-Requested-With"); + allowedHeaders.add("Content-Type"); + allowedHeaders.add("Accept"); + allowedHeaders.add("Origin"); + LOG.info("Allowed Headers: " + getAllowedHeadersHeader()); + } {code} 5. Should we include Access-Control-Max-Age? 6. Is it better to invoke doCrossFilter after chain.doFilter, in case devs are going to do something special in servlet with the res object directly? {code} + @Override + public void doFilter(ServletRequest req, ServletResponse res, FilterChain chain) + throws IOException, ServletException { + doCrossFilter((HttpServletRequest) req, (HttpServletResponse) res); + chain.doFilter(req, res); + } {code} > Add Cross-Origin support to the ATS REST API > -------------------------------------------- > > Key: YARN-2277 > URL: https://issues.apache.org/jira/browse/YARN-2277 > Project: Hadoop YARN > Issue Type: Sub-task > Reporter: Jonathan Eagles > Assignee: Jonathan Eagles > Attachments: YARN-2277-CORS.patch, YARN-2277-JSONP.patch, > YARN-2277-v2.patch, YARN-2277-v3.patch, YARN-2277-v3.patch, > YARN-2277-v4.patch, YARN-2277-v5.patch, YARN-2277-v6.patch > > > As the Application Timeline Server is not provided with built-in UI, it may > make sense to enable JSONP or CORS Rest API capabilities to allow for remote > UI to access the data directly via javascript without cross side server > browser blocks coming into play. > Example client may be like > http://api.jquery.com/jQuery.getJSON/ > This can alleviate the need to create a local proxy cache. -- This message was sent by Atlassian JIRA (v6.2#6252)