Skip to content

Commit

Permalink
[Issue 2512] S3 Presign URL (#2563)
Browse files Browse the repository at this point in the history
## Summary
Fixes #{[2512](#2512)}

### Time to review: __20 mins__

## Changes proposed
Added test files for opportunity attachment
Updated local seed db with func to upload test files to S3
Updated factories to set opportunity attachment "file_location" from any
value in static list
Updated the response schema to return new field `download_path` which is
the link to the pre-signed url
Added  default Expiration limit for the s3 url in s3 config
Added reusable function to get aws  boto3 session for local development
Added new env variable for local development
Added fixture to upload test files 
Added unit test. Tests if we can successfully download s3 file.

## Context for reviewers
Return a pre-signed url for an attachment a user can then use to
download the s3 file

## Additional information
> Screenshots, GIF demos, code examples or output to help show the
changes working as expected.

---------

Co-authored-by: nava-platform-bot <[email protected]>
  • Loading branch information
babebe and nava-platform-bot authored Oct 30, 2024
1 parent 17321cf commit f1fa121
Show file tree
Hide file tree
Showing 17 changed files with 152 additions and 10 deletions.
3 changes: 3 additions & 0 deletions api/local.env
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,9 @@ S3_ENDPOINT_URL=http://localstack:4566

S3_OPPORTUNITY_BUCKET=local-opportunities

# This env var is used to set local AWS credentials
IS_LOCAL_AWS=1

############################
# Feature Flags
############################
Expand Down
2 changes: 1 addition & 1 deletion api/openapi.generated.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2036,7 +2036,7 @@ components:
OpportunityAttachmentV1:
type: object
properties:
file_location:
download_path:
type: string
description: The URL to download the attachment
example: https://...
Expand Down
3 changes: 2 additions & 1 deletion api/src/adapters/aws/__init__.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
from .aws_session import get_boto_session
from .s3_adapter import S3Config, get_s3_client

__all__ = ["get_s3_client", "S3Config"]
__all__ = ["get_s3_client", "S3Config", "get_boto_session"]
11 changes: 11 additions & 0 deletions api/src/adapters/aws/aws_session.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import os

import boto3


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()
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
2 changes: 1 addition & 1 deletion api/src/api/opportunities_v1/opportunity_schemas.py
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,7 @@ class OpportunityV1Schema(Schema):


class OpportunityAttachmentV1Schema(Schema):
file_location = fields.String(
download_path = fields.String(
metadata={
"description": "The URL to download the attachment",
"example": "https://...",
Expand Down
37 changes: 35 additions & 2 deletions api/src/services/opportunities_v1/get_opportunity.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,11 @@

import src.adapters.db as db
import src.util.datetime_util as datetime_util
from src.adapters.aws import S3Config, get_boto_session, 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 +37,39 @@ def _fetch_opportunity(
return opportunity


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,
)
if s3_config.s3_endpoint_url:
# Only relevant when local, due to docker path issues
pre_sign_file_loc = pre_sign_file_loc.replace(
s3_config.s3_endpoint_url, "http://localhost:4566"
)

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
1 change: 0 additions & 1 deletion api/tests/src/api/opportunities_v1/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,6 @@ def validate_opportunity_attachments(

assert len(db_attachments) == len(resp_attachments)
for db_attachment, resp_attachment in zip(db_attachments, resp_attachments, strict=True):
assert db_attachment.file_location == resp_attachment["file_location"]
assert db_attachment.mime_type == resp_attachment["mime_type"]
assert db_attachment.file_name == resp_attachment["file_name"]
assert db_attachment.file_description == resp_attachment["file_description"]
Expand Down
32 changes: 29 additions & 3 deletions api/tests/src/api/opportunities_v1/test_opportunity_route_get.py
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,33 @@ 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"]
print("response_data", response_data)
presigned_url = response_data["attachments"][0]["download_path"]

# Validate pre-signed url
response = requests.get(presigned_url, timeout=5)
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

0 comments on commit f1fa121

Please sign in to comment.