From 872be8605a35feade7136108968b2a715132b977 Mon Sep 17 00:00:00 2001 From: Sam Chow Date: Fri, 22 Jun 2018 13:21:44 -0400 Subject: [PATCH] Fix error case in uploading tar, more comments --- config_app/config_endpoints/api/suconfig.py | 1 - config_app/config_endpoints/api/superuser.py | 13 ++++++++++++- .../config_util/config/inmemoryprovider.py | 11 ++++------- .../load-config/load-config.component.ts | 16 +++++++++++++--- config_app/js/setup/setup.component.js | 7 +++---- data/migrations/env.py | 6 +++++- 6 files changed, 37 insertions(+), 17 deletions(-) diff --git a/config_app/config_endpoints/api/suconfig.py b/config_app/config_endpoints/api/suconfig.py index e50f3859a..ec4a84105 100644 --- a/config_app/config_endpoints/api/suconfig.py +++ b/config_app/config_endpoints/api/suconfig.py @@ -335,4 +335,3 @@ class SuperUserConfigFile(ApiResource): return { 'status': True } - diff --git a/config_app/config_endpoints/api/superuser.py b/config_app/config_endpoints/api/superuser.py index b7528820b..0a4705d03 100644 --- a/config_app/config_endpoints/api/superuser.py +++ b/config_app/config_endpoints/api/superuser.py @@ -1,11 +1,13 @@ import logging import pathvalidate +import os + from flask import request, jsonify from config_app.config_endpoints.exception import InvalidRequest from config_app.config_endpoints.api import resource, ApiResource, nickname from config_app.config_util.ssl import load_certificate, CertInvalidException -from config_app.c_app import config_provider +from config_app.c_app import app, config_provider from config_app.config_endpoints.api.superuser_models_pre_oci import pre_oci_model @@ -45,6 +47,15 @@ class SuperUserCustomCertificate(ApiResource): logger.exception('Got IO error for cert %s', certpath) return '', 204 + # TODO(QUAY-991): properly install the custom certs provided by user + # Call the update script to install the certificate immediately. + # if not app.config['TESTING']: + # logger.debug('Calling certs_install.sh') + # if os.system('/conf/init/certs_install.sh') != 0: + # raise Exception('Could not install certificates') + # + # logger.debug('certs_install.sh completed') + return '', 204 @nickname('deleteCustomCertificate') diff --git a/config_app/config_util/config/inmemoryprovider.py b/config_app/config_util/config/inmemoryprovider.py index 638c5458a..7db5d1a89 100644 --- a/config_app/config_util/config/inmemoryprovider.py +++ b/config_app/config_util/config/inmemoryprovider.py @@ -1,6 +1,7 @@ import logging import yaml import io +import os from config_app.config_util.config.baseprovider import BaseProvider @@ -36,7 +37,7 @@ class InMemoryProvider(BaseProvider): return True def volume_file_exists(self, filename): - return any([ name.startswith(filename) for name in self.files ]) + return any([name.startswith(filename) for name in self.files]) def get_volume_file(self, filename, mode='r'): return io.BytesIO(self.files[filename]) @@ -53,7 +54,7 @@ class InMemoryProvider(BaseProvider): return string[string.rfind('/') + 1:] return string - return [ strip_directory(name) for name in self.files if name.startswith(path) ] + return [strip_directory(name) for name in self.files if name.startswith(path)] def save_volume_file(self, filename, flask_file): self.files[filename] = flask_file.read() @@ -62,11 +63,7 @@ class InMemoryProvider(BaseProvider): raise Exception('Not implemented yet') def get_volume_path(self, directory, filename): - # Here we can just access the filename since we're storing the tarball files with their full path - if directory.endswith('/'): - return directory + filename - else: - return directory + '/' + filename + return os.path.join(directory, filename) def load_from_tarball(self, tarfile): for tarinfo in tarfile.getmembers(): diff --git a/config_app/js/components/load-config/load-config.component.ts b/config_app/js/components/load-config/load-config.component.ts index 612702686..eb93b36cd 100644 --- a/config_app/js/components/load-config/load-config.component.ts +++ b/config_app/js/components/load-config/load-config.component.ts @@ -2,6 +2,8 @@ import {Component, EventEmitter, Inject, Output} from 'ng-metadata/core'; const templateUrl = require('./load-config.html'); const styleUrl = require('./load-config.css'); +declare var bootbox: any; + @Component({ selector: 'load-config', templateUrl, @@ -29,9 +31,17 @@ export class LoadConfigComponent { if (success) { this.configLoaded.emit({}); } else { - this.apiService.errorDisplay('Error loading configuration', - 'Could not upload configuration. Please reload the page and try again.\n' + - 'If this problem persists, please contact support')(); + bootbox.dialog({ + "message": 'Could not upload configuration. Please check you have provided a valid tar file' + + 'If this problem persists, please contact support', + "title": 'Error Loading Configuration', + "buttons": { + "close": { + "label": "Close", + "className": "btn-primary" + } + } + }); } }); } diff --git a/config_app/js/setup/setup.component.js b/config_app/js/setup/setup.component.js index 1ce709099..b8fe72bd3 100644 --- a/config_app/js/setup/setup.component.js +++ b/config_app/js/setup/setup.component.js @@ -31,10 +31,9 @@ const templateUrl = require('./setup.html'); $scope.HOSTNAME_REGEX = '^[a-zA-Z-0-9_\.\-]+(:[0-9]+)?$'; $scope.validateHostname = function(hostname) { - // TODO(sam): maybe revert? - // if (hostname.indexOf('127.0.0.1') == 0 || hostname.indexOf('localhost') == 0) { - // return 'Please specify a non-localhost hostname. "localhost" will refer to the container, not your machine.' - // } + if (hostname.indexOf('127.0.0.1') == 0 || hostname.indexOf('localhost') == 0) { + return 'Please specify a non-localhost hostname. "localhost" will refer to the container, not your machine.' + } return null; }; diff --git a/data/migrations/env.py b/data/migrations/env.py index 7f10b65d7..d920dd13d 100644 --- a/data/migrations/env.py +++ b/data/migrations/env.py @@ -21,8 +21,12 @@ from util.morecollections import AttrDict config = context.config DB_URI = config.get_main_option('db_uri', 'sqlite:///test/data/test.db') + +# This option exists because alembic needs the db proxy to be configured in order +# to perform migrations. The app import does the init of the proxy, but we don't +# want that in the case of the config app, as we are explicitly connecting to a +# db that the user has passed in, and we can't have import dependency on app if config.get_main_option('alembic_setup_app', 'True') == 'True': - # needed for db connections from app import app DB_URI = app.config['DB_URI']