From 7667333d1be9514ddb4204264eec88015876267a Mon Sep 17 00:00:00 2001 From: "Dr. Stefan Schimanski" Date: Fri, 10 May 2024 13:34:25 +0200 Subject: [PATCH 1/3] apis/tenancy: make Workspace.Spec.Cluster a logicalcluster.Name Signed-off-by: Dr. Stefan Schimanski --- .../crdnooverlappinggvr_admission.go | 2 +- pkg/index/index.go | 14 +++++++------- pkg/index/index_test.go | 6 +++--- .../workspace/workspace_reconcile_deletion.go | 2 +- .../workspace/workspace_reconcile_phase.go | 4 ++-- .../workspace_reconcile_scheduling.go | 2 +- pkg/server/home_workspaces.go | 2 +- sdk/apis/tenancy/v1alpha1/types_workspace.go | 4 +++- test/e2e/apibinding/apibinding_test.go | 6 +++--- test/e2e/audit/audit_log_test.go | 4 ++-- test/e2e/authorizer/authorizer_test.go | 2 +- .../cross_logical_cluster_list_test.go | 4 ++-- test/e2e/conformance/webhook_test.go | 2 +- test/e2e/framework/kcp.go | 2 +- test/e2e/framework/workspaces.go | 2 +- test/e2e/reconciler/cache/replication_test.go | 4 ++-- .../workspacedeletion/controller_test.go | 3 +-- test/e2e/virtual/apiexport/authorizer_test.go | 6 +++--- test/e2e/virtual/apiexport/binding_test.go | 5 ++--- .../virtual/apiexport/virtualworkspace_test.go | 18 +++++++++--------- .../virtualworkspace_test.go | 12 ++++++------ test/e2e/workspacetype/controller_test.go | 4 ++-- 22 files changed, 55 insertions(+), 55 deletions(-) diff --git a/pkg/admission/crdnooverlappinggvr/crdnooverlappinggvr_admission.go b/pkg/admission/crdnooverlappinggvr/crdnooverlappinggvr_admission.go index d64967387ae..6f97a1e5614 100644 --- a/pkg/admission/crdnooverlappinggvr/crdnooverlappinggvr_admission.go +++ b/pkg/admission/crdnooverlappinggvr/crdnooverlappinggvr_admission.go @@ -82,7 +82,7 @@ func (p *crdNoOverlappingGVRAdmission) Validate(ctx context.Context, a admission if err != nil { return fmt.Errorf("failed to retrieve cluster from context: %w", err) } - clusterName := logicalcluster.Name(cluster.String()) // TODO(sttts): remove this cast once ClusterNameFrom returns a tenancy.Name + clusterName := cluster // ignore CRDs targeting system and non-root workspaces if clusterName == apibinding.SystemBoundCRDsClusterName || clusterName == apiserver.LocalAdminCluster { return nil diff --git a/pkg/index/index.go b/pkg/index/index.go index f3d22fe2a61..c0bbfa54961 100644 --- a/pkg/index/index.go +++ b/pkg/index/index.go @@ -92,14 +92,14 @@ func (c *State) UpsertWorkspace(shard string, ws *tenancyv1alpha1.Workspace) { // 1. cluster name is different // 2. mount object string is different (updated, added, or removed) // When we promote this to workspace structure, we should make this check smarter and better tested. - if (cluster.String() == ws.Spec.Cluster) && (ws.Annotations[tenancyv1alpha1.ExperimentalWorkspaceMountAnnotationKey] != "" && mountObjString == ws.Annotations[tenancyv1alpha1.ExperimentalWorkspaceMountAnnotationKey]) { + if (cluster == ws.Spec.Cluster) && (ws.Annotations[tenancyv1alpha1.ExperimentalWorkspaceMountAnnotationKey] != "" && mountObjString == ws.Annotations[tenancyv1alpha1.ExperimentalWorkspaceMountAnnotationKey]) { return } c.lock.Lock() defer c.lock.Unlock() - if cluster := c.shardWorkspaceNameCluster[shard][clusterName][ws.Name]; cluster.String() != ws.Spec.Cluster { + if cluster := c.shardWorkspaceNameCluster[shard][clusterName][ws.Name]; cluster != ws.Spec.Cluster { if c.shardWorkspaceNameCluster[shard] == nil { c.shardWorkspaceNameCluster[shard] = map[logicalcluster.Name]map[string]logicalcluster.Name{} c.shardWorkspaceName[shard] = map[logicalcluster.Name]string{} @@ -108,9 +108,9 @@ func (c *State) UpsertWorkspace(shard string, ws *tenancyv1alpha1.Workspace) { if c.shardWorkspaceNameCluster[shard][clusterName] == nil { c.shardWorkspaceNameCluster[shard][clusterName] = map[string]logicalcluster.Name{} } - c.shardWorkspaceNameCluster[shard][clusterName][ws.Name] = logicalcluster.Name(ws.Spec.Cluster) - c.shardWorkspaceName[shard][logicalcluster.Name(ws.Spec.Cluster)] = ws.Name - c.shardClusterParentCluster[shard][logicalcluster.Name(ws.Spec.Cluster)] = clusterName + c.shardWorkspaceNameCluster[shard][clusterName][ws.Name] = ws.Spec.Cluster + c.shardWorkspaceName[shard][ws.Spec.Cluster] = ws.Name + c.shardClusterParentCluster[shard][ws.Spec.Cluster] = clusterName } if mountObjString := c.clusterWorkspaceMountAnnotation[clusterName][ws.Name]; mountObjString != ws.Annotations[tenancyv1alpha1.ExperimentalWorkspaceMountAnnotationKey] { @@ -145,12 +145,12 @@ func (c *State) DeleteWorkspace(shard string, ws *tenancyv1alpha1.Workspace) { delete(c.shardWorkspaceNameCluster, shard) } - delete(c.shardWorkspaceName[shard], logicalcluster.Name(ws.Spec.Cluster)) + delete(c.shardWorkspaceName[shard], ws.Spec.Cluster) if len(c.shardWorkspaceName[shard]) == 0 { delete(c.shardWorkspaceName, shard) } - delete(c.shardClusterParentCluster[shard], logicalcluster.Name(ws.Spec.Cluster)) + delete(c.shardClusterParentCluster[shard], ws.Spec.Cluster) if len(c.shardClusterParentCluster[shard]) == 0 { delete(c.shardClusterParentCluster, shard) } diff --git a/pkg/index/index_test.go b/pkg/index/index_test.go index 9a6db887efa..7ac1b821b99 100644 --- a/pkg/index/index_test.go +++ b/pkg/index/index_test.go @@ -487,15 +487,15 @@ func validateLookupOutput(t *testing.T, path logicalcluster.Path, shard string, } } -func newWorkspace(name, cluster, scheduledCluster string) *tenancyv1alpha1.Workspace { +func newWorkspace(name string, cluster, scheduledCluster logicalcluster.Name) *tenancyv1alpha1.Workspace { return &tenancyv1alpha1.Workspace{ - ObjectMeta: metav1.ObjectMeta{Name: name, Annotations: map[string]string{"kcp.io/cluster": cluster}}, + ObjectMeta: metav1.ObjectMeta{Name: name, Annotations: map[string]string{"kcp.io/cluster": cluster.String()}}, Spec: tenancyv1alpha1.WorkspaceSpec{Cluster: scheduledCluster}, Status: tenancyv1alpha1.WorkspaceStatus{Phase: corev1alpha1.LogicalClusterPhaseReady}, } } -func newWorkspaceWithAnnotation(name, cluster, scheduledCluster string, annotations map[string]string) *tenancyv1alpha1.Workspace { +func newWorkspaceWithAnnotation(name string, cluster, scheduledCluster logicalcluster.Name, annotations map[string]string) *tenancyv1alpha1.Workspace { ws := newWorkspace(name, cluster, scheduledCluster) for k, v := range annotations { ws.Annotations[k] = v diff --git a/pkg/reconciler/tenancy/workspace/workspace_reconcile_deletion.go b/pkg/reconciler/tenancy/workspace/workspace_reconcile_deletion.go index 28ee179fa76..6ecd8979982 100644 --- a/pkg/reconciler/tenancy/workspace/workspace_reconcile_deletion.go +++ b/pkg/reconciler/tenancy/workspace/workspace_reconcile_deletion.go @@ -49,7 +49,7 @@ func (r *deletionReconciler) reconcile(ctx context.Context, workspace *tenancyv1 // If the logical cluster hasn't been created yet, we have to look at the annotation to // get the cluster name, instead of looking at status. - clusterName := logicalcluster.Name(workspace.Spec.Cluster) + clusterName := workspace.Spec.Cluster if workspace.Status.Phase == corev1alpha1.LogicalClusterPhaseScheduling { a, ok := workspace.Annotations[workspaceClusterAnnotationKey] if !ok { diff --git a/pkg/reconciler/tenancy/workspace/workspace_reconcile_phase.go b/pkg/reconciler/tenancy/workspace/workspace_reconcile_phase.go index 99648ff7974..31e461ed77c 100644 --- a/pkg/reconciler/tenancy/workspace/workspace_reconcile_phase.go +++ b/pkg/reconciler/tenancy/workspace/workspace_reconcile_phase.go @@ -48,7 +48,7 @@ func (r *phaseReconciler) reconcile(ctx context.Context, workspace *tenancyv1alp case corev1alpha1.LogicalClusterPhaseInitializing: logger = logger.WithValues("cluster", workspace.Spec.Cluster) - logicalCluster, err := r.getLogicalCluster(ctx, logicalcluster.NewPath(workspace.Spec.Cluster)) + logicalCluster, err := r.getLogicalCluster(ctx, workspace.Spec.Cluster.Path()) if err != nil && !apierrors.IsNotFound(err) { return reconcileStatusStopAndRequeue, err } else if apierrors.IsNotFound(err) { @@ -78,7 +78,7 @@ func (r *phaseReconciler) reconcile(ctx context.Context, workspace *tenancyv1alp if !workspace.DeletionTimestamp.IsZero() { logger = logger.WithValues("cluster", workspace.Spec.Cluster) - logicalCluster, err := r.getLogicalCluster(ctx, logicalcluster.NewPath(workspace.Spec.Cluster)) + logicalCluster, err := r.getLogicalCluster(ctx, workspace.Spec.Cluster.Path()) if err != nil && !apierrors.IsNotFound(err) { return reconcileStatusStopAndRequeue, err } else if apierrors.IsNotFound(err) { diff --git a/pkg/reconciler/tenancy/workspace/workspace_reconcile_scheduling.go b/pkg/reconciler/tenancy/workspace/workspace_reconcile_scheduling.go index 03a1fbc567f..5b6f51b296b 100644 --- a/pkg/reconciler/tenancy/workspace/workspace_reconcile_scheduling.go +++ b/pkg/reconciler/tenancy/workspace/workspace_reconcile_scheduling.go @@ -179,7 +179,7 @@ func (r *schedulingReconciler) reconcile(ctx context.Context, workspace *tenancy return reconcileStatusStopAndRequeue, err // requeue } u.Path = path.Join(u.Path, canonicalPath.RequestPath()) - workspace.Spec.Cluster = clusterName.String() + workspace.Spec.Cluster = clusterName workspace.Spec.URL = u.String() logging.WithObject(logger, shard).Info("scheduled workspace to shard") return reconcileStatusStopAndRequeue, nil diff --git a/pkg/server/home_workspaces.go b/pkg/server/home_workspaces.go index cd643ab8f48..23558cbc008 100644 --- a/pkg/server/home_workspaces.go +++ b/pkg/server/home_workspaces.go @@ -307,7 +307,7 @@ func (h *homeWorkspaceHandler) ServeHTTP(rw http.ResponseWriter, req *http.Reque CreationTimestamp: logicalCluster.CreationTimestamp, }, Spec: tenancyv1alpha1.WorkspaceSpec{ - Cluster: logicalcluster.From(logicalCluster).String(), + Cluster: logicalcluster.From(logicalCluster), URL: logicalCluster.Status.URL, }, Status: tenancyv1alpha1.WorkspaceStatus{ diff --git a/sdk/apis/tenancy/v1alpha1/types_workspace.go b/sdk/apis/tenancy/v1alpha1/types_workspace.go index 199661c482b..b660dd748c5 100644 --- a/sdk/apis/tenancy/v1alpha1/types_workspace.go +++ b/sdk/apis/tenancy/v1alpha1/types_workspace.go @@ -19,6 +19,8 @@ package v1alpha1 import ( "fmt" + "github.com/kcp-dev/logicalcluster/v3" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" corev1alpha1 "github.com/kcp-dev/kcp/sdk/apis/core/v1alpha1" @@ -168,7 +170,7 @@ type WorkspaceSpec struct { // // +optional // +kubebuilder:validation:XValidation:rule="self == oldSelf",message="cluster is immutable" - Cluster string `json:"cluster,omitempty"` + Cluster logicalcluster.Name `json:"cluster,omitempty"` // URL is the address under which the Kubernetes-cluster-like endpoint // can be found. This URL can be used to access the workspace with standard Kubernetes diff --git a/test/e2e/apibinding/apibinding_test.go b/test/e2e/apibinding/apibinding_test.go index c71825c85d4..3ff29d08653 100644 --- a/test/e2e/apibinding/apibinding_test.go +++ b/test/e2e/apibinding/apibinding_test.go @@ -148,13 +148,13 @@ func TestAPIBinding(t *testing.T) { orgPath, _ := framework.NewOrganizationFixture(t, server) provider1Path, provider1 := framework.NewWorkspaceFixture(t, server, orgPath, framework.WithName("service-provider-1")) - provider1ClusterName := logicalcluster.Name(provider1.Spec.Cluster) + provider1ClusterName := provider1.Spec.Cluster provider2Path, provider2 := framework.NewWorkspaceFixture(t, server, orgPath, framework.WithName("service-provider-2")) - provider2ClusterName := logicalcluster.Name(provider2.Spec.Cluster) + provider2ClusterName := provider2.Spec.Cluster consumer1Path, _ := framework.NewWorkspaceFixture(t, server, orgPath, framework.WithName("consumer-1-bound-against-1")) consumer2Path, _ := framework.NewWorkspaceFixture(t, server, orgPath, framework.WithName("consumer-2-bound-against-1")) consumer3Path, consumer3Workspace := framework.NewWorkspaceFixture(t, server, orgPath, framework.WithName("consumer-3-bound-against-2")) - consumer3ClusterName := logicalcluster.Name(consumer3Workspace.Spec.Cluster) + consumer3ClusterName := consumer3Workspace.Spec.Cluster cfg := server.BaseConfig(t) diff --git a/test/e2e/audit/audit_log_test.go b/test/e2e/audit/audit_log_test.go index 5f054b9b4f9..67a9ec4ac4b 100644 --- a/test/e2e/audit/audit_log_test.go +++ b/test/e2e/audit/audit_log_test.go @@ -69,8 +69,8 @@ func TestAuditLogs(t *testing.T) { require.NoError(t, err, "Error parsing JSON data") workspaceNameSent := ws.Spec.Cluster - require.Equal(t, workspaceNameSent, requestAuditEvent.Annotations["tenancy.kcp.io/workspace"]) - require.Equal(t, workspaceNameSent, responseAuditEvent.Annotations["tenancy.kcp.io/workspace"]) + require.Equal(t, workspaceNameSent.String(), requestAuditEvent.Annotations["tenancy.kcp.io/workspace"]) + require.Equal(t, workspaceNameSent.String(), responseAuditEvent.Annotations["tenancy.kcp.io/workspace"]) for _, annotation := range []string{ "request.auth.kcp.io/01-requiredgroups", diff --git a/test/e2e/authorizer/authorizer_test.go b/test/e2e/authorizer/authorizer_test.go index 33859033f03..9d47889d972 100644 --- a/test/e2e/authorizer/authorizer_test.go +++ b/test/e2e/authorizer/authorizer_test.go @@ -265,7 +265,7 @@ func TestAuthorizer(t *testing.T) { deepSARClient, err := kcpkubernetesclientset.NewForConfig(authorization.WithDeepSARConfig(rest.CopyConfig(server.RootShardSystemMasterBaseConfig(t)))) require.NoError(t, err) framework.Eventually(t, func() (bool, string) { - resp, err = deepSARClient.Cluster(logicalcluster.NewPath(org2Workspace1.Spec.Cluster)).AuthorizationV1().SubjectAccessReviews().Create(ctx, sar, metav1.CreateOptions{}) + resp, err = deepSARClient.Cluster(org2Workspace1.Spec.Cluster.Path()).AuthorizationV1().SubjectAccessReviews().Create(ctx, sar, metav1.CreateOptions{}) if err != nil { return false, fmt.Sprintf("failed to create SAR: %v", err) } diff --git a/test/e2e/conformance/cross_logical_cluster_list_test.go b/test/e2e/conformance/cross_logical_cluster_list_test.go index 6e237f6c277..26110e27e8f 100644 --- a/test/e2e/conformance/cross_logical_cluster_list_test.go +++ b/test/e2e/conformance/cross_logical_cluster_list_test.go @@ -70,8 +70,8 @@ func TestCrossLogicalClusterList(t *testing.T) { _, ws1 := framework.NewOrganizationFixture(t, server, framework.WithRootShard()) _, ws2 := framework.NewOrganizationFixture(t, server, framework.WithRootShard()) logicalClusters := []logicalcluster.Name{ - logicalcluster.Name(ws1.Spec.Cluster), - logicalcluster.Name(ws2.Spec.Cluster), + ws1.Spec.Cluster, + ws2.Spec.Cluster, } expectedWorkspaces := sets.New[string]() for i, clusterName := range logicalClusters { diff --git a/test/e2e/conformance/webhook_test.go b/test/e2e/conformance/webhook_test.go index 825ab95e7a4..da32d882bb6 100644 --- a/test/e2e/conformance/webhook_test.go +++ b/test/e2e/conformance/webhook_test.go @@ -77,7 +77,7 @@ func TestMutatingWebhookInWorkspace(t *testing.T) { if err := json.Unmarshal(review.Request.Object.Raw, &u.Object); err != nil { return nil, err } - clusterInReviewObject.Store(logicalcluster.From(&u).String()) + clusterInReviewObject.Store(logicalcluster.From(&u)) return &v1.AdmissionResponse{Allowed: true}, nil }, ObjectGVK: schema.GroupVersionKind{ diff --git a/test/e2e/framework/kcp.go b/test/e2e/framework/kcp.go index 43c260ea4bc..20f94450e91 100644 --- a/test/e2e/framework/kcp.go +++ b/test/e2e/framework/kcp.go @@ -987,7 +987,7 @@ func NewFakeWorkloadServer(t *testing.T, server RunningServer, org logicalcluste t.Helper() path, ws := NewWorkspaceFixture(t, server, org, WithName(syncTargetName+"-sink"), TODO_WithoutMultiShardSupport()) - logicalClusterName := logicalcluster.Name(ws.Spec.Cluster) + logicalClusterName := ws.Spec.Cluster rawConfig, err := server.RawConfig() require.NoError(t, err, "failed to read config for server") logicalConfig, kubeconfigPath := WriteLogicalClusterConfig(t, rawConfig, "base", path) diff --git a/test/e2e/framework/workspaces.go b/test/e2e/framework/workspaces.go index bebb8009beb..b4a5f9da577 100644 --- a/test/e2e/framework/workspaces.go +++ b/test/e2e/framework/workspaces.go @@ -184,7 +184,7 @@ func newWorkspaceFixture[O WorkspaceOption](t *testing.T, createClusterClient, c }, workspaceInitTimeout, time.Millisecond*100, "failed to wait for %s workspace %s to become ready", ws.Spec.Type, parent.Join(ws.Name)) Eventually(t, func() (bool, string) { - if _, err := clusterClient.Cluster(logicalcluster.NewPath(ws.Spec.Cluster)).CoreV1alpha1().LogicalClusters().Get(ctx, corev1alpha1.LogicalClusterName, metav1.GetOptions{}); err != nil { + if _, err := clusterClient.Cluster(ws.Spec.Cluster.Path()).CoreV1alpha1().LogicalClusters().Get(ctx, corev1alpha1.LogicalClusterName, metav1.GetOptions{}); err != nil { return false, fmt.Sprintf("failed to get LogicalCluster %s by cluster name %s: %v", parent.Join(ws.Name), ws.Spec.Cluster, err) } if _, err := clusterClient.Cluster(parent.Join(ws.Name)).CoreV1alpha1().LogicalClusters().Get(ctx, corev1alpha1.LogicalClusterName, metav1.GetOptions{}); err != nil { diff --git a/test/e2e/reconciler/cache/replication_test.go b/test/e2e/reconciler/cache/replication_test.go index 41c56f0a933..d0eebceea0f 100644 --- a/test/e2e/reconciler/cache/replication_test.go +++ b/test/e2e/reconciler/cache/replication_test.go @@ -337,7 +337,7 @@ func replicateResource(ctx context.Context, t *testing.T, orgPath, _ := framework.NewOrganizationFixture(t, server) if clusterName.Empty() { _, ws := framework.NewWorkspaceFixture(t, server, orgPath, framework.WithRootShard()) - clusterName = logicalcluster.Name(ws.Spec.Cluster) + clusterName = ws.Spec.Cluster } resMeta, err := meta.Accessor(res) require.NoError(t, err) @@ -390,7 +390,7 @@ func replicateResourceNegative(ctx context.Context, t *testing.T, orgPath, _ := framework.NewOrganizationFixture(t, server) if clusterName.Empty() { _, ws := framework.NewWorkspaceFixture(t, server, orgPath, framework.WithRootShard()) - clusterName = logicalcluster.Name(ws.Spec.Cluster) + clusterName = ws.Spec.Cluster } resMeta, err := meta.Accessor(res) require.NoError(t, err) diff --git a/test/e2e/reconciler/workspacedeletion/controller_test.go b/test/e2e/reconciler/workspacedeletion/controller_test.go index 26c39b432b1..0ef2476ce9e 100644 --- a/test/e2e/reconciler/workspacedeletion/controller_test.go +++ b/test/e2e/reconciler/workspacedeletion/controller_test.go @@ -23,7 +23,6 @@ import ( "time" kcpkubernetesclientset "github.com/kcp-dev/client-go/kubernetes" - "github.com/kcp-dev/logicalcluster/v3" "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" @@ -201,7 +200,7 @@ func TestWorkspaceDeletion(t *testing.T) { t.Logf("Create a workspace with in the org workspace") _, ws := framework.NewWorkspaceFixture(t, server, orgPath, framework.WithName("org-ws-cleanup"), framework.WithRootShard()) - wsClusterName := logicalcluster.Name(ws.Spec.Cluster) + wsClusterName := ws.Spec.Cluster t.Logf("Should have finalizer added in workspace") require.Eventually(t, func() bool { diff --git a/test/e2e/virtual/apiexport/authorizer_test.go b/test/e2e/virtual/apiexport/authorizer_test.go index acf7f4e7905..5004166517e 100644 --- a/test/e2e/virtual/apiexport/authorizer_test.go +++ b/test/e2e/virtual/apiexport/authorizer_test.go @@ -432,7 +432,7 @@ metadata: t.Logf("verify that service-provider-2-admin can lists sherriffs resources in the tenant workspace %q via the virtual apiexport apiserver", tenantPath) framework.Eventually(t, func() (success bool, reason string) { - _, err = serviceProvider2DynamicVWClientForTenantWorkspace.Cluster(logicalcluster.Name(tenantWorkspace.Spec.Cluster).Path()).Resource(schema.GroupVersionResource{Version: "v1alpha1", Resource: "sheriffs", Group: "wild.wild.west"}).List(ctx, metav1.ListOptions{}) + _, err = serviceProvider2DynamicVWClientForTenantWorkspace.Cluster(tenantWorkspace.Spec.Cluster.Path()).Resource(schema.GroupVersionResource{Version: "v1alpha1", Resource: "sheriffs", Group: "wild.wild.west"}).List(ctx, metav1.ListOptions{}) if err != nil { return false, fmt.Sprintf("error while waiting to list sherriffs: %v", err) } @@ -458,7 +458,7 @@ metadata: t.Logf("verify that service-provider-2-admin cannot list CRD shadowed cowboy resources in the tenant workspace %q via the virtual apiexport apiserver", tenantShadowCRDPath) framework.Eventually(t, func() (bool, string) { - _, err = serviceProvider2DynamicVWClientForShadowTenantWorkspace.Cluster(logicalcluster.Name(tenantShadowCRDWorkspace.Spec.Cluster).Path()).Resource(schema.GroupVersionResource{Version: "v1alpha1", Resource: "cowboys", Group: "wildwest.dev"}).List(ctx, metav1.ListOptions{}) + _, err = serviceProvider2DynamicVWClientForShadowTenantWorkspace.Cluster(tenantShadowCRDWorkspace.Spec.Cluster.Path()).Resource(schema.GroupVersionResource{Version: "v1alpha1", Resource: "cowboys", Group: "wildwest.dev"}).List(ctx, metav1.ListOptions{}) if err == nil { return false, "expected error, got none" } @@ -551,7 +551,7 @@ func TestRootAPIExportAuthorizers(t *testing.T) { servicePath, _ := framework.NewWorkspaceFixture(t, server, orgPath, framework.WithName("provider")) userPath, userWorkspace := framework.NewWorkspaceFixture(t, server, orgPath, framework.WithName("consumer")) - userClusterName := logicalcluster.Name(userWorkspace.Spec.Cluster) + userClusterName := userWorkspace.Spec.Cluster cfg := server.BaseConfig(t) diff --git a/test/e2e/virtual/apiexport/binding_test.go b/test/e2e/virtual/apiexport/binding_test.go index 4b2771970eb..24a88b2fa0e 100644 --- a/test/e2e/virtual/apiexport/binding_test.go +++ b/test/e2e/virtual/apiexport/binding_test.go @@ -24,7 +24,6 @@ import ( "time" kcpkubernetesclientset "github.com/kcp-dev/client-go/kubernetes" - "github.com/kcp-dev/logicalcluster/v3" "github.com/stretchr/testify/require" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -141,7 +140,7 @@ spec: }, wait.ForeverTestTimeout, 100*time.Millisecond, "waiting on virtual workspace to be ready") framework.Eventually(t, func() (success bool, reason string) { - err = apply(t, ctx, logicalcluster.Name(consumerWorkspace.Spec.Cluster).Path(), serviceProviderVirtualWorkspaceConfig, fmt.Sprintf(` + err = apply(t, ctx, consumerWorkspace.Spec.Cluster.Path(), serviceProviderVirtualWorkspaceConfig, fmt.Sprintf(` apiVersion: apis.kcp.io/v1alpha1 kind: APIBinding metadata: @@ -188,7 +187,7 @@ roleRef: `)) framework.Eventually(t, func() (bool, string) { - err := apply(t, ctx, logicalcluster.Name(consumerWorkspace.Spec.Cluster).Path(), serviceProviderVirtualWorkspaceConfig, fmt.Sprintf(` + err := apply(t, ctx, consumerWorkspace.Spec.Cluster.Path(), serviceProviderVirtualWorkspaceConfig, fmt.Sprintf(` apiVersion: apis.kcp.io/v1alpha1 kind: APIBinding metadata: diff --git a/test/e2e/virtual/apiexport/virtualworkspace_test.go b/test/e2e/virtual/apiexport/virtualworkspace_test.go index d35f5b54373..a066d23237f 100644 --- a/test/e2e/virtual/apiexport/virtualworkspace_test.go +++ b/test/e2e/virtual/apiexport/virtualworkspace_test.go @@ -93,7 +93,7 @@ func TestAPIExportVirtualWorkspace(t *testing.T) { orgPath, _ := framework.NewOrganizationFixture(t, server) serviceProviderPath, _ := framework.NewWorkspaceFixture(t, server, orgPath) consumerPath, consumerWorkspace := framework.NewWorkspaceFixture(t, server, orgPath) - consumerClusterName := logicalcluster.Name(consumerWorkspace.Spec.Cluster) + consumerClusterName := consumerWorkspace.Spec.Cluster framework.AdmitWorkspaceAccess(ctx, t, kubeClusterClient, serviceProviderPath, []string{"user-1"}, nil, false) @@ -406,10 +406,10 @@ func TestAPIExportAPIBindingsAccess(t *testing.T) { verifyBindings(ws1Path, "export1", func(bindings []apisv1alpha1.APIBinding) error { for _, b := range bindings { clusterName := logicalcluster.From(&b) - if clusterName == logicalcluster.Name(ws1.Spec.Cluster) && b.Name == "binding1" { + if clusterName == ws1.Spec.Cluster && b.Name == "binding1" { continue } - if clusterName == logicalcluster.Name(ws2.Spec.Cluster) && b.Name == "binding1" { + if clusterName == ws2.Spec.Cluster && b.Name == "binding1" { continue } @@ -423,7 +423,7 @@ func TestAPIExportAPIBindingsAccess(t *testing.T) { verifyBindings(ws1Path, "export2", func(bindings []apisv1alpha1.APIBinding) error { for _, b := range bindings { clusterName := logicalcluster.From(&b) - if clusterName == logicalcluster.Name(ws1.Spec.Cluster) && b.Name == "binding2" { + if clusterName == ws1.Spec.Cluster && b.Name == "binding2" { continue } @@ -480,8 +480,8 @@ func TestAPIExportAPIBindingsAccess(t *testing.T) { for _, b := range bindings { clusterName := logicalcluster.From(&b) - if clusterName != logicalcluster.Name(ws1.Spec.Cluster) { - if clusterName == logicalcluster.Name(ws2.Spec.Cluster) && b.Name == "binding1" { + if clusterName != ws1.Spec.Cluster { + if clusterName == ws2.Spec.Cluster && b.Name == "binding1" { continue } @@ -503,7 +503,7 @@ func TestAPIExportAPIBindingsAccess(t *testing.T) { verifyBindings(ws1Path, "export2", func(bindings []apisv1alpha1.APIBinding) error { for _, b := range bindings { clusterName := logicalcluster.From(&b) - if clusterName == logicalcluster.Name(ws1.Spec.Cluster) && b.Name == "binding2" { + if clusterName == ws1.Spec.Cluster && b.Name == "binding2" { continue } @@ -534,10 +534,10 @@ func TestAPIExportAPIBindingsAccess(t *testing.T) { verifyBindings(ws1Path, "export1", func(bindings []apisv1alpha1.APIBinding) error { for _, b := range bindings { clusterName := logicalcluster.From(&b) - if clusterName == logicalcluster.Name(ws1.Spec.Cluster) && b.Name == "binding1" { + if clusterName == ws1.Spec.Cluster && b.Name == "binding1" { continue } - if clusterName == logicalcluster.Name(ws2.Spec.Cluster) && b.Name == "binding1" { + if clusterName == ws2.Spec.Cluster && b.Name == "binding1" { continue } diff --git a/test/e2e/virtual/initializingworkspaces/virtualworkspace_test.go b/test/e2e/virtual/initializingworkspaces/virtualworkspace_test.go index ef70312beaa..1beab8cad39 100644 --- a/test/e2e/virtual/initializingworkspaces/virtualworkspace_test.go +++ b/test/e2e/virtual/initializingworkspaces/virtualworkspace_test.go @@ -383,9 +383,9 @@ func TestInitializingWorkspacesVirtualWorkspaceAccess(t *testing.T) { }, wait.ForeverTestTimeout, 100*time.Millisecond) } - lclusters, expectedClusters := sets.New[string](), sets.New[string]() + lclusters, expectedClusters := sets.New[logicalcluster.Name](), sets.New[logicalcluster.Name]() for i := range actual.Items { - lclusters.Insert(logicalcluster.From(&actual.Items[i]).String()) + lclusters.Insert(logicalcluster.From(&actual.Items[i])) } for i := range expected { expectedClusters.Insert(expected[i].Spec.Cluster) @@ -393,7 +393,7 @@ func TestInitializingWorkspacesVirtualWorkspaceAccess(t *testing.T) { sort.Slice(actual.Items, func(i, j int) bool { return actual.Items[i].UID < actual.Items[j].UID }) - require.Equal(t, sets.List[string](expectedClusters), sets.List[string](lclusters), "unexpected clusters for initializers %q", initializers) + require.Equal(t, sets.List[logicalcluster.Name](expectedClusters), sets.List[logicalcluster.Name](lclusters), "unexpected clusters for initializers %q", initializers) } t.Log("Start WATCH streams to confirm behavior on changes") @@ -432,7 +432,7 @@ func TestInitializingWorkspacesVirtualWorkspaceAccess(t *testing.T) { ws, err = sourceKcpClusterClient.TenancyV1alpha1().Cluster(wsPath).Workspaces().Get(ctx, ws.Name, metav1.GetOptions{}) require.NoError(t, err) - wsClusterName := logicalcluster.Name(ws.Spec.Cluster) + wsClusterName := ws.Spec.Cluster t.Logf("Waiting for a watcher for %s initializer to see the logicalcluster in %s for workspace %s", initializer, ws.Spec.Cluster, ws.Name) for { @@ -440,7 +440,7 @@ func TestInitializingWorkspacesVirtualWorkspaceAccess(t *testing.T) { case evt := <-watcher.ResultChan(): // there might be other actors doing who-knows-what on the workspaces, so we need to specifically // look for the first event *relating to the new workspace* that we get - if logicalcluster.From(evt.Object.(metav1.Object)).String() != ws.Spec.Cluster { + if logicalcluster.From(evt.Object.(metav1.Object)) != ws.Spec.Cluster { continue } require.Equal(t, evt.Type, watch.Added) @@ -542,7 +542,7 @@ func TestInitializingWorkspacesVirtualWorkspaceAccess(t *testing.T) { } // there might be other actors doing who-knows-what on the workspaces, so we need to specifically // look for the first event *relating to the new workspace* that we get - if logicalcluster.From(evt.Object.(metav1.Object)).String() != ws.Spec.Cluster { + if logicalcluster.From(evt.Object.(metav1.Object)) != ws.Spec.Cluster { continue } require.Equal(t, evt.Type, watch.Deleted) diff --git a/test/e2e/workspacetype/controller_test.go b/test/e2e/workspacetype/controller_test.go index adf562dab7f..b03229b5a14 100644 --- a/test/e2e/workspacetype/controller_test.go +++ b/test/e2e/workspacetype/controller_test.go @@ -304,10 +304,10 @@ func TestWorkspaceTypes(t *testing.T) { t.Logf("Remove initializer") err = retry.RetryOnConflict(retry.DefaultRetry, func() error { - logicalCluster, err := server.kcpClusterClient.CoreV1alpha1().LogicalClusters().Cluster(logicalcluster.Name(workspace.Spec.Cluster).Path()).Get(ctx, corev1alpha1.LogicalClusterName, metav1.GetOptions{}) + logicalCluster, err := server.kcpClusterClient.CoreV1alpha1().LogicalClusters().Cluster(workspace.Spec.Cluster.Path()).Get(ctx, corev1alpha1.LogicalClusterName, metav1.GetOptions{}) require.NoError(t, err) logicalCluster.Status.Initializers = initialization.EnsureInitializerAbsent(initialization.InitializerForType(wt), logicalCluster.Status.Initializers) - _, err = server.kcpClusterClient.CoreV1alpha1().LogicalClusters().Cluster(logicalcluster.Name(workspace.Spec.Cluster).Path()).UpdateStatus(ctx, logicalCluster, metav1.UpdateOptions{}) + _, err = server.kcpClusterClient.CoreV1alpha1().LogicalClusters().Cluster(workspace.Spec.Cluster.Path()).UpdateStatus(ctx, logicalCluster, metav1.UpdateOptions{}) return err }) require.NoError(t, err) From a334dcbcbfa84b36654dcbc0a497405f9bd0ee50 Mon Sep 17 00:00:00 2001 From: "Dr. Stefan Schimanski" Date: Fri, 10 May 2024 14:06:22 +0200 Subject: [PATCH 2/3] apis/apis: make APIBinding.Status.APIExportClusterName a logicalcluster.Name Signed-off-by: Dr. Stefan Schimanski --- pkg/admission/mutatingwebhook/plugin.go | 2 +- pkg/admission/validatingwebhook/plugin.go | 2 +- pkg/reconciler/apis/apibinding/apibinding_reconcile.go | 2 +- sdk/apis/apis/v1alpha1/types_apibinding.go | 4 +++- 4 files changed, 6 insertions(+), 4 deletions(-) diff --git a/pkg/admission/mutatingwebhook/plugin.go b/pkg/admission/mutatingwebhook/plugin.go index b316c290759..b935cd5299a 100644 --- a/pkg/admission/mutatingwebhook/plugin.go +++ b/pkg/admission/mutatingwebhook/plugin.go @@ -165,7 +165,7 @@ func (p *Plugin) getSourceClusterForGroupResource(clusterName logicalcluster.Nam for _, br := range apiBinding.Status.BoundResources { if br.Group == groupResource.Group && br.Resource == groupResource.Resource { // GroupResource comes from an APIBinding/APIExport - return logicalcluster.Name(apiBinding.Status.APIExportClusterName), nil + return apiBinding.Status.APIExportClusterName, nil } } } diff --git a/pkg/admission/validatingwebhook/plugin.go b/pkg/admission/validatingwebhook/plugin.go index 5c19d9f3fe8..ab2e2b049a2 100644 --- a/pkg/admission/validatingwebhook/plugin.go +++ b/pkg/admission/validatingwebhook/plugin.go @@ -164,7 +164,7 @@ func (p *Plugin) getSourceClusterForGroupResource(clusterName logicalcluster.Nam for _, br := range apiBinding.Status.BoundResources { if br.Group == groupResource.Group && br.Resource == groupResource.Resource { // GroupResource comes from an APIBinding/APIExport - return logicalcluster.Name(apiBinding.Status.APIExportClusterName), nil + return apiBinding.Status.APIExportClusterName, nil } } } diff --git a/pkg/reconciler/apis/apibinding/apibinding_reconcile.go b/pkg/reconciler/apis/apibinding/apibinding_reconcile.go index f115d20af4f..0e3d548013f 100644 --- a/pkg/reconciler/apis/apibinding/apibinding_reconcile.go +++ b/pkg/reconciler/apis/apibinding/apibinding_reconcile.go @@ -208,7 +208,7 @@ func (r *bindingReconciler) reconcile(ctx context.Context, apiBinding *apisv1alp // Record the APIExport's host cluster name for lookup in webhooks. // The full path is unreliable for this purpose. clusterName := logicalcluster.From(apiExport) - apiBinding.Status.APIExportClusterName = clusterName.String() + apiBinding.Status.APIExportClusterName = clusterName var needToWaitForRequeueWhenEstablished []string diff --git a/sdk/apis/apis/v1alpha1/types_apibinding.go b/sdk/apis/apis/v1alpha1/types_apibinding.go index fd7d1e282f3..95174ffcb7b 100644 --- a/sdk/apis/apis/v1alpha1/types_apibinding.go +++ b/sdk/apis/apis/v1alpha1/types_apibinding.go @@ -17,6 +17,8 @@ limitations under the License. package v1alpha1 import ( + "github.com/kcp-dev/logicalcluster/v3" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" conditionsv1alpha1 "github.com/kcp-dev/kcp/sdk/apis/third_party/conditions/apis/conditions/v1alpha1" @@ -142,7 +144,7 @@ type APIBindingStatus struct { // APIExportClusterName records the name (not path) of the logical cluster that contains the APIExport. // // +optional - APIExportClusterName string `json:"apiExportClusterName,omitempty"` + APIExportClusterName logicalcluster.Name `json:"apiExportClusterName,omitempty"` // boundResources records the state of bound APIs. // From 0ae978d8ec16371ffec0a7fe925b6c5445eed7ff Mon Sep 17 00:00:00 2001 From: "Dr. Stefan Schimanski" Date: Fri, 10 May 2024 16:07:45 +0200 Subject: [PATCH 3/3] make codegen Signed-off-by: Dr. Stefan Schimanski --- .../applyconfiguration/apis/v1alpha1/apibindingstatus.go | 6 ++++-- .../applyconfiguration/tenancy/v1alpha1/workspacespec.go | 8 ++++++-- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/sdk/client/applyconfiguration/apis/v1alpha1/apibindingstatus.go b/sdk/client/applyconfiguration/apis/v1alpha1/apibindingstatus.go index 0e17539edab..3f2dd6ee92e 100644 --- a/sdk/client/applyconfiguration/apis/v1alpha1/apibindingstatus.go +++ b/sdk/client/applyconfiguration/apis/v1alpha1/apibindingstatus.go @@ -19,6 +19,8 @@ limitations under the License. package v1alpha1 import ( + v3 "github.com/kcp-dev/logicalcluster/v3" + apisv1alpha1 "github.com/kcp-dev/kcp/sdk/apis/apis/v1alpha1" conditionsv1alpha1 "github.com/kcp-dev/kcp/sdk/apis/third_party/conditions/apis/conditions/v1alpha1" ) @@ -26,7 +28,7 @@ import ( // APIBindingStatusApplyConfiguration represents an declarative configuration of the APIBindingStatus type for use // with apply. type APIBindingStatusApplyConfiguration struct { - APIExportClusterName *string `json:"apiExportClusterName,omitempty"` + APIExportClusterName *v3.Name `json:"apiExportClusterName,omitempty"` BoundResources []BoundAPIResourceApplyConfiguration `json:"boundResources,omitempty"` Phase *apisv1alpha1.APIBindingPhaseType `json:"phase,omitempty"` Conditions *conditionsv1alpha1.Conditions `json:"conditions,omitempty"` @@ -43,7 +45,7 @@ func APIBindingStatus() *APIBindingStatusApplyConfiguration { // WithAPIExportClusterName sets the APIExportClusterName field in the declarative configuration to the given value // and returns the receiver, so that objects can be built by chaining "With" function invocations. // If called multiple times, the APIExportClusterName field is set to the value of the last call. -func (b *APIBindingStatusApplyConfiguration) WithAPIExportClusterName(value string) *APIBindingStatusApplyConfiguration { +func (b *APIBindingStatusApplyConfiguration) WithAPIExportClusterName(value v3.Name) *APIBindingStatusApplyConfiguration { b.APIExportClusterName = &value return b } diff --git a/sdk/client/applyconfiguration/tenancy/v1alpha1/workspacespec.go b/sdk/client/applyconfiguration/tenancy/v1alpha1/workspacespec.go index cadd613e50d..919858109fb 100644 --- a/sdk/client/applyconfiguration/tenancy/v1alpha1/workspacespec.go +++ b/sdk/client/applyconfiguration/tenancy/v1alpha1/workspacespec.go @@ -18,12 +18,16 @@ limitations under the License. package v1alpha1 +import ( + v3 "github.com/kcp-dev/logicalcluster/v3" +) + // WorkspaceSpecApplyConfiguration represents an declarative configuration of the WorkspaceSpec type for use // with apply. type WorkspaceSpecApplyConfiguration struct { Type *WorkspaceTypeReferenceApplyConfiguration `json:"type,omitempty"` Location *WorkspaceLocationApplyConfiguration `json:"location,omitempty"` - Cluster *string `json:"cluster,omitempty"` + Cluster *v3.Name `json:"cluster,omitempty"` URL *string `json:"URL,omitempty"` } @@ -52,7 +56,7 @@ func (b *WorkspaceSpecApplyConfiguration) WithLocation(value *WorkspaceLocationA // WithCluster sets the Cluster field in the declarative configuration to the given value // and returns the receiver, so that objects can be built by chaining "With" function invocations. // If called multiple times, the Cluster field is set to the value of the last call. -func (b *WorkspaceSpecApplyConfiguration) WithCluster(value string) *WorkspaceSpecApplyConfiguration { +func (b *WorkspaceSpecApplyConfiguration) WithCluster(value v3.Name) *WorkspaceSpecApplyConfiguration { b.Cluster = &value return b }