🐛 Create VirtualWorkSpaceURL on first APIBinding

pkg/indexers/apibinding.go

@@ -74,3 +74,15 @@ func IndexAPIBindingByBoundResources(obj interface{}) ([]string, error) {
 func APIBindingBoundResourceValue(clusterName logicalcluster.Name, group, resource string) string {
 	return fmt.Sprintf("%s|%s.%s", clusterName, resource, group)
 }
+
+const APIBindingsByAPIExport = "APIBindingByAPIExport"
+
+// IndexAPIBindingByAPIExport indexes the APIBindings by their APIExport's Reference Path and Name.
+func IndexAPIBindingByAPIExport(obj interface{}) ([]string, error) {
+	apiBinding, ok := obj.(*apisv1alpha1.APIBinding)
+	if !ok {
+		return []string{}, fmt.Errorf("obj %T is not an APIBinding", obj)
+	}
+
+	return []string{ClusterPathAndAPIExportName(apiBinding.Spec.Reference.Workspace.Path, apiBinding.Spec.Reference.Workspace.ExportName)}, nil
+}

pkg/indexers/apiexport.go

@@ -28,7 +28,7 @@ import (
 const (
 	// APIExportByIdentity is the indexer name for retrieving APIExports by identity hash.
 	APIExportByIdentity = "APIExportByIdentity"
-	// APIExportBySecret is the indexer name for retrieving APIExports by
+	// APIExportBySecret is the indexer name for retrieving APIExports by secret.
 	APIExportBySecret = "APIExportSecret"
 )
 
@@ -65,3 +65,7 @@ func IndexAPIExportBySecret(obj interface{}) ([]string, error) {
 
 	return []string{kcpcache.ToClusterAwareKey(logicalcluster.From(apiExport).String(), ref.Namespace, ref.Name)}, nil
 }
+
+func ClusterPathAndAPIExportName(clusterPath, exportName string) string {
+	return fmt.Sprintf("%s|%s", clusterPath, exportName)
+}

pkg/reconciler/apis/apiexport/apiexport_controller.go

@@ -41,6 +41,7 @@ import (
 
 	apisv1alpha1 "github.com/kcp-dev/kcp/pkg/apis/apis/v1alpha1"
 	tenancyv1alpha1 "github.com/kcp-dev/kcp/pkg/apis/tenancy/v1alpha1"
+	"github.com/kcp-dev/kcp/pkg/apis/third_party/conditions/util/conditions"
 	kcpclient "github.com/kcp-dev/kcp/pkg/client/clientset/versioned"
 	apisinformers "github.com/kcp-dev/kcp/pkg/client/informers/externalversions/apis/v1alpha1"
 	tenancyinformers "github.com/kcp-dev/kcp/pkg/client/informers/externalversions/tenancy/v1alpha1"
@@ -64,6 +65,7 @@ func NewController(
 	kubeClusterClient kubernetesclient.Interface,
 	namespaceInformer coreinformers.NamespaceInformer,
 	secretInformer coreinformers.SecretInformer,
+	apiBindingInformer apisinformers.APIBindingInformer,
 ) (*controller, error) {
 	queue := workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), controllerName)
 
@@ -86,6 +88,10 @@ func NewController(
 			_, err := kubeClusterClient.CoreV1().Secrets(secret.Namespace).Create(logicalcluster.WithCluster(ctx, clusterName), secret, metav1.CreateOptions{})
 			return err
 		},
+		getAPIBindingsForAPIExport: func(clusterName logicalcluster.Name, name string) ([]interface{}, error) {
+			clusterPathAndName := indexers.ClusterPathAndAPIExportName(clusterName.String(), name)
+			return apiBindingInformer.Informer().GetIndexer().ByIndex(indexers.APIBindingsByAPIExport, clusterPathAndName)
+		},
 		listClusterWorkspaceShards: func() ([]*tenancyv1alpha1.ClusterWorkspaceShard, error) {
 			return clusterWorkspaceShardInformer.Lister().List(labels.Everything())
 		},
@@ -102,6 +108,13 @@ func NewController(
 		},
 	)
 
+	indexers.AddIfNotPresentOrDie(
+		apiBindingInformer.Informer().GetIndexer(),
+		cache.Indexers{
+			indexers.APIBindingsByAPIExport: indexers.IndexAPIBindingByAPIExport,
+		},
+	)
+
 	apiExportInformer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{
 		AddFunc: func(obj interface{}) {
 			c.enqueueAPIExport(obj)
@@ -140,6 +153,20 @@ func NewController(
 		},
 	)
 
+	apiBindingInformer.Informer().AddEventHandler(
+		cache.ResourceEventHandlerFuncs{
+			AddFunc: func(obj interface{}) {
+				c.enqueueFromAPIBinding(obj)
+			},
+			UpdateFunc: func(_, newObj interface{}) {
+				c.enqueueFromAPIBinding(newObj)
+			},
+			DeleteFunc: func(obj interface{}) {
+				c.enqueueFromAPIBinding(obj)
+			},
+		},
+	)
+
 	return c, nil
 }
 
@@ -168,6 +195,8 @@ type controller struct {
 	getSecret    func(ctx context.Context, clusterName logicalcluster.Name, ns, name string) (*corev1.Secret, error)
 	createSecret func(ctx context.Context, clusterName logicalcluster.Name, secret *corev1.Secret) error
 
+	getAPIBindingsForAPIExport func(clustername logicalcluster.Name, name string) ([]interface{}, error)
+
 	listClusterWorkspaceShards func() ([]*tenancyv1alpha1.ClusterWorkspaceShard, error)
 	commit                     CommitFunc
 }
@@ -230,6 +259,28 @@ func (c *controller) enqueueSecret(obj interface{}) {
 	}
 }
 
+func (c *controller) enqueueFromAPIBinding(obj interface{}) {
+	binding, ok := obj.(*apisv1alpha1.APIBinding)
+	if !ok {
+		return
+	}
+
+	// Skip any bindings that haven't progressed to initially bound.
+	if !conditions.IsTrue(binding, apisv1alpha1.InitialBindingCompleted) {
+		return
+	}
+
+	logger := logging.WithObject(logging.WithReconciler(klog.Background(), controllerName), binding)
+
+	if binding.Spec.Reference.Workspace == nil {
+		return
+	}
+
+	key := kcpcache.ToClusterAwareKey(binding.Spec.Reference.Workspace.Path, "", binding.Spec.Reference.Workspace.ExportName)
+	logging.WithQueueKey(logger, key).V(2).Info("queueing APIExport via APIBinding")
+	c.queue.Add(key)
+}
+
 // Start starts the controller, which stops when ctx.Done() is closed.
 func (c *controller) Start(ctx context.Context, numThreads int) {
 	defer runtime.HandleCrash()

pkg/reconciler/apis/apiexport/apiexport_controller_test.go

@@ -48,6 +48,8 @@ func TestReconcile(t *testing.T) {
 		hasPreexistingVerifyFailure          bool
 		listClusterWorkspaceShardsError      error
 
+		apiBindings []interface{}
+
 		wantGenerationFailed          bool
 		wantError                     bool
 		wantCreateSecretCalled        bool
@@ -85,8 +87,6 @@ func TestReconcile(t *testing.T) {
 
 			wantStatusHashSet: true,
 			wantIdentityValid: true,
-
-			wantVirtualWorkspaceURLsReady: true,
 		},
 		"identity verification fails when reference secret doesn't exist": {
 			secretRefSet: true,
@@ -109,8 +109,6 @@ func TestReconcile(t *testing.T) {
 			hasPreexistingVerifyFailure: true,
 
 			wantIdentityValid: true,
-
-			wantVirtualWorkspaceURLsReady: true,
 		},
 		"error listing clusterworkspaceshards": {
 			secretRefSet: true,
@@ -119,9 +117,24 @@ func TestReconcile(t *testing.T) {
 			wantStatusHashSet: true,
 			wantIdentityValid: true,
 
+			apiBindings: []interface{}{
+				"something",
+			},
 			listClusterWorkspaceShardsError: errors.New("foo"),
 			wantVirtualWorkspaceURLsError:   true,
 		},
+		"virtualWorkspaceURLs set when APIBindings present": {
+			secretRefSet: true,
+			secretExists: true,
+
+			wantStatusHashSet: true,
+			wantIdentityValid: true,
+
+			apiBindings: []interface{}{
+				"something",
+			},
+			wantVirtualWorkspaceURLsReady: true,
+		},
 	}
 
 	for name, tc := range tests {
@@ -163,6 +176,13 @@ func TestReconcile(t *testing.T) {
 					createSecretCalled = true
 					return tc.createSecretError
 				},
+				getAPIBindingsForAPIExport: func(_ logicalcluster.Name, _ string) ([]interface{}, error) {
+					if len(tc.apiBindings) > 0 {
+						return tc.apiBindings, nil
+					}
+
+					return make([]interface{}, 0), nil
+				},
 				listClusterWorkspaceShards: func() ([]*tenancyv1alpha1.ClusterWorkspaceShard, error) {
 					if tc.listClusterWorkspaceShardsError != nil {
 						return nil, tc.listClusterWorkspaceShardsError

pkg/reconciler/apis/apiexport/apiexport_reconcile.go

@@ -95,6 +95,17 @@ func (c *controller) reconcile(ctx context.Context, apiExport *apisv1alpha1.APIE
 		)
 	}
 
+	// 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)
+	}
+
+	// If there are no bindings, then we can't create a URL yet.
+	if len(apiBindings) == 0 {
+		return nil
+	}
+
 	if err := c.updateVirtualWorkspaceURLs(ctx, apiExport); err != nil {
 		conditions.MarkFalse(
 			apiExport,

pkg/server/controllers.go

@@ -718,6 +718,7 @@ func (s *Server) installAPIExportController(ctx context.Context, config *rest.Co
 		kubeClusterClient,
 		s.KubeSharedInformerFactory.Core().V1().Namespaces(),
 		s.KubeSharedInformerFactory.Core().V1().Secrets(),
+		s.KcpSharedInformerFactory.Apis().V1alpha1().APIBindings(),
 	)
 	if err != nil {
 		return err

test/e2e/apibinding/apibinding_test.go

@@ -93,8 +93,8 @@ func TestAPIBinding(t *testing.T) {
 		return true
 	}, wait.ForeverTestTimeout, 100*time.Millisecond, "expected all ClusterWorkspaceShards to have a VirtualWorkspaceURL assigned")
 
+	exportName := "today-cowboys"
 	serviceProviderWorkspaces := []logicalcluster.Name{serviceProvider1Workspace, serviceProvider2Workspace}
-
 	for _, serviceProviderWorkspace := range serviceProviderWorkspaces {
 		t.Logf("Install today cowboys APIResourceSchema into %q", serviceProviderWorkspace)
 
@@ -105,45 +105,17 @@ func TestAPIBinding(t *testing.T) {
 		err = helpers.CreateResourceFromFS(ctx, dynamicClusterClient.Cluster(serviceProviderWorkspace), mapper, nil, "apiresourceschema_cowboys.yaml", testFiles)
 		require.NoError(t, err)
 
-		t.Logf("Create an APIExport today-cowboys in %q", serviceProviderWorkspace)
 		cowboysAPIExport := &apisv1alpha1.APIExport{
 			ObjectMeta: metav1.ObjectMeta{
-				Name: "today-cowboys",
+				Name: exportName,
 			},
 			Spec: apisv1alpha1.APIExportSpec{
 				LatestResourceSchemas: []string{"today.cowboys.wildwest.dev"},
 			},
 		}
-		cowboysAPIExport, err = kcpClusterClient.ApisV1alpha1().APIExports().Create(logicalcluster.WithCluster(ctx, serviceProviderWorkspace), cowboysAPIExport, metav1.CreateOptions{})
+		t.Logf("Create an APIExport today-cowboys in %q", serviceProviderWorkspace)
+		_, err = kcpClusterClient.ApisV1alpha1().APIExports().Create(logicalcluster.WithCluster(ctx, serviceProviderWorkspace), cowboysAPIExport, metav1.CreateOptions{})
 		require.NoError(t, err)
-
-		var expectedURLs []string
-		for _, urlString := range clusterWorkspaceShardVirtualWorkspaceURLs.List() {
-			u, err := url.Parse(urlString)
-			require.NoError(t, err, "error parsing %q", urlString)
-			u.Path = path.Join(u.Path, "services", "apiexport", serviceProviderWorkspace.String(), cowboysAPIExport.Name)
-			expectedURLs = append(expectedURLs, u.String())
-		}
-
-		t.Logf("Make sure the APIExport gets status.virtualWorkspaceURLs set")
-		framework.Eventually(t, func() (bool, string) {
-			e, err := kcpClusterClient.ApisV1alpha1().APIExports().Get(logicalcluster.WithCluster(ctx, serviceProviderWorkspace), cowboysAPIExport.Name, metav1.GetOptions{})
-			if err != nil {
-				t.Logf("Unexpected error getting APIExport %s|%s: %v", serviceProviderWorkspace, cowboysAPIExport.Name, err)
-			}
-
-			var actualURLs []string
-			for _, u := range e.Status.VirtualWorkspaces {
-				actualURLs = append(actualURLs, u.URL)
-			}
-
-			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",
-			serviceProviderWorkspace, cowboysAPIExport.Name)
 	}
 
 	bindConsumerToProvider := func(consumerWorkspace, providerWorkspace logicalcluster.Name) {
@@ -248,13 +220,45 @@ func TestAPIBinding(t *testing.T) {
 		}, wait.ForeverTestTimeout, 100*time.Millisecond, "expected naming conflict")
 	}
 
+	verifyVirtualWorkspaceURLs := func(serviceProviderWorkspace logicalcluster.Name) {
+		var expectedURLs []string
+		for _, urlString := range clusterWorkspaceShardVirtualWorkspaceURLs.List() {
+			u, err := url.Parse(urlString)
+			require.NoError(t, err, "error parsing %q", urlString)
+			u.Path = path.Join(u.Path, "services", "apiexport", serviceProviderWorkspace.String(), exportName)
+			expectedURLs = append(expectedURLs, u.String())
+		}
+
+		t.Logf("Make sure the APIExport gets status.virtualWorkspaceURLs set")
+		framework.Eventually(t, func() (bool, string) {
+			e, err := kcpClusterClient.ApisV1alpha1().APIExports().Get(logicalcluster.WithCluster(ctx, serviceProviderWorkspace), exportName, metav1.GetOptions{})
+			if err != nil {
+				t.Logf("Unexpected error getting APIExport %s|%s: %v", serviceProviderWorkspace, exportName, err)
+			}
+
+			var actualURLs []string
+			for _, u := range e.Status.VirtualWorkspaces {
+				actualURLs = append(actualURLs, u.URL)
+			}
+
+			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",
+			serviceProviderWorkspace, exportName)
+	}
+
 	consumersOfServiceProvider1 := []logicalcluster.Name{consumer1Workspace, consumer2Workspace}
 	for _, consumerWorkspace := range consumersOfServiceProvider1 {
 		bindConsumerToProvider(consumerWorkspace, serviceProvider1Workspace)
 	}
+	verifyVirtualWorkspaceURLs(serviceProvider1Workspace)
 
 	t.Logf("=== Binding %q to %q", consumer3Workspace, serviceProvider2Workspace)
 	bindConsumerToProvider(consumer3Workspace, serviceProvider2Workspace)
+	verifyVirtualWorkspaceURLs(serviceProvider2Workspace)
 
 	t.Logf("=== Testing identity wildcards")
 

test/e2e/virtual/apiexport/virtualworkspace_test.go

@@ -102,6 +102,11 @@ func TestAPIExportVirtualWorkspace(t *testing.T) {
 
 	setUpServiceProvider(ctx, dynamicClusterClient, kcpClients, serviceProviderWorkspace, cfg, t)
 
+	t.Logf("test that the virtualWorkspaceURL is not set on initial APIExport creation")
+	apiExport, err := kcpClients.ApisV1alpha1().APIExports().Get(logicalcluster.WithCluster(ctx, serviceProviderWorkspace), "today-cowboys", metav1.GetOptions{})
+	require.Empty(t, apiExport.Status.VirtualWorkspaces)
+	require.NoError(t, err, "error getting APIExport")
+
 	// create API bindings in consumerWorkspace as user-3 with only bind permissions in serviceProviderWorkspace but not general access.
 	user3KcpClient, err := kcpclientset.NewForConfig(framework.UserConfig("user-3", rest.CopyConfig(cfg)))
 	require.NoError(t, err, "failed to construct client for user-3")
@@ -127,7 +132,7 @@ func TestAPIExportVirtualWorkspace(t *testing.T) {
 	}, wait.ForeverTestTimeout, 100*time.Millisecond, "expected all ClusterWorkspaceShards to have a VirtualWorkspaceURL assigned")
 
 	t.Logf("test that the admin user can use the virtual workspace to get cowboys")
-	apiExport, err := kcpClients.ApisV1alpha1().APIExports().Get(logicalcluster.WithCluster(ctx, serviceProviderWorkspace), "today-cowboys", metav1.GetOptions{})
+	apiExport, err = kcpClients.ApisV1alpha1().APIExports().Get(logicalcluster.WithCluster(ctx, serviceProviderWorkspace), "today-cowboys", metav1.GetOptions{})
 	require.NoError(t, err, "error getting APIExport")
 	require.Len(t, apiExport.Status.VirtualWorkspaces, clusterWorkspaceShardVirtualWorkspaceURLs.Len(), "unexpected virtual workspace URLs: %#v", apiExport.Status.VirtualWorkspaces)