OCPBUGS-47773: Add API validation to reject # or whitespace in spec.path #1958

Thealisyed · 2025-04-28T15:50:44Z

Api validation was added to library-go in order to reject any router that is created with a '#' or whitespace in the route spec.path # as this causes HaProxy error and the ingress to become degraded

openshift-ci-robot · 2025-04-28T15:50:51Z

@Thealisyed: This pull request references Jira Issue OCPBUGS-47773, which is invalid:

expected the bug to target the "4.19.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:

Api validation was added to library-go in order to reject any router that is created with a '#' or whitespace in the route spec.path # as this causes HaProxy error and the ingress to become degraded

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.

alebedev87 · 2025-04-29T14:05:43Z

/test unit

pkg/route/validation/validation.go

Thealisyed · 2025-05-01T11:00:06Z

/jira refresh

openshift-ci-robot · 2025-05-01T11:00:15Z

@Thealisyed: This pull request references Jira Issue OCPBUGS-47773, 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.19.0) matches configured target version for branch (4.19.0)
bug is in the state ASSIGNED, which is one of the valid states (NEW, ASSIGNED, POST)

Requesting review from QA contact:
/cc @ShudiLi

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

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.

pkg/route/validation/validation.go

Miciah · 2025-05-01T13:29:03Z

Please add test cases in TestValidateRouteUpdate.

Thealisyed · 2025-05-06T15:11:22Z

I've addressed all the review comments and added the corresponding test cases in TestValidateRouteUpdate. Please have a look and let me know if everything looks good or if any adjustments are needed.
Thanks @Miciah

ShudiLi · 2025-05-07T05:55:00Z

tested it with 4.19.0-0-2025-05-07-031421-test-ci-ln-0yn4242-latest

1. created 4 routes with path having space, # characters
% oc -n test get route
NAME          HOST/PORT                                                                     PATH                             SERVICES      PORT          TERMINATION   WILDCARD
edge1         edge1-test.apps.ci-ln-0yn4242-72292.origin-ci-int-gce.dev.rhcloud.com         /route-admission-test!#          unsec-apach   unsec-apach   edge          None
reencrypt1    reencrypt1-test.apps.ci-ln-0yn4242-72292.origin-ci-int-gce.dev.rhcloud.com    /route-admission-test1 / test1   sec-apach2    sec-apach2    reencrypt     None
reencrypt2    reencrypt2-test.apps.ci-ln-0yn4242-72292.origin-ci-int-gce.dev.rhcloud.com    /route-admission-test!#          sec-apach2    sec-apach2    reencrypt     None
unsec-apach   unsec-apach-test.apps.ci-ln-0yn4242-72292.origin-ci-int-gce.dev.rhcloud.com   /route-admission-test1 / test1   unsec-apach   unsec-apach                 None

2.
% oc -n test get route -oyaml | grep haproxy.router.openshift.io/rewrite-target
      haproxy.router.openshift.io/rewrite-target: /
      haproxy.router.openshift.io/rewrite-target: /
      haproxy.router.openshift.io/rewrite-target: /
      haproxy.router.openshift.io/rewrite-target: /

3.
% oc get co ingress
NAME      VERSION                                                AVAILABLE   PROGRESSING   DEGRADED   SINCE   MESSAGE
ingress   4.19.0-0-2025-05-07-031421-test-ci-ln-0yn4242-latest   True        False         False      118m    

4.
% oc get clusterversion
NAME      VERSION                                                AVAILABLE   PROGRESSING   SINCE   STATUS
version   4.19.0-0-2025-05-07-031421-test-ci-ln-0yn4242-latest   True        False         109m    Cluster version is 4.19.0-0-2025-05-07-031421-test-ci-ln-0yn4242-latest

5.
% oc -n test get route reencrypt3 -oyaml | grep -E "path|rewrite-target"
    haproxy.router.openshift.io/rewrite-target: /
  path: '/route-admission-test! #&*()[]-_:1{}2'

6.
% oc -n openshift-ingress logs router-default-58779dc76-p5tsj --tail=6 
I0507 08:26:00.886439       1 template_helper.go:370] "msg"="parseIPList empty list found" "logger"="template"
E0507 08:26:00.909202       1 limiter.go:165] error reloading router: exit status 1
[NOTICE]   (366) : haproxy version is 2.8.10-f28885f
[NOTICE]   (366) : path to executable is /usr/sbin/haproxy
[ALERT]    (366) : config : parsing [/var/lib/haproxy/conf/haproxy.config:353] : error detected in backend 'be_secure:test:reencrypt3' while parsing 'http-request replace-path' rule : expects exactly 2 arguments <match-regex> and <replace-format>.
[ALERT]    (366) : config : Error(s) found in configuration file : /var/lib/haproxy/conf/haproxy.config

7.
% oc get co ingress
NAME      VERSION                                                AVAILABLE   PROGRESSING   DEGRADED   SINCE   MESSAGE
ingress   4.19.0-0-2025-05-07-031421-test-ci-ln-0yn4242-latest   True        False         False      129m

ShudiLi · 2025-05-07T05:55:39Z

/label qe-approved
thanks

pkg/route/validation/validation.go

pkg/route/validation/validation_test.go

candita · 2025-05-16T18:25:20Z

pkg/route/validation/validation.go

 	hostnameUpdated := route.Spec.Host != older.Spec.Host
 	allErrs = append(allErrs, validateRoute(ctx, route, hostnameUpdated && validLabels(older.Spec.Host), sarc, secrets, opts)...)
+	if route.Spec.Path != older.Spec.Path {
+		pathFieldPath := field.NewPath("spec", "path")


nit: Ideally this pathFieldPath would be specified within validatePath instead of passed as a parameter, since it would always be the same value. I can live with it unless you need to make other changes.

The convention is that if validateFoo is validating field foo and uses validateBar to validate sub-field foo.bar, then validateFoo passes the path foo.bar to validateBar. This keeps the validation functions well scoped and reusable. For example, if you also have a validateBaz that needs to validate baz.bar, it can re-use validateBar and just pass the path baz.bar to validateBar.

candita · 2025-05-16T18:26:03Z

pkg/route/validation/validation.go

 func ValidateRoute(ctx context.Context, route *routev1.Route, sarCreator routecommon.SubjectAccessReviewCreator, secretsGetter corev1client.SecretsGetter, opts routecommon.RouteValidationOptions) field.ErrorList {
-	return validateRoute(ctx, route, true, sarCreator, secretsGetter, opts)
+	allErrs := field.ErrorList{}
+	pathFieldPath := field.NewPath("spec", "path")


nit: see https://github.com/openshift/library-go/pull/1958/files#r2093506086

pkg/route/validation/validation_test.go

candita

Almost there. Looks like you removed too much in the last change. Also, please update your commit message to include the fact that we allow old routes that had the invalid value to be updated without an error.

pkg/route/validation/validation.go

This commit introduces api validation to reject # or whitespace in the spec.path. This is becaause it causes HaProxy error and the inress to become degraded. The validation will reject any new route that tries to create with whitespace or # in spec.path. It will allow for ratcheting. Test cases are also introduced in the validation_test.go file Related bug: https://issues.redhat.com/browse/OCPBUGS-47773

Thealisyed · 2025-05-22T13:49:12Z

Updated the commit message to be more detailed

Thealisyed · 2025-06-03T08:43:06Z

Need this ART bump to be merged as there is currently golang and k8s version differences

wangke19 · 2025-06-06T04:12:49Z

Need this ART bump to be merged as there is currently golang and k8s version differences

For main/master branch, we have one pr openshift/openshift-apiserver#486, openshift/openshift-apiserver#521 is for 4.19 branch.

candita · 2025-06-07T00:03:10Z

pkg/route/validation/validation_test.go

+			name:           "spec.path should not contain whitespace in the spec.path",
+			route:          routeWithPath("/new path"),
+			expectedErrors: 1,
+		},


Sorry I didn't notice this before, but can you also test input with tabs?

candita · 2025-06-23T17:08:28Z

/lgtm
/approve

openshift-ci · 2025-06-23T17:09:26Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: candita, Thealisyed

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:

~~pkg/route/OWNERS~~ [candita]

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

openshift-ci · 2025-06-23T17:32:07Z

@Thealisyed: all tests passed!

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 · 2025-06-23T17:36:22Z

@Thealisyed: Jira Issue OCPBUGS-47773: All pull requests linked via external trackers have merged:

openshift/library-go#1958

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

Details

In response to this:

Api validation was added to library-go in order to reject any router that is created with a '#' or whitespace in the route spec.path # as this causes HaProxy error and the ingress to become degraded

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.

Thealisyed · 2025-09-12T09:24:13Z

/cherry-pick release-4.19

openshift-cherrypick-robot · 2025-09-12T09:25:04Z

@Thealisyed: new pull request created: #2012

Details

In response to this:

/cherry-pick release-4.19

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.

Thealisyed · 2026-01-21T16:42:33Z

/cherry-pick release-4.18

openshift-cherrypick-robot · 2026-01-21T16:43:25Z

@Thealisyed: new pull request created: #2083

Details

In response to this:

/cherry-pick release-4.18

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.

openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 28, 2025

openshift-ci bot requested review from Miciah and alebedev87 April 28, 2025 15:51

Thealisyed force-pushed the spec.path branch 2 times, most recently from 5400853 to f269da7 Compare April 29, 2025 15:23

Thealisyed mentioned this pull request Apr 29, 2025

[WIP] Vendoring library go for testing openshift/openshift-apiserver#506

Closed

candita reviewed Apr 29, 2025

View reviewed changes

pkg/route/validation/validation.go Show resolved Hide resolved

candita reviewed Apr 29, 2025

View reviewed changes

pkg/route/validation/validation.go Outdated Show resolved Hide resolved

Thealisyed force-pushed the spec.path branch 2 times, most recently from 80f0f67 to f53aaa9 Compare May 1, 2025 09:32

Thealisyed changed the title ~~[WIP] OCPBUGS-47773: Add API validation to reject # or whitespace in spec.path~~ OCPBUGS-47773: Add API validation to reject # or whitespace in spec.path May 1, 2025

openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 1, 2025

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 May 1, 2025

openshift-ci bot requested a review from ShudiLi May 1, 2025 11:00

Miciah reviewed May 1, 2025

View reviewed changes

pkg/route/validation/validation.go Outdated Show resolved Hide resolved

Miciah reviewed May 1, 2025

View reviewed changes

pkg/route/validation/validation.go Outdated Show resolved Hide resolved

Miciah reviewed May 1, 2025

View reviewed changes

pkg/route/validation/validation.go Outdated Show resolved Hide resolved

openshift-ci bot added the qe-approved Signifies that QE has signed off on this PR label May 7, 2025

candita reviewed May 8, 2025

View reviewed changes

pkg/route/validation/validation.go Outdated Show resolved Hide resolved

candita reviewed May 8, 2025

View reviewed changes

pkg/route/validation/validation_test.go Outdated Show resolved Hide resolved

candita reviewed May 8, 2025

View reviewed changes

pkg/route/validation/validation_test.go Outdated Show resolved Hide resolved

candita reviewed May 8, 2025

View reviewed changes

pkg/route/validation/validation_test.go Outdated Show resolved Hide resolved

Thealisyed force-pushed the spec.path branch from 2d8a5a3 to 8760bbc Compare May 8, 2025 13:08

candita reviewed May 16, 2025

View reviewed changes

pkg/route/validation/validation_test.go Show resolved Hide resolved

candita suggested changes May 16, 2025

View reviewed changes

Miciah reviewed May 19, 2025

View reviewed changes

pkg/route/validation/validation.go Outdated Show resolved Hide resolved

Thealisyed force-pushed the spec.path branch from 8760bbc to 4f43ba2 Compare May 22, 2025 13:43

candita reviewed Jun 7, 2025

View reviewed changes

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jun 23, 2025

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 23, 2025

openshift-merge-bot bot merged commit 60108c9 into openshift:master Jun 23, 2025
4 checks passed

Miciah mentioned this pull request Aug 14, 2025

OCPBUGS-47773: Bump library-go to fix route validation openshift/openshift-apiserver#535

Merged

openshift-cherrypick-robot mentioned this pull request Sep 12, 2025

[release-4.19] OCPBUGS-61662: Add API validation to reject # or whitespace in spec.path #2012

Merged

openshift-cherrypick-robot mentioned this pull request Jan 21, 2026

[release-4.18] OCPBUGS-74234: Add API validation to reject # or whitespace in spec.path #2083

Closed

OCPBUGS-47773: Add API validation to reject # or whitespace in spec.path #1958

OCPBUGS-47773: Add API validation to reject # or whitespace in spec.path #1958

Conversation

Thealisyed commented Apr 28, 2025

Uh oh!

openshift-ci-robot commented Apr 28, 2025

Uh oh!

alebedev87 commented Apr 29, 2025

Uh oh!

Uh oh!

Uh oh!

Thealisyed commented May 1, 2025

Uh oh!

openshift-ci-robot commented May 1, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Miciah commented May 1, 2025

Uh oh!

Thealisyed commented May 6, 2025

Uh oh!

ShudiLi commented May 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ShudiLi commented May 7, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

candita May 16, 2025

Choose a reason for hiding this comment

Uh oh!

Miciah May 19, 2025

Choose a reason for hiding this comment

Uh oh!

candita May 16, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

candita left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Thealisyed commented May 22, 2025

Uh oh!

Thealisyed commented Jun 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wangke19 commented Jun 6, 2025

Uh oh!

candita Jun 7, 2025

Choose a reason for hiding this comment

Uh oh!

candita commented Jun 23, 2025

Uh oh!

openshift-ci bot commented Jun 23, 2025

Uh oh!

openshift-ci bot commented Jun 23, 2025

Uh oh!

Uh oh!

openshift-ci-robot commented Jun 23, 2025

Uh oh!

Thealisyed commented Sep 12, 2025

Uh oh!

openshift-cherrypick-robot commented Sep 12, 2025

Uh oh!

Thealisyed commented Jan 21, 2026

Uh oh!

openshift-cherrypick-robot commented Jan 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

ShudiLi commented May 7, 2025 •

edited

Loading

candita left a comment •

edited

Loading

Thealisyed commented Jun 3, 2025 •

edited

Loading