feat: add configuration for deploying a cloudfront distribution for the veda-backend #229

smohiudd · 2023-09-25T16:07:27Z

Include Cloudfront Distribution construct in veda-back which should help with the STAC Browser construct feature.

One way to address the API Gateway url not resolving was to use the restapi_id in the url pattern: https://{restapi_id}.execute-api.{region}.amazonaws.com/ to access both the stac and raster apis.

This branch has been test deployed as the path-prefix-veda-backend stack using the delta-backend.xyz domain.

emileten

Looks good to me, do have an important broader question regarding our usage of CloudFront though

emileten · 2023-09-27T06:41:23Z

routes/infrastructure/construct.py

+            bucket_name=veda_route_settings.stac_browser_bucket,
+        )
+
+        no_cache_policy = cf.CachePolicy(


Can we use the CachingDisabled managed policy instead ? Looks pretty similar ?

Longer note :

So we're turning off any caching Cloud Front is doing with that policy. We've actually been doing that on MAAP as well (albeit manually on the console, so I don't have any code to link), after noticing that STAC metadata wasn't getting updated frequently enough on the STAC browser.

However, I'm still puzzled that I am doing this because, isn't the point of CloudFront to cache the content in the closest edge point to the user and decrease latency thanks to that ? If we turn that off entirely, why are we using CloudFront ?

In the first place I remember that we started to use CloudFront because that's what the AWS docs tell us to do for serving static content stored in S3 over HTTPs. However, is the CloudFront layer really needed for this, can't we just create a route53 record that directly points to the bucket ?

Note : those are mainly questions, I am not suggesting we stop using CloudFront

emileten · 2023-09-27T06:43:30Z

routes/infrastructure/construct.py

+            else None
+        )
+
+        self.distribution = cf.Distribution(


Am I right that with this set of behaviors, the mapping is going to be, assuming the domain is dev-stac.delta-backend.com :

https://dev-stac.delta-backend.com -> browser

https://dev-stac.delta-backend.com/api/stac STAC API

https://dev-stac.delta-backend.com/api/raster raster API

emileten · 2023-09-27T06:47:18Z

app.py

+    veda_stack,
+    "routes",
+    raster_api_id=raster_api.raster_api.api_id,
+    stac_api_id=stac_api.stac_api.api_id,


One way to address the API Gateway url not resolving was to use the restapi_id in the url pattern: https://{restapi_id}.execute-api.{region}.amazonaws.com/ to access both the stac and raster apis.

😱 very nice trick. Spent so much time finding a workaround for this.

ividito

Going to spend a bit more time looking at this today (particularly looking at how the ingest API is working with the CF distribution in this stack). The failing action will pass if the environment secret is updated with the ingest_url and stac_browser_bucket values.

smohiudd · 2023-09-27T14:10:08Z

@ividito Also the routes construct isn't configured as as optional (similar to how the domain construct works). So the ingest_url and stac_browser_bucket configs need to be optional.

anayeaye

just a couple comments on the config

anayeaye · 2023-09-28T16:35:00Z

routes/infrastructure/config.py

+        description="Certificate’s ARN",
+    )
+
+    using_mcp_acct: Optional[bool] = False


I think using_mcp_acct is not used? Also, not sure about the intent but possibly we could be inferring this info from whether or not VEDA_PERMISSIONS_BOUNDARY_POLICY_NAME is configured?

What was an old config when I copied over Slesa's original cloudfront code. It's not used anywhere, can be removed.

anayeaye · 2023-09-28T17:09:13Z

routes/infrastructure/config.py

@@ -0,0 +1,52 @@
+"""Settings for Cloudfront distribution - any environment variables starting with


Can you add a line to the readme advanced config about this? (sorry if this is a repeat comment)

slesaad · 2023-10-02T21:41:12Z

One way to address the API Gateway url not resolving was to use the restapi_id in the url pattern: https://{restapi_id}.execute-api.{region}.amazonaws.com/ to access both the stac and raster apis.

I'm kinda surprised that this is the change that made it work. We have the same setup in ghgc, but it still doesn't resolve the urls 🤔

http://staging.ghg.center/api/stac :(

slesaad · 2023-10-02T21:50:41Z

routes/infrastructure/construct.py

+            no_cache_policy = cf.CachePolicy(
+                self,
+                "no-cache-policy",
+                default_ttl=Duration.seconds(0),
+                min_ttl=Duration.seconds(0),
+                max_ttl=Duration.seconds(0),
+            )


there's already no cache policy defined in cdk,

cf.CachePolicy.CACHING_DISABLED

smohiudd · 2023-10-02T21:53:54Z

@slesaad I did some experimentation last week and realized that for the cloudfront stac api to work you need the trailing slash... http://staging.ghg.center/api/stac/

Without the trailing slash you get 307 temporary redirect the goes to api gateway url

slesaad · 2023-10-03T14:21:25Z

@smohiudd yeah, but the URLs in the catalogs are still the same api gateway urls

smohiudd · 2023-10-05T04:53:23Z

@slesaad the issue with that is cloudfront is not able to forward the host header to api gateway. I tried another way which seems to work using a load balancer in this PR: #232

pydocstyle fixes format isort changes optional config format config changes format config changes construct change

smohiudd · 2023-10-05T19:15:10Z

Thanks @vincentsarago! I got the host header working in APIgateway using the parameter mapping. Doesn't look like we need to go down the ALB route.

anayeaye · 2023-10-05T20:55:42Z

routes/infrastructure/config.py

+        description="Boolean if Cloudfront Distribution should be deployed",
+    )
+
+    # STAC S#3 browser bucket name


this is the name of an existing bucket?

why is the default an empty string instead of None (same for ingest url below)

should we add addition info in description to explain that an existing s3 bucket is required if using cloudfront

also: typo in the comment S#3

Yes, for now it's the name of an existing bucket. But that may change depending on how the stac browser construct @emileten is working on is set up

Yes you're right they should None

Agreed

anayeaye · 2023-10-05T21:05:50Z

stac_api/infrastructure/config.py

    class Config:
        """model config"""

        env_file = ".env"
-        env_prefix = "VEDA_STAC_"
+        env_prefix = "VEDA_"


I think this is a breaking change because it causes existing implementations to update the secrets stored in the secrets manager? I think we should address changing <SCOPE>_VARNAME across all constructs if we want to break the pattern (especially because it could be useful to share a variable that should be the same in the scopes of different constructs).

Is the change needed now, though, or can we keep scoping our environment variables?

looks like staging has no VEDA_STAC_ vars in the secrets store so this comment can be downgraded to a nit

anayeaye · 2023-10-05T21:16:41Z

stac_api/infrastructure/construct.py

+                        if veda_stac_settings.domain_hosted_zone_name
+                        else None,
+                    )
+                ),
            )


this is awesome + I wonder if the prefix can be managed here also (in the future)?

Question: I need to try this out but to me it seems that if domain_hosted_zone_name is None, then "host" will be overwritten with None? Or does it just end up being a no-op?

I wanted to make it like a no-op if hosted zone wasn't present. I can double check if that's what it's doing

vincentsarago · 2023-10-06T12:41:13Z

@smohiudd Glad it's working 🙏 , sorry for not seeing this issue earlier 😭

smohiudd · 2023-10-06T15:19:58Z

routes/infrastructure/construct.py

+            )
+
+            # Certificate must be in zone us-east-1
+            domain_cert = (


I believe the certificate used in cloudfront must be in the us-east-1 region. Right now the arn is being passed manually. Should we be creating a new certificate in us-east-1 instead.

slesaad · 2023-10-09T14:32:45Z

stac_api/infrastructure/construct.py

+                construct_id,
+                handler=lambda_function,
+                parameter_mapping=(
+                    aws_apigatewayv2_alpha.ParameterMapping().overwrite_header(
+                        "host",
+                        aws_apigatewayv2_alpha.MappingValue(
+                            veda_stac_settings.domain_hosted_zone_name
+                        )
+                        if veda_stac_settings.domain_hosted_zone_name
+                        else None,
+                    )
+                ),


@smohiudd I realized we'd want to do this to the raster API too, because otherwise when we use /mosaic/register, it gives back the API gateway URL

Thanks @slesaad have you tested that on GHGC yet?

yes! - https://github.com/US-GHG-Center/ghgc-backend/pull/46/files
deployed to dev.ghg.center/api/raster

anayeaye · 2023-10-11T17:38:41Z

raster_api/infrastructure/construct.py

+                handler=veda_raster_function,
+                parameter_mapping=(
+                    aws_apigatewayv2_alpha.ParameterMapping().overwrite_header(
+                        "host",


At least temporarily, I think we need to make this assignment conditional based on whether or not cloudfront is used. It appears that the staging APIs will also be overwritten. Same for the stac api

[~] AWS::ApiGatewayV2::Integration raster-api/veda-backend-uah-staging-raster-api/DefaultRoute/raster-api rasterapivedabackenduahstagingrasterapiDefaultRouterasterapi20F21B19 └─ [+] RequestParameters └─ {"overwrite:header.host":"delta-backend.com"}

I added a conditional for the parameter mapping

vincentsarago · 2023-10-23T19:19:39Z

@smohiudd @anayeaye developmentseed/titiler#716 might be of interest. Basically when using an API Gateway prefix, it seems that you'll need to set a root_path in mangum but also in the fastAPI application as well https://github.com/developmentseed/titiler/blob/main/src/titiler/application/titiler/application/main.py#L65

# What - When a custom host is configured, overwrite the host in the api integration (bring back the parameter mapping method introduced in #229) - Include origin_path in cloudfront ingest api behavior. Note: this path will not work until the root_path is changed to `api/ingest` in the ingest api - Domain construct now only generates custom subdomains for endpoints when configured (existing stacks must provide new configuration variable). _Note: indentation changes that wrap all subdomain constructs make the change appear larger than they really are. The change removes an unused shared subdomain construct and only generates api-specific subdomains when create subdomains configuration is true._ ## Environment variable changes ### New `VEDA_CUSTOM_DOMAIN` optional configuration to provide a complete url for host overrides in lambda APIs (i.e. `dev.somedomain.com`) ### Removed `VEDA_DOMAIN_SHARED_SUBDOMAIN` is no longer used (logic is managed with custom subdomains config below) ### New+Breaking `VEDA_DOMAIN_CREATE_CUSTOM_SUBDOMAINS` boolean defaults false. Existing deployments with custom subdomains for each raster and stac api endpoints need to set this variable to TRUE.

…cloudfront (#244) # What - adds optional configuration to deploy all endpoints with a custom root path (i.e. `/api/stac` and `/api/raster`) #229, #241 - adds support for using a cloudfront as a reverse proxy #229, #241, #245 - titiler-pgstac upgraded from 0.2.3 to 0.8.0 #239 - titiler upgrade for custom colormap configuration #243 # How tested A temporary test stack was deployed to confirm that raster-api works as expected and that the two-subdomain staging-stac and staging-raster pattern is still supported. The [pre-deploy diff action for this pr](https://github.com/NASA-IMPACT/veda-backend/actions/runs/6791564314/job/18463354484#step:11:919) confirms that the raster API lambda will be upgraded and no domain name changes will be caused.

cloudfront in veda backend

d653734

smohiudd requested review from ividito, anayeaye and emileten September 25, 2023 16:07

format

30223d9

smohiudd marked this pull request as ready for review September 25, 2023 16:09

smohiudd changed the title ~~Include Cloudfront Distribution Construct~~ feat: Add configuration for deploying a cloudfront distribution for the veda-backend Sep 25, 2023

smohiudd changed the title ~~feat: Add configuration for deploying a cloudfront distribution for the veda-backend~~ feat: add configuration for deploying a cloudfront distribution for the veda-backend Sep 25, 2023

emileten approved these changes Sep 27, 2023

View reviewed changes

ividito reviewed Sep 27, 2023

View reviewed changes

anayeaye reviewed Sep 28, 2023

View reviewed changes

slesaad reviewed Oct 2, 2023

View reviewed changes

smohiudd added 2 commits October 5, 2023 12:55

mypy, pydocstyle fixes

41e011d

pydocstyle fixes format isort changes optional config format config changes format config changes construct change

include apigateway parameter mapping

dccaf49

smohiudd force-pushed the feature/cloudfront-stac-browser branch from ead8e14 to dccaf49 Compare October 5, 2023 19:01

remove unused import

30985be

smohiudd mentioned this pull request Oct 5, 2023

feat: add load balancer and configuration for deploying cloudfront distribution #232

Closed

smohiudd added 2 commits October 5, 2023 13:19

update readme

5b02663

remove unused config

1e222d8

anayeaye reviewed Oct 5, 2023

View reviewed changes

smohiudd commented Oct 6, 2023

View reviewed changes

slesaad reviewed Oct 9, 2023

View reviewed changes

include parameter mapping in raster api

8cf092c

anayeaye requested changes Oct 11, 2023

View reviewed changes

smohiudd added 2 commits October 12, 2023 10:28

include conditional param mapping

7924a6d

change param mapping kwargs

dd1d675

anayeaye approved these changes Oct 12, 2023

View reviewed changes

smohiudd merged commit 8965651 into develop Oct 12, 2023

This was referenced Nov 6, 2023

feat!: titiler upgrade, optional custom base path for apis, optional cloudfront #244

Merged

fix(apis)!: overwrite api host when provided in config #245

Merged

anayeaye deleted the feature/cloudfront-stac-browser branch March 12, 2024 21:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add configuration for deploying a cloudfront distribution for the veda-backend #229

feat: add configuration for deploying a cloudfront distribution for the veda-backend #229

smohiudd commented Sep 25, 2023 •

edited

Loading

emileten left a comment

emileten Sep 27, 2023

emileten Sep 27, 2023

emileten Sep 27, 2023 •

edited

Loading

ividito left a comment

smohiudd commented Sep 27, 2023

anayeaye left a comment

anayeaye Sep 28, 2023

smohiudd Sep 28, 2023

anayeaye Sep 28, 2023

smohiudd Oct 5, 2023

slesaad commented Oct 2, 2023

slesaad Oct 2, 2023

smohiudd Oct 5, 2023

smohiudd commented Oct 2, 2023

slesaad commented Oct 3, 2023

smohiudd commented Oct 5, 2023

smohiudd commented Oct 5, 2023 •

edited

Loading

anayeaye Oct 5, 2023

smohiudd Oct 5, 2023

anayeaye Oct 5, 2023

anayeaye Oct 5, 2023

anayeaye Oct 5, 2023

smohiudd Oct 5, 2023

vincentsarago commented Oct 6, 2023

smohiudd Oct 6, 2023

slesaad Oct 9, 2023

smohiudd Oct 10, 2023

slesaad Oct 10, 2023

anayeaye Oct 11, 2023

smohiudd Oct 12, 2023

vincentsarago commented Oct 23, 2023

		@@ -0,0 +1,52 @@
		"""Settings for Cloudfront distribution - any environment variables starting with

feat: add configuration for deploying a cloudfront distribution for the veda-backend #229

feat: add configuration for deploying a cloudfront distribution for the veda-backend #229

Conversation

smohiudd commented Sep 25, 2023 • edited Loading

emileten left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

emileten Sep 27, 2023 • edited Loading

Choose a reason for hiding this comment

ividito left a comment

Choose a reason for hiding this comment

smohiudd commented Sep 27, 2023

anayeaye left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

slesaad commented Oct 2, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

smohiudd commented Oct 2, 2023

slesaad commented Oct 3, 2023

smohiudd commented Oct 5, 2023

smohiudd commented Oct 5, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vincentsarago commented Oct 6, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vincentsarago commented Oct 23, 2023

smohiudd commented Sep 25, 2023 •

edited

Loading

emileten Sep 27, 2023 •

edited

Loading

smohiudd commented Oct 5, 2023 •

edited

Loading