You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hawq.apache.org by interma <gi...@git.apache.org> on 2017/01/11 06:53:52 UTC

[GitHub] incubator-hawq pull request #1081: HAWQ-1257. Prompt all tables which user d...

GitHub user interma opened a pull request:

    https://github.com/apache/incubator-hawq/pull/1081

    HAWQ-1257. Prompt all tables which user doesn't have right once

    

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/interma/interma-hawq HAWQ-1257

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/incubator-hawq/pull/1081.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #1081
    
----
commit baef5a3925b370270dbb8d7ed659df705b422e89
Author: interma <in...@outlook.com>
Date:   2017-01-11T06:51:21Z

    HAWQ-1257. Prompt all tables which user doesn't have right once

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq pull request #1081: HAWQ-1257. Prompt all tables which user d...

Posted by interma <gi...@git.apache.org>.
Github user interma commented on a diff in the pull request:

    https://github.com/apache/incubator-hawq/pull/1081#discussion_r95715673
  
    --- Diff: src/backend/libpq/rangerrest.c ---
    @@ -381,29 +449,38 @@ int call_ranger_rest(CURL_HANDLE curl_handle, const char* request)
     }
     
     /*
    - * arg_list: List of RangerRequestJsonArgs
    + * check privilege(s) from ranger
    + * @param	request_list	List of RangerRequestJsonArgs
    + * @param	result_list		List of RangerPrivilegeResults
    + * @return	0 get response from ranger and parse success; -1 other error
      */
    -int check_privilege_from_ranger(List *arg_list)
    +int check_privilege_from_ranger(List *request_list, List *result_list)
     {
    -	json_object* jrequest = create_ranger_request_json(arg_list);
    +	json_object* jrequest = create_ranger_request_json(request_list, result_list);
     	Assert(jrequest != NULL);
    +
     	const char *request = json_object_to_json_string(jrequest);
    -	elog(LOG, "Send JSON request to Ranger: %s", request);
     	Assert(request != NULL);
    +	elog(LOG, "Send JSON request to Ranger: %s", request);
    --- End diff --
    
    Fixed!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq pull request #1081: HAWQ-1257. Prompt all tables which user d...

Posted by linwen <gi...@git.apache.org>.
Github user linwen commented on a diff in the pull request:

    https://github.com/apache/incubator-hawq/pull/1081#discussion_r95529591
  
    --- Diff: src/include/utils/rangerrest.h ---
    @@ -80,6 +80,13 @@ typedef struct RangerPrivilegeResults
     {
       RangerACLResult result;
       Oid relOid;
    +
    +  /* 
    +   * string_hash of access[i] field of ranger request 
    +   * use the sigh to identify each resource result
    --- End diff --
    
    sign, not sigh


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq pull request #1081: HAWQ-1257. Prompt all tables which user d...

Posted by linwen <gi...@git.apache.org>.
Github user linwen commented on a diff in the pull request:

    https://github.com/apache/incubator-hawq/pull/1081#discussion_r95713954
  
    --- Diff: src/backend/libpq/rangerrest.c ---
    @@ -381,29 +449,38 @@ int call_ranger_rest(CURL_HANDLE curl_handle, const char* request)
     }
     
     /*
    - * arg_list: List of RangerRequestJsonArgs
    + * check privilege(s) from ranger
    + * @param	request_list	List of RangerRequestJsonArgs
    + * @param	result_list		List of RangerPrivilegeResults
    + * @return	0 get response from ranger and parse success; -1 other error
      */
    -int check_privilege_from_ranger(List *arg_list)
    +int check_privilege_from_ranger(List *request_list, List *result_list)
     {
    -	json_object* jrequest = create_ranger_request_json(arg_list);
    +	json_object* jrequest = create_ranger_request_json(request_list, result_list);
     	Assert(jrequest != NULL);
    +
     	const char *request = json_object_to_json_string(jrequest);
    -	elog(LOG, "Send JSON request to Ranger: %s", request);
     	Assert(request != NULL);
    +	elog(LOG, "Send JSON request to Ranger: %s", request);
    +
     	struct curl_context_t curl_context;
     	memset(&curl_context, 0, sizeof(struct curl_context_t));
     
     	/* call GET method to send request*/
     	if (call_ranger_rest(&curl_context, request) < 0)
     	{
    -		return RANGERCHECK_NO_PRIV;
    +		return -1;
     	}
     
     	/* free the JSON object */
     	json_object_put(jrequest);
     
     	/* parse the JSON-format result */
    -	RangerACLResult ret = parse_ranger_response(curl_context.response.buffer);
    +	int ret = parse_ranger_response(curl_context.response.buffer, result_list);
    +	if (ret < 0) 
    +	{
    +		elog(WARNING, "parse_ranger_response() failed");
    --- End diff --
    
    agree.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq pull request #1081: HAWQ-1257. Prompt all tables which user d...

Posted by interma <gi...@git.apache.org>.
Github user interma commented on a diff in the pull request:

    https://github.com/apache/incubator-hawq/pull/1081#discussion_r95521660
  
    --- Diff: src/include/utils/rangerrest.h ---
    @@ -80,6 +80,13 @@ typedef struct RangerPrivilegeResults
     {
       RangerACLResult result;
       Oid relOid;
    +
    +  /* 
    +   * string_hash of access[i] field of ranger request 
    +   * use the sigh to identify each resource result
    +   */ 
    +  uint32 resource_sign;
    +  uint32 privilege_sign;
    --- End diff --
    
    On a batch ranger acl check request/response:
    The acl check result order isn't the same as request order.
    When reponse's resource&privilege field is same as request, then we consider them is a pair.
    
    So we need storage resource&privilege sign in here.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq pull request #1081: HAWQ-1257. Prompt all tables which user d...

Posted by stanlyxiang <gi...@git.apache.org>.
Github user stanlyxiang commented on a diff in the pull request:

    https://github.com/apache/incubator-hawq/pull/1081#discussion_r95528917
  
    --- Diff: src/backend/libpq/rangerrest.c ---
    @@ -68,51 +69,99 @@ static void getClientIP(char *remote_host)
     	}
     }
     
    -RangerACLResult parse_ranger_response(char* buffer)
    +/*
    + * parse ranger response
    + * @param	buffer	ranger response 	
    + * @param	result_list		List of RangerPrivilegeResults
    + * @return	0 parse success; -1 other error
    + */
    +static int parse_ranger_response(char* buffer, List *result_list)
     {
     	if (buffer == NULL || strlen(buffer) == 0)
    -		return RANGERCHECK_UNKNOWN;
    +		return -1;
    --- End diff --
    
    I don't think -1 is more readable than "RANGERCHECK_UNKNOWN".


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq issue #1081: HAWQ-1257. Prompt all tables which user doesn't ...

Posted by linwen <gi...@git.apache.org>.
Github user linwen commented on the issue:

    https://github.com/apache/incubator-hawq/pull/1081
  
    +1


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq pull request #1081: HAWQ-1257. Prompt all tables which user d...

Posted by interma <gi...@git.apache.org>.
Github user interma commented on a diff in the pull request:

    https://github.com/apache/incubator-hawq/pull/1081#discussion_r95547972
  
    --- Diff: src/backend/catalog/aclchk.c ---
    @@ -2712,15 +2712,20 @@ List *pg_rangercheck_batch(List *arg_list)
       ListCell *arg;
       foreach(arg, arg_list) {
         RangerPrivilegeArgs *arg_ptr = (RangerPrivilegeArgs *) lfirst(arg);
    +
         AclObjectKind objkind = arg_ptr->objkind;
         Oid object_oid = arg_ptr->object_oid;
         char *objectname = getNameFromOid(objkind, object_oid);
         char *rolename = getRoleName(arg_ptr->roleid);
         List* actions = getActionName(arg_ptr->mask);
         bool isAll = (arg_ptr->how == ACLMASK_ALL) ? true: false;
    +
         RangerPrivilegeResults *aclresult = (RangerPrivilegeResults *) palloc(sizeof(RangerPrivilegeResults));
    -    aclresult->result = -1;
    +    aclresult->result = RANGERCHECK_NO_PRIV;
         aclresult->relOid = object_oid;
    +	// this two sign fields will be set in create_ranger_request_json()
    --- End diff --
    
    fixed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq issue #1081: HAWQ-1257. Prompt all tables which user doesn't ...

Posted by interma <gi...@git.apache.org>.
Github user interma commented on the issue:

    https://github.com/apache/incubator-hawq/pull/1081
  
    My main modified is commented overhead.
    @paul-guo- @stanlyxiang @linwen @zhangh43 pls review, thanks!



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq pull request #1081: HAWQ-1257. Prompt all tables which user d...

Posted by interma <gi...@git.apache.org>.
Github user interma commented on a diff in the pull request:

    https://github.com/apache/incubator-hawq/pull/1081#discussion_r95716521
  
    --- Diff: src/backend/catalog/aclchk.c ---
    @@ -2760,9 +2766,6 @@ List *pg_rangercheck_batch(List *arg_list)
         requestargs = NULL;
       }
     
    -  if(ret != RANGERCHECK_OK){
    -    elog(ERROR, "ACL check failed\n");
    -  }
       elog(LOG, "oids%d\n", arg_list->length);
    --- End diff --
    
    Fixed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq issue #1081: HAWQ-1257. Prompt all tables which user doesn't ...

Posted by paul-guo- <gi...@git.apache.org>.
Github user paul-guo- commented on the issue:

    https://github.com/apache/incubator-hawq/pull/1081
  
    I think some coding style (please follow the pg coding style) should be uniform.
    At least, e.g.:
    1) do not mix // and /*
    2) do not have the function return type be on the same line of the declaration.
    List *
    deconstruct_jointree(PlannerInfo *root)
    3) Please make sure whether ereport instead of elog should be used.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq pull request #1081: HAWQ-1257. Prompt all tables which user d...

Posted by interma <gi...@git.apache.org>.
Github user interma commented on a diff in the pull request:

    https://github.com/apache/incubator-hawq/pull/1081#discussion_r95522001
  
    --- Diff: src/backend/parser/parse_relation.c ---
    @@ -2714,12 +2714,12 @@ warnAutoRange(ParseState *pstate, RangeVar *relation, int location)
     void
     ExecCheckRTPerms(List *rangeTable)
     {
    -  if (enable_ranger && !fallBackToNativeChecks(ACL_KIND_CLASS,rangeTable,GetUserId()))
    -  {
    -    if(rangeTable!=NULL)
    -      ExecCheckRTPermsWithRanger(rangeTable);
    -    return;
    -  }
    +	if (enable_ranger && !fallBackToNativeChecks(ACL_KIND_CLASS,rangeTable,GetUserId()))
    +	{
    +		if(rangeTable!=NULL)
    +			ExecCheckRTPermsWithRanger(rangeTable);
    +		return;
    +	}
    --- End diff --
    
    Only the two function used 2space ident, and other used Tab.
    So I unify them to Tab.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq pull request #1081: HAWQ-1257. Prompt all tables which user d...

Posted by linwen <gi...@git.apache.org>.
Github user linwen commented on a diff in the pull request:

    https://github.com/apache/incubator-hawq/pull/1081#discussion_r95714867
  
    --- Diff: src/backend/catalog/aclchk.c ---
    @@ -2760,9 +2766,6 @@ List *pg_rangercheck_batch(List *arg_list)
         requestargs = NULL;
       }
     
    -  if(ret != RANGERCHECK_OK){
    -    elog(ERROR, "ACL check failed\n");
    -  }
       elog(LOG, "oids%d\n", arg_list->length);
    --- End diff --
    
    This log info is ambiguous.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq issue #1081: HAWQ-1257. Prompt all tables which user doesn't ...

Posted by interma <gi...@git.apache.org>.
Github user interma commented on the issue:

    https://github.com/apache/incubator-hawq/pull/1081
  
    All fixed, pls review again.
    Thanks!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq pull request #1081: HAWQ-1257. Prompt all tables which user d...

Posted by linwen <gi...@git.apache.org>.
Github user linwen commented on a diff in the pull request:

    https://github.com/apache/incubator-hawq/pull/1081#discussion_r95714559
  
    --- Diff: src/backend/parser/parse_relation.c ---
    @@ -2734,70 +2734,99 @@ ExecCheckRTPerms(List *rangeTable)
     void
     ExecCheckRTPermsWithRanger(List *rangeTable)
     {
    -  List *ranger_check_args = NIL;
    -  ListCell *l;
    -  foreach(l, rangeTable)
    -  {
    -
    -    AclMode requiredPerms;
    -    Oid relOid;
    -    Oid userid;
    -    RangeTblEntry *rte = (RangeTblEntry *) lfirst(l);
    -
    -    if (rte->rtekind != RTE_RELATION)
    -      continue;
    -    requiredPerms = rte->requiredPerms;
    -    if (requiredPerms == 0)
    -      continue;
    -    
    -    relOid = rte->relid;
    -    userid = rte->checkAsUser ? rte->checkAsUser : GetUserId();
    -
    -    RangerPrivilegeArgs *ranger_check_arg = (RangerPrivilegeArgs *) palloc(sizeof(RangerPrivilegeArgs));
    -    ranger_check_arg->objkind = ACL_KIND_CLASS;
    -    ranger_check_arg->object_oid = relOid;
    -    ranger_check_arg->roleid = userid;
    -    ranger_check_arg->mask = requiredPerms;
    -    ranger_check_arg->how = ACLMASK_ALL;
    -    ranger_check_args = lappend(ranger_check_args, ranger_check_arg);
    -
    -  } // foreach
    -
    -  if (ranger_check_args == NIL)
    -    return;
    -
    -  // ranger ACL check with package Oids
    -  List *aclresults = NIL;
    -  aclresults = pg_rangercheck_batch(ranger_check_args);
    -  if (aclresults == NIL)
    -  {
    -    elog(ERROR, "ACL check failed\n");
    -    return;
    -  }
    -
    -  // check result
    -  ListCell *result;
    -  foreach(result, aclresults)
    -  {
    -    RangerPrivilegeResults *result_ptr = (RangerPrivilegeResults *) lfirst(result);
    -    if(result_ptr->result != RANGERCHECK_OK)
    -    {
    -      Oid relOid = result_ptr->relOid;
    -      const char *rel_name = get_rel_name_partition(relOid);
    -      aclcheck_error(ACLCHECK_NO_PRIV, ACL_KIND_CLASS, rel_name);
    -    }
    -  }
    -  
    -  if (ranger_check_args)
    -  {
    -    list_free_deep(ranger_check_args);
    -    ranger_check_args = NIL;
    -  }
    -  if (aclresults)
    -  {
    -    list_free_deep(aclresults);
    -    aclresults = NIL;
    -  }
    +	List *ranger_check_args = NIL;
    +	ListCell *l;
    +	foreach(l, rangeTable)
    +	{
    +
    +		AclMode requiredPerms;
    +		Oid relOid;
    +		Oid userid;
    +		RangeTblEntry *rte = (RangeTblEntry *) lfirst(l);
    +
    +		if (rte->rtekind != RTE_RELATION)
    +			continue;
    +		requiredPerms = rte->requiredPerms;
    +		if (requiredPerms == 0)
    +			continue;
    +
    +		relOid = rte->relid;
    +		userid = rte->checkAsUser ? rte->checkAsUser : GetUserId();
    +
    +		RangerPrivilegeArgs *ranger_check_arg = (RangerPrivilegeArgs *) palloc(sizeof(RangerPrivilegeArgs));
    +		ranger_check_arg->objkind = ACL_KIND_CLASS;
    +		ranger_check_arg->object_oid = relOid;
    +		ranger_check_arg->roleid = userid;
    +		ranger_check_arg->mask = requiredPerms;
    +		ranger_check_arg->how = ACLMASK_ALL;
    +		ranger_check_args = lappend(ranger_check_args, ranger_check_arg);
    +
    +	} // foreach
    +
    +	if (ranger_check_args == NIL)
    +		return;
    +
    +	// ranger ACL check with package Oids
    --- End diff --
    
    this comment should use /**/ instead of // 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq pull request #1081: HAWQ-1257. Prompt all tables which user d...

Posted by interma <gi...@git.apache.org>.
Github user interma commented on a diff in the pull request:

    https://github.com/apache/incubator-hawq/pull/1081#discussion_r95716306
  
    --- Diff: src/backend/libpq/rangerrest.c ---
    @@ -381,29 +449,38 @@ int call_ranger_rest(CURL_HANDLE curl_handle, const char* request)
     }
     
     /*
    - * arg_list: List of RangerRequestJsonArgs
    + * check privilege(s) from ranger
    + * @param	request_list	List of RangerRequestJsonArgs
    + * @param	result_list		List of RangerPrivilegeResults
    + * @return	0 get response from ranger and parse success; -1 other error
      */
    -int check_privilege_from_ranger(List *arg_list)
    +int check_privilege_from_ranger(List *request_list, List *result_list)
     {
    -	json_object* jrequest = create_ranger_request_json(arg_list);
    +	json_object* jrequest = create_ranger_request_json(request_list, result_list);
     	Assert(jrequest != NULL);
    +
     	const char *request = json_object_to_json_string(jrequest);
    -	elog(LOG, "Send JSON request to Ranger: %s", request);
     	Assert(request != NULL);
    +	elog(LOG, "Send JSON request to Ranger: %s", request);
    +
     	struct curl_context_t curl_context;
     	memset(&curl_context, 0, sizeof(struct curl_context_t));
     
     	/* call GET method to send request*/
     	if (call_ranger_rest(&curl_context, request) < 0)
     	{
    -		return RANGERCHECK_NO_PRIV;
    +		return -1;
     	}
     
     	/* free the JSON object */
     	json_object_put(jrequest);
     
     	/* parse the JSON-format result */
    -	RangerACLResult ret = parse_ranger_response(curl_context.response.buffer);
    +	int ret = parse_ranger_response(curl_context.response.buffer, result_list);
    +	if (ret < 0) 
    +	{
    +		elog(WARNING, "parse_ranger_response() failed");
    --- End diff --
    
    Fixed, I modifid it to parse ranger response fail, response[the JSON-response-string]


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq pull request #1081: HAWQ-1257. Prompt all tables which user d...

Posted by linwen <gi...@git.apache.org>.
Github user linwen commented on a diff in the pull request:

    https://github.com/apache/incubator-hawq/pull/1081#discussion_r95713912
  
    --- Diff: src/backend/libpq/rangerrest.c ---
    @@ -381,29 +449,38 @@ int call_ranger_rest(CURL_HANDLE curl_handle, const char* request)
     }
     
     /*
    - * arg_list: List of RangerRequestJsonArgs
    + * check privilege(s) from ranger
    + * @param	request_list	List of RangerRequestJsonArgs
    + * @param	result_list		List of RangerPrivilegeResults
    + * @return	0 get response from ranger and parse success; -1 other error
      */
    -int check_privilege_from_ranger(List *arg_list)
    +int check_privilege_from_ranger(List *request_list, List *result_list)
     {
    -	json_object* jrequest = create_ranger_request_json(arg_list);
    +	json_object* jrequest = create_ranger_request_json(request_list, result_list);
     	Assert(jrequest != NULL);
    +
     	const char *request = json_object_to_json_string(jrequest);
    -	elog(LOG, "Send JSON request to Ranger: %s", request);
     	Assert(request != NULL);
    +	elog(LOG, "Send JSON request to Ranger: %s", request);
    --- End diff --
    
    This debug info can be set to DEBUG3. If use LOG, there will be a lot of messages in log file, which is not easy to read. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq pull request #1081: HAWQ-1257. Prompt all tables which user d...

Posted by stanlyxiang <gi...@git.apache.org>.
Github user stanlyxiang commented on a diff in the pull request:

    https://github.com/apache/incubator-hawq/pull/1081#discussion_r95531777
  
    --- Diff: src/backend/libpq/rangerrest.c ---
    @@ -381,29 +449,38 @@ int call_ranger_rest(CURL_HANDLE curl_handle, const char* request)
     }
     
     /*
    - * arg_list: List of RangerRequestJsonArgs
    + * check privilege(s) from ranger
    + * @param	request_list	List of RangerRequestJsonArgs
    + * @param	result_list		List of RangerPrivilegeResults
    + * @return	0 get response from ranger and parse success; -1 other error
      */
    -int check_privilege_from_ranger(List *arg_list)
    +int check_privilege_from_ranger(List *request_list, List *result_list)
     {
    -	json_object* jrequest = create_ranger_request_json(arg_list);
    +	json_object* jrequest = create_ranger_request_json(request_list, result_list);
     	Assert(jrequest != NULL);
    +
     	const char *request = json_object_to_json_string(jrequest);
    -	elog(LOG, "Send JSON request to Ranger: %s", request);
     	Assert(request != NULL);
    +	elog(LOG, "Send JSON request to Ranger: %s", request);
    +
     	struct curl_context_t curl_context;
     	memset(&curl_context, 0, sizeof(struct curl_context_t));
     
     	/* call GET method to send request*/
     	if (call_ranger_rest(&curl_context, request) < 0)
     	{
    -		return RANGERCHECK_NO_PRIV;
    +		return -1;
     	}
     
     	/* free the JSON object */
     	json_object_put(jrequest);
     
     	/* parse the JSON-format result */
    -	RangerACLResult ret = parse_ranger_response(curl_context.response.buffer);
    +	int ret = parse_ranger_response(curl_context.response.buffer, result_list);
    +	if (ret < 0) 
    +	{
    +		elog(WARNING, "parse_ranger_response() failed");
    --- End diff --
    
    this warning says a function failed. devs or users maybe don't know what does this mean. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq pull request #1081: HAWQ-1257. Prompt all tables which user d...

Posted by stanlyxiang <gi...@git.apache.org>.
Github user stanlyxiang commented on a diff in the pull request:

    https://github.com/apache/incubator-hawq/pull/1081#discussion_r95751617
  
    --- Diff: src/backend/libpq/rangerrest.c ---
    @@ -463,7 +464,12 @@ int check_privilege_from_ranger(List *request_list, List *result_list)
     	json_object_put(jrequest);
     
     	/* parse the JSON-format result */
    -	RangerACLResult ret = parse_ranger_response(curl_context_ranger.response.buffer);
    +	int ret = parse_ranger_response(curl_context_ranger.response.buffer, result_list);
    +	if (ret < 0)
    +	{
    +		elog(WARNING, "parse ranger response failed, response[%s]", 
    --- End diff --
    
    The response content is a json format. like {""requestId"":1,""privileges"":[""usage""],""allowed"":true}]},  if we add "[]" outside it,  square brackets can be a part of json content. Do you think so  ? 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq pull request #1081: HAWQ-1257. Prompt all tables which user d...

Posted by interma <gi...@git.apache.org>.
Github user interma commented on a diff in the pull request:

    https://github.com/apache/incubator-hawq/pull/1081#discussion_r95715912
  
    --- Diff: src/backend/parser/parse_relation.c ---
    @@ -2734,70 +2734,99 @@ ExecCheckRTPerms(List *rangeTable)
     void
     ExecCheckRTPermsWithRanger(List *rangeTable)
     {
    -  List *ranger_check_args = NIL;
    -  ListCell *l;
    -  foreach(l, rangeTable)
    -  {
    -
    -    AclMode requiredPerms;
    -    Oid relOid;
    -    Oid userid;
    -    RangeTblEntry *rte = (RangeTblEntry *) lfirst(l);
    -
    -    if (rte->rtekind != RTE_RELATION)
    -      continue;
    -    requiredPerms = rte->requiredPerms;
    -    if (requiredPerms == 0)
    -      continue;
    -    
    -    relOid = rte->relid;
    -    userid = rte->checkAsUser ? rte->checkAsUser : GetUserId();
    -
    -    RangerPrivilegeArgs *ranger_check_arg = (RangerPrivilegeArgs *) palloc(sizeof(RangerPrivilegeArgs));
    -    ranger_check_arg->objkind = ACL_KIND_CLASS;
    -    ranger_check_arg->object_oid = relOid;
    -    ranger_check_arg->roleid = userid;
    -    ranger_check_arg->mask = requiredPerms;
    -    ranger_check_arg->how = ACLMASK_ALL;
    -    ranger_check_args = lappend(ranger_check_args, ranger_check_arg);
    -
    -  } // foreach
    -
    -  if (ranger_check_args == NIL)
    -    return;
    -
    -  // ranger ACL check with package Oids
    -  List *aclresults = NIL;
    -  aclresults = pg_rangercheck_batch(ranger_check_args);
    -  if (aclresults == NIL)
    -  {
    -    elog(ERROR, "ACL check failed\n");
    -    return;
    -  }
    -
    -  // check result
    -  ListCell *result;
    -  foreach(result, aclresults)
    -  {
    -    RangerPrivilegeResults *result_ptr = (RangerPrivilegeResults *) lfirst(result);
    -    if(result_ptr->result != RANGERCHECK_OK)
    -    {
    -      Oid relOid = result_ptr->relOid;
    -      const char *rel_name = get_rel_name_partition(relOid);
    -      aclcheck_error(ACLCHECK_NO_PRIV, ACL_KIND_CLASS, rel_name);
    -    }
    -  }
    -  
    -  if (ranger_check_args)
    -  {
    -    list_free_deep(ranger_check_args);
    -    ranger_check_args = NIL;
    -  }
    -  if (aclresults)
    -  {
    -    list_free_deep(aclresults);
    -    aclresults = NIL;
    -  }
    +	List *ranger_check_args = NIL;
    +	ListCell *l;
    +	foreach(l, rangeTable)
    +	{
    +
    +		AclMode requiredPerms;
    +		Oid relOid;
    +		Oid userid;
    +		RangeTblEntry *rte = (RangeTblEntry *) lfirst(l);
    +
    +		if (rte->rtekind != RTE_RELATION)
    +			continue;
    +		requiredPerms = rte->requiredPerms;
    +		if (requiredPerms == 0)
    +			continue;
    +
    +		relOid = rte->relid;
    +		userid = rte->checkAsUser ? rte->checkAsUser : GetUserId();
    +
    +		RangerPrivilegeArgs *ranger_check_arg = (RangerPrivilegeArgs *) palloc(sizeof(RangerPrivilegeArgs));
    +		ranger_check_arg->objkind = ACL_KIND_CLASS;
    +		ranger_check_arg->object_oid = relOid;
    +		ranger_check_arg->roleid = userid;
    +		ranger_check_arg->mask = requiredPerms;
    +		ranger_check_arg->how = ACLMASK_ALL;
    +		ranger_check_args = lappend(ranger_check_args, ranger_check_arg);
    +
    +	} // foreach
    +
    +	if (ranger_check_args == NIL)
    +		return;
    +
    +	// ranger ACL check with package Oids
    --- End diff --
    
    Fixed!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq issue #1081: HAWQ-1257. Prompt all tables which user doesn't ...

Posted by interma <gi...@git.apache.org>.
Github user interma commented on the issue:

    https://github.com/apache/incubator-hawq/pull/1081
  
    Thanks paul!
    > 1. do not mix // and /*
    > 2. do not have the function return type be on the same line of the declaration.
    > 3. Please make sure whether ereport instead of elog should be used.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq pull request #1081: HAWQ-1257. Prompt all tables which user d...

Posted by stanlyxiang <gi...@git.apache.org>.
Github user stanlyxiang commented on a diff in the pull request:

    https://github.com/apache/incubator-hawq/pull/1081#discussion_r95715330
  
    --- Diff: src/backend/libpq/rangerrest.c ---
    @@ -381,29 +449,38 @@ int call_ranger_rest(CURL_HANDLE curl_handle, const char* request)
     }
     
     /*
    - * arg_list: List of RangerRequestJsonArgs
    + * check privilege(s) from ranger
    + * @param	request_list	List of RangerRequestJsonArgs
    + * @param	result_list		List of RangerPrivilegeResults
    + * @return	0 get response from ranger and parse success; -1 other error
      */
    -int check_privilege_from_ranger(List *arg_list)
    +int check_privilege_from_ranger(List *request_list, List *result_list)
     {
    -	json_object* jrequest = create_ranger_request_json(arg_list);
    +	json_object* jrequest = create_ranger_request_json(request_list, result_list);
     	Assert(jrequest != NULL);
    +
     	const char *request = json_object_to_json_string(jrequest);
    -	elog(LOG, "Send JSON request to Ranger: %s", request);
     	Assert(request != NULL);
    +	elog(LOG, "Send JSON request to Ranger: %s", request);
    --- End diff --
    
    In my previous pr, I have updated this log level and message. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq pull request #1081: HAWQ-1257. Prompt all tables which user d...

Posted by interma <gi...@git.apache.org>.
Github user interma commented on a diff in the pull request:

    https://github.com/apache/incubator-hawq/pull/1081#discussion_r95521591
  
    --- Diff: src/include/utils/rangerrest.h ---
    @@ -80,6 +80,13 @@ typedef struct RangerPrivilegeResults
     {
       RangerACLResult result;
       Oid relOid;
    +
    +  /* 
    --- End diff --
    
    On a *batch* ranger acl check request/response:
    The acl check result order isn't the same as request.
    When reponse's resource&privilege field is same as request, then we consider them is a pair.
     
    So we need storage resource&privilege sign in here.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq pull request #1081: HAWQ-1257. Prompt all tables which user d...

Posted by interma <gi...@git.apache.org>.
Github user interma closed the pull request at:

    https://github.com/apache/incubator-hawq/pull/1081


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq pull request #1081: HAWQ-1257. Prompt all tables which user d...

Posted by interma <gi...@git.apache.org>.
Github user interma commented on a diff in the pull request:

    https://github.com/apache/incubator-hawq/pull/1081#discussion_r95522261
  
    --- Diff: src/backend/libpq/rangerrest.c ---
    @@ -68,51 +69,99 @@ static void getClientIP(char *remote_host)
     	}
     }
     
    -RangerACLResult parse_ranger_response(char* buffer)
    +/*
    + * parse ranger response
    + * @param	buffer	ranger response 	
    + * @param	result_list		List of RangerPrivilegeResults
    + * @return	0 parse success; -1 other error
    + */
    +static int parse_ranger_response(char* buffer, List *result_list)
     {
     	if (buffer == NULL || strlen(buffer) == 0)
    -		return RANGERCHECK_UNKNOWN;
    +		return -1;
     
     	elog(LOG, "read from Ranger Restful API: %s", buffer);
     
     	struct json_object *response = json_tokener_parse(buffer);
     	if (response == NULL) 
     	{
     		elog(WARNING, "json_tokener_parse failed");
    -		return RANGERCHECK_NO_PRIV;
    +		return -1;
     	}
     
     	struct json_object *accessObj = NULL;
     	if (!json_object_object_get_ex(response, "access", &accessObj))
     	{
     		elog(WARNING, "get json access field failed");
    -		return RANGERCHECK_NO_PRIV;
    +		return -1;
     	}
     
     	int arraylen = json_object_array_length(accessObj);
     	elog(LOG, "Array Length: %d",arraylen);
    -
    -	// here should return which table's acl check failed in future.
    +	
     	for (int i=0; i< arraylen; i++){
     		struct json_object *jvalue = NULL;
     		struct json_object *jallow = NULL;
    +		struct json_object *jresource = NULL;
    +		struct json_object *jprivilege = NULL;
     
     		jvalue = json_object_array_get_idx(accessObj, i);
    +		if (jvalue == NULL) 
    +			return -1;
     		if (!json_object_object_get_ex(jvalue, "allowed", &jallow))
    -		{
    -			return RANGERCHECK_NO_PRIV;
    -		}
    -		json_bool result = json_object_get_boolean(jallow);
    -		if(result != 1){
    -			return RANGERCHECK_NO_PRIV;
    +			return -1;
    +		if (!json_object_object_get_ex(jvalue, "resource", &jresource))
    +			return -1;
    +		if (!json_object_object_get_ex(jvalue, "privileges", &jprivilege))
    +			return -1;
    +		
    +		json_bool ok = json_object_get_boolean(jallow);
    +
    +		const char *resource_str = json_object_get_string(jresource);
    +		const char *privilege_str = json_object_get_string(jprivilege);
    +		uint32 resource_sign = string_hash(resource_str, strlen(resource_str));
    +		uint32 privilege_sign = string_hash(privilege_str, strlen(privilege_str));
    +		elog(DEBUG3, "ranger reponse access sign, resource_str:%s, privilege_str:%s", 
    +			resource_str, privilege_str);
    +
    +		ListCell *result;
    +		/* get each resource result by use sign */
    +		foreach(result, result_list) {
    +			/* loop find is enough for performence*/
    +			RangerPrivilegeResults *result_ptr = (RangerPrivilegeResults *) lfirst(result);
    +			if (result_ptr->resource_sign != resource_sign || result_ptr->privilege_sign != privilege_sign)
    +				continue;
    +
    +			if (ok == 1)
    +				result_ptr->result = RANGERCHECK_OK;
    +			else 
    +				result_ptr->result = RANGERCHECK_NO_PRIV;
    --- End diff --
    
    Match response sign to request, and judge a **pair**.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq pull request #1081: HAWQ-1257. Prompt all tables which user d...

Posted by interma <gi...@git.apache.org>.
Github user interma commented on a diff in the pull request:

    https://github.com/apache/incubator-hawq/pull/1081#discussion_r95548234
  
    --- Diff: src/include/utils/rangerrest.h ---
    @@ -80,6 +80,13 @@ typedef struct RangerPrivilegeResults
     {
       RangerACLResult result;
       Oid relOid;
    +
    +  /* 
    +   * string_hash of access[i] field of ranger request 
    +   * use the sigh to identify each resource result
    --- End diff --
    
    Sorry... Fixed!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq pull request #1081: HAWQ-1257. Prompt all tables which user d...

Posted by linwen <gi...@git.apache.org>.
Github user linwen commented on a diff in the pull request:

    https://github.com/apache/incubator-hawq/pull/1081#discussion_r95527964
  
    --- Diff: src/backend/catalog/aclchk.c ---
    @@ -2712,15 +2712,20 @@ List *pg_rangercheck_batch(List *arg_list)
       ListCell *arg;
       foreach(arg, arg_list) {
         RangerPrivilegeArgs *arg_ptr = (RangerPrivilegeArgs *) lfirst(arg);
    +
         AclObjectKind objkind = arg_ptr->objkind;
         Oid object_oid = arg_ptr->object_oid;
         char *objectname = getNameFromOid(objkind, object_oid);
         char *rolename = getRoleName(arg_ptr->roleid);
         List* actions = getActionName(arg_ptr->mask);
         bool isAll = (arg_ptr->how == ACLMASK_ALL) ? true: false;
    +
         RangerPrivilegeResults *aclresult = (RangerPrivilegeResults *) palloc(sizeof(RangerPrivilegeResults));
    -    aclresult->result = -1;
    +    aclresult->result = RANGERCHECK_NO_PRIV;
         aclresult->relOid = object_oid;
    +	// this two sign fields will be set in create_ranger_request_json()
    --- End diff --
    
    indent here doesn't match previous. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq issue #1081: HAWQ-1257. Prompt all tables which user doesn't ...

Posted by zhangh43 <gi...@git.apache.org>.
Github user zhangh43 commented on the issue:

    https://github.com/apache/incubator-hawq/pull/1081
  
    +1


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq pull request #1081: HAWQ-1257. Prompt all tables which user d...

Posted by interma <gi...@git.apache.org>.
Github user interma commented on a diff in the pull request:

    https://github.com/apache/incubator-hawq/pull/1081#discussion_r95522139
  
    --- Diff: src/backend/libpq/rangerrest.c ---
    @@ -68,51 +69,99 @@ static void getClientIP(char *remote_host)
     	}
     }
     
    -RangerACLResult parse_ranger_response(char* buffer)
    +/*
    + * parse ranger response
    + * @param	buffer	ranger response 	
    + * @param	result_list		List of RangerPrivilegeResults
    + * @return	0 parse success; -1 other error
    + */
    +static int parse_ranger_response(char* buffer, List *result_list)
     {
     	if (buffer == NULL || strlen(buffer) == 0)
    -		return RANGERCHECK_UNKNOWN;
    +		return -1;
    --- End diff --
    
    I modified this function's return value.
    The acl check results will be stored in the param `result_list`.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq pull request #1081: HAWQ-1257. Prompt all tables which user d...

Posted by interma <gi...@git.apache.org>.
Github user interma commented on a diff in the pull request:

    https://github.com/apache/incubator-hawq/pull/1081#discussion_r95522472
  
    --- Diff: src/include/utils/rangerrest.h ---
    @@ -90,9 +97,6 @@ typedef struct RangerRequestJsonArgs {
       bool isAll;
     } RangerRequestJsonArgs;
     
    -RangerACLResult parse_ranger_response(char *);
    -json_object *create_ranger_request_json(List *);
    -int call_ranger_rest(CURL_HANDLE curl_handle, const char *request);
    -extern int check_privilege_from_ranger(List *);
    +int check_privilege_from_ranger(List *request_list, List *result_list);
    --- End diff --
    
    Hide no outer used function.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq pull request #1081: HAWQ-1257. Prompt all tables which user d...

Posted by interma <gi...@git.apache.org>.
Github user interma commented on a diff in the pull request:

    https://github.com/apache/incubator-hawq/pull/1081#discussion_r95715434
  
    --- Diff: src/backend/libpq/rangerrest.c ---
    @@ -68,51 +69,99 @@ static void getClientIP(char *remote_host)
     	}
     }
     
    -RangerACLResult parse_ranger_response(char* buffer)
    +/*
    + * parse ranger response
    + * @param	buffer	ranger response 	
    + * @param	result_list		List of RangerPrivilegeResults
    + * @return	0 parse success; -1 other error
    + */
    +static int parse_ranger_response(char* buffer, List *result_list)
     {
     	if (buffer == NULL || strlen(buffer) == 0)
    -		return RANGERCHECK_UNKNOWN;
    +		return -1;
    --- End diff --
    
    I think is ok\u3002
    Because I modified this function's return type: **from RangerACLResult to int** (only indicate whether parse is success, and check result is stored in param result_list)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq issue #1081: HAWQ-1257. Prompt all tables which user doesn't ...

Posted by interma <gi...@git.apache.org>.
Github user interma commented on the issue:

    https://github.com/apache/incubator-hawq/pull/1081
  
    Commited, close this PR.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq pull request #1081: HAWQ-1257. Prompt all tables which user d...

Posted by interma <gi...@git.apache.org>.
Github user interma commented on a diff in the pull request:

    https://github.com/apache/incubator-hawq/pull/1081#discussion_r95522410
  
    --- Diff: src/backend/libpq/rangerrest.c ---
    @@ -247,12 +299,27 @@ json_object *create_ranger_request_json(List *args)
     		ListCell *cell;
     		foreach(cell, arg_ptr->actions)
     		{
    -		    json_object* jaction = json_object_new_string((char *)cell->data.ptr_value);
    +			/* need more normalization in future */
    --- End diff --
    
    If the json string changed in future, we should do more normalization works.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---