Skip to content

Conversation

@a18e
Copy link
Contributor

@a18e a18e commented Mar 27, 2025

This Pull-Request completes the True-Client-Ip feature introduced with #759. With this change, we add three options to deal with the header:

  • forward_only_if_route_service: Forward only in route service scenario. This is the default behavior which adds support for the True-Client-Ip header in the route service scenario.
  • always_set: Always set the header, overwrite if existing.
  • always_forward: Always forward if existing, set otherwise.

b1tamara
b1tamara previously approved these changes Mar 28, 2025
Copy link
Contributor

@b1tamara b1tamara left a comment

Choose a reason for hiding this comment

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

approving to run CI

@github-project-automation github-project-automation bot moved this from Inbox to Pending Merge | Prioritized in Application Runtime Platform Working Group Mar 28, 2025
@b1tamara b1tamara added the run-ci Allow this PR to be tested on Concourse label Mar 28, 2025
@a18e a18e marked this pull request as ready for review March 28, 2025 07:48
@a18e a18e requested review from a team and CFN-CI as code owners March 28, 2025 07:48
@a18e a18e changed the title Feat/true client ip rs Retain True-Client-Ip for Route Services Mar 28, 2025
@peanball peanball added run-ci Allow this PR to be tested on Concourse and removed run-ci Allow this PR to be tested on Concourse labels Mar 28, 2025
Expect(recordedHeaders).To(HaveKey(headerKey))
Expect(recordedHeaders[headerKey]).To(HaveLen(1))
Expect(recordedHeaders[headerKey][0]).To(Equal("8.8.8.8"))
})
Copy link
Member

Choose a reason for hiding this comment

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

Please add tests for the other two cases as well.

Copy link
Contributor

Choose a reason for hiding this comment

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

Please add tests for the other two cases as well.

Done

Copy link
Contributor

@b1tamara b1tamara left a comment

Choose a reason for hiding this comment

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

Approving to run CI

Copy link
Member

@maxmoehl maxmoehl left a comment

Choose a reason for hiding this comment

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

Once merged, please cut a major release right away to make sure we don't accidentally release this with a minor / patch.

@peanball peanball merged commit 87a36db into cloudfoundry:master Apr 8, 2025
4 checks passed
@github-project-automation github-project-automation bot moved this from Pending Merge | Prioritized to Done in Application Runtime Platform Working Group Apr 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

run-ci Allow this PR to be tested on Concourse

Projects

Development

Successfully merging this pull request may close these issues.

6 participants