Skip to content

Fix ARM64 build: copy lighttpd config directly to conf-enabled#10

Merged
Sol455 merged 1 commit into
masterfrom
feat/GHA-workflow
Dec 19, 2025
Merged

Fix ARM64 build: copy lighttpd config directly to conf-enabled#10
Sol455 merged 1 commit into
masterfrom
feat/GHA-workflow

Conversation

@Sol455
Copy link
Copy Markdown

@Sol455 Sol455 commented Dec 19, 2025

The lighttpd-enable-mod command doesn't exist in the ARM64 base image. Instead of using the helper script, copy the proxy configuration directly to /etc/lighttpd/conf-enabled/90-proxy.conf.

The lighttpd-enable-mod command doesn't exist in the ARM64 base image.
Instead of using the helper script, copy the proxy configuration
directly to /etc/lighttpd/conf-enabled/90-proxy.conf.
@Sol455 Sol455 requested a review from jonnyspicer December 19, 2025 01:51
@claude
Copy link
Copy Markdown

claude Bot commented Dec 19, 2025

Code Review - PR #10: Fix ARM64 build

Summary

This PR addresses a legitimate ARM64 build issue by replacing the lighttpd-enable-mod helper script (which doesn't exist in the ARM64 base image) with a direct copy of the lighttpd proxy configuration to the enabled directory. The change is targeted, correct, and solves the stated problem.


✅ Code Quality & Best Practices

Strengths:

  • Minimal, focused change: Only touches what's necessary to fix the issue
  • Correct approach: Directly placing config in conf-enabled/ is equivalent to using lighttpd-enable-mod, which traditionally just symlinks from conf-available/ to conf-enabled/
  • Maintains functionality: The proxy configuration will still be loaded by lighttpd
  • Clean diff: Reduces 2 lines to 1, simpler is better

Minor observation:

  • The change removes the intermediate step of copying to conf-available/ first. While functionally correct, this means the config won't appear in the "available" directory anymore. This is fine for this use case but worth noting for documentation purposes.

🔍 Potential Issues

No blocking issues found. However, consider:

  1. Testing verification: Ensure the build now completes successfully on ARM64 and that the proxy functionality works as expected after deployment
  2. Base image consistency: If the base image (ghcr.io/sdr-enthusiasts/docker-tar1090:latest) ever adds lighttpd-enable-mod support, this won't break anything, but you might want to document why the direct approach was chosen

⚡ Performance Considerations

No performance impact. This is a build-time change only:

  • Removes one unnecessary file copy operation
  • Removes one command execution (lighttpd-enable-mod)
  • Slightly faster Docker build time (negligible, but technically improved)

🔒 Security Concerns

No security issues identified:

  • The configuration file (docker/lighttpd-proxy.conf) remains unchanged
  • File permissions should be preserved (both COPY operations use the same mechanism)
  • The proxy configuration itself looks safe (Dockerfile.tar1090:13, proxying /data/aircraft.json to localhost:3000)

Recommendation: As a general best practice, ensure /etc/lighttpd/conf-enabled/90-proxy.conf has appropriate read permissions (should be handled by the base image's lighttpd setup)


🧪 Test Coverage

Current state:

  • This is an infrastructure/build fix, so automated unit tests don't apply
  • The GitHub Actions workflow (.github/workflows/docker_build.yml:44) builds for linux/arm64, which will validate this fix

Recommendations:

  1. ✅ Verify the workflow succeeds with this change
  2. ✅ Test the built image on an ARM64 device to confirm:
    • Lighttpd starts without errors
    • The proxy configuration is loaded (check lighttpd -f /etc/lighttpd/lighttpd.conf -t inside the container)
    • The /data/aircraft.json endpoint correctly proxies to the Node.js service (proxy/server.js)

📋 Additional Recommendations

  1. Documentation: Consider adding a comment in the Dockerfile explaining why the direct copy approach is used (ARM64 compatibility)

    # Copy proxy config directly to conf-enabled (lighttpd-enable-mod not available in ARM64 base image)
    COPY docker/lighttpd-proxy.conf /etc/lighttpd/conf-enabled/90-proxy.conf
  2. Consistency check: Verify that other lighttpd configurations in the base image don't conflict with this approach

  3. Multi-arch consideration: If you plan to support both AMD64 and ARM64, confirm this approach works on both architectures (direct copy should work universally)


✨ Conclusion

LGTM with minor suggestion. This is a solid fix that:

  • Solves the immediate ARM64 build problem
  • Uses a standard, reliable approach
  • Simplifies the Dockerfile
  • Has no negative side effects

The only suggestion is to add a brief comment explaining the ARM64 compatibility rationale for future maintainers.

Approval Status: ✅ Approved pending successful CI build verification

Copy link
Copy Markdown

@jonnyspicer jonnyspicer left a comment

Choose a reason for hiding this comment

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

lgtm

@Sol455 Sol455 merged commit 42a8886 into master Dec 19, 2025
6 checks passed
@Sol455 Sol455 deleted the feat/GHA-workflow branch December 19, 2025 11:48
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