-
Notifications
You must be signed in to change notification settings - Fork 235
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
actual Dockerfile and config in README #90
base: master
Are you sure you want to change the base?
actual Dockerfile and config in README #90
Conversation
README.md
Outdated
allow all | ||
deny all # unreachable rule, remaining requests are matched by `allow all` above | ||
} | ||
:80, :443 { |
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.
does this set up auto-redirect from 80 to 443?
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.
No, this runs the application "Caddy" in Docker on port 80 (for http) and on port 443 (for https), if not, then it cannot serve one of the ports.
You can separate them in the configuration, then you can set a different config for them, but in my cases, the same rules are processed.
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.
So :80 is not encrypted, and runs a proxy? That's a security and privacy issue -- we can't have such configuration suggested as a default.
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.
I misunderstood the purpose of this line in the configuration, I corrected it in the commit: 78bd7d8
docker-build/Dockerfile
Outdated
COPY --from=builder /go/forwardproxy/cmd/caddy/caddy /usr/bin/caddy | ||
EXPOSE 80 443 | ||
ENTRYPOINT ["/usr/bin/caddy"] | ||
CMD ["-conf", "/etc/caddy/Caddyfile", "--log", "/dev/stdout", "--agree=true"] |
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.
where did /etc/caddy/Caddyfile come from?
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.
This case is good for Kubernetes - there you can put a config file as Config Map. I can describe a solution for Kubernetes, or put a default config in docker. In any case, it seems to me that it would be good to start the application with a configuration file.
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.
How will it be more convenient?
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.
Currently existing gen_caddyfile_and_start.sh
script (if it's still functional, haven't touched in a while) generates the config automatically. That seems convenient
WORKDIR /go/forwardproxy/cmd/caddy | ||
RUN go build caddy.go | ||
|
||
FROM ubuntu:20.04 |
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.
Yes, however, "Scratch" image gives you an empty file system, That is all it does. scratch on it's own does absolutely nothing, and has nothing in it.
actual Dockerfile and config in README