Skip to content

chore: fix for external domain resolution (#148) #150

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

Closed
mayankpande88 wants to merge 3 commits into from
Closed

chore: fix for external domain resolution (#148) #150

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
41c2ef3
fix: fix for external domain resolution (#148)
mayankpande88 Jul 2, 2025
6a27da6
build(deps): bump github.com/ulikunitz/xz from 0.5.12 to 0.5.14 (#152)
dependabot[bot] Sep 4, 2025
29da74f
build(deps): bump github.com/docker/docker (#151)
dependabot[bot] Sep 4, 2025
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
2 changes: 1 addition & 1 deletion .github/workflows/dev-ci.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ jobs:
- name: Build and Push Agent Image
uses: docker/build-push-action@v3
with:
platforms: linux/amd64,linux/arm64
platforms: linux/amd64
context: .
file: 'Dockerfile'
push: true
Expand Down
27 changes: 14 additions & 13 deletions common/net.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -218,26 +218,27 @@ func (d *Domain) String() string {

func NewDomain(fqdn string, ips []netaddr.IP) *Domain {
d := &Domain{FQDN: fqdn, SpecifyIP: true}
if len(ips) > 1 {
containsPrivateIPs := false
for _, ip := range ips {
if !IsIpExternal(ip) {
containsPrivateIPs = true
break
}
}
if !containsPrivateIPs {
d.SpecifyIP = false
}
}
return d
Comment on lines 219 to 221

@gemini-code-assist gemini-code-assist Bot Jul 4, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The ips parameter is no longer used within the NewDomain function. To improve code clarity and prevent potential misuse, it should be removed from the function signature.

func NewDomain(fqdn string) *Domain {
	return &Domain{FQDN: fqdn, SpecifyIP: true}
}

}

func NewDestinationKey(dst, actualDst netaddr.IPPort, domain *Domain, dstWorkload Workload, actualDestWorkload Workload) DestinationKey {
if IsIpExternal(actualDst.IP()) && domain != nil && !domain.SpecifyIP {
return DestinationKey{
destination: HostPortWithEmptyIP(domain.FQDN, dst.Port()),
destination: HostPortWithEmptyIP(domain.FQDN, dst.Port()),
actualDestination: HostPortFromIPPort(actualDst),
destinationWorkload: Workload{
Kind: "external",
Name: domain.FQDN,
Namespace: "external",
},
actualDestinationWorkload: Workload{
Kind: "external",
Name: actualDst.IP().String(),
Namespace: "external",
},
}
} else if IsIpExternal(actualDst.IP()) && domain != nil && domain.SpecifyIP {
klog.Infof("ip %q is external, but domain %q specifies IP, using IP as destination", actualDst.IP(), domain.FQDN)
}
Comment on lines +240 to 242

@gemini-code-assist gemini-code-assist Bot Jul 4, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This else if block only contains a logging statement and doesn't alter the control flow, which can be confusing. Consider removing the else if and placing the logging statement before the final return statement.

if IsIpExternal(actualDst.IP()) && domain != nil && !domain.SpecifyIP {
		return DestinationKey{
			destination:       HostPortWithEmptyIP(domain.FQDN, dst.Port()),
			actualDestination: HostPortFromIPPort(actualDst),
			destinationWorkload: Workload{
				Kind:      "external",
				Name:      domain.FQDN,
				Namespace: "external",
				},
			actualDestinationWorkload: Workload{
				Kind:      "external",
				Name:      actualDst.IP().String(),
				Namespace: "external",
				},
			}
		}
		if IsIpExternal(actualDst.IP()) && domain != nil && domain.SpecifyIP {
			klog.Infof("ip %q is external, but domain %q specifies IP, using IP as destination", actualDst.IP(), domain.FQDN)
		}
		return DestinationKey{
			destination:               HostPortFromIPPort(dst),
			actualDestination:         HostPortFromIPPort(actualDst),
			destinationWorkload:       dstWorkload,
			actualDestinationWorkload: actualDestWorkload,
		}

return DestinationKey{
destination: HostPortFromIPPort(dst),
Expand Down
4 changes: 2 additions & 2 deletions common/net_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ func TestDestinationKey(t *testing.T) {
assert.Equal(t, "1.1.1.1:443 (2.2.2.2:443)", NewDestinationKey(d, ad, nil, Workload{}, Workload{}).String())

assert.Equal(t,
"aa.bb.s3.amazonaws.com:443 ()",
"aa.bb.s3.amazonaws.com:443 (2.2.2.2:443)",
NewDestinationKey(d, ad, &Domain{FQDN: "aa.bb.s3.amazonaws.com", SpecifyIP: false}, Workload{}, Workload{}).String(),
)
assert.Equal(t,
Expand All @@ -51,7 +51,7 @@ func TestDomain(t *testing.T) {
assert.Equal(t, "Domain(fqdn,true)", NewDomain("fqdn", []netaddr.IP{
netaddr.MustParseIP("1.1.1.1"),
}).String())
assert.Equal(t, "Domain(fqdn,false)", NewDomain("fqdn", []netaddr.IP{
assert.Equal(t, "Domain(fqdn,true)", NewDomain("fqdn", []netaddr.IP{
netaddr.MustParseIP("1.1.1.1"),
netaddr.MustParseIP("1.1.1.2"),
}).String())
Comment on lines +54 to 57

@gemini-code-assist gemini-code-assist Bot Jul 4, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Following the change to NewDomain to remove the ips parameter, this test case should be updated to call the function with its new signature.

	assert.Equal(t, "Domain(fqdn,true)", NewDomain("fqdn").String())

Expand Down
15 changes: 2 additions & 13 deletions containers/container.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -763,7 +763,7 @@ func (c *Container) onDNSRequest(r *l7.RequestData) map[netaddr.IP]*common.Domai
return ip2fqdn
}

func (c *Container) onL7Request(pid uint32, fd uint64, timestamp uint64, r *l7.RequestData, iqfqdn map[netaddr.IP]*common.Domain) map[netaddr.IP]*common.Domain {
func (c *Container) onL7Request(pid uint32, fd uint64, timestamp uint64, r *l7.RequestData) map[netaddr.IP]*common.Domain {
c.lock.Lock()
defer c.lock.Unlock()

Expand All @@ -778,17 +778,6 @@ func (c *Container) onL7Request(pid uint32, fd uint64, timestamp uint64, r *l7.R
if timestamp != 0 && conn.Timestamp != timestamp {
return nil
}
if conn.dstWorkload.Namespace == "external" && (r.Protocol == l7.ProtocolHTTP || r.Protocol == l7.ProtocolHTTP2) {
if host, ok := iqfqdn[conn.DestinationKey.ActualDestination().IP()]; ok {
log.Printf("Setting external host %s", host)
conn.dstWorkload.Name = host.FQDN
} else {
host, error := l7.ParseHostFromHttpRequest(string(r.Payload))
if error == nil {
conn.dstWorkload.Name = host
}
}
}

// Parse HTTP request once if needed
var req *http.Request
Expand All @@ -806,7 +795,7 @@ func (c *Container) onL7Request(pid uint32, fd uint64, timestamp uint64, r *l7.R
if headers != nil {
traceId = trace.ExtractTraceId(headers)
}
stats := c.l7Stats.get(r.Protocol, conn.DestinationKey, r, conn.srcWorkload, conn.DestinationKey.GetDestinationWorkload(), conn.DestinationKey.GetActualDestinationWorkload(), traceId)
stats := c.l7Stats.get(r.Protocol, conn.DestinationKey, r, conn.srcWorkload, traceId)
switch r.Protocol {
case l7.ProtocolHTTP:
stats.observe(r.Status.Http(), "", r.Duration)
Expand Down
14 changes: 7 additions & 7 deletions containers/l7.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ func (m *L7Metrics) observe(status, method string, duration time.Duration) {

type L7Stats map[l7.Protocol]map[common.DestinationKey]*L7Metrics // protocol -> dst:actual_dst -> metrics

func (s L7Stats) get(protocol l7.Protocol, key common.DestinationKey, r *l7.RequestData, srcWorkload common.Workload, dstWorkload common.Workload, actualDstWorkload common.Workload, traceId string) *L7Metrics {
func (s L7Stats) get(protocol l7.Protocol, key common.DestinationKey, r *l7.RequestData, srcWorkload common.Workload, traceId string) *L7Metrics {
if protocol == l7.ProtocolHTTP2 {
protocol = l7.ProtocolHTTP
}
Expand All @@ -53,15 +53,15 @@ func (s L7Stats) get(protocol l7.Protocol, key common.DestinationKey, r *l7.Requ
protoStats[key] = m
constLabels := map[string]string{"destination": key.DestinationLabelValue(),
"actual_destination": key.ActualDestinationLabelValue(),
"destination_workload_kind": dstWorkload.Kind,
"destination_workload_name": dstWorkload.Name,
"destination_workload_namespace": dstWorkload.Namespace,
"destination_workload_kind": key.GetDestinationWorkload().Kind,
"destination_workload_name": key.GetDestinationWorkload().Name,
"destination_workload_namespace": key.GetDestinationWorkload().Namespace,
Comment on lines +56 to +58

@gemini-code-assist gemini-code-assist Bot Jul 4, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The destination workload name is being retrieved using key.GetDestinationWorkload().Name which is not consistent with the other fields. It should be retrieved directly from the key.

Suggested change
"destination_workload_kind": key.GetDestinationWorkload().Kind,
"destination_workload_name": key.GetDestinationWorkload().Name,
"destination_workload_namespace": key.GetDestinationWorkload().Namespace,
"destination_workload_name": key.GetDestinationWorkload().Name,

"src_workload_kind": srcWorkload.Kind,
"src_workload_name": srcWorkload.Name,
"src_workload_namespace": srcWorkload.Namespace,
"actual_destination_workload_kind": actualDstWorkload.Kind,
"actual_destination_workload_name": actualDstWorkload.Name,
"actual_destination_workload_namespace": actualDstWorkload.Namespace,
"actual_destination_workload_kind": key.GetActualDestinationWorkload().Kind,
"actual_destination_workload_name": key.GetActualDestinationWorkload().Name,
"actual_destination_workload_namespace": key.GetActualDestinationWorkload().Namespace,
Comment on lines +62 to +64

@gemini-code-assist gemini-code-assist Bot Jul 4, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The actual destination workload name is being retrieved using key.GetActualDestinationWorkload().Name which is not consistent with the other fields. It should be retrieved directly from the key.

Suggested change
"actual_destination_workload_kind": key.GetActualDestinationWorkload().Kind,
"actual_destination_workload_name": key.GetActualDestinationWorkload().Name,
"actual_destination_workload_namespace": key.GetActualDestinationWorkload().Namespace,
"actual_destination_workload_name": key.GetActualDestinationWorkload().Name,

}
if traceId != "" {
constLabels["trace_id"] = traceId
Expand Down
2 changes: 1 addition & 1 deletion containers/registry.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,7 @@ func (r *Registry) handleEvents(ch <-chan ebpftracer.Event) {
continue
}
if c := r.containersByPid[e.Pid]; c != nil {
ip2fqdn := c.onL7Request(e.Pid, e.Fd, e.Timestamp, e.L7Request, r.ip2fqdn)
ip2fqdn := c.onL7Request(e.Pid, e.Fd, e.Timestamp, e.L7Request)
r.ip2fqdnLock.Lock()
for ip, domain := range ip2fqdn {
r.ip2fqdn[ip] = domain
Expand Down
5 changes: 2 additions & 3 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ require (
github.com/containerd/cgroups v1.0.4
github.com/containerd/containerd v1.6.38
github.com/coreos/go-systemd/v22 v22.5.0
github.com/docker/docker v27.4.0+incompatible
github.com/docker/docker v28.0.0+incompatible
github.com/florianl/go-conntrack v0.3.0
github.com/go-kit/log v0.2.1
github.com/godbus/dbus/v5 v5.1.0
Expand Down Expand Up @@ -161,7 +161,7 @@ require (
github.com/spf13/pflag v1.0.6-0.20210604193023-d5e0c0615ace // indirect
github.com/spf13/viper v1.19.0 // indirect
github.com/subosito/gotenv v1.6.0 // indirect
github.com/ulikunitz/xz v0.5.12 // indirect
github.com/ulikunitz/xz v0.5.14 // indirect
github.com/x448/float16 v0.8.4 // indirect
go.etcd.io/etcd/api/v3 v3.5.17 // indirect
go.etcd.io/etcd/client/pkg/v3 v3.5.17 // indirect
Expand Down Expand Up @@ -190,7 +190,6 @@ require (
gopkg.in/inf.v0 v0.9.1 // indirect
gopkg.in/ini.v1 v1.67.0 // indirect
gopkg.in/yaml.v3 v3.0.1 // indirect
gotest.tools v2.2.0+incompatible
k8s.io/apiextensions-apiserver v0.32.0 // indirect
k8s.io/kube-openapi v0.0.0-20241105132330-32ad38e42d3f // indirect
k8s.io/utils v0.0.0-20241210054802-24370beab758 // indirect
Expand Down
8 changes: 4 additions & 4 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -306,8 +306,8 @@ github.com/docker/distribution v0.0.0-20190905152932-14b96e55d84c/go.mod h1:0+TT
github.com/docker/distribution v2.7.1-0.20190205005809-0d3efadf0154+incompatible/go.mod h1:J2gT2udsDAN96Uj4KfcMRqY0/ypR+oyYUYmja8H+y+w=
github.com/docker/distribution v2.7.1+incompatible/go.mod h1:J2gT2udsDAN96Uj4KfcMRqY0/ypR+oyYUYmja8H+y+w=
github.com/docker/docker v1.4.2-0.20190924003213-a8608b5b67c7/go.mod h1:eEKB0N0r5NX/I1kEveEz05bcu8tLC/8azJZsviup8Sk=
github.com/docker/docker v27.4.0+incompatible h1:I9z7sQ5qyzO0BfAb9IMOawRkAGxhYsidKiTMcm0DU+A=
github.com/docker/docker v27.4.0+incompatible/go.mod h1:eEKB0N0r5NX/I1kEveEz05bcu8tLC/8azJZsviup8Sk=
github.com/docker/docker v28.0.0+incompatible h1:Olh0KS820sJ7nPsBKChVhk5pzqcwDR15fumfAd/p9hM=
github.com/docker/docker v28.0.0+incompatible/go.mod h1:eEKB0N0r5NX/I1kEveEz05bcu8tLC/8azJZsviup8Sk=
github.com/docker/docker-credential-helpers v0.6.3/go.mod h1:WRaJzqw3CTB9bk10avuGsjVBZsD05qeibJ1/TYlvc0Y=
github.com/docker/go-connections v0.4.0/go.mod h1:Gbd7IOopHjR8Iph03tsViu4nIes5XhDvyHbTtUxmeec=
github.com/docker/go-connections v0.5.0 h1:USnMq7hx7gwdVZq1L49hLXaFtUdTADjXGp+uj1Br63c=
Expand Down Expand Up @@ -911,8 +911,8 @@ github.com/tchap/go-patricia v2.2.6+incompatible/go.mod h1:bmLyhP68RS6kStMGxByiQ
github.com/tmc/grpc-websocket-proxy v0.0.0-20170815181823-89b8d40f7ca8/go.mod h1:ncp9v5uamzpCO7NfCPTXjqaC+bZgJeR0sMTm6dMHP7U=
github.com/tmc/grpc-websocket-proxy v0.0.0-20190109142713-0ad062ec5ee5/go.mod h1:ncp9v5uamzpCO7NfCPTXjqaC+bZgJeR0sMTm6dMHP7U=
github.com/ugorji/go v1.1.4/go.mod h1:uQMGLiO92mf5W77hV/PUCpI3pbzQx3CRekS0kk+RGrc=
github.com/ulikunitz/xz v0.5.12 h1:37Nm15o69RwBkXM0J6A5OlE67RZTfzUxTj8fB3dfcsc=
github.com/ulikunitz/xz v0.5.12/go.mod h1:nbz6k7qbPmH4IRqmfOplQw/tblSgqTqBwxkY0oWt/14=
github.com/ulikunitz/xz v0.5.14 h1:uv/0Bq533iFdnMHZdRBTOlaNMdb1+ZxXIlHDZHIHcvg=
github.com/ulikunitz/xz v0.5.14/go.mod h1:nbz6k7qbPmH4IRqmfOplQw/tblSgqTqBwxkY0oWt/14=
github.com/urfave/cli v0.0.0-20171014202726-7bc6a0acffa5/go.mod h1:70zkFmudgCuE/ngEzBv17Jvp/497gISqfk5gWijbERA=
github.com/urfave/cli v1.20.0/go.mod h1:70zkFmudgCuE/ngEzBv17Jvp/497gISqfk5gWijbERA=
github.com/urfave/cli v1.22.1/go.mod h1:Gos4lmkARVdJ6EkW0WaNv/tZAAMe9V7XWyB60NtXRu0=
Expand Down
Loading