Skip to content
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

Update caddy to 2.8 #70

Merged
merged 3 commits into from
Sep 26, 2024
Merged

Update caddy to 2.8 #70

merged 3 commits into from
Sep 26, 2024

Conversation

rahearn
Copy link
Contributor

@rahearn rahearn commented Sep 26, 2024

Changes:

  • Updates binary to caddy 2.8
  • Makes minor changes to allow local development to be a bit better on M1 macs, though validate is still broken due to incompatible architecture issues
  • Removes the unused caddy-teapot module from the test github action

Marking this as draft until I'm able to deploy this to a known-working 2.7 proxy setup to verify no downstream surprises

@rahearn rahearn requested review from mogul and a team September 26, 2024 13:33
@rahearn rahearn self-assigned this Sep 26, 2024
Copy link
Contributor

@mogul mogul left a comment

Choose a reason for hiding this comment

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

I'm fine with merging as-is, if you view this as too minor to address.

Makefile Show resolved Hide resolved
sed -i 's/# tls cert.pem key.pem/tls cert.pem key.pem/g' proxy/Caddyfile
rm allow.acl deny.acl
sed -i.bak 's/# tls cert.pem key.pem/tls cert.pem key.pem/g' proxy/Caddyfile
rm proxy/Caddyfile.bak allow.acl deny.acl
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here... Why create a .bak file if we have no intention of leaving it around?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

because sed -i 's... doesn't work on macos (-i requires an argument) and the internet says that while sed -i '' will work on macos it will break on linux. The only fully cross-platform way is to create the backup file and then remove it

Copy link
Contributor

Choose a reason for hiding this comment

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

Ahhhh, so that's the M1-specific update you made... I was wondering.

Do you want to leave a comment to that effect, with a reference? That will stop ignorant folks like me from removing it in future updates.

Copy link
Contributor

Choose a reason for hiding this comment

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

would it make more sense to just install gnu-sed with homebrew?

https://daoyuan.li/a-normal-sed-on-mac/

@rahearn rahearn marked this pull request as ready for review September 26, 2024 21:03
@rahearn rahearn merged commit f939e11 into main Sep 26, 2024
1 check passed
@rahearn rahearn deleted the update-caddy branch September 26, 2024 21:03
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.

3 participants