[ https://issues.apache.org/jira/browse/HADOOP-13930?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15785624#comment-15785624 ]
Steve Loughran commented on HADOOP-13930: ----------------------------------------- initial review * I'm not sure about the name {{fs.azure.enable.authorization}}. I'd prefer something like {{fs.azure.authorization}} *for all azure auth properties. This would include {{"fs.azure.remote.auth.service.url"}}, as an example * {{NativeAzureFileSystem}} auth checks all look like cut & paste code. Better to have some method "void checkAuth(String operation) throws IOE {}" which is a no-op if auth isn't needed; checks and rejects if not. * {{RemoteWasbAuthorizerImpl}}. That exception handler should just be a single multipart {{catch(URISyntaxException | WasbRemoteCallException ...)}} handler * and as the WasB exceptions should all be IOEs, don't bother catch/wrap/rethrowing those * Auth interface methods should declare the right to raise IOEs, not just WasbAuthorizationException, h2. {{WasbRemoteCallHelper}} * check content type matches expected. proxies getting in the way could serve up TEXT/PLAIN and that needs to be rejected with a meaningful message, rather than an json parse exception. * look at content length, recognise & react to not enough content. As it is, an endpoint set to serve up an infinite amount of data on a GET Would overload the client heap, trivially * methods to declare throws IOE. * do include URL at fault in all exception messages. Your support team will appreciate this. h2. {{WasbAuthorizerInterface}} What does it mean if init fails? Better: make {{void}} return type, add {{throws IOE}}; allow implementations to do this h3. Testing * Please use {{ContractTestUtils}} for all assertions about FS state; it includes diagnostics on problems. * Your tests need to use paths which are unique for the test suite; using the working path doesn't do that. We need this for parallel test execution * test path must be robustly deleted in teardown; some appear to create, only one deletes. I know the other tests are expected to fail, but if they do create the path, there's trouble in the FS. * {{testRenameAccessCheckNegative}}: why catch/rethrow the exception? * {{testReadAccessCheckPositive()}}. In {{ContractTestUtils}} there's something to read data off a file. * No tests for {{RemoteWasbAuthorizerImpl}}. A mock remote {{WasbRemoteCallHelper}} could enable that; serve up different JSON docs, including invalid ones, HTTP errors, etc. > Azure: Add Authorization support to WASB > ---------------------------------------- > > Key: HADOOP-13930 > URL: https://issues.apache.org/jira/browse/HADOOP-13930 > Project: Hadoop Common > Issue Type: Improvement > Components: azure, fs/azure > Affects Versions: 2.8.0 > Reporter: Dushyanth > Assignee: Dushyanth > Attachments: HADOOP-13930.001.patch > > > As highlighted in HADOOP-13863, current implementation of WASB does not > support authorization to any File System operations. This jira is created to > add authorization support for WASB. The current approach is to enforce > authorization via an external REST service (One approach could be to use > component like Ranger to enforce authorization). The support for > authorization would be hiding behind a configuration flag : > "fs.azure.enable.authorization" and the remote service is expected to be > provided via config : "fs.azure.remote.auth.service.url". > The remote service is expected to provide support for the following REST > call: {URL}/CHECK_AUTHORIZATION``` > An example request: > {URL}/CHECK_AUTHORIZATION?wasb_absolute_path=<absolute_path>&operation_type=<operation > type>&delegation_token=<delegation token> -- This message was sent by Atlassian JIRA (v6.3.4#6332) --------------------------------------------------------------------- To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-issues-h...@hadoop.apache.org