-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
restrict network ports #272
Conversation
I verified for #268, and this part seems to be fixed. However, when opening the frontend, the page is not loading correctly. I can see: Uh, oh! could not retrieve currencies
main.(*frontendServer).homeHandler
/usr/src/app/handlers.go:64
main.instrumentHandler.func1
/usr/src/app/middleware.go:122
net/http.HandlerFunc.ServeHTTP
/usr/local/go/src/net/http/server.go:2047
go.opentelemetry.io/contrib/instrumentation/github.com/gorilla/mux/otelmux.traceware.ServeHTTP
/go/pkg/mod/go.opentelemetry.io/contrib/instrumentation/github.com/gorilla/mux/otelmux@v0.29.0/mux.go:145
github.com/gorilla/mux.(*Router).ServeHTTP
/go/pkg/mod/github.com/gorilla/mux@v1.8.0/mux.go:210
main.(*logHandler).ServeHTTP
/usr/src/app/middleware.go:96
main.ensureSessionID.func1
/usr/src/app/middleware.go:150
net/http.HandlerFunc.ServeHTTP
/usr/local/go/src/net/http/server.go:2047
net/http.serverHandler.ServeHTTP
/usr/local/go/src/net/http/server.go:2879
net/http.(*conn).serve
/usr/local/go/src/net/http/server.go:1930
runtime.goexit
/usr/local/go/src/runtime/asm_amd64.s:1581``` |
@mviitane did you use the |
Yes, I did. It didn't seem to help. |
@mviitane, I wonder if the currency service was recreated and attached to the network properly. A couple of things to try and diagnose.
|
No, it seems to be stopped.
No, the currency service is not visible. The other services are visible.
|
@mviitane Can you get the logs from the currency service about why it didn't start, and try restarting the service itself?
|
@puckpuck Seems weird, but now it started working. For me, it looks like the rebase done in your hide-ports branch fixed it. Hard to say exactly what caused the problem. Anyway, we are good to go now. This was the currency-service log from the failing case:
|
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.
Looks good! Verified for #268.
* restrict network ports * restrict network ports
Fixes #257, #268
Changes
Adds a default docker compose network
Removes host port bindings for all ports that don't require end-user access (service-service)
This may require you to add the
--force-recreate
to docker compose the first time running this after this change (or delete the demo images from your local system).CHANGELOG.md
updated for non-trivial changes