ACCESS-215: Rename the configuration properties from access* to sentry*

Review Request #3270 - Created July 9, 2013 and updated

Prasad Mujumdar
old-access
master
ACCESS-215
access
Rename the configuration properties from access* to sentry*
full unit test run (along with ACCESS-216 patch)
  • 1
  • 0
  • 5
  • 1
  • 7
Description From Last Updated
How are other components supposed to configure sentry? It seems to make sense that things like: policy file location sentry.provider ... Lenni Kuff
  1. Looks good overall. I've some comments though.
  2. We should check if the access conf is set before setting the deprecatedConfig to true. i.e., 
     
    {
      if (hiveauthzConf != null &&    !hiveauthzConf.isEmpty()) {   
       depreicatedConfig = true}
      else {
        raise exception
      }
    }
    
  3. 
      
  1. 
      
    1. I have made the changes directly the ACCESS-216 branch which is on top of this patch. The changes are not major and since the file names are changed in the final code, it can't be be rebased.
      Let me know if you still prefer to update the review here as well.
  2. Does it matter ? 
    The depreicatedConfig is only checked for throwing the correct error message later if the loading the config properties fail.
    1. I think its cleaner to refactor it, but will leave it to you.
  3. 
      
  1. 
      
  2. I'm not sure why some of these config value need to have "hive" prefix to them. For example, why can't:
    "hive.sentry.server"
    just be:
    "sentry.server" 
    
    This will make it easier for other components to reuse the configuration without having anything Hive specific in the names.
    1. This bindings module is implemented specifically for hive and hence the configs are hive.x.y. The other modules and the policy files don't have properties that contain hive. Are you using the same hive binding or these properties for Impala ?
      Please feel free to file a separate ticket if you find any generic config settings that shouldn't be prefixed with hive.
  3. 
      
  1. 
      
  2. How are other components supposed to configure sentry? It seems to make sense  that things like:
    
    policy file location 
    sentry.provider
    server.name
    
    be generic configuration parameters in a common sentry-site.xml.
    
    Are you saying the sentry-site.xml is a hive specific configuration file? If it is Hive specific, perhaps it should be renamed to something that clarifies this.
    1. The other components needs to implement their own bindings to interact with policy engine. The hive-binding module is a hive plugin introspect hive query and validates that access using senty-provider. The 'sentry-site.xml' is just a convention, hive bindings reads the actual file name/path from the hive configuration.
      
      If Impala's auth implementation is using code or configuration from hive-bindings, then let's make it generic.
  3. 
      
  1. 
      
  2. Currently, Impala is not using this code for configuration because it is hive specific :). I think it would be good to come up with a common configuration file/scheme for sentry and common configuration parameters. This will become more important as additional common configuration parameters are added (such as db connections strings for the database backed policy provider).
    No need to address this in this patch, but  if you are already renaming these it might be good to do it in a way that meets future needs.
  3. 
      
Loading...