Skip to content

Extract SSL cert logic into testable setup-ssl.sh script#1637

Merged
kmcginnes merged 1 commit intomainfrom
extract-ssl-script
Apr 6, 2026
Merged

Extract SSL cert logic into testable setup-ssl.sh script#1637
kmcginnes merged 1 commit intomainfrom
extract-ssl-script

Conversation

@kmcginnes
Copy link
Copy Markdown
Collaborator

@kmcginnes kmcginnes commented Apr 4, 2026

Description

Extract the SSL certificate generation and validation logic from docker-entrypoint.sh into a standalone setup-ssl.sh script so it can be tested in isolation, following the same pattern used for process-environment.sh.

  • Move cert generation and existing-cert validation into setup-ssl.sh, parameterized by CERT_DIR and HOST environment variables
  • Use absolute paths with CERT_DIR instead of cd + relative paths
  • Use portable sed pattern matching instead of hardcoded line numbers (necessary for cross-platform test execution)
  • Preserve existing behavior including known bugs (duplicate rootCA.crt check that misses server.key)
  • Add chmod a+x ./setup-ssl.sh to Dockerfile
  • Add 7 tests covering cert generation, SAN injection, existing cert reuse, and missing file detection

This is a behavior-preserving extraction to enable a follow-up PR that fixes the bugs with test coverage already in place.

Validation

  • pnpm checks passes
  • pnpm test passes (7 new tests in setup-ssl.test.ts)
  • Bug test case documents the duplicate rootCA.crt / missing server.key defect
  • Docker image built and tested with Finch across three scenarios:
    • HTTPS disabled — starts cleanly on HTTP ✅
    • HTTPS enabled with HOST — generates certs and starts on HTTPS ✅
    • HTTPS enabled without HOST, no existing certs — prints error but continues past exit 1 due to missing set -e in docker-entrypoint.sh, silently falls back to HTTP. This is pre-existing behavior preserved by this PR and will be fixed in a follow-up.

Related Issues

Check List

  • I confirm that my contribution is made under the terms of the Apache 2.0 license.
  • I have verified pnpm checks passes with no errors.
  • I have verified pnpm test passes with no failures.
  • I have covered new added functionality with unit tests if necessary.
  • I have updated documentation if necessary.

- Move cert generation and validation from docker-entrypoint.sh to setup-ssl.sh
- Accept CERT_DIR and HOST as environment variables
- Use absolute paths with CERT_DIR instead of cd + relative paths
- Use portable sed pattern matching instead of hardcoded line numbers
- Preserve existing behavior including known bugs (duplicate rootCA.crt check)
- Add 7 tests documenting current behavior including bug test case
@kmcginnes kmcginnes marked this pull request as ready for review April 4, 2026 00:55
@kmcginnes kmcginnes merged commit f831b20 into main Apr 6, 2026
5 checks passed
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.

2 participants