Regression in readiness path handling causes parsing error

[georeeve]

We have recently updated Knative, and have encountered an error with our readiness probe after upgrading past version v1.12.0. This seems to occur when omitting a leading slash in the readiness HTTP path.

What version of Knative?

v12.0.0

Expected Behavior

The readiness probe passes.

Actual Behavior

The readiness probe errors with error constructing probe url parse "HTTP://127.0.0.1:3000ping": invalid port ":3000ping" after host

Steps to Reproduce the Problem

We have a readiness probe defined with the following YAML:

readiness:
  path: 'ping'
  portName: http1

Adding a leading slash to the path resolves the error:

readiness:
  path: '/ping'
  portName: http1

I don't have any experience with the Knative codebase, but I took a look at the release notes of v1.12.0 to see if I could find any related PRs. PR #14273 changed the probe path handling logic, and it looks like it could have caused this issue. @nak3 as you created that PR, do you think it could have caused this error?

I know that this release was a while ago, and that supporting paths without a leading slash may not have been originally intended; but as users may have relied on this, could the previous behaviour be reinstated to resolve this currently breaking change?

[nak3]

I don't have any experience with the Knative codebase, but I took a look at the release notes of v1.12.0 to see if I could find any related PRs. PR #14273 changed the probe path handling logic, and it looks like it could have caused this issue. @nak3 as you created that PR, do you think it could have caused this error?

Oh, I’m sorry. Yes, I think the behavior changed with #14273.
As you also mentioned, supporting paths without a leading slash may not have been originally intended, and I hadn’t anticipated such a configuration either.

I know that this release was a while ago, and that supporting paths without a leading slash may not have been originally intended; but as users may have relied on this, could the previous behaviour be reinstated to resolve this currently breaking change?

Apologies, but I have moved on to another company and project, so I leave the decision of whether to change it to the current maintainers.

[skonto]

Hi @nak3!!! Hope you are doing great :) @georeeve hi, Afaik K8s supports both types of paths, with or without a leading "/", both are valid it seems. So I think we could fix this to keep it compatible. cc @dprotaso any objections?