Skip to content

Fix bootstrapping, add ETag caching, and 502/404 fallback in pull-config.sh#95

Merged
runleveldev merged 9 commits intomainfrom
copilot/fix-bootstrapping-issues-pull-config
Nov 1, 2025
Merged

Fix bootstrapping, add ETag caching, and 502/404 fallback in pull-config.sh#95
runleveldev merged 9 commits intomainfrom
copilot/fix-bootstrapping-issues-pull-config

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Oct 28, 2025

✅ Fix Bootstrapping Problems in pull-config.sh - COMPLETE

This PR successfully addresses all three bootstrapping issues with the nginx reverse proxy configuration script.

✅ All Requirements Met

1. Check for existence of reverse-proxy.conf before backup operations

  • Script now checks if config file exists before attempting backup
  • Checks if backup exists before attempting restore
  • Handles first-run scenario gracefully without errors

2. Store ETag and use If-None-Match header for caching

  • Stores ETag in ${CONF_FILE}.etag (derived from CONF_FILE)
  • Sends If-None-Match header with existing ETag on subsequent requests
  • Exits early on 304 Not Modified (no unnecessary downloads or reloads)
  • Significantly reduces bandwidth usage and nginx reload frequency

3. Fallback to internal cluster URL on 502 and 404 errors

  • Primary: https://create-a-container.opensource.mieweb.org/nginx.conf
  • Fallback: http://create-a-container.cluster.mieweb.org:3000/nginx.conf
  • Automatically falls back on 502 Bad Gateway and 404 Not Found errors
  • Solves the chicken-and-egg bootstrapping problem
  • Handles unknown subdomains that may return 404 instead of 502

📊 Latest Changes (Current Commit)

Added 404 handling to fallback logic:

  1. Handle 404 responses (line 56)
    • Also fallback to internal URL on 404 responses
    • Handles case where unknown subdomains return 404 instead of 502
    • Maintains existing 502 and connection failure (000) handling

📊 Previous Changes

Robustness improvements:

  1. Check conf file before using etag (line 35)
    • Only use etag if both CONF_FILE and ETAG_FILE exist
    • Handles edge case where etag file is orphaned (exists without conf)

Code review improvements:

  1. Derive ETAG_FILE from CONF_FILE (line 6)

    • Changed from hardcoded path to ETAG_FILE="${CONF_FILE}.etag"
  2. Global variables for temp files (lines 7-9)

    • Added TEMP_FILE="${CONF_FILE}.tmp"
    • Added HEADERS_FILE="${CONF_FILE}.headers"
    • Added BACKUP_FILE="${CONF_FILE}.bak"
    • Removed local variable repetition throughout file
  3. Build curl command as array (lines 26-42)

    • Replaced conditional if/else with bash array
    • Conditionally append ETag header to array
    • Cleaner, more maintainable code
  4. Cleanup function with trap EXIT (lines 14-16)

    • Added cleanup() function to remove temp files
    • Set trap cleanup EXIT to ensure cleanup always runs
    • Removed manual cleanup before each exit
    • Handles both normal exits and failures via set -e

✅ Quality Assurance

  • All tests pass:
    • First run without existing config
    • ETag cache hit (304 Not Modified)
    • Config validation failure and rollback
    • 502 error with fallback to internal URL
    • 404 error with fallback to internal URL
    • Orphaned etag file scenario
  • Shellcheck: ✅ No warnings or errors

🔒 Security Summary

No security vulnerabilities introduced:

  • Config validation still performed before applying

  • Rollback mechanism prevents bad configs

  • All temporary files properly cleaned up via trap

  • No new external dependencies added

  • Errors are properly reported to admins

  • Fixes Bootstrapping problems in pull-config.sh #94

Original prompt

This section details on the original issue you should resolve

<issue_title>Bootstrapping problems in pull-config.sh</issue_title>
<issue_description>https://github.com/mieweb/opensource-server/blob/main/nginx-reverse-proxy/pull-config.sh has the problem where, before there is any reverse proxy configuration, create-a-container.opensource.mieweb.org is inaccessible since it relies on the reverse proxy to handle requests for *.opensource.mieweb.org. The script also expects /etc/nginx/conf.d/reverse-proxy.conf to exist so it can make a backup and fails due to set -e otherwise. The script also pulls the config every minute, even if there's been no changes. In effect, we need the following changes:

  1. Check for the existence of /etc/nginx/conf.d/reverse-proxy.conf and skip the backup and restore operations if it doesn't exist
  2. Store the ETag of the reverse-proxy.conf file in a seperate file, say /etc/nginx/conf.d/reverse-proxy.etag, send that value to the server when requesting the new config. On a cache hit do nothing, we already have the latest file. On a cache miss, proceed with the backup -> download -> check -> reload/restore process
  3. If create-a-container.opensource.mieweb.org generates a gateway error (502), fallback to the http://create-a-container.cluster.mieweb.org:3000/nginx.conf (ie. the internal IP and port for the create-a-container service)</issue_description>

Comments on the Issue (you are @copilot in this section)


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI and others added 3 commits October 28, 2025 21:51
…config.sh

Co-authored-by: runleveldev <44057501+runleveldev@users.noreply.github.com>
Co-authored-by: runleveldev <44057501+runleveldev@users.noreply.github.com>
Co-authored-by: runleveldev <44057501+runleveldev@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix bootstrapping problems in pull-config.sh Fix bootstrapping, add ETag caching, and 502 fallback in pull-config.sh Oct 28, 2025
Copilot AI requested a review from runleveldev October 28, 2025 21:59
- Derive ETAG_FILE from CONF_FILE (line 6)
- Use global variables for TEMP_FILE, HEADERS_FILE, BACKUP_FILE (lines 7-9)
- Build curl command as array instead of conditionals (lines 26-42)
- Add cleanup() function with trap EXIT to ensure cleanup (lines 14-19)
- Update test scripts to match new etag filename (.conf.etag)

Co-authored-by: runleveldev <44057501+runleveldev@users.noreply.github.com>
- Check CONF_FILE exists before using ETAG_FILE (line 35)
- Suppress nginx -t and reload output to avoid cron warnings (lines 88, 103)
- Handles edge case where etag exists but conf doesn't

Co-authored-by: runleveldev <44057501+runleveldev@users.noreply.github.com>
Copilot AI requested a review from runleveldev October 30, 2025 15:30
@runleveldev runleveldev force-pushed the copilot/fix-bootstrapping-issues-pull-config branch from 83449ee to 1641cec Compare October 30, 2025 15:34
Copy link
Copy Markdown
Collaborator

@runleveldev runleveldev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested on create-a-container-dev LB

@runleveldev runleveldev marked this pull request as ready for review October 30, 2025 15:37
- Also fallback to internal URL on 404 responses
- Handles case where unknown subdomains return 404 instead of 502

Co-authored-by: runleveldev <44057501+runleveldev@users.noreply.github.com>
Copilot AI changed the title Fix bootstrapping, add ETag caching, and 502 fallback in pull-config.sh Fix bootstrapping, add ETag caching, and 502/404 fallback in pull-config.sh Oct 30, 2025
Copilot AI requested a review from runleveldev October 30, 2025 18:16
Copy link
Copy Markdown
Collaborator

@cmyers-mieweb cmyers-mieweb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved

Copy link
Copy Markdown
Collaborator

@runleveldev runleveldev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

@runleveldev runleveldev merged commit 855e0ab into main Nov 1, 2025
3 checks passed
@runleveldev runleveldev deleted the copilot/fix-bootstrapping-issues-pull-config branch November 1, 2025 12:07
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.

Bootstrapping problems in pull-config.sh

3 participants