OCPBUGS-42972: Support appProtocol: kubernetes.io/h2c by Miciah · Pull Request #627 · openshift/router

Miciah · 2024-10-10T17:16:14Z

Rename `mustCreate` to `mustCreateRoute`

Rename the mustCreate type used in tests to mustCreateRoute.

Add test cases for `appProtocol`

Define a new mustCreateEndpointSlice type for tests, and use it to define new test cases where the endpointslice associated with the route specifies appProtocol with the recognized value "h2c" and unrecognized values "unknown-value" and "kubernetes.io/h2c".

Support `appProtocol: kubernetes.io/h2c`

Recognize both "h2c" and "kubernetes.io/h2c" as valid values for appProtocol. Update the expectations for the "route with appProtocol: kubernetes.io/h2c" test case accordingly.

Follow-up to #328.

openshift-ci-robot · 2024-10-10T17:16:20Z

@Miciah: This pull request references Jira Issue OCPBUGS-42972, which is invalid:

expected the bug to target the "4.18.0" version, but no target version was set

Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.

The bug has been updated to refer to the pull request using the external bug tracker.

Details

In response to this:

Rename mustCreate to mustCreateRoute

Rename the mustCreate type used in tests to mustCreateRoute.

Add test cases for appProtocol

Define a new mustCreateEndpointSlice type for tests, and use it to define new test cases where the endpointslice associated with the route specifies appProtocol with the recognized value "h2c" and unrecognized values "unknown-value" and "kubernetes.io/h2c".

Support appProtocol: kubernetes.io/h2c

Recognize both "h2c" and "kubernetes.io/h2c" as valid values for appProtocol. Update the expectations for the "route with appProtocol: kubernetes.io/h2c" test case accordingly.

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 openshift-eng/jira-lifecycle-plugin repository.

Miciah · 2024-10-10T17:19:18Z

/jira refresh

openshift-ci-robot · 2024-10-10T17:19:26Z

@Miciah: This pull request references Jira Issue OCPBUGS-42972, which is valid. The bug has been moved to the POST state.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target version (4.18.0) matches configured target version for branch (4.18.0)
bug is in the state ASSIGNED, which is one of the valid states (NEW, ASSIGNED, POST)

Details

In response to this:

/jira refresh

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 openshift-eng/jira-lifecycle-plugin repository.

openshift-ci-robot · 2024-10-14T13:16:53Z

@Miciah: This pull request references Jira Issue OCPBUGS-42972, which is valid.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target version (4.18.0) matches configured target version for branch (4.18.0)
bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, POST)

Details

In response to this:

Rename mustCreate to mustCreateRoute

Rename the mustCreate type used in tests to mustCreateRoute.

Add test cases for appProtocol

Define a new mustCreateEndpointSlice type for tests, and use it to define new test cases where the endpointslice associated with the route specifies appProtocol with the recognized value "h2c" and unrecognized values "unknown-value" and "kubernetes.io/h2c".

Support appProtocol: kubernetes.io/h2c

Recognize both "h2c" and "kubernetes.io/h2c" as valid values for appProtocol. Update the expectations for the "route with appProtocol: kubernetes.io/h2c" test case accordingly.

Follow-up to #328.

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 openshift-eng/jira-lifecycle-plugin repository.

candita · 2024-10-16T15:39:12Z

/assign @grzpiotrowski
/assign @Miciah
/assign

candita · 2024-10-16T15:39:48Z

/unassign @Miciah
/assign @alebedev87

lihongan · 2024-10-23T02:25:25Z

@ShudiLi PTAL, thanks.
cc @melvinjoseph86 @rhamini3

ShudiLi · 2024-10-24T03:05:38Z

Tested it with 4.17.0-0.ci.test-2024-10-24-012937-ci-ln-5zxwybb-latest

1.
% oc get clusterversion
NAME      VERSION                                                   AVAILABLE   PROGRESSING   SINCE   STATUS
version   4.17.0-0.ci.test-2024-10-24-012937-ci-ln-5zxwybb-latest   True        False         42m     Cluster version is 4.17.0-0.ci.test-2024-10-24-012937-ci-ln-5zxwybb-latest

2.
% oc -n openshift-ingress rsh router-default-568cd975d7-25zg6
sh-5.1$ grep -A2 -B1 "kubernetes.io/h2c" haproxy-config.template
                {{- else if or (eq $cfg.TLSTermination "") (eq $cfg.TLSTermination "edge") }}
                  {{- if or (eq $endpoint.AppProtocol "h2c") (eq $endpoint.AppProtocol "kubernetes.io/h2c") }} proto h2
                  {{- end }}
                {{- end }}{{/* end type specific options*/}}
sh-5.1$

ShudiLi · 2024-10-30T03:31:55Z

enabled appProtocol: kubernetes.io/h2c under the unsecure service, created the edge route, then curl the route and check the packets on server side, it was http2

1.
% oc -n test get route  edge1 
NAME    HOST/PORT                                                               PATH   SERVICES      PORT          TERMINATION   WILDCARD
edge1   edge1-test.apps.ci-ln-vf9bj3k-72292.origin-ci-int-gce.dev.rhcloud.com          unsec-apach   unsec-apach   edge          None

2.
% oc -n test get svc unsec-apach -oyaml
apiVersion: v1
kind: Service
metadata:
  creationTimestamp: "2024-10-30T03:16:06Z"
  name: unsec-apach
  namespace: test
  resourceVersion: "67298"
  uid: a118660b-e3e1-41d5-ad49-9ce4658078d4
spec:
  clusterIP: 172.30.64.212
  clusterIPs:
  - 172.30.64.212
  internalTrafficPolicy: Cluster
  ipFamilies:
  - IPv4
  ipFamilyPolicy: SingleStack
  ports:
  - appProtocol: kubernetes.io/h2c
    name: unsec-apach
    port: 28080
    protocol: TCP
    targetPort: 8080
  selector:
    name: appach-server
  sessionAffinity: None
  type: ClusterIP
status:
  loadBalancer: {}

% curl --http2-prior-knowledge -k https://edge1-test.apps.ci-ln-vf9bj3k-72292.origin-ci-int-gce.dev.rhcloud.com -Ik
HTTP/2 200 
date: Wed, 30 Oct 2024 03:29:19 GMT
server: Apache/2.4.37 (centos) OpenSSL/1.1.1k
last-modified: Tue, 19 Mar 2024 10:30:40 GMT
etag: "e-61400f730f400"
accept-ranges: bytes
content-length: 14
content-type: text/html; charset=UTF-8
set-cookie: 835da53de51d7864edb325af46cf9adb=47e1f10201739bab858e1b21e58cff91; path=/; HttpOnly; Secure; SameSite=None
cache-control: private

4. check the captured packet on server side, it was http2
0.000802	10.131.0.6	10.131.0.24	HTTP2	129	Magic, SETTINGS[0]

/label qe-approved
Thanks @Miciah for helping verifying this bug!

openshift-ci-robot · 2024-10-30T03:32:01Z

@Miciah: This pull request references Jira Issue OCPBUGS-42972, which is valid.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target version (4.18.0) matches configured target version for branch (4.18.0)
bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, POST)

Requesting review from QA contact:
/cc @ShudiLi

Details

In response to this:

Rename mustCreate to mustCreateRoute

Rename the mustCreate type used in tests to mustCreateRoute.

Add test cases for appProtocol

Define a new mustCreateEndpointSlice type for tests, and use it to define new test cases where the endpointslice associated with the route specifies appProtocol with the recognized value "h2c" and unrecognized values "unknown-value" and "kubernetes.io/h2c".

Support appProtocol: kubernetes.io/h2c

Recognize both "h2c" and "kubernetes.io/h2c" as valid values for appProtocol. Update the expectations for the "route with appProtocol: kubernetes.io/h2c" test case accordingly.

Follow-up to #328.

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 openshift-eng/jira-lifecycle-plugin repository.

Miciah · 2024-11-13T16:48:31Z

/label priority/backlog

openshift-ci · 2024-11-13T16:48:44Z

@Miciah: The label(s) /label priority/backlog cannot be applied. These labels are supported: acknowledge-critical-fixes-only, platform/aws, platform/azure, platform/baremetal, platform/google, platform/libvirt, platform/openstack, ga, tide/merge-method-merge, tide/merge-method-rebase, tide/merge-method-squash, px-approved, docs-approved, qe-approved, no-qe, downstream-change-needed, rebase/manual, cluster-config-api-changed, approved, backport-risk-assessed, bugzilla/valid-bug, cherry-pick-approved, jira/valid-bug, staff-eng-approved. Is this label configured under labels -> additional_labels or labels -> restricted_labels in plugin.yaml?

Details

In response to this:

/label priority/backlog

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-sigs/prow repository.

alebedev87 · 2024-11-15T10:16:45Z

/lgtm
/approve

openshift-ci · 2024-11-15T10:17:04Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: alebedev87

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [alebedev87]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

openshift-ci-robot · 2024-11-15T11:20:00Z

/retest-required

Remaining retests: 0 against base HEAD 6c988a5 and 2 for PR HEAD 5a17c61 in total

* pkg/router/router_test.go (mustCreate): Rename... (mustCreateRoute): ... to this.

* pkg/router/router_test.go (harness): Add a client field. (TestMain): Initialize the harness client field with the Kubernetes client set. (TestConfigTemplate): Add test cases where the endpointslice associated with the route specifies appProtocol with the recognized value "h2c" and unrecognized values "unknown-value" and "kubernetes.io/h2c". Verify that the recognized value causes "proto h2" to be added to the backend server line and that the unrecognized values do not. Modify the test logic to apply an optional mustCreateEndpointSlice from the test expectations. (mustCreateRoute): Add targetServiceName field. ((mustCreateRoute).Apply): Initialize the route's spec.to.name field to the targetServiceName value from the mustCreateRoute parameters if it is specified. This new parameter isn't strictly required for the new test cases, but it makes them more explicit and easier to read. (mustCreateEndpointSlice): New type, used in the new test cases. ((mustCreateEndpointSlice).Apply): Create an endpointslice using the Kubernetes API client set from the harness and the specified endpointslice name, service name, and port appProtocol value from the mustCreateEndpointSlice parameters. (mustCreateWithConfig): Add mustCreateEndpointSlice so that the new test cases can use it. (matchConfig): Add logic for parsing the "server" attribute, which the new test cases use.

This commit fixes OCPBUGS-42972. https://issues.redhat.com/browse/OCPBUGS-42972 * images/router/haproxy/conf/haproxy-config.template: * pkg/router/template/configmanager/haproxy/backend.go ((Backend).UpdateServerInfo): * pkg/router/template/configmanager/haproxy/manager.go ((haproxyConfigManager).ReplaceRouteEndpoints): Recognize both "h2c" and "kubernetes.io/h2c" as valid values for appProtocol. * pkg/router/router_test.go (TestConfigTemplate): Update the expectations for the "route with appProtocol: kubernetes.io/h2c" test case.

Miciah · 2024-11-15T14:20:50Z

https://github.com/openshift/router/compare/5a17c618fb29df8148628327324eeef495e79a0c..a40595370fde26f1fcf4094cb8b2e21522b03686 rebases to fix a failing unit test case with the old mustCreate identifier, added in #564.

alebedev87 · 2024-11-15T14:29:23Z

/lgtm

openshift-ci · 2024-11-15T18:01:08Z

@Miciah: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/okd-scos-e2e-aws-ovn	`a405953`	link	false	`/test okd-scos-e2e-aws-ovn`

Full PR test history. Your PR dashboard.

Details

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-sigs/prow repository. I understand the commands that are listed here.

openshift-ci-robot · 2024-11-15T18:04:07Z

@Miciah: Jira Issue OCPBUGS-42972: All pull requests linked via external trackers have merged:

openshift/router#627

Jira Issue OCPBUGS-42972 has been moved to the MODIFIED state.

Details

In response to this:

Rename mustCreate to mustCreateRoute

Rename the mustCreate type used in tests to mustCreateRoute.

Add test cases for appProtocol

Define a new mustCreateEndpointSlice type for tests, and use it to define new test cases where the endpointslice associated with the route specifies appProtocol with the recognized value "h2c" and unrecognized values "unknown-value" and "kubernetes.io/h2c".

Support appProtocol: kubernetes.io/h2c

Recognize both "h2c" and "kubernetes.io/h2c" as valid values for appProtocol. Update the expectations for the "route with appProtocol: kubernetes.io/h2c" test case accordingly.

Follow-up to #328.

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 openshift-eng/jira-lifecycle-plugin repository.

openshift-bot · 2024-11-15T22:02:58Z

[ART PR BUILD NOTIFIER]

Distgit: ose-haproxy-router-base
This PR has been included in build ose-haproxy-router-base-container-v4.19.0-202411152138.p0.g7832237.assembly.stream.el9.
All builds following this will include this PR.

openshift-bot · 2024-11-15T22:15:09Z

[ART PR BUILD NOTIFIER]

Distgit: openshift-enterprise-haproxy-router
This PR has been included in build openshift-enterprise-haproxy-router-container-v4.19.0-202411152138.p0.g7832237.assembly.stream.el9.
All builds following this will include this PR.

@disabled

Root cause: - HAProxy does not support HTTP/1.1 -> HTTP/2 cleartext (h2c) upgrade - HTTP/2 over plain HTTP requires Service configuration with appProtocol: kubernetes.io/h2c (see openshift/router#627) - The test framework does not support configuring Service appProtocol - s390x OpenShift cluster does not have h2c enabled by default Attempted solutions: 1. Client h2c configuration - Failed: HAProxy blocks h2c without appProtocol 2. HTTPS with ALPN - Failed: Causes Keycloak container startup issues The test is disabled with @disabled annotation and clear documentation of the limitation until either: - Test framework adds support for Service appProtocol configuration - s390x cluster is configured to support h2c by default - Or we find a way to configure this via application properties This allows the other tests in the suite to pass while documenting the known limitation for this specific scenario. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

@disabled

…support This implements Approach B - pre-creating a Service template with the correct appProtocol configuration to enable h2c (HTTP/2 over plain HTTP) on OpenShift. Changes: 1. Added quarkus-openshift extension to pom.xml - Enables Kubernetes/OpenShift resource generation - Allows custom resource templates to be merged 2. Created src/main/kubernetes/openshift.yml - Defines Service with appProtocol: kubernetes.io/h2c - This tells OpenShift HAProxy to use "proto h2" for backend connections - Required because HAProxy doesn't support HTTP Upgrade to h2c 3. Enabled OpenShift resource generation in application.properties - Added quarkus.kubernetes.deployment-target=openshift - Quarkus will now generate and merge OpenShift resources 4. Re-enabled http2Server test with h2c client configuration - Removed @disabled annotation - Added h2c WebClientOptions (HTTP_2 protocol, SSL=false) - Added documentation explaining the appProtocol requirement How it works: - Quarkus OpenShift extension reads src/main/kubernetes/openshift.yml - Merges it with auto-generated resources during build - Deployed Service includes appProtocol: kubernetes.io/h2c - OpenShift router configures HAProxy with "proto h2" for this service - Client can now successfully negotiate HTTP/2 over plain HTTP This should work on all architectures (s390x, x86_64, ppc64le) since OpenShift router v4.x supports appProtocol h2c configuration. Reference: openshift/router#627 Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

openshift-ci Bot requested review from candita and knobunc October 10, 2024 17:16

openshift-ci-robot added jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. and removed jira/invalid-bug Indicates that a referenced Jira bug is invalid for the branch this PR is targeting. labels Oct 10, 2024

grzpiotrowski mentioned this pull request Oct 16, 2024

OCPBUGS-38078: Validate HAProxy health check interval time value #618

Merged

openshift-ci Bot assigned candita, grzpiotrowski and Miciah Oct 16, 2024

openshift-ci Bot assigned alebedev87 and unassigned Miciah Oct 16, 2024

openshift-ci Bot added the qe-approved Signifies that QE has signed off on this PR label Oct 30, 2024

openshift-ci Bot requested a review from ShudiLi October 30, 2024 03:32

alebedev87 mentioned this pull request Nov 14, 2024

NE-1815: Add verifyhost to dynamic server slots for re-encrypt routes (DCM) #637

Merged

openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Nov 15, 2024

openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 15, 2024

Miciah added 3 commits November 15, 2024 09:18

Rename mustCreate to mustCreateRoute

83087f0

* pkg/router/router_test.go (mustCreate): Rename... (mustCreateRoute): ... to this.

Miciah force-pushed the OCPBUGS-42972-support-appProtocol-kubernetes.io-slash-h2c branch from 5a17c61 to a405953 Compare November 15, 2024 14:19

openshift-ci Bot removed the lgtm Indicates that a PR is ready to be merged. label Nov 15, 2024

openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Nov 15, 2024

openshift-merge-bot Bot merged commit 7832237 into openshift:master Nov 15, 2024

Miciah mentioned this pull request Sep 4, 2026

[release-4.17] OCPBUGS-86725: Prevent SSRF via FQDN-typed EndpointSlices #835

Open

Uh oh!

Conversation

Miciah commented Oct 10, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Rename mustCreate to mustCreateRoute

Add test cases for appProtocol

Support appProtocol: kubernetes.io/h2c

Uh oh!

openshift-ci-robot commented Oct 10, 2024

Rename mustCreate to mustCreateRoute

Add test cases for appProtocol

Support appProtocol: kubernetes.io/h2c

Uh oh!

Miciah commented Oct 10, 2024

Uh oh!

openshift-ci-robot commented Oct 10, 2024

Uh oh!

openshift-ci-robot commented Oct 14, 2024

Rename mustCreate to mustCreateRoute

Add test cases for appProtocol

Support appProtocol: kubernetes.io/h2c

Uh oh!

candita commented Oct 16, 2024

Uh oh!

candita commented Oct 16, 2024

Uh oh!

lihongan commented Oct 23, 2024

Uh oh!

ShudiLi commented Oct 24, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ShudiLi commented Oct 30, 2024

Uh oh!

openshift-ci-robot commented Oct 30, 2024

Rename mustCreate to mustCreateRoute

Add test cases for appProtocol

Support appProtocol: kubernetes.io/h2c

Uh oh!

Miciah commented Nov 13, 2024

Uh oh!

openshift-ci Bot commented Nov 13, 2024

Uh oh!

alebedev87 commented Nov 15, 2024

Uh oh!

openshift-ci Bot commented Nov 15, 2024

Uh oh!

openshift-ci-robot commented Nov 15, 2024

Uh oh!

Miciah commented Nov 15, 2024

Uh oh!

alebedev87 commented Nov 15, 2024

Uh oh!

openshift-ci Bot commented Nov 15, 2024

Uh oh!

openshift-ci-robot commented Nov 15, 2024

Rename mustCreate to mustCreateRoute

Add test cases for appProtocol

Support appProtocol: kubernetes.io/h2c

Uh oh!

openshift-bot commented Nov 15, 2024

Uh oh!

openshift-bot commented Nov 15, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

Miciah commented Oct 10, 2024 •

edited

Loading

Rename `mustCreate` to `mustCreateRoute`

Add test cases for `appProtocol`

Support `appProtocol: kubernetes.io/h2c`

Rename `mustCreate` to `mustCreateRoute`

Add test cases for `appProtocol`

Support `appProtocol: kubernetes.io/h2c`

Rename `mustCreate` to `mustCreateRoute`

Add test cases for `appProtocol`

Support `appProtocol: kubernetes.io/h2c`

ShudiLi commented Oct 24, 2024 •

edited

Loading

Rename `mustCreate` to `mustCreateRoute`

Add test cases for `appProtocol`

Support `appProtocol: kubernetes.io/h2c`

Rename `mustCreate` to `mustCreateRoute`

Add test cases for `appProtocol`

Support `appProtocol: kubernetes.io/h2c`