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

Undocument spin-client-addr header #2989

Open
lann opened this issue Jan 24, 2025 · 4 comments
Open

Undocument spin-client-addr header #2989

lann opened this issue Jan 24, 2025 · 4 comments

Comments

@lann
Copy link
Collaborator

lann commented Jan 24, 2025

The spin-client-addr header gives the IP and source port of some downstream client. I'd like to remove this from the "Spin contract" (and specifically developer.fermyon.com) for two reasons:

  • in many environments it won't point at the end-user client but rather some proxy; this is controlled by the host network infrastructure, not Spin
  • source port number for an http client is only very rarely useful info at the application layer

We can continue populating the header on a best-effort basis for backward compat but users should look to e.g. x-forwarded-for for this info like in any other environment.

@itowlson
Copy link
Contributor

itowlson commented Jan 26, 2025

The Spin CLI doesn't seem to support x-forwarded-for? (or did this get added recently and I've not rebased)

spin-client-addr = Some(HeaderValue { inner: String("127.0.0.1:42608") })
x-forwarded-for = None

@itowlson
Copy link
Contributor

Looking at the X-Forwarded-For docs, the original client address is the first item in the header value, followed by the list of proxies. So I think the CLI behaviour should be:

  • If the incoming request already has a X-Forwarded-For header, append the originating IP address (the current spin-client-addr) to that header value
  • Otherwise, insert a X-Forwarded-For header whose value is the originating IP address.

Does that sound right?

@lann
Copy link
Collaborator Author

lann commented Jan 27, 2025

append the originating IP address

I'm not sure about this; you don't usually stick the last source IP in x-forwarded-for. 🤔 On the other hand, I don't think we would otherwise expose that information. 🤔 On the otherer hand, it isn't clear to me that exposing internal IP addresses without explicit configuration is a good idea... 🤔

@lann
Copy link
Collaborator Author

lann commented Jan 27, 2025

The Spin CLI doesn't seem to support x-forwarded-for? (or did this get added recently and I've not rebased)

Ah, I was unclear. My suggestion is that we should leave it to the operator of the Spin host to ensure standard(-ish) headers like x-forwarded-for if that is desired.

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

No branches or pull requests

2 participants