Skip to content

fix: #419 TLS https URL for SSE endpoint #420

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

Merged
merged 3 commits into from
Apr 9, 2025
Merged

Conversation

jexp
Copy link
Contributor

@jexp jexp commented Apr 9, 2025

Fixes that in SSL deployments, e.g. on k8s or cloud run the SSE endpoint was statically returning http:

not a scheme based on request TLS attribute.

Fixes #419

@jexp jexp requested a review from a team as a code owner April 9, 2025 00:01
@jexp jexp changed the title Fix TLS https URL for SSE endpoint Fixes #419 TLS https URL for SSE endpoint Apr 9, 2025
@jexp jexp changed the title Fixes #419 TLS https URL for SSE endpoint Fix TLS https URL for SSE endpoint Apr 9, 2025
@jexp jexp changed the title Fix TLS https URL for SSE endpoint fix: #419 TLS https URL for SSE endpoint Apr 9, 2025
@jexp
Copy link
Contributor Author

jexp commented Apr 9, 2025

Hmm, didn't seem to help after rebuild of this in a new image and redeploy it still seems to return http.
Not sure if I did something wrong in the code or in the deployment :(

@jexp
Copy link
Contributor Author

jexp commented Apr 9, 2025

One of my colleagues said

Hey 🙂 by not working do you mean that the if r.TLS != nil code is always false and therefore not appending the 's' to the protocol (always http?).

If so I've seen something like this before in the past in environments with TLS termination - r.TLS isn’t reliable when using a reverse proxy (in my case nginx) which might terminate TLS, so the generated URL ended up with the wrong scheme. I ended up inspecting headers like X-Forwarded-Proto (or whatever the proxy sets) to correctly determine if the URL should use "http" or "https."

@jexp
Copy link
Contributor Author

jexp commented Apr 9, 2025

Works now

Screenshot 2025-04-09 at 15 11 23

@Yuan325
Copy link
Contributor

Yuan325 commented Apr 9, 2025

/gcbrun

@Yuan325 Yuan325 added the tests: run Label to trigger Github Action tests. label Apr 9, 2025
@github-actions github-actions bot removed the tests: run Label to trigger Github Action tests. label Apr 9, 2025
Copy link
Contributor

@Yuan325 Yuan325 left a comment

Choose a reason for hiding this comment

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

Thank you for contributing! :)

@Yuan325 Yuan325 merged commit 0a7d3ff into googleapis:main Apr 9, 2025
8 checks passed
annguy3n pushed a commit to annguy3n/genai-toolbox that referenced this pull request Apr 10, 2025
Fixes that in SSL deployments, e.g. on k8s or cloud run the SSE endpoint
was statically returning `http:`

not a scheme based on request TLS attribute.

Fixes googleapis#419

---------

Co-authored-by: Yuan <45984206+Yuan325@users.noreply.github.com>
duwenxin99 added a commit that referenced this pull request Apr 23, 2025
🤖 I have created a release *beep* *boop*
---


##
[0.4.0](v0.3.0...v0.4.0)
(2025-04-23)


### Features

* Add `AuthRequired` to Neo4j & Dgraph Tools
([#434](#434))
([afbf4b2](afbf4b2))
* Add `AuthRequired` to tool manifest
([#433](#433))
([d9388ad](d9388ad))
* Add BigQuery source and tool
([#463](#463))
([8055aa5](8055aa5))
* Add Bigtable source and tool
([#418](#418))
([ae53b8e](ae53b8e))
* Add IAM AuthN to AlloyDB Source
([#399](#399))
([e8ed447](e8ed447))
* Add IAM AuthN to Cloud SQL Sources
([#414](#414))
([be85b82](be85b82))
* Add toolset feature to mcp
([#425](#425))
([e307857](e307857)),
closes [#403](#403)
* Add SQLite source and tool
([#438](#438))
([fc14cbf](fc14cbf))
* Support env replacement for tools.yaml
([#462](#462))
([eadb678](eadb678))


### Bug Fixes

* [#419](#419) TLS
https URL for SSE endpoint
([#420](#420))
([0a7d3ff](0a7d3ff))
* **docs:** Fix link 'Edit this page'
([#454](#454))
([969065e](969065e)),
closes [#427](#427)
* Update http error code from invocation
([#468](#468))
([ff7c0ff](ff7c0ff)),
closes [#465](#465)

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

---------

Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
Co-authored-by: Wenxin Du <117315983+duwenxin99@users.noreply.github.com>
Co-authored-by: Kurtis Van Gent <31518063+kurtisvg@users.noreply.github.com>
github-actions bot pushed a commit that referenced this pull request Apr 23, 2025
🤖 I have created a release *beep* *boop*
---

##
[0.4.0](v0.3.0...v0.4.0)
(2025-04-23)

### Features

* Add `AuthRequired` to Neo4j & Dgraph Tools
([#434](#434))
([afbf4b2](afbf4b2))
* Add `AuthRequired` to tool manifest
([#433](#433))
([d9388ad](d9388ad))
* Add BigQuery source and tool
([#463](#463))
([8055aa5](8055aa5))
* Add Bigtable source and tool
([#418](#418))
([ae53b8e](ae53b8e))
* Add IAM AuthN to AlloyDB Source
([#399](#399))
([e8ed447](e8ed447))
* Add IAM AuthN to Cloud SQL Sources
([#414](#414))
([be85b82](be85b82))
* Add toolset feature to mcp
([#425](#425))
([e307857](e307857)),
closes [#403](#403)
* Add SQLite source and tool
([#438](#438))
([fc14cbf](fc14cbf))
* Support env replacement for tools.yaml
([#462](#462))
([eadb678](eadb678))

### Bug Fixes

* [#419](#419) TLS
https URL for SSE endpoint
([#420](#420))
([0a7d3ff](0a7d3ff))
* **docs:** Fix link 'Edit this page'
([#454](#454))
([969065e](969065e)),
closes [#427](#427)
* Update http error code from invocation
([#468](#468))
([ff7c0ff](ff7c0ff)),
closes [#465](#465)

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

---------

Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
Co-authored-by: Wenxin Du <117315983+duwenxin99@users.noreply.github.com>
Co-authored-by: Kurtis Van Gent <31518063+kurtisvg@users.noreply.github.com> 4ed16cc
github-actions bot pushed a commit to renovate-bot/googleapis-_-genai-toolbox that referenced this pull request Apr 23, 2025
🤖 I have created a release *beep* *boop*
---

##
[0.4.0](googleapis/genai-toolbox@v0.3.0...v0.4.0)
(2025-04-23)

### Features

* Add `AuthRequired` to Neo4j & Dgraph Tools
([googleapis#434](googleapis#434))
([afbf4b2](googleapis@afbf4b2))
* Add `AuthRequired` to tool manifest
([googleapis#433](googleapis#433))
([d9388ad](googleapis@d9388ad))
* Add BigQuery source and tool
([googleapis#463](googleapis#463))
([8055aa5](googleapis@8055aa5))
* Add Bigtable source and tool
([googleapis#418](googleapis#418))
([ae53b8e](googleapis@ae53b8e))
* Add IAM AuthN to AlloyDB Source
([googleapis#399](googleapis#399))
([e8ed447](googleapis@e8ed447))
* Add IAM AuthN to Cloud SQL Sources
([googleapis#414](googleapis#414))
([be85b82](googleapis@be85b82))
* Add toolset feature to mcp
([googleapis#425](googleapis#425))
([e307857](googleapis@e307857)),
closes [googleapis#403](googleapis#403)
* Add SQLite source and tool
([googleapis#438](googleapis#438))
([fc14cbf](googleapis@fc14cbf))
* Support env replacement for tools.yaml
([googleapis#462](googleapis#462))
([eadb678](googleapis@eadb678))

### Bug Fixes

* [googleapis#419](googleapis#419) TLS
https URL for SSE endpoint
([googleapis#420](googleapis#420))
([0a7d3ff](googleapis@0a7d3ff))
* **docs:** Fix link 'Edit this page'
([googleapis#454](googleapis#454))
([969065e](googleapis@969065e)),
closes [googleapis#427](googleapis#427)
* Update http error code from invocation
([googleapis#468](googleapis#468))
([ff7c0ff](googleapis@ff7c0ff)),
closes [googleapis#465](googleapis#465)

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

---------

Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
Co-authored-by: Wenxin Du <117315983+duwenxin99@users.noreply.github.com>
Co-authored-by: Kurtis Van Gent <31518063+kurtisvg@users.noreply.github.com> 4ed16cc
jeffreyrubi pushed a commit to jeffreyrubi/genai-toolbox that referenced this pull request Jun 7, 2025
Fixes that in SSL deployments, e.g. on k8s or cloud run the SSE endpoint
was statically returning `http:`

not a scheme based on request TLS attribute.

Fixes googleapis#419

---------

Co-authored-by: Yuan <45984206+Yuan325@users.noreply.github.com>
jeffreyrubi pushed a commit to jeffreyrubi/genai-toolbox that referenced this pull request Jun 7, 2025
🤖 I have created a release *beep* *boop*
---


##
[0.4.0](googleapis/genai-toolbox@v0.3.0...v0.4.0)
(2025-04-23)


### Features

* Add `AuthRequired` to Neo4j & Dgraph Tools
([googleapis#434](googleapis#434))
([afbf4b2](googleapis@afbf4b2))
* Add `AuthRequired` to tool manifest
([googleapis#433](googleapis#433))
([d9388ad](googleapis@d9388ad))
* Add BigQuery source and tool
([googleapis#463](googleapis#463))
([8055aa5](googleapis@8055aa5))
* Add Bigtable source and tool
([googleapis#418](googleapis#418))
([ae53b8e](googleapis@ae53b8e))
* Add IAM AuthN to AlloyDB Source
([googleapis#399](googleapis#399))
([e8ed447](googleapis@e8ed447))
* Add IAM AuthN to Cloud SQL Sources
([googleapis#414](googleapis#414))
([be85b82](googleapis@be85b82))
* Add toolset feature to mcp
([googleapis#425](googleapis#425))
([e307857](googleapis@e307857)),
closes [googleapis#403](googleapis#403)
* Add SQLite source and tool
([googleapis#438](googleapis#438))
([fc14cbf](googleapis@fc14cbf))
* Support env replacement for tools.yaml
([googleapis#462](googleapis#462))
([eadb678](googleapis@eadb678))


### Bug Fixes

* [googleapis#419](googleapis#419) TLS
https URL for SSE endpoint
([googleapis#420](googleapis#420))
([0a7d3ff](googleapis@0a7d3ff))
* **docs:** Fix link 'Edit this page'
([googleapis#454](googleapis#454))
([969065e](googleapis@969065e)),
closes [googleapis#427](googleapis#427)
* Update http error code from invocation
([googleapis#468](googleapis#468))
([ff7c0ff](googleapis@ff7c0ff)),
closes [googleapis#465](googleapis#465)

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

---------

Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
Co-authored-by: Wenxin Du <117315983+duwenxin99@users.noreply.github.com>
Co-authored-by: Kurtis Van Gent <31518063+kurtisvg@users.noreply.github.com>
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.

deploy to cloud-run responds with http not https URL in SSE response
2 participants