Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Issue 2512] S3 Presign URL #2563

Merged
merged 40 commits into from
Oct 30, 2024
Merged
Show file tree
Hide file tree
Changes from 28 commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
4d9dcff
test files
babebe Oct 24, 2024
7728a06
update factory and seed local db
babebe Oct 24, 2024
3b29af1
update opp with presigned url and return
babebe Oct 24, 2024
c5bee75
clean up
babebe Oct 24, 2024
e0dda32
Merge branch 'main' into S3-presign-URL
babebe Oct 24, 2024
66cc915
clean
babebe Oct 24, 2024
387655b
cleanup
babebe Oct 24, 2024
071ffee
merge conlfict
babebe Oct 24, 2024
7da3f4c
add test
babebe Oct 25, 2024
d7510b8
cleanup
babebe Oct 25, 2024
c8ee7ef
unit test
babebe Oct 28, 2024
c454096
cleanup
babebe Oct 28, 2024
c6e62bc
Merge branch 'main' into 2512-s3-presign-url
babebe Oct 28, 2024
88487ae
add timeout to request
babebe Oct 28, 2024
a866aa6
update attachment test
babebe Oct 28, 2024
698687e
fomrat
babebe Oct 28, 2024
bc7b605
update test fixture set new local env
babebe Oct 28, 2024
2cc0466
updatew unit test
babebe Oct 29, 2024
39f048d
Create ERD diagram and Update OpenAPI spec
nava-platform-bot Oct 29, 2024
292f886
clean
babebe Oct 29, 2024
cc6bcac
update response schema and test
babebe Oct 29, 2024
49c7655
Merge remote-tracking branch 'origin/2512-s3-presign-url' into 2512-s…
babebe Oct 29, 2024
451ed0a
Merge branch 'main' into 2512-s3-presign-url
babebe Oct 29, 2024
bf19076
Create ERD diagram and Update OpenAPI spec
nava-platform-bot Oct 29, 2024
1838315
clean up
babebe Oct 29, 2024
f52ebc7
Merge remote-tracking branch 'origin/2512-s3-presign-url' into 2512-s…
babebe Oct 29, 2024
1201d6a
Create ERD diagram and Update OpenAPI spec
nava-platform-bot Oct 29, 2024
6c6a740
update test
babebe Oct 29, 2024
a443fa6
timeout
babebe Oct 29, 2024
1974217
Merge branch 'main' into 2512-s3-presign-url
babebe Oct 29, 2024
6317a45
rm file_location from test
babebe Oct 29, 2024
f56413c
set to localhost when local
babebe Oct 29, 2024
5da0c66
Merge branch 'main' into 2512-s3-presign-url
babebe Oct 29, 2024
2cf28ae
format
babebe Oct 29, 2024
147ea61
clean
babebe Oct 30, 2024
3bd6e18
update get_opportunity
babebe Oct 30, 2024
071134e
format lint
babebe Oct 30, 2024
8a6a622
reusable func and cleanup
babebe Oct 30, 2024
8947231
Create ERD diagram and Update OpenAPI spec
nava-platform-bot Oct 30, 2024
3dd9563
Merge branch 'main' into 2512-s3-presign-url
babebe Oct 30, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions api/local.env
Original file line number Diff line number Diff line change
Expand Up @@ -109,3 +109,6 @@ IS_LOCAL_FOREIGN_TABLE=true

# File path for the export_opportunity_data task
EXPORT_OPP_DATA_FILE_PATH=/tmp

# This env var is used to set local AWS credentials
IS_LOCAL_AWS=1
4 changes: 2 additions & 2 deletions api/openapi.generated.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2036,9 +2036,9 @@ components:
OpportunityAttachmentV1:
type: object
properties:
file_location:
download_path:
type: string
description: The URL to download the attachment
description: The pre-signed s3 URL to download the attachment
example: https://...
mime_type:
type: string
Expand Down
1 change: 1 addition & 0 deletions api/src/adapters/aws/s3_adapter.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ class S3Config(PydanticBaseEnvConfig):
# We should generally not need to set this except
# locally to use localstack
s3_endpoint_url: str | None = None
presigned_s3_duration: int = 1800

### S3 Buckets
# note that we default these to None
Expand Down
4 changes: 2 additions & 2 deletions api/src/api/opportunities_v1/opportunity_schemas.py
Original file line number Diff line number Diff line change
Expand Up @@ -318,9 +318,9 @@ class OpportunityV1Schema(Schema):


class OpportunityAttachmentV1Schema(Schema):
file_location = fields.String(
download_path = fields.String(
metadata={
"description": "The URL to download the attachment",
"description": "The pre-signed s3 URL to download the attachment",
babebe marked this conversation as resolved.
Show resolved Hide resolved
"example": "https://...",
}
)
Expand Down
42 changes: 40 additions & 2 deletions api/src/services/opportunities_v1/get_opportunity.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,18 @@
import os
from datetime import date
from typing import List
babebe marked this conversation as resolved.
Show resolved Hide resolved

import boto3
from sqlalchemy import select
from sqlalchemy.orm import noload, selectinload

import src.adapters.db as db
import src.util.datetime_util as datetime_util
from src.adapters.aws import S3Config, get_s3_client
from src.api.route_utils import raise_flask_error
from src.db.models.agency_models import Agency
from src.db.models.opportunity_models import Opportunity, OpportunitySummary
from src.db.models.opportunity_models import Opportunity, OpportunityAttachment, OpportunitySummary
from src.util.file_util import split_s3_url


def _fetch_opportunity(
Expand Down Expand Up @@ -35,8 +40,41 @@ def _fetch_opportunity(
return opportunity


def get_boto_session() -> boto3.Session:
is_local = bool(os.getenv("IS_LOCAL_AWS", False))
if is_local:
return boto3.Session(aws_access_key_id="NO_CREDS", aws_secret_access_key="NO_CREDS")

return boto3.Session()
babebe marked this conversation as resolved.
Show resolved Hide resolved


def pre_sign_opportunity_file_location(
opp_atts: List,
) -> List[OpportunityAttachment]:
s3_config = S3Config()

s3_client = get_s3_client(s3_config, get_boto_session())
for opp_att in opp_atts:
file_loc = opp_att.file_location
bucket, key = split_s3_url(file_loc)
pre_sign_file_loc = s3_client.generate_presigned_url(
"get_object",
Params={"Bucket": bucket, "Key": key},
ExpiresIn=s3_config.presigned_s3_duration,
)
opp_att.download_path = pre_sign_file_loc

return opp_atts


def get_opportunity(db_session: db.Session, opportunity_id: int) -> Opportunity:
return _fetch_opportunity(db_session, opportunity_id, load_all_opportunity_summaries=False)
opportunity = _fetch_opportunity(
db_session, opportunity_id, load_all_opportunity_summaries=False
)

pre_sign_opportunity_file_location(opportunity.opportunity_attachments)

return opportunity


def get_opportunity_versions(db_session: db.Session, opportunity_id: int) -> dict:
Expand Down
23 changes: 23 additions & 0 deletions api/tests/conftest.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import logging
import os
import pathlib
import uuid

import _pytest.monkeypatch
Expand Down Expand Up @@ -50,6 +52,26 @@ def test_example(monkeypatch):
load_local_env_vars()


### Uploads test files
@pytest.fixture
def upload_opportunity_attachment_s3(reset_aws_env_vars, mock_s3_bucket):
s3_client = boto3.client("s3")
test_folder_path = (
pathlib.Path(__file__).parent.resolve() / "lib/opportunity_attachment_test_files"
)

for root, _, files in os.walk(test_folder_path):
for file in files:
file_path = os.path.join(root, file)
s3_client.upload_file(
file_path, Bucket=mock_s3_bucket, Key=os.path.relpath(file_path, test_folder_path)
)

# Check files were uploaded to mock s3
s3_files = s3_client.list_objects_v2(Bucket=mock_s3_bucket)
assert len(s3_files["Contents"]) == 5


####################
# Test DB session
####################
Expand Down Expand Up @@ -244,6 +266,7 @@ def reset_aws_env_vars(monkeypatch):
monkeypatch.setenv("AWS_SECURITY_TOKEN", "testing")
monkeypatch.setenv("AWS_SESSION_TOKEN", "testing")
monkeypatch.setenv("AWS_DEFAULT_REGION", "us-east-1")
monkeypatch.delenv("S3_ENDPOINT_URL")


@pytest.fixture
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Hello, world
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Hello, world - again!
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Hello, motto
Binary file not shown.
Binary file not shown.
31 changes: 31 additions & 0 deletions api/tests/lib/seed_local_db.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,18 @@
import logging
import os
import pathlib
import random

import boto3
import click
from botocore.exceptions import ClientError
from sqlalchemy import func

import src.adapters.db as db
import src.logging
import src.util.datetime_util as datetime_util
import tests.src.db.models.factories as factories
from src.adapters.aws import S3Config, get_s3_client
from src.adapters.db import PostgresDBClient
from src.db.models.agency_models import Agency
from src.db.models.opportunity_models import Opportunity
Expand All @@ -16,6 +21,30 @@

logger = logging.getLogger(__name__)

TESTS_FOLDER = pathlib.Path(__file__).parent.resolve()


def _upload_opportunity_attachments_s3():
s3_config = S3Config()
s3_client = get_s3_client(
s3_config, boto3.Session(aws_access_key_id="NO_CREDS", aws_secret_access_key="NO_CREDS")
)
test_folder_path = TESTS_FOLDER / "opportunity_attachment_test_files"

for root, _, files in os.walk(test_folder_path):
for file in files:
file_path = os.path.join(root, file)
object_name = os.path.relpath(file_path, test_folder_path)

try:
s3_client.upload_file(file_path, s3_config.s3_opportunity_bucket, object_name)
logger.info("Successfully uploaded files")
except ClientError as e:
logger.error(
"Error uploading to s3: %s",
extra={"object_name": object_name, "file_path": file_path, "error": e},
)


def _add_history(
opps: list[Opportunity],
Expand Down Expand Up @@ -173,6 +202,8 @@ def seed_local_db(iterations: int, include_history: bool) -> None:
logger.info("Running seed script for local DB")
error_if_not_local()

_upload_opportunity_attachments_s3()

db_client = PostgresDBClient()

with db_client.get_session() as db_session:
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
import pytest
import requests

from tests.src.api.opportunities_v1.conftest import (
validate_opportunity,
validate_opportunity_with_attachments,
)
from tests.src.db.models.factories import (
CurrentOpportunitySummaryFactory,
OpportunityAttachmentFactory,
OpportunityFactory,
OpportunitySummaryFactory,
)
Expand Down Expand Up @@ -47,7 +49,6 @@ def test_get_opportunity_200(
db_opportunity = OpportunityFactory.create(
**opportunity_params, current_opportunity_summary=None
) # We'll set the current opportunity below

if opportunity_summary_params is not None:
db_opportunity_summary = OpportunitySummaryFactory.create(
**opportunity_summary_params, opportunity=db_opportunity
Expand All @@ -70,8 +71,6 @@ def test_get_opportunity_with_attachment_200(
):
# Create an opportunity with an attachment
opportunity = OpportunityFactory.create()

# Ensure the opportunity is committed to the database
db_session.commit()

# Make the GET request
Expand All @@ -88,6 +87,32 @@ def test_get_opportunity_with_attachment_200(
validate_opportunity_with_attachments(opportunity, response_data)


def test_get_opportunity_s3_endpoint_url_200(
upload_opportunity_attachment_s3, client, api_auth_token, enable_factory_create, db_session
):
# Create an opportunity with a specific attachment
opportunity = OpportunityFactory.create(opportunity_attachments=[])
bucket = "test_bucket"
object_name = "test_file_1.txt"
file_loc = f"s3://{bucket}/{object_name}"
OpportunityAttachmentFactory.create(file_location=file_loc, opportunity=opportunity)

# Make the GET request
resp = client.get(
f"/v1/opportunities/{opportunity.opportunity_id}", headers={"X-Auth": api_auth_token}
)

# Check the response
assert resp.status_code == 200
response_data = resp.get_json()["data"]
presigned_url = response_data["attachments"][0]["download_path"]

# Validate pre-signed url
response = requests.get(presigned_url)
assert response.status_code == 200
assert response.text == "Hello, world"


def test_get_opportunity_404_not_found(client, api_auth_token, truncate_opportunities):
resp = client.get("/v1/opportunities/1", headers={"X-Auth": api_auth_token})
assert resp.status_code == 404
Expand Down
13 changes: 12 additions & 1 deletion api/tests/src/db/models/factories.py
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,14 @@ class CustomProvider(BaseProvider):

YN_YESNO_BOOLEAN_VALUES = ["Y", "N", "Yes", "No"]

OPPORTUNITY_ATTACHMENT_S3_PATHS = [
"s3://local-opportunities/test_file_1.txt",
"s3://local-opportunities/test_file_2.txt",
"s3://local-opportunities/test_file_3.txt",
"s3://local-opportunities/test_file_4.pdf",
"s3://local-opportunities/test_file_5.pdf",
]

def agency(self) -> str:
return self.random_element(self.AGENCIES)

Expand Down Expand Up @@ -188,6 +196,9 @@ def yn_boolean(self) -> str:
def yn_yesno_boolean(self) -> str:
return self.random_element(self.YN_YESNO_BOOLEAN_VALUES)

def s3_file_location(self) -> str:
return self.random_element(self.OPPORTUNITY_ATTACHMENT_S3_PATHS)


fake = faker.Faker()
fake.add_provider(CustomProvider)
Expand Down Expand Up @@ -239,7 +250,7 @@ class OpportunityAttachmentFactory(BaseFactory):
class Meta:
model = opportunity_models.OpportunityAttachment

file_location = factory.Faker("url")
file_location = factory.Faker("s3_file_location")
mime_type = factory.Faker("mime_type")
file_name = factory.Faker("file_name")
file_description = factory.Faker("sentence")
Expand Down
Loading