Disable deprecated APIExport VirtualWorkspace url population

Signed-off-by: Mangirdas Judeikis <Mangirdas@Judeikis.LT>
On-behalf-of: @SAP mangirdas.judeikis@sap.com

Author: mjudeikis

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:

pkg/features/kcp_features.go

@@ -42,6 +42,12 @@ const (
 	// alpha: v0.1
 	// Enables workspace mounts via frontProxy.
 	WorkspaceMounts featuregate.Feature = "WorkspaceMounts"
+
+	// 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.
@@ -112,7 +118,9 @@ var defaultVersionedGenericControlPlaneFeatureGates = map[featuregate.Feature]fe
 	WorkspaceMounts: {
 		{Version: version.MustParse("1.28"), 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:
 

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
 

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))
-			}
 		})
 	}
 }

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,
 		})

pkg/server/controllers.go

@@ -1049,6 +1049,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(),
@@ -1067,6 +1068,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
 			})

pkg/server/server.go

@@ -432,6 +432,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())
 

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)