Overhaul: Part 1: Making Kubernetes naming scheme exchangeable#209
Overhaul: Part 1: Making Kubernetes naming scheme exchangeable#209xrstf wants to merge 13 commits into
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Extracts the existing Kubernetes object naming logic into an interchangeable naming.Scheme (v1) and threads it through controllers/resources/tests as groundwork for a future conflict-free naming v2 (epic #164), aiming to keep current operator behavior unchanged.
Changes:
- Introduces
internal/resources/namingwith aSchemeinterface and a v1 implementation mirroring the current naming behavior. - Refactors controllers and resource reconcilers to accept and use a
naming.Schemeinstead of globalresources.Get*helpers. - Updates e2e and utility test code to use the injected naming scheme for object names/labels.
Reviewed changes
Copilot reviewed 57 out of 57 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| test/utils/utils.go | Uses naming scheme for root shard proxy connectivity; adds helper to compute FP hostname via scheme. |
| test/utils/deploy.go | Passes naming scheme into deploy helpers and uses it for base hosts. |
| test/e2e/virtualworkspaces/virtualworkspaces_test.go | Replaces hardcoded names/labels with naming scheme calls; updates pod selection labels. |
| test/e2e/shards/shards_test.go | Uses naming scheme for bundle name and expected object names. |
| test/e2e/rootshards/rootshards_test.go | Passes naming scheme into root shard deploy helper. |
| test/e2e/rootshards/proxy_test.go | Uses naming scheme for FP hostname and deploy calls. |
| test/e2e/kubeconfig-rbac/frontproxies_test.go | Uses naming scheme for FP hostname and deploy calls. |
| test/e2e/frontproxies/frontproxies_test.go | Uses naming scheme for FP hostname and deploy calls. |
| test/e2e/cacheserver/cacheserver_test.go | Passes naming scheme into root shard deploy calls. |
| internal/resources/virtualworkspace/service.go | Injects naming scheme; uses it for service name and labels. |
| internal/resources/virtualworkspace/deployment.go | Injects naming scheme; uses it for names, labels, secret mounts, and URLs. |
| internal/resources/virtualworkspace/certificate.go | Injects naming scheme; uses it for cert/issuer/labels/DNS. |
| internal/resources/utils/deployments.go | Plumbs naming scheme into front-proxy auth config path. |
| internal/resources/utils/authentication_test.go | Updates expectations to use v1 naming scheme. |
| internal/resources/utils/authentication.go | Injects naming scheme for shard/root shard cert secret names. |
| internal/resources/shard/service.go | Injects naming scheme; uses it for service name and labels. |
| internal/resources/shard/kubeconfigs.go | Injects naming scheme; uses it for kubeconfig secret names and server URLs. |
| internal/resources/shard/deployment.go | Injects naming scheme; uses it for names, labels, secret mounts, and args. |
| internal/resources/shard/certificates.go | Injects naming scheme; uses it for cert names, labels, issuers, DNS. |
| internal/resources/shard/ca_bundle.go | Injects naming scheme; uses it for merged bundle and referenced secret names. |
| internal/resources/rootshard/service.go | Injects naming scheme; uses it for service name and labels. |
| internal/resources/rootshard/kubeconfigs.go | Injects naming scheme; uses it for kubeconfig secret names and server URLs. |
| internal/resources/rootshard/issuers.go | Injects naming scheme; uses it for issuer names and labels. |
| internal/resources/rootshard/deployment.go | Injects naming scheme; uses it for names, labels, mounts, and args. |
| internal/resources/rootshard/certificates.go | Injects naming scheme; uses it for cert names, labels, issuers, DNS. |
| internal/resources/rootshard/ca_certificates.go | Injects naming scheme; uses it for CA cert names and issuer refs. |
| internal/resources/rootshard/ca_bundle.go | Injects naming scheme; uses it for merged bundle and referenced secret names. |
| internal/resources/resources.go | Removes global naming helpers/constants now encapsulated by naming scheme. |
| internal/resources/naming/v1.go | Adds v1 naming scheme containing the legacy naming behavior. |
| internal/resources/naming/scheme.go | Adds scheme interface and fqdn helper with default cluster domain. |
| internal/resources/kubeconfig/secret.go | Injects naming scheme so kubeconfig targets resolve base URLs via scheme. |
| internal/resources/kubeconfig/certificate.go | Adjusts kubeconfig certificate secret template labels. |
| internal/resources/frontproxy/service.go | Uses naming scheme for front-proxy/root-shard-proxy service naming. |
| internal/resources/frontproxy/secrets.go | Uses naming scheme for dynamic kubeconfig secret naming and server URL. |
| internal/resources/frontproxy/reconciler.go | Stores naming scheme on reconciler; constructors now require scheme. |
| internal/resources/frontproxy/deployment_test.go | Updates expectations and constructors to use v1 naming scheme. |
| internal/resources/frontproxy/deployment.go | Uses naming scheme for deployment name, mounts, auth config integration. |
| internal/resources/frontproxy/configmap_test.go | Updates constructors to use v1 naming scheme. |
| internal/resources/frontproxy/configmap.go | Uses naming scheme for configmap naming and root shard base URL. |
| internal/resources/frontproxy/certificates.go | Uses naming scheme for cert names and issuer refs; improves DNS for cluster domain. |
| internal/resources/frontproxy/ca_bundle.go | Uses naming scheme for merged CA/bundle naming and CA secret references. |
| internal/resources/cacheserver/service.go | Injects naming scheme; uses it for service name and labels. |
| internal/resources/cacheserver/kubeconfigs.go | Injects naming scheme; uses it for kubeconfig secret name and base URL. |
| internal/resources/cacheserver/issuers.go | Injects naming scheme; uses it for issuer name and labels. |
| internal/resources/cacheserver/deployment.go | Injects naming scheme; uses it for deployment naming and secret mounts. |
| internal/resources/cacheserver/certificates.go | Injects naming scheme; uses it for cert names, labels, issuers, DNS. |
| internal/controller/virtualworkspace/controller.go | Instantiates v1 scheme; threads scheme into reconcile/status and issuer mapping. |
| internal/controller/shard/controller.go | Instantiates v1 scheme; threads scheme into reconcile/status/deletion/bundle handling. |
| internal/controller/rootshard/controller.go | Instantiates v1 scheme; threads scheme into reconcile/status/bundle/proxy reconciliation. |
| internal/controller/kubeconfig/controller.go | Instantiates v1 scheme; threads scheme into kubeconfig cert + secret reconciliation. |
| internal/controller/frontproxy/controller.go | Instantiates v1 scheme; threads scheme into reconcile/status/bundle handling. |
| internal/controller/cacheserver/controller.go | Instantiates v1 scheme; threads scheme into cacheserver reconcilers. |
| internal/controller/bundle/objects.go | Refactors bundle object name generation to use naming scheme. |
| internal/controller/bundle/helpers.go | Refactors bundle naming/ready condition to use naming scheme. |
| internal/controller/bundle/controller.go | Instantiates v1 scheme to compute required bundle objects. |
| internal/client/frontproxy.go | Instantiates v1 scheme and passes it to client constructors. |
| internal/client/clients.go | Adds naming scheme plumbing into client construction and TLS secret lookup. |
Comments suppressed due to low confidence (1)
test/utils/utils.go:1
- This helper hardcodes object names (
front-proxy,r00t) and relies on them staying in sync with deploy helpers. To reduce test brittleness, consider passing the actualFrontProxy/RootShardobjects (or their names) into this function, or deriving these names from the deploy helpers’ return values.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 58 out of 58 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Adds --requestheader-allowed-names to RootShard, Shard and VirtualWorkspace deployments. k/k 1.35 requires --requestheader-allowed-names to be passed if --requestheader-client-ca-file and --client-ca-file point to the same cert. On-behalf-of: @SAP robert.vasek@sap.com Signed-off-by: Robert Vasek <robert.vasek@clyso.com>
Fixes kcp-dev#213 When spec.shardBaseURL is set to a value different from spec.external.hostname, the shardBaseURL hostname is now extracted and included in the server certificate's DNS Subject Alternative Names. This prevents TLS certificate validation errors when clients connect using the shardBaseURL instead of the external hostname. Changes: - Add ExtractHostnameFromURL utility function to parse hostnames from URLs - Update RootShard server certificates to include shardBaseURL hostname - Update Shard server certificates to include shardBaseURL hostname - Add comprehensive tests for URL hostname extraction - Add tests for DNS name building in both RootShard and Shard certificates On-behalf-of: @SAP christoph.mewes@sap.com
On-behalf-of: @SAP christoph.mewes@sap.com
On-behalf-of: @SAP christoph.mewes@sap.com
…me to prepare for the overhaul On-behalf-of: @SAP christoph.mewes@sap.com
On-behalf-of: @SAP christoph.mewes@sap.com
On-behalf-of: @SAP christoph.mewes@sap.com
…ong certs in the FP bundle On-behalf-of: @SAP christoph.mewes@sap.com
|
@xrstf: The following test failed, say
DetailsInstructions 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. |
Summary
This is part of the naming overhaul epic (#164).
My current plan is roughly like this:
resources.GetSomethingName()into a func on a type)Since it will take me some time to build up the necessary changes, I am planning to build up this feature in multiple PRs, all targetting the new
naming-v2branch. Once it's ready, we can decide when to merge it to main and ship it to the endusers.This is PR number 1: Extracting the existing naming scheme. This PR doesn't change the operator's behaviour at all, but has extracted the code to make it interchangeable. During development on this PR I defined a "chaos" scheme where every object got a
chaos-prefix to spot all places in the entire codebase where we accidentally hardcoded the old v1 naming scheme. I think this is somewhat valuable, but am not yet sure how we could have an e2e test with "chaos mode" yet.The idea is that the naming scheme kind of sort of later depends on runtime state and where the user wants to migrate which component, and so therefore I am not planning on having one "global" naming scheme. Instead during the migration I think most controllers will instantiate the schemes as necessary.
What Type of PR Is This?
/kind cleanup
Release Notes