Skip to content

Commit

Permalink
enable full namespaceSelector usage and webhook validation
Browse files Browse the repository at this point in the history
  • Loading branch information
@killianmuldoon
killianmuldoon committed Jun 10, 2022
1 parent e498f3d commit 9568a6c
Show file tree
Hide file tree
Showing 5 changed files with 227 additions and 99 deletions.
45 changes: 32 additions & 13 deletions internal/runtime/client/client.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ import (
"time"

"github.com/pkg/errors"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/labels"
"k8s.io/apimachinery/pkg/runtime"
Expand All @@ -40,6 +39,7 @@ import (
"k8s.io/apimachinery/pkg/util/validation"
"k8s.io/client-go/transport"
"k8s.io/utils/pointer"
ctrlclient "sigs.k8s.io/controller-runtime/pkg/client"

runtimev1 "sigs.k8s.io/cluster-api/exp/runtime/api/v1alpha1"
runtimehooksv1 "sigs.k8s.io/cluster-api/exp/runtime/hooks/api/v1alpha1"
Expand All @@ -53,15 +53,17 @@ const defaultDiscoveryTimeout = 10 * time.Second

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

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

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

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

func (c *client) WarmUp(extensionConfigList *runtimev1.ExtensionConfigList) error {
Expand Down Expand Up @@ -293,8 +296,13 @@ func (c *client) CallExtension(ctx context.Context, hook runtimecatalog.Hook, fo
return errors.Errorf("ExtensionHandler %q does not match group %s, hook %s", name, hookGVH.Group, hookGVH.Hook)
}

// Compute whether the object the call is being made for matches the namespaceSelector
namespaceLabels, 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 !registration.NamespaceSelector.Matches(labelsForNamespace(forObject.GetNamespace())) {
if !namespaceLabels {
return nil
}

Expand Down Expand Up @@ -558,9 +566,20 @@ func defaultDiscoveryResponse(discovery *runtimehooksv1.DiscoveryResponse) *runt
return discovery
}

// labelsForNamespace returns a labelSet containing the default LabelMetadataName for a namespace.
func labelsForNamespace(namespace string) labels.Labels {
return labels.Set{
corev1.LabelMetadataName: namespace,
}
// 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) {
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
}
return true, nil
}
238 changes: 153 additions & 85 deletions internal/runtime/client/client_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,14 @@ import (
"regexp"
"testing"

corev1 "k8s.io/api/core/v1"

. "github.com/onsi/gomega"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/labels"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/utils/pointer"
ctrlclient "sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/client/fake"

clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
runtimev1 "sigs.k8s.io/cluster-api/exp/runtime/api/v1alpha1"
Expand Down Expand Up @@ -509,7 +510,15 @@ func Test_defaultAndValidateDiscoveryResponse(t *testing.T) {

func TestClient_CallExtension(t *testing.T) {
testHostPort := "127.0.0.1:9090"

ns := &corev1.Namespace{
TypeMeta: metav1.TypeMeta{
Kind: "Namespace",
APIVersion: corev1.SchemeGroupVersion.String(),
},
ObjectMeta: metav1.ObjectMeta{
Name: "foo",
},
}
fpFail := runtimev1.FailurePolicyFail
fpIgnore := runtimev1.FailurePolicyIgnore

Expand Down Expand Up @@ -554,55 +563,6 @@ func TestClient_CallExtension(t *testing.T) {
},
},
}

correctNamespaceSelector := runtimev1.ExtensionConfig{
Spec: runtimev1.ExtensionConfigSpec{
ClientConfig: runtimev1.ClientConfig{
URL: pointer.String(fmt.Sprintf("http://%s/", testHostPort)),
},
NamespaceSelector: &metav1.LabelSelector{
MatchLabels: map[string]string{
corev1.LabelMetadataName: "foo",
},
}},
Status: runtimev1.ExtensionConfigStatus{
Handlers: []runtimev1.ExtensionHandler{
{
Name: "valid-extension",
RequestHook: runtimev1.GroupVersionHook{
APIVersion: fakev1alpha1.GroupVersion.String(),
Hook: "FakeHook",
},
TimeoutSeconds: pointer.Int32Ptr(1),
FailurePolicy: &fpIgnore,
},
},
},
}
incorrectNamespaceSelector := runtimev1.ExtensionConfig{
Spec: runtimev1.ExtensionConfigSpec{
ClientConfig: runtimev1.ClientConfig{
URL: pointer.String(fmt.Sprintf("http://%s/", testHostPort)),
},
NamespaceSelector: &metav1.LabelSelector{
MatchLabels: map[string]string{
corev1.LabelMetadataName: "bar",
},
}},
Status: runtimev1.ExtensionConfigStatus{
Handlers: []runtimev1.ExtensionHandler{
{
Name: "valid-extension",
RequestHook: runtimev1.GroupVersionHook{
APIVersion: fakev1alpha1.GroupVersion.String(),
Hook: "FakeHook",
},
TimeoutSeconds: pointer.Int32Ptr(1),
FailurePolicy: &fpIgnore,
},
},
},
}
type args struct {
hook runtimecatalog.Hook
name string
Expand Down Expand Up @@ -752,32 +712,6 @@ func TestClient_CallExtension(t *testing.T) {
},
wantErr: true,
},
{
name: "should succeed with correct NamespaceSelector",
registeredExtensionConfigs: []runtimev1.ExtensionConfig{correctNamespaceSelector},
testServer: testServerConfig{
start: false,
}, args: args{
hook: fakev1alpha1.FakeHook,
name: "valid-extension",
request: &fakev1alpha1.FakeRequest{},
response: &fakev1alpha1.FakeResponse{},
},
wantErr: false,
},
{
name: "should succeed with incorrect NamespaceSelector",
registeredExtensionConfigs: []runtimev1.ExtensionConfig{incorrectNamespaceSelector},
testServer: testServerConfig{
start: false,
}, args: args{
hook: fakev1alpha1.FakeHook,
name: "valid-extension",
request: &fakev1alpha1.FakeRequest{},
response: &fakev1alpha1.FakeResponse{},
},
wantErr: false,
},
}

for _, tt := range tests {
Expand All @@ -796,10 +730,14 @@ func TestClient_CallExtension(t *testing.T) {
cat := runtimecatalog.New()
_ = fakev1alpha1.AddToCatalog(cat)
_ = fakev1alpha2.AddToCatalog(cat)
fakeClient := fake.NewClientBuilder().
WithObjects(ns).
Build()

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

obj := &clusterv1.Cluster{
Expand All @@ -821,7 +759,15 @@ func TestClient_CallExtension(t *testing.T) {

func TestClient_CallAllExtensions(t *testing.T) {
testHostPort := "127.0.0.1:9090"

ns := &corev1.Namespace{
TypeMeta: metav1.TypeMeta{
Kind: "Namespace",
APIVersion: corev1.SchemeGroupVersion.String(),
},
ObjectMeta: metav1.ObjectMeta{
Name: "foo",
},
}
fpFail := runtimev1.FailurePolicyFail

extensionConfig := runtimev1.ExtensionConfig{
Expand Down Expand Up @@ -987,13 +933,21 @@ func TestClient_CallAllExtensions(t *testing.T) {
cat := runtimecatalog.New()
_ = fakev1alpha1.AddToCatalog(cat)
_ = fakev1alpha2.AddToCatalog(cat)

fakeClient := fake.NewClientBuilder().
WithObjects(ns).
Build()
c := New(Options{
Catalog: cat,
Registry: registry(tt.registeredExtensionConfigs),
Catalog: cat,
Registry: registry(tt.registeredExtensionConfigs),
Kubeclient: fakeClient,
})

obj := &clusterv1.Cluster{}
obj := &clusterv1.Cluster{
ObjectMeta: metav1.ObjectMeta{
Name: "cluster",
Namespace: "foo",
},
}
err := c.CallAllExtensions(context.Background(), tt.args.hook, obj, tt.args.request, tt.args.response)

if tt.wantErr {
Expand All @@ -1005,6 +959,120 @@ func TestClient_CallAllExtensions(t *testing.T) {
}
}

func Test_client_matchNamespace(t *testing.T) {
g := NewWithT(t)
foo := &corev1.Namespace{
TypeMeta: metav1.TypeMeta{
Kind: "Namespace",
APIVersion: corev1.SchemeGroupVersion.String(),
},
ObjectMeta: metav1.ObjectMeta{
Name: "foo",
Labels: map[string]string{
corev1.LabelMetadataName: "foo",
},
},
}
bar := &corev1.Namespace{
TypeMeta: metav1.TypeMeta{
Kind: "Namespace",
APIVersion: corev1.SchemeGroupVersion.String(),
},
ObjectMeta: metav1.ObjectMeta{
Name: "bar",
Labels: map[string]string{
corev1.LabelMetadataName: "bar",
},
},
}
matchingMatchExpressions, err := metav1.LabelSelectorAsSelector(&metav1.LabelSelector{
MatchExpressions: []metav1.LabelSelectorRequirement{
{
Key: corev1.LabelMetadataName,
Operator: metav1.LabelSelectorOpIn,
Values: []string{"foo", "bar"},
},
},
})
g.Expect(err).To(BeNil())
notMatchingMatchExpressions, err := metav1.LabelSelectorAsSelector(&metav1.LabelSelector{
MatchExpressions: []metav1.LabelSelectorRequirement{
{
Key: corev1.LabelMetadataName,
Operator: metav1.LabelSelectorOpIn,
Values: []string{"non-existing", "other"},
},
},
})
g.Expect(err).To(BeNil())
tests := []struct {
name string
selector labels.Selector
namespace string
existingNamespaces []ctrlclient.Object
want bool
wantErr bool
}{
{
name: "succeed with single label selector",
selector: labels.SelectorFromSet(labels.Set{corev1.LabelMetadataName: foo.Name}),
namespace: "foo",
existingNamespaces: []ctrlclient.Object{foo, bar},
want: true,
wantErr: false,
},
{
name: "error with non-existent namespace",
selector: labels.SelectorFromSet(labels.Set{}),
namespace: "non-existent",
existingNamespaces: []ctrlclient.Object{foo, bar},
want: false,
wantErr: true,
},
{
name: "return not-found if namespaceSelector doesn't match namespace",
selector: labels.SelectorFromSet(labels.Set{corev1.LabelMetadataName: bar.Name}),
namespace: "foo",
existingNamespaces: []ctrlclient.Object{foo, bar},
want: false,
wantErr: false,
},
{
name: "return not-found if match expressions match namespace",
selector: matchingMatchExpressions,
namespace: "bar",
existingNamespaces: []ctrlclient.Object{foo, bar},
want: true,
wantErr: false,
},
{
name: "return not-found if match expressions doesn't match namespace",
selector: notMatchingMatchExpressions,
namespace: "foo",
existingNamespaces: []ctrlclient.Object{foo, bar},
want: false,
wantErr: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
c := client{
kubeClient: fake.NewClientBuilder().
WithObjects(tt.existingNamespaces...).
Build(),
}
got, err := c.matchNamespace(context.Background(), tt.selector, tt.namespace)
if (err != nil) != tt.wantErr {
t.Errorf("matchNamespace() error = %v, wantErr %v", err, tt.wantErr)
return
}
if got != tt.want {
t.Errorf("matchNamespace() got = %v, want %v", got, tt.want)
}
})
}
}

func Test_aggregateResponses(t *testing.T) {
tests := []struct {
name string
Expand Down
Loading

0 comments on commit 9568a6c

Please sign in to comment.