diff --git a/pkg/indexers/apibinding.go b/pkg/indexers/apibinding.go index c0fe1249438..c61dac2f42b 100644 --- a/pkg/indexers/apibinding.go +++ b/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 +} diff --git a/pkg/indexers/apiexport.go b/pkg/indexers/apiexport.go index f26e150cb12..66d2b28eee2 100644 --- a/pkg/indexers/apiexport.go +++ b/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) +} diff --git a/pkg/reconciler/apis/apiexport/apiexport_controller.go b/pkg/reconciler/apis/apiexport/apiexport_controller.go index 61796bed119..537071495ee 100644 --- a/pkg/reconciler/apis/apiexport/apiexport_controller.go +++ b/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() diff --git a/pkg/reconciler/apis/apiexport/apiexport_controller_test.go b/pkg/reconciler/apis/apiexport/apiexport_controller_test.go index 8de325d3789..bced26810c3 100644 --- a/pkg/reconciler/apis/apiexport/apiexport_controller_test.go +++ b/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 diff --git a/pkg/reconciler/apis/apiexport/apiexport_reconcile.go b/pkg/reconciler/apis/apiexport/apiexport_reconcile.go index 5bdc89b68dc..44c549a94e2 100644 --- a/pkg/reconciler/apis/apiexport/apiexport_reconcile.go +++ b/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, diff --git a/pkg/server/controllers.go b/pkg/server/controllers.go index d8f8baeff69..b9d9b4fbd05 100644 --- a/pkg/server/controllers.go +++ b/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 diff --git a/test/e2e/apibinding/apibinding_test.go b/test/e2e/apibinding/apibinding_test.go index a08eb9bd2bd..9fb691bb58b 100644 --- a/test/e2e/apibinding/apibinding_test.go +++ b/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") diff --git a/test/e2e/virtual/apiexport/virtualworkspace_test.go b/test/e2e/virtual/apiexport/virtualworkspace_test.go index 5e801f35708..aa89e71c5af 100644 --- a/test/e2e/virtual/apiexport/virtualworkspace_test.go +++ b/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)