Эх сурвалжийг харах

[raz_s3] Do not enable S3 browser in ADLS (#2562)

- Check for RAZ URL and AWS_ACCOUNT Hostname configs too this time.
- In RAZ ADLS cluster, the AWS_ACCOUNT configs should not be present practically.
Harsh Gupta 4 жил өмнө
parent
commit
2cc6c8534b

+ 1 - 2
desktop/libs/aws/src/aws/client.py

@@ -24,7 +24,6 @@ from aws import conf as aws_conf
 from aws.s3.s3connection import url_client_connect_to_region, RazS3Connection
 from aws.s3.s3fs import S3FileSystem, S3FileSystemException
 
-from desktop.conf import RAZ
 from desktop.lib.idbroker import conf as conf_idbroker
 from desktop.lib.idbroker.client import IDBroker
 
@@ -45,7 +44,7 @@ def get_credential_provider(identifier, user):
 def _make_client(identifier, user):
   client_conf = aws_conf.AWS_ACCOUNTS[identifier] if identifier in aws_conf.AWS_ACCOUNTS else None
 
-  if RAZ.IS_ENABLED.get() and not aws_conf.IS_SELF_SIGNING_ENABLED.get():
+  if aws_conf.is_raz_s3():
     host = aws_conf.get_default_host() or client_conf.HOST.get()
     s3_client = RazS3Connection(username=user, host=host)  # Note: Remaining AWS configuration is fully skipped
     s3_client_expiration = None

+ 5 - 7
desktop/libs/aws/src/aws/conf.py

@@ -283,12 +283,10 @@ AWS_ACCOUNTS = UnspecifiedConfigSection(
 
 
 def is_enabled():
-  from desktop.conf import RAZ  # Must be imported dynamically in order to have proper value
   return ('default' in list(AWS_ACCOUNTS.keys()) and AWS_ACCOUNTS['default'].get_raw() and AWS_ACCOUNTS['default'].ACCESS_KEY_ID.get()) or \
       has_iam_metadata() or \
       conf_idbroker.is_idbroker_enabled('s3a') or \
-      IS_SELF_SIGNING_ENABLED.get() or \
-      (RAZ.IS_ENABLED.get() and 'default' in list(AWS_ACCOUNTS.keys()) and AWS_ACCOUNTS['default'].get_raw())
+      is_raz_s3()
 
 
 def is_ec2_instance():
@@ -346,16 +344,16 @@ def has_iam_metadata():
 
 def has_s3_access(user):
   from desktop.auth.backend import is_admin
-  from desktop.conf import RAZ  # Must be imported dynamically in order to have proper value
 
   return user.is_authenticated and user.is_active and (
-    is_admin(user) or user.has_hue_permission(action="s3_access", app="filebrowser") or RAZ.IS_ENABLED.get())
+    is_admin(user) or user.has_hue_permission(action="s3_access", app="filebrowser") or is_raz_s3())
 
 
-def has_raz_s3():
+def is_raz_s3():
   from desktop.conf import RAZ  # Must be imported dynamically in order to have proper value
 
-  return (RAZ.IS_ENABLED.get() and bool(get_raz_s3_default_bucket()))
+  return (RAZ.IS_ENABLED.get() and 'default' in list(AWS_ACCOUNTS.keys()) and \
+          AWS_ACCOUNTS['default'].HOST.get() and AWS_ACCOUNTS['default'].get_raw() and not IS_SELF_SIGNING_ENABLED.get())
 
 
 def config_validator(user):

+ 58 - 3
desktop/libs/aws/src/aws/conf_tests.py

@@ -40,11 +40,20 @@ class TestAWSConf(unittest.TestCase):
     self.client = make_logged_in_client(username="test_user", groupname="default", recreate=True, is_superuser=False)
     self.user = User.objects.get(username="test_user")
 
-  def test_is_enabled_when_raz_enabled(self):
 
+  def test_is_enabled(self):
     # When RAZ is not enabled
     assert_false(conf.is_enabled())
 
+    # When only RAZ is enabled (S3 in Azure cluster)
+    reset = RAZ.IS_ENABLED.set_for_testing(True)
+    try:
+      assert_false(conf.is_enabled())
+    finally:
+      reset()
+      conf.clear_cache()
+
+    # When RAZ is enabled along with S3 config
     resets = [
       RAZ.IS_ENABLED.set_for_testing(True),
       conf.AWS_ACCOUNTS.set_for_testing({'default': {
@@ -60,15 +69,61 @@ class TestAWSConf(unittest.TestCase):
       for reset in resets:
         reset()
       conf.clear_cache()
-  
-  def test_has_s3_access_when_raz_enabled(self):
 
+
+  def test_has_s3_access(self):
     # When RAZ is not enabled
     assert_false(conf.has_s3_access(self.user))
 
+    # When only RAZ is enabled (S3 in Azure cluster)
     reset = RAZ.IS_ENABLED.set_for_testing(True)
+    try:
+      assert_false(conf.has_s3_access(self.user))
+    finally:
+      reset()
+      conf.clear_cache()
+
+    # When RAZ is enabled along with S3 config
+    resets = [
+      RAZ.IS_ENABLED.set_for_testing(True),
+      conf.AWS_ACCOUNTS.set_for_testing({'default': {
+        'region': 'us-west-2',
+        'host': 's3-us-west-2.amazonaws.com',
+        'allow_environment_credentials': 'false'
+      }})
+    ]
     try:
       assert_true(conf.has_s3_access(self.user))
+    finally:
+      for reset in resets:
+        reset()
+      conf.clear_cache()
+
+
+  def test_is_raz_s3(self):
+    # When RAZ is not enabled
+    assert_false(conf.is_raz_s3())
+
+    # When only RAZ is enabled (S3 in Azure cluster)
+    reset = RAZ.IS_ENABLED.set_for_testing(True)
+    try:
+      assert_false(conf.is_raz_s3())
     finally:
       reset()
       conf.clear_cache()
+
+    # When RAZ is enabled along with S3 config
+    resets = [
+      RAZ.IS_ENABLED.set_for_testing(True),
+      conf.AWS_ACCOUNTS.set_for_testing({'default': {
+        'region': 'us-west-2',
+        'host': 's3-us-west-2.amazonaws.com',
+        'allow_environment_credentials': 'false'
+      }})
+    ]
+    try:
+      assert_true(conf.is_raz_s3())
+    finally:
+      for reset in resets:
+        reset()
+      conf.clear_cache()