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.

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

make iptables timeout configurable #75

Merged
merged 1 commit into from
Jan 5, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
69 changes: 49 additions & 20 deletions iptables/iptables.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ type IPTables struct {
v2 int
v3 int
mode string // the underlying iptables operating mode, e.g. nf_tables
timeout int // time to wait for the iptables lock, default waits forever
}

// Stat represents a structured statistic entry.
Expand All @@ -89,19 +90,42 @@ type Stat struct {
Options string `json:"options"`
}

// New creates a new IPTables.
// For backwards compatibility, this always uses IPv4, i.e. "iptables".
func New() (*IPTables, error) {
return NewWithProtocol(ProtocolIPv4)
type option func(*IPTables)

func IPFamily(proto Protocol) option {
return func(ipt *IPTables) {
ipt.proto = proto
}
}

// New creates a new IPTables for the given proto.
// The proto will determine which command is used, either "iptables" or "ip6tables".
func NewWithProtocol(proto Protocol) (*IPTables, error) {
path, err := exec.LookPath(getIptablesCommand(proto))
func Timeout(timeout int) option {
return func(ipt *IPTables) {
ipt.timeout = timeout
}
}

// New creates a new IPTables configured with the options passed as parameter.
// For backwards compatibility, by default always uses IPv4 and timeout 0.
// i.e. you can create an IPv6 IPTables using a timeout of 5 seconds passing
// the IPFamily and Timeout options as follow:
// ip6t := New(IPFamily(ProtocolIPv6), Timeout(5))
func New(opts ...option) (*IPTables, error) {

Choose a reason for hiding this comment

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

Technically this could still be a breaking change if someone was depending on the type of New to pass it, but that seems unlikely, and it would be relatively easy to fix I think.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we want to support all the options in iptables in the long term, wait interval comes to mind, I think is the more sustainable way.

I didn't like to add a NewWithProtocolTimeout() constructor, but I can change this to NewWithOptions() though

Choose a reason for hiding this comment

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

to be clear this LGTM to me already, but I'm not sure how this repo is maintained

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hey, I have merge rights, and do releases periodically. Anyways.

Can you update the docblock here to show how options work?


ipt := &IPTables{
proto: ProtocolIPv4,
timeout: 0,
}

for _, opt := range opts {
opt(ipt)
}

path, err := exec.LookPath(getIptablesCommand(ipt.proto))
if err != nil {
return nil, err
}
ipt.path = path

vstring, err := getIptablesVersionString(path)
if err != nil {
return nil, fmt.Errorf("could not get iptables version: %v", err)
Expand All @@ -110,21 +134,23 @@ func NewWithProtocol(proto Protocol) (*IPTables, error) {
if err != nil {
return nil, fmt.Errorf("failed to extract iptables version from [%s]: %v", vstring, err)
}
ipt.v1 = v1
ipt.v2 = v2
ipt.v3 = v3
ipt.mode = mode

checkPresent, waitPresent, randomFullyPresent := getIptablesCommandSupport(v1, v2, v3)
ipt.hasCheck = checkPresent
ipt.hasWait = waitPresent
ipt.hasRandomFully = randomFullyPresent

ipt := IPTables{
path: path,
proto: proto,
hasCheck: checkPresent,
hasWait: waitPresent,
hasRandomFully: randomFullyPresent,
v1: v1,
v2: v2,
v3: v3,
mode: mode,
}
return &ipt, nil
return ipt, nil
}

// New creates a new IPTables for the given proto.
// The proto will determine which command is used, either "iptables" or "ip6tables".
func NewWithProtocol(proto Protocol) (*IPTables, error) {
return New(IPFamily(proto), Timeout(0))
}

// Proto returns the protocol used by this IPTables.
Expand Down Expand Up @@ -426,6 +452,9 @@ func (ipt *IPTables) runWithOutput(args []string, stdout io.Writer) error {
args = append([]string{ipt.path}, args...)
if ipt.hasWait {
args = append(args, "--wait")
if ipt.timeout != 0 {
args = append(args, strconv.Itoa(ipt.timeout))
}
} else {
fmu, err := newXtablesFileLock()
if err != nil {
Expand Down
19 changes: 19 additions & 0 deletions iptables/iptables_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,25 @@ func TestProto(t *testing.T) {
}
}

func TestTimeout(t *testing.T) {
ipt, err := New()
if err != nil {
t.Fatalf("New failed: %v", err)
}
if ipt.timeout != 0 {
t.Fatalf("Expected timeout 0 (wait forever), got %v", ipt.timeout)
}

ipt2, err := New(Timeout(5))
if err != nil {
t.Fatalf("New failed: %v", err)
}
if ipt2.timeout != 5 {
t.Fatalf("Expected timeout 5, got %v", ipt.timeout)
}

}

func randChain(t *testing.T) string {
n, err := rand.Int(rand.Reader, big.NewInt(1000000))
if err != nil {
Expand Down