From c30214c7a89c726aeec0f98d324f7870c5ba7fec Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Mon, 13 Aug 2018 18:09:05 -0400 Subject: [PATCH 1/7] Start on a basic registry_model interface and change a single module to use it. This will allow us to completely abstract out how we deal with registry-related tables and ensure that transitioning to the new OCI-like model will be easier to do. --- buildman/jobutil/buildjob.py | 68 ++++--------------- data/model/tag.py | 27 ++++++++ data/registry_model/__init__.py | 3 + data/registry_model/datatypes.py | 20 ++++++ data/registry_model/interface.py | 21 ++++++ data/registry_model/registry_pre_oci_model.py | 27 ++++++++ .../registry_model/test/test_pre_oci_model.py | 40 +++++++++++ 7 files changed, 150 insertions(+), 56 deletions(-) create mode 100644 data/registry_model/__init__.py create mode 100644 data/registry_model/datatypes.py create mode 100644 data/registry_model/interface.py create mode 100644 data/registry_model/registry_pre_oci_model.py create mode 100644 data/registry_model/test/test_pre_oci_model.py diff --git a/buildman/jobutil/buildjob.py b/buildman/jobutil/buildjob.py index 9ea3cac56..253d85ac9 100644 --- a/buildman/jobutil/buildjob.py +++ b/buildman/jobutil/buildjob.py @@ -5,6 +5,8 @@ from app import app from cachetools import lru_cache from notifications import spawn_notification from data import model +from data.registry_model import registry_model +from data.registry_model.datatypes import RepositoryReference from data.database import UseThenDisconnect from util.imagetree import ImageTree from util.morecollections import AttrDict @@ -27,7 +29,7 @@ class BuildJob(object): self.build_notifier = BuildJobNotifier(self.build_uuid) except ValueError: raise BuildJobLoadException( - 'Could not parse build queue item config with ID %s' % self.job_details['build_uuid'] + 'Could not parse build queue item config with ID %s' % self.job_details['build_uuid'] ) @property @@ -95,70 +97,24 @@ class BuildJob(object): def determine_cached_tag(self, base_image_id=None, cache_comments=None): """ Returns the tag to pull to prime the cache or None if none. """ - cached_tag = None - if base_image_id and cache_comments: - cached_tag = self._determine_cached_tag_by_comments(base_image_id, cache_comments) - - if not cached_tag: - cached_tag = self._determine_cached_tag_by_tag() - + cached_tag = self._determine_cached_tag_by_tag() logger.debug('Determined cached tag %s for %s: %s', cached_tag, base_image_id, cache_comments) - return cached_tag - def _determine_cached_tag_by_comments(self, base_image_id, cache_commands): - """ Determines the tag to use for priming the cache for this build job, by matching commands - starting at the given base_image_id. This mimics the Docker cache checking, so it should, - in theory, provide "perfect" caching. - """ - with UseThenDisconnect(app.config): - # Lookup the base image in the repository. If it doesn't exist, nothing more to do. - repo_build = self.repo_build - repo_namespace = repo_build.repository.namespace_user.username - repo_name = repo_build.repository.name - - base_image = model.image.get_image(repo_build.repository, base_image_id) - if base_image is None: - return None - - # Build an in-memory tree of the full heirarchy of images in the repository. - all_images = model.image.get_repository_images_without_placements(repo_build.repository, - with_ancestor=base_image) - - all_tags = model.tag.list_repository_tags(repo_namespace, repo_name) - tree = ImageTree(all_images, all_tags, base_filter=base_image.id) - - # Find a path in the tree, starting at the base image, that matches the cache comments - # or some subset thereof. - def checker(step, image): - if step >= len(cache_commands): - return False - - full_command = '["/bin/sh", "-c", "%s"]' % cache_commands[step] - logger.debug('Checking step #%s: %s, %s == %s', step, image.id, image.command, full_command) - - return image.command == full_command - - path = tree.find_longest_path(base_image.id, checker) - if not path: - return None - - # Find any tag associated with the last image in the path. - return tree.tag_containing_image(path[-1]) - - def _determine_cached_tag_by_tag(self): """ Determines the cached tag by looking for one of the tags being built, and seeing if it exists in the repository. This is a fallback for when no comment information is available. """ with UseThenDisconnect(app.config): tags = self.build_config.get('docker_tags', ['latest']) - repository = self.repo_build.repository - existing_tags = model.tag.list_repository_tags(repository.namespace_user.username, - repository.name) - cached_tags = set(tags) & set([tag.name for tag in existing_tags]) - if cached_tags: - return list(cached_tags)[0] + repository = RepositoryReference.for_repo_obj(self.repo_build.repository) + matching_tag = registry_model.find_matching_tag(repository, tags) + if matching_tag is not None: + return matching_tag.name + + most_recent_tag = registry_model.get_most_recent_tag(repository) + if most_recent_tag is not None: + return most_recent_tag.name return None diff --git a/data/model/tag.py b/data/model/tag.py index ef49cf51f..9748b5b73 100644 --- a/data/model/tag.py +++ b/data/model/tag.py @@ -722,3 +722,30 @@ def change_tag_expiration(tag, expiration_date): .execute()) return (tag.lifetime_end_ts, result > 0) + + +def find_matching_tag(repo_id, tag_names): + """ Finds the most recently pushed alive tag in the repository with one of the given names, + if any. + """ + try: + return (_tag_alive(RepositoryTag + .select() + .where(RepositoryTag.repository == repo_id, + RepositoryTag.name << list(tag_names)) + .order_by(RepositoryTag.lifetime_start_ts.desc())) + .get()) + except RepositoryTag.DoesNotExist: + return None + + +def get_most_recent_tag(repo_id): + """ Returns the most recently pushed alive tag in the repository, or None if none. """ + try: + return (_tag_alive(RepositoryTag + .select() + .where(RepositoryTag.repository == repo_id) + .order_by(RepositoryTag.lifetime_start_ts.desc())) + .get()) + except RepositoryTag.DoesNotExist: + return None diff --git a/data/registry_model/__init__.py b/data/registry_model/__init__.py new file mode 100644 index 000000000..484bb7e41 --- /dev/null +++ b/data/registry_model/__init__.py @@ -0,0 +1,3 @@ +from data.registry_model.registry_pre_oci_model import pre_oci_model + +registry_model = pre_oci_model diff --git a/data/registry_model/datatypes.py b/data/registry_model/datatypes.py new file mode 100644 index 000000000..3f2cae187 --- /dev/null +++ b/data/registry_model/datatypes.py @@ -0,0 +1,20 @@ +from collections import namedtuple + +class RepositoryReference(object): + """ RepositoryReference is a reference to a repository, passed to registry interface methods. """ + def __init__(self, repo_id): + self.repo_id = repo_id + + @classmethod + def for_repo_obj(cls, repo_obj): + return RepositoryReference(repo_obj.id) + + +class Tag(namedtuple('Tag', ['id', 'name'])): + """ Tag represents a tag in a repository, which points to a manifest or image. """ + @classmethod + def for_repository_tag(cls, repository_tag): + if repository_tag is None: + return None + + return Tag(id=repository_tag.id, name=repository_tag.name) diff --git a/data/registry_model/interface.py b/data/registry_model/interface.py new file mode 100644 index 000000000..e67366733 --- /dev/null +++ b/data/registry_model/interface.py @@ -0,0 +1,21 @@ +from abc import ABCMeta, abstractmethod +from six import add_metaclass + +@add_metaclass(ABCMeta) +class RegistryDataInterface(object): + """ Interface for code to work with the registry data model. The registry data model consists + of all tables that store registry-specific information, such as Manifests, Blobs, Images, + and Labels. + """ + + @abstractmethod + def find_matching_tag(self, repository_ref, tag_names): + """ Finds an alive tag in the repository matching one of the given tag names and returns it + or None if none. + """ + + @abstractmethod + def get_most_recent_tag(self, repository_ref): + """ Returns the most recently pushed alive tag in the repository, if any. If none, returns + None. + """ diff --git a/data/registry_model/registry_pre_oci_model.py b/data/registry_model/registry_pre_oci_model.py new file mode 100644 index 000000000..5cd6ed631 --- /dev/null +++ b/data/registry_model/registry_pre_oci_model.py @@ -0,0 +1,27 @@ +from data import model +from data.registry_model.interface import RegistryDataInterface +from data.registry_model.datatypes import Tag + + +class PreOCIModel(RegistryDataInterface): + """ + PreOCIModel implements the data model for the registry API using a database schema + before it was changed to support the OCI specification. + """ + + def find_matching_tag(self, repository_ref, tag_names): + """ Finds an alive tag in the repository matching one of the given tag names and returns it + or None if none. + """ + found_tag = model.tag.find_matching_tag(repository_ref.repo_id, tag_names) + return Tag.for_repository_tag(found_tag) + + def get_most_recent_tag(self, repository_ref): + """ Returns the most recently pushed alive tag in the repository, if any. If none, returns + None. + """ + found_tag = model.tag.get_most_recent_tag(repository_ref.repo_id) + return Tag.for_repository_tag(found_tag) + + +pre_oci_model = PreOCIModel() diff --git a/data/registry_model/test/test_pre_oci_model.py b/data/registry_model/test/test_pre_oci_model.py new file mode 100644 index 000000000..3d7140475 --- /dev/null +++ b/data/registry_model/test/test_pre_oci_model.py @@ -0,0 +1,40 @@ +import pytest + +from data import model +from data.registry_model.registry_pre_oci_model import PreOCIModel +from data.registry_model.datatypes import RepositoryReference +from test.fixtures import * + +@pytest.fixture() +def pre_oci_model(initialized_db): + return PreOCIModel() + + +@pytest.mark.parametrize('names, expected', [ + (['unknown'], None), + (['latest'], 'latest'), + (['latest', 'prod'], 'latest'), + (['foo', 'prod'], 'prod'), +]) +def test_find_matching_tag(names, expected, pre_oci_model): + repo = model.repository.get_repository('devtable', 'simple') + repository_ref = RepositoryReference.for_repo_obj(repo) + found = pre_oci_model.find_matching_tag(repository_ref, names) + if expected is None: + assert found is None + else: + assert found.name == expected + + +@pytest.mark.parametrize('repo_namespace, repo_name, expected', [ + ('devtable', 'simple', 'latest'), + ('buynlarge', 'orgrepo', 'latest'), +]) +def test_get_most_recent_tag(repo_namespace, repo_name, expected, pre_oci_model): + repo = model.repository.get_repository(repo_namespace, repo_name) + repository_ref = RepositoryReference.for_repo_obj(repo) + found = pre_oci_model.get_most_recent_tag(repository_ref) + if expected is None: + assert found is None + else: + assert found.name == expected From 5b400f4c22f678ae96787f29a2a7618d8579d6e1 Mon Sep 17 00:00:00 2001 From: Sam Chow Date: Wed, 15 Aug 2018 17:17:41 -0400 Subject: [PATCH 2/7] Add one-action transplant of kube secret --- .../config/TransientDirectoryProvider.py | 21 ++++++++-- config_app/config_util/k8saccessor.py | 38 +++++++++++++++++-- util/config/validator.py | 1 + 3 files changed, 53 insertions(+), 7 deletions(-) diff --git a/config_app/config_util/config/TransientDirectoryProvider.py b/config_app/config_util/config/TransientDirectoryProvider.py index 2525baaea..33c6f4216 100644 --- a/config_app/config_util/config/TransientDirectoryProvider.py +++ b/config_app/config_util/config/TransientDirectoryProvider.py @@ -1,8 +1,12 @@ import os +import base64 + from backports.tempfile import TemporaryDirectory from config_app.config_util.config.fileprovider import FileConfigProvider from config_app.config_util.k8saccessor import KubernetesAccessorSingleton +from util.config.validator import EXTRA_CA_DIRECTORY, EXTRA_CA_DIRECTORY_PREFIX + class TransientDirectoryProvider(FileConfigProvider): @@ -38,10 +42,21 @@ class TransientDirectoryProvider(FileConfigProvider): return self.config_volume def save_configuration_to_kubernetes(self): - config_path = self.get_config_dir_path() + data = {} - for name in os.listdir(config_path): + certs_dir = os.path.join(self.config_volume, EXTRA_CA_DIRECTORY) + if os.path.exists(certs_dir): + for extra_cert in os.listdir(certs_dir): + with open(os.path.join(certs_dir, extra_cert)) as f: + data[EXTRA_CA_DIRECTORY_PREFIX + extra_cert] = base64.b64encode(f.read()) + + + for name in os.listdir(self.config_volume): file_path = os.path.join(self.config_volume, name) - KubernetesAccessorSingleton.get_instance().save_file_as_secret(name, file_path) + if not os.path.isdir(file_path): + with open(file_path) as f: + data[name] = base64.b64encode(f.read()) + + KubernetesAccessorSingleton.get_instance().replace_qe_secret(data) return 200 diff --git a/config_app/config_util/k8saccessor.py b/config_app/config_util/k8saccessor.py index 61efb04fc..d498ede4d 100644 --- a/config_app/config_util/k8saccessor.py +++ b/config_app/config_util/k8saccessor.py @@ -35,10 +35,40 @@ class KubernetesAccessorSingleton(object): return cls._instance - def save_file_as_secret(self, name, file_path): - with open(file_path) as f: - value = f.read() - self._update_secret_file(name, value) + def save_file_as_secret(self, name, file_pointer): + value = file_pointer.read() + self._update_secret_file(name, value) + + def replace_qe_secret(self, new_secret_data): + """ + Removes the old config and replaces it with the new_secret_data as one action + """ + # Check first that the namespace for Quay Enterprise exists. If it does not, report that + # as an error, as it seems to be a common issue. + namespace_url = 'namespaces/%s' % (self.kube_config.qe_namespace) + response = self._execute_k8s_api('GET', namespace_url) + if response.status_code // 100 != 2: + msg = 'A Kubernetes namespace with name `%s` must be created to save config' % self.kube_config.qe_namespace + raise Exception(msg) + + # Check if the secret exists. If not, then we create an empty secret and then update the file + # inside. + secret_url = 'namespaces/%s/secrets/%s' % (self.kube_config.qe_namespace, self.kube_config.qe_config_secret) + secret = self._lookup_secret() + if secret is None: + self._assert_success(self._execute_k8s_api('POST', secret_url, { + "kind": "Secret", + "apiVersion": "v1", + "metadata": { + "name": self.kube_config.qe_config_secret + }, + "data": {} + })) + + # Update the secret to reflect the file change. + secret['data'] = new_secret_data + + self._assert_success(self._execute_k8s_api('PUT', secret_url, secret)) def get_qe_deployments(self): """" diff --git a/util/config/validator.py b/util/config/validator.py index 54f23938d..24d45570d 100644 --- a/util/config/validator.py +++ b/util/config/validator.py @@ -41,6 +41,7 @@ CONFIG_FILENAMES = (SSL_FILENAMES + DB_SSL_FILENAMES + JWT_FILENAMES + ACI_CERT_ LDAP_FILENAMES) CONFIG_FILE_SUFFIXES = ['-cloudfront-signing-key.pem'] EXTRA_CA_DIRECTORY = 'extra_ca_certs' +EXTRA_CA_DIRECTORY_PREFIX = 'extra_ca_certs_' VALIDATORS = { DatabaseValidator.name: DatabaseValidator.validate, From ff294d6c52d004262a739e3fb677d73e47b86fac Mon Sep 17 00:00:00 2001 From: Sam Chow Date: Thu, 16 Aug 2018 15:42:01 -0400 Subject: [PATCH 3/7] Add init script to download extra ca certs --- conf/init/02_get_kube_certs.py | 71 +++++++++++++++++++ conf/init/02_get_kube_certs.sh | 12 ++++ conf/init/certs_install.sh | 30 ++++---- config_app/conf/server-base.conf | 3 + config_app/config_endpoints/api/superuser.py | 4 +- .../config/TransientDirectoryProvider.py | 4 ++ .../kube-deploy-modal.component.ts | 5 +- 7 files changed, 112 insertions(+), 17 deletions(-) create mode 100644 conf/init/02_get_kube_certs.py create mode 100755 conf/init/02_get_kube_certs.sh diff --git a/conf/init/02_get_kube_certs.py b/conf/init/02_get_kube_certs.py new file mode 100644 index 000000000..3f88a09ac --- /dev/null +++ b/conf/init/02_get_kube_certs.py @@ -0,0 +1,71 @@ +import json +import os +import base64 + +from requests import Request, Session + +QUAYPATH = os.environ.get('QUAYPATH', '.') +KUBE_EXTRA_CA_CERTDIR = os.environ.get('KUBE_EXTRA_CA_CERTDIR', '%s/conf/kube_extra_certs' % QUAYPATH) + +KUBERNETES_API_HOST = os.environ.get('KUBERNETES_SERVICE_HOST', '') +port = os.environ.get('KUBERNETES_SERVICE_PORT') +if port: + KUBERNETES_API_HOST += ':' + port + +SERVICE_ACCOUNT_TOKEN_PATH = '/var/run/secrets/kubernetes.io/serviceaccount/token' + +QE_NAMESPACE = os.environ.get('QE_K8S_NAMESPACE', 'quay-enterprise') +QE_CONFIG_SECRET = os.environ.get('QE_K8S_CONFIG_SECRET', 'quay-enterprise-config-secret') +EXTRA_CA_DIRECTORY_PREFIX = 'extra_ca_certs_' + + +def _lookup_secret(service_token): + secret_url = 'namespaces/%s/secrets/%s' % (QE_NAMESPACE, QE_CONFIG_SECRET) + response = _execute_k8s_api(service_token, 'GET', secret_url) + if response.status_code != 200: + raise Exception('Cannot get the config secret') + return json.loads(response.text) + +def _execute_k8s_api(service_account_token, method, relative_url, data=None, api_prefix='api/v1', content_type='application/json'): + headers = { + 'Authorization': 'Bearer ' + service_account_token + } + + if data: + headers['Content-Type'] = content_type + + data = json.dumps(data) if data else None + session = Session() + url = 'https://%s/%s/%s' % (KUBERNETES_API_HOST, api_prefix, relative_url) + + request = Request(method, url, data=data, headers=headers) + return session.send(request.prepare(), verify=False, timeout=2) + +def is_extra_cert(key): + return key.find(EXTRA_CA_DIRECTORY_PREFIX) == 0 + +def main(): + # Load the service account token from the local store. + if not os.path.exists(SERVICE_ACCOUNT_TOKEN_PATH): + raise Exception('Cannot load Kubernetes service account token') + + with open(SERVICE_ACCOUNT_TOKEN_PATH, 'r') as f: + service_token = f.read() + + secret_data = _lookup_secret(service_token).get('data', {}) + cert_keys = filter(is_extra_cert, secret_data.keys()) + + for cert_key in cert_keys: + if not os.path.exists(KUBE_EXTRA_CA_CERTDIR): + os.mkdir(KUBE_EXTRA_CA_CERTDIR) + + cert_value = base64.b64decode(secret_data[cert_key]) + cert_filename = cert_key.replace(EXTRA_CA_DIRECTORY_PREFIX, '') + print "Found an extra cert %s in config-secret, copying to kube ca dir" + + with open(os.path.join(KUBE_EXTRA_CA_CERTDIR, cert_filename), 'w') as f: + f.write(cert_value) + + +if __name__ == '__main__': + main() diff --git a/conf/init/02_get_kube_certs.sh b/conf/init/02_get_kube_certs.sh new file mode 100755 index 000000000..997847d17 --- /dev/null +++ b/conf/init/02_get_kube_certs.sh @@ -0,0 +1,12 @@ +#!/usr/bin/env bash + +QUAYDIR=${QUAYDIR:-"/"} +QUAYPATH=${QUAYPATH:-"."} +QUAYCONF=${QUAYCONF:-"$QUAYPATH/conf"} + +cd $QUAYDIR + +if [[ "$KUBERNETES_SERVICE_HOST" != "" ]];then + echo "Running on kubernetes, attempting to retrieve extra certs from secret" + venv/bin/python $QUAYCONF/init/02_get_kube_certs.py +fi \ No newline at end of file diff --git a/conf/init/certs_install.sh b/conf/init/certs_install.sh index 29be4e8e5..e58d282bb 100755 --- a/conf/init/certs_install.sh +++ b/conf/init/certs_install.sh @@ -3,6 +3,12 @@ set -e QUAYPATH=${QUAYPATH:-"."} QUAYCONF=${QUAYCONF:-"$QUAYPATH/conf/stack"} QUAYCONFIG=${QUAYCONFIG:-"$QUAYCONF/stack"} +CERTDIR=${QUAYCONFIG/extra_ca_certs} + +# If we're running under kube, the previous script (02_get_kube_certs.sh) will put the certs in a different location +if [[ "$KUBERNETES_SERVICE_HOST" != "" ]];then + CERTDIR=${KUBE_EXTRA_CA_CERTDIR:-"$QUAYPATH/conf/kube_extra_certs"} +fi cd ${QUAYDIR:-"/quay-registry"} @@ -13,25 +19,25 @@ then fi # Add extra trusted certificates (as a directory) -if [ -d $QUAYCONFIG/extra_ca_certs ]; then - if test "$(ls -A "$QUAYCONFIG/extra_ca_certs")"; then - echo "Installing extra certificates found in $QUAYCONFIG/extra_ca_certs directory" - cp $QUAYCONFIG/extra_ca_certs/* /usr/local/share/ca-certificates/ - cat $QUAYCONFIG/extra_ca_certs/* >> venv/lib/python2.7/site-packages/requests/cacert.pem - cat $QUAYCONFIG/extra_ca_certs/* >> venv/lib/python2.7/site-packages/certifi/cacert.pem +if [ -d $CERTDIR ]; then + if test "$(ls -A "$CERTDIR")"; then + echo "Installing extra certificates found in $CERTDIR directory" + cp $CERTDIR/* /usr/local/share/ca-certificates/ + cat $CERTDIR/* >> venv/lib/python2.7/site-packages/requests/cacert.pem + cat $CERTDIR/* >> venv/lib/python2.7/site-packages/certifi/cacert.pem fi fi # Add extra trusted certificates (as a file) -if [ -f $QUAYCONFIG/extra_ca_certs ]; then - echo "Installing extra certificates found in $QUAYCONFIG/extra_ca_certs file" - csplit -z -f /usr/local/share/ca-certificates/extra-ca- $QUAYCONFIG/extra_ca_certs '/-----BEGIN CERTIFICATE-----/' '{*}' - cat $QUAYCONFIG/extra_ca_certs >> venv/lib/python2.7/site-packages/requests/cacert.pem - cat $QUAYCONFIG/extra_ca_certs >> venv/lib/python2.7/site-packages/certifi/cacert.pem +if [ -f $CERTDIR ]; then + echo "Installing extra certificates found in $CERTDIR file" + csplit -z -f /usr/local/share/ca-certificates/extra-ca- $CERTDIR '/-----BEGIN CERTIFICATE-----/' '{*}' + cat $CERTDIR >> venv/lib/python2.7/site-packages/requests/cacert.pem + cat $CERTDIR >> venv/lib/python2.7/site-packages/certifi/cacert.pem fi # Add extra trusted certificates (prefixed) -for f in $(find $QUAYCONFIG/ -maxdepth 1 -type f -name "extra_ca*") +for f in $(find $CERTDIR/ -maxdepth 1 -type f -name "extra_ca*") do echo "Installing extra cert $f" cp "$f" /usr/local/share/ca-certificates/ diff --git a/config_app/conf/server-base.conf b/config_app/conf/server-base.conf index b1732ed20..bb7af7bf9 100644 --- a/config_app/conf/server-base.conf +++ b/config_app/conf/server-base.conf @@ -10,6 +10,9 @@ proxy_redirect off; proxy_set_header Transfer-Encoding $http_transfer_encoding; +# The DB migrations sometimes take a while, so increase timeoutso we don't report an error +proxy_read_timeout 300s; + location / { proxy_pass http://web_app_server; } diff --git a/config_app/config_endpoints/api/superuser.py b/config_app/config_endpoints/api/superuser.py index 7cca94012..a3f1039b3 100644 --- a/config_app/config_endpoints/api/superuser.py +++ b/config_app/config_endpoints/api/superuser.py @@ -54,8 +54,8 @@ class SuperUserCustomCertificate(ApiResource): return '', 204 # Call the update script with config dir location to install the certificate immediately. - if subprocess.call([os.path.join(INIT_SCRIPTS_LOCATION, 'certs_install.sh')], - env={ 'QUAYCONFIG': config_provider.get_config_dir_path() }) != 0: + cert_dir = os.path.join(config_provider.get_config_dir_path(), EXTRA_CA_DIRECTORY) + if subprocess.call([os.path.join(INIT_SCRIPTS_LOCATION, 'certs_install.sh')], env={ 'CERTDIR': cert_dir }) != 0: raise Exception('Could not install certificates') return '', 204 diff --git a/config_app/config_util/config/TransientDirectoryProvider.py b/config_app/config_util/config/TransientDirectoryProvider.py index 33c6f4216..7dab7dcf3 100644 --- a/config_app/config_util/config/TransientDirectoryProvider.py +++ b/config_app/config_util/config/TransientDirectoryProvider.py @@ -44,6 +44,10 @@ class TransientDirectoryProvider(FileConfigProvider): def save_configuration_to_kubernetes(self): data = {} + # Kubernetes secrets don't have sub-directories, so for the extra_ca_certs dir + # we have to put the extra certs in with a prefix, and then one of our init scripts + # (02_get_kube_certs.sh) will expand the prefixed certs into the equivalent directory + # so that they'll be installed correctly on startup by the certs_install script certs_dir = os.path.join(self.config_volume, EXTRA_CA_DIRECTORY) if os.path.exists(certs_dir): for extra_cert in os.listdir(certs_dir): diff --git a/config_app/js/components/kube-deploy-modal/kube-deploy-modal.component.ts b/config_app/js/components/kube-deploy-modal/kube-deploy-modal.component.ts index 47eec013b..d66b442da 100644 --- a/config_app/js/components/kube-deploy-modal/kube-deploy-modal.component.ts +++ b/config_app/js/components/kube-deploy-modal/kube-deploy-modal.component.ts @@ -24,7 +24,7 @@ export class KubeDeployModalComponent { this.state = 'loadingDeployments'; ApiService.scGetNumDeployments().then(resp => { - this.deploymentsStatus = resp.items.map(dep => ({ name: dep.metadata.name, numPods: dep.status.replicas })); + this.deploymentsStatus = resp.items.map(dep => ({ name: dep.metadata.name, numPods: dep.spec.replicas })); this.state = 'readyToDeploy'; }).catch(err => { this.state = 'error'; @@ -37,7 +37,7 @@ export class KubeDeployModalComponent { deployConfiguration(): void { this.ApiService.scDeployConfiguration().then(() => { - const deploymentNames: string[]= this.deploymentsStatus.map(dep => dep.name); + const deploymentNames: string[] = this.deploymentsStatus.map(dep => dep.name); this.ApiService.scCycleQEDeployments({ deploymentNames }).then(() => { this.state = 'deployed' @@ -46,7 +46,6 @@ export class KubeDeployModalComponent { this.errorMessage = `Could cycle the deployments with the new configuration. Error: ${err.toString()}`; }) }).catch(err => { - console.log(err) this.state = 'error'; this.errorMessage = `Could not deploy the configuration. Error: ${err.toString()}`; }) From e30b746aeffee4968105a317f37b2f00b914797b Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Sun, 19 Aug 2018 23:50:18 -0400 Subject: [PATCH 4/7] Fix TagManifests with shared digests under the same repository. TagManifests can (apparently, in very rare scenarios) share manifests with the exact same digests, so we need to support that case in the backfill worker. We also need to remove a unique constraint on the manifest column in the mapping table to support this case. --- data/database.py | 2 +- ...emove_unique_from_tagmanifesttomanifest.py | 43 +++++++++++++++++++ workers/manifestbackfillworker.py | 21 +++++++-- workers/test/test_manifestbackfillworker.py | 30 +++++++++++++ 4 files changed, 91 insertions(+), 5 deletions(-) create mode 100644 data/migrations/versions/13411de1c0ff_remove_unique_from_tagmanifesttomanifest.py diff --git a/data/database.py b/data/database.py index 866542c2b..6eb320400 100644 --- a/data/database.py +++ b/data/database.py @@ -1435,7 +1435,7 @@ class TagManifest(BaseModel): class TagManifestToManifest(BaseModel): """ NOTE: Only used for the duration of the migrations. """ tag_manifest = ForeignKeyField(TagManifest, index=True, unique=True) - manifest = ForeignKeyField(Manifest, index=True, unique=True) + manifest = ForeignKeyField(Manifest, index=True) broken = BooleanField(index=True, default=False) diff --git a/data/migrations/versions/13411de1c0ff_remove_unique_from_tagmanifesttomanifest.py b/data/migrations/versions/13411de1c0ff_remove_unique_from_tagmanifesttomanifest.py new file mode 100644 index 000000000..6aa576e31 --- /dev/null +++ b/data/migrations/versions/13411de1c0ff_remove_unique_from_tagmanifesttomanifest.py @@ -0,0 +1,43 @@ +"""Remove unique from TagManifestToManifest + +Revision ID: 13411de1c0ff +Revises: 654e6df88b71 +Create Date: 2018-08-19 23:30:24.969549 + +""" + +# revision identifiers, used by Alembic. +revision = '13411de1c0ff' +down_revision = '654e6df88b71' + +from alembic import op +import sqlalchemy as sa + +def upgrade(tables, tester): + # Note: Because of a restriction in MySQL, we cannot simply remove the index and re-add + # it without the unique=False, nor can we simply alter the index. To make it work, we'd have to + # remove the primary key on the field, so instead we simply drop the table entirely and + # recreate it with the modified index. The backfill will re-fill this in. + op.drop_table('tagmanifesttomanifest') + + op.create_table('tagmanifesttomanifest', + sa.Column('id', sa.Integer(), nullable=False), + sa.Column('tag_manifest_id', sa.Integer(), nullable=False), + sa.Column('manifest_id', sa.Integer(), nullable=False), + sa.Column('broken', sa.Boolean(), nullable=False, server_default=sa.sql.expression.false()), + sa.ForeignKeyConstraint(['manifest_id'], ['manifest.id'], name=op.f('fk_tagmanifesttomanifest_manifest_id_manifest')), + sa.ForeignKeyConstraint(['tag_manifest_id'], ['tagmanifest.id'], name=op.f('fk_tagmanifesttomanifest_tag_manifest_id_tagmanifest')), + sa.PrimaryKeyConstraint('id', name=op.f('pk_tagmanifesttomanifest')) + ) + op.create_index('tagmanifesttomanifest_broken', 'tagmanifesttomanifest', ['broken'], unique=False) + op.create_index('tagmanifesttomanifest_manifest_id', 'tagmanifesttomanifest', ['manifest_id'], unique=False) + op.create_index('tagmanifesttomanifest_tag_manifest_id', 'tagmanifesttomanifest', ['tag_manifest_id'], unique=True) + + tester.populate_table('tagmanifesttomanifest', [ + ('manifest_id', tester.TestDataType.Foreign('manifest')), + ('tag_manifest_id', tester.TestDataType.Foreign('tagmanifest')), + ]) + + +def downgrade(tables, tester): + pass diff --git a/workers/manifestbackfillworker.py b/workers/manifestbackfillworker.py index 3a6aab4c6..19139cd38 100644 --- a/workers/manifestbackfillworker.py +++ b/workers/manifestbackfillworker.py @@ -5,7 +5,7 @@ from peewee import JOIN, fn, IntegrityError from app import app from data.database import (UseThenDisconnect, TagManifest, TagManifestToManifest, Image, - db_transaction) + Manifest, db_transaction) from data.model.image import get_parent_images from data.model.tag import populate_manifest from data.model.blob import get_repo_blob_by_digest, BlobDoesNotExist @@ -158,9 +158,22 @@ def backfill_manifest(tag_manifest): except TagManifest.DoesNotExist: return True - # Create the new-style rows for the manifest. - manifest_row = populate_manifest(tag_manifest.tag.repository, manifest, - tag_manifest.tag.image, storage_ids) + # Ensure it wasn't already created. + if lookup_map_row(tag_manifest): + return False + + # Check for a pre-existing manifest matching the digest in the repository. This can happen + # if we've already created the manifest row (typically for tag reverision). + try: + manifest_row = Manifest.get(digest=manifest.digest, repository=tag_manifest.tag.repository) + except Manifest.DoesNotExist: + # Create the new-style rows for the manifest. + try: + manifest_row = populate_manifest(tag_manifest.tag.repository, manifest, + tag_manifest.tag.image, storage_ids) + except IntegrityError: + # Pre-empted. + return False # Create the mapping row. If we find another was created for this tag manifest in the # meantime, then we've been preempted. diff --git a/workers/test/test_manifestbackfillworker.py b/workers/test/test_manifestbackfillworker.py index 0a63325de..4d8a9182e 100644 --- a/workers/test/test_manifestbackfillworker.py +++ b/workers/test/test_manifestbackfillworker.py @@ -138,3 +138,33 @@ def test_manifestbackfillworker_mislinked_invalid_manifest(clear_rows, initializ manifest_blobs = list(ManifestBlob.select().where(ManifestBlob.manifest == manifest_row)) assert len(manifest_blobs) == 0 + + +def test_manifestbackfillworker_repeat_digest(clear_rows, initialized_db): + """ Tests that a manifest with a shared digest will be properly linked. """ + # Delete existing tag manifest so we can reuse the tag. + TagManifestLabel.delete().execute() + TagManifest.delete().execute() + + repo = model.repository.get_repository('devtable', 'gargantuan') + tag_v30 = model.tag.get_active_tag('devtable', 'gargantuan', 'v3.0') + tag_v50 = model.tag.get_active_tag('devtable', 'gargantuan', 'v5.0') + + # Build a manifest and assign it to both tags (this is allowed in the old model). + builder = DockerSchema1ManifestBuilder('devtable', 'gargantuan', 'sometag') + builder.add_layer('sha256:deadbeef', '{"id": "foo"}') + manifest = builder.build(docker_v2_signing_key) + + manifest_1 = TagManifest.create(json_data=manifest.bytes, digest=manifest.digest, + tag=tag_v30) + manifest_2 = TagManifest.create(json_data=manifest.bytes, digest=manifest.digest, + tag=tag_v50) + + # Backfill "both" manifests and ensure both are pointed to by a single resulting row. + assert backfill_manifest(manifest_1) + assert backfill_manifest(manifest_2) + + map_row1 = TagManifestToManifest.get(tag_manifest=manifest_1) + map_row2 = TagManifestToManifest.get(tag_manifest=manifest_2) + + assert map_row1.manifest == map_row2.manifest From ed897626a2fcac4c3956fb46a72def4317b2d5d2 Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Mon, 20 Aug 2018 00:00:16 -0400 Subject: [PATCH 5/7] Handle data model exceptions when loading parent images in manifest backfill --- workers/manifestbackfillworker.py | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/workers/manifestbackfillworker.py b/workers/manifestbackfillworker.py index 19139cd38..1d9be2992 100644 --- a/workers/manifestbackfillworker.py +++ b/workers/manifestbackfillworker.py @@ -6,6 +6,7 @@ from peewee import JOIN, fn, IntegrityError from app import app from data.database import (UseThenDisconnect, TagManifest, TagManifestToManifest, Image, Manifest, db_transaction) +from data.model import DataModelException from data.model.image import get_parent_images from data.model.tag import populate_manifest from data.model.blob import get_repo_blob_by_digest, BlobDoesNotExist @@ -129,7 +130,16 @@ def backfill_manifest(tag_manifest): repository = tag_manifest.tag.repository image_storage_id_map = {root_image.storage.content_checksum: root_image.storage.id} - parent_images = get_parent_images(repository.namespace_user.username, repository.name, root_image) + + try: + parent_images = get_parent_images(repository.namespace_user.username, repository.name, + root_image) + except DataModelException: + logger.exception('Exception when trying to load parent images for manifest `%s`', + tag_manifest.id) + parent_images = {} + is_broken = True + for parent_image in parent_images: image_storage_id_map[parent_image.storage.content_checksum] = parent_image.storage.id From 301532279cb1097441eca4fc9146aa2ca4412f99 Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Mon, 20 Aug 2018 11:36:04 -0400 Subject: [PATCH 6/7] Fix broken registry data interface tests --- data/registry_model/test/test_pre_oci_model.py | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/data/registry_model/test/test_pre_oci_model.py b/data/registry_model/test/test_pre_oci_model.py index 3d7140475..6e81c3fff 100644 --- a/data/registry_model/test/test_pre_oci_model.py +++ b/data/registry_model/test/test_pre_oci_model.py @@ -12,9 +12,10 @@ def pre_oci_model(initialized_db): @pytest.mark.parametrize('names, expected', [ (['unknown'], None), - (['latest'], 'latest'), - (['latest', 'prod'], 'latest'), - (['foo', 'prod'], 'prod'), + (['latest'], {'latest'}), + (['latest', 'prod'], {'latest', 'prod'}), + (['latest', 'prod', 'another'], {'latest', 'prod'}), + (['foo', 'prod'], {'prod'}), ]) def test_find_matching_tag(names, expected, pre_oci_model): repo = model.repository.get_repository('devtable', 'simple') @@ -23,12 +24,12 @@ def test_find_matching_tag(names, expected, pre_oci_model): if expected is None: assert found is None else: - assert found.name == expected + assert found.name in expected @pytest.mark.parametrize('repo_namespace, repo_name, expected', [ - ('devtable', 'simple', 'latest'), - ('buynlarge', 'orgrepo', 'latest'), + ('devtable', 'simple', {'latest'}), + ('buynlarge', 'orgrepo', {'latest', 'prod'}), ]) def test_get_most_recent_tag(repo_namespace, repo_name, expected, pre_oci_model): repo = model.repository.get_repository(repo_namespace, repo_name) @@ -37,4 +38,4 @@ def test_get_most_recent_tag(repo_namespace, repo_name, expected, pre_oci_model) if expected is None: assert found is None else: - assert found.name == expected + assert found.name in expected From df25ec00612eceaeed57521926835f4050eb35e0 Mon Sep 17 00:00:00 2001 From: Sam Chow Date: Mon, 20 Aug 2018 13:47:10 -0400 Subject: [PATCH 7/7] Add ability to populate config from kube cluster --- config_app/config_endpoints/api/__init__.py | 3 +- config_app/config_endpoints/api/kubeconfig.py | 69 +++++++++++++++++++ config_app/config_endpoints/api/suconfig.py | 45 +----------- config_app/config_util/k8saccessor.py | 26 +++++++ .../config-setup-app.component.ts | 11 ++- config_app/js/setup/setup.html | 8 +-- .../static/css/config-setup-app-component.css | 15 ++++ 7 files changed, 126 insertions(+), 51 deletions(-) create mode 100644 config_app/config_endpoints/api/kubeconfig.py diff --git a/config_app/config_endpoints/api/__init__.py b/config_app/config_endpoints/api/__init__.py index 21d13cc19..d9425670e 100644 --- a/config_app/config_endpoints/api/__init__.py +++ b/config_app/config_endpoints/api/__init__.py @@ -152,8 +152,9 @@ nickname = partial(add_method_metadata, 'nickname') import config_endpoints.api import config_endpoints.api.discovery +import config_endpoints.api.kubeconfig import config_endpoints.api.suconfig import config_endpoints.api.superuser -import config_endpoints.api.user import config_endpoints.api.tar_config_loader +import config_endpoints.api.user diff --git a/config_app/config_endpoints/api/kubeconfig.py b/config_app/config_endpoints/api/kubeconfig.py new file mode 100644 index 000000000..a2f9219c8 --- /dev/null +++ b/config_app/config_endpoints/api/kubeconfig.py @@ -0,0 +1,69 @@ +from flask import request + +from data.database import configure + +from config_app.c_app import app, config_provider +from config_app.config_endpoints.api import resource, ApiResource, nickname, kubernetes_only +from config_app.config_util.k8saccessor import KubernetesAccessorSingleton + + +@resource('/v1/kubernetes/deployments/') +class SuperUserKubernetesDeployment(ApiResource): + """ Resource for the getting the status of Quay Enterprise deployments and cycling them """ + schemas = { + 'ValidateDeploymentNames': { + 'type': 'object', + 'description': 'Validates deployment names for cycling', + 'required': [ + 'deploymentNames' + ], + 'properties': { + 'deploymentNames': { + 'type': 'array', + 'description': 'The names of the deployments to cycle' + }, + }, + } + } + + @kubernetes_only + @nickname('scGetNumDeployments') + def get(self): + return KubernetesAccessorSingleton.get_instance().get_qe_deployments() + + @kubernetes_only + @nickname('scCycleQEDeployments') + def put(self): + deployment_names = request.get_json()['deploymentNames'] + return KubernetesAccessorSingleton.get_instance().cycle_qe_deployments(deployment_names) + + +@resource('/v1/superuser/config/kubernetes') +class SuperUserKubernetesConfiguration(ApiResource): + """ Resource for saving the config files to kubernetes secrets. """ + + @kubernetes_only + @nickname('scDeployConfiguration') + def post(self): + return config_provider.save_configuration_to_kubernetes() + + +@resource('/v1/kubernetes/config/populate') +class KubernetesConfigurationPopulator(ApiResource): + """ Resource for populating the local configuration from the cluster's kubernetes secrets. """ + + @kubernetes_only + @nickname('scKubePopulateConfig') + def post(self): + # Get a clean transient directory to write the config into + config_provider.new_config_dir() + KubernetesAccessorSingleton.get_instance().save_secret_to_directory(config_provider.get_config_dir_path()) + + # We update the db configuration to connect to their specified one + # (Note, even if this DB isn't valid, it won't affect much in the config app, since we'll report an error, + # and all of the options create a new clean dir, so we'll never pollute configs) + combined = dict(**app.config) + combined.update(config_provider.get_config()) + configure(combined) + + return 200 diff --git a/config_app/config_endpoints/api/suconfig.py b/config_app/config_endpoints/api/suconfig.py index ce6d34539..01532c47d 100644 --- a/config_app/config_endpoints/api/suconfig.py +++ b/config_app/config_endpoints/api/suconfig.py @@ -3,11 +3,9 @@ import logging from flask import abort, request from config_app.config_endpoints.api.suconfig_models_pre_oci import pre_oci_model as model -from config_app.config_endpoints.api import resource, ApiResource, nickname, validate_json_request, \ - kubernetes_only +from config_app.config_endpoints.api import resource, ApiResource, nickname, validate_json_request from config_app.c_app import (app, config_provider, superusers, ip_resolver, instance_keys, INIT_SCRIPTS_LOCATION) -from config_app.config_util.k8saccessor import KubernetesAccessorSingleton from data.database import configure from data.runmigration import run_alembic_migration @@ -265,47 +263,6 @@ class SuperUserConfigValidate(ApiResource): return validate_service_for_config(service, validator_context) -@resource('/v1/kubernetes/deployments/') -class SuperUserKubernetesDeployment(ApiResource): - """ Resource for the getting the status of Quay Enterprise deployments and cycling them """ - schemas = { - 'ValidateDeploymentNames': { - 'type': 'object', - 'description': 'Validates deployment names for cycling', - 'required': [ - 'deploymentNames' - ], - 'properties': { - 'deploymentNames': { - 'type': 'array', - 'description': 'The names of the deployments to cycle' - }, - }, - } - } - - @kubernetes_only - @nickname('scGetNumDeployments') - def get(self): - return KubernetesAccessorSingleton.get_instance().get_qe_deployments() - - @kubernetes_only - @nickname('scCycleQEDeployments') - def put(self): - deployment_names = request.get_json()['deploymentNames'] - return KubernetesAccessorSingleton.get_instance().cycle_qe_deployments(deployment_names) - - -@resource('/v1/superuser/config/kubernetes') -class SuperUserKubernetesConfiguration(ApiResource): - """ Resource for saving the config files to kubernetes secrets. """ - - @kubernetes_only - @nickname('scDeployConfiguration') - def post(self): - return config_provider.save_configuration_to_kubernetes() - - @resource('/v1/superuser/config/file/') class SuperUserConfigFile(ApiResource): """ Resource for fetching the status of config files and overriding them. """ diff --git a/config_app/config_util/k8saccessor.py b/config_app/config_util/k8saccessor.py index d498ede4d..dd7f596e2 100644 --- a/config_app/config_util/k8saccessor.py +++ b/config_app/config_util/k8saccessor.py @@ -2,8 +2,10 @@ import logging import json import base64 import datetime +import os from requests import Request, Session +from util.config.validator import EXTRA_CA_DIRECTORY, EXTRA_CA_DIRECTORY_PREFIX from config_app.config_util.k8sconfig import KubernetesConfig @@ -35,6 +37,30 @@ class KubernetesAccessorSingleton(object): return cls._instance + def save_secret_to_directory(self, dir_path): + """ + Saves all files in the kubernetes secret to a local directory. + Assumes the directory is empty. + """ + secret = self._lookup_secret() + + secret_data = secret.get('data', {}) + + # Make the `extra_ca_certs` dir to ensure we can populate extra certs + extra_ca_dir_path = os.path.join(dir_path, EXTRA_CA_DIRECTORY) + os.mkdir(extra_ca_dir_path) + + for secret_filename, data in secret_data.iteritems(): + write_path = os.path.join(dir_path, secret_filename) + + if EXTRA_CA_DIRECTORY_PREFIX in secret_filename: + write_path = os.path.join(extra_ca_dir_path, secret_filename.replace(EXTRA_CA_DIRECTORY_PREFIX, '')) + + with open(write_path, 'w') as f: + f.write(base64.b64decode(data)) + + return 200 + def save_file_as_secret(self, name, file_pointer): value = file_pointer.read() self._update_secret_file(name, value) diff --git a/config_app/js/components/config-setup-app/config-setup-app.component.ts b/config_app/js/components/config-setup-app/config-setup-app.component.ts index 16a796ed4..3b05c304c 100644 --- a/config_app/js/components/config-setup-app/config-setup-app.component.ts +++ b/config_app/js/components/config-setup-app/config-setup-app.component.ts @@ -15,7 +15,6 @@ export class ConfigSetupAppComponent { : 'choice' | 'setup' | 'load' - | 'populate' | 'download' | 'deploy'; @@ -45,7 +44,15 @@ export class ConfigSetupAppComponent { } private choosePopulate(): void { - this.state = 'populate'; + this.apiService.scKubePopulateConfig() + .then(() => { + this.state = 'setup'; + }) + .catch(err => { + this.apiService.errorDisplay( + `Could not populate the configuration from your cluster. Please report this error: ${JSON.stringify(err)}` + )() + }) } private configLoaded(): void { diff --git a/config_app/js/setup/setup.html b/config_app/js/setup/setup.html index 04f959770..5548e52f5 100644 --- a/config_app/js/setup/setup.html +++ b/config_app/js/setup/setup.html @@ -209,19 +209,19 @@ -
{{ errors.SuperuserCreationError }}
-
- Database Setup Failed. Please report this to support: {{ errors.DatabaseSetupError }} + Database Setup Failed: {{ errors.DatabaseSetupError }}
-
+
Database Validation Issue: {{ errors.DatabaseValidationError }}
diff --git a/config_app/static/css/config-setup-app-component.css b/config_app/static/css/config-setup-app-component.css index e7f62883b..3c3acfab0 100644 --- a/config_app/static/css/config-setup-app-component.css +++ b/config_app/static/css/config-setup-app-component.css @@ -28,6 +28,21 @@ text-decoration: none; } +.quay-config-app .alert-danger { + padding: 25px; + display: flex; +} + +.quay-config-app .alert-danger:before { + content: "\f071"; + font-family: Font Awesome\ 5 Free; + font-weight: 900; + font-size: 30px; + padding-right: 15px; + color: #c53c3f; + text-align: center; +} + /* Overrides for fixing old quay styles*/