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
Implement HTTP header case adjustment API #496
Implement HTTP header case adjustment API #496
Conversation
828810b
to
80ce602
Compare
80ce602
to
9e20a17
Compare
9e20a17
to
b027e25
Compare
@@ -319,6 +325,11 @@ func TestDesiredRouterDeployment(t *testing.T) { | |||
|
|||
checkDeploymentHasEnvVar(t, deployment, "ROUTER_USE_PROXY_PROTOCOL", true, "true") | |||
|
|||
checkDeploymentHasEnvVar(t, deployment, "ROUTER_UNIQUE_ID_HEADER_NAME", true, "unique-id") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The env checks for ROUTER_UNIQUE_ID_HEADER_NAME
and ROUTER_UNIQUE_ID_FORMAT
were missing, and this PR just adds them independent of the header case changes, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, I snuck that in (sorry!).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No worries! Just wanted to make sure that was the case.
for _, v := range ci.Spec.HTTPHeaders.HeaderNameCaseAdjustments { | ||
adjustments = append(adjustments, string(v)) | ||
} | ||
v := strings.Join(adjustments, ",") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If Spec.HTTPHeaders.HeaderNameCaseAdjustments
has a length of 1
, the extra comma after the single entry won't break anything, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, never mind, strings.Join handles that case 😄
b027e25
to
834421e
Compare
Bump to github.com/openshift/api@c6debb38648fa2094d36ef7dd2ffc0dca97da879 to get the HTTP header case adjustment API. * go.mod: Bump. * go.sum: * manifests/00-custom-resource-definition.yaml: * manifests/00-custom-resource-definition-internal.yaml: * pkg/manifests/bindata.go: * vendor/github.com/openshift/api/*: * vendor/modules.txt: Regenerate.
* pkg/operator/controller/ingress/deployment.go (RouterHTTPHeaderNameCaseAdjustments): New constant for the related router environment variable. (desiredRouterDeployment): Set the ROUTER_H1_CASE_ADJUST environment variable as appropriate. * pkg/operator/controller/ingress/deployment_test.go (TestDesiredRouterDeployment): Verify that spec.httpHeaders.headerNameCaseAdjustments has the expected effect. * test/e2e/http_header_name_case_adjustment_test.go: New file. (TestHeaderNameCaseAdjustment): New test. Verify that spec.httpHeaders.headerNameCaseAdjustments has the expected effect. * vendor/github.com/openshift/api/*: Bump. * manifests/00-custom-resource-definition.yaml: * pkg/manifests/bindata.go: Regenerate.
834421e
to
a49c989
Compare
Rebased. |
/retitle Implement HTTP header case adjustment API |
/test e2e-aws-operator |
/retest |
|
/test e2e-aws-operator |
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: frobware, 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 |
pkg/operator/controller/ingress/deployment.go
(RouterHTTPHeaderNameCaseAdjustments
): New constant for the related router environment variable.(
desiredRouterDeployment
): Set theROUTER_H1_CASE_ADJUST
environment variable as appropriate.pkg/operator/controller/ingress/deployment_test.go
(TestDesiredRouterDeployment
): Verify thatspec.httpHeaders.headerNameCaseAdjustments
has the expected effect.vendor/github.com/openshift/api/operator/v1/*
: Bump.Depends on the following PRs: