sablierapp / sablier

Start your containers on demand, shut them down automatically when there's no activity. Docker, Docker Swarm Mode and Kubernetes compatible.
https://sablierapp.dev/
GNU Affero General Public License v3.0
1.36k stars 46 forks source link

feat(traefik) allow websocket traffic #275

Open DrummyFloyd opened 6 months ago

DrummyFloyd commented 6 months ago

as @tomaszduda23 mentionned, i reoppend a PR to make some reviews

atm

but tested manually

with docker

i can reach ws://localhost:8080/echo container is ready , i can talk with the whoami WS even, it's not shtudown , once i stop , after session-duration hte container i stopped

edit: can i split a bit the code like

Closed: #21 #42 #152

tomaszduda23 commented 6 months ago
  • Kube => bad request (seems to be a race conditions, because runing test mannualy seems to work 302 error on first try)

This is by design you need to follow redirection. Explained in https://github.com/acouvreur/sablier/pull/241

DrummyFloyd commented 6 months ago
  • Kube => bad request (seems to be a race conditions, because runing test mannualy seems to work 302 error on first try)

This is by design you need to follow redirection. Explained in #241

will check that! thank you

EDIT: tbh , i don't fully understand what happnd , can you please have a look

DrummyFloyd commented 6 months ago
  • Kube => bad request (seems to be a race conditions, because runing test mannualy seems to work 302 error on first try)

This is by design you need to follow redirection. Explained in #241

will check that! thank you

EDIT: tbh , i don't fully understand what happnd , can you please have a look @tomaszduda23

tomaszduda23 commented 5 months ago

@acouvreur could you please enable github action on this PR. I would like to see the test results.

tomaszduda23 commented 5 months ago

It looks that you have to handle retry on client side https://github.com/trazfr/tcp-over-websocket/blob/fcb1b85e9fd0e282eee88815efeca8d729bd762b/httpClient.go#L86C14-L86C36

DrummyFloyd commented 5 months ago

It looks that you have to handle retry on client side https://github.com/trazfr/tcp-over-websocket/blob/fcb1b85e9fd0e282eee88815efeca8d729bd762b/httpClient.go#L86C14-L86C36

Will have a look a it asap

Thank you :)

tomaszduda23 commented 3 months ago

any progress on that?

DrummyFloyd commented 3 months ago

Did not have time sorry to debug this Will try to give it a try next week

EDIT: been a while, since i touched Sablier's code,

@tomaszduda23 can you please elaborate a bit what you means , by retry on client side ? because if i remembell well , this retry mechanism was already made your PR #241

sonarcloud[bot] commented 3 months ago

Quality Gate Passed Quality Gate passed

Issues
3 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

tomaszduda23 commented 3 months ago

@tomaszduda23 can you please elaborate a bit what you means , by retry on client side ? because if i remembell well , this retry mechanism was already made your PR #241

It seems that go client won't handle redirect by itself. You can reference this https://github.com/trazfr/tcp-over-websocket/blob/fcb1b85e9fd0e282eee88815efeca8d729bd762b/httpClient.go#L86C14-L86C36. It works similar way to curl without -L option. Most clients will handle this by itself though. Redirect from http to https is pretty common.

DrummyFloyd commented 3 months ago

@tomaszduda23 can you please elaborate a bit what you means , by retry on client side ? because if i remembell well , this retry mechanism was already made your PR #241

It seems that go client won't handle redirect by itself. You can reference this https://github.com/trazfr/tcp-over-websocket/blob/fcb1b85e9fd0e282eee88815efeca8d729bd762b/httpClient.go#L86C14-L86C36. It works similar way to curl without -L option. Most clients will handle this by itself though. Redirect from http to https is pretty common.

i tried some stuff, but everytime i brooke something else, dunno if i'll be able to finish that quickly