Skip to content

Commit

Permalink
Ensure service entries cannot select across namespaces
Browse files Browse the repository at this point in the history 
Signed-off-by: Kevin Dorosh <kevin.dorosh@solo.io>
  • Loading branch information
Kevin Dorosh committed Jul 6, 2023
1 parent ce14466 commit eb05c10
Show file tree
Hide file tree
Showing 3 changed files with 69 additions and 27 deletions.
7 changes: 3 additions & 4 deletions pilot/pkg/serviceregistry/kube/controller/ambientindex.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import (

"google.golang.org/protobuf/proto"
v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
klabels "k8s.io/apimachinery/pkg/labels"
"k8s.io/apimachinery/pkg/types"
"k8s.io/client-go/tools/cache"
Expand Down Expand Up @@ -524,7 +523,7 @@ func (c *Controller) setupIndex() *AmbientIndexImpl {
idx.servicesMap[serviceEntryNamespacedName] = svc
}

pods := c.podsClient.List(metav1.NamespaceAll, klabels.Everything())
pods := c.podsClient.List(svc.Attributes.ServiceEntryNamespace, klabels.Everything())
wls := make(map[string]*model.WorkloadInfo, len(pods))
for _, pod := range pods {
newWl := idx.extractWorkload(pod, c)
Expand All @@ -540,7 +539,7 @@ func (c *Controller) setupIndex() *AmbientIndexImpl {
}
}

workloadEntries := c.getAllControllerWorkloadEntries()
workloadEntries := c.getControllerWorkloadEntries(svc.Attributes.ServiceEntryNamespace)
for _, w := range workloadEntries {
wl := idx.extractWorkloadEntry(w, c)
// Can be nil if the WorkloadEntry IP has not been mapped yet
Expand Down Expand Up @@ -902,7 +901,7 @@ func (c *Controller) constructWorkload(pod *v1.Pod, waypoint *workloadapi.Gatewa
for _, podIP := range pod.Status.PodIPs {
addresses = append(addresses, parseIP(podIP.IP))
}
for nsName, ports := range a.getWorkloadServices(nil, pod.Labels) {
for nsName, ports := range a.getWorkloadServices(nil, pod.GetNamespace(), pod.Labels) {
workloadServices[nsName] = ports
}

Expand Down
20 changes: 9 additions & 11 deletions pilot/pkg/serviceregistry/kube/controller/ambientindex_external.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ func (c *Controller) constructWorkloadFromWorkloadEntry(workloadEntry *v1alpha3.

// for constructing a workload from a standalone workload entry, which can be selected by many service entries
if parentServiceEntry == nil {
for nsName, ports := range a.getWorkloadServices(workloadEntry, workloadEntry.Labels) {
for nsName, ports := range a.getWorkloadServices(workloadEntry, workloadEntryNamespace, workloadEntry.Labels) {
workloadServices[nsName] = ports
}
}
Expand Down Expand Up @@ -334,15 +334,6 @@ func (c *Controller) getWorkloadEntriesInService(svc *v1.Service) []*apiv1alpha3
return workloadEntries
}

func (c *Controller) getAllControllerWorkloadEntries() []*apiv1alpha3.WorkloadEntry {
namespaces := c.namespaces.List(metav1.NamespaceAll, klabels.Everything())
var allWorkloadEntries []*apiv1alpha3.WorkloadEntry
for _, ns := range namespaces {
allWorkloadEntries = append(allWorkloadEntries, c.getControllerWorkloadEntries(ns.GetObjectMeta().GetName())...)
}
return allWorkloadEntries
}

func (c *Controller) getControllerWorkloadEntries(ns string) []*apiv1alpha3.WorkloadEntry {
var allWorkloadEntries []*apiv1alpha3.WorkloadEntry
allUnstructuredWorkloadEntries := c.configController.List(gvk.WorkloadEntry, ns)
Expand Down Expand Up @@ -394,7 +385,9 @@ func (a *AmbientIndexImpl) cleanupOldWorkloadEntriesInlinedOnServiceEntry(svc *m
}
}

func (a *AmbientIndexImpl) getWorkloadServices(workloadEntry *v1alpha3.WorkloadEntry, workloadLabels map[string]string) map[string]*workloadapi.PortList {
func (a *AmbientIndexImpl) getWorkloadServices(workloadEntry *v1alpha3.WorkloadEntry, workloadNamespace string,
workloadLabels map[string]string,
) map[string]*workloadapi.PortList {
workloadServices := map[string]*workloadapi.PortList{}
for _, svc := range a.servicesMap {

Expand All @@ -404,6 +397,11 @@ func (a *AmbientIndexImpl) getWorkloadServices(workloadEntry *v1alpha3.WorkloadE
continue
}

if svc.Attributes.ServiceEntryNamespace != workloadNamespace {
// service entry can only select workloads in the same namespace
continue
}

if svc.Attributes.ServiceEntry.WorkloadSelector == nil {
// nothing to do. we construct the ztunnel config if `endpoints` are provided in the service entry handler
continue
Expand Down
69 changes: 57 additions & 12 deletions pilot/pkg/serviceregistry/kube/controller/ambientindex_serviceentry_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -115,17 +115,17 @@ func TestAmbientIndex_ServiceEntry(t *testing.T) {

// test code path where service entry creates a workload entry via `ServiceEntry.endpoints`
// and the inlined WE has a port override
addServiceEntry("se.istio.io", []string{"240.240.23.45"}, "name1", "ns", nil)
addServiceEntry("se.istio.io", []string{"240.240.23.45"}, "name1", "ns1", nil)
assertWorkloads(t, controller, "", workloadapi.WorkloadStatus_HEALTHY, "name1")
assertEvent(t, fx, "cluster0/networking.istio.io/ServiceEntry/ns/name1/127.0.0.1", "ns/se.istio.io")
assertEvent(t, fx, "cluster0/networking.istio.io/ServiceEntry/ns1/name1/127.0.0.1", "ns1/se.istio.io")
assert.Equal(t, len(controller.ambientIndex.(*AmbientIndexImpl).byWorkloadEntry), 1)
assert.Equal(t, controller.ambientIndex.Lookup("testnetwork/127.0.0.1"), []*model.AddressInfo{{
Address: &workloadapi.Address{
Type: &workloadapi.Address_Workload{
Workload: &workloadapi.Workload{
Uid: "cluster0/networking.istio.io/ServiceEntry/ns/name1/127.0.0.1",
Uid: "cluster0/networking.istio.io/ServiceEntry/ns1/name1/127.0.0.1",
Name: "name1",
Namespace: "ns",
Namespace: "ns1",
Addresses: [][]byte{parseIP("127.0.0.1")},
Node: "",
Network: "testnetwork",
Expand All @@ -134,7 +134,7 @@ func TestAmbientIndex_ServiceEntry(t *testing.T) {
WorkloadType: workloadapi.WorkloadType_POD,
WorkloadName: "name1",
Services: map[string]*workloadapi.PortList{
"ns/se.istio.io": {
"ns1/se.istio.io": {
Ports: []*workloadapi.Port{
{
ServicePort: 80,
Expand All @@ -148,7 +148,7 @@ func TestAmbientIndex_ServiceEntry(t *testing.T) {
},
}})

deleteServiceEntry("se.istio.io", []string{"240.240.23.45"}, "name1", "ns", nil)
deleteServiceEntry("se.istio.io", []string{"240.240.23.45"}, "name1", "ns1", nil)
assert.Equal(t, len(controller.ambientIndex.(*AmbientIndexImpl).byWorkloadEntry), 0)
assert.Equal(t, controller.ambientIndex.Lookup("testnetwork/127.0.0.1"), nil)
fx.Clear()
Expand All @@ -163,10 +163,55 @@ func TestAmbientIndex_ServiceEntry(t *testing.T) {
assertWorkloads(t, controller, "", workloadapi.WorkloadStatus_HEALTHY, "pod1", "pod2", "name1")
assertEvent(t, fx, "cluster0/networking.istio.io/WorkloadEntry/ns1/name1")

addServiceEntry("se.istio.io", []string{"240.240.23.45"}, "name1", "ns", map[string]string{"app": "a"})
// a service entry should not be able to select across namespaces
addServiceEntry("mismatched.istio.io", []string{"240.240.23.45"}, "name1", "mismatched-ns", map[string]string{"app": "a"})
assertEvent(t, fx, "mismatched-ns/mismatched.istio.io")
assert.Equal(t, controller.ambientIndex.Lookup("testnetwork/140.140.0.10"), []*model.AddressInfo{{
Address: &workloadapi.Address{
Type: &workloadapi.Address_Workload{
Workload: &workloadapi.Workload{
Uid: "cluster0//Pod/ns1/pod1",
Name: "pod1",
Namespace: "ns1",
Addresses: [][]byte{parseIP("140.140.0.10")},
Node: "node1",
Network: "testnetwork",
ClusterId: "cluster0",
CanonicalName: "a",
CanonicalRevision: "latest",
ServiceAccount: "sa1",
WorkloadType: workloadapi.WorkloadType_POD,
WorkloadName: "pod1",
Services: nil, // should not be selected by the mismatched service entry
},
},
},
}})
assert.Equal(t, controller.ambientIndex.Lookup("testnetwork/240.240.34.56"), []*model.AddressInfo{{
Address: &workloadapi.Address{
Type: &workloadapi.Address_Workload{
Workload: &workloadapi.Workload{
Uid: "cluster0/networking.istio.io/WorkloadEntry/ns1/name1",
Name: "name1",
Namespace: "ns1",
Addresses: [][]byte{parseIP("240.240.34.56")},
Node: "",
Network: "testnetwork",
CanonicalName: "a",
CanonicalRevision: "latest",
ServiceAccount: "sa1",
WorkloadType: workloadapi.WorkloadType_POD,
WorkloadName: "name1",
Services: nil, // should not be selected by the mismatched service entry
},
},
},
}})

addServiceEntry("se.istio.io", []string{"240.240.23.45"}, "name1", "ns1", map[string]string{"app": "a"})
assertWorkloads(t, controller, "", workloadapi.WorkloadStatus_HEALTHY, "pod1", "pod2", "name1")
// we should see an update for the workloads selected by the service entry
assertEvent(t, fx, "cluster0//Pod/ns1/pod1", "cluster0//Pod/ns1/pod2", "cluster0/networking.istio.io/WorkloadEntry/ns1/name1", "ns/se.istio.io")
assertEvent(t, fx, "cluster0//Pod/ns1/pod1", "cluster0//Pod/ns1/pod2", "cluster0/networking.istio.io/WorkloadEntry/ns1/name1", "ns1/se.istio.io")

assert.Equal(t, controller.ambientIndex.Lookup("testnetwork/140.140.0.10"), []*model.AddressInfo{{
Address: &workloadapi.Address{
Expand All @@ -185,7 +230,7 @@ func TestAmbientIndex_ServiceEntry(t *testing.T) {
WorkloadType: workloadapi.WorkloadType_POD,
WorkloadName: "pod1",
Services: map[string]*workloadapi.PortList{
"ns/se.istio.io": {
"ns1/se.istio.io": {
Ports: []*workloadapi.Port{
{
ServicePort: 80,
Expand Down Expand Up @@ -237,7 +282,7 @@ func TestAmbientIndex_ServiceEntry(t *testing.T) {
WorkloadType: workloadapi.WorkloadType_POD,
WorkloadName: "name1",
Services: map[string]*workloadapi.PortList{
"ns/se.istio.io": {
"ns1/se.istio.io": {
Ports: []*workloadapi.Port{
{
ServicePort: 80,
Expand All @@ -251,10 +296,10 @@ func TestAmbientIndex_ServiceEntry(t *testing.T) {
},
}})

deleteServiceEntry("se.istio.io", []string{"240.240.23.45"}, "name1", "ns", map[string]string{"app": "a"})
deleteServiceEntry("se.istio.io", []string{"240.240.23.45"}, "name1", "ns1", map[string]string{"app": "a"})
assertWorkloads(t, controller, "", workloadapi.WorkloadStatus_HEALTHY, "pod1", "pod2", "name1")
// we should see an update for the workloads selected by the service entry
assertEvent(t, fx, "cluster0//Pod/ns1/pod1", "cluster0//Pod/ns1/pod2", "cluster0/networking.istio.io/WorkloadEntry/ns1/name1", "ns/se.istio.io")
assertEvent(t, fx, "cluster0//Pod/ns1/pod1", "cluster0//Pod/ns1/pod2", "cluster0/networking.istio.io/WorkloadEntry/ns1/name1", "ns1/se.istio.io")
assert.Equal(t, controller.ambientIndex.Lookup("testnetwork/140.140.0.10"), []*model.AddressInfo{{
Address: &workloadapi.Address{
Type: &workloadapi.Address_Workload{
Expand Down

0 comments on commit eb05c10

Please sign in to comment.