From d7ffb543336b53edfad8203ccd9ae7d4cba63ad9 Mon Sep 17 00:00:00 2001 From: Sam Chow Date: Thu, 28 Jun 2018 16:56:33 -0400 Subject: [PATCH] Move tar filter to file, add tests for it --- config_app/config_endpoints/api/suconfig.py | 6 +++- .../config_endpoints/api/tar_config_loader.py | 16 ++-------- config_app/config_util/tar.py | 14 +++++++++ config_app/config_util/test/test_tar.py | 29 +++++++++++++++++++ .../download-tarball-modal.component.html | 14 +++++---- .../download-tarball-modal.css | 11 +++++-- .../js/core-config-setup/core-config-setup.js | 12 ++++---- util/config/validator.py | 9 ++++-- util/config/validators/validate_storage.py | 4 +++ 9 files changed, 83 insertions(+), 32 deletions(-) create mode 100644 config_app/config_util/tar.py create mode 100644 config_app/config_util/test/test_tar.py diff --git a/config_app/config_endpoints/api/suconfig.py b/config_app/config_endpoints/api/suconfig.py index 9e17701ab..97bd2a39d 100644 --- a/config_app/config_endpoints/api/suconfig.py +++ b/config_app/config_endpoints/api/suconfig.py @@ -271,11 +271,15 @@ class SuperUserConfigValidate(ApiResource): # Note: This method is called to validate the database configuration before super users exists, # so we also allow it to be called if there is no valid registry configuration setup. Note that # this is also safe since this method does not access any information not given in the request. + + # We can skip localstorage validation, since we can't guarantee that this will be the same machine + # Q.E. will run under config = request.get_json()['config'] validator_context = ValidatorContext.from_app(app, config, request.get_json().get('password', ''), instance_keys=instance_keys, ip_resolver=ip_resolver, - config_provider=config_provider) + config_provider=config_provider, + skip_localstorage_validation=True) return validate_service_for_config(service, validator_context) diff --git a/config_app/config_endpoints/api/tar_config_loader.py b/config_app/config_endpoints/api/tar_config_loader.py index 011de5531..9cde0f68f 100644 --- a/config_app/config_endpoints/api/tar_config_loader.py +++ b/config_app/config_endpoints/api/tar_config_loader.py @@ -5,10 +5,10 @@ import tarfile from flask import request, make_response, send_file from data.database import configure -from util.config.validator import EXTRA_CA_DIRECTORY from config_app.c_app import app, config_provider from config_app.config_endpoints.api import resource, ApiResource, nickname +from config_app.config_util.tar import tarinfo_filter_partial @resource('/v1/configapp/initialization') class ConfigInitialization(ApiResource): @@ -17,7 +17,7 @@ class ConfigInitialization(ApiResource): """ @nickname('scStartNewConfig') - def get(self): + def post(self): config_provider.new_config_dir() return make_response('OK') @@ -37,22 +37,12 @@ class TarConfigLoader(ApiResource): # remove the initial trailing / from the prefix path, and add the last dir one tar_dir_prefix = config_path[1:] + '/' - def tarinfo_filter(tarinfo): - # remove leading directory info - tarinfo.name = tarinfo.name.replace(tar_dir_prefix, '') - - # ignore any directory that isn't the specified extra ca one: - if tarinfo.isdir() and not tarinfo.name == EXTRA_CA_DIRECTORY: - return None - - return tarinfo - temp = tempfile.NamedTemporaryFile() tar = tarfile.open(temp.name, mode="w|gz") for name in os.listdir(config_path): - tar.add(os.path.join(config_path, name), filter=tarinfo_filter) + tar.add(os.path.join(config_path, name), filter=tarinfo_filter_partial(tar_dir_prefix)) tar.close() diff --git a/config_app/config_util/tar.py b/config_app/config_util/tar.py new file mode 100644 index 000000000..379bdfae1 --- /dev/null +++ b/config_app/config_util/tar.py @@ -0,0 +1,14 @@ +from util.config.validator import EXTRA_CA_DIRECTORY + +def tarinfo_filter_partial(prefix): + def tarinfo_filter(tarinfo): + # remove leading directory info + tarinfo.name = tarinfo.name.replace(prefix, '') + + # ignore any directory that isn't the specified extra ca one: + if tarinfo.isdir() and not tarinfo.name == EXTRA_CA_DIRECTORY: + return None + + return tarinfo + + return tarinfo_filter diff --git a/config_app/config_util/test/test_tar.py b/config_app/config_util/test/test_tar.py new file mode 100644 index 000000000..432501e82 --- /dev/null +++ b/config_app/config_util/test/test_tar.py @@ -0,0 +1,29 @@ +import pytest + +from config_app.config_util.tar import tarinfo_filter_partial + +from util.config.validator import EXTRA_CA_DIRECTORY + +from test.fixtures import * + +class MockTarInfo: + def __init__(self, name, isdir): + self.name = name + self.isdir = lambda: isdir + + def __eq__(self, other): + return other is not None and self.name == other.name + +@pytest.mark.parametrize('prefix,tarinfo,expected', [ + # It should handle simple files + ('Users/sam/', MockTarInfo('Users/sam/config.yaml', False), MockTarInfo('config.yaml', False)), + # It should allow the extra CA dir + ('Users/sam/', MockTarInfo('Users/sam/%s' % EXTRA_CA_DIRECTORY, True), MockTarInfo('%s' % EXTRA_CA_DIRECTORY, True)), + # it should allow a file in that extra dir + ('Users/sam/', MockTarInfo('Users/sam/%s/cert.crt' % EXTRA_CA_DIRECTORY, False), MockTarInfo('%s/cert.crt' % EXTRA_CA_DIRECTORY, False)), + # it should not allow a directory that isn't the CA dir + ('Users/sam/', MockTarInfo('Users/sam/dirignore', True), None), +]) +def test_tarinfo_filter(prefix, tarinfo, expected): + partial = tarinfo_filter_partial(prefix) + assert partial(tarinfo) == expected diff --git a/config_app/js/components/download-tarball-modal/download-tarball-modal.component.html b/config_app/js/components/download-tarball-modal/download-tarball-modal.component.html index 89898455f..dfc95c578 100644 --- a/config_app/js/components/download-tarball-modal/download-tarball-modal.component.html +++ b/config_app/js/components/download-tarball-modal/download-tarball-modal.component.html @@ -8,15 +8,17 @@

Download Configuration

-
+
Please download your updated configuration. To deploy these changes to your Quay Enterprise instances, please see the docs.
- Warning: - Your configuration and certificates are kept unencrypted. Please keep this file secure. +
+ Warning: + Your configuration and certificates are kept unencrypted. Please keep this file secure. +
@@ -25,8 +27,10 @@ see the docs.
- Warning: - Your configuration and certificates are kept unencrypted. Please keep this file secure. +
+ Warning: + Your configuration and certificates are kept unencrypted. Please keep this file secure. +
diff --git a/config_app/js/components/download-tarball-modal/download-tarball-modal.css b/config_app/js/components/download-tarball-modal/download-tarball-modal.css index 3f68a1a7a..eada2902c 100644 --- a/config_app/js/components/download-tarball-modal/download-tarball-modal.css +++ b/config_app/js/components/download-tarball-modal/download-tarball-modal.css @@ -1,6 +1,11 @@ -.modal__warning-box { - background-color: #ddd; +.co-dialog .modal-body.download-tarball-modal { padding: 15px; - border-radius: 5px; +} + +.modal__warning-box { margin-top: 15px; } + +.modal__warning-box .co-alert.co-alert-warning::before { + font-family:Font Awesome\ 5 Free; +} diff --git a/config_app/js/core-config-setup/core-config-setup.js b/config_app/js/core-config-setup/core-config-setup.js index 21fc8aa38..73841712a 100644 --- a/config_app/js/core-config-setup/core-config-setup.js +++ b/config_app/js/core-config-setup/core-config-setup.js @@ -39,13 +39,7 @@ angular.module("quay-config") $scope.SERVICES = [ {'id': 'redis', 'title': 'Redis'}, - {'id': 'registry-storage', 'title': 'Registry Storage', 'condition': (config) => { - // We can skip validation if all of the storage locations are local, as we can't - // guarantee that this will be the same machine Q.E. will run under. Therefore, - // we just have a warning to the user that Q.E. won't start if the locations don't match - return Object.values(config.DISTRIBUTED_STORAGE_CONFIG) - .some(storageTuple => storageTuple[0] !== 'LocalStorage') - }}, + {'id': 'registry-storage', 'title': 'Registry Storage'}, {'id': 'time-machine', 'title': 'Time Machine'}, @@ -417,6 +411,10 @@ angular.module("quay-config") $scope.saveConfiguration = function() { $scope.savingConfiguration = true; + // Make sure to note that fully verified setup is completed. We use this as a signal + // in the setup tool. + $scope.config['SETUP_COMPLETE'] = true; + var data = { 'config': $scope.config, 'hostname': window.location.host, diff --git a/util/config/validator.py b/util/config/validator.py index a0924c9e2..444d8340a 100644 --- a/util/config/validator.py +++ b/util/config/validator.py @@ -102,7 +102,8 @@ class ValidatorContext(object): def __init__(self, config, user_password=None, http_client=None, context=None, url_scheme_and_hostname=None, jwt_auth_max=None, registry_title=None, ip_resolver=None, feature_sec_scanner=False, is_testing=False, - uri_creator=None, config_provider=None, instance_keys=None): + uri_creator=None, config_provider=None, instance_keys=None, + skip_localstorage_validation=False): self.config = config self.user = get_authenticated_user() self.user_password = user_password @@ -117,10 +118,11 @@ class ValidatorContext(object): self.uri_creator = uri_creator self.config_provider = config_provider self.instance_keys = instance_keys + self.skip_localstorage_validation = skip_localstorage_validation @classmethod def from_app(cls, app, config, user_password, ip_resolver, instance_keys, client=None, - config_provider=None): + config_provider=None, skip_localstorage_validation=False): """ Creates a ValidatorContext from an app config, with a given config to validate :param app: the Flask app to pull configuration information from @@ -146,4 +148,5 @@ class ValidatorContext(object): is_testing=app.config.get('TESTING', False), uri_creator=get_blob_download_uri_getter(app.test_request_context('/'), url_scheme_and_hostname), config_provider=config_provider, - instance_keys=instance_keys) + instance_keys=instance_keys, + skip_localstorage_validation=skip_localstorage_validation) diff --git a/util/config/validators/validate_storage.py b/util/config/validators/validate_storage.py index 3e3de74ee..366405739 100644 --- a/util/config/validators/validate_storage.py +++ b/util/config/validators/validate_storage.py @@ -12,6 +12,7 @@ class StorageValidator(BaseValidator): client = validator_context.http_client ip_resolver = validator_context.ip_resolver config_provider = validator_context.config_provider + skip_localstorage_validation = validator_context.skip_localstorage_validation replication_enabled = config.get('FEATURE_STORAGE_REPLICATION', False) @@ -20,6 +21,9 @@ class StorageValidator(BaseValidator): raise ConfigValidationException('Storage configuration required') for name, (storage_type, driver) in providers: + if skip_localstorage_validation and storage_type == 'LocalStorage': + continue + try: if replication_enabled and storage_type == 'LocalStorage': raise ConfigValidationException('Locally mounted directory not supported ' +