Skip to content

Commit

Permalink
Merge pull request #2690 from nrb/fix-apiexport-404
Browse files Browse the repository at this point in the history
🐛 Do not look up APIExports in the generic webhook
  • Loading branch information
openshift-merge-robot authored Jan 27, 2023
2 parents 7d8e616 + b2a592d commit 0944f91
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 28 deletions.
25 changes: 6 additions & 19 deletions pkg/admission/webhook/generic_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,6 @@ type WebhookDispatcher struct {
dispatcher generic.Dispatcher
hookSource ClusterAwareSource

getAPIExport func(clusterName logicalcluster.Name, name string) (*apisv1alpha1.APIExport, error)
getAPIBindings func(clusterName logicalcluster.Name) ([]*apisv1alpha1.APIBinding, error)

informersHaveSynced func() bool
Expand Down Expand Up @@ -124,9 +123,9 @@ func (p *WebhookDispatcher) Dispatch(ctx context.Context, attr admission.Attribu
var whAccessor []webhook.WebhookAccessor

// Determine the type of request, is it api binding or not.
if workspace, isAPIBinding, err := p.getAPIExportCluster(attr, lcluster); err != nil {
if workspace, err := p.getAPIExportCluster(attr, lcluster); err != nil {
return err
} else if isAPIBinding {
} else if !workspace.Empty() {
whAccessor = p.hookSource.Webhooks(workspace)
attr.SetCluster(workspace)
klog.FromContext(ctx).V(7).WithValues("cluster", workspace).Info("restricting call to api registration hooks in cluster")
Expand All @@ -139,24 +138,19 @@ func (p *WebhookDispatcher) Dispatch(ctx context.Context, attr admission.Attribu
return p.dispatcher.Dispatch(ctx, attr, o, whAccessor)
}

func (p *WebhookDispatcher) getAPIExportCluster(attr admission.Attributes, clusterName logicalcluster.Name) (logicalcluster.Name, bool, error) {
func (p *WebhookDispatcher) getAPIExportCluster(attr admission.Attributes, clusterName logicalcluster.Name) (logicalcluster.Name, error) {
objs, err := p.getAPIBindings(clusterName)
if err != nil {
return "", false, err
return "", err
}
for _, apiBinding := range objs {
for _, br := range apiBinding.Status.BoundResources {
if br.Group == attr.GetResource().Group && br.Resource == attr.GetResource().Resource {
clusterName := logicalcluster.Name(apiBinding.Status.APIExportClusterName)
export, err := p.getAPIExport(clusterName, apiBinding.Spec.Reference.Export.Name)
if err != nil {
return "", false, err
}
return logicalcluster.From(export), true, nil
return logicalcluster.Name(apiBinding.Status.APIExportClusterName), nil
}
}
}
return "", false, nil
return "", nil
}

func (p *WebhookDispatcher) SetHookSource(factory func(cluster logicalcluster.Name) generic.Source, hasSynced func() bool) {
Expand All @@ -174,13 +168,6 @@ func (p *WebhookDispatcher) SetKcpInformers(local, global kcpinformers.SharedInf
p.getAPIBindings = func(clusterName logicalcluster.Name) ([]*apisv1alpha1.APIBinding, error) {
return local.Apis().V1alpha1().APIBindings().Lister().Cluster(clusterName).List(labels.Everything())
}
p.getAPIExport = func(clusterName logicalcluster.Name, name string) (*apisv1alpha1.APIExport, error) {
export, err := local.Apis().V1alpha1().APIExports().Lister().Cluster(clusterName).Get(name)
if err != nil {
return global.Apis().V1alpha1().APIExports().Lister().Cluster(clusterName).Get(name)
}
return export, nil
}

synced := func() bool {
return local.Apis().V1alpha1().APIBindings().Informer().HasSynced() &&
Expand Down
9 changes: 0 additions & 9 deletions pkg/admission/webhook/generic_webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import (

"github.com/kcp-dev/logicalcluster/v3"

"k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime/schema"
Expand Down Expand Up @@ -267,14 +266,6 @@ func TestDispatch(t *testing.T) {
getAPIBindings: func(clusterName logicalcluster.Name) ([]*apisv1alpha1.APIBinding, error) {
return tc.apiBindings, nil
},
getAPIExport: func(clusterName logicalcluster.Name, name string) (*apisv1alpha1.APIExport, error) {
for _, export := range tc.apiExports {
if export.Name == name && clusterName == logicalcluster.Name(export.Annotations[logicalcluster.AnnotationKey]) {
return export, nil
}
}
return nil, errors.NewNotFound(apisv1alpha1.Resource("APIExport"), name)
},
}

if tc.informersHaveSynced == nil {
Expand Down
3 changes: 3 additions & 0 deletions pkg/reconciler/workload/resource/resource_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -335,6 +335,9 @@ func (c *Controller) processResource(ctx context.Context, key string) error {
return nil
}
obj, err := inf.Lister().ByCluster(lclusterName).ByNamespace(namespace).Get(name)
if errors.IsNotFound(err) {
return nil
}
if err != nil {
logger.Error(err, "error getting object from indexer")
return err
Expand Down

0 comments on commit 0944f91

Please sign in to comment.