Skip to content

Refactor files related to AWS deployment - #362

Closed
cherriechang wants to merge 47 commits into
mainfrom
refactorAWS
Closed

Refactor files related to AWS deployment#362
cherriechang wants to merge 47 commits into
mainfrom
refactorAWS

Conversation

@cherriechang

Copy link
Copy Markdown
Contributor

This should be more easily merged into main after fixAWS branch is merged.

jkhartshorne and others added 30 commits November 17, 2023 17:03
…d against null distributionList.Items (line 671);
- Check for existing OAC by name before creating new one
- Reuse existing OAC if found, avoiding duplicate creation error
- Add 'Enter a custom domain/subdomain' option to domain selection
- Allow users to input subdomains not in Route53 registered domains list

Fixes deployment errors:
- OriginAccessControlAlreadyExists when OAC exists but not in awsResources
- No way to specify subdomains during interactive deployment
…in DNS fallback, and database config standardization
@changeset-bot

changeset-bot Bot commented Oct 16, 2025

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 40e3513

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

// Enable SSL for AWS RDS or non-localhost connections
ssl:
(
dbInfo.url.includes(".rds.amazonaws.com") ||

Check failure

Code scanning / CodeQL

Incomplete URL substring sanitization High

'
.rds.amazonaws.com
' can be anywhere in the URL, and arbitrary hosts may come before or after it.

Copilot Autofix

AI 10 months ago

The best fix is to parse the host portion of dbInfo.url using a reliable URL parsing library (preferably Node's built-in url module or the WHATWG URL API), and then check if the hostname ends with .rds.amazonaws.com (or matches it exactly). This should replace the substring test. To implement this, import the URL constructor from the standard library (no external dependency required), parse dbInfo.url, and update the conditional to examine only the host portion. Ensure compatibility with both full URLs and bare hostnames (if the field is not always in URL format). You may need a fallback for cases where the value is not a valid URL: if only a hostname is provided ("localhost" or some DNS name), treat it as the host. Edit only the relevant section in the function buildKnexConfig in packages/pushkin-cli/src/commands/setupdb/index.js.

Suggested changeset 1
packages/pushkin-cli/src/commands/setupdb/index.js

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/packages/pushkin-cli/src/commands/setupdb/index.js b/packages/pushkin-cli/src/commands/setupdb/index.js
--- a/packages/pushkin-cli/src/commands/setupdb/index.js
+++ b/packages/pushkin-cli/src/commands/setupdb/index.js
@@ -1,4 +1,6 @@
 import path from "path";
+// Added below import to access the WHATWG URL constructor
+import { URL } from "url";
 import fs from "graceful-fs";
 import jsYaml from "js-yaml";
 import knex from "knex";
@@ -226,7 +228,18 @@
       // Enable SSL for AWS RDS or non-localhost connections
       ssl:
         (
-          dbInfo.url.includes(".rds.amazonaws.com") ||
+          (() => {
+            let host;
+            try {
+              // If dbInfo.url is a full URL, parse it
+              host = new URL(dbInfo.url).hostname;
+            } catch (e) {
+              // If not, assume it is a host name
+              host = dbInfo.url;
+            }
+            return (host.endsWith(".rds.amazonaws.com") || host === "rds.amazonaws.com");
+          })()
+          ||
           (dbInfo.url !== "localhost" && !dbInfo.url.includes("localhost"))
         ) ?
           { rejectUnauthorized: false }
EOF
@@ -1,4 +1,6 @@
import path from "path";
// Added below import to access the WHATWG URL constructor
import { URL } from "url";
import fs from "graceful-fs";
import jsYaml from "js-yaml";
import knex from "knex";
@@ -226,7 +228,18 @@
// Enable SSL for AWS RDS or non-localhost connections
ssl:
(
dbInfo.url.includes(".rds.amazonaws.com") ||
(() => {
let host;
try {
// If dbInfo.url is a full URL, parse it
host = new URL(dbInfo.url).hostname;
} catch (e) {
// If not, assume it is a host name
host = dbInfo.url;
}
return (host.endsWith(".rds.amazonaws.com") || host === "rds.amazonaws.com");
})()
||
(dbInfo.url !== "localhost" && !dbInfo.url.includes("localhost"))
) ?
{ rejectUnauthorized: false }
Copilot is powered by AI and may make mistakes. Always verify output.
Add two foundational utilities for AWS deployment refactoring:

AWS Client Factory (Phase 1.1):
- Centralized factory for creating AWS SDK v3 clients
- Handles both string and object profile formats (backward compatible)
- Single configuration point for all AWS clients
- Easy to mock for testing
- Supports 14 AWS service clients (RDS, S3, ECS, etc.)

Retry Utility (Phase 1.2):
- Generic retry logic with exponential backoff
- Three strategies: exponential backoff, constant delay, immediate
- Configurable maxRetries, delays, and shouldRetry logic
- Handles common AWS throttling errors automatically
- Can replace hardcoded retry patterns throughout codebase

Testing:
- Full unit test coverage for both utilities
- Verified with real AWS credentials
- Successfully compiled with Babel

Impact:
- Updated checkIAMUser() to use new factory (proof of concept)
- Lays groundwork for extracting service modules
- Zero breaking changes to existing functionality

Related: Part of 7-week AWS refactoring plan to improve maintainability,
testability, and reduce 5000+ line monolithic file to modular architecture.
…(idempotent), only create new when it's missing
@cherriechang
cherriechang deleted the refactorAWS branch November 24, 2025 23:31
@cherriechang

Copy link
Copy Markdown
Contributor Author

closed in favor of #374 after renaming branch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants