Add forwarded-header-policy enhancement #371
Conversation
|
|
||
| The HAProxy configuration template generates a backend configuration block for | ||
| each route based on the value in the route's annotation or the value of | ||
| `ROUTER_SET_FORWARDED_HEADERS`. To implement `append` for the `X-Forwarded-For` |
There was a problem hiding this comment.
based on the value in the route's annotation or the value of
ROUTER_SET_FORWARDED_HEADERS.
If the route's annotation and ROUTER_SET_FORWARDED_HEADERS specify a forwarded header policy, the route's annotation will override ROUTER_SET_FORWARDED_HEADERS, correct?
| The HAProxy configuration template validates the | ||
| `haproxy.router.openshift.io/set-forwarded-headers` annotation. If the | ||
| annotation specifies an invalid value, then the IngressController uses its | ||
| configured policy. |
There was a problem hiding this comment.
If the annotation specifies an invalid value, then the IngressController uses its configured policy.
Maybe we should reflect the policy used by the ingress controller in status.
There was a problem hiding this comment.
Do you mean reflect the policy in the ingress controller's status, or in the route's?
|
Issues go stale after 90d of inactivity. Mark the issue as fresh by commenting If this issue is safe to close now please do so with /lifecycle stale |
openshift-ci-robot
added
the
lifecycle/stale
|
/remove-lifecycle stale |
openshift-ci-robot
removed
the
lifecycle/stale
Miciah
force-pushed
the
add-forwarded-header-policy-enhancement
branch
from
e92b0eb
to
fbb2e2f
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Miciah The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci-robot
added
the
approved
|
Latest push changes the status to "implemented", updates the test plan and documentation status, updates the API definitions to match what was merged in openshift/api, and fixes some minor errors of spelling and grammar. |
|
Issues go stale after 90d of inactivity. Mark the issue as fresh by commenting If this issue is safe to close now please do so with /lifecycle stale |
openshift-ci-robot
added
the
lifecycle/stale
|
/remove-lifecycle stale |
openshift-ci-robot
removed
the
lifecycle/stale
|
Issues go stale after 90d of inactivity. Mark the issue as fresh by commenting If this issue is safe to close now please do so with /lifecycle stale |
openshift-ci
bot
added
the
lifecycle/stale
|
Stale issues rot after 30d of inactivity. Mark the issue as fresh by commenting If this issue is safe to close now please do so with /lifecycle rotten |
openshift-ci
bot
added
lifecycle/rotten
|
/remove-lifecycle rotten |
openshift-ci
bot
removed
the
lifecycle/rotten
| // By default, the policy is "Append". | ||
| // | ||
| // +optional | ||
| ForwardedHeaderPolicy IngressControllerHTTPHeaderPolicy `json:"forwardedHeaderPolicy,omitempty"` |
There was a problem hiding this comment.
set the default as CRD marker
There was a problem hiding this comment.
The feature was implemented in OpenShift 4.6 without merging the enhancement. (I acknowledge that the enhancement should have been merged before implementing the feature. My team only has one other reviewer, and we haven't had sufficient bandwidth to keep up with reviews, which I acknowledge is also a problem.) Is there any risk in adding the defaulting behavior to the existing, already in-use API definition?
There was a problem hiding this comment.
There is no risk. But I understand this is documentation of something that has long merged. So no worries, this is a nit.
| #### Validation | ||
|
|
||
| Omitting `spec.httpHeaders` or omitting `spec.httpHeaders.forwardedHeaderPolicy` | ||
| specifies the default behavior. |
There was a problem hiding this comment.
be precize. Is this defaulted in the CRD or is this just default behaviour of the controllers?
There was a problem hiding this comment.
It's the default behavior of the controllers.
|
Sgtm. Some team member has to lgtm. |
openshift-ci
bot
added
the
lifecycle/rotten
|
Rotten enhancement proposals close after 7d of inactivity. See https://github.com/openshift/enhancements#life-cycle for details. Reopen the proposal by commenting /close |
|
@openshift-bot: Closed this PR. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
/reopen |
|
@Miciah: Reopened this PR. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
openshift-ci
bot
removed
the
lifecycle/rotten
|
[APPROVALNOTIFIER] This PR is APPROVED Approval requirements bypassed by manually added approval. This pull-request has been approved by: The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
Inactive enhancement proposals go stale after 28d of inactivity. See https://github.com/openshift/enhancements#life-cycle for details. Mark the proposal as fresh by commenting If this proposal is safe to close now please do so with /lifecycle stale |
openshift-ci
bot
added
the
lifecycle/stale
|
Stale enhancement proposals rot after 7d of inactivity. See https://github.com/openshift/enhancements#life-cycle for details. Mark the proposal as fresh by commenting If this proposal is safe to close now please do so with /lifecycle rotten |
openshift-ci
bot
added
lifecycle/rotten
|
Rotten enhancement proposals close after 7d of inactivity. See https://github.com/openshift/enhancements#life-cycle for details. Reopen the proposal by commenting /close |
|
@openshift-bot: Closed this PR. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
/reopen |
|
@Miciah: Reopened this PR. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
openshift-ci
bot
removed
the
lifecycle/rotten
|
@Miciah: The following test failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
|
Inactive enhancement proposals go stale after 28d of inactivity. See https://github.com/openshift/enhancements#life-cycle for details. Mark the proposal as fresh by commenting If this proposal is safe to close now please do so with /lifecycle stale |
openshift-ci
bot
added
the
lifecycle/stale
|
Stale enhancement proposals rot after 7d of inactivity. See https://github.com/openshift/enhancements#life-cycle for details. Mark the proposal as fresh by commenting If this proposal is safe to close now please do so with /lifecycle rotten |
openshift-ci
bot
added
lifecycle/rotten
|
Rotten enhancement proposals close after 7d of inactivity. See https://github.com/openshift/enhancements#life-cycle for details. Reopen the proposal by commenting /close |
|
@openshift-bot: Closed this PR. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
No description provided.