cloudoperators / greenhouse

Cloud operations platform
https://cloudoperators.github.io/greenhouse/
Apache License 2.0
14 stars 2 forks source link

[FEAT] - Validation and Generation of ServiceProxy URLs #78

Closed IvoGoman closed 1 day ago

IvoGoman commented 7 months ago

Priority

(Medium) I'm annoyed but I'll live

Description

The service-proxy allows to expose Kubernetes Service resources from an onboarded cluster. In the past this has caused troubles if the combined $service-$namespace-$cluster is longer than 63 characters.

The labels must follow the rules for ARPANET host names. They must start with a letter, end with a letter or digit, and have as interior characters only letters, digits, and hyphen. There are also some restrictions on the length. Labels must be 63 characters or less. RFC1035

From an initial discussion with @databus23 this solution is proposed:

With some proposals for the generated name:

  1. We should an additional annotation to the service that can specify the service name. The name of service object is to long as it contains the helm release. E.g. kube-monitoring-external-prometheus --> external-prometheus. So by specifying something like greenhouse.sap/expose-name: "external-prometheus" in addition to greenhouse.sap/expose: "true" we can generate a url with a shorter more meaningful name
  2. We should maybe constraint the length of cluster names so that we don't end up with names like garden-greenhouse--monitoring-external. This cluster should really be named monitoring-external in the greenhouse context.
  3. As long as there isn't a clash of service names in a cluster (some service name in multiple namespaces) we use $service--$cluster as the hostname. If there is a clash we add the namespace to the hostname: $service--$cluster--$namespace. This might introduce a change in urls for an existing service if a second one with the same name is created in the cluster but at least we can resolve the conflict.
  4. If the resulting string is > 63 characters we hash the name with something short similar to how the deployment hashes are generated and we take a 52 character prefix from the generated name and add "-[10 digit hash]" to it. Because we changed the ordering of things in 3. we have a higher chance that service and cluster name will still be visible and only the namespace will be replaced by a hash.

Reference Issues

No response

uwe-mayer commented 1 month ago

With introducing a first solution to capping the host: https://github.com/cloudoperators/greenhouse/pull/611 We introduced a regression because we changed order of service, cluster and namespace in the host. The decomposition was decoupled of the URl creation and therefore broke. We hotfixed here: https://github.com/cloudoperators/greenhouse/pull/617

But still need:

Also see @abhijith-darshan comment for reference