Skip to content

Commit

Permalink
Fix SDK iptables.Config marshalling (#20451)
Browse files Browse the repository at this point in the history
This fixes behavior introduced by #20232 where
a function was added to the iptables configuration struct. Since this
struct is actually marshalled into json by consul-k8s, we should not be
placing functions inside of it.
  • Loading branch information
hashi-derek authored Feb 2, 2024
1 parent 22e6ce0 commit 1fe0a87
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 16 deletions.
34 changes: 20 additions & 14 deletions sdk/iptables/iptables.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,18 +69,18 @@ type Config struct {

// IptablesProvider is the Provider that will apply iptables rules.
IptablesProvider Provider

// AddAdditionalRulesFn can be implemented by the caller to
// add environment specific rules (like ECS) that needs to
// be executed for traffic redirection to work properly.
//
// This gets called by the Setup function after all the
// first class iptable rules are added. The implemented
// function should only call the `AddRule` and optionally
// the `Rules` method of the provider.
AddAdditionalRulesFn func(iptablesProvider Provider)
}

// AdditionalRulesFn can be implemented by the caller to
// add environment specific rules (like ECS) that needs to
// be executed for traffic redirection to work properly.
//
// This gets called by the Setup function after all the
// first class iptable rules are added. The implemented
// function should only call the `AddRule` and optionally
// the `Rules` method of the provider.
type AdditionalRulesFn func(iptablesProvider Provider)

// Provider is an interface for executing iptables rules.
type Provider interface {
// AddRule adds a rule without executing it.
Expand All @@ -98,9 +98,15 @@ type Provider interface {

// Setup will set up iptables interception and redirection rules
// based on the configuration provided in cfg.
// This implementation was inspired by
// https://github.com/openservicemesh/osm/blob/650a1a1dcf081ae90825f3b5dba6f30a0e532725/pkg/injector/iptables.go
func Setup(cfg Config) error {
return SetupWithAdditionalRules(cfg, nil)
}

// SetupWithAdditionalRules will set up iptables interception and redirection rules
// based on the configuration provided in cfg. The additionalRulesFn will be applied
// after the normal set of rules. This implementation was inspired by
// https://github.com/openservicemesh/osm/blob/650a1a1dcf081ae90825f3b5dba6f30a0e532725/pkg/injector/iptables.go
func SetupWithAdditionalRules(cfg Config, additionalRulesFn AdditionalRulesFn) error {
if cfg.IptablesProvider == nil {
cfg.IptablesProvider = &iptablesExecutor{cfg: cfg}
}
Expand Down Expand Up @@ -193,8 +199,8 @@ func Setup(cfg Config) error {
}

// Call function to add any additional rules passed on by the caller
if cfg.AddAdditionalRulesFn != nil {
cfg.AddAdditionalRulesFn(cfg.IptablesProvider)
if additionalRulesFn != nil {
additionalRulesFn(cfg.IptablesProvider)
}

return cfg.IptablesProvider.ApplyRules()
Expand Down
5 changes: 3 additions & 2 deletions sdk/iptables/iptables_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -303,15 +303,16 @@ func TestSetup(t *testing.T) {

for _, c := range cases {
t.Run(c.name, func(t *testing.T) {
var fn AdditionalRulesFn
if c.additionalRules != nil {
c.cfg.AddAdditionalRulesFn = func(provider Provider) {
fn = func(provider Provider) {
for _, rule := range c.additionalRules {
provider.AddRule(rule[0], rule[1:]...)
}
}
}

err := Setup(c.cfg)
err := SetupWithAdditionalRules(c.cfg, fn)
require.NoError(t, err)
require.Equal(t, c.expectedRules, c.cfg.IptablesProvider.Rules())
})
Expand Down

0 comments on commit 1fe0a87

Please sign in to comment.