WIP to fit floci into local dev build - #105
Open
teddmason wants to merge 4 commits into
Open
Conversation
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
There was a problem hiding this comment.
Warning
- Copilot's review of this pull request may be incomplete because some of the changed files are excluded by your Copilot content exclusion settings. See Excluding content from Copilot for details.
Pull request overview
This PR updates the local development Docker setup to replace the LocalStack container with a Floci container and adjusts the supporting scripts accordingly (API Gateway base URL format, hot-reload bucket creation), while also adding a couple of ad-hoc API Gateway/Lambda test scripts.
Changes:
- Switch
docker/infrastructure.ymlfromlocalstackto aflociservice/container. - Update docker helper scripts to use the
/restapis/{id}/{stage}/_user_request_base URL pattern and add creation of ahot-reloadS3 bucket. - Add
docker/scripts/test.shanddocker/scripts/test-teardown.shfor manual LocalStack/Floci API Gateway + Lambda setup/teardown; bump package version in lockfile.
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| package-lock.json | Bumps the package version recorded in the lockfile. |
| docker/scripts/test.sh | Adds a manual script to create a sample Lambda and REST API in LocalStack/Floci. |
| docker/scripts/test-teardown.sh | Adds a manual teardown script for the sample Lambda and REST API. |
| docker/scripts/register-lambda-functions.sh | Updates API Gateway base URL handling and creates the hot-reload bucket for Floci. |
| docker/scripts/register-api-gateway.sh | Renames the REST API and changes deployment/stage creation flow. |
| docker/scripts/load-dummy-data.sh | Updates the API Gateway URL used for posting dummy messages. |
| docker/infrastructure.yml | Replaces the LocalStack service definition with a Floci service definition. |
Files excluded by content exclusion policy (1)
- docker/.env
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+1
to
+6
| # docker compose -f ../../docker/infrastructure.yml -f ../../docker/networks.yml up floci -d | ||
|
|
||
|
|
||
| export AWS_ENDPOINT_URL=http://localhost:4566 | ||
| export AWS_PAGER="" | ||
|
|
Comment on lines
+1
to
+4
|
|
||
| export AWS_ENDPOINT_URL=http://localhost:4566 | ||
| export AWS_PAGER="" | ||
|
|
Comment on lines
+12
to
+14
| API_ID=$(awslocal apigateway get-rest-apis \ | ||
| --query 'items[?name==`My API`].id' --output text \ | ||
| --endpoint-url $AWS_ENDPOINT_URL) |
Comment on lines
6
to
8
| lambda_functions_dir="lib/functions" | ||
| deployed_cpx_agw_url=http://$(awslocal apigateway get-rest-apis | jq -r ".items[0].id").execute-api.localhost.localstack.cloud:4566/local | ||
| deployed_cpx_agw_url=http://localhost:4566/restapis/$(awslocal apigateway get-rest-apis | jq -r ".items[0].id")/local/_user_request_ | ||
|
|
|
|
||
| echo Function URL for archiveMessages is $(awslocal lambda get-function-url-config --function-name archiveMessages | jq -r .FunctionUrl) | ||
| echo API Gateway root URL is http://$(awslocal apigateway get-rest-apis | jq -r ".items[0].id").execute-api.localhost.localstack.cloud:4566/local | ||
| echo API Gateway base URL is http://localhost:4566/restapis/$(awslocal apigateway get-rest-apis | jq -r ".items[0].id")/local/_user_request_ |
Comment on lines
10
to
12
| DATA_FILE="test/lib/functions/data/nws-alert.xml" | ||
| LAMBDA_URL=http://$(awslocal apigateway get-rest-apis | jq -r ".items[0].id").execute-api.localhost.localstack.cloud:4566/local/message | ||
| LAMBDA_URL=http://localhost:4566/restapis/$(awslocal apigateway get-rest-apis | jq -r ".items[0].id")/local/_user_request_/message | ||
|
|
Comment on lines
+41
to
+42
| deployment_id=$(awslocal apigateway create-deployment \ | ||
| --rest-api-id $cap_xml_rest_api_id | jq -r '.id') |
Comment on lines
2
to
6
| floci: | ||
| container_name: "floci-main" | ||
| image: floci-floci:latest | ||
| ports: | ||
| - "127.0.0.1:4566:4566" # LocalStack Gateway |
There was a problem hiding this comment.
🟡 Changes recommended
Local-dev provisioning scripts/docs now invoke aws without --endpoint-url, so they may accidentally target real AWS instead of Floci, which is a significant safety and correctness risk.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Warning
- Copilot's review of this pull request may be incomplete because some of the changed files are excluded by your Copilot content exclusion settings. See Excluding content from Copilot for details.
Review details
Files excluded by content exclusion policy (1)
- docker/.env
Suppressed comments (2)
docker/scripts/register-lambda-functions.sh:7
- The script now calls
aws ...directly, but the AWS CLI does not automatically use anAWS_ENDPOINT_URLenvironment variable; without--endpoint-urlthis can accidentally target real AWS depending on credentials/config. Consider enforcingAWS_ENDPOINT_URLand wrappingawsso every invocation is pinned to the local Floci endpoint.
set -e
lambda_functions_dir="lib/functions"
deployed_cpx_agw_url=http://localhost:4566/restapis/$(aws apigateway get-rest-apis | jq -r ".items[0].id")/local/_user_request_
docker/scripts/load-dummy-data.sh:11
aws apigateway get-rest-apisis used here without--endpoint-url. Since AWS CLI doesn’t honorAWS_ENDPOINT_URLautomatically, this can query real AWS rather than Floci, producing the wrong REST API id (and potentially leaking/mutating real resources).
BASE_AREA="TESTAREA"
DATA_FILE="test/lib/functions/data/nws-alert.xml"
LAMBDA_URL=http://localhost:4566/restapis/$(aws apigateway get-rest-apis | jq -r ".items[0].id")/local/_user_request_/message
- Files reviewed: 21/22 changed files
- Comments generated: 4
- Review effort level: Lite
Comment on lines
+8
to
+12
| echo "Creating API Gateway" | ||
| echo $AWS_ENDPOINT_URL | ||
|
|
||
| cap_xml_rest_api_id=$(awslocal apigateway create-rest-api --name "FWS API Gateway" | jq -r '.id') | ||
| cap_xml_rest_api_root_resource_id=$(awslocal apigateway get-resources --rest-api-id $cap_xml_rest_api_id | jq -r '.items[0].id') | ||
| cap_xml_rest_api_id=$(aws apigateway create-rest-api --name "CPX API Gateway" | jq -r '.id') | ||
| cap_xml_rest_api_root_resource_id=$(aws apigateway get-resources --rest-api-id $cap_xml_rest_api_id | jq -r '.items[0].id') |
Comment on lines
+26
to
30
| From within the development container, use the AWS CLI (configured with the **AWS_ENDPOINT_URL** environment variable pointing at Floci) to retrieve the identifier of the deployed REST API from the API Gateway. For example, the command below can be used when an initial attempt to create a containerised development environment succeeds (resulting in the creation of one REST API instance) | ||
|
|
||
| ```sh | ||
| awslocal apigateway get-rest-apis | jq -r '.items[0].id' | ||
| aws apigateway get-rest-apis | jq -r '.items[0].id' | ||
| ``` |
| * This could be caused by an existing application using the subnet configured for Floci to act as a DNS server (192.168.0.0/24). | ||
| * The configured subnet avoids conflict with Oracle VirtualBox networking. | ||
| * If the configured subnet conflicts with another application that cannot be stopped, try changing the networking configuration in the [development container Docker Compose file](../.devcontainer/devcontainer.yml) to use a different subnet (such as 10.0.2.0/24 as used in [LocalStack network connectivity documentation](https://blog.localstack.cloud/2024-03-04-making-connecting-to-localstack-easier/)), [teardown](../common/teardown.md) existing development container based resources and create a new development container. | ||
| * If the configured subnet conflicts with another application that cannot be stopped, try changing the networking configuration in the [development container Docker Compose file](../.devcontainer/devcontainer.yml) to use a different subnet (such as 10.0.2.0/24), [teardown](../common/teardown.md) existing development container based resources and create a new development container. |
Comment on lines
+14
to
15
| * The AWS CLI (configured with the **AWS_ENDPOINT_URL** environment variable pointing at Floci) is | ||
| used during provisioning and can also be used at runtime. |
|
There was a problem hiding this comment.
🟡 Changes recommended
The API Gateway registration script has a confirmed bug that can create routes under the wrong parent resource, and several scripts/docs hardcode localhost URLs that can break in devcontainer networking.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Warning
- Copilot's review of this pull request may be incomplete because some of the changed files are excluded by your Copilot content exclusion settings. See Excluding content from Copilot for details.
Review details
Files excluded by content exclusion policy (1)
- docker/.env
Suppressed comments (5)
docker/scripts/register-lambda-functions.sh:10
deployed_cpx_agw_urlhardcodeshttp://localhost:4566, which won’t work when these scripts run inside a container (wherelocalhostis the container itself). UseAWS_ENDPOINT_URLas the base so the URL works both on the host and within compose networks (e.g.http://floci-main:4566).
lambda_functions_dir="lib/functions"
deployed_cpx_agw_url=http://localhost:4566/restapis/$(aws --endpoint-url "$AWS_ENDPOINT_URL" apigateway get-rest-apis | jq -r ".items[0].id")/local/_user_request_
docker/scripts/load-dummy-data.sh:13
LAMBDA_URLhardcodeshttp://localhost:4566, which won’t work if the script runs inside a devcontainer/compose network where Floci is reached viafloci-main(or similar). Build the URL fromAWS_ENDPOINT_URLfor portability.
# Constants
BASE_GUID="4eb3b7350ab7aa443650fc9351f02940E"
BASE_AREA="TESTAREA"
DATA_FILE="test/lib/functions/data/nws-alert.xml"
LAMBDA_URL=http://localhost:4566/restapis/$(aws --endpoint-url "$AWS_ENDPOINT_URL" apigateway get-rest-apis | jq -r ".items[0].id")/local/_user_request_/message
docs/local-development/common/running-and-debugging-lambda-functions.md:30
- This section says the AWS CLI is “configured with AWS_ENDPOINT_URL”, but the example command does not pass
--endpoint-url(unlike the provisioning scripts), which risks targeting a real AWS account. Update the example to explicitly pass--endpoint-url "$AWS_ENDPOINT_URL".
From within the development container, use the AWS CLI (configured with the **AWS_ENDPOINT_URL** environment variable pointing at Floci) to retrieve the identifier of the deployed REST API from the API Gateway. For example, the command below can be used when an initial attempt to create a containerised development environment succeeds (resulting in the creation of one REST API instance)
```sh
aws apigateway get-rest-apis | jq -r '.items[0].id'
**docs/local-development/common/running-and-debugging-lambda-functions.md:61**
* This curl example hardcodes `http://localhost:4566`, which can be incorrect when running inside a container network. Prefer `$AWS_ENDPOINT_URL` so it matches the configured Floci endpoint.
curl "http://localhost:4566/restapis/<<REST-API-ID>>/local/_user_request_/message/<<MESSAGE-ID>>"**docs/local-development/common/running-and-debugging-lambda-functions.md:90**
* This POST example hardcodes `http://localhost:4566`; using `$AWS_ENDPOINT_URL` makes it work regardless of whether Floci is accessed via localhost or a compose service hostname.
curl -H "Content-Type: text/xml" -d "@<</path/to/message/file>>" "http://localhost:4566/restapis/<<REST-API-ID>>/local/_user_request_/message"
- **Files reviewed:** 22/23 changed files
- **Comments generated:** 4
- **Review effort level:** Lite
</details>
Comment on lines
+13
to
15
| cap_xml_rest_api_id=$(aws --endpoint-url "$AWS_ENDPOINT_URL" apigateway create-rest-api --name "CPX API Gateway" | jq -r '.id') | ||
| cap_xml_rest_api_root_resource_id=$(aws --endpoint-url "$AWS_ENDPOINT_URL" apigateway get-resources --rest-api-id $cap_xml_rest_api_id | jq -r '.items[0].id') | ||
| lambda_functions_dir="lib/functions" |
Comment on lines
103
to
105
| create_resource() { | ||
| cap_xml_rest_api_root_resource_id=$1 | ||
| cap_xml_rest_api_path_part=$2 |
Comment on lines
+73
to
+74
| echo Function URL for archiveMessages is $(aws --endpoint-url "$AWS_ENDPOINT_URL" lambda get-function-url-config --function-name archiveMessages | jq -r .FunctionUrl) | ||
| echo API Gateway base URL is http://localhost:4566/restapis/$(aws --endpoint-url "$AWS_ENDPOINT_URL" apigateway get-rest-apis | jq -r ".items[0].id")/local/_user_request_ |
Comment on lines
51
to
53
| ```sh | ||
| curl "http://<<REST-API-ID>>.execute-api.localhost.localstack.cloud:4566/local/messages.atom" | ||
| curl "http://localhost:4566/restapis/<<REST-API-ID>>/local/_user_request_/messages.atom" | ||
| ``` |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.