Skip to content

Commit

Permalink
Fix bug where pod source identity could be incorrectly resolved (#165)
Browse files Browse the repository at this point in the history
  • Loading branch information
@orishoshan
orishoshan authored Dec 4, 2023
1 parent d098c45 commit fc9f1dc
Show file tree
Hide file tree
Showing 4 changed files with 354 additions and 22 deletions.
9 changes: 3 additions & 6 deletions src/mapper/pkg/resolvers/helpers.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,21 +26,18 @@ func (r *mutationResolver) discoverSrcIdentity(ctx context.Context, src model.Re
srcPod, err := r.kubeFinder.ResolveIPToPod(ctx, src.SrcIP)
if err != nil {
if errors.Is(err, kubefinder.ErrFoundMoreThanOnePod) {
logrus.WithError(err).Debugf("Ip %s belongs to more than one pod, ignoring", src.SrcIP)
return model.OtterizeServiceIdentity{}, nil
return model.OtterizeServiceIdentity{}, fmt.Errorf("IP %s belongs to more than one pod, ignoring", src.SrcIP)
}
return model.OtterizeServiceIdentity{}, fmt.Errorf("could not resolve %s to pod: %w", src.SrcIP, err)
}
if src.SrcHostname != "" && srcPod.Name != src.SrcHostname {
// This could mean a new pod is reusing the same IP
// TODO: Use the captured hostname to actually find the relevant pod (instead of the IP that might no longer exist or be reused)
logrus.Warnf("Found pod %s (by ip %s) doesn't match captured hostname %s, ignoring", srcPod.Name, src.SrcIP, src.SrcHostname)
return model.OtterizeServiceIdentity{}, nil
return model.OtterizeServiceIdentity{}, fmt.Errorf("found pod %s (by ip %s) doesn't match captured hostname %s, ignoring", srcPod.Name, src.SrcIP, src.SrcHostname)
}

if srcPod.DeletionTimestamp != nil {
logrus.Debugf("Pod %s is being deleted, ignoring", srcPod.Name)
return model.OtterizeServiceIdentity{}, nil
return model.OtterizeServiceIdentity{}, fmt.Errorf("pod %s is being deleted, ignoring", srcPod.Name)
}

srcService, err := r.serviceIdResolver.ResolvePodToServiceIdentity(ctx, srcPod)
Expand Down
339 changes: 339 additions & 0 deletions src/mapper/pkg/resolvers/resolver_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,13 @@ import (
"github.com/otterize/network-mapper/src/shared/testbase"
"github.com/sirupsen/logrus"
"github.com/stretchr/testify/suite"
"golang.org/x/exp/slices"
v1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/wait"
"net/http/httptest"
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
"testing"
"time"
)
Expand Down Expand Up @@ -151,6 +157,339 @@ func (s *ResolverTestSuite) TestReportCaptureResults() {
})
}

func (s *ResolverTestSuite) TestReportCaptureResultsHostnameMismatch() {
s.AddDeploymentWithService("service1", []string{"1.1.1.1"}, map[string]string{"app": "service1"}, "10.0.0.16")
s.AddDeploymentWithService("service2", []string{"1.1.1.2"}, map[string]string{"app": "service2"}, "10.0.0.17")
s.AddDaemonSetWithService("service3", []string{"1.1.1.3"}, map[string]string{"app": "service3"}, "10.0.0.18")
s.AddPod("pod4", "1.1.1.4", nil, nil)
s.Require().True(s.Mgr.GetCache().WaitForCacheSync(context.Background()))

packetTime := time.Now().Add(time.Minute)
_, err := test_gql_client.ReportCaptureResults(context.Background(), s.client, test_gql_client.CaptureResults{
Results: []test_gql_client.RecordedDestinationsForSrc{
{
SrcIp: "1.1.1.1",
Destinations: []test_gql_client.Destination{
{
Destination: fmt.Sprintf("svc-service2.%s.svc.cluster.local", s.TestNamespace),
LastSeen: packetTime,
},
},
},
{
SrcIp: "1.1.1.3",
Destinations: []test_gql_client.Destination{
{
Destination: fmt.Sprintf("svc-service1.%s.svc.cluster.local", s.TestNamespace),
LastSeen: packetTime,
},
{
Destination: fmt.Sprintf("svc-service2.%s.svc.cluster.local", s.TestNamespace),
LastSeen: packetTime,
},
},
},
// should be discarded - hostname mismatch
{
SrcIp: "1.1.1.4",
SrcHostname: "pod5",
Destinations: []test_gql_client.Destination{
{
Destination: fmt.Sprintf("svc-service2.%s.svc.cluster.local", s.TestNamespace),
LastSeen: packetTime,
},
},
},
},
})
s.Require().NoError(err)

res, err := test_gql_client.ServiceIntents(context.Background(), s.client, nil)
s.Require().NoError(err)
s.Require().ElementsMatch(res.ServiceIntents, []test_gql_client.ServiceIntentsServiceIntents{
{
Client: test_gql_client.ServiceIntentsServiceIntentsClientOtterizeServiceIdentity{
Name: fmt.Sprintf("deployment-%s", "service1"),
Namespace: s.TestNamespace,
PodOwnerKind: test_gql_client.ServiceIntentsServiceIntentsClientOtterizeServiceIdentityPodOwnerKindGroupVersionKind{
Group: "apps",
Kind: "Deployment",
Version: "v1",
},
},
Intents: []test_gql_client.ServiceIntentsServiceIntentsIntentsOtterizeServiceIdentity{
{
Name: fmt.Sprintf("deployment-%s", "service2"),
Namespace: s.TestNamespace,
KubernetesService: "svc-service2",
},
},
},
{
Client: test_gql_client.ServiceIntentsServiceIntentsClientOtterizeServiceIdentity{
Name: fmt.Sprintf("daemonset-%s", "service3"),
Namespace: s.TestNamespace,
PodOwnerKind: test_gql_client.ServiceIntentsServiceIntentsClientOtterizeServiceIdentityPodOwnerKindGroupVersionKind{
Group: "apps",
Kind: "DaemonSet",
Version: "v1",
},
},
Intents: []test_gql_client.ServiceIntentsServiceIntentsIntentsOtterizeServiceIdentity{
{
Name: fmt.Sprintf("deployment-%s", "service1"),
Namespace: s.TestNamespace,
KubernetesService: "svc-service1",
},
{
Name: fmt.Sprintf("deployment-%s", "service2"),
Namespace: s.TestNamespace,
KubernetesService: "svc-service2",
},
},
},
})
}

func (s *ResolverTestSuite) TestReportCaptureResultsPodDeletion() {
s.AddDeploymentWithService("service1", []string{"1.1.1.1"}, map[string]string{"app": "service1"}, "10.0.0.16")
s.AddDeploymentWithService("service2", []string{"1.1.1.2"}, map[string]string{"app": "service2"}, "10.0.0.17")
s.AddDaemonSetWithService("service3", []string{"1.1.1.3"}, map[string]string{"app": "service3"}, "10.0.0.18")
pod := s.AddPod("pod4", "1.1.1.4", nil, nil)
var podToUpdate v1.Pod
err := s.Mgr.GetClient().Get(context.Background(), types.NamespacedName{Name: pod.GetName(), Namespace: pod.GetNamespace()}, &podToUpdate)
s.Require().NoError(err)
s.Require().True(controllerutil.AddFinalizer(&podToUpdate, "intents.otterize.com/finalizer-so-that-object-cant-be-deleted-for-this-test"))
err = s.Mgr.GetClient().Update(context.Background(), &podToUpdate)
s.Require().NoError(err)
s.Require().NoError(wait.PollImmediate(1*time.Second, 10*time.Second, func() (done bool, err error) {
var readPod v1.Pod
err = s.Mgr.GetClient().Get(context.Background(), types.NamespacedName{Name: pod.GetName(), Namespace: pod.GetNamespace()}, &readPod)
if errors.IsNotFound(err) {
return false, nil
}
if err != nil {
return false, err
}

if !slices.Contains(readPod.Finalizers, "intents.otterize.com/finalizer-so-that-object-cant-be-deleted-for-this-test") {
return false, nil
}
return true, nil
}))

err = s.Mgr.GetClient().Delete(context.Background(), pod)
s.Require().NoError(err)

packetTime := time.Now().Add(time.Minute)
_, err = test_gql_client.ReportCaptureResults(context.Background(), s.client, test_gql_client.CaptureResults{
Results: []test_gql_client.RecordedDestinationsForSrc{
{
SrcIp: "1.1.1.1",
Destinations: []test_gql_client.Destination{
{
Destination: fmt.Sprintf("svc-service2.%s.svc.cluster.local", s.TestNamespace),
LastSeen: packetTime,
},
},
},
{
SrcIp: "1.1.1.3",
Destinations: []test_gql_client.Destination{
{
Destination: fmt.Sprintf("svc-service1.%s.svc.cluster.local", s.TestNamespace),
LastSeen: packetTime,
},
{
Destination: fmt.Sprintf("svc-service2.%s.svc.cluster.local", s.TestNamespace),
LastSeen: packetTime,
},
},
},
// should be discarded - deleted pod
{
SrcIp: "1.1.1.4",
Destinations: []test_gql_client.Destination{
{
Destination: fmt.Sprintf("svc-service2.%s.svc.cluster.local", s.TestNamespace),
LastSeen: packetTime,
},
},
},
},
})
s.Require().NoError(err)

res, err := test_gql_client.ServiceIntents(context.Background(), s.client, nil)
s.Require().NoError(err)
s.Require().ElementsMatch(res.ServiceIntents, []test_gql_client.ServiceIntentsServiceIntents{
{
Client: test_gql_client.ServiceIntentsServiceIntentsClientOtterizeServiceIdentity{
Name: fmt.Sprintf("deployment-%s", "service1"),
Namespace: s.TestNamespace,
PodOwnerKind: test_gql_client.ServiceIntentsServiceIntentsClientOtterizeServiceIdentityPodOwnerKindGroupVersionKind{
Group: "apps",
Kind: "Deployment",
Version: "v1",
},
},
Intents: []test_gql_client.ServiceIntentsServiceIntentsIntentsOtterizeServiceIdentity{
{
Name: fmt.Sprintf("deployment-%s", "service2"),
Namespace: s.TestNamespace,
KubernetesService: "svc-service2",
},
},
},
{
Client: test_gql_client.ServiceIntentsServiceIntentsClientOtterizeServiceIdentity{
Name: fmt.Sprintf("daemonset-%s", "service3"),
Namespace: s.TestNamespace,
PodOwnerKind: test_gql_client.ServiceIntentsServiceIntentsClientOtterizeServiceIdentityPodOwnerKindGroupVersionKind{
Group: "apps",
Kind: "DaemonSet",
Version: "v1",
},
},
Intents: []test_gql_client.ServiceIntentsServiceIntentsIntentsOtterizeServiceIdentity{
{
Name: fmt.Sprintf("deployment-%s", "service1"),
Namespace: s.TestNamespace,
KubernetesService: "svc-service1",
},
{
Name: fmt.Sprintf("deployment-%s", "service2"),
Namespace: s.TestNamespace,
KubernetesService: "svc-service2",
},
},
},
})
}

func (s *ResolverTestSuite) TestReportCaptureResultsIPReuse() {
s.AddDeploymentWithService("service1", []string{"1.1.1.1"}, map[string]string{"app": "service1"}, "10.0.0.16")
s.AddDeploymentWithService("service2", []string{"1.1.1.2"}, map[string]string{"app": "service2"}, "10.0.0.17")
s.AddDaemonSetWithService("service3", []string{"1.1.1.3"}, map[string]string{"app": "service3"}, "10.0.0.18")
s.AddPod("pod4", "1.1.1.4", nil, nil)
// intentionally reusing Pod IP
s.AddDaemonSetWithService("network-sniffer", []string{"1.1.1.5"}, map[string]string{"app": "network-sniffer"}, "10.0.0.19")
s.AddDaemonSetWithService("network-sniffer-2", []string{"1.1.1.5"}, map[string]string{"app": "network-sniffer"}, "10.0.0.20")
s.Require().True(s.Mgr.GetCache().WaitForCacheSync(context.Background()))

packetTime := time.Now().Add(time.Minute)
_, err := test_gql_client.ReportCaptureResults(context.Background(), s.client, test_gql_client.CaptureResults{
Results: []test_gql_client.RecordedDestinationsForSrc{
{
SrcIp: "1.1.1.1",
Destinations: []test_gql_client.Destination{
{
Destination: fmt.Sprintf("svc-service2.%s.svc.cluster.local", s.TestNamespace),
LastSeen: packetTime,
},
},
},
{
SrcIp: "1.1.1.3",
Destinations: []test_gql_client.Destination{
{
Destination: fmt.Sprintf("svc-service1.%s.svc.cluster.local", s.TestNamespace),
LastSeen: packetTime,
},
{
Destination: fmt.Sprintf("svc-service2.%s.svc.cluster.local", s.TestNamespace),
LastSeen: packetTime,
},
},
},
{
SrcIp: "1.1.1.4",
Destinations: []test_gql_client.Destination{
{
Destination: fmt.Sprintf("svc-service2.%s.svc.cluster.local", s.TestNamespace),
LastSeen: packetTime,
},
},
},
// should be discarded - IP belongs to more than one pod
{
SrcIp: "1.1.1.5",
Destinations: []test_gql_client.Destination{
{
Destination: fmt.Sprintf("svc-service2.%s.svc.cluster.local", s.TestNamespace),
LastSeen: packetTime,
},
},
},
},
})
s.Require().NoError(err)

res, err := test_gql_client.ServiceIntents(context.Background(), s.client, nil)
s.Require().NoError(err)
s.Require().ElementsMatch(res.ServiceIntents, []test_gql_client.ServiceIntentsServiceIntents{
{
Client: test_gql_client.ServiceIntentsServiceIntentsClientOtterizeServiceIdentity{
Name: fmt.Sprintf("deployment-%s", "service1"),
Namespace: s.TestNamespace,
PodOwnerKind: test_gql_client.ServiceIntentsServiceIntentsClientOtterizeServiceIdentityPodOwnerKindGroupVersionKind{
Group: "apps",
Kind: "Deployment",
Version: "v1",
},
},
Intents: []test_gql_client.ServiceIntentsServiceIntentsIntentsOtterizeServiceIdentity{
{
Name: fmt.Sprintf("deployment-%s", "service2"),
Namespace: s.TestNamespace,
KubernetesService: "svc-service2",
},
},
},
{
Client: test_gql_client.ServiceIntentsServiceIntentsClientOtterizeServiceIdentity{
Name: fmt.Sprintf("daemonset-%s", "service3"),
Namespace: s.TestNamespace,
PodOwnerKind: test_gql_client.ServiceIntentsServiceIntentsClientOtterizeServiceIdentityPodOwnerKindGroupVersionKind{
Group: "apps",
Kind: "DaemonSet",
Version: "v1",
},
},
Intents: []test_gql_client.ServiceIntentsServiceIntentsIntentsOtterizeServiceIdentity{
{
Name: fmt.Sprintf("deployment-%s", "service1"),
Namespace: s.TestNamespace,
KubernetesService: "svc-service1",
},
{
Name: fmt.Sprintf("deployment-%s", "service2"),
Namespace: s.TestNamespace,
KubernetesService: "svc-service2",
},
},
},
{
Client: test_gql_client.ServiceIntentsServiceIntentsClientOtterizeServiceIdentity{
Name: "pod4",
Namespace: s.TestNamespace,
PodOwnerKind: test_gql_client.ServiceIntentsServiceIntentsClientOtterizeServiceIdentityPodOwnerKindGroupVersionKind{
Group: "",
Kind: "Pod",
Version: "v1",
},
},
Intents: []test_gql_client.ServiceIntentsServiceIntentsIntentsOtterizeServiceIdentity{
{
Name: fmt.Sprintf("deployment-%s", "service2"),
Namespace: s.TestNamespace,
KubernetesService: "svc-service2",
},
},
},
})
}

func (s *ResolverTestSuite) TestReportCaptureResultsIgnoreOldPacket() {
s.AddDeploymentWithService("service1", []string{"1.1.1.1"}, map[string]string{"app": "service1"}, "10.0.0.19")
s.AddDeploymentWithService("service2", []string{"1.1.1.2"}, map[string]string{"app": "service2"}, "10.0.0.20")
Expand Down
Loading

0 comments on commit fc9f1dc

Please sign in to comment.