From 100aa7ba5377c22340455a4605238ae8d0b705b4 Mon Sep 17 00:00:00 2001 From: Jonathan Sick Date: Mon, 21 Mar 2022 18:19:15 -0400 Subject: [PATCH 01/17] Install ping for network debugging --- bin/install-base-packages.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bin/install-base-packages.sh b/bin/install-base-packages.sh index 9713eb1e..58599017 100755 --- a/bin/install-base-packages.sh +++ b/bin/install-base-packages.sh @@ -29,7 +29,7 @@ apt-get -y upgrade # Install system packages # - build-essentiall needed for uwsgi # - git needed for setuptools_scm -apt-get -y install --no-install-recommends git build-essential redis-server dnsutils wget +apt-get -y install --no-install-recommends git build-essential redis-server dnsutils wget iputils-ping # Delete cached files we don't need anymore: apt-get clean From c2cf53ad58326843da47e9e37c152d481e1d315e Mon Sep 17 00:00:00 2001 From: Jonathan Sick Date: Sun, 20 Mar 2022 12:50:41 -0400 Subject: [PATCH 02/17] Make public-read ACL configurable per org This allows orgs to set whether objects should be public-readable from their bucket. Right now this change is hard-coded via a property on the Organization model, but will ultimately become a database column. --- keeper/models.py | 5 +++++ keeper/s3.py | 39 ++++++++++++++++++++-------------- keeper/services/createbuild.py | 17 +++++++++++---- keeper/tasks/editionrebuild.py | 2 ++ keeper/tasks/renameedition.py | 2 ++ 5 files changed, 45 insertions(+), 20 deletions(-) diff --git a/keeper/models.py b/keeper/models.py index 2d45ab93..3734eaf4 100644 --- a/keeper/models.py +++ b/keeper/models.py @@ -358,6 +358,11 @@ class Organization(db.Model): # type: ignore back_populates="organization", ) + # FIXME convert this into a database column + @property + def bucket_public_read(self) -> bool: + return False + def set_fastly_api_key(self, api_key: Optional[SecretStr]) -> None: """Encrypt and set the Fastly API key.""" if api_key is None: diff --git a/keeper/s3.py b/keeper/s3.py index b8c1054e..e297b627 100644 --- a/keeper/s3.py +++ b/keeper/s3.py @@ -129,6 +129,7 @@ def copy_directory( cache_control: Optional[str] = None, surrogate_control: Optional[str] = None, create_directory_redirect_object: bool = True, + use_public_read_acl: bool = False, ) -> None: """Copy objects from one directory in a bucket to another directory in the same bucket. @@ -178,6 +179,8 @@ def copy_directory( ``x-amz-meta-dir-redirect=true`` HTTP header. LSST the Docs' Fastly VCL is configured to redirect requests for a directory path to the directory's ``index.html`` (known as *courtesy redirects*). + use_public_read_acl : bool + If True, apply the ``public-read`` ACL to bucket objects. Raises ------ @@ -226,27 +229,31 @@ def copy_directory( if surrogate_key is not None: metadata["surrogate-key"] = surrogate_key - s3.meta.client.copy_object( - Bucket=bucket_name, - Key=dest_key_path, - CopySource={"Bucket": bucket_name, "Key": src_obj.key}, - MetadataDirective="REPLACE", - Metadata=metadata, - ACL="public-read", - CacheControl=cache_control, - ContentType=content_type, - ) + copy_kwargs = { + "Bucket": bucket_name, + "Key": dest_key_path, + "CopySource": {"Bucket": bucket_name, "Key": src_obj.key}, + "MetadataDirective": "REPLACE", + "Metadata": metadata, + "CacheControl": cache_control, + "ContentType": content_type, + } + if use_public_read_acl: + copy_kwargs["ACL"] = "public-read" + s3.meta.client.copy_object(**copy_kwargs) if create_directory_redirect_object: dest_dirname = dest_path.rstrip("/") obj = bucket.Object(dest_dirname) metadata = {"dir-redirect": "true"} - obj.put( - Body="", - ACL="public-read", - Metadata=metadata, - CacheControl=cache_control, - ) + put_kwargs = { + "Body": "", + "Metadata": metadata, + "CacheControl": cache_control, + } + if use_public_read_acl: + put_kwargs["ACL"] = "public-read" + obj.put(**put_kwargs) def presign_post_url_for_prefix( diff --git a/keeper/services/createbuild.py b/keeper/services/createbuild.py index 6beb83f5..0ecca9bc 100644 --- a/keeper/services/createbuild.py +++ b/keeper/services/createbuild.py @@ -176,6 +176,7 @@ def create_presigned_post_urls( organization = build.product.organization aws_id = organization.aws_id aws_secret = organization.get_aws_secret_key() + use_public_read_acl = organization.bucket_public_read if aws_secret is None: s3_session = open_s3_session(key_id=aws_id, access_key="") @@ -194,6 +195,7 @@ def create_presigned_post_urls( bucket_name=build.product.bucket_name, prefix=bucket_prefix, surrogate_key=build.surrogate_key, + use_public_read_acl=use_public_read_acl, ) presigned_prefix_urls[d] = deepcopy(presigned_prefix_url) @@ -202,6 +204,7 @@ def create_presigned_post_urls( bucket_name=build.product.bucket_name, key=dir_key, surrogate_key=build.surrogate_key, + use_public_read_acl=use_public_read_acl, ) presigned_dir_urls[d] = deepcopy(presigned_dir_url) @@ -222,10 +225,10 @@ def _create_presigned_url_for_prefix( bucket_name: str, prefix: str, surrogate_key: str, + use_public_read_acl: bool, ) -> Dict[str, Any]: # These conditions become part of the URL's presigned policy url_conditions = [ - {"acl": "public-read"}, {"Cache-Control": "max-age=31536000"}, # Make sure the surrogate-key is always consistent {"x-amz-meta-surrogate-key": surrogate_key}, @@ -235,13 +238,16 @@ def _create_presigned_url_for_prefix( # is returned by S3. This is what we want. {"success_action_status": "204"}, ] + if use_public_read_acl: + url_conditions.append({"acl": "public-read"}) # These fields are pre-populated for clients url_fields = { - "acl": "public-read", "Cache-Control": "max-age=31536000", "x-amz-meta-surrogate-key": surrogate_key, "success_action_status": "204", } + if use_public_read_acl: + url_fields["acl"] = "public-read" return presign_post_url_for_prefix( s3_session=s3_session, bucket_name=bucket_name, @@ -258,10 +264,10 @@ def _create_presigned_url_for_directory( bucket_name: str, key: str, surrogate_key: str, + use_public_read_acl: bool, ) -> Dict[str, Any]: # These conditions become part of the URL's presigned policy url_conditions = [ - {"acl": "public-read"}, {"Cache-Control": "max-age=31536000"}, # Make sure the surrogate-key is always consistent {"x-amz-meta-surrogate-key": surrogate_key}, @@ -269,12 +275,15 @@ def _create_presigned_url_for_directory( # is returned by S3. This is what we want. {"success_action_status": "204"}, ] + if use_public_read_acl: + url_conditions.append({"acl": "public-read"}) url_fields = { - "acl": "public-read", "Cache-Control": "max-age=31536000", "x-amz-meta-surrogate-key": surrogate_key, "success_action_status": "204", } + if use_public_read_acl: + url_fields["acl"] = "public-read" return presign_post_url_for_directory_object( s3_session=s3_session, bucket_name=bucket_name, diff --git a/keeper/tasks/editionrebuild.py b/keeper/tasks/editionrebuild.py index 77007aca..6c7f145b 100644 --- a/keeper/tasks/editionrebuild.py +++ b/keeper/tasks/editionrebuild.py @@ -62,6 +62,7 @@ def rebuild_edition( aws_id = organization.aws_id aws_secret = organization.get_aws_secret_key + use_public_read_acl = organization.bucket_public_read fastly_service_id = organization.fastly_service_id fastly_key = organization.get_fastly_api_id @@ -77,6 +78,7 @@ def rebuild_edition( dest_path=edition.bucket_root_dirname, aws_access_key_id=aws_id, aws_secret_access_key=aws_secret.get_secret_value(), + use_public_read_acl=use_public_read_acl, surrogate_key=edition.surrogate_key, # Force Fastly to cache the edition for 1 year surrogate_control="max-age=31536000", diff --git a/keeper/tasks/renameedition.py b/keeper/tasks/renameedition.py index ef35118f..01dc0eab 100644 --- a/keeper/tasks/renameedition.py +++ b/keeper/tasks/renameedition.py @@ -42,6 +42,7 @@ def rename_edition( organization = edition.product.organization aws_id = organization.aws_id aws_secret = organization.get_aws_secret_key() + use_public_read_acl = organization.bucket_public_read if ( aws_id is not None and aws_secret is not None @@ -54,6 +55,7 @@ def rename_edition( aws_id, aws_secret.get_secret_value(), surrogate_key=self.surrogate_key, + use_public_read_acl=use_public_read_acl, ) s3.delete_directory( self.product.bucket_name, From fd4cdb3b9d2ebf5ee7431246e2a50f64bd9e8bf3 Mon Sep 17 00:00:00 2001 From: Jonathan Sick Date: Sun, 20 Mar 2022 15:38:04 -0400 Subject: [PATCH 03/17] Use signature v4 algorithm for S3 The v4 signature algorithm is required for new AWS regions, and v2 is being deprecated actively from older regions. --- keeper/s3.py | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/keeper/s3.py b/keeper/s3.py index e297b627..d87a557a 100644 --- a/keeper/s3.py +++ b/keeper/s3.py @@ -21,6 +21,7 @@ import boto3 import botocore.exceptions +from botocore.client import Config from keeper.exceptions import S3Error @@ -82,7 +83,7 @@ def delete_directory( aws_access_key_id=aws_access_key_id, aws_secret_access_key=aws_secret_access_key, ) - s3 = session.resource("s3") + s3 = session.resource("s3", config=Config(signature_version="s3v4")) client = s3.meta.client # Normalize directory path for searching patch prefixes of objects @@ -206,7 +207,7 @@ def copy_directory( aws_access_key_id=aws_access_key_id, aws_secret_access_key=aws_secret_access_key, ) - s3 = session.resource("s3") + s3 = session.resource("s3", config=Config(signature_version="s3v4")) bucket = s3.Bucket(bucket_name) # Copy each object from source to destination @@ -326,7 +327,9 @@ def presign_post_url_for_prefix( else: key = f"{prefix}/${{filename}}" - s3_client = s3_session.client("s3") + s3_client = s3_session.client( + "s3", config=Config(signature_version="s3v4") + ) try: response = s3_client.generate_presigned_post( bucket_name, @@ -416,7 +419,9 @@ def presign_post_url_for_directory_object( condition={"x-amz-meta-dir-redirect": "true"}, ) - s3_client = s3_session.client("s3") + s3_client = s3_session.client( + "s3", config=Config(signature_version="s3v4") + ) try: response = s3_client.generate_presigned_post( bucket_name, From 93ad316b94f144d00cc161613355a0c47bb07dfc Mon Sep 17 00:00:00 2001 From: Jonathan Sick Date: Sun, 20 Mar 2022 16:22:49 -0400 Subject: [PATCH 04/17] Fix getting aws secret in rebuild_edition --- keeper/tasks/editionrebuild.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/keeper/tasks/editionrebuild.py b/keeper/tasks/editionrebuild.py index 6c7f145b..0f860a7d 100644 --- a/keeper/tasks/editionrebuild.py +++ b/keeper/tasks/editionrebuild.py @@ -61,7 +61,7 @@ def rebuild_edition( new_build = Build.query.get(build_id) aws_id = organization.aws_id - aws_secret = organization.get_aws_secret_key + aws_secret = organization.get_aws_secret_key() use_public_read_acl = organization.bucket_public_read fastly_service_id = organization.fastly_service_id From 8b84dd9e24c047dc544d18c5a072654efb28fb72 Mon Sep 17 00:00:00 2001 From: Jonathan Sick Date: Sun, 20 Mar 2022 21:39:09 -0400 Subject: [PATCH 05/17] Add aws_region property to model Also refactor code for getting the AWS session to embed the region into the authentication, while also configuring the S3 client to use v4 signature verification. --- keeper/models.py | 5 + keeper/s3.py | 92 ++++++++-------- keeper/services/createbuild.py | 26 ++--- keeper/tasks/editionrebuild.py | 9 +- keeper/tasks/renameedition.py | 22 ++-- tests/test_builds_v2.py | 6 +- tests/test_s3.py | 138 +++++++++++++++--------- tests/v2api/test_project.py | 6 +- tests/v2api/test_project_path_layout.py | 6 +- tox.ini | 1 + 10 files changed, 183 insertions(+), 128 deletions(-) diff --git a/keeper/models.py b/keeper/models.py index 3734eaf4..cdbe6cbe 100644 --- a/keeper/models.py +++ b/keeper/models.py @@ -358,6 +358,11 @@ class Organization(db.Model): # type: ignore back_populates="organization", ) + # FIXME convert this into a database column + @property + def aws_region(self) -> str: + return "ca-central-1" + # FIXME convert this into a database column @property def bucket_public_read(self) -> bool: diff --git a/keeper/s3.py b/keeper/s3.py index d87a557a..56715160 100644 --- a/keeper/s3.py +++ b/keeper/s3.py @@ -29,6 +29,8 @@ import botocore.client.S3 __all__ = ( + "open_aws_session", + "open_s3_resource", "delete_directory", "copy_directory", "presign_post_url_for_prefix", @@ -37,8 +39,10 @@ ) -def open_s3_session(*, key_id: str, access_key: str) -> boto3.session.Session: - """Create a boto3 S3 session that can be reused by multiple requests. +def open_aws_session( + *, key_id: str, access_key: str, aws_region: str +) -> boto3.session.Session: + """Create a boto3 AWS session that can be reused by multiple requests. Parameters ---------- @@ -46,31 +50,43 @@ def open_s3_session(*, key_id: str, access_key: str) -> boto3.session.Session: The access key for your AWS account. Also set `aws_secret_access_key`. aws_secret_access_key : str The secret key for your AWS account. + aws_region : str + The AWS region (``us-east-1`` or ``ca-central-1``). """ return boto3.session.Session( - aws_access_key_id=key_id, aws_secret_access_key=access_key + aws_access_key_id=key_id, + aws_secret_access_key=access_key, + region_name=aws_region, ) +def open_s3_resource( + *, key_id: str, access_key: str, aws_region: str +) -> boto3.resources.base.ServiceResource: + session = open_aws_session( + key_id=key_id, access_key=access_key, aws_region=aws_region + ) + s3 = session.resource("s3", config=Config(signature_version="s3v4")) + return s3 + + def delete_directory( + *, + s3: boto3.resources.base.ServiceResource, bucket_name: str, root_path: str, - aws_access_key_id: str, - aws_secret_access_key: str, ) -> None: """Delete all objects in the S3 bucket named `bucket_name` that are found in the `root_path` directory. Parameters ---------- + s3 + The S3 service. bucket_name : str Name of an S3 bucket. root_path : str Directory in the S3 bucket that will be deleted. - aws_access_key_id : str - The access key for your AWS account. Also set `aws_secret_access_key`. - aws_secret_access_key : str - The secret key for your AWS account. Raises ------ @@ -79,18 +95,13 @@ def delete_directory( """ log = logging.getLogger(__name__) - session = boto3.session.Session( - aws_access_key_id=aws_access_key_id, - aws_secret_access_key=aws_secret_access_key, - ) - s3 = session.resource("s3", config=Config(signature_version="s3v4")) - client = s3.meta.client + s3_client = s3.meta.client # Normalize directory path for searching patch prefixes of objects if not root_path.endswith("/"): root_path.rstrip("/") - paginator = client.get_paginator("list_objects_v2") + paginator = s3_client.get_paginator("list_objects_v2") pages = paginator.paginate(Bucket=bucket_name, Prefix=root_path) keys: Dict[str, List[Dict[str, str]]] = dict(Objects=[]) @@ -103,7 +114,7 @@ def delete_directory( # the delete_objects method can only take a maximum of 1000 keys if len(keys["Objects"]) >= 1000: try: - client.delete_objects(Bucket=bucket_name, Delete=keys) + s3_client.delete_objects(Bucket=bucket_name, Delete=keys) except Exception: message = "Error deleting objects from %r" % root_path log.exception("Error deleting objects from %r", root_path) @@ -113,7 +124,7 @@ def delete_directory( # Delete remaining keys if len(keys["Objects"]) > 0: try: - client.delete_objects(Bucket=bucket_name, Delete=keys) + s3_client.delete_objects(Bucket=bucket_name, Delete=keys) except Exception: message = "Error deleting objects from %r" % root_path log.exception(message) @@ -121,11 +132,11 @@ def delete_directory( def copy_directory( + *, + s3: boto3.resources.base.ServiceResource, bucket_name: str, src_path: str, dest_path: str, - aws_access_key_id: str, - aws_secret_access_key: str, surrogate_key: Optional[str] = None, cache_control: Optional[str] = None, surrogate_control: Optional[str] = None, @@ -143,6 +154,8 @@ def copy_directory( Parameters ---------- + s3 + The S3 resource. bucket_name : str Name of an S3 bucket. src_path : str @@ -152,10 +165,6 @@ def copy_directory( Destination directory in the S3 bucket. The `dest_path` should ideally end in a trailing `'/'`. E.g. `'dir/dir2/'`. The destination path cannot contain the source path. - aws_access_key_id : str - The access key for your AWS account. Also set `aws_secret_access_key`. - aws_secret_access_key : str - The secret key for your AWS account. surrogate_key : str, optional The surrogate key to insert in the header of all objects in the ``x-amz-meta-surrogate-key`` field. This key is used to purge @@ -188,6 +197,8 @@ def copy_directory( app.exceptions.S3Error Thrown by any unexpected faults from the S3 API. """ + s3_client = s3.meta.client + if not src_path.endswith("/"): src_path += "/" if not dest_path.endswith("/"): @@ -199,15 +210,8 @@ def copy_directory( assert common_prefix != dest_path # Delete any existing objects in the destination - delete_directory( - bucket_name, dest_path, aws_access_key_id, aws_secret_access_key - ) + delete_directory(s3=s3, bucket_name=bucket_name, root_path=dest_path) - session = boto3.session.Session( - aws_access_key_id=aws_access_key_id, - aws_secret_access_key=aws_secret_access_key, - ) - s3 = session.resource("s3", config=Config(signature_version="s3v4")) bucket = s3.Bucket(bucket_name) # Copy each object from source to destination @@ -216,7 +220,7 @@ def copy_directory( dest_key_path = os.path.join(dest_path, src_rel_path) # the src_obj (ObjectSummary) doesn't include headers afaik - head = s3.meta.client.head_object(Bucket=bucket_name, Key=src_obj.key) + head = s3_client.head_object(Bucket=bucket_name, Key=src_obj.key) metadata = head["Metadata"] content_type = head["ContentType"] @@ -241,7 +245,7 @@ def copy_directory( } if use_public_read_acl: copy_kwargs["ACL"] = "public-read" - s3.meta.client.copy_object(**copy_kwargs) + s3_client.copy_object(**copy_kwargs) if create_directory_redirect_object: dest_dirname = dest_path.rstrip("/") @@ -259,7 +263,7 @@ def copy_directory( def presign_post_url_for_prefix( *, - s3_session: botocore.client.S3, + s3: boto3.resources.base.ServiceResource, bucket_name: str, prefix: str, fields: Optional[Dict[str, str]] = None, @@ -271,8 +275,8 @@ def presign_post_url_for_prefix( Parameters ---------- - s3_session - S3 session, typically created with `open_s3_session`. + s3 + S3 service. bucket_name : `str` Name of the S3 bucket. prefix : `str` @@ -327,9 +331,7 @@ def presign_post_url_for_prefix( else: key = f"{prefix}/${{filename}}" - s3_client = s3_session.client( - "s3", config=Config(signature_version="s3v4") - ) + s3_client = s3.meta.client try: response = s3_client.generate_presigned_post( bucket_name, @@ -347,7 +349,7 @@ def presign_post_url_for_prefix( def presign_post_url_for_directory_object( *, - s3_session: botocore.client.S3, + s3: boto3.resources.base.ServiceResource, bucket_name: str, key: str, fields: Optional[Dict[str, str]] = None, @@ -364,8 +366,8 @@ def presign_post_url_for_directory_object( Parameters ---------- - s3_session - S3 session, typically created with `open_s3_session`. + s3 + The S3 service. bucket_name : `str` Name of the S3 bucket. key : `str` @@ -419,9 +421,7 @@ def presign_post_url_for_directory_object( condition={"x-amz-meta-dir-redirect": "true"}, ) - s3_client = s3_session.client( - "s3", config=Config(signature_version="s3v4") - ) + s3_client = s3.meta.client try: response = s3_client.generate_presigned_post( bucket_name, diff --git a/keeper/services/createbuild.py b/keeper/services/createbuild.py index 0ecca9bc..1f1bb251 100644 --- a/keeper/services/createbuild.py +++ b/keeper/services/createbuild.py @@ -11,7 +11,7 @@ from keeper.models import Build, Edition, Permission, db from keeper.s3 import ( format_bucket_prefix, - open_s3_session, + open_s3_resource, presign_post_url_for_directory_object, presign_post_url_for_prefix, ) @@ -176,14 +176,14 @@ def create_presigned_post_urls( organization = build.product.organization aws_id = organization.aws_id aws_secret = organization.get_aws_secret_key() + aws_region = organization.aws_region use_public_read_acl = organization.bucket_public_read - if aws_secret is None: - s3_session = open_s3_session(key_id=aws_id, access_key="") - else: - s3_session = open_s3_session( - key_id=aws_id, access_key=aws_secret.get_secret_value() - ) + s3_service = open_s3_resource( + key_id=aws_id, + access_key=aws_secret.get_secret_value() if aws_secret else "", + aws_region=aws_region, + ) presigned_prefix_urls = {} presigned_dir_urls = {} for d in set(directories): @@ -191,7 +191,7 @@ def create_presigned_post_urls( dir_key = bucket_prefix.rstrip("/") presigned_prefix_url = _create_presigned_url_for_prefix( - s3_session=s3_session, + s3=s3_service, bucket_name=build.product.bucket_name, prefix=bucket_prefix, surrogate_key=build.surrogate_key, @@ -200,7 +200,7 @@ def create_presigned_post_urls( presigned_prefix_urls[d] = deepcopy(presigned_prefix_url) presigned_dir_url = _create_presigned_url_for_directory( - s3_session=s3_session, + s3=s3_service, bucket_name=build.product.bucket_name, key=dir_key, surrogate_key=build.surrogate_key, @@ -221,7 +221,7 @@ def create_presigned_post_urls( def _create_presigned_url_for_prefix( *, - s3_session: boto3.session.Session, + s3: boto3.resources.base.ServiceResource, bucket_name: str, prefix: str, surrogate_key: str, @@ -249,7 +249,7 @@ def _create_presigned_url_for_prefix( if use_public_read_acl: url_fields["acl"] = "public-read" return presign_post_url_for_prefix( - s3_session=s3_session, + s3=s3, bucket_name=bucket_name, prefix=prefix, expiration=3600, @@ -260,7 +260,7 @@ def _create_presigned_url_for_prefix( def _create_presigned_url_for_directory( *, - s3_session: boto3.session.Session, + s3: boto3.resources.base.ServiceResource, bucket_name: str, key: str, surrogate_key: str, @@ -285,7 +285,7 @@ def _create_presigned_url_for_directory( if use_public_read_acl: url_fields["acl"] = "public-read" return presign_post_url_for_directory_object( - s3_session=s3_session, + s3=s3, bucket_name=bucket_name, key=key, fields=url_fields, diff --git a/keeper/tasks/editionrebuild.py b/keeper/tasks/editionrebuild.py index 0f860a7d..a48fb370 100644 --- a/keeper/tasks/editionrebuild.py +++ b/keeper/tasks/editionrebuild.py @@ -62,6 +62,7 @@ def rebuild_edition( aws_id = organization.aws_id aws_secret = organization.get_aws_secret_key() + aws_region = organization.aws_region use_public_read_acl = organization.bucket_public_read fastly_service_id = organization.fastly_service_id @@ -72,12 +73,16 @@ def rebuild_edition( if aws_id is not None and aws_secret is not None: logger.info("Starting copy_directory") + s3_service = s3.open_s3_resource( + key_id=aws_id, + access_key=aws_secret.get_secret_value(), + aws_region=aws_region, + ) s3.copy_directory( + s3=s3_service, bucket_name=edition.product.bucket_name, src_path=new_build.bucket_root_dirname, dest_path=edition.bucket_root_dirname, - aws_access_key_id=aws_id, - aws_secret_access_key=aws_secret.get_secret_value(), use_public_read_acl=use_public_read_acl, surrogate_key=edition.surrogate_key, # Force Fastly to cache the edition for 1 year diff --git a/keeper/tasks/renameedition.py b/keeper/tasks/renameedition.py index 01dc0eab..dcd47a2b 100644 --- a/keeper/tasks/renameedition.py +++ b/keeper/tasks/renameedition.py @@ -42,26 +42,30 @@ def rename_edition( organization = edition.product.organization aws_id = organization.aws_id aws_secret = organization.get_aws_secret_key() + aws_region = organization.aws_region use_public_read_acl = organization.bucket_public_read if ( aws_id is not None and aws_secret is not None and self.build is not None ): + s3_service = s3.open_s3_resource( + key_id=aws_id, + access_key=aws_secret.get_secret_value(), + aws_region=aws_region, + ) s3.copy_directory( - self.product.bucket_name, - old_bucket_root_dir, - new_bucket_root_dir, - aws_id, - aws_secret.get_secret_value(), + s3=s3_service, + bucket_name=self.product.bucket_name, + src_path=old_bucket_root_dir, + dest_path=new_bucket_root_dir, surrogate_key=self.surrogate_key, use_public_read_acl=use_public_read_acl, ) s3.delete_directory( - self.product.bucket_name, - old_bucket_root_dir, - aws_id, - aws_secret.get_secret_value(), + s3=s3_service, + bucket_name=self.product.bucket_name, + root_path=old_bucket_root_dir, ) edition.pending_rebuild = False diff --git a/tests/test_builds_v2.py b/tests/test_builds_v2.py index fbfeaa35..459a3778 100644 --- a/tests/test_builds_v2.py +++ b/tests/test_builds_v2.py @@ -37,8 +37,8 @@ def test_builds_v2(client: TestClient, mocker: Mock) -> None: "keeper.services.createbuild.presign_post_url_for_directory_object", new=MagicMock(return_value=mock_presigned_url), ) - s3_session_mock = mocker.patch( - "keeper.services.createbuild.open_s3_session" + s3_resource_mock = mocker.patch( + "keeper.services.createbuild.open_s3_resource" ) # Create default organization @@ -103,7 +103,7 @@ def test_builds_v2(client: TestClient, mocker: Mock) -> None: "/products/pipelines/builds/", b1, headers={"Accept": v2_json_type} ) task_queue.apply_task_side_effects() - s3_session_mock.assert_called_once() + s3_resource_mock.assert_called_once() presign_post_mock.assert_called_once() assert r.status == 201 assert r.json["product_url"] == product_url diff --git a/tests/test_s3.py b/tests/test_s3.py index baba9e22..4a18ca5c 100644 --- a/tests/test_s3.py +++ b/tests/test_s3.py @@ -24,22 +24,21 @@ import tempfile import uuid from typing import TYPE_CHECKING, Any, List +from unittest.mock import Mock, PropertyMock -import boto3 import pytest from keeper.s3 import ( copy_directory, delete_directory, format_bucket_prefix, + open_s3_resource, presign_post_url_for_directory_object, presign_post_url_for_prefix, set_condition, ) if TYPE_CHECKING: - from unittest.mock import Mock - from _pytest.fixtures import FixtureRequest @@ -52,22 +51,21 @@ "LTD_KEEPER_TEST_BUCKET", ) def test_delete_directory(request: FixtureRequest) -> None: - session = boto3.session.Session( - aws_access_key_id=os.getenv("LTD_KEEPER_TEST_AWS_ID"), - aws_secret_access_key=os.getenv("LTD_KEEPER_TEST_AWS_SECRET"), + s3_service = open_s3_resource( + key_id=os.getenv("LTD_KEEPER_TEST_AWS_ID", ""), + access_key=os.getenv("LTD_KEEPER_TEST_AWS_SECRET", ""), + aws_region=os.getenv("LTD_KEEPER_TEST_AWS_REGION", "us-east-1"), ) - s3 = session.resource("s3") - bucket = s3.Bucket(os.getenv("LTD_KEEPER_TEST_BUCKET")) + bucket = s3_service.Bucket(os.getenv("LTD_KEEPER_TEST_BUCKET", "")) bucket_root = str(uuid.uuid4()) + "/" def cleanup() -> None: print("Cleaning up the bucket") delete_directory( - os.getenv("LTD_KEEPER_TEST_BUCKET", ""), - bucket_root, - os.getenv("LTD_KEEPER_TEST_AWS_ID", ""), - os.getenv("LTD_KEEPER_TEST_AWS_SECRET", ""), + s3=s3_service, + bucket_name=os.getenv("LTD_KEEPER_TEST_BUCKET", ""), + root_path=bucket_root, ) request.addfinalizer(cleanup) @@ -84,10 +82,9 @@ def cleanup() -> None: # Delete b/* delete_directory( - os.getenv("LTD_KEEPER_TEST_BUCKET", ""), - bucket_root + "a/b/", - os.getenv("LTD_KEEPER_TEST_AWS_ID", ""), - os.getenv("LTD_KEEPER_TEST_AWS_SECRET", ""), + s3=s3_service, + bucket_name=os.getenv("LTD_KEEPER_TEST_BUCKET", ""), + root_path=bucket_root + "a/b/", ) # Ensure paths outside of that are still available, but paths in b/ are @@ -107,10 +104,9 @@ def cleanup() -> None: # Attempt to delete an empty prefix. Ensure it does not raise an exception. delete_directory( - os.getenv("LTD_KEEPER_TEST_BUCKET", ""), - bucket_root + "empty-prefix/", - os.getenv("LTD_KEEPER_TEST_AWS_ID", ""), - os.getenv("LTD_KEEPER_TEST_AWS_SECRET", ""), + s3=s3_service, + bucket_name=os.getenv("LTD_KEEPER_TEST_BUCKET", ""), + root_path=bucket_root + "empty-prefix/", ) @@ -123,22 +119,21 @@ def cleanup() -> None: "LTD_KEEPER_TEST_BUCKET", ) def test_copy_directory(request: FixtureRequest) -> None: - session = boto3.session.Session( - aws_access_key_id=os.getenv("LTD_KEEPER_TEST_AWS_ID"), - aws_secret_access_key=os.getenv("LTD_KEEPER_TEST_AWS_SECRET"), + s3_service = open_s3_resource( + key_id=os.getenv("LTD_KEEPER_TEST_AWS_ID", ""), + access_key=os.getenv("LTD_KEEPER_TEST_AWS_SECRET", ""), + aws_region=os.getenv("LTD_KEEPER_TEST_AWS_REGION", "us-east-1"), ) - s3 = session.resource("s3") - bucket = s3.Bucket(os.getenv("LTD_KEEPER_TEST_BUCKET")) + bucket = s3_service.Bucket(os.getenv("LTD_KEEPER_TEST_BUCKET", "")) bucket_root = str(uuid.uuid4()) + "/" def cleanup() -> None: print("Cleaning up the bucket") delete_directory( - os.getenv("LTD_KEEPER_TEST_BUCKET", ""), - bucket_root, - os.getenv("LTD_KEEPER_TEST_AWS_ID", ""), - os.getenv("LTD_KEEPER_TEST_AWS_SECRET", ""), + s3=s3_service, + bucket_name=os.getenv("LTD_KEEPER_TEST_BUCKET", ""), + root_path=bucket_root, ) request.addfinalizer(cleanup) @@ -166,11 +161,10 @@ def cleanup() -> None: # copy files copy_directory( + s3=s3_service, bucket_name=os.getenv("LTD_KEEPER_TEST_BUCKET", ""), src_path=bucket_root + "b/", dest_path=bucket_root + "a/", - aws_access_key_id=os.getenv("LTD_KEEPER_TEST_AWS_ID", ""), - aws_secret_access_key=os.getenv("LTD_KEEPER_TEST_AWS_SECRET", ""), surrogate_key="new-key", surrogate_control="max-age=31536000", cache_control="no-cache", @@ -181,7 +175,7 @@ def cleanup() -> None: bucket_path = os.path.relpath(obj.key, start=bucket_root + "a/") assert bucket_path in new_paths # ensure correct metadata - head = s3.meta.client.head_object( + head = s3_service.meta.client.head_object( Bucket=os.getenv("LTD_KEEPER_TEST_BUCKET"), Key=obj.key ) assert head["CacheControl"] == "no-cache" @@ -196,20 +190,56 @@ def cleanup() -> None: assert os.path.join(bucket_root, "a") in bucket_paths +@pytest.mark.skipif( + os.getenv("LTD_KEEPER_TEST_AWS_ID") is None + or os.getenv("LTD_KEEPER_TEST_AWS_SECRET") is None + or os.getenv("LTD_KEEPER_TEST_BUCKET") is None, + reason="Set LTD_KEEPER_TEST_AWS_ID, " + "LTD_KEEPER_TEST_AWS_SECRET and " + "LTD_KEEPER_TEST_BUCKET", +) def test_copy_dir_src_in_dest() -> None: """Test that copy_directory fails raises an assertion error if source in destination. """ + s3_service = open_s3_resource( + key_id=os.getenv("LTD_KEEPER_TEST_AWS_ID", ""), + access_key=os.getenv("LTD_KEEPER_TEST_AWS_SECRET", ""), + aws_region=os.getenv("LTD_KEEPER_TEST_AWS_REGION", "us-east-1"), + ) with pytest.raises(AssertionError): - copy_directory("example", "dest/src", "dest", "id", "key") + copy_directory( + s3=s3_service, + bucket_name="example", + src_path="dest/src", + dest_path="dest", + ) +@pytest.mark.skipif( + os.getenv("LTD_KEEPER_TEST_AWS_ID") is None + or os.getenv("LTD_KEEPER_TEST_AWS_SECRET") is None + or os.getenv("LTD_KEEPER_TEST_BUCKET") is None, + reason="Set LTD_KEEPER_TEST_AWS_ID, " + "LTD_KEEPER_TEST_AWS_SECRET and " + "LTD_KEEPER_TEST_BUCKET", +) def test_copy_dir_dest_in_src() -> None: """Test that copy_directory fails raises an assertion error if destination is part of the source. """ + s3_service = open_s3_resource( + key_id=os.getenv("LTD_KEEPER_TEST_AWS_ID", ""), + access_key=os.getenv("LTD_KEEPER_TEST_AWS_SECRET", ""), + aws_region=os.getenv("LTD_KEEPER_TEST_AWS_REGION", "us-east-1"), + ) with pytest.raises(AssertionError): - copy_directory("example", "src", "src/dest", "id", "key") + copy_directory( + s3=s3_service, + bucket_name="example", + src_path="src", + dest_path="src/dest", + ) def _upload_files( @@ -334,14 +364,16 @@ def test_set_condition( def test_presign_post_url_for_prefix(mocker: Mock) -> None: - mock_s3_session = mocker.MagicMock() + mock_s3_service = mocker.MagicMock() + mock_s3_meta = mocker.MagicMock() mock_s3_client = mocker.MagicMock() - mock_s3_session.client.return_value = mock_s3_client + type(mock_s3_meta).client = mock_s3_client + type(mock_s3_service).meta = PropertyMock(return_value=mock_s3_meta) expiration = 3600 bucket_name = "example-bucket" presign_post_url_for_prefix( - s3_session=mock_s3_session, + s3=mock_s3_service, bucket_name=bucket_name, prefix="base/prefix", expiration=expiration, @@ -359,14 +391,16 @@ def test_presign_post_url_for_prefix_malformed(mocker: Mock) -> None: """Same test as test_presign_post_url_for_prefix, but prefix has a trailing slash. """ - mock_s3_session = mocker.MagicMock() + mock_s3_service = mocker.MagicMock() + mock_s3_meta = mocker.MagicMock() mock_s3_client = mocker.MagicMock() - mock_s3_session.client.return_value = mock_s3_client + type(mock_s3_meta).client = mock_s3_client + type(mock_s3_service).meta = PropertyMock(return_value=mock_s3_meta) expiration = 3600 bucket_name = "example-bucket" presign_post_url_for_prefix( - s3_session=mock_s3_session, + s3=mock_s3_service, bucket_name=bucket_name, prefix="base/prefix/", expiration=expiration, @@ -381,9 +415,11 @@ def test_presign_post_url_for_prefix_malformed(mocker: Mock) -> None: def test_presign_post_url_for_prefix_with_conditions(mocker: Mock) -> None: - mock_s3_session = mocker.MagicMock() + mock_s3_service = mocker.MagicMock() + mock_s3_meta = mocker.MagicMock() mock_s3_client = mocker.MagicMock() - mock_s3_session.client.return_value = mock_s3_client + type(mock_s3_meta).client = mock_s3_client + type(mock_s3_service).meta = PropertyMock(return_value=mock_s3_meta) url_conditions = [ {"acl": "public-read"}, @@ -402,7 +438,7 @@ def test_presign_post_url_for_prefix_with_conditions(mocker: Mock) -> None: expiration = 3600 bucket_name = "example-bucket" presign_post_url_for_prefix( - s3_session=mock_s3_session, + s3=mock_s3_service, bucket_name=bucket_name, prefix="base/prefix", expiration=expiration, @@ -419,14 +455,16 @@ def test_presign_post_url_for_prefix_with_conditions(mocker: Mock) -> None: def test_presign_post_url_for_directory_objects(mocker: Mock) -> None: - mock_s3_session = mocker.MagicMock() + mock_s3_service = mocker.MagicMock() + mock_s3_meta = mocker.MagicMock() mock_s3_client = mocker.MagicMock() - mock_s3_session.client.return_value = mock_s3_client + type(mock_s3_meta).client = mock_s3_client + type(mock_s3_service).meta = PropertyMock(return_value=mock_s3_meta) expiration = 3600 bucket_name = "example-bucket" presign_post_url_for_directory_object( - s3_session=mock_s3_session, + s3=mock_s3_service, bucket_name=bucket_name, key="base/prefix", expiration=expiration, @@ -443,14 +481,16 @@ def test_presign_post_url_for_directory_objects(mocker: Mock) -> None: def test_presign_post_url_for_directory_objects_with_conditions( mocker: Mock, ) -> None: - mock_s3_session = mocker.MagicMock() + mock_s3_service = mocker.MagicMock() + mock_s3_meta = mocker.MagicMock() mock_s3_client = mocker.MagicMock() - mock_s3_session.client.return_value = mock_s3_client + type(mock_s3_meta).client = mock_s3_client + type(mock_s3_service).meta = PropertyMock(return_value=mock_s3_meta) expiration = 3600 bucket_name = "example-bucket" presign_post_url_for_directory_object( - s3_session=mock_s3_session, + s3=mock_s3_service, bucket_name=bucket_name, key="base/prefix", expiration=expiration, diff --git a/tests/v2api/test_project.py b/tests/v2api/test_project.py index f3d11e5e..3460733b 100644 --- a/tests/v2api/test_project.py +++ b/tests/v2api/test_project.py @@ -32,8 +32,8 @@ def test_projects(client: TestClient, mocker: Mock) -> None: "keeper.services.createbuild.presign_post_url_for_directory_object", new=MagicMock(return_value=mock_presigned_url), ) - s3_session_mock = mocker.patch( - "keeper.services.createbuild.open_s3_session" + s3_resource_mock = mocker.patch( + "keeper.services.createbuild.open_s3_resource" ) # Create a default organization =========================================== @@ -107,7 +107,7 @@ def test_projects(client: TestClient, mocker: Mock) -> None: build1_request = {"git_ref": "main"} r = client.post(project1_builds_url, build1_request) task_queue.apply_task_side_effects() - s3_session_mock.assert_called_once() + s3_resource_mock.assert_called_once() presign_post_mock.assert_called_once() assert r.status == 201 assert r.json["project_url"] == project1_url diff --git a/tests/v2api/test_project_path_layout.py b/tests/v2api/test_project_path_layout.py index 8dda6afe..b075f291 100644 --- a/tests/v2api/test_project_path_layout.py +++ b/tests/v2api/test_project_path_layout.py @@ -32,8 +32,8 @@ def test_projects(client: TestClient, mocker: Mock) -> None: "keeper.services.createbuild.presign_post_url_for_directory_object", new=MagicMock(return_value=mock_presigned_url), ) - s3_session_mock = mocker.patch( - "keeper.services.createbuild.open_s3_session" + s3_service_mock = mocker.patch( + "keeper.services.createbuild.open_s3_resource" ) # Create a default organization =========================================== @@ -98,7 +98,7 @@ def test_projects(client: TestClient, mocker: Mock) -> None: build1_request = {"git_ref": "main"} r = client.post(project1_builds_url, build1_request) task_queue.apply_task_side_effects() - s3_session_mock.assert_called_once() + s3_service_mock.assert_called_once() presign_post_mock.assert_called_once() assert r.status == 201 assert r.json["project_url"] == project1_url diff --git a/tox.ini b/tox.ini index 0ff43f22..1ba0426c 100644 --- a/tox.ini +++ b/tox.ini @@ -54,6 +54,7 @@ setenv = passenv = LTD_KEEPER_TEST_AWS_ID LTD_KEEPER_TEST_AWS_SECRET + LTD_KEEPER_TEST_AWS_REGION LTD_KEEPER_TEST_BUCKET deps = -r{toxinidir}/requirements/main.txt From 92d6ee162443a9c43d540c8c791294ccbf81eaad Mon Sep 17 00:00:00 2001 From: Jonathan Sick Date: Mon, 21 Mar 2022 14:49:54 -0400 Subject: [PATCH 06/17] Enhance logging of edition rebuild --- keeper/services/requesteditionrebuild.py | 9 +++++++++ keeper/services/updatebuild.py | 5 +++++ keeper/taskrunner.py | 4 ++-- 3 files changed, 16 insertions(+), 2 deletions(-) diff --git a/keeper/services/requesteditionrebuild.py b/keeper/services/requesteditionrebuild.py index 106f4f4e..81ba4c2f 100644 --- a/keeper/services/requesteditionrebuild.py +++ b/keeper/services/requesteditionrebuild.py @@ -4,6 +4,8 @@ from typing import TYPE_CHECKING +from structlog import get_logger + from keeper.exceptions import ValidationError from keeper.taskrunner import queue_task_command @@ -14,6 +16,13 @@ def request_edition_rebuild(*, edition: Edition, build: Build) -> Edition: + logger = get_logger(__name__) + logger.info( + "Starting request_edition_rebuild", + edition=edition.slug, + build=build.slug, + ) + # if edition.pending_rebuild: # raise ValidationError( # "This edition already has a pending rebuild, this request " diff --git a/keeper/services/updatebuild.py b/keeper/services/updatebuild.py index 83929bd9..21781906 100644 --- a/keeper/services/updatebuild.py +++ b/keeper/services/updatebuild.py @@ -4,6 +4,8 @@ from typing import TYPE_CHECKING, Optional +import structlog + from keeper.models import db from .updateedition import update_edition @@ -30,6 +32,9 @@ def update_build(*, build: Build, uploaded: Optional[bool]) -> Build: build : `keeper.models.Build` Build model. """ + logger = structlog.get_logger(__name__) + logger.info("Updating build", build=build.slug, uploaded=uploaded) + if uploaded is True: build.register_uploaded_build() db.session.add(build) diff --git a/keeper/taskrunner.py b/keeper/taskrunner.py index e39a5e95..607362d7 100644 --- a/keeper/taskrunner.py +++ b/keeper/taskrunner.py @@ -38,11 +38,11 @@ def launch_tasks() -> celery.chain: inspect_task_queue(task_commands) if len(task_commands) == 0: - logger.debug("Did not launch any tasks", ntasks=0) + logger.info("Did not launch any tasks", ntasks=0) return if not current_app.config["ENABLE_TASKS"]: - logger.debug("Celery taks are disabled") + logger.info("Celery taks are disabled") return celery_task_signatures: List[celery.Signature] = [] From 6c63fab158a9d0821738760fd45f6efd8f46b678 Mon Sep 17 00:00:00 2001 From: Jonathan Sick Date: Mon, 21 Mar 2022 15:29:17 -0400 Subject: [PATCH 07/17] Fix patch_edition v2 endpoint --- keeper/v2api/editions.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/keeper/v2api/editions.py b/keeper/v2api/editions.py index 38603199..9656ac03 100644 --- a/keeper/v2api/editions.py +++ b/keeper/v2api/editions.py @@ -113,7 +113,7 @@ def patch_edition( .filter(Organization.slug == org) .filter(Product.slug == project) .filter(Edition.slug == id) - .all() + .first_or_404() ) request_data = EditionPatchRequest.parse_obj(request.json) From 8ca4e19ccc271a2b8595a36dd8ee743edbcdfdb5 Mon Sep 17 00:00:00 2001 From: Jonathan Sick Date: Mon, 21 Mar 2022 15:42:58 -0400 Subject: [PATCH 08/17] Fix Fastly support in rebuild_edition --- keeper/models.py | 4 ++-- keeper/tasks/editionrebuild.py | 8 ++++++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/keeper/models.py b/keeper/models.py index cdbe6cbe..54be2314 100644 --- a/keeper/models.py +++ b/keeper/models.py @@ -374,11 +374,11 @@ def set_fastly_api_key(self, api_key: Optional[SecretStr]) -> None: return self.fastly_encrypted_api_key = self._encrypt_secret_str(api_key) - def get_fastly_api_key(self) -> SecretStr: + def get_fastly_api_key(self) -> Optional[SecretStr]: """Get the decrypted Fastly API key.""" encrypted_key = self.fastly_encrypted_api_key if encrypted_key is None: - raise ValueError("fastly_encrypted_api_key is not set.") + return None return self._decrypt_to_secret_str(encrypted_key) def set_aws_secret_key(self, secret_key: Optional[SecretStr]) -> None: diff --git a/keeper/tasks/editionrebuild.py b/keeper/tasks/editionrebuild.py index a48fb370..8929acf6 100644 --- a/keeper/tasks/editionrebuild.py +++ b/keeper/tasks/editionrebuild.py @@ -66,7 +66,7 @@ def rebuild_edition( use_public_read_acl = organization.bucket_public_read fastly_service_id = organization.fastly_service_id - fastly_key = organization.get_fastly_api_id + fastly_key = organization.get_fastly_api_key() try: edition.set_pending_rebuild(new_build) @@ -96,7 +96,11 @@ def rebuild_edition( "Skipping rebuild because AWS credentials are not set" ) - if fastly_service_id is not None and fastly_key is not None: + if ( + organization.fastly_support + and fastly_service_id is not None + and fastly_key is not None + ): logger.info("Starting Fastly purge_key") fastly_service = fastly.FastlyService( fastly_service_id, fastly_key.get_secret_value() From 00566de3406759a1ac1bbbc4051b3339feb53b17 Mon Sep 17 00:00:00 2001 From: Jonathan Sick Date: Mon, 21 Mar 2022 15:59:10 -0400 Subject: [PATCH 09/17] Update structlog config for production --- keeper/config.py | 66 +++++++++++++++++++++++++++++++++++++----------- 1 file changed, 51 insertions(+), 15 deletions(-) diff --git a/keeper/config.py b/keeper/config.py index 489a1345..5f6b9ddc 100644 --- a/keeper/config.py +++ b/keeper/config.py @@ -6,9 +6,11 @@ import logging import os import sys -from typing import TYPE_CHECKING, Dict, Optional, Type +from typing import TYPE_CHECKING, Any, Dict, List, Optional, Type import structlog +from structlog.stdlib import add_log_level +from structlog.types import EventDict from keeper.models import EditionKind @@ -198,28 +200,62 @@ def init_app(cls, app: Flask) -> None: stream_handler = logging.StreamHandler(stream=sys.stdout) stream_handler.setFormatter(logging.Formatter("%(message)s")) logger = logging.getLogger("keeper") + logger.handlers = [] logger.addHandler(stream_handler) - if logger.hasHandlers(): - logger.handlers.clear() - logger.setLevel(logging.INFO) + logger.setLevel("INFO") + + processors: List[Any] = [ + structlog.stdlib.filter_by_level, + structlog.stdlib.add_logger_name, + structlog.stdlib.PositionalArgumentsFormatter(), + structlog.processors.StackInfoRenderer(), + structlog.processors.UnicodeDecoder(), + ] + # JSON-formatted logging + processors.append(add_log_severity) + processors.append(structlog.processors.format_exc_info) + processors.append(structlog.processors.JSONRenderer()) structlog.configure( - processors=[ - structlog.stdlib.filter_by_level, - structlog.stdlib.add_logger_name, - structlog.stdlib.add_log_level, - structlog.stdlib.PositionalArgumentsFormatter(), - structlog.processors.StackInfoRenderer(), - structlog.processors.format_exc_info, - structlog.processors.UnicodeDecoder(), - structlog.processors.JSONRenderer(), - ], - context_class=structlog.threadlocal.wrap_dict(dict), + processors=processors, logger_factory=structlog.stdlib.LoggerFactory(), + wrapper_class=structlog.stdlib.BoundLogger, cache_logger_on_first_use=True, ) +def add_log_severity( + logger: logging.Logger, method_name: str, event_dict: EventDict +) -> EventDict: + """Add the log level to the event dict as ``severity``. + + Intended for use as a structlog processor. + + This is the same as `structlog.stdlib.add_log_level` except that it + uses the ``severity`` key rather than ``level`` for compatibility with + Google Log Explorer and its automatic processing of structured logs. + + Parameters + ---------- + logger : `logging.Logger` + The wrapped logger object. + method_name : `str` + The name of the wrapped method (``warning`` or ``error``, for + example). + event_dict : `structlog.types.EventDict` + Current context and current event. This parameter is also modified in + place, matching the normal behavior of structlog processors. + + Returns + ------- + event_dict : `structlog.types.EventDict` + The modified `~structlog.types.EventDict` with the added key. + """ + severity = add_log_level(logger, method_name, {})["level"] + event_dict["severity"] = severity + return event_dict + + config: Dict[str, Type[Config]] = { "development": DevelopmentConfig, "testing": TestConfig, From a75c51594e225fbada37f1be23cf2de581da29a4 Mon Sep 17 00:00:00 2001 From: Jonathan Sick Date: Mon, 21 Mar 2022 17:02:46 -0400 Subject: [PATCH 10/17] Fix getting product in build_dashboard task --- keeper/tasks/dashboardbuild.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/keeper/tasks/dashboardbuild.py b/keeper/tasks/dashboardbuild.py index 7b78e0b9..3f5d6f40 100644 --- a/keeper/tasks/dashboardbuild.py +++ b/keeper/tasks/dashboardbuild.py @@ -31,7 +31,7 @@ def build_dashboard(self: celery.task.Task, product_id: str) -> None: self.request.retries, ) - product = Product.query(id=product_id).one() + product = Product.query.get(product_id) build_dashboard_svc(product, logger) logger.info("Finished triggering dashboard build") From 4f05b0cf8e3aab74e7010f5536f94916eb214215 Mon Sep 17 00:00:00 2001 From: Jonathan Sick Date: Mon, 21 Mar 2022 17:16:57 -0400 Subject: [PATCH 11/17] Re-raise exceptions in patch builds An error with PATCH /v1/orgs/:org/projects/:project/builds was being handled to roll-back the database, but this resulted in a success (202) being returned to the client rather than an error message. --- keeper/models.py | 12 ++++++++++-- keeper/s3.py | 1 + keeper/services/updatebuild.py | 5 +++++ keeper/services/updateedition.py | 5 +++++ keeper/tasks/editionrebuild.py | 23 ++++++++++++++++------- keeper/v2api/builds.py | 4 ++++ 6 files changed, 41 insertions(+), 9 deletions(-) diff --git a/keeper/models.py b/keeper/models.py index 54be2314..21e0c935 100644 --- a/keeper/models.py +++ b/keeper/models.py @@ -689,11 +689,17 @@ def register_uploaded_build(self) -> None: def get_tracking_editions(self) -> List[Edition]: """Get the editions that should rebuild to this build.""" + logger = get_logger(__name__) editions = ( Edition.query.autoflush(False) .filter(Edition.product == self.product) .all() ) + logger.debug( + "In get_tracking_editions found editions for product", + count=len(editions), + editions=str(editions), + ) return [ edition @@ -852,12 +858,15 @@ def should_rebuild(self, build: Build) -> bool: `True` if the edition should be rebuilt using this Build, or `False` otherwise. """ + logger = get_logger(__name__) + logger.debug("Inside Edition.should_rebuild") + # shim during refactoring from keeper.api._urls import url_for_edition logger = get_logger(__name__) - logger.debug( + logger.info( "Edition {!r} in should_rebuild".format(url_for_edition(self)) ) @@ -872,7 +881,6 @@ def should_rebuild(self, build: Build) -> bool: try: tracking_mode = edition_tracking_modes[self.mode] except (KeyError, ValidationError): - tracking_mode = edition_tracking_modes[self.default_mode_id] logger.warning( "Edition {!r} has an unknown tracking" diff --git a/keeper/s3.py b/keeper/s3.py index 56715160..2eea2bd6 100644 --- a/keeper/s3.py +++ b/keeper/s3.py @@ -216,6 +216,7 @@ def copy_directory( # Copy each object from source to destination for src_obj in bucket.objects.filter(Prefix=src_path): + print(f"Copying {src_obj.key}") src_rel_path = os.path.relpath(src_obj.key, start=src_path) dest_key_path = os.path.join(dest_path, src_rel_path) diff --git a/keeper/services/updatebuild.py b/keeper/services/updatebuild.py index 21781906..f7219b2b 100644 --- a/keeper/services/updatebuild.py +++ b/keeper/services/updatebuild.py @@ -41,6 +41,11 @@ def update_build(*, build: Build, uploaded: Optional[bool]) -> Build: db.session.commit() editions_to_rebuild = build.get_tracking_editions() + logger.info( + "Found editions tracking this uploaded build", + build=build.slug, + count=len(editions_to_rebuild), + ) for edition in editions_to_rebuild: update_edition(edition=edition, build=build) diff --git a/keeper/services/updateedition.py b/keeper/services/updateedition.py index a19443ca..78932939 100644 --- a/keeper/services/updateedition.py +++ b/keeper/services/updateedition.py @@ -32,6 +32,11 @@ def update_edition( build. """ logger = get_logger(__name__) + logger.info( + "Updating edition", + edition=edition.slug, + new_build=build.slug if build else None, + ) if tracked_ref is not None: edition.tracked_refs = [tracked_ref] diff --git a/keeper/tasks/editionrebuild.py b/keeper/tasks/editionrebuild.py index 8929acf6..6e6488cf 100644 --- a/keeper/tasks/editionrebuild.py +++ b/keeper/tasks/editionrebuild.py @@ -45,12 +45,6 @@ def rebuild_edition( 2. Purge Fastly's cache for this edition. 2. Send a ``edition.updated`` payload to LTD Events (if configured). """ - logger.info( - "Starting rebuild edition edition_id=%s retry=%d", - edition_id, - self.request.retries, - ) - # LTD_EVENTS_URL = current_app.config["LTD_EVENTS_URL"] # api_url_parts = urlsplit(edition_url) @@ -60,6 +54,15 @@ def rebuild_edition( organization = edition.product.organization new_build = Build.query.get(build_id) + logger.info( + "Starting rebuild_edition for %s/%s/%s with build %s retry=%d", + organization.slug, + edition.product.slug, + edition.slug, + new_build.slug, + self.request.retries, + ) + aws_id = organization.aws_id aws_secret = organization.get_aws_secret_key() aws_region = organization.aws_region @@ -72,7 +75,12 @@ def rebuild_edition( edition.set_pending_rebuild(new_build) if aws_id is not None and aws_secret is not None: - logger.info("Starting copy_directory") + logger.info( + "Starting copy_directory %s to %s public_read=%s", + new_build.bucket_root_dirname, + edition.bucket_root_dirname, + use_public_read_acl, + ) s3_service = s3.open_s3_resource( key_id=aws_id, access_key=aws_secret.get_secret_value(), @@ -120,6 +128,7 @@ def rebuild_edition( # if LTD_EVENTS_URL is not None: # send_edition_updated_event(edition, LTD_EVENTS_URL, api_root) except Exception: + logger.exception("Error during copy") db.session.rollback() edition = Edition.query.get(edition_id) diff --git a/keeper/v2api/builds.py b/keeper/v2api/builds.py index 6957eff5..43f1eaa0 100644 --- a/keeper/v2api/builds.py +++ b/keeper/v2api/builds.py @@ -4,6 +4,7 @@ from typing import Dict, Tuple +import structlog from flask import request from flask_accept import accept_fallback @@ -110,6 +111,7 @@ def post_build(org: str, project: str) -> Tuple[str, int, Dict[str, str]]: def patch_build( org: str, project: str, id: str ) -> Tuple[str, int, Dict[str, str]]: + logger = structlog.get_logger() build = ( Build.query.join(Product, Product.id == Build.product_id) .join(Organization, Organization.id == Product.organization_id) @@ -124,7 +126,9 @@ def patch_build( try: build = update_build(build=build, uploaded=request_data.uploaded) except Exception: + logger.exception("Error patching build") db.session.rollback() + raise # Run the task queue task = launch_tasks() From cd4c9c212c1f9552db536e667bc42400cf66fcc3 Mon Sep 17 00:00:00 2001 From: Jonathan Sick Date: Thu, 24 Mar 2022 18:51:36 -0400 Subject: [PATCH 12/17] Stop calculating v1 URL This was a holdover from Keeper v1 to maintain consistency in logging, but overall its not safe to do this because the v1 APIs may not be enabled for a given deployment. --- keeper/models.py | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/keeper/models.py b/keeper/models.py index 21e0c935..c96b9ebe 100644 --- a/keeper/models.py +++ b/keeper/models.py @@ -861,15 +861,8 @@ def should_rebuild(self, build: Build) -> bool: logger = get_logger(__name__) logger.debug("Inside Edition.should_rebuild") - # shim during refactoring - from keeper.api._urls import url_for_edition - logger = get_logger(__name__) - logger.info( - "Edition {!r} in should_rebuild".format(url_for_edition(self)) - ) - candidate_build = build # Prefilter @@ -883,8 +876,7 @@ def should_rebuild(self, build: Build) -> bool: except (KeyError, ValidationError): tracking_mode = edition_tracking_modes[self.default_mode_id] logger.warning( - "Edition {!r} has an unknown tracking" - "mode".format(url_for_edition(self)) + "Edition {!r} has an unknown tracking" "mode".format(self.slug) ) return tracking_mode.should_update(self, candidate_build) From 2770f4f3ba891aa4dc4739469deaf99d6dc45c54 Mon Sep 17 00:00:00 2001 From: Jonathan Sick Date: Thu, 24 Mar 2022 17:19:51 -0400 Subject: [PATCH 13/17] Add missing services/__init__.py --- keeper/services/__init__.py | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 keeper/services/__init__.py diff --git a/keeper/services/__init__.py b/keeper/services/__init__.py new file mode 100644 index 00000000..e69de29b From 3a5c2111339c671de1410189dc4f16d15636f35f Mon Sep 17 00:00:00 2001 From: Jonathan Sick Date: Thu, 24 Mar 2022 18:20:28 -0400 Subject: [PATCH 14/17] Resolve typing issues These appeared with mypy 0.942. --- keeper/services/createbuild.py | 2 +- keeper/services/createedition.py | 9 +++++---- keeper/services/updateedition.py | 2 +- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/keeper/services/createbuild.py b/keeper/services/createbuild.py index 1f1bb251..1e8063a6 100644 --- a/keeper/services/createbuild.py +++ b/keeper/services/createbuild.py @@ -28,7 +28,7 @@ def create_build( *, product: Product, - git_ref: str, + git_ref: Optional[str], github_requester: Optional[str] = None, slug: Optional[str] = None, ) -> Tuple[Build, Optional[Edition]]: diff --git a/keeper/services/createedition.py b/keeper/services/createedition.py index 85f428f1..3c9cd22e 100644 --- a/keeper/services/createedition.py +++ b/keeper/services/createedition.py @@ -15,11 +15,11 @@ def create_edition( *, product: Product, - title: str, + title: Optional[str], tracking_mode: Optional[str] = None, slug: Optional[str] = None, - autoincrement_slug: bool = False, - tracked_ref: str = "main", + autoincrement_slug: Optional[bool] = False, + tracked_ref: Optional[str] = "main", build: Optional[Build] = None, ) -> Edition: """Create a new edition. @@ -39,7 +39,8 @@ def create_edition( The URL-safe slug for this edition. Can be `None` if ``autoincrement_slug`` is True. title : str - The human-readable title. + The human-readable title; can be None if ``autoincrement_slug`` is + True. autoincrement_slug : bool If True, rather then use the provided ``slug``, the slug is an integer that is incremented by one from the previously-existing integer diff --git a/keeper/services/updateedition.py b/keeper/services/updateedition.py index 78932939..a012ec49 100644 --- a/keeper/services/updateedition.py +++ b/keeper/services/updateedition.py @@ -49,7 +49,7 @@ def update_edition( edition.title = title if slug is not None: - request_edition_rename(edition=edition) + request_edition_rename(edition=edition, slug=slug) product = edition.product From 04e1be252a375aace0f135cb5f137f0151077c91 Mon Sep 17 00:00:00 2001 From: Jonathan Sick Date: Thu, 24 Mar 2022 18:28:03 -0400 Subject: [PATCH 15/17] Disable auto package discovery This will eventually be fixed by moving to a src-based package layout. --- setup.cfg | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/setup.cfg b/setup.cfg index 8accd2a0..bbb363a0 100644 --- a/setup.cfg +++ b/setup.cfg @@ -23,7 +23,8 @@ keywords = [options] zip_safe = False include_package_data = True -packages=find: +packages= + keeper python_requires = >=3.8 setup_requires = setuptools_scm From bd3a69908fb5455d5d64a6f4354f0f6c3fb7ee7a Mon Sep 17 00:00:00 2001 From: Jonathan Sick Date: Fri, 25 Mar 2022 14:45:33 -0400 Subject: [PATCH 16/17] Add org aws_region and bucket_public_read cols These columns for Organization replace the stand-in properties used for development. They are initially set as nullable to support migrations, with getter methods to support the migration for type checking. --- keeper/models.py | 35 ++++++++++++++----- keeper/services/createbuild.py | 4 +-- keeper/services/createorg.py | 4 +++ keeper/tasks/editionrebuild.py | 4 +-- keeper/v2api/_models.py | 20 ++++++++++- keeper/v2api/organizations.py | 4 ++- ...add_organization_aws_region_and_public_.py | 29 +++++++++++++++ tests/v2api/test_orgs.py | 3 +- tests/v2api/test_project.py | 2 +- tests/v2api/test_project_path_layout.py | 2 +- 10 files changed, 90 insertions(+), 17 deletions(-) create mode 100644 migrations/versions/8fa19dcad1d1_add_organization_aws_region_and_public_.py diff --git a/keeper/models.py b/keeper/models.py index c96b9ebe..dfdd913c 100644 --- a/keeper/models.py +++ b/keeper/models.py @@ -342,6 +342,17 @@ class Organization(db.Model): # type: ignore aws_encrypted_secret_key = db.Column(db.LargeBinary, nullable=True) """The AWS secret key.""" + # FIXME nullable for migration + aws_region = db.Column(db.Unicode(255), nullable=True, default="us-east-1") + """The AWS region of the S3 bucket.""" + + # FIXME nullable for migration + bucket_public_read = db.Column(db.Boolean, nullable=True, default=False) + """If True, objects in the S3 bucket will have the ``public-read`` ACL. + + For objects using a proxy, this can be False. + """ + products = db.relationship( "Product", back_populates="organization", lazy="dynamic" ) @@ -358,15 +369,23 @@ class Organization(db.Model): # type: ignore back_populates="organization", ) - # FIXME convert this into a database column - @property - def aws_region(self) -> str: - return "ca-central-1" + def get_aws_region(self) -> str: + """Get the AWS region (adapter while column is nullable for + migration). + """ + if self.aws_region is None: + return "us-east-1" + else: + return self.aws_region - # FIXME convert this into a database column - @property - def bucket_public_read(self) -> bool: - return False + def get_bucket_public_read(self) -> bool: + """Get the S3 public public-read ACL configuration (adapter while + column is nullable for migration. + """ + if self.bucket_public_read is None: + return False + else: + return self.bucket_public_read def set_fastly_api_key(self, api_key: Optional[SecretStr]) -> None: """Encrypt and set the Fastly API key.""" diff --git a/keeper/services/createbuild.py b/keeper/services/createbuild.py index 1e8063a6..7a4d811e 100644 --- a/keeper/services/createbuild.py +++ b/keeper/services/createbuild.py @@ -176,8 +176,8 @@ def create_presigned_post_urls( organization = build.product.organization aws_id = organization.aws_id aws_secret = organization.get_aws_secret_key() - aws_region = organization.aws_region - use_public_read_acl = organization.bucket_public_read + aws_region = organization.get_aws_region() + use_public_read_acl = organization.get_bucket_public_read() s3_service = open_s3_resource( key_id=aws_id, diff --git a/keeper/services/createorg.py b/keeper/services/createorg.py index ca2d2368..e61c2f73 100644 --- a/keeper/services/createorg.py +++ b/keeper/services/createorg.py @@ -17,8 +17,10 @@ def create_organization( domain: str, path_prefix: str, bucket_name: str, + s3_public_read: bool, fastly_support: bool, aws_id: Optional[str], + aws_region: Optional[str], aws_secret: Optional[SecretStr], fastly_domain: Optional[str], fastly_service_id: Optional[str], @@ -35,7 +37,9 @@ def create_organization( root_domain=domain, root_path_prefix=path_prefix, bucket_name=bucket_name, + bucket_public_read=s3_public_read, aws_id=aws_id, + aws_region=aws_region, fastly_support=fastly_support, fastly_domain=fastly_domain, fastly_service_id=fastly_service_id, diff --git a/keeper/tasks/editionrebuild.py b/keeper/tasks/editionrebuild.py index 6e6488cf..4c40882c 100644 --- a/keeper/tasks/editionrebuild.py +++ b/keeper/tasks/editionrebuild.py @@ -65,8 +65,8 @@ def rebuild_edition( aws_id = organization.aws_id aws_secret = organization.get_aws_secret_key() - aws_region = organization.aws_region - use_public_read_acl = organization.bucket_public_read + aws_region = organization.get_aws_region() + use_public_read_acl = organization.get_bucket_public_read() fastly_service_id = organization.fastly_service_id fastly_key = organization.get_fastly_api_key() diff --git a/keeper/v2api/_models.py b/keeper/v2api/_models.py index 2b74f38f..051779d6 100644 --- a/keeper/v2api/_models.py +++ b/keeper/v2api/_models.py @@ -84,6 +84,14 @@ class OrganizationResponse(BaseModel): s3_bucket: Optional[str] """Name of the S3 bucket hosting builds.""" + s3_public_read: bool + """Whether objects in the S3 bucket have a public-read ACL applied or + not. + """ + + aws_region: str + """Name of the AWS region for the S3 bucket.""" + self_url: HttpUrl """The URL of the organization response.""" @@ -103,6 +111,8 @@ def from_organization(cls, org: Organization) -> OrganizationResponse: org.fastly_service_id if org.fastly_support else None ), s3_bucket=org.bucket_name, + s3_public_read=org.get_bucket_public_read(), + aws_region=org.get_aws_region(), self_url=url_for_organization(org), projects_url=url_for_organization_projects(org), ) @@ -154,7 +164,7 @@ class OrganizationPostRequest(BaseModel): if documentation is served from the root of a domain. """ - bucket_name: str + s3_bucket: str """Name of the S3 bucket hosting builds.""" fastly_support: bool @@ -175,6 +185,14 @@ class OrganizationPostRequest(BaseModel): aws_secret: Optional[SecretStr] = None """AWS secret key.""" + aws_region: str = "us-east-1" + """AWS region of the S3 bucket.""" + + s3_public_read: bool = False + """Whether objects in the S3 bucket have a public-read ACL applied or + not. + """ + @validator("slug") def check_slug(cls, v: str) -> str: try: diff --git a/keeper/v2api/organizations.py b/keeper/v2api/organizations.py index 86c8be24..e34f7956 100644 --- a/keeper/v2api/organizations.py +++ b/keeper/v2api/organizations.py @@ -60,9 +60,11 @@ def create_organization() -> Tuple[str, int, Dict[str, Any]]: layout=request_data.layout.layout_model_enum, domain=request_data.domain, path_prefix=request_data.path_prefix, - bucket_name=request_data.bucket_name, + bucket_name=request_data.s3_bucket, + s3_public_read=request_data.s3_public_read, aws_id=request_data.aws_id, aws_secret=request_data.aws_secret, + aws_region=request_data.aws_region, fastly_support=request_data.fastly_support, fastly_domain=request_data.fastly_domain, fastly_service_id=request_data.fastly_service_id, diff --git a/migrations/versions/8fa19dcad1d1_add_organization_aws_region_and_public_.py b/migrations/versions/8fa19dcad1d1_add_organization_aws_region_and_public_.py new file mode 100644 index 00000000..ec0f8f9b --- /dev/null +++ b/migrations/versions/8fa19dcad1d1_add_organization_aws_region_and_public_.py @@ -0,0 +1,29 @@ +"""Add organization aws_region and public read + +Revision ID: 8fa19dcad1d1 +Revises: f8eeb27a49e7 +Create Date: 2022-03-25 13:46:29.088536 +""" + +import sqlalchemy as sa +from alembic import op + +# revision identifiers, used by Alembic. +revision = "8fa19dcad1d1" +down_revision = "f8eeb27a49e7" + + +def upgrade(): + with op.batch_alter_table("organizations", schema=None) as batch_op: + batch_op.add_column( + sa.Column("aws_region", sa.Unicode(length=255), nullable=True) + ) + batch_op.add_column( + sa.Column("bucket_public_read", sa.Boolean(), nullable=True) + ) + + +def downgrade(): + with op.batch_alter_table("organizations", schema=None) as batch_op: + batch_op.drop_column("bucket_public_read") + batch_op.drop_column("aws_region") diff --git a/tests/v2api/test_orgs.py b/tests/v2api/test_orgs.py index 149424f4..8fa7ca9d 100644 --- a/tests/v2api/test_orgs.py +++ b/tests/v2api/test_orgs.py @@ -29,6 +29,7 @@ def test_get_orgs(client: TestClient, mocker: Mock) -> None: root_domain="lsst.io", fastly_domain="global.ssl.fastly.net", bucket_name="bucket-name", + bucket_public_read=False, ) db.session.add(org1) db.session.commit() @@ -72,7 +73,7 @@ def test_create_fastly_org(client: TestClient, mocker: Mock) -> None: "layout": "subdomain", "domain": "example.org", "path_prefix": "/", - "bucket_name": "test-bucket", + "s3_bucket": "test-bucket", "fastly_support": True, "fastly_domain": "fastly.example.org", "fastly_service_id": "abc", diff --git a/tests/v2api/test_project.py b/tests/v2api/test_project.py index 3460733b..6ecab096 100644 --- a/tests/v2api/test_project.py +++ b/tests/v2api/test_project.py @@ -45,7 +45,7 @@ def test_projects(client: TestClient, mocker: Mock) -> None: "layout": "subdomain", "domain": "example.org", "path_prefix": "/", - "bucket_name": "test-bucket", + "s3_bucket": "test-bucket", "fastly_support": True, "fastly_domain": "fastly.example.org", "fastly_service_id": "abc", diff --git a/tests/v2api/test_project_path_layout.py b/tests/v2api/test_project_path_layout.py index b075f291..1402bc9d 100644 --- a/tests/v2api/test_project_path_layout.py +++ b/tests/v2api/test_project_path_layout.py @@ -45,7 +45,7 @@ def test_projects(client: TestClient, mocker: Mock) -> None: "layout": "path", "domain": "www.example.org", "path_prefix": "/orgprefix", - "bucket_name": "test-bucket", + "s3_bucket": "test-bucket", "fastly_support": False, } r = client.post("/v2/orgs", request_data) From 05426abf5bd939c42e3d91e58048466ed4b9f806 Mon Sep 17 00:00:00 2001 From: Jonathan Sick Date: Fri, 25 Mar 2022 14:59:04 -0400 Subject: [PATCH 17/17] Only stamp DB version if DB didn't exist This ensures that DB stamping only happens when a. the DB is initially created b. when a migration is run --- bin/start-api.bash | 4 ---- keeper/cli.py | 16 +++++++++++----- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/bin/start-api.bash b/bin/start-api.bash index dc3b10a5..07ad86d0 100755 --- a/bin/start-api.bash +++ b/bin/start-api.bash @@ -2,10 +2,6 @@ set -eu -echo $PATH -pwd -ls migrations - flask createdb migrations/alembic.ini flask init uwsgi uwsgi.ini diff --git a/keeper/cli.py b/keeper/cli.py index fe1e6d10..08990991 100644 --- a/keeper/cli.py +++ b/keeper/cli.py @@ -16,6 +16,9 @@ from keeper.models import Permission, User, db from keeper.version import get_version +# from psycopg2.errors import UndefinedTable + + if TYPE_CHECKING: from flask import Flask @@ -54,11 +57,14 @@ def createdb_command(alembicconf: str) -> None: To migrate database servers, see the copydb sub-command. """ - db.create_all() - - # stamp tables with latest schema version - alembic_cfg = alembic.config.Config(alembicconf) - alembic.command.stamp(alembic_cfg, "head") + try: + User.query.get(1) + except Exception: + db.create_all() + + # stamp tables with latest schema version + alembic_cfg = alembic.config.Config(alembicconf) + alembic.command.stamp(alembic_cfg, "head") @click.command("init")