diff --git a/dvc.conf b/dvc.conf index 94ee81888..9c923287b 100644 --- a/dvc.conf +++ b/dvc.conf @@ -2,17 +2,16 @@ DataDir = data CacheDir = cache StateDir = state -Cloud = google +Cloud = +StoragePath = # Log levels: Debug, Info, Warning and Error LogLevel = Info [AWS] -AccessKeyId = -SecretAccessKey = - StoragePath = +CredentialPath = Region = us-east-1 Zone = us-east-1a @@ -33,9 +32,6 @@ SecurityGroup = dvc-group [GC] +StoragePath = + ProjectName = -StoragePath = - - -[Data] -StoragePath = diff --git a/dvc/command/data_sync.py b/dvc/command/data_sync.py index 678c5e165..c461feb87 100644 --- a/dvc/command/data_sync.py +++ b/dvc/command/data_sync.py @@ -85,7 +85,6 @@ class CmdDataSync(CmdBase): def _get_bucket_aws(self): """ get a bucket object, aws """ - conn = S3Connection(self.config.aws_access_key_id, self.config.aws_secret_access_key, host=self.config.aws_region_host) bucket_name = self.config.storage_bucket bucket = conn.lookup(bucket_name) @@ -179,19 +178,19 @@ class CmdDataSync(CmdBase): def sync_from_cloud(self, item): - cloud = self.settings.config.get_cloud + cloud = self.settings.config.cloud assert cloud in ['amazon', 'google'], 'unknown cloud %s' % cloud - if self.settings.config.get_cloud == 'amazon': + if cloud == 'amazon': return self._sync_from_cloud_aws(item) - elif self.settings.config.get_cloud == 'google': + elif cloud == 'google': return self._sync_from_cloud_gcp(item) def sync_to_cloud(self, data_item): - cloud = self.settings.config.get_cloud + cloud = self.settings.config.cloud assert cloud in ['amazon', 'google'], 'unknown cloud %s' % cloud - if self.settings.config.get_cloud == 'amazon': + if cloud == 'amazon': return self._sync_to_cloud_aws(data_item) - elif self.settings.config.get_cloud == 'google': + elif cloud == 'google': return self._sync_to_cloud_gcp(data_item) diff --git a/dvc/command/init.py b/dvc/command/init.py index 26acfcea9..a1eadbed6 100644 --- a/dvc/command/init.py +++ b/dvc/command/init.py @@ -19,16 +19,15 @@ DataDir = {} CacheDir = {} StateDir = {} Cloud = +StoragePath = # Log levels: Debug, Info, Warning and Error LogLevel = Info [AWS] -AccessKeyId = -SecretAccessKey = - StoragePath = dvc/tutorial +CredentialPath = Region = us-east-1 Zone = us-east-1a @@ -48,10 +47,8 @@ KeyName = dvc-key SecurityGroup = dvc-group [GC] -ProjectName = - -[Data] StoragePath = +ProjectName = ''' def __init__(self, settings): diff --git a/dvc/command/test.py b/dvc/command/test.py index f92fbc164..80d90a075 100644 --- a/dvc/command/test.py +++ b/dvc/command/test.py @@ -23,7 +23,7 @@ class CmdTest(CmdBase): def run(self): - cloud = self.settings._config.get_cloud + cloud = self.settings._config.cloud assert cloud in ['amazon', 'google'], 'unknown cloud: %s' % cloud if cloud == 'amazon': print 'TODO' diff --git a/dvc/config.py b/dvc/config.py index 4590cfab0..8dd6e1125 100644 --- a/dvc/config.py +++ b/dvc/config.py @@ -36,18 +36,27 @@ class ConfigI(object): class Config(ConfigI): CONFIG = 'dvc.conf' - - def __init__(self, conf_file): + def __init__(self, conf_file, conf_pseudo_file=None): + """ + Params: + conf_file (String): configuration file + conf_pseudo_file (String): for unit testing, something that supports readline; supersedes conf_file + """ self._conf_file = conf_file - self._config = configparser.ConfigParser() + self._config = configparser.SafeConfigParser() - if not os.path.isfile(conf_file): - raise ConfigError('Config file "{}" does not exist'.format(conf_file)) - self._config.read(conf_file) + if conf_pseudo_file is not None: + self._config.readfp(conf_pseudo_file) + else: + if not os.path.isfile(conf_file): + raise ConfigError('Config file "{}" does not exist'.format(conf_file)) + self._config.read(conf_file) level = self._config['Global']['LogLevel'] Logger.set_level(level) + self._aws_creds = None + super(Config, self).__init__(self._config['Global']['DataDir'], self._config['Global']['CacheDir'], self._config['Global']['StateDir']) @@ -59,11 +68,15 @@ class Config(ConfigI): @property def aws_access_key_id(self): - return self._config['AWS']['AccessKeyId'] + if not self._aws_creds: + self._aws_creds = self.get_aws_credentials() + return self._aws_creds[0] @property def aws_secret_access_key(self): - return self._config['AWS']['SecretAccessKey'] + if not self._aws_creds: + self._aws_creds = get_aws_credentials() + return self._aws_creds[1] @property def aws_region_host(self): @@ -87,11 +100,11 @@ class Config(ConfigI): Precedence: Storage, then cloud specific """ - path = self._config['Data'].get('StoragePath', None) + path = self._config['Global'].get('StoragePath', None) if path: return path - cloud = self.get_cloud + cloud = self.cloud assert cloud in ['amazon', 'google'], 'unknown cloud %s' % cloud if cloud == 'amazon': path = self._config['AWS'].get('StoragePath', None) @@ -121,6 +134,101 @@ class Config(ConfigI): return self._config['GC']['ProjectName'] @property - def get_cloud(self): + def cloud(self): """ get cloud choice: currently one of ['amazon', 'google'] """ - return self._config['Global']['Cloud'] + conf = self._config['Global'].get('Cloud', '') + if conf.lower() in ['amazon', 'aws']: + return 'amazon' + if conf.lower() in ['gc', 'google', 'google cloud', 'googlecloud']: + return 'google' + return conf + + + def get_aws_credentials(self): + """ gets aws credentials, looking in various places + + Params: + + Searches: + 1 any override in dvc.conf [AWS] CredentialPath; + 2 ~/.aws/credentials + + + Returns: + if successfully found, (access_key_id, secret) + None otherwise + """ + default = os.path.expanduser('~/.aws/credentials') + + paths = [] + credpath = self._config['AWS'].get('CredentialPath', None) + if credpath is not None and len(credpath) > 0: + credpath = os.path.expanduser(credpath) + if os.path.isfile(credpath): + paths.append(credpath) + else: + Logger.warn('AWS CredentialPath "%s" not found; falling back to default "%s"' % (credpath, default)) + paths.append(default) + else: + paths.append(default) + + for path in paths: + cc = configparser.SafeConfigParser() + threw = False + try: + # use readfp(open( ... to aid mocking. + cc.readfp(open(path, 'r')) + except Exception as e: + threw = True + if not threw and 'default' in cc.keys(): + access_key = cc['default'].get('aws_access_key_id', None) + secret = cc['default'].get('aws_secret_access_key', None) + + if access_key is not None and secret is not None: + return (access_key, secret) + + return None + + + def sanity_check(self): + """ sanity check a config + + check that we have a cloud and storagePath + if aws, check can read credentials + if google, check ProjectName + + Returns: + (T,) if good + (F, issues) if bad + """ + errors = [] + for key in ['Cloud']: + if key.lower() not in self._config['Global'].keys() or len(self._config['Global'][key]) < 1: + errors.append('Please set %s in section Global in config file %s' % (key, self.file)) + + # now that a cloud is chosen, can check StoragePath + sp = self.storage_path + if sp is None or len(sp) == 0: + errors.append('Please set StoragePath = bucket/{optional path} in conf file "%s" ' + 'either in Global or a cloud specific section' % self.CONFIG) + + cloud = self.cloud + if cloud == '': + # not set; already complained above + pass + elif cloud == 'amazon': + creds = self.get_aws_credentials() + if creds is None: + errors.append('can\'t find aws credentials. TODO') + self._aws_creds = creds + elif cloud == 'google': + project = self._config['GC'].get('ProjectName', None) + if project is None or len(project) < 1: + errors.append('can\'t read google cloud project name. Please set ProjectName in section GC. TODO HOW FIND') + else: + errors.append('unknown Cloud %s' % cloud) + + return (len(errors) == 0, errors) + + + diff --git a/dvc/runtime.py b/dvc/runtime.py index 8b52df148..5e543aca7 100644 --- a/dvc/runtime.py +++ b/dvc/runtime.py @@ -29,6 +29,13 @@ class Runtime(object): if parse_config: runtime_config = Config(Runtime.conf_file_path(runtime_git.git_dir)) + + good = runtime_config.sanity_check() + if good[0] == False: + Logger.error('config \'%s\' is not correctly setup. Please fix:' % Runtime.CONFIG) + for e in good[1]: + Logger.error(' ' + e) + sys.exit(-1) else: runtime_config = ConfigI() diff --git a/requirements.txt b/requirements.txt index 7d38fb5c0..65bb20b7d 100644 --- a/requirements.txt +++ b/requirements.txt @@ -9,6 +9,7 @@ decorator==4.0.11 dill==0.2.6 enum34==1.1.6 fasteners==0.14.1 +funcsigs==1.0.2 future==0.16.0 futures==3.0.5 gapic-google-cloud-datastore-v1==0.15.3 @@ -48,6 +49,7 @@ httplib2==0.10.3 ipython==5.3.0 ipython-genutils==0.2.0 macholib==1.8 +mock==2.0.0 modulegraph==0.14 monotonic==1.3 nose==1.3.7 @@ -55,6 +57,7 @@ oauth2client==3.0.0 packaging==16.8 pathlib==1.0.1 pathlib2==2.2.1 +pbr==3.0.0 pexpect==4.2.1 pickleshare==0.7.4 ply==3.8 diff --git a/tests/test_config.py b/tests/test_config.py new file mode 100644 index 000000000..6e5ce1749 --- /dev/null +++ b/tests/test_config.py @@ -0,0 +1,129 @@ +import io +import os +import shutil +import StringIO +import traceback +from unittest import TestCase +import mock + + +from dvc.path.path import Path +from dvc.system import System +from dvc.config import Config + + +class TestConfigTest(TestCase): + def setUp(self): + pass + + def storage_path_hierarchy_test(self): + """ StoragePath should be read first from global and then from cloud section """ + + c = ("[Global]", + "LogLevel =", + "DataDir = ", + "CacheDir = ", + "StateDir = ", + "Cloud = amazon", + "StoragePath = globalsb/global_storage_path", + "[AWS]", + "StoragePath = awssb/aws_storage_path", + "CredentialPath =", + "[GC]", + "StoragePath = googlesb/google_storage_path" + ) + s = StringIO.StringIO('\n'.join(c)) + conf = Config(s, conf_pseudo_file = s) + + self.assertEqual(conf.cloud, 'amazon') + self.assertEqual(conf.storage_bucket, 'globalsb') + self.assertEqual(conf.storage_prefix, 'global_storage_path') + + c = ("[Global]", + "LogLevel =", + "DataDir = ", + "CacheDir = ", + "StateDir = ", + "Cloud = amazon", + "[AWS]", + "StoragePath = awssb/aws_storage_path", + "CredentialPath =", + "[GC]", + "StoragePath = googlesb/google_storage_path" + ) + s = StringIO.StringIO('\n'.join(c)) + conf = Config(s, conf_pseudo_file=s) + + self.assertEqual(conf.cloud, 'amazon') + self.assertEqual(conf.storage_bucket, 'awssb') + self.assertEqual(conf.storage_prefix, 'aws_storage_path') + + + def mocked_open_aws_default_credentials(klass, file, asdf): + """ mocked open for ~/.aws/credentials """ + if file == os.path.expanduser('~/.aws/credentials'): + creds0 = ("[default]", + "aws_access_key_id = default_access_id", + "aws_secret_access_key = default_sekret") + return io.BytesIO('\n'.join(creds0)) + + creds1 = ("[default]", + "aws_access_key_id = override_access_id", + "aws_secret_access_key = override_sekret") + + return io.BytesIO('\n'.join(creds1)) + + def aws_credentials_default_test(self): + """ in absence of [AWS] -> CredentialPath, aws creds should be read from ~/.aws/credentials """ + + default_path = os.path.expanduser('~/.aws/credentials') + c = ("[Global]", + "LogLevel =", + "DataDir = ", + "CacheDir = ", + "StateDir = ", + "Cloud = amazon", + "[AWS]", + "StoragePath = awssb/aws_storage_path", + "CredentialPath =", + ) + s = StringIO.StringIO('\n'.join(c)) + + patcher = mock.patch('__builtin__.open', side_effect=self.mocked_open_aws_default_credentials) + patcher.start() + conf = Config(s, conf_pseudo_file=s) + aws_creds = conf.get_aws_credentials() + patcher.stop() + + self.assertEqual(aws_creds[0], 'default_access_id') + self.assertEqual(aws_creds[1], 'default_sekret') + + + def mocked_isfile(file): + """ tell os some_credential_path exists """ + return file == 'some_credential_path' + + @mock.patch('dvc.config.os.path.isfile', side_effect = mocked_isfile) + def aws_credentials_specified_test(self, isfile_function): + """ in presence of [AWS] -> CredentialPath, use those credentials """ + + c = ("[Global]", + "LogLevel =", + "DataDir = ", + "CacheDir = ", + "StateDir = ", + "Cloud = amazon", + "[AWS]", + "StoragePath = awssb/aws_storage_path", + "CredentialPath = some_credential_path", + ) + s = StringIO.StringIO('\n'.join(c)) + + patcher = mock.patch('__builtin__.open', side_effect=self.mocked_open_aws_default_credentials) + patcher.start() + conf = Config(s, conf_pseudo_file=s) + aws_creds = conf.get_aws_credentials() + patcher.stop() + + self.assertEqual(aws_creds[0], 'override_access_id') + self.assertEqual(aws_creds[1], 'override_sekret') diff --git a/tests/test_executor.py b/tests/test_executor.py index 2d6f7dabb..ae22e4347 100644 --- a/tests/test_executor.py +++ b/tests/test_executor.py @@ -18,6 +18,8 @@ class TestExecutor(TestCase): def tearDown(self): shutil.rmtree(self.test_dir) + if os.path.exists('myfile.py'): + os.remove('myfile.py') def test_suppress_output(self): output = Executor.exec_cmd_only_success(['python', self.code_file])