feat(cors): Allowed more wildcard options

[jaynis]

A few weeks ago the allowed CORS origins have been changed from a regex to a wildcard notation (#2389). Implementation wise all kinds of wildcards are supported, however, the validation regex on the SecurityPolicy CRD limits the CORS options to hostnames prefixed with an wildcard followed by a dot, allowing all subdomains of that host. This reduces the freedom when allowing cross origins a lot compared to how it was before. This PR aims to relax the validation regex a bit to enable the following use cases:

• Allowing all hosts of an specific scheme (https://*)
• Allowing all hosts regardless of the scheme (*)
• Allowing all ports of a specific host (http://localhost:*)

While allowing all hosts in the context of CORS might sound a bit hacky, this is sometimes required. For instance when a web service provides an API which is consumed by many third-party web applications hosted under arbitrary domains not under the control of the maintainer of aforementioned web service. In addition to that it can be very useful during application development. This is why I have added the option to allow all ports of a specific host as well.

Review the new and the old validation regexes.

[shawnh2]

Suggested change

	// wildcard label such as “*.example.com”. The optional port can be a wildcard
	// wildcard label such as "*.example.com". The optional port can be a wildcard

[shawnh2]

there're still some quotes that in the wrong character encoding, can you please help fixing them? thanks

[jaynis]

done

[arkodg]

@jaynis can sign your commits and repush ? DCO is failing

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (20b8497) 64.71% compared to head (c4ff422) 64.71%.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #2453   +/-   ##
=======================================
  Coverage   64.71%   64.71%           
=======================================
  Files         115      115           
  Lines       17431    17431           
=======================================
  Hits        11281    11281           
  Misses       5433     5433           
  Partials      717      717           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[zhaohuabing]

@jaynis Thanks for the improvement in the wildcard host matching. The implementation looks good to me.

I only have a little hesitation about the port wildcard matching. Suffix/Port wildcard matching is not a common practice for hostnames. In your use case, do you have many ports for a given hostname?

[shawnh2]

lgtm, defer to @zhaohuabing

[jaynis]

Thank you for your review @zhaohuabing.

I only have a little hesitation about the port wildcard matching. Suffix/Port wildcard matching is not a common practice for hostnames. In your use case, do you have many ports for a given hostname?

The port range matching was solely meant to be a dev feature so that one can configure CORS for a host (e.g. localhost) regardless of the port the application runs on. But this scenario could be covered by the general wildcard as well, therefore I would also be fine with deleting it again if you think it is not required. Just let me know your preference.

[zhaohuabing]

Thank you for your review @zhaohuabing.

I only have a little hesitation about the port wildcard matching. Suffix/Port wildcard matching is not a common practice for hostnames. In your use case, do you have many ports for a given hostname?

The port range matching was solely meant to be a dev feature so that one can configure CORS for a host (e.g. localhost) regardless of the port the application runs on. But this scenario could be covered by the general wildcard as well, therefore I would also be fine with deleting it again if you think it is not required. Just let me know your preference.

Prefer to remove the suffix matching to keep it aligned with the common practice. Thanks.

[zhaohuabing]

LGTM. Thanks!