Formally define and validate the cluster name format #32641
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
/test |
008d7fe to
49b3df8
Compare
|
/ci-clustermesh |
Directly register the target function, rather than creating a wrapper, both for simplicity, and to make a subsequent introduction of a new parameter in ClusterInfo.Validate transparent from this point of view. Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
49b3df8 to
cc8561a
Compare
Formally define and validate that a cluster name must respect the following constraints: * It must contain at most 32 characters; * It must begin and end with a lower case alphanumeric character; * It may contain lower case alphanumerics and dashes between; * The "default" name is reserved, and forbidden with ClusterID != 0. The specification almost matches the cluster name definition from the Kubernetes multi-cluster services API [1] (except for the shorter maximum length), and derives from the already implicit requirements due to the usage of the cluster name as: * a k8s label value [2] (for CiliumIdentities), * a hostname [3] when configuring the host aliases during clustermesh interconnection; * part of TLS certificates common name [4]. The goal of the explicit validation is to ensure that Cilium components fail to start with a clear error if the cluster name is invalid, rather than failing silently at a later stage. Given the above constraints, the vast majority of existing deployments are not expected to affected by this change. Still, to enable users performing a smooth transition, we currently only emit an error log in case of invalid cluster names. The cluster name format will start being strictly enforced starting from the Cilium version. [1]: https://github.com/kubernetes/enhancements/tree/master/keps/sig-multicluster/1645-multi-cluster-services-api#proposal [2]: https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/#syntax-and-character-set [3]: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#dns-label-names [4]: https://stackoverflow.com/a/5142550 Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
Following the formalization of the cluster name format, let's additionally emit an error log when trying to connect to a cluster associated with an invalid name. Starting from v1.17, Cilium will reject connecting to a cluster with an invalid name. Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
Describe the cluster name specifications, and mimic the same checks performed by the Cilium components, to provide early feedback in case the cluster name is invalid. To enable users performing a smooth transition, helm validation can be skipped setting upgradeCompatibility to 1.15 or earlier. In that case, Cilium components will still emit error logs to warn users in case the cluster name is invalid. Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
To detect and prevent possible regressions causing valid names to not be correctly supported. Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
cc8561a to
24a384b
Compare
|
/test |
youngnick
approved these changes
Jun 4, 2024
brlbil
approved these changes
Jun 4, 2024
|
@derailed Gentle ping 🙏 |
derailed
approved these changes
Jun 7, 2024
|
Could we at least allow for 63 characters as https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#dns-label-names defined? This is because I usually use |
3 tasks
Discussion ongoing in #34006. I encourage users hitting the same limitation to leave a comment to that issue, so that we can figure out the best path forward. based on the overall consensus. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Formally define and validate that a cluster name must respect the following constraints:
The specification almost matches the cluster name definition from the Kubernetes multi-cluster services API [1] (except for the shorter maximum length), and derives from the already implicit requirements due to the usage of the cluster name as:
The goal of the explicit validation is to ensure that Cilium components fail to start with a clear error if the cluster name is invalid, rather than failing silently at a later stage.
Given the above constraints, the vast majority of existing deployments are not expected to affected by this change. Still, to enable users performing a smooth transition, we currently only emit an error log in case of invalid cluster names. The cluster name format will start being strictly enforced starting from the Cilium version. Similarly, helm validation can be temporarily skipped setting
upgradeCompatibilityto 1.15 or earlier.[1]: https://github.com/kubernetes/enhancements/tree/master/keps/sig-multicluster/1645-multi-cluster-services-api#proposal
[2]: https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/#syntax-and-character-set
[3]: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#dns-label-names
[4]: https://stackoverflow.com/a/5142550