Skip to content
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

Swap out nsenter with Go-native code #264

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Swap out nsenter with Go-native code #264

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 1 addition & 0 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ go 1.21

require (
github.com/containernetworking/cni v1.1.2
github.com/containernetworking/plugins v1.3.0
github.com/mitchellh/mapstructure v1.5.0
github.com/sirupsen/logrus v1.9.3
github.com/spf13/cobra v1.7.0
Expand Down
6 changes: 4 additions & 2 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ github.com/chzyer/readline v0.0.0-20180603132655-2972be24d48e/go.mod h1:nSuG5e5P
github.com/chzyer/test v0.0.0-20180213035817-a1ea475d72b1/go.mod h1:Q3SI9o4m/ZMnBNeIyt5eFwwo7qiLfzFZmjNmxjkiQlU=
github.com/containernetworking/cni v1.1.2 h1:wtRGZVv7olUHMOqouPpn3cXJWpJgM6+EUl31EQbXALQ=
github.com/containernetworking/cni v1.1.2/go.mod h1:sDpYKmGVENF3s6uvMvGgldDWeG8dMxakj/u+i9ht9vw=
github.com/containernetworking/plugins v1.3.0 h1:QVNXMT6XloyMUoO2wUOqWTC1hWFV62Q6mVDp5H1HnjM=
github.com/containernetworking/plugins v1.3.0/go.mod h1:Pc2wcedTQQCVuROOOaLBPPxrEXqqXBFt3cZ+/yVg6l0=
github.com/cpuguy83/go-md2man/v2 v2.0.2/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o=
github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E=
github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
Expand Down Expand Up @@ -50,8 +52,8 @@ github.com/google/gofuzz v1.0.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/
github.com/google/gofuzz v1.2.0 h1:xRy4A+RhZaiKjJ1bPfwQ8sedCA+YS2YcCHW6ec7JMi0=
github.com/google/gofuzz v1.2.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg=
github.com/google/pprof v0.0.0-20210407192527-94a9f03dee38/go.mod h1:kpwsk12EmLew5upagYY7GY0pfYCcupk39gWOCRROcvE=
github.com/google/pprof v0.0.0-20210720184732-4bb14d4b1be1 h1:K6RDEckDVWvDI9JAJYCmNdQXq6neHJOYx3V6jnqNEec=
github.com/google/pprof v0.0.0-20210720184732-4bb14d4b1be1/go.mod h1:kpwsk12EmLew5upagYY7GY0pfYCcupk39gWOCRROcvE=
github.com/google/pprof v0.0.0-20230323073829-e72429f035bd h1:r8yyd+DJDmsUhGrRBxH5Pj7KeFK5l+Y3FsgT8keqKtk=
github.com/google/pprof v0.0.0-20230323073829-e72429f035bd/go.mod h1:79YE0hCXdHag9sBkw2o+N/YnZtTkXi0UT9Nnixa5eYk=
github.com/google/uuid v1.3.0 h1:t6JiXgmwXMjEs8VusXIJk2BXHsn+wx8BZdTaoZ5fu7I=
github.com/google/uuid v1.3.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
github.com/hpcloud/tail v1.0.0/go.mod h1:ab1qPbhIpdTxEkNHXyeSf5vhxWSCs/tWer42PpOxQnU=
Expand Down
58 changes: 45 additions & 13 deletions internal/iptables/iptables.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"strings"
"time"

ns "github.com/containernetworking/plugins/pkg/ns"
log "github.com/sirupsen/logrus"

util "github.com/linkerd/linkerd2-proxy-init/internal/util"
Expand Down Expand Up @@ -229,28 +230,59 @@ func makeMultiportDestinations(portsToIgnore []string) [][]string {
}

func executeCommand(firewallConfiguration FirewallConfiguration, cmd *exec.Cmd) ([]byte, error) {
if firewallConfiguration.NetNs != "" {
// BusyBox's `nsenter` needs `--` to separate nsenter arguments from the
// command.
//
// See https://github.com/rancher/k3s/issues/1434#issuecomment-629315909
nsArgs := fmt.Sprintf("--net=%s", firewallConfiguration.NetNs)
args := append([]string{nsArgs, "--"}, cmd.Args...)
cmd = exec.Command("nsenter", args...)
}
// Always log the command to apply for tracing purposes
log.Info(cmd.String())

// Short out early if we are just simulating the network
if firewallConfiguration.SimulateOnly {
return nil, nil
}

out, err := cmd.CombinedOutput()
// Helper for reusing code when actually calling out to the command
doCommand := func() ([]byte, error) {
out, err := cmd.CombinedOutput()

if len(out) > 0 {
log.Infof("%s", out)
// Log out the output, if any
if len(out) > 0 {
log.Infof("%s", out)
}

return out, err
}

return out, err
// If we need to run within a target network namespace, then wrap the command
// in that namespace.
//
// Note: Network namespace switching is very volatile in Go. Care should be taken
// to ensure that all namespaced commands be wrapped in `targetNamespace.Do`, as explained in
// the link below.
//
// See: https://pkg.go.dev/github.com/containernetworking/plugins/pkg/ns#readme-do-the-recommended-thing
if firewallConfiguration.NetNs != "" {
// Fetch the target net namespace, ensuring that it exists
netNs, err := ns.GetNS(firewallConfiguration.NetNs)
if err != nil {
log.Errorf("could not switch to target network namespace \"%s\": %s", firewallConfiguration.NetNs, err.Error())
return nil, err
}

Copy link
Member

Choose a reason for hiding this comment

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

Can we instrument the logger with the current network namespace we're executing in? I'd err on the side of caution here since ub in this space will be incredibly painful to debug without good logging.

// Actually run the command in the namespace
// Note: Try to keep this code short! Goroutine switches might cause the
// namespace to change...
//
// Note: Result needs to be defined here since `netNs.Do` only returns
// an error.
result := make([]byte, 0)
Copy link
Member

Choose a reason for hiding this comment

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

can we do var result []byte? Even if it's a zero length array we still allocate a pointer that is then set to whatever cmd.CombinedOutput returns, so it might be redundant?

err = netNs.Do(func(_ ns.NetNS) error {
result, err = doCommand()

return err
})

return result, err
} else {
return doCommand()
Copy link
Member

Choose a reason for hiding this comment

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

Should we log at the debug level that we're executing in the root network namespace?

}
}

func (fc FirewallConfiguration) makeIgnoreUserID(chainName string, uid int, comment string) *exec.Cmd {
Expand Down