Skip to content

Loop over namespaces when listing reconciled objects #3251

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jan 12, 2023
Merged

Loop over namespaces when listing reconciled objects #3251

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1c18467
Loop over namespaces as well as kinds when getting
Jan 10, 2023
adaa1d3
Cleanup whitespace
Jan 10, 2023
3f9ddbb
add integration test for GetReconciledObjects
makkes Jan 12, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions core/server/events_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
. "github.com/onsi/gomega"
pb "github.com/weaveworks/weave-gitops/pkg/api/core"
"github.com/weaveworks/weave-gitops/pkg/kube"
"google.golang.org/grpc/metadata"
corev1 "k8s.io/api/core/v1"
v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/rand"
Expand Down Expand Up @@ -88,8 +89,11 @@ func TestListEvents(t *testing.T) {
g.Expect(k.Create(ctx, helmEvent)).To(Succeed())
g.Expect(k.Create(ctx, otherEvent)).To(Succeed())

md := metadata.Pairs(MetadataUserKey, "anne", MetadataGroupsKey, "system:masters")
outgoingCtx := metadata.NewOutgoingContext(ctx, md)

// Get kustomization events
res, err := c.ListEvents(ctx, &pb.ListEventsRequest{
res, err := c.ListEvents(outgoingCtx, &pb.ListEventsRequest{
InvolvedObject: &pb.ObjectRef{
Name: kustomizationObjectName,
Namespace: ns.Name,
Expand All @@ -103,7 +107,7 @@ func TestListEvents(t *testing.T) {
g.Expect(res.Events[0].Component).To(Equal(kustomizeEvent.Source.Component))

// Get kustomization events, explicit cluster
res, err = c.ListEvents(ctx, &pb.ListEventsRequest{
res, err = c.ListEvents(outgoingCtx, &pb.ListEventsRequest{
InvolvedObject: &pb.ObjectRef{
Name: kustomizationObjectName,
Namespace: ns.Name,
Expand All @@ -118,7 +122,7 @@ func TestListEvents(t *testing.T) {
g.Expect(res.Events[0].Component).To(Equal(kustomizeEvent.Source.Component))

// Get helmrelease events
res, err = c.ListEvents(ctx, &pb.ListEventsRequest{
res, err = c.ListEvents(outgoingCtx, &pb.ListEventsRequest{
InvolvedObject: &pb.ObjectRef{
Name: helmObjectName,
Namespace: ns.Name,
Expand Down
101 changes: 55 additions & 46 deletions core/server/fluxruntime.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -235,62 +235,69 @@ func (cs *coreServer) GetReconciledObjects(ctx context.Context, msg *pb.GetRecon
wg = sync.WaitGroup{}
)

for _, gvk := range msg.Kinds {
wg.Add(1)

go func(clusterName string, gvk *pb.GroupVersionKind) {
defer wg.Done()
clusterUserNamespaces := cs.clustersManager.GetUserNamespaces(auth.Principal(ctx))

listResult := unstructured.UnstructuredList{}
for _, namespaces := range clusterUserNamespaces {
for _, ns := range namespaces {
nsOpts := client.InNamespace(ns.Name)

listResult.SetGroupVersionKind(schema.GroupVersionKind{
Group: gvk.Group,
Kind: gvk.Kind,
Version: gvk.Version,
})
for _, gvk := range msg.Kinds {
wg.Add(1)

if err := clustersClient.List(ctx, msg.ClusterName, &listResult, opts); err != nil {
if k8serrors.IsForbidden(err) {
cs.logger.V(logger.LogLevelDebug).Info(
"forbidden list request",
"cluster", msg.ClusterName,
"automation", msg.AutomationName,
"namespace", msg.Namespace,
"gvk", gvk.String(),
)
// Our service account (or impersonated user) may not have the ability to see the resource in question,
// in the given namespace. We pretend it doesn't exist and keep looping.
// We need logging to make this error more visible.
return
}
go func(clusterName string, gvk *pb.GroupVersionKind) {
defer wg.Done()

if k8serrors.IsTimeout(err) {
cs.logger.Error(err, "List timedout", "gvk", gvk.String())
listResult := unstructured.UnstructuredList{}

return
}

errsMu.Lock()
errs = multierror.Append(errs, fmt.Errorf("listing unstructured object: %w", err))
errsMu.Unlock()
}

resultMu.Lock()
for _, u := range listResult.Items {
uid := u.GetUID()
listResult.SetGroupVersionKind(schema.GroupVersionKind{
Group: gvk.Group,
Kind: gvk.Kind,
Version: gvk.Version,
})

if !checkDup[uid] {
result = append(result, u)
checkDup[uid] = true
}
if err := clustersClient.List(ctx, msg.ClusterName, &listResult, opts, nsOpts); err != nil {
if k8serrors.IsForbidden(err) {
cs.logger.V(logger.LogLevelDebug).Info(
"forbidden list request",
"cluster", msg.ClusterName,
"automation", msg.AutomationName,
"namespace", msg.Namespace,
"gvk", gvk.String(),
)
// Our service account (or impersonated user) may not have the ability to see the resource in question,
// in the given namespace. We pretend it doesn't exist and keep looping.
// We need logging to make this error more visible.
return
}

if k8serrors.IsTimeout(err) {
cs.logger.Error(err, "List timedout", "gvk", gvk.String())

return
}

errsMu.Lock()
errs = multierror.Append(errs, fmt.Errorf("listing unstructured object: %w", err))
errsMu.Unlock()
}

resultMu.Lock()
for _, u := range listResult.Items {
uid := u.GetUID()

if !checkDup[uid] {
result = append(result, u)
checkDup[uid] = true
}
}
resultMu.Unlock()
}(msg.ClusterName, gvk)
}
resultMu.Unlock()
}(msg.ClusterName, gvk)
}
}

wg.Wait()

clusterUserNamespaces := cs.clustersManager.GetUserNamespaces(auth.Principal(ctx))
objects := []*pb.Object{}
respErrors := multierror.Error{}

Expand Down Expand Up @@ -327,6 +334,8 @@ func (cs *coreServer) GetChildObjects(ctx context.Context, msg *pb.GetChildObjec
return nil, fmt.Errorf("error getting impersonating client: %w", err)
}

opts := client.InNamespace(msg.Namespace)

listResult := unstructured.UnstructuredList{}

listResult.SetGroupVersionKind(schema.GroupVersionKind{
Expand All @@ -335,7 +344,7 @@ func (cs *coreServer) GetChildObjects(ctx context.Context, msg *pb.GetChildObjec
Kind: msg.GroupVersionKind.Kind,
})

if err := clustersClient.List(ctx, msg.ClusterName, &listResult); err != nil {
if err := clustersClient.List(ctx, msg.ClusterName, &listResult, opts); err != nil {
return nil, fmt.Errorf("could not get unstructured object: %s", err)
}

Expand Down
178 changes: 142 additions & 36 deletions core/server/fluxruntime_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package server_test

import (
"context"
"encoding/json"
"fmt"
"testing"

Expand All @@ -12,8 +13,10 @@ import (
stypes "github.com/weaveworks/weave-gitops/core/server/types"
pb "github.com/weaveworks/weave-gitops/pkg/api/core"
"github.com/weaveworks/weave-gitops/pkg/kube"
"google.golang.org/grpc/metadata"
appsv1 "k8s.io/api/apps/v1"
corev1 "k8s.io/api/core/v1"
rbacv1 "k8s.io/api/rbac/v1"
apiextensions "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
Expand All @@ -37,53 +40,154 @@ func TestGetReconciledObjects(t *testing.T) {
g.Expect(err).NotTo(HaveOccurred())

automationName := "my-automation"
ns := newNamespace(ctx, k, g)
ns1 := newNamespace(ctx, k, g)
ns2 := newNamespace(ctx, k, g)

reconciledObjs := []client.Object{
&appsv1.Deployment{
ObjectMeta: metav1.ObjectMeta{
Name: "my-deployment",
Namespace: ns1.Name,
Labels: map[string]string{
server.KustomizeNameKey: automationName,
server.KustomizeNamespaceKey: ns1.Name,
},
},
Spec: appsv1.DeploymentSpec{
Selector: &metav1.LabelSelector{
MatchLabels: map[string]string{
"app": automationName,
},
},
Template: corev1.PodTemplateSpec{
ObjectMeta: metav1.ObjectMeta{
Labels: map[string]string{"app": automationName},
},
Spec: corev1.PodSpec{
Containers: []corev1.Container{{
Name: "nginx",
Image: "nginx",
}},
},
},
},
},
&corev1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{
Name: "my-configmap",
Namespace: ns2.Name,
Labels: map[string]string{
server.KustomizeNameKey: automationName,
server.KustomizeNamespaceKey: ns1.Name,
},
},
},
}

reconciledObj := appsv1.Deployment{
for _, obj := range reconciledObjs {
g.Expect(k.Create(ctx, obj)).Should(Succeed())
}

crb := rbacv1.RoleBinding{
ObjectMeta: metav1.ObjectMeta{
Name: "my-deployment",
Namespace: ns.Name,
Labels: map[string]string{
server.KustomizeNameKey: automationName,
server.KustomizeNamespaceKey: ns.Name,
},
Namespace: ns1.Name,
Name: "ns-admin",
},
Spec: appsv1.DeploymentSpec{
Selector: &metav1.LabelSelector{
MatchLabels: map[string]string{
"app": automationName,
RoleRef: rbacv1.RoleRef{
APIGroup: rbacv1.SchemeGroupVersion.Group,
Kind: "ClusterRole",
Name: "cluster-admin",
},
Subjects: []rbacv1.Subject{{
APIGroup: rbacv1.SchemeGroupVersion.Group,
Kind: rbacv1.UserKind,
Name: "ns-admin",
}},
}
g.Expect(k.Create(ctx, &crb)).Should((Succeed()))

type objectAssertion struct {
kind string
name string
}

tests := []struct {
name string
user string
group string
expectedLen int
expectedObjects []objectAssertion
}{
{
name: "unknown user doesn't receive any objects",
user: "anne",
expectedLen: 0,
},
{
name: "ns-admin sees only objects in their namespace",
user: "ns-admin",
expectedLen: 1,
expectedObjects: []objectAssertion{
{
kind: "Deployment",
name: reconciledObjs[0].GetName(),
},
},
Template: corev1.PodTemplateSpec{
ObjectMeta: metav1.ObjectMeta{
Labels: map[string]string{"app": automationName},
},
{
name: "master user receives all objects",
user: "anne",
group: "system:masters",
expectedLen: 2,
expectedObjects: []objectAssertion{
{
kind: "Deployment",
name: reconciledObjs[0].GetName(),
},
Spec: corev1.PodSpec{
Containers: []corev1.Container{{
Name: "nginx",
Image: "nginx",
}},
{
kind: "ConfigMap",
name: reconciledObjs[1].GetName(),
},
},
},
}

g.Expect(k.Create(ctx, &reconciledObj)).Should(Succeed())

res, err := c.GetReconciledObjects(ctx, &pb.GetReconciledObjectsRequest{
AutomationName: automationName,
Namespace: ns.Name,
AutomationKind: kustomizev1.KustomizationKind,
Kinds: []*pb.GroupVersionKind{{Group: "apps", Version: "v1", Kind: "Deployment"}},
ClusterName: cluster.DefaultCluster,
})

g.Expect(err).NotTo(HaveOccurred())
g.Expect(res.Objects).To(HaveLen(1))
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
g = NewGomegaWithT(t)

md := metadata.Pairs(MetadataUserKey, tt.user, MetadataGroupsKey, tt.group)
outgoingCtx := metadata.NewOutgoingContext(ctx, md)
res, err := c.GetReconciledObjects(outgoingCtx, &pb.GetReconciledObjectsRequest{
AutomationName: automationName,
Namespace: ns1.Name,
AutomationKind: kustomizev1.KustomizationKind,
Kinds: []*pb.GroupVersionKind{
{Group: appsv1.SchemeGroupVersion.Group, Version: appsv1.SchemeGroupVersion.Version, Kind: "Deployment"},
{Group: corev1.SchemeGroupVersion.Group, Version: corev1.SchemeGroupVersion.Version, Kind: "ConfigMap"},
},
ClusterName: cluster.DefaultCluster,
})

first := res.Objects[0]
g.Expect(first.Payload).To(ContainSubstring("Deployment"))
g.Expect(first.Payload).To(ContainSubstring(reconciledObj.Name))
g.Expect(err).NotTo(HaveOccurred())
g.Expect(res.Objects).To(HaveLen(tt.expectedLen), "unexpected size of returned object list")

actualObjs := make([]objectAssertion, len(res.Objects))

for idx, actualObj := range res.Objects {
var object map[string]interface{}

g.Expect(json.Unmarshal([]byte(actualObj.Payload), &object)).To(Succeed(), "failed unmarshalling result object")
metadata, ok := object["metadata"].(map[string]interface{})
g.Expect(ok).To(BeTrue(), "object has unexpected metadata type")
actualObjs[idx] = objectAssertion{
kind: object["kind"].(string),
name: metadata["name"].(string),
}
}
g.Expect(actualObjs).To(ContainElements(tt.expectedObjects))
})
}
}

func TestGetReconciledObjectsWithSecret(t *testing.T) {
Expand Down Expand Up @@ -120,7 +224,9 @@ func TestGetReconciledObjectsWithSecret(t *testing.T) {

g.Expect(k.Create(ctx, &reconciledObj)).Should(Succeed())

res, err := c.GetReconciledObjects(ctx, &pb.GetReconciledObjectsRequest{
md := metadata.Pairs(MetadataUserKey, "anne", MetadataGroupsKey, "system:masters")
outgoingCtx := metadata.NewOutgoingContext(ctx, md)
res, err := c.GetReconciledObjects(outgoingCtx, &pb.GetReconciledObjectsRequest{
AutomationName: automationName,
Namespace: ns.Name,
AutomationKind: kustomizev1.KustomizationKind,
Expand Down
Loading