From b449811bb256d4c2f4d5fad55c4990466f75854a Mon Sep 17 00:00:00 2001 From: Mangirdas Judeikis Date: Tue, 27 May 2025 08:10:09 +0300 Subject: [PATCH 1/3] Disable APIExport VW url population and precreate EndpointSlice for it Signed-off-by: Mangirdas Judeikis On-behalf-of: @SAP mangirdas.judeikis@sap.com --- pkg/features/kcp_features.go | 10 ++- .../apis/apiexport/apiexport_controller.go | 13 ++++ .../apiexport/apiexport_controller_test.go | 60 ++++++++---------- .../apis/apiexport/apiexport_reconcile.go | 61 ++++++++++++------- pkg/server/controllers.go | 2 + pkg/server/server.go | 1 + 6 files changed, 89 insertions(+), 58 deletions(-) diff --git a/pkg/features/kcp_features.go b/pkg/features/kcp_features.go index bf7593b2f9b..f54bbe38007 100644 --- a/pkg/features/kcp_features.go +++ b/pkg/features/kcp_features.go @@ -47,6 +47,12 @@ const ( // alpha: v0.1 // Enables cache apis and controllers. CacheAPIs featuregate.Feature = "CacheAPIs" + + // owner: @mjudeikis + // alpha: v0.1 + // Enables VirtualWorkspace urls on APIExport. This enables to use Deprecated APIExport VirtualWorkspace urls. + // This is a temporary feature to ease the migration to the new VirtualWorkspace urls. + EnableDeprecatedAPIExportVirtualWorkspacesUrls featuregate.Feature = "EnableDeprecatedAPIExportVirtualWorkspacesUrls" ) // DefaultFeatureGate exposes the upstream feature gate, but with our gate setting applied. @@ -120,7 +126,9 @@ var defaultVersionedGenericControlPlaneFeatureGates = map[featuregate.Feature]fe CacheAPIs: { {Version: version.MustParse("1.32"), Default: false, PreRelease: featuregate.Alpha}, }, - + EnableDeprecatedAPIExportVirtualWorkspacesUrls: { + {Version: version.MustParse("1.32"), Default: false, PreRelease: featuregate.Alpha}, + }, // inherited features from generic apiserver, relisted here to get a conflict if it is changed // unintentionally on either side: genericfeatures.APIResponseCompression: { diff --git a/pkg/reconciler/apis/apiexport/apiexport_controller.go b/pkg/reconciler/apis/apiexport/apiexport_controller.go index 3e74bd6e4ce..f6409c3f7ac 100644 --- a/pkg/reconciler/apis/apiexport/apiexport_controller.go +++ b/pkg/reconciler/apis/apiexport/apiexport_controller.go @@ -41,10 +41,12 @@ import ( "github.com/kcp-dev/kcp/pkg/logging" "github.com/kcp-dev/kcp/pkg/reconciler/committer" "github.com/kcp-dev/kcp/pkg/reconciler/events" + apisv1alpha1 "github.com/kcp-dev/kcp/sdk/apis/apis/v1alpha1" apisv1alpha2 "github.com/kcp-dev/kcp/sdk/apis/apis/v1alpha2" corev1alpha1 "github.com/kcp-dev/kcp/sdk/apis/core/v1alpha1" kcpclientset "github.com/kcp-dev/kcp/sdk/client/clientset/versioned/cluster" apisv1alpha2client "github.com/kcp-dev/kcp/sdk/client/clientset/versioned/typed/apis/v1alpha2" + apisv1alpha1informers "github.com/kcp-dev/kcp/sdk/client/informers/externalversions/apis/v1alpha1" apisv1alpha2informers "github.com/kcp-dev/kcp/sdk/client/informers/externalversions/apis/v1alpha2" corev1alpha1informers "github.com/kcp-dev/kcp/sdk/client/informers/externalversions/core/v1alpha1" ) @@ -59,6 +61,7 @@ const ( func NewController( kcpClusterClient kcpclientset.ClusterInterface, apiExportInformer apisv1alpha2informers.APIExportClusterInformer, + apiExportEndpointSliceInformer apisv1alpha1informers.APIExportEndpointSliceClusterInformer, globalShardInformer corev1alpha1informers.ShardClusterInformer, kubeClusterClient kcpkubernetesclientset.ClusterInterface, namespaceInformer kcpcorev1informers.NamespaceClusterInformer, @@ -89,6 +92,13 @@ func NewController( getAPIExport: func(clusterName logicalcluster.Name, name string) (*apisv1alpha2.APIExport, error) { return apiExportInformer.Lister().Cluster(clusterName).Get(name) }, + getAPIExportEndpointSlice: func(clusterName logicalcluster.Name, name string) (*apisv1alpha1.APIExportEndpointSlice, error) { + return apiExportEndpointSliceInformer.Lister().Cluster(clusterName).Get(name) + }, + createAPIExportEndpointSlice: func(ctx context.Context, clusterName logicalcluster.Path, apiExportEndpointSlice *apisv1alpha1.APIExportEndpointSlice) error { + _, err := kcpClusterClient.Cluster(clusterName).ApisV1alpha1().APIExportEndpointSlices().Create(ctx, apiExportEndpointSlice, metav1.CreateOptions{}) + return err + }, getNamespace: func(clusterName logicalcluster.Name, name string) (*corev1.Namespace, error) { return namespaceInformer.Lister().Cluster(clusterName).Get(name) @@ -183,6 +193,9 @@ type controller struct { listAPIExportsForSecret func(secret *corev1.Secret) ([]*apisv1alpha2.APIExport, error) getAPIExport func(clusterName logicalcluster.Name, name string) (*apisv1alpha2.APIExport, error) + getAPIExportEndpointSlice func(clusterName logicalcluster.Name, name string) (*apisv1alpha1.APIExportEndpointSlice, error) + createAPIExportEndpointSlice func(ctx context.Context, clusterName logicalcluster.Path, apiExportEndpointSlice *apisv1alpha1.APIExportEndpointSlice) error + getNamespace func(clusterName logicalcluster.Name, name string) (*corev1.Namespace, error) createNamespace func(ctx context.Context, clusterName logicalcluster.Path, ns *corev1.Namespace) error diff --git a/pkg/reconciler/apis/apiexport/apiexport_controller_test.go b/pkg/reconciler/apis/apiexport/apiexport_controller_test.go index 3c62885d91e..853c6aa6de1 100644 --- a/pkg/reconciler/apis/apiexport/apiexport_controller_test.go +++ b/pkg/reconciler/apis/apiexport/apiexport_controller_test.go @@ -49,19 +49,19 @@ func TestReconcile(t *testing.T) { apiExportHasSomeOtherHash bool hasPreexistingVerifyFailure bool listShardsError error + apiExportEndpointSliceNotFound bool apiBindings []interface{} - wantGenerationFailed bool - wantError bool - wantCreateSecretCalled bool - wantUnsetIdentity bool - wantDefaultSecretRef bool - wantStatusHashSet bool - wantVerifyFailure bool - wantIdentityValid bool - wantVirtualWorkspaceURLsError bool - wantVirtualWorkspaceURLsReady bool + wantGenerationFailed bool + wantError bool + wantCreateSecretCalled bool + wantUnsetIdentity bool + wantDefaultSecretRef bool + wantStatusHashSet bool + wantVerifyFailure bool + wantIdentityValid bool + wantCreateAPIExportEndpointSlice bool }{ "create secret when ref is nil and secret doesn't exist": { secretExists: false, @@ -122,20 +122,16 @@ func TestReconcile(t *testing.T) { apiBindings: []interface{}{ "something", }, - listShardsError: errors.New("foo"), - wantVirtualWorkspaceURLsError: true, + listShardsError: errors.New("foo"), }, - "virtualWorkspaceURLs set when APIBindings present": { + "create APIExportEndpointSlice when APIBindings present": { secretRefSet: true, secretExists: true, - wantStatusHashSet: true, - wantIdentityValid: true, - - apiBindings: []interface{}{ - "something", - }, - wantVirtualWorkspaceURLsReady: true, + wantStatusHashSet: true, + apiExportEndpointSliceNotFound: true, + wantCreateAPIExportEndpointSlice: true, + wantIdentityValid: true, }, } @@ -155,6 +151,15 @@ func TestReconcile(t *testing.T) { return nil }, secretNamespace: "default-ns", + getAPIExportEndpointSlice: func(clusterName logicalcluster.Name, name string) (*apisv1alpha1.APIExportEndpointSlice, error) { + if tc.apiExportEndpointSliceNotFound { + return nil, apierrors.NewNotFound(corev1.Resource("apiexportendpointslices"), name) + } + return &apisv1alpha1.APIExportEndpointSlice{}, nil + }, + createAPIExportEndpointSlice: func(ctx context.Context, clusterName logicalcluster.Path, apiExportEndpointSlice *apisv1alpha1.APIExportEndpointSlice) error { + return nil + }, getSecret: func(ctx context.Context, clusterName logicalcluster.Name, ns, name string) (*corev1.Secret, error) { if tc.secretExists { secret := &corev1.Secret{ @@ -287,21 +292,6 @@ func TestReconcile(t *testing.T) { if tc.wantIdentityValid { requireConditionMatches(t, apiExport, conditions.TrueCondition(apisv1alpha2.APIExportIdentityValid)) } - - if tc.wantVirtualWorkspaceURLsError { - requireConditionMatches(t, apiExport, - conditions.FalseCondition( - apisv1alpha2.APIExportVirtualWorkspaceURLsReady, - apisv1alpha2.ErrorGeneratingURLsReason, - conditionsv1alpha1.ConditionSeverityError, - "", - ), - ) - } - - if tc.wantVirtualWorkspaceURLsReady { - requireConditionMatches(t, apiExport, conditions.TrueCondition(apisv1alpha2.APIExportVirtualWorkspaceURLsReady)) - } }) } } diff --git a/pkg/reconciler/apis/apiexport/apiexport_reconcile.go b/pkg/reconciler/apis/apiexport/apiexport_reconcile.go index 726321d720d..6d63a73d63e 100644 --- a/pkg/reconciler/apis/apiexport/apiexport_reconcile.go +++ b/pkg/reconciler/apis/apiexport/apiexport_reconcile.go @@ -31,10 +31,12 @@ import ( "github.com/kcp-dev/logicalcluster/v3" virtualworkspacesoptions "github.com/kcp-dev/kcp/cmd/virtual-workspaces/options" + kcpfeatures "github.com/kcp-dev/kcp/pkg/features" "github.com/kcp-dev/kcp/pkg/logging" apiexportbuilder "github.com/kcp-dev/kcp/pkg/virtual/apiexport/builder" apisv1alpha1 "github.com/kcp-dev/kcp/sdk/apis/apis/v1alpha1" apisv1alpha2 "github.com/kcp-dev/kcp/sdk/apis/apis/v1alpha2" + "github.com/kcp-dev/kcp/sdk/apis/core" conditionsv1alpha1 "github.com/kcp-dev/kcp/sdk/apis/third_party/conditions/apis/conditions/v1alpha1" "github.com/kcp-dev/kcp/sdk/apis/third_party/conditions/util/conditions" ) @@ -46,6 +48,7 @@ func (c *controller) reconcile(ctx context.Context, apiExport *apisv1alpha2.APIE } clusterName := logicalcluster.From(apiExport) + clusterPath := apiExport.Annotations[core.LogicalClusterPathAnnotationKey] if identity.SecretRef == nil { c.ensureSecretNamespaceExists(ctx, clusterName) @@ -97,29 +100,43 @@ func (c *controller) reconcile(ctx context.Context, apiExport *apisv1alpha2.APIE ) } - // TODO(sttts): reactivate this with multi-shard support eventually - /* - // check if any APIBindings are bound to this APIExport. If so, add a virtualworkspaceURL - apiBindings, err := c.getAPIBindingsForAPIExport(clusterName, apiExport.Name) - if err != nil { - return fmt.Errorf("error checking for APIBindings with APIExport %s|%s: %w", clusterName, apiExport.Name, err) + // Ensure the APIExportEndpointSlice exists + _, err := c.getAPIExportEndpointSlice(clusterName, apiExport.Name) + if err != nil { + if errors.IsNotFound(err) { + // Create the APIExportEndpointSlice + apiExportEndpointSlice := &apisv1alpha1.APIExportEndpointSlice{ + ObjectMeta: metav1.ObjectMeta{ + Name: apiExport.Name, + }, + Spec: apisv1alpha1.APIExportEndpointSliceSpec{ + APIExport: apisv1alpha1.ExportBindingReference{ + Name: apiExport.Name, + Path: clusterPath, + }, + }, + } + if err := c.createAPIExportEndpointSlice(ctx, clusterName.Path(), apiExportEndpointSlice); err != nil { + return fmt.Errorf("error creating APIExportEndpointSlice for APIExport %s|%s: %w", clusterName, apiExport.Name, err) + } + } else { + return fmt.Errorf("error getting APIExportEndpointSlice for APIExport %s|%s: %w", clusterName, apiExport.Name, err) } + } - // If there are no bindings, then we can't create a URL yet. - if len(apiBindings) == 0 { - return nil + // Ensure the APIExportEndpointSlice has a virtual workspace URL + // TODO(mjudeikis): Remove this and move to batteries. + if kcpfeatures.DefaultFeatureGate.Enabled(kcpfeatures.EnableDeprecatedAPIExportVirtualWorkspacesUrls) { + if err := c.updateVirtualWorkspaceURLs(ctx, apiExport); err != nil { + conditions.MarkFalse( + apiExport, + apisv1alpha2.APIExportVirtualWorkspaceURLsReady, + apisv1alpha2.ErrorGeneratingURLsReason, + conditionsv1alpha1.ConditionSeverityError, + "%v", + err, + ) } - */ - - if err := c.updateVirtualWorkspaceURLs(ctx, apiExport); err != nil { - conditions.MarkFalse( - apiExport, - apisv1alpha2.APIExportVirtualWorkspaceURLsReady, - apisv1alpha2.ErrorGeneratingURLsReason, - conditionsv1alpha1.ConditionSeverityError, - "%v", - err, - ) } return nil @@ -216,11 +233,11 @@ func (c *controller) updateVirtualWorkspaceURLs(ctx context.Context, apiExport * desiredURLs.Insert(u.String()) } - //nolint:staticcheck // SA1019 VirtualWorkspaces is deprecated but not removed yet + //nolint:staticcheck apiExport.Status.VirtualWorkspaces = nil for _, u := range sets.List[string](desiredURLs) { - //nolint:staticcheck // SA1019 VirtualWorkspaces is deprecated but not removed yet + //nolint:staticcheck apiExport.Status.VirtualWorkspaces = append(apiExport.Status.VirtualWorkspaces, apisv1alpha2.VirtualWorkspace{ URL: u, }) diff --git a/pkg/server/controllers.go b/pkg/server/controllers.go index 6f62bc892b2..cabf0e8c03c 100644 --- a/pkg/server/controllers.go +++ b/pkg/server/controllers.go @@ -1050,6 +1050,7 @@ func (s *Server) installAPIExportController(ctx context.Context, config *rest.Co c, err := apiexport.NewController( kcpClusterClient, s.KcpSharedInformerFactory.Apis().V1alpha2().APIExports(), + s.KcpSharedInformerFactory.Apis().V1alpha1().APIExportEndpointSlices(), s.CacheKcpSharedInformerFactory.Core().V1alpha1().Shards(), kubeClusterClient, s.KubeSharedInformerFactory.Core().V1().Namespaces(), @@ -1068,6 +1069,7 @@ func (s *Server) installAPIExportController(ctx context.Context, config *rest.Co return wait.PollUntilContextCancel(ctx, waitPollInterval, true, func(ctx context.Context) (bool, error) { return s.ApiExtensionsSharedInformerFactory.Apiextensions().V1().CustomResourceDefinitions().Informer().HasSynced() && s.KcpSharedInformerFactory.Apis().V1alpha2().APIExports().Informer().HasSynced() && + s.KcpSharedInformerFactory.Apis().V1alpha1().APIExportEndpointSlices().Informer().HasSynced() && s.KubeSharedInformerFactory.Core().V1().Namespaces().Informer().HasSynced() && s.KubeSharedInformerFactory.Core().V1().Secrets().Informer().HasSynced(), nil }) diff --git a/pkg/server/server.go b/pkg/server/server.go index 81a29ba1eb6..74dd73f4a07 100644 --- a/pkg/server/server.go +++ b/pkg/server/server.go @@ -439,6 +439,7 @@ func (s *Server) Run(ctx context.Context) error { logger.Info("finished bootstrapping the shard workspace") go s.KcpSharedInformerFactory.Apis().V1alpha2().APIExports().Informer().Run(hookContext.Done()) + go s.KcpSharedInformerFactory.Apis().V1alpha1().APIExportEndpointSlices().Informer().Run(hookContext.Done()) go s.CacheKcpSharedInformerFactory.Apis().V1alpha2().APIExports().Informer().Run(hookContext.Done()) go s.KcpSharedInformerFactory.Core().V1alpha1().LogicalClusters().Informer().Run(hookContext.Done()) go s.KcpSharedInformerFactory.Cache().V1alpha1().CachedResources().Informer().Run(hookContext.Done()) From 2bba901e0cb13d24e00db24749e1d095fe91e1bb Mon Sep 17 00:00:00 2001 From: Mangirdas Judeikis Date: Tue, 27 May 2025 08:10:46 +0300 Subject: [PATCH 2/3] Update tests for changed beahviour Signed-off-by: Mangirdas Judeikis On-behalf-of: @SAP mangirdas.judeikis@sap.com --- Makefile | 2 +- .../apibinding_logicalcluster_test.go | 26 +- .../apibinding/apibinding_protected_test.go | 37 --- test/e2e/apibinding/apibinding_test.go | 68 ++++-- test/e2e/framework/util.go | 12 +- .../apiexportendpointslice_test.go | 13 - test/e2e/virtual/apiexport/authorizer_test.go | 79 ++++-- test/e2e/virtual/apiexport/binding_test.go | 225 ++++++++++-------- test/e2e/virtual/apiexport/openapi_test.go | 12 +- .../apiexport/virtualworkspace_test.go | 14 +- 10 files changed, 270 insertions(+), 218 deletions(-) diff --git a/Makefile b/Makefile index 046c03340a3..eb88a2571d0 100644 --- a/Makefile +++ b/Makefile @@ -349,7 +349,7 @@ test-e2e-sharded-minimal: build-all # This is just easy target to run 2 shard test server locally until manually killed. # You can targer test to it by running: -# go test ./test/e2e/apibinding/... --kcp-kubeconfig=${WORK_DIR:-.}/.kcp/admin.kubeconfig --shard-kubeconfigs=root=${WORK_DIR:-.}/.kcp-0/admin.kubeconfig -run=^TestAPIBindingEndpointSlicesSharded$ +# go test ./test/e2e/apibinding/... --kcp-kubeconfig=$(pwd)/.kcp/admin.kubeconfig --shard-kubeconfigs=root=$(pwd)/.kcp-0/admin.kubeconfig -run=^TestAPIBinding$ test-run-sharded-server: WORK_DIR ?= $(PWD) test-run-sharded-server: LOG_DIR ?= $(WORK_DIR)/.kcp test-run-sharded-server: diff --git a/test/e2e/apibinding/apibinding_logicalcluster_test.go b/test/e2e/apibinding/apibinding_logicalcluster_test.go index b0a59090be2..954e43d921f 100644 --- a/test/e2e/apibinding/apibinding_logicalcluster_test.go +++ b/test/e2e/apibinding/apibinding_logicalcluster_test.go @@ -22,6 +22,7 @@ import ( "testing" "time" + "github.com/davecgh/go-spew/spew" "github.com/stretchr/testify/require" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" @@ -32,6 +33,7 @@ import ( kcpapiextensionsclientset "github.com/kcp-dev/client-go/apiextensions/client" kcpdynamic "github.com/kcp-dev/client-go/dynamic" + "github.com/kcp-dev/kcp/sdk/apis/apis/v1alpha1" apisv1alpha2 "github.com/kcp-dev/kcp/sdk/apis/apis/v1alpha2" corev1alpha1 "github.com/kcp-dev/kcp/sdk/apis/core/v1alpha1" "github.com/kcp-dev/kcp/sdk/apis/third_party/conditions/util/conditions" @@ -132,8 +134,13 @@ func TestAPIBindingLogicalCluster(t *testing.T) { return kcpClusterClient.Cluster(consumerPath).ApisV1alpha2().APIBindings().Get(ctx, exportName, metav1.GetOptions{}) }, kcptestinghelpers.Is(apisv1alpha2.PermissionClaimsValid), "unable to see valid claims") - export, err := kcpClusterClient.Cluster(providerPath).ApisV1alpha2().APIExports().Get(ctx, exportName, metav1.GetOptions{}) - require.NoError(t, err) + t.Logf("Waiting for APIExportEndpointSlice to be available") + var exportEndpointSlice *v1alpha1.APIExportEndpointSlice + kcptestinghelpers.Eventually(t, func() (bool, string) { + var err error + exportEndpointSlice, err = kcpClusterClient.Cluster(providerPath).ApisV1alpha1().APIExportEndpointSlices().Get(ctx, exportName, metav1.GetOptions{}) + return err == nil && len(exportEndpointSlice.Status.APIExportEndpoints) > 0, fmt.Sprintf("failed to get APIExportEndpointSlice: %v, %s", err, spew.Sdump(exportEndpointSlice)) + }, wait.ForeverTestTimeout, time.Second*1) rawConfig, err := server.RawConfig() require.NoError(t, err) @@ -143,8 +150,7 @@ func TestAPIBindingLogicalCluster(t *testing.T) { kcptestinghelpers.Eventually(t, func() (bool, string) { items := []unstructured.Unstructured{} - //nolint:staticcheck // SA1019 VirtualWorkspaces is deprecated but not removed yet - for _, vw := range export.Status.VirtualWorkspaces { + for _, vw := range exportEndpointSlice.Status.APIExportEndpoints { vwClusterClient, err := kcpdynamic.NewForConfig(apiexportVWConfig(t, rawConfig, vw.URL)) require.NoError(t, err) @@ -265,8 +271,13 @@ func TestAPIBindingCRDs(t *testing.T) { return kcpClusterClient.Cluster(consumerPath).ApisV1alpha2().APIBindings().Get(ctx, exportName, metav1.GetOptions{}) }, kcptestinghelpers.Is(apisv1alpha2.PermissionClaimsValid), "unable to see valid claims") - export, err := kcpClusterClient.Cluster(providerPath).ApisV1alpha2().APIExports().Get(ctx, exportName, metav1.GetOptions{}) - require.NoError(t, err) + t.Logf("Waiting for APIExportEndpointSlice to be available") + var exportEndpointSlice *v1alpha1.APIExportEndpointSlice + kcptestinghelpers.Eventually(t, func() (bool, string) { + var err error + exportEndpointSlice, err = kcpClusterClient.Cluster(providerPath).ApisV1alpha1().APIExportEndpointSlices().Get(ctx, exportName, metav1.GetOptions{}) + return err == nil && len(exportEndpointSlice.Status.APIExportEndpoints) > 0, fmt.Sprintf("failed to get APIExportEndpointSlice: %v. %s", err, spew.Sdump(exportEndpointSlice)) + }, time.Second*60, time.Second*1) rawConfig, err := server.RawConfig() require.NoError(t, err) @@ -276,8 +287,7 @@ func TestAPIBindingCRDs(t *testing.T) { kcptestinghelpers.Eventually(t, func() (bool, string) { items := []unstructured.Unstructured{} - //nolint:staticcheck // SA1019 VirtualWorkspaces is deprecated but not removed yet - for _, vw := range export.Status.VirtualWorkspaces { + for _, vw := range exportEndpointSlice.Status.APIExportEndpoints { vwClusterClient, err := kcpdynamic.NewForConfig(apiexportVWConfig(t, rawConfig, vw.URL)) require.NoError(t, err) diff --git a/test/e2e/apibinding/apibinding_protected_test.go b/test/e2e/apibinding/apibinding_protected_test.go index f35fce69c53..0996afb47eb 100644 --- a/test/e2e/apibinding/apibinding_protected_test.go +++ b/test/e2e/apibinding/apibinding_protected_test.go @@ -30,11 +30,9 @@ import ( "k8s.io/client-go/restmapper" kcpdynamic "github.com/kcp-dev/client-go/dynamic" - "github.com/kcp-dev/logicalcluster/v3" "github.com/kcp-dev/kcp/config/helpers" apisv1alpha2 "github.com/kcp-dev/kcp/sdk/apis/apis/v1alpha2" - "github.com/kcp-dev/kcp/sdk/apis/tenancy" "github.com/kcp-dev/kcp/sdk/apis/third_party/conditions/util/conditions" kcpclientset "github.com/kcp-dev/kcp/sdk/client/clientset/versioned/cluster" kcptesting "github.com/kcp-dev/kcp/sdk/testing" @@ -129,38 +127,3 @@ func TestProtectedAPI(t *testing.T) { return resourceExists(resources, "tlsroutes") }, wait.ForeverTestTimeout, time.Millisecond*100, "consumer workspace %q discovery is missing tlsroutes resource", consumerPath) } - -// TestProtectedAPIFromServiceExports is testing if we can access service exports -// from root workspace. See https://github.com/kcp-dev/kcp/issues/2184 for more details. -func TestProtectedAPIFromServiceExports(t *testing.T) { - t.Parallel() - framework.Suite(t, "control-plane") - - server := kcptesting.SharedKcpServer(t) - - ctx, cancel := context.WithCancel(context.Background()) - t.Cleanup(cancel) - - rootWorkspace := logicalcluster.NewPath("root") - cfg := server.BaseConfig(t) - - kcpClusterClient, err := kcpclientset.NewForConfig(cfg) - require.NoError(t, err, "failed to construct kcp cluster client for server") - - t.Logf("Get tenancy APIExport from root workspace") - tenanciesRoot, err := kcpClusterClient.ApisV1alpha2().APIExports().Cluster(rootWorkspace).Get(ctx, tenancy.GroupName, metav1.GetOptions{}) - require.NoError(t, err) - - t.Logf("Construct VirtualWorkspace client") - //nolint:staticcheck // SA1019 VirtualWorkspaces is deprecated but not removed yet - vwURL := tenanciesRoot.Status.VirtualWorkspaces[0].URL - cfgVW := server.RootShardSystemMasterBaseConfig(t) - cfgVW.Host = vwURL - - vwClient, err := kcpclientset.NewForConfig(cfgVW) - require.NoError(t, err) - - t.Logf("Make sure we can access tenancy API from VirtualWorkspace") - _, err = vwClient.TenancyV1alpha1().Workspaces().List(ctx, metav1.ListOptions{}) - require.NoError(t, err) -} diff --git a/test/e2e/apibinding/apibinding_test.go b/test/e2e/apibinding/apibinding_test.go index 9cd515ee92c..a4ec08407ce 100644 --- a/test/e2e/apibinding/apibinding_test.go +++ b/test/e2e/apibinding/apibinding_test.go @@ -23,6 +23,7 @@ import ( "net/url" "path" "reflect" + "sort" "strings" "testing" "time" @@ -49,6 +50,7 @@ import ( apisv1alpha2 "github.com/kcp-dev/kcp/sdk/apis/apis/v1alpha2" "github.com/kcp-dev/kcp/sdk/apis/core" corev1alpha1 "github.com/kcp-dev/kcp/sdk/apis/core/v1alpha1" + tenancyv1alpha1 "github.com/kcp-dev/kcp/sdk/apis/tenancy/v1alpha1" "github.com/kcp-dev/kcp/sdk/apis/third_party/conditions/util/conditions" kcpclientset "github.com/kcp-dev/kcp/sdk/client/clientset/versioned/cluster" kcptesting "github.com/kcp-dev/kcp/sdk/testing" @@ -163,21 +165,47 @@ func TestAPIBinding(t *testing.T) { t.Parallel() server := kcptesting.SharedKcpServer(t) + cfg := server.BaseConfig(t) ctx, cancel := context.WithCancel(context.Background()) t.Cleanup(cancel) + t.Logf("Check if we can access shards") + var shards *corev1alpha1.ShardList + { + kcpClusterClient, err := kcpclientset.NewForConfig(cfg) + require.NoError(t, err, "failed to construct kcp cluster client for server") + + shards, err = kcpClusterClient.Cluster(core.RootCluster.Path()).CoreV1alpha1().Shards().List(ctx, metav1.ListOptions{}) + require.NoError(t, err, "failed to list shards") + } + + t.Logf("Shards: %v", shards.Items) + orgPath, _ := framework.NewOrganizationFixture(t, server) //nolint:staticcheck // TODO: switch to NewWorkspaceFixture. - provider1Path, provider1 := kcptesting.NewWorkspaceFixture(t, server, orgPath, kcptesting.WithName("service-provider-1")) + var provider1Path, provider2Path logicalcluster.Path + var provider1, provider2 *tenancyv1alpha1.Workspace + if len(shards.Items) == 1 { + provider1Path, provider1 = kcptesting.NewWorkspaceFixture(t, server, orgPath, kcptesting.WithName("service-provider-1"), kcptesting.WithShard(shards.Items[0].Name)) + provider2Path, provider2 = kcptesting.NewWorkspaceFixture(t, server, orgPath, kcptesting.WithName("service-provider-2"), kcptesting.WithShard(shards.Items[0].Name)) + } else { + provider1Path, provider1 = kcptesting.NewWorkspaceFixture(t, server, orgPath, kcptesting.WithName("service-provider-1"), kcptesting.WithShard(shards.Items[0].Name)) + provider2Path, provider2 = kcptesting.NewWorkspaceFixture(t, server, orgPath, kcptesting.WithName("service-provider-2"), kcptesting.WithShard(shards.Items[1].Name)) + } provider1ClusterName := logicalcluster.Name(provider1.Spec.Cluster) - provider2Path, provider2 := kcptesting.NewWorkspaceFixture(t, server, orgPath, kcptesting.WithName("service-provider-2")) provider2ClusterName := logicalcluster.Name(provider2.Spec.Cluster) - consumer1Path, _ := kcptesting.NewWorkspaceFixture(t, server, orgPath, kcptesting.WithName("consumer-1-bound-against-1")) - consumer2Path, _ := kcptesting.NewWorkspaceFixture(t, server, orgPath, kcptesting.WithName("consumer-2-bound-against-1")) - consumer3Path, consumer3Workspace := kcptesting.NewWorkspaceFixture(t, server, orgPath, kcptesting.WithName("consumer-3-bound-against-2")) - consumer3ClusterName := logicalcluster.Name(consumer3Workspace.Spec.Cluster) - cfg := server.BaseConfig(t) + consumer1Path, _ := kcptesting.NewWorkspaceFixture(t, server, orgPath, kcptesting.WithName("consumer-1-bound-against-1"), kcptesting.WithShard(shards.Items[0].Name)) + var consumer2Path, consumer3Path logicalcluster.Path + var consumer3Workspace *tenancyv1alpha1.Workspace + if len(shards.Items) == 1 { + consumer2Path, _ = kcptesting.NewWorkspaceFixture(t, server, orgPath, kcptesting.WithName("consumer-2-bound-against-1"), kcptesting.WithShard(shards.Items[0].Name)) + consumer3Path, consumer3Workspace = kcptesting.NewWorkspaceFixture(t, server, orgPath, kcptesting.WithName("consumer-3-bound-against-2"), kcptesting.WithShard(shards.Items[0].Name)) + } else { + consumer2Path, _ = kcptesting.NewWorkspaceFixture(t, server, orgPath, kcptesting.WithName("consumer-2-bound-against-1"), kcptesting.WithShard(shards.Items[1].Name)) + consumer3Path, consumer3Workspace = kcptesting.NewWorkspaceFixture(t, server, orgPath, kcptesting.WithName("consumer-3-bound-against-2"), kcptesting.WithShard(shards.Items[1].Name)) + } + consumer3ClusterName := logicalcluster.Name(consumer3Workspace.Spec.Cluster) kcpClusterClient, err := kcpclientset.NewForConfig(cfg) require.NoError(t, err, "failed to construct kcp cluster client for server") @@ -187,8 +215,6 @@ func TestAPIBinding(t *testing.T) { shardVirtualWorkspaceURLs := sets.New[string]() t.Logf("Getting a list of VirtualWorkspaceURLs assigned to Shards") - shards, err := kcpClusterClient.Cluster(core.RootCluster.Path()).CoreV1alpha1().Shards().List(ctx, metav1.ListOptions{}) - require.NoError(t, err) // Filtering out shards that are not schedulable var shardItems []corev1alpha1.Shard for _, s := range shards.Items { @@ -365,25 +391,27 @@ func TestAPIBinding(t *testing.T) { expectedURLs = append(expectedURLs, u.String()) } - t.Logf("Make sure the APIExport gets status.virtualWorkspaceURLs set") + t.Logf("Make sure the APIExportEndpointSlice gets status.virtualWorkspaceURLs set") kcptestinghelpers.Eventually(t, func() (bool, string) { - e, err := kcpClusterClient.Cluster(serviceProviderClusterName.Path()).ApisV1alpha2().APIExports().Get(ctx, exportName, metav1.GetOptions{}) + apiExportEndpointSlice, err := kcpClusterClient.Cluster(serviceProviderClusterName.Path()).ApisV1alpha1().APIExportEndpointSlices().Get(ctx, exportName, metav1.GetOptions{}) if err != nil { - t.Logf("Unexpected error getting APIExport %s|%s: %v", serviceProviderClusterName.Path(), exportName, err) + t.Logf("Unexpected error getting APIExportEndpointSlice %s|%s: %v", serviceProviderClusterName.Path(), exportName, err) } var actualURLs []string - //nolint:staticcheck // SA1019 VirtualWorkspaces is deprecated but not removed yet - for _, u := range e.Status.VirtualWorkspaces { + for _, u := range apiExportEndpointSlice.Status.APIExportEndpoints { actualURLs = append(actualURLs, u.URL) } - + sort.Strings(expectedURLs) + sort.Strings(actualURLs) + t.Logf("Expected URLs: %v", expectedURLs) + t.Logf("Actual URLs: %v", actualURLs) if !reflect.DeepEqual(expectedURLs, actualURLs) { return false, fmt.Sprintf("Unexpected URLs. Diff: %s", cmp.Diff(expectedURLs, actualURLs)) } return true, "" - }, wait.ForeverTestTimeout, 100*time.Millisecond, "APIExport %s|%s didn't get status.virtualWorkspaceURLs set correctly", + }, wait.ForeverTestTimeout, 100*time.Millisecond, "APIExportEndpointSlices %s|%s didn't get status.virtualWorkspaceURLs set correctly", serviceProviderClusterName, exportName) } @@ -438,15 +466,14 @@ func TestAPIBinding(t *testing.T) { t.Logf("=== Verify that %s|%s export virtual workspace shows cowboys", provider2Path, exportName) rawConfig, err := server.RawConfig() require.NoError(t, err) - export2, err := kcpClusterClient.Cluster(provider2Path).ApisV1alpha2().APIExports().Get(ctx, exportName, metav1.GetOptions{}) + apiExportEndpointSlice2, err := kcpClusterClient.Cluster(provider2Path).ApisV1alpha1().APIExportEndpointSlices().Get(ctx, exportName, metav1.GetOptions{}) require.NoError(t, err) kcptestinghelpers.Eventually(t, func() (bool, string) { foundOnShards := 0 var listErrs []error - //nolint:staticcheck // SA1019 VirtualWorkspaces is deprecated but not removed yet - for _, vw := range export2.Status.VirtualWorkspaces { + for _, vw := range apiExportEndpointSlice2.Status.APIExportEndpoints { vw2ClusterClient, err := kcpdynamic.NewForConfig(apiexportVWConfig(t, rawConfig, vw.URL)) require.NoError(t, err) @@ -476,8 +503,7 @@ func TestAPIBinding(t *testing.T) { return false, "couldn't list via virtual workspaces because the user is not ready yet" } - //nolint:staticcheck // SA1019 VirtualWorkspaces is deprecated but not removed yet - require.Equal(t, 1, foundOnShards, "cowboys not found exactly on one shard, but on %d/%d", foundOnShards, len(export2.Status.VirtualWorkspaces)) + require.Equal(t, 1, foundOnShards, "cowboys not found exactly on one shard, but on %d/%d", foundOnShards, len(apiExportEndpointSlice2.Status.APIExportEndpoints)) return true, "" }, wait.ForeverTestTimeout, 100*time.Millisecond, "expected to have cowboys exactly on one shard") diff --git a/test/e2e/framework/util.go b/test/e2e/framework/util.go index 27f22a5e382..01ea50ffde3 100644 --- a/test/e2e/framework/util.go +++ b/test/e2e/framework/util.go @@ -32,7 +32,7 @@ import ( "github.com/kcp-dev/kcp/pkg/authorization" bootstrappolicy "github.com/kcp-dev/kcp/pkg/authorization/bootstrap" "github.com/kcp-dev/kcp/pkg/server" - apisv1alpha2 "github.com/kcp-dev/kcp/sdk/apis/apis/v1alpha2" + apisv1alpha1 "github.com/kcp-dev/kcp/sdk/apis/apis/v1alpha1" tenancyv1alpha1 "github.com/kcp-dev/kcp/sdk/apis/tenancy/v1alpha1" kcpclientset "github.com/kcp-dev/kcp/sdk/client/clientset/versioned/cluster" testing2 "github.com/kcp-dev/kcp/sdk/testing" @@ -84,12 +84,10 @@ func VirtualWorkspaceURL(ctx context.Context, kcpClusterClient kcpclientset.Clus } // ExportVirtualWorkspaceURLs returns the URLs of the virtual workspaces of the -// given APIExport. -func ExportVirtualWorkspaceURLs(export *apisv1alpha2.APIExport) []string { - //nolint:staticcheck // SA1019 VirtualWorkspaces is deprecated but not removed yet - urls := make([]string, 0, len(export.Status.VirtualWorkspaces)) - //nolint:staticcheck // SA1019 VirtualWorkspaces is deprecated but not removed yet - for _, vw := range export.Status.VirtualWorkspaces { +// given APIExportEndpointSlice. +func ExportVirtualWorkspaceURLs(export *apisv1alpha1.APIExportEndpointSlice) []string { + urls := make([]string, 0, len(export.Status.APIExportEndpoints)) + for _, vw := range export.Status.APIExportEndpoints { urls = append(urls, vw.URL) } return urls diff --git a/test/e2e/reconciler/apiexportendpointslice/apiexportendpointslice_test.go b/test/e2e/reconciler/apiexportendpointslice/apiexportendpointslice_test.go index f7658d8db65..5b26f9b26c0 100644 --- a/test/e2e/reconciler/apiexportendpointslice/apiexportendpointslice_test.go +++ b/test/e2e/reconciler/apiexportendpointslice/apiexportendpointslice_test.go @@ -275,19 +275,6 @@ func TestAPIBindingEndpointSlicesSharded(t *testing.T) { } } - // TODO(mjudeikis): This will be deprecated when we deperecate APIExport urls. - t.Logf("Check that APIExport has 2 virtual workspaces") - { - kcpClusterClient, err := kcpclientset.NewForConfig(cfg) - require.NoError(t, err, "failed to construct kcp cluster client for server") - - apiExport, err := kcpClusterClient.Cluster(providerPath).ApisV1alpha2().APIExports().Get(ctx, "today-cowboys", metav1.GetOptions{}) - require.NoError(t, err) - - //nolint:staticcheck // SA1019 VirtualWorkspaces is deprecated but not removed yet - require.Len(t, apiExport.Status.VirtualWorkspaces, len(shards.Items)) - } - t.Logf("Create a topology PartitionSet for the providers") var partition *topologyv1alpha1.Partition { diff --git a/test/e2e/virtual/apiexport/authorizer_test.go b/test/e2e/virtual/apiexport/authorizer_test.go index bf0f7e74af2..d49e8cdde60 100644 --- a/test/e2e/virtual/apiexport/authorizer_test.go +++ b/test/e2e/virtual/apiexport/authorizer_test.go @@ -54,6 +54,7 @@ import ( apisv1alpha1 "github.com/kcp-dev/kcp/sdk/apis/apis/v1alpha1" apisv1alpha2 "github.com/kcp-dev/kcp/sdk/apis/apis/v1alpha2" "github.com/kcp-dev/kcp/sdk/apis/core" + corev1alpha1 "github.com/kcp-dev/kcp/sdk/apis/core/v1alpha1" "github.com/kcp-dev/kcp/sdk/apis/tenancy" tenancyv1alpha1 "github.com/kcp-dev/kcp/sdk/apis/tenancy/v1alpha1" "github.com/kcp-dev/kcp/sdk/apis/third_party/conditions/util/conditions" @@ -195,7 +196,6 @@ func TestAPIExportAuthorizers(t *testing.T) { }, }, }, - &rbacv1.ClusterRole{ ObjectMeta: metav1.ObjectMeta{Name: "tenant-user-bind"}, Rules: []rbacv1.PolicyRule{ @@ -402,13 +402,12 @@ metadata: serviceProvider2AdminClient, err := kcpclientset.NewForConfig(serviceProvider2Admin) require.NoError(t, err) kcptestinghelpers.Eventually(t, func() (bool, string) { - apiExport, err := serviceProvider2AdminClient.Cluster(serviceProvider2Path).ApisV1alpha2().APIExports().Get(ctx, "today-cowboys", metav1.GetOptions{}) + apiExportEndpointSlice, err := serviceProvider2AdminClient.Cluster(serviceProvider2Path).ApisV1alpha1().APIExportEndpointSlices().Get(ctx, "today-cowboys", metav1.GetOptions{}) require.NoError(t, err) var found bool - serviceProvider2AdminApiExportVWCfg.Host, found, err = framework.VirtualWorkspaceURL(ctx, kcpClient, tenantWorkspace, framework.ExportVirtualWorkspaceURLs(apiExport)) + serviceProvider2AdminApiExportVWCfg.Host, found, err = framework.VirtualWorkspaceURL(ctx, kcpClient, tenantWorkspace, framework.ExportVirtualWorkspaceURLs(apiExportEndpointSlice)) require.NoError(t, err) - //nolint:staticcheck // SA1019 VirtualWorkspaces is deprecated but not removed yet - return found, fmt.Sprintf("waiting for virtual workspace URLs to be available: %v", apiExport.Status.VirtualWorkspaces) + return found, fmt.Sprintf("waiting for virtual workspace URLs to be available: %v", apiExportEndpointSlice.Status.APIExportEndpoints) }, wait.ForeverTestTimeout, time.Millisecond*100) serviceProvider2DynamicVWClientForTenantWorkspace, err := kcpdynamic.NewForConfig(serviceProvider2AdminApiExportVWCfg) @@ -473,13 +472,12 @@ metadata: shadowVWClient, err := kcpclientset.NewForConfig(serviceProvider2Admin) require.NoError(t, err) kcptestinghelpers.Eventually(t, func() (bool, string) { - apiExport, err := shadowVWClient.Cluster(serviceProvider2Path).ApisV1alpha2().APIExports().Get(ctx, "today-cowboys", metav1.GetOptions{}) + apiExportEndpointSlice, err := shadowVWClient.Cluster(serviceProvider2Path).ApisV1alpha1().APIExportEndpointSlices().Get(ctx, "today-cowboys", metav1.GetOptions{}) require.NoError(t, err) var found bool - shadowVWCfg.Host, found, err = framework.VirtualWorkspaceURL(ctx, kcpClient, tenantShadowCRDWorkspace, framework.ExportVirtualWorkspaceURLs(apiExport)) + shadowVWCfg.Host, found, err = framework.VirtualWorkspaceURL(ctx, kcpClient, tenantShadowCRDWorkspace, framework.ExportVirtualWorkspaceURLs(apiExportEndpointSlice)) require.NoError(t, err) - //nolint:staticcheck // SA1019 VirtualWorkspaces is deprecated but not removed yet - return found, fmt.Sprintf("waiting for virtual workspace URLs to be available: %v", apiExport.Status.VirtualWorkspaces) + return found, fmt.Sprintf("waiting for virtual workspace URLs to be available: %v", apiExportEndpointSlice.Status.APIExportEndpoints) }, wait.ForeverTestTimeout, time.Millisecond*100) serviceProvider2DynamicVWClientForShadowTenantWorkspace, err := kcpdynamic.NewForConfig(shadowVWCfg) @@ -508,12 +506,23 @@ func TestAPIExportBindingAuthorizer(t *testing.T) { t.Cleanup(cancel) orgPath, _ := framework.NewOrganizationFixture(t, server) //nolint:staticcheck + cfg := server.BaseConfig(t) + + // We list shards so we create fake consumers in each shard. This way APIExportEndpointSlice url will be available in each shard. + // And we can replace logicalcluster in the url and test other workspaces for authorization leaking. + t.Logf("Check if we can access shards") + var shards *corev1alpha1.ShardList + { + kcpClusterClient, err := kcpclientset.NewForConfig(cfg) + require.NoError(t, err, "failed to construct kcp cluster client for server") + + shards, err = kcpClusterClient.Cluster(core.RootCluster.Path()).CoreV1alpha1().Shards().List(ctx, metav1.ListOptions{}) + require.NoError(t, err, "failed to list shards") + } serviceProviderPath, _ := kcptesting.NewWorkspaceFixture(t, server, orgPath, kcptesting.WithName("service-provider")) tenantPath, tenantWorkspace := kcptesting.NewWorkspaceFixture(t, server, orgPath, kcptesting.WithName("tenant")) - cfg := server.BaseConfig(t) - serviceProviderAdmin := server.ClientCAUserConfig(t, rest.CopyConfig(cfg), "service-provider-admin") tenantUser := server.ClientCAUserConfig(t, rest.CopyConfig(cfg), "tenant-user") @@ -578,18 +587,54 @@ func TestAPIExportBindingAuthorizer(t *testing.T) { }, )) + t.Logf("Bind \"wild.wild.west\" APIExport in workspace %q to tenant workspace %q", serviceProviderPath, tenantPath) + apiBinding := &apisv1alpha2.APIBinding{ + ObjectMeta: metav1.ObjectMeta{ + Name: "wild.wild.west", + }, + Spec: apisv1alpha2.APIBindingSpec{ + Reference: apisv1alpha2.BindingReference{ + Export: &apisv1alpha2.ExportBindingReference{ + Path: serviceProviderPath.String(), + Name: "wild.wild.west", + }, + }, + PermissionClaims: []apisv1alpha2.AcceptablePermissionClaim{ + { + PermissionClaim: apisv1alpha2.PermissionClaim{ + GroupResource: apisv1alpha2.GroupResource{Resource: "configmaps"}, + Verbs: []string{"*"}, + All: true, + }, + State: apisv1alpha2.ClaimAccepted, + }, + }, + }, + } + + // Bind to second tenant workspace so url pops up. So we use this to check tenant1 access. + for _, shard := range shards.Items { + tenant2Path, _ := kcptesting.NewWorkspaceFixture(t, server, orgPath, kcptesting.WithName(shard.Name+"-tenant-2")) + kcptestinghelpers.Eventually(t, func() (bool, string) { + _, err = kcpClient.Cluster(tenant2Path).ApisV1alpha2().APIBindings().Create(ctx, apiBinding, metav1.CreateOptions{}) + return err == nil, fmt.Sprintf("Error creating APIBinding: %v", err) + }, wait.ForeverTestTimeout, time.Millisecond*100) + } + t.Logf("Create virtual workspace client for \"today-sherriffs\" APIExport in workspace %q", serviceProviderPath) serviceProviderAdminApiExportVWCfg := rest.CopyConfig(serviceProviderAdmin) serviceProviderAdminClient, err := kcpclientset.NewForConfig(serviceProviderAdmin) require.NoError(t, err) kcptestinghelpers.Eventually(t, func() (bool, string) { - apiExport, err := serviceProviderAdminClient.Cluster(serviceProviderPath).ApisV1alpha2().APIExports().Get(ctx, "wild.wild.west", metav1.GetOptions{}) + apiExportEndpointSlice, err := serviceProviderAdminClient.Cluster(serviceProviderPath).ApisV1alpha1().APIExportEndpointSlices().Get(ctx, "wild.wild.west", metav1.GetOptions{}) require.NoError(t, err) var found bool - serviceProviderAdminApiExportVWCfg.Host, found, err = framework.VirtualWorkspaceURL(ctx, kcpClient, tenantWorkspace, framework.ExportVirtualWorkspaceURLs(apiExport)) + serviceProviderAdminApiExportVWCfg.Host, found, err = framework.VirtualWorkspaceURL(ctx, kcpClient, tenantWorkspace, framework.ExportVirtualWorkspaceURLs(apiExportEndpointSlice)) require.NoError(t, err) - //nolint:staticcheck // SA1019 VirtualWorkspaces is deprecated but not removed yet - return found, fmt.Sprintf("waiting for virtual workspace URLs to be available: %v", apiExport.Status.VirtualWorkspaces) + if !found { + return false, fmt.Sprintf("waiting for virtual workspace URLs to be available: %v", apiExportEndpointSlice.Status.APIExportEndpoints) + } + return true, "" }, wait.ForeverTestTimeout, time.Millisecond*100) serviceProviderDynamicVWClientForTenantWorkspace, err := kcpdynamic.NewForConfig(serviceProviderAdminApiExportVWCfg) @@ -1015,9 +1060,9 @@ func vwURL(t *testing.T, kcpClusterClient kcpclientset.ClusterInterface, path lo var vwURL string kcptestinghelpers.Eventually(t, func() (bool, string) { - export, err := kcpClusterClient.Cluster(path).ApisV1alpha2().APIExports().Get(ctx, export, metav1.GetOptions{}) + exportEndpointSlice, err := kcpClusterClient.Cluster(path).ApisV1alpha1().APIExportEndpointSlices().Get(ctx, export, metav1.GetOptions{}) require.NoError(t, err) - urls := framework.ExportVirtualWorkspaceURLs(export) + urls := framework.ExportVirtualWorkspaceURLs(exportEndpointSlice) var found bool vwURL, found, err = framework.VirtualWorkspaceURL(ctx, kcpClusterClient, ws, urls) require.NoError(t, err) diff --git a/test/e2e/virtual/apiexport/binding_test.go b/test/e2e/virtual/apiexport/binding_test.go index cbacc09fea0..3c93efc613d 100644 --- a/test/e2e/virtual/apiexport/binding_test.go +++ b/test/e2e/virtual/apiexport/binding_test.go @@ -28,6 +28,7 @@ import ( "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" + rbacv1 "k8s.io/api/rbac/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/wait" "k8s.io/client-go/rest" @@ -72,46 +73,59 @@ func TestBinding(t *testing.T) { framework.AdmitWorkspaceAccess(ctx, t, kubeClient, consumerWorkspacePath, []string{"service-provider"}, nil, true) t.Logf("Creating 'api-manager' APIExport in service-provider workspace with apibindings permission claim") - require.NoError(t, apply(t, ctx, serviceWorkspacePath, cfg, ` -apiVersion: apis.kcp.io/v1alpha2 -kind: APIExport -metadata: - name: api-manager -spec: - permissionClaims: - - group: "apis.kcp.io" - resource: "apibindings" - all: true - verbs: ["*"] -`)) - + require.NoError(t, apply(t, ctx, serviceWorkspacePath, cfg, + &apisv1alpha2.APIExport{ + ObjectMeta: metav1.ObjectMeta{Name: "api-manager"}, + Spec: apisv1alpha2.APIExportSpec{ + PermissionClaims: []apisv1alpha2.PermissionClaim{ + { + GroupResource: apisv1alpha2.GroupResource{ + Group: "apis.kcp.io", + Resource: "apibindings", + }, + All: true, + Verbs: []string{"*"}, + }, + }, + }, + }, + )) t.Logf("Creating APIExport in restricted workspace without anybody allowed to bind") - require.NoError(t, apply(t, ctx, restrictedWorkspacePath, cfg, ` -apiVersion: apis.kcp.io/v1alpha2 -kind: APIExport -metadata: - name: restricted-service -spec: {} -`)) + require.NoError(t, apply(t, ctx, restrictedWorkspacePath, cfg, + &apisv1alpha2.APIExport{ + ObjectMeta: metav1.ObjectMeta{Name: "restricted-service"}, + Spec: apisv1alpha2.APIExportSpec{}, + }, + )) t.Logf("Binding to 'api-manager' APIExport succeeds because service-provider user is admin in 'service-provider' workspace") kcptestinghelpers.Eventually(t, func() (success bool, reason string) { - err = apply(t, ctx, consumerWorkspacePath, serviceProviderUser, fmt.Sprintf(` -apiVersion: apis.kcp.io/v1alpha1 -kind: APIBinding -metadata: - name: api-manager -spec: - permissionClaims: - - group: apis.kcp.io - resource: apibindings - state: Accepted - all: true - reference: - export: - name: api-manager - path: %v -`, serviceWorkspacePath.String())) + err = apply(t, ctx, consumerWorkspacePath, serviceProviderUser, + &apisv1alpha2.APIBinding{ + ObjectMeta: metav1.ObjectMeta{Name: "api-manager"}, + Spec: apisv1alpha2.APIBindingSpec{ + PermissionClaims: []apisv1alpha2.AcceptablePermissionClaim{ + { + PermissionClaim: apisv1alpha2.PermissionClaim{ + GroupResource: apisv1alpha2.GroupResource{ + Group: "apis.kcp.io", + Resource: "apibindings", + }, + All: true, + Verbs: []string{"*"}, + }, + State: apisv1alpha2.ClaimAccepted, + }, + }, + Reference: apisv1alpha2.BindingReference{ + Export: &apisv1alpha2.ExportBindingReference{ + Name: "api-manager", + Path: serviceWorkspacePath.String(), + }, + }, + }, + }, + ) if err != nil { return false, fmt.Sprintf("Waiting on binding 'api-manager' export: %v", err.Error()) } @@ -120,16 +134,19 @@ spec: t.Logf("Binding directly to 'restricted-service' APIExport should be forbidden") kcptestinghelpers.Eventually(t, func() (success bool, reason string) { - err = apply(t, ctx, consumerWorkspacePath, serviceProviderUser, fmt.Sprintf(` -apiVersion: apis.kcp.io/v1alpha1 -kind: APIBinding -metadata: - name: should-fail -spec: - reference: - export: - name: restricted-service - path: %v`, restrictedWorkspacePath.String())) + err = apply(t, ctx, consumerWorkspacePath, serviceProviderUser, + &apisv1alpha2.APIBinding{ + ObjectMeta: metav1.ObjectMeta{Name: "should-fail"}, + Spec: apisv1alpha2.APIBindingSpec{ + Reference: apisv1alpha2.BindingReference{ + Export: &apisv1alpha2.ExportBindingReference{ + Name: "restricted-service", + Path: restrictedWorkspacePath.String(), + }, + }, + }, + }, + ) require.Error(t, err) want := "unable to create APIBinding: no permission to bind to export" if got := err.Error(); !strings.Contains(got, want) { @@ -141,29 +158,31 @@ spec: t.Logf("Waiting for 'api-manager' APIExport virtual workspace URL") serviceProviderVirtualWorkspaceConfig := rest.CopyConfig(serviceProviderUser) kcptestinghelpers.Eventually(t, func() (bool, string) { - apiExport, err := kcpClient.Cluster(serviceWorkspacePath).ApisV1alpha2().APIExports().Get(ctx, "api-manager", metav1.GetOptions{}) + apiExportEndpointSlice, err := kcpClient.Cluster(serviceWorkspacePath).ApisV1alpha1().APIExportEndpointSlices().Get(ctx, "api-manager", metav1.GetOptions{}) if err != nil { return false, fmt.Sprintf("waiting on apiexport to be available %v", err.Error()) } var found bool - serviceProviderVirtualWorkspaceConfig.Host, found, err = framework.VirtualWorkspaceURL(ctx, kcpClient, consumerWorkspace, framework.ExportVirtualWorkspaceURLs(apiExport)) + serviceProviderVirtualWorkspaceConfig.Host, found, err = framework.VirtualWorkspaceURL(ctx, kcpClient, consumerWorkspace, framework.ExportVirtualWorkspaceURLs(apiExportEndpointSlice)) require.NoError(t, err) - //nolint:staticcheck // SA1019 VirtualWorkspaces is deprecated but not removed yet - return found, fmt.Sprintf("waiting for virtual workspace URLs to be available: %v", apiExport.Status.VirtualWorkspaces) + return found, fmt.Sprintf("waiting for virtual workspace URLs to be available: %v", apiExportEndpointSlice.Status.APIExportEndpoints) }, wait.ForeverTestTimeout, 100*time.Millisecond, "waiting on virtual workspace to be ready") t.Logf("Binding to 'restricted-service' APIExport through 'api-manager' APIExport virtual workspace is forbidden") kcptestinghelpers.Eventually(t, func() (success bool, reason string) { - err = apply(t, ctx, logicalcluster.Name(consumerWorkspace.Spec.Cluster).Path(), serviceProviderVirtualWorkspaceConfig, fmt.Sprintf(` -apiVersion: apis.kcp.io/v1alpha1 -kind: APIBinding -metadata: - name: should-fail -spec: - reference: - export: - name: restricted-service - path: %v`, restrictedWorkspacePath.String())) + err = apply(t, ctx, logicalcluster.Name(consumerWorkspace.Spec.Cluster).Path(), serviceProviderVirtualWorkspaceConfig, + &apisv1alpha2.APIBinding{ + ObjectMeta: metav1.ObjectMeta{Name: "should-fail"}, + Spec: apisv1alpha2.APIBindingSpec{ + Reference: apisv1alpha2.BindingReference{ + Export: &apisv1alpha2.ExportBindingReference{ + Name: "restricted-service", + Path: restrictedWorkspacePath.String(), + }, + }, + }, + }, + ) require.Error(t, err) want := "unable to create APIBinding: no permission to bind to export" if got := err.Error(); !strings.Contains(got, want) { @@ -173,46 +192,51 @@ spec: }, wait.ForeverTestTimeout, 1000*time.Millisecond, "waiting on binding to 'restricted-service' APIExport to fail because it is forbidden") t.Logf("Giving service-provider 'bind' access to 'restricted-service' APIExport") - require.NoError(t, apply(t, ctx, restrictedWorkspacePath, cfg, ` -apiVersion: rbac.authorization.k8s.io/v1 -kind: ClusterRole -metadata: - name: restricted-service:bind -rules: -- apiGroups: - - apis.kcp.io - resources: - - apiexports - verbs: - - 'bind' - resourceNames: - - 'restricted-service' -`, ` -kind: ClusterRoleBinding -apiVersion: rbac.authorization.k8s.io/v1 -metadata: - name: service-provider:restricted-service:bind -subjects: -- kind: User - name: service-provider -roleRef: - apiGroup: rbac.authorization.k8s.io - kind: ClusterRole - name: restricted-service:bind -`)) + require.NoError(t, apply(t, ctx, restrictedWorkspacePath, cfg, + &rbacv1.ClusterRole{ + ObjectMeta: metav1.ObjectMeta{Name: "restricted-service:bind"}, + Rules: []rbacv1.PolicyRule{ + { + APIGroups: []string{"apis.kcp.io"}, + Resources: []string{"apiexports"}, + Verbs: []string{"bind"}, + ResourceNames: []string{ + "restricted-service", + }, + }, + }, + }, + &rbacv1.ClusterRoleBinding{ + ObjectMeta: metav1.ObjectMeta{Name: "service-provider:restricted-service:bind"}, + Subjects: []rbacv1.Subject{ + { + Kind: "User", + Name: "service-provider", + }, + }, + RoleRef: rbacv1.RoleRef{ + APIGroup: "rbac.authorization.k8s.io", + Kind: "ClusterRole", + Name: "restricted-service:bind", + }, + }, + )) t.Logf("Binding to 'restricted-service' APIExport through 'api-manager' APIExport virtual workspace succeeds, proving that the service provider identity is used through the APIExport virtual workspace") kcptestinghelpers.Eventually(t, func() (bool, string) { - err := apply(t, ctx, logicalcluster.Name(consumerWorkspace.Spec.Cluster).Path(), serviceProviderVirtualWorkspaceConfig, fmt.Sprintf(` -apiVersion: apis.kcp.io/v1alpha1 -kind: APIBinding -metadata: - name: should-not-fail -spec: - reference: - export: - name: restricted-service - path: %v`, restrictedWorkspacePath.String())) + err := apply(t, ctx, logicalcluster.Name(consumerWorkspace.Spec.Cluster).Path(), serviceProviderVirtualWorkspaceConfig, + &apisv1alpha2.APIBinding{ + ObjectMeta: metav1.ObjectMeta{Name: "should-not-fail"}, + Spec: apisv1alpha2.APIBindingSpec{ + Reference: apisv1alpha2.BindingReference{ + Export: &apisv1alpha2.ExportBindingReference{ + Name: "restricted-service", + Path: restrictedWorkspacePath.String(), + }, + }, + }, + }, + ) if err != nil { return false, fmt.Sprintf("waiting on being able to bind: %v", err.Error()) } @@ -285,13 +309,12 @@ func TestAPIBindingPermissionClaimsVerbs(t *testing.T) { t.Logf("Waiting for APIExport to have a virtual workspace URL for the bound workspace %q", consumerPath) apiExportVWCfg := rest.CopyConfig(cfg) kcptestinghelpers.Eventually(t, func() (bool, string) { - apiExport, err := kcpClusterClient.Cluster(providerPath).ApisV1alpha2().APIExports().Get(ctx, "today-cowboys", metav1.GetOptions{}) + apiExportEndpointSlice, err := kcpClusterClient.Cluster(providerPath).ApisV1alpha1().APIExportEndpointSlices().Get(ctx, "today-cowboys", metav1.GetOptions{}) require.NoError(t, err) var found bool - apiExportVWCfg.Host, found, err = framework.VirtualWorkspaceURL(ctx, kcpClusterClient, consumerWorkspace, framework.ExportVirtualWorkspaceURLs(apiExport)) + apiExportVWCfg.Host, found, err = framework.VirtualWorkspaceURL(ctx, kcpClusterClient, consumerWorkspace, framework.ExportVirtualWorkspaceURLs(apiExportEndpointSlice)) require.NoError(t, err) - //nolint:staticcheck // SA1019 VirtualWorkspaces is deprecated but not removed yet - return found, fmt.Sprintf("waiting for virtual workspace URLs to be available: %v", apiExport.Status.VirtualWorkspaces) + return found, fmt.Sprintf("waiting for virtual workspace URLs to be available: %v", apiExportEndpointSlice.Status.APIExportEndpoints) }, wait.ForeverTestTimeout, time.Millisecond*100) kubeClusterClient, err := kcpkubernetesclientset.NewForConfig(apiExportVWCfg) diff --git a/test/e2e/virtual/apiexport/openapi_test.go b/test/e2e/virtual/apiexport/openapi_test.go index bc61fed46bc..2c2ced3433b 100644 --- a/test/e2e/virtual/apiexport/openapi_test.go +++ b/test/e2e/virtual/apiexport/openapi_test.go @@ -62,23 +62,25 @@ func TestAPIExportOpenAPI(t *testing.T) { orgPath, _ := framework.NewOrganizationFixture(t, server) //nolint:staticcheck // TODO: switch to NewWorkspaceFixture. serviceProviderPath, _ := kcptesting.NewWorkspaceFixture(t, server, orgPath) - _, consumerWorkspace := kcptesting.NewWorkspaceFixture(t, server, orgPath) + consumerPath, consumerWorkspace := kcptesting.NewWorkspaceFixture(t, server, orgPath) consumerClusterName := logicalcluster.Name(consumerWorkspace.Spec.Cluster) framework.AdmitWorkspaceAccess(ctx, t, kubeClusterClient, serviceProviderPath, []string{"user-1"}, nil, false) setUpServiceProvider(ctx, t, dynamicClusterClient, kcpClients, serviceProviderPath, cfg, nil) + t.Logf("set up binding") + bindConsumerToProvider(ctx, t, consumerPath, serviceProviderPath, kcpClients, cfg) + t.Logf("Waiting for APIExport to have a virtual workspace URL for the bound workspace %q", consumerWorkspace.Name) apiExportVWCfg := rest.CopyConfig(cfg) kcptestinghelpers.Eventually(t, func() (bool, string) { - apiExport, err := kcpClients.Cluster(serviceProviderPath).ApisV1alpha2().APIExports().Get(ctx, "today-cowboys", metav1.GetOptions{}) + apiExportEndpointSlice, err := kcpClients.Cluster(serviceProviderPath).ApisV1alpha1().APIExportEndpointSlices().Get(ctx, "today-cowboys", metav1.GetOptions{}) require.NoError(t, err) var found bool - apiExportVWCfg.Host, found, err = framework.VirtualWorkspaceURL(ctx, kcpClients, consumerWorkspace, framework.ExportVirtualWorkspaceURLs(apiExport)) + apiExportVWCfg.Host, found, err = framework.VirtualWorkspaceURL(ctx, kcpClients, consumerWorkspace, framework.ExportVirtualWorkspaceURLs(apiExportEndpointSlice)) require.NoError(t, err) - //nolint:staticcheck // SA1019 VirtualWorkspaces is deprecated but not removed yet - return found, fmt.Sprintf("waiting for virtual workspace URLs to be available: %v", apiExport.Status.VirtualWorkspaces) + return found, fmt.Sprintf("waiting for virtual workspace URLs to be available: %v", apiExportEndpointSlice.Status.APIExportEndpoints) }, wait.ForeverTestTimeout, time.Millisecond*100) t.Logf("Checking /openapi/v3 paths for %q", orgPath) diff --git a/test/e2e/virtual/apiexport/virtualworkspace_test.go b/test/e2e/virtual/apiexport/virtualworkspace_test.go index 18e34093749..4075a9f2bf5 100644 --- a/test/e2e/virtual/apiexport/virtualworkspace_test.go +++ b/test/e2e/virtual/apiexport/virtualworkspace_test.go @@ -110,13 +110,12 @@ func TestAPIExportVirtualWorkspace(t *testing.T) { t.Logf("Waiting for APIExport to have a virtual workspace URL for the bound workspace %q", consumerWorkspace.Name) apiExportVWCfg := rest.CopyConfig(cfg) kcptestinghelpers.Eventually(t, func() (bool, string) { - apiExport, err := kcpClients.Cluster(serviceProviderPath).ApisV1alpha2().APIExports().Get(ctx, "today-cowboys", metav1.GetOptions{}) + apiExportEndpointSlice, err := kcpClients.Cluster(serviceProviderPath).ApisV1alpha1().APIExportEndpointSlices().Get(ctx, "today-cowboys", metav1.GetOptions{}) require.NoError(t, err) var found bool - apiExportVWCfg.Host, found, err = framework.VirtualWorkspaceURL(ctx, kcpClients, consumerWorkspace, framework.ExportVirtualWorkspaceURLs(apiExport)) + apiExportVWCfg.Host, found, err = framework.VirtualWorkspaceURL(ctx, kcpClients, consumerWorkspace, framework.ExportVirtualWorkspaceURLs(apiExportEndpointSlice)) require.NoError(t, err) - //nolint:staticcheck // SA1019 VirtualWorkspaces is deprecated but not removed yet - return found, fmt.Sprintf("waiting for virtual workspace URLs to be available: %v", apiExport.Status.VirtualWorkspaces) + return found, fmt.Sprintf("waiting for virtual workspace URLs to be available: %v", apiExportEndpointSlice.Status.APIExportEndpoints) }, wait.ForeverTestTimeout, time.Millisecond*100) t.Logf("Verifying that the virtual workspace includes the cowboy resource") @@ -611,13 +610,12 @@ func TestAPIExportPermissionClaims(t *testing.T) { t.Logf("Waiting for cowboy APIExport to have a virtual workspace URL for the bound workspaces %q", consumer1Path) consumer1VWCfg := rest.CopyConfig(cfg) kcptestinghelpers.Eventually(t, func() (bool, string) { - apiExport, err := kcpClusterClient.Cluster(claimerPath).ApisV1alpha2().APIExports().Get(ctx, "today-cowboys", metav1.GetOptions{}) + apiExportEndpointSlice, err := kcpClusterClient.Cluster(claimerPath).ApisV1alpha1().APIExportEndpointSlices().Get(ctx, "today-cowboys", metav1.GetOptions{}) require.NoError(t, err) var found bool - consumer1VWCfg.Host, found, err = framework.VirtualWorkspaceURL(ctx, kcpClusterClient, consumer1, framework.ExportVirtualWorkspaceURLs(apiExport)) + consumer1VWCfg.Host, found, err = framework.VirtualWorkspaceURL(ctx, kcpClusterClient, consumer1, framework.ExportVirtualWorkspaceURLs(apiExportEndpointSlice)) require.NoError(t, err) - //nolint:staticcheck // SA1019 VirtualWorkspaces is deprecated but not removed yet - return found, fmt.Sprintf("waiting for virtual workspace URLs to be available: %v", apiExport.Status.VirtualWorkspaces) + return found, fmt.Sprintf("waiting for virtual workspace URLs to be available: %v", apiExportEndpointSlice.Status.APIExportEndpoints) }, wait.ForeverTestTimeout, time.Millisecond*100) t.Logf("Verify that the exported resource is retrievable") From fd0b98b1dcc6d594e99f93a6f55d90b12619a20c Mon Sep 17 00:00:00 2001 From: Mangirdas Judeikis Date: Tue, 27 May 2025 10:58:57 +0300 Subject: [PATCH 3/3] Add APIExport annotation to skip APIExportEndpointSlice creation --- .../apiexport/apiexport_controller_test.go | 58 +++++++++++++++++++ .../apis/apiexport/apiexport_reconcile.go | 53 +++++++++++------ sdk/apis/apis/v1alpha2/types_apiexport.go | 5 ++ 3 files changed, 97 insertions(+), 19 deletions(-) diff --git a/pkg/reconciler/apis/apiexport/apiexport_controller_test.go b/pkg/reconciler/apis/apiexport/apiexport_controller_test.go index 853c6aa6de1..d13839b1d71 100644 --- a/pkg/reconciler/apis/apiexport/apiexport_controller_test.go +++ b/pkg/reconciler/apis/apiexport/apiexport_controller_test.go @@ -31,6 +31,7 @@ import ( "github.com/kcp-dev/logicalcluster/v3" + kcpfeatures "github.com/kcp-dev/kcp/pkg/features" apisv1alpha1 "github.com/kcp-dev/kcp/sdk/apis/apis/v1alpha1" apisv1alpha2 "github.com/kcp-dev/kcp/sdk/apis/apis/v1alpha2" corev1alpha1 "github.com/kcp-dev/kcp/sdk/apis/core/v1alpha1" @@ -39,6 +40,9 @@ import ( ) func TestReconcile(t *testing.T) { + // Save original feature gate value + originalFeatureGate := kcpfeatures.DefaultFeatureGate.Enabled(kcpfeatures.EnableDeprecatedAPIExportVirtualWorkspacesUrls) + tests := map[string]struct { secretRefSet bool secretExists bool @@ -50,6 +54,7 @@ func TestReconcile(t *testing.T) { hasPreexistingVerifyFailure bool listShardsError error apiExportEndpointSliceNotFound bool + skipEndpointSliceAnnotation bool apiBindings []interface{} @@ -62,6 +67,7 @@ func TestReconcile(t *testing.T) { wantVerifyFailure bool wantIdentityValid bool wantCreateAPIExportEndpointSlice bool + wantVirtualWorkspaceURLs bool }{ "create secret when ref is nil and secret doesn't exist": { secretExists: false, @@ -133,11 +139,44 @@ func TestReconcile(t *testing.T) { wantCreateAPIExportEndpointSlice: true, wantIdentityValid: true, }, + "skip APIExportEndpointSlice creation when skip annotation is present": { + secretRefSet: true, + secretExists: true, + + wantStatusHashSet: true, + apiExportEndpointSliceNotFound: true, + wantCreateAPIExportEndpointSlice: false, + wantIdentityValid: true, + skipEndpointSliceAnnotation: true, + }, + "virtual workspace URLs when feature gate enabled": { + secretRefSet: true, + secretExists: true, + + wantStatusHashSet: true, + wantIdentityValid: true, + wantVirtualWorkspaceURLs: true, + }, + "error listing shards with feature gate enabled": { + secretRefSet: true, + secretExists: true, + + wantStatusHashSet: true, + wantIdentityValid: true, + wantVirtualWorkspaceURLs: false, + listShardsError: errors.New("foo"), + }, } for name, tc := range tests { t.Run(name, func(t *testing.T) { + // Skip virtual workspace URL tests if feature gate is not enabled + if tc.wantVirtualWorkspaceURLs && !originalFeatureGate { + t.Skip("Skipping test that requires EnableDeprecatedAPIExportVirtualWorkspacesUrls feature gate") + } + createSecretCalled := false + createEndpointSliceCalled := false expectedKey := "abc" expectedHash := fmt.Sprintf("%x", sha256.Sum256([]byte(expectedKey))) @@ -158,6 +197,7 @@ func TestReconcile(t *testing.T) { return &apisv1alpha1.APIExportEndpointSlice{}, nil }, createAPIExportEndpointSlice: func(ctx context.Context, clusterName logicalcluster.Path, apiExportEndpointSlice *apisv1alpha1.APIExportEndpointSlice) error { + createEndpointSliceCalled = true return nil }, getSecret: func(ctx context.Context, clusterName logicalcluster.Name, ns, name string) (*corev1.Secret, error) { @@ -242,6 +282,10 @@ func TestReconcile(t *testing.T) { conditions.MarkFalse(apiExport, apisv1alpha2.APIExportIdentityValid, apisv1alpha2.IdentityVerificationFailedReason, conditionsv1alpha1.ConditionSeverityError, "") } + if tc.skipEndpointSliceAnnotation { + apiExport.Annotations[apisv1alpha2.APIExportEndpointSliceSkipAnnotation] = "true" + } + err := c.reconcile(context.Background(), apiExport) if tc.wantError { require.Error(t, err, "expected an error") @@ -292,6 +336,20 @@ func TestReconcile(t *testing.T) { if tc.wantIdentityValid { requireConditionMatches(t, apiExport, conditions.TrueCondition(apisv1alpha2.APIExportIdentityValid)) } + + require.Equal(t, tc.wantCreateAPIExportEndpointSlice, createEndpointSliceCalled, "expected createEndpointSliceCalled to be %v", tc.wantCreateAPIExportEndpointSlice) + + if tc.wantVirtualWorkspaceURLs { + //nolint:staticcheck + require.NotNil(t, apiExport.Status.VirtualWorkspaces, "expected virtual workspace URLs to be set") + //nolint:staticcheck + require.Len(t, apiExport.Status.VirtualWorkspaces, 2, "expected 2 virtual workspace URLs") + require.True(t, conditions.IsTrue(apiExport, apisv1alpha2.APIExportVirtualWorkspaceURLsReady), "expected virtual workspace URLs to be ready") + } else { + //nolint:staticcheck + require.Nil(t, apiExport.Status.VirtualWorkspaces, "expected virtual workspace URLs to be nil") + require.False(t, conditions.Has(apiExport, apisv1alpha2.APIExportVirtualWorkspaceURLsReady), "expected virtual workspace URLs condition to not exist") + } }) } } diff --git a/pkg/reconciler/apis/apiexport/apiexport_reconcile.go b/pkg/reconciler/apis/apiexport/apiexport_reconcile.go index 6d63a73d63e..ef167ac7991 100644 --- a/pkg/reconciler/apis/apiexport/apiexport_reconcile.go +++ b/pkg/reconciler/apis/apiexport/apiexport_reconcile.go @@ -101,31 +101,41 @@ func (c *controller) reconcile(ctx context.Context, apiExport *apisv1alpha2.APIE } // Ensure the APIExportEndpointSlice exists - _, err := c.getAPIExportEndpointSlice(clusterName, apiExport.Name) - if err != nil { - if errors.IsNotFound(err) { - // Create the APIExportEndpointSlice - apiExportEndpointSlice := &apisv1alpha1.APIExportEndpointSlice{ - ObjectMeta: metav1.ObjectMeta{ - Name: apiExport.Name, - }, - Spec: apisv1alpha1.APIExportEndpointSliceSpec{ - APIExport: apisv1alpha1.ExportBindingReference{ + if _, ok := apiExport.Annotations[apisv1alpha2.APIExportEndpointSliceSkipAnnotation]; !ok { + _, err := c.getAPIExportEndpointSlice(clusterName, apiExport.Name) + if err != nil { + if errors.IsNotFound(err) { + // Create the APIExportEndpointSlice + apiExportEndpointSlice := &apisv1alpha1.APIExportEndpointSlice{ + ObjectMeta: metav1.ObjectMeta{ Name: apiExport.Name, - Path: clusterPath, + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: apisv1alpha1.SchemeGroupVersion.String(), + Kind: "APIExport", + Name: apiExport.Name, + UID: apiExport.UID, + }, + }, }, - }, - } - if err := c.createAPIExportEndpointSlice(ctx, clusterName.Path(), apiExportEndpointSlice); err != nil { - return fmt.Errorf("error creating APIExportEndpointSlice for APIExport %s|%s: %w", clusterName, apiExport.Name, err) + Spec: apisv1alpha1.APIExportEndpointSliceSpec{ + APIExport: apisv1alpha1.ExportBindingReference{ + Name: apiExport.Name, + Path: clusterPath, + }, + }, + } + if err := c.createAPIExportEndpointSlice(ctx, clusterName.Path(), apiExportEndpointSlice); err != nil { + return fmt.Errorf("error creating APIExportEndpointSlice for APIExport %s|%s: %w", clusterName, apiExport.Name, err) + } + } else { + return fmt.Errorf("error getting APIExportEndpointSlice for APIExport %s|%s: %w", clusterName, apiExport.Name, err) } - } else { - return fmt.Errorf("error getting APIExportEndpointSlice for APIExport %s|%s: %w", clusterName, apiExport.Name, err) } } - // Ensure the APIExportEndpointSlice has a virtual workspace URL - // TODO(mjudeikis): Remove this and move to batteries. + // Ensure the APIExport has a virtual workspace URL + // TODO(mjudeikis): Remove this once we remove feature gate. if kcpfeatures.DefaultFeatureGate.Enabled(kcpfeatures.EnableDeprecatedAPIExportVirtualWorkspacesUrls) { if err := c.updateVirtualWorkspaceURLs(ctx, apiExport); err != nil { conditions.MarkFalse( @@ -137,6 +147,11 @@ func (c *controller) reconcile(ctx context.Context, apiExport *apisv1alpha2.APIE err, ) } + } else { + // Remove the condition and status.virtualWorkspaces if the feature gate is disabled. + conditions.Delete(apiExport, apisv1alpha2.APIExportVirtualWorkspaceURLsReady) + //nolint:staticcheck + apiExport.Status.VirtualWorkspaces = nil } return nil diff --git a/sdk/apis/apis/v1alpha2/types_apiexport.go b/sdk/apis/apis/v1alpha2/types_apiexport.go index a63eb7ed955..7916f6b165b 100644 --- a/sdk/apis/apis/v1alpha2/types_apiexport.go +++ b/sdk/apis/apis/v1alpha2/types_apiexport.go @@ -26,6 +26,11 @@ import ( conditionsv1alpha1 "github.com/kcp-dev/kcp/sdk/apis/third_party/conditions/apis/conditions/v1alpha1" ) +const ( + // APIExportEndpointSliceSkipAnnotation is an annotation that can be set on an APIExport to skip the creation of default APIExportEndpointSlice. + APIExportEndpointSliceSkipAnnotation = "apiexports.apis.kcp.io/skip-endpointslice" +) + // These are valid conditions of APIExport. const ( APIExportIdentityValid conditionsv1alpha1.ConditionType = "IdentityValid"