Skip to content

Commit

Permalink
alter error handling in CallAllExtensions and nits
Browse files Browse the repository at this point in the history
  • Loading branch information
killianmuldoon committed Jun 13, 2022
1 parent 9aef446 commit 26a84eb
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 29 deletions.
52 changes: 33 additions & 19 deletions internal/runtime/client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@ import (
"strings"
"time"

"sigs.k8s.io/cluster-api/util"
ctrlclient "sigs.k8s.io/controller-runtime/pkg/client"

"github.com/pkg/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/labels"
Expand All @@ -45,6 +48,7 @@ import (
runtimehooksv1 "sigs.k8s.io/cluster-api/exp/runtime/hooks/api/v1alpha1"
runtimecatalog "sigs.k8s.io/cluster-api/internal/runtime/catalog"
runtimeregistry "sigs.k8s.io/cluster-api/internal/runtime/registry"
"sigs.k8s.io/cluster-api/util"
)

type errCallingExtensionHandler error
Expand All @@ -53,17 +57,17 @@ const defaultDiscoveryTimeout = 10 * time.Second

// Options are creation options for a Client.
type Options struct {
Catalog *runtimecatalog.Catalog
Registry runtimeregistry.ExtensionRegistry
Kubeclient ctrlclient.Client
Catalog *runtimecatalog.Catalog
Registry runtimeregistry.ExtensionRegistry
Client ctrlclient.Client
}

// New returns a new Client.
func New(options Options) Client {
return &client{
catalog: options.Catalog,
registry: options.Registry,
kubeClient: options.Kubeclient,
catalog: options.Catalog,
registry: options.Registry,
Client: options.Client,
}
}

Expand Down Expand Up @@ -97,9 +101,9 @@ type Client interface {
var _ Client = &client{}

type client struct {
catalog *runtimecatalog.Catalog
registry runtimeregistry.ExtensionRegistry
kubeClient ctrlclient.Client
catalog *runtimecatalog.Catalog
registry runtimeregistry.ExtensionRegistry
Client ctrlclient.Client
}

func (c *client) WarmUp(extensionConfigList *runtimev1.ExtensionConfigList) error {
Expand Down Expand Up @@ -210,6 +214,15 @@ func (c *client) CallAllExtensions(ctx context.Context, hook runtimecatalog.Hook
}
tmpResponse := responseObject.(runtimehooksv1.ResponseObject)

// Compute whether the object the call is being made for matches the namespaceSelector
namespaceMatches, err := c.matchNamespace(ctx, registration.NamespaceSelector, forObject.GetNamespace())
if err != nil {
return errors.Errorf("ExtensionHandler %q namespaceSelector could not be resolved", registration.Name)
}
// If the object namespace isn't matched by the registration NamespaceSelector skip the call.
if !namespaceMatches {
continue
}
err = c.CallExtension(ctx, hook, forObject, registration.Name, request, tmpResponse)
// If one of the extension handlers fails lets short-circuit here and return early.
if err != nil {
Expand Down Expand Up @@ -297,13 +310,13 @@ func (c *client) CallExtension(ctx context.Context, hook runtimecatalog.Hook, fo
}

// Compute whether the object the call is being made for matches the namespaceSelector
namespaceLabels, err := c.matchNamespace(ctx, registration.NamespaceSelector, forObject.GetNamespace())
namespaceMatches, err := c.matchNamespace(ctx, registration.NamespaceSelector, forObject.GetNamespace())
if err != nil {
return errors.Errorf("ExtensionHandler %q namespaceSelector could not be resolved", name)
}
// If the object namespace isn't matched by the registration NamespaceSelector return without calling.
if !namespaceLabels {
return nil
// If the object namespace isn't matched by the registration NamespaceSelector return an error.
if !namespaceMatches {
return errors.Errorf("ExtensionHandler %q namespaceSelector did not match object %s", name, util.ObjectKey(forObject))
}

var timeoutDuration time.Duration
Expand Down Expand Up @@ -569,17 +582,18 @@ func defaultDiscoveryResponse(discovery *runtimehooksv1.DiscoveryResponse) *runt
// matchNamespace returns true if the passed namespace matches the selector. It returns an error if the namespace does
// not exist in the API server.
func (c *client) matchNamespace(ctx context.Context, selector labels.Selector, namespace string) (bool, error) {
// return early if the selector is empty.
if selector.Empty() {
return true, nil
}
ns := &metav1.PartialObjectMetadata{}
ns.SetGroupVersionKind(schema.GroupVersionKind{
Group: "",
Version: "v1",
Kind: "Namespace",
})
if err := c.kubeClient.Get(ctx, ctrlclient.ObjectKey{Name: namespace}, ns); err != nil {
return false, err
}
if !selector.Matches(labels.Set(ns.GetLabels())) {
return false, nil
if err := c.Client.Get(ctx, ctrlclient.ObjectKey{Name: namespace}, ns); err != nil {
return false, errors.Wrapf(err, "failed to find namespace %s for extension namespaceSelector", namespace)
}
return true, nil
return selector.Matches(labels.Set(ns.GetLabels())), nil
}
16 changes: 8 additions & 8 deletions internal/runtime/client/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -735,9 +735,9 @@ func TestClient_CallExtension(t *testing.T) {
Build()

c := New(Options{
Catalog: cat,
Registry: registry(tt.registeredExtensionConfigs),
Kubeclient: fakeClient,
Catalog: cat,
Registry: registry(tt.registeredExtensionConfigs),
Client: fakeClient,
})

obj := &clusterv1.Cluster{
Expand Down Expand Up @@ -937,9 +937,9 @@ func TestClient_CallAllExtensions(t *testing.T) {
WithObjects(ns).
Build()
c := New(Options{
Catalog: cat,
Registry: registry(tt.registeredExtensionConfigs),
Kubeclient: fakeClient,
Catalog: cat,
Registry: registry(tt.registeredExtensionConfigs),
Client: fakeClient,
})

obj := &clusterv1.Cluster{
Expand Down Expand Up @@ -1023,7 +1023,7 @@ func Test_client_matchNamespace(t *testing.T) {
},
{
name: "error with non-existent namespace",
selector: labels.SelectorFromSet(labels.Set{}),
selector: labels.SelectorFromSet(labels.Set{corev1.LabelMetadataName: foo.Name}),
namespace: "non-existent",
existingNamespaces: []ctrlclient.Object{foo, bar},
want: false,
Expand Down Expand Up @@ -1057,7 +1057,7 @@ func Test_client_matchNamespace(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
c := client{
kubeClient: fake.NewClientBuilder().
Client: fake.NewClientBuilder().
WithObjects(tt.existingNamespaces...).
Build(),
}
Expand Down
4 changes: 2 additions & 2 deletions internal/webhooks/runtime/extensionconfig_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -214,14 +214,14 @@ func validateExtensionConfigSpec(e *runtimev1.ExtensionConfig) field.ErrorList {
}
if e.Spec.NamespaceSelector == nil {
allErrs = append(allErrs, field.Required(
specPath.Child("NamespaceSelector"),
specPath.Child("namespaceSelector"),
"must be defined",
))
}

if _, err := metav1.LabelSelectorAsSelector(e.Spec.NamespaceSelector); err != nil {
allErrs = append(allErrs, field.Invalid(
specPath.Child("NamespaceSelector"),
specPath.Child("namespaceSelector"),
e.Spec.NamespaceSelector,
err.Error(),
))
Expand Down

0 comments on commit 26a84eb

Please sign in to comment.