-
Notifications
You must be signed in to change notification settings - Fork 2
fix: replace deprecated log_access directive with Squid 5+ syntax #506
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
The `log_access` directive was removed in Squid 5.0. The ubuntu/squid:latest Docker image uses Squid 5+ which does not recognize this directive, causing Squid to fail to parse the config and crash with exit code 1. This replaces the deprecated syntax: ``` acl healthcheck_localhost src 127.0.0.1 ::1 log_access deny healthcheck_localhost access_log /var/log/squid/access.log firewall_detailed ``` With the modern Squid 5+ syntax that uses ACL filters on access_log: ``` acl healthcheck_localhost src 127.0.0.1 ::1 access_log /var/log/squid/access.log firewall_detailed !healthcheck_localhost ``` The `!` negates the ACL, meaning "log everything EXCEPT healthcheck_localhost". Fixes CI breakage after PR #432. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
🎬 THE END — Smoke Claude MISSION: ACCOMPLISHED! The hero saves the day! ✨ |
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 |
|
Chroot tests passed! Smoke Chroot - All security and functionality tests succeeded. |
✅ Coverage Check PassedOverall Coverage
Coverage comparison generated by |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes CI breakage caused by the deprecated log_access directive in Squid 5+. The ubuntu/squid:latest Docker image uses Squid 5.0+, which removed support for the log_access directive that was introduced in PR #432. The fix replaces the deprecated three-line configuration with modern two-line ACL filter syntax.
Changes:
- Replaced deprecated
log_access deny healthcheck_localhostdirective with ACL filter syntax on theaccess_logdirective (!healthcheck_localhost) - Updated tests to verify the new Squid 5+ syntax and ensure deprecated directive is not present
- Updated test descriptions and comments to clarify the Squid 5+ migration
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/squid-config.ts | Replaces deprecated log_access directive with modern Squid 5+ ACL filter syntax on access_log directive |
| src/squid-config.test.ts | Updates tests to verify new syntax, ensure deprecated directive is absent, and correctly validates ACL ordering |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Smoke Test Results (Copilot)Last 2 Merged PRs:
Test Results:
Status: PARTIAL PASS (4/5 tests passed, Playwright unavailable in CI) cc @Mossaka
|
Smoke Test ResultsLast 2 Merged PRs:
Test Results:
Overall Status: PARTIAL PASS (3/4 tests passed)
|
|
Summary
log_accessdirective in Squid 5+access_logdirectiveProblem
After merging PR #432, CI started failing because the Squid container was crashing immediately on startup with exit code 1. The root cause is that the
log_accessdirective was removed from Squid starting with version 5.0, and theubuntu/squid:latestDocker image uses Squid 5+.Solution
Replace the deprecated syntax:
With the modern Squid 5+ syntax:
The
!negates the ACL, meaning "log everything EXCEPT healthcheck_localhost".Test plan
npm test -- squid-config.test.ts- 119 tests pass)sudo awf --allow-domains github.com -- curl -s https://api.github.com/zencompletes successfully🤖 Generated with Claude Code