diff --git a/README.md b/README.md index b09c460..492e785 100644 --- a/README.md +++ b/README.md @@ -29,6 +29,7 @@ Engineering conventions and best practices for Sea Haven Industries. - [Git Hooks](hooks/) -- shim for the global security pre-push hook - [Scripts](scripts/) -- repo provisioning, automation tooling - [CDK Constructs](constructs/) -- shared VPN EC2 instance construct and other reusable patterns +- [Confluence Export](confluence/) -- sanitized standards pages for partner teams ## Contributing diff --git a/confluence/00-engineering-standards.md b/confluence/00-engineering-standards.md new file mode 100644 index 0000000..3d36c04 --- /dev/null +++ b/confluence/00-engineering-standards.md @@ -0,0 +1,30 @@ +# Engineering Standards + +These pages describe how we build, review, and ship software at Sea Haven Industries. They apply to everyone who contributes code, including partner and contract teams. + +## Pages + +| Page | Covers | +|---|---| +| Naming Conventions | How to name repos, branches, and cloud resources | +| Git Workflow | Branches, commits, versioning, and push rules | +| Pull Requests and Code Review | PR titles and descriptions, review categories, merging | +| Issue Tracking | Jira projects and the ticket template | +| Secrets and Configuration | Where sensitive and non-sensitive values live | +| AWS Infrastructure | Infrastructure as code, Lambda defaults, tagging | +| CI/CD and Deployments | Pipelines, environments, releases, and rollback | + +## Core rules + +If you read nothing else, follow these: + +1. Use kebab-case for every name. +2. Never commit to `main` directly. Every change goes through a pull request. +3. Every PR title ends with its Jira key, for example `feat(api): add receipt search (DEV-123)`. +4. Never put secrets in code, environment variables, `.env` files in git, tickets, chat, or wiki pages. +5. Every AWS resource is created by infrastructure as code. Nothing is built by hand in the console. +6. Nobody deploys from a laptop. The pipeline deploys. + +## Questions and changes + +Ask your Sea Haven point of contact. To propose a change to these standards, open a Jira ticket in the matching project (see Issue Tracking). diff --git a/confluence/01-naming-conventions.md b/confluence/01-naming-conventions.md new file mode 100644 index 0000000..5cb38ea --- /dev/null +++ b/confluence/01-naming-conventions.md @@ -0,0 +1,43 @@ +# Naming Conventions + +## The rule + +Use kebab-case for everything: lowercase words separated by hyphens. Do not use `snake_case`, `PascalCase`, `camelCase`, or mixed styles. + +## Where it applies + +| Resource | Example | +|---|---| +| Repository | `expense-approval-bot` | +| Stack or workspace name | `expense-approval-bot` (matches the repo name) | +| Lambda function | `expense-approval-bot-process-receipt` | +| DynamoDB table | `expense-approval-bot-receipts` | +| S3 bucket | `expense-approval-bot-uploads` | +| Secrets Manager secret | `expense-approval-bot/slack-signing` | +| Branch | `feature/add-receipt-parser` | +| Workflow file | `deploy-api.yaml` | + +Resource names start with the repo or stack name so ownership is obvious. + +## Examples + +| Bad | Good | Why | +|---|---|---| +| `ExpenseApprovalBot` | `expense-approval-bot` | PascalCase | +| `expense_approval_bot` | `expense-approval-bot` | snake_case | +| `expenseApprovalBot` | `expense-approval-bot` | camelCase | +| `feature/AddParser` | `feature/add-parser` | PascalCase in branch | + +## Existing names + +A few older resources do not follow this convention. Leave them as they are: renaming them would force a rebuild of production resources. Everything new must use kebab-case. + +## CDK note + +CDK generates PascalCase stack names by default. Always set `stackName` explicitly: + +```typescript +new MyStack(app, 'MyStack', { + stackName: 'my-stack', +}); +``` diff --git a/confluence/02-git-workflow.md b/confluence/02-git-workflow.md new file mode 100644 index 0000000..7c858c8 --- /dev/null +++ b/confluence/02-git-workflow.md @@ -0,0 +1,94 @@ +# Git Workflow + +## Branches + +Never commit directly to `main`. Create a branch using one of these prefixes plus a kebab-case description: + +| Prefix | Use when | +|---|---| +| `feature/` | Adding or enhancing functionality | +| `fix/` | Fixing a defect before it reaches production | +| `hotfix/` | Fixing a production issue that needs immediate attention | +| `chore/` | Maintenance, cleanup, dependency bumps, tooling | +| `docs/` | Documentation-only changes | +| `refactor/` | Restructuring code without changing behavior | +| `release/` | Preparing a release | + +Example: `feature/add-receipt-parser`. + +Do not put the Jira key in the branch name. It goes in the PR title (see Pull Requests and Code Review). + +Delete your branch after it merges. + +## Commits + +Commit each logical change on its own. We use [Conventional Commits](https://www.conventionalcommits.org/): + +``` +type(scope): description +``` + +- **type** (required): one of the types below +- **scope** (optional): the area affected, lowercase, such as `auth`, `api`, or `deps` +- **description** (required): imperative mood, lowercase, no trailing period + +| Type | Use for | +|---|---| +| `feat` | A new feature | +| `fix` | A bug fix | +| `docs` | Documentation only | +| `style` | Formatting with no behavior change | +| `refactor` | Code change that is neither a fix nor a feature | +| `perf` | Performance improvement | +| `test` | Adding or correcting tests | +| `build` | Build system or dependency changes | +| `ci` | CI/CD configuration | +| `chore` | Routine maintenance | +| `revert` | Reverting a previous commit | +| `release` | Cutting a release | + +### Commit message rules + +- Keep the header under 72 characters. +- Write what the commit does: `add retry logic`, not `added retry logic`. +- Use the body to explain why. The diff already shows what. +- No vague messages: `fix: stuff`, `chore: update code`, and `chore: address review comments` are not acceptable. +- No `WIP` commits on shared branches. + +Mark breaking changes with `!` and a `BREAKING CHANGE:` footer: + +``` +feat(api)!: remove the deprecated /v1/receipts endpoint + +BREAKING CHANGE: clients must migrate to /v2/receipts. +``` + +Optionally reference the Jira ticket in a trailer: + +``` +feat(payments): add retry logic for transient upstream failures + +The payment processor returns 503 during its deployments. Without +retries these surface as user-facing errors. This adds exponential +backoff with 3 attempts. + +Refs: DEV-123 +``` + +## Push rules + +- Push regularly. Do not sit on unpushed work. +- Never force-push `main`, and never rewrite commits that are already on `main`. +- Never bypass git hooks with `--no-verify`. + +## Versioning + +We use [Semantic Versioning](https://semver.org/): `MAJOR.MINOR.PATCH`. + +| Increment | When | +|---|---| +| `MAJOR` | Breaking changes to an API or contract | +| `MINOR` | New backwards-compatible functionality | +| `PATCH` | Backwards-compatible fixes and dependency updates | + +New projects start at `v0.1.0` and move to `v1.0.0` when the interface is stable. How releases are cut is described in CI/CD and Deployments. diff --git a/confluence/03-pull-requests-and-code-review.md b/confluence/03-pull-requests-and-code-review.md new file mode 100644 index 0000000..aa7d124 --- /dev/null +++ b/confluence/03-pull-requests-and-code-review.md @@ -0,0 +1,105 @@ +# Pull Requests and Code Review + +## Scope + +One PR is one logical change. If the title needs the word "and", split it. + +| Good scope | Too broad | +|---|---| +| Add receipt parser Lambda | Add receipt parser and refactor auth middleware | +| Fix timeout in payment processor | Fix timeout and update dependencies | + +## Title + +Use the commit format plus the Jira key at the end: + +``` +type(scope): description (DEV-123) +``` + +- Valid Jira keys start with `DEV`, `PLAT`, or `SEC`. +- Keep the title at or under 120 characters. +- Describe the change, not the ticket. + +| Good | Bad | +|---|---| +| `feat(parser): add retry logic for upstream failures (DEV-42)` | `DEV-42` | +| `fix(auth): correct null check in session handler (PLAT-7)` | `Bug fix` | + +Only automated dependency-update PRs may leave out the Jira key. + +## Description + +Use exactly these four sections, in this order. If a section has nothing to say, write `None.` instead of deleting it. + +```markdown +## Summary +What changed and why, in 1-3 sentences. + +## Validation +How you verified it works: steps, commands, screenshots for UI changes. + +## Tests +Tests added, updated, or run. If there are no automated tests, describe the manual testing. + +## Notes +Deploy order, migrations, follow-ups, breaking changes. Otherwise `None.` +``` + +State facts a reviewer can check. Do not add AI-tool attribution footers. + +## When to open a PR + +Open a PR when the work is ready for review and you have verified it works. Do not use PRs to park unfinished work. + +## Reviewing + +### Always check + +- Does the change do what the description says? +- Are there bugs, missed edge cases, or gaps in error handling? +- Are naming and secrets handling correct? +- Are there hardcoded values that belong in configuration? +- Are IAM permissions as narrow as they can be? Is input validated at the boundaries? +- Is the README updated when behavior changed? +- Are tests appropriate for the change? + +### Do not nitpick + +- Formatting that linters handle +- Personal style preferences +- Reasonable names you would have chosen differently + +### Finding categories + +Label each review comment with a category: + +| Category | Meaning | Author must act? | +|---|---|---| +| BLOCK | Must fix before merge | Yes, the PR cannot merge | +| FIX | Strongly recommended | Yes, unless deferred to a ticket | +| NIT | Optional improvement | No, author's choice | +| QUESTION | Reviewer needs clarification | Yes, answer it | + +### Giving feedback + +- Be specific. "This fails when the list is empty" helps. "Needs work" does not. +- Ask rather than assume: "Is this intentional?" +- If the PR is good, say so. + +### Deferred findings + +If a finding will not be fixed in the current PR, the author files a Jira ticket before merging, links it from the review comment, and references the PR in the ticket. This applies to every finding category. + +### Turnaround + +- Review within one business day of the request. +- If you cannot, say so and suggest another reviewer. +- One approval is enough for most changes. + +## Merging + +- Squash-merge branches with messy interim commits. +- Use a regular merge for branches whose commit history is clean and meaningful. +- The PR must have green CI and an approval. +- Delete the branch after merging. diff --git a/confluence/04-issue-tracking.md b/confluence/04-issue-tracking.md new file mode 100644 index 0000000..55318c1 --- /dev/null +++ b/confluence/04-issue-tracking.md @@ -0,0 +1,72 @@ +# Issue Tracking + +All work is tracked in Jira. Jira is the source of truth for work; GitHub is the source of truth for code. Link them rather than duplicating issues. + +## Projects + +| Key | Project | Use for | +|---|---|---| +| `DEV` | Software Development | Product features, application bugs, customer-facing work | +| `PLAT` | Infrastructure and Platform | AWS, networking, CI/CD, internal tooling | +| `SEC` | Security | Security findings, remediations, audits, access reviews | + +Board columns are **To Do / In Progress / Blocked / Done**. + +## Linking to GitHub + +GitHub is connected to Jira. When the Jira key appears at the end of the PR title, for example `feat(parser): add receipt parser (DEV-123)`, the branch, commits, and PR show up on the Jira issue automatically. + +## Ticket template + +Every ticket description has these four sections. If a section has nothing in it, say so; do not delete it. + +```markdown +## Context + +## Scope + +## Business rules + +## Acceptance Criteria +``` + +### Context + +Why the work exists. Name the affected systems specifically (repo, service, function name) and link related tickets and PRs. + +| Good | Bad | +|---|---| +| The `payments-processor` Lambda times out on files over 10 MB. Found while working DEV-123. | The importer is slow sometimes. | + +### Scope + +What is in scope, and explicitly what is not. Link the ticket that owns anything out of scope. + +| Good | Bad | +|---|---| +| In scope: raise the timeout and add a size guard. Out of scope: streaming parser rewrite (DEV-124). | Fix the importer. | + +### Business rules + +Constraints that must hold during and after the work: data that must not be lost, behavior that must not regress, limits that must not be exceeded. + +| Good | Bad | +|---|---| +| In-flight uploads must not be dropped during the deploy. Existing records keep their IDs. | Don't break anything. | + +### Acceptance Criteria + +A checklist of items that can each be verified on its own. Prefer a command or observable result over a claim. + +```markdown +## Acceptance Criteria +- [ ] A 25 MB upload completes without a timeout error in the logs +- [ ] Uploads over 50 MB return 413 with a descriptive message +- [ ] The README documents the new size limit +``` + +## Ticket hygiene + +- **Close with evidence.** When closing, comment with what resolved it: the merged PR or the ticket that took over the remaining work. +- **Fix stale descriptions first.** Before starting an old ticket, check it against the current state and rewrite anything that no longer holds. +- **Keep epics honest.** Close an epic once its children are done, or add tickets for the work that remains. diff --git a/confluence/05-secrets-and-configuration.md b/confluence/05-secrets-and-configuration.md new file mode 100644 index 0000000..4fbbb03 --- /dev/null +++ b/confluence/05-secrets-and-configuration.md @@ -0,0 +1,64 @@ +# Secrets and Configuration + +Sensitive and non-sensitive configuration are kept strictly apart. + +## AWS Secrets Manager: sensitive values + +Use Secrets Manager for every sensitive value: + +- API tokens and keys +- Signing secrets used to verify requests +- Webhook URLs that act as authentication +- Connection strings containing passwords +- Anything that would cause harm if leaked + +If you are unsure whether something is sensitive, treat it as sensitive. + +Name secrets `/`, for example `my-stack/stripe-key`. + +## SSM Parameter Store: non-sensitive values + +Use Parameter Store only for non-sensitive configuration: + +- Feature flags +- Public endpoint URLs +- Schedule expressions +- Non-sensitive identifiers, such as channel IDs +- Resource names that deploy pipelines read, such as bucket names and function names + +## Reading secrets in a Lambda + +1. Store the value in Secrets Manager. +2. Grant the function's role `secretsmanager:GetSecretValue` on only the secrets it needs. +3. Read the secret on cold start and cache it in a module-level variable. + +```python +import json + +import boto3 + +_client = boto3.client("secretsmanager") +_cached = None + + +def get_config(): + global _cached + if _cached is None: + resp = _client.get_secret_value(SecretId="my-stack/config") + _cached = json.loads(resp["SecretString"]) + return _cached + + +def handler(event, context): + config = get_config() + # use config values +``` + +## Never + +- Put sensitive values in Lambda environment variables. They show in plain text in the AWS console. +- Commit `.env` files or any file containing real credentials. +- Share credentials in Jira, Confluence, Slack, email, or any other plain-text tool. +- Hardcode account IDs, ARNs, or resource names that belong in configuration. + +If a secret is committed or exposed by mistake, tell your Sea Haven contact immediately so it can be rotated. Deleting the commit is not enough. diff --git a/confluence/06-aws-infrastructure.md b/confluence/06-aws-infrastructure.md new file mode 100644 index 0000000..9a0b374 --- /dev/null +++ b/confluence/06-aws-infrastructure.md @@ -0,0 +1,71 @@ +# AWS Infrastructure + +## Infrastructure as code + +- Every AWS resource is managed by infrastructure as code. Do not create Lambdas, roles, buckets, or anything else by hand in the console. +- **Terraform** (run by HCP Terraform) is the default for all new projects. +- Some existing projects use **AWS SAM** or **AWS CDK**. Keep working in the tool the project already uses. Do not start a new project on SAM or CDK without agreement from Sea Haven. +- Terraform creates the infrastructure. The CI/CD pipeline deploys the application code into it. Terraform does not package or deploy application code. + +## Lambda defaults + +Every Lambda function uses these settings: + +| Setting | Value | +|---|---| +| Runtime | Python 3.12, or Node.js 24 (`nodejs24.x`) | +| Architecture | `arm64` | +| Log retention | 60 days, set explicitly in code | +| Name | kebab-case, prefixed with the stack or repo name | + +- Always set log retention explicitly. The AWS default keeps logs forever. +- Do not start new functions on older Node runtimes such as `nodejs22.x`. +- Give each function an IAM role with only the permissions it needs. Never use `AdministratorAccess` or broad wildcards. + +## S3 + +- Tag every bucket with `Purpose` and `ManagedBy`. +- Define lifecycle policies in code. +- Use Glacier Deep Archive for archival data. + +## Outputs + +Every stack exposes the function ARNs and any externally used URLs (such as API endpoints) as outputs. + +## Dependency versions + +- Pin dependencies to exact versions and commit the lockfile. +- Automated dependency-update PRs keep those pins current. Do not disable them or add blanket ignore rules. +- If one specific release is broken, ignore only that release, with a comment explaining why. + +## README + +Every repo has a README that accurately describes: + +- Architecture +- Services and functions +- Data flow +- Required configuration + +Update the README in the same PR that changes the behavior it describes. + +## Project layout (Terraform) + +``` +project-name/ +├── terraform/ # Infrastructure (applied by HCP Terraform) +│ ├── bootstrap/ # Placeholder packages so Terraform can create functions +│ ├── lambda.tf +│ ├── s3.tf +│ ├── variables.tf +│ ├── outputs.tf +│ └── versions.tf +├── src/ # Application code (deployed by the pipeline) +├── .github/ +│ └── workflows/ +│ ├── ci.yaml +│ └── deploy-.yaml +└── README.md +``` + +CI runs `terraform fmt -check -recursive` and `terraform validate` on every PR. Terraform plans for PRs run in HCP Terraform. diff --git a/confluence/07-ci-cd-and-deployments.md b/confluence/07-ci-cd-and-deployments.md new file mode 100644 index 0000000..773f4a9 --- /dev/null +++ b/confluence/07-ci-cd-and-deployments.md @@ -0,0 +1,69 @@ +# CI/CD and Deployments + +## Rules + +- Every deployable repo has a CI workflow and a deploy workflow. A project is not production-ready without them. +- Nobody deploys from a workstation. Only the pipeline deploys, and it deploys a reviewed commit. +- GitHub Actions is the CI/CD platform. + +## CI + +- CI runs on every pull request to `main`. +- A PR cannot merge until the required CI check passes. +- Lint, formatting, type checks, and tests run in CI. + +## Environments and deploys + +| Environment | Deployed when | Approval | +|---|---|---| +| `dev` | A PR merges to `main` | None | +| `prod` | A person publishes a GitHub Release | Required | + +### Releasing to production + +A person cuts a release from `main` after the change has been verified in dev: + +```bash +gh release create vX.Y.Z --target main --generate-notes +``` + +Releases are never created by a workflow. After the release is published, the production deploy waits for a Sea Haven approver. + +### Rollback + +Re-run the deploy workflow manually (`workflow_dispatch`) at the previous release tag and have it approved. Do not roll back by reverting infrastructure code. + +### Hotfixes + +When production is broken and the fix cannot wait for `main`: + +```bash +git fetch --tags +git checkout -b hotfix/describe-the-break v1.2.3 +# commit and push, or open a PR targeting the hotfix branch +gh release create v1.2.4 --target hotfix/describe-the-break --generate-notes +# after the prod deploy is approved and verified: +# merge hotfix/describe-the-break into main +``` + +### Verify the live system + +A green workflow run is not proof of a working deploy. Check the live system: health endpoints report the new version, and the application behaves as expected. + +## Workflow conventions + +- Workflow files are kebab-case, one deploy workflow per deployable: `deploy-web.yaml`, `deploy-api.yaml`. +- Pin every GitHub Action and reusable workflow to a full commit SHA, with the version in a comment: + + ```yaml + uses: actions/checkout@ # v4.1.0 + ``` + + Never reference a branch or a tag alone. Automated update PRs move the pins forward. +- Deploys authenticate to AWS with OIDC roles. Never use long-lived AWS access keys in GitHub. +- Deploy jobs use `cancel-in-progress: false`. Cancelling a deploy halfway leaves the environment half-updated. CI jobs may cancel superseded runs. +- Do not hardcode bucket names, distribution IDs, or function names in workflow YAML. Read them from configuration. + +## Repository settings + +Sea Haven manages repository settings, branch protection, secret scanning, and Dependabot alerts. If a setting blocks your work, raise it with your Sea Haven contact rather than working around it. diff --git a/confluence/README.md b/confluence/README.md new file mode 100644 index 0000000..8ca1003 --- /dev/null +++ b/confluence/README.md @@ -0,0 +1,33 @@ +# Confluence Export: External Standards + +A simplified, sanitized copy of this handbook for partner and contract engineering teams. Each numbered file is one Confluence page, and its H1 is the page title. Publish `00-engineering-standards.md` as the parent page and the rest as its children, in number order. + +This README is internal. Do not publish it. + +## Rules for these pages + +- Link between pages by page title, not by file path. Repo-relative links break in Confluence. +- No internal repo names, AWS account names or IDs, IAM role or trust details, org secret names, Jira ticket numbers, people's names, or incident history. +- State the current standard only. No migration history, legacy lanes, or one-off exceptions. +- When a root handbook page changes a rule, update the matching page here in the same PR. + +## Source mapping + +| Page | Handbook source | +|---|---| +| `00-engineering-standards.md` | `README.md` | +| `01-naming-conventions.md` | `naming-conventions.md` | +| `02-git-workflow.md` | `git-workflow.md`, `commit-messages.md` | +| `03-pull-requests-and-code-review.md` | `pull-requests.md`, `code-review.md`, `code-review-rubric.md` | +| `04-issue-tracking.md` | `issue-tracking.md` | +| `05-secrets-and-configuration.md` | `secrets-and-config.md` | +| `06-aws-infrastructure.md` | `aws-infrastructure.md`, `terraform-project-layout.md` | +| `07-ci-cd-and-deployments.md` | `cicd.md`, `github-standards.md` | + +## Deliberately excluded + +- `dev-environment.md`: personal workstation layout, macOS launchd, cleanup cadence +- `hcp-terraform.md`: workspace, exec-role, and org-baseline internals +- `sam-project-layout.md`, `cdk-project-layout.md`, `lambda-template.md`, `bedrock.md`: legacy-lane and team-specific detail +- `hooks/`, `scripts/`, `constructs/`: code containing account IDs and network ranges +- Named internal repos, legacy stack names, retired Jira projects, contractor repo names, sign-off by a named person, deploy-then-merge exception, incident references, ruleset migration state, OIDC trust claims, formatter App secrets