HUE-8925 [fb] Support user credentials from IDBroker for S3

Review Request #14116 - Created July 23, 2019 and submitted

Jean-Francois Desjeans Gauthier
hue
master
HUE-8925
hue
jgauthier, romain

commit 582834f3fc18d1736aef27eaac46a2e24541fa89
Author: Jean-Francois Desjeans Gauthier <jf.desjeans.gauthier@gmail.com>
Date: Thu Jul 18 15:51:43 2019 -0700

HUE-8925 \[fb\] Support user credentials from IDBroker for S3

:100644 100644 58699a9038... 5d70ef151d... M apps/filebrowser/src/filebrowser/templates/fb_components.mako
:100644 100644 d53684135d... af5ae0b3dd... M desktop/core/src/desktop/lib/fs/proxyfs.py
:100644 100644 0e50bc474b... 0ebfb32b88... M desktop/core/src/desktop/lib/fsmanager.py
:100644 100644 6be29199a2... 31d378dc8d... M desktop/libs/aws/src/aws/__init__.py
:100644 100644 f03a7bc444... 6c8e4893c3... M desktop/libs/aws/src/aws/client.py
:100644 100644 d3a7e02042... fc41608580... M desktop/libs/aws/src/aws/conf.py
:100644 100644 a6a9a4996f... 81576f72eb... M desktop/libs/aws/src/aws/s3/s3fs.py
:100644 100644 55c5d63f54... 6ad4cb1640... M desktop/libs/aws/src/aws/s3/upload.py
:100644 100644 052e73dedf... 94382ab9e2... M desktop/libs/azure/src/azure/client.py
:100644 100644 6f94a0cf45... 0770d15d95... M desktop/libs/hadoop/src/hadoop/cluster.py
:100644 100644 f3f5a71322... ef46dc5bad... M desktop/libs/hadoop/src/hadoop/core_site.py
:000000 100644 0000000000... f0cd6694cd... A desktop/libs/idbroker/Makefile
:000000 100644 0000000000... 80d0662588... A desktop/libs/idbroker/babel.cfg
:000000 100644 0000000000... f907d04b8a... A desktop/libs/idbroker/hueversion.py
:000000 100644 0000000000... 1427431345... A desktop/libs/idbroker/setup.py
:000000 100644 0000000000... 03619acd50... A desktop/libs/idbroker/src/idbroker/__init__.py
:000000 100644 0000000000... c11054e25f... A desktop/libs/idbroker/src/idbroker/client.py
:000000 100644 0000000000... 2371c8effa... A desktop/libs/idbroker/src/idbroker/conf.py

Tested with user/password and with kerberos on cluster

Added mocks for aws.client.get_client & IDBroker

Had to remove some of the circular dependencies to do the mocks:
1) Moved PERMISSION_ACTION definition to conf
2) Moved aws get_client from init to aws.client
3) Removed aws import in aws.conf

The move of is_enabled & has_access to fsmanager is not strickly necessary, but some cleanup is due; more to come.

  • 0
  • 0
  • 17
  • 7
  • 24
Description From Last Updated
  1. 
      
  2. desktop/Makefile (Diff revision 1)
     
     

    desktop/core/src/desktop/lib/idbroker?

  3. nit in current code: no parenthesis

  4. desktop/libs/aws/src/aws/__init__.py (Diff revision 1)
     
     

    MagicMock test calling S3 with IdBroker on?

  5. desktop/libs/aws/src/aws/__init__.py (Diff revision 1)
     
     

    Movable to cache framework in next step?

  6. desktop/libs/aws/src/aws/__init__.py (Diff revision 1)
     
     

    nit: spaces around -

  7. desktop/libs/aws/src/aws/__init__.py (Diff revision 1)
     
     

    Then could be moved on top if in desktop.lib

  8. desktop/libs/aws/src/aws/__init__.py (Diff revision 1)
     
     

    nit: indent

    return {
    'AccessKeyId': conf.get_default_access_key_id(),
    'SecretAccessKey': ...
    }

  9. desktop/libs/aws/src/aws/__init__.py (Diff revision 1)
     
     

    nit: Log.exception already adds the exception trace, maybe adding an info string is better (same as in the ValueError)

  10. desktop/libs/idbroker/hueversion.py (Diff revision 1)
     
     

    symlink?

  11. 2 MagicMock tests?

    (kerb/basic calling get_auth_token/get_cab?)

  12. 
      
  1. Great stuff!

  2. desktop/libs/aws/src/aws/__init__.py (Diff revisions 1 - 3)
     
     

    can't be on top?

  3. desktop/libs/aws/src/aws/__init__.py (Diff revisions 1 - 3)
     
     
     

    nit: still can't be in top?

  4. desktop/libs/aws/src/aws/__init__.py (Diff revisions 1 - 3)
     
     

    nit:

    return {
    'AccessKeyId': conf.get_default_access_key_id(),
    'SecretAccessKey': conf.get_default_secret_key(),
    'SessionToken': conf.get_default_session_token(),
    'AllowEnvironmentCredentials': allow_env_cred
    }

    https://docs.djangoproject.com/en/dev/internals/contributing/writing-code/coding-style/
    Use four space hanging indentation rather than vertical alignment:

    raise AttributeError(
    'Here is a multiline error message '
    'shortened for clarity.'
    )

  5. I see, purely configured via core-site.xml

  6. nit: LOG.exeption already logs the trace

  7. 
      
  1. Looks great!
    Just a few nits

    We can see later if we need to rely on a core-site.xml all the time and no hue.ini.

  2. nit:

    • as e
    • LOG.exception('Failed to obtain IDBroker Token')
  3. test_username_authentication?

  4. nit:

    conf --> idbroker_conf

  5. test_kerberos_authentication?

  6. nit

    conf1 --> core_site_conf
    conf2 ...

  7. desktop/core/src/desktop/views.py (Diff revision 4)
     
     

    nit: maybe we could have a

    is_s3_enabled(user) to replace all those two combos here and in desktop.models

  8. desktop/libs/aws/src/aws/tests.py (Diff revision 4)
     
     

    Remove?

  9. desktop/libs/aws/src/aws/tests.py (Diff revision 4)
     
     
     
  10. 
      
Review request changed

Status: Closed (submitted)

Loading...