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

Remove lua-resty-waf feature #4779

Merged
merged 1 commit into from
Nov 27, 2019
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
3 changes: 0 additions & 3 deletions internal/ingress/annotations/annotations.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ import (
"k8s.io/ingress-nginx/internal/ingress/annotations/ipwhitelist"
"k8s.io/ingress-nginx/internal/ingress/annotations/loadbalancing"
"k8s.io/ingress-nginx/internal/ingress/annotations/log"
"k8s.io/ingress-nginx/internal/ingress/annotations/luarestywaf"
"k8s.io/ingress-nginx/internal/ingress/annotations/mirror"
"k8s.io/ingress-nginx/internal/ingress/annotations/parser"
"k8s.io/ingress-nginx/internal/ingress/annotations/portinredirect"
Expand Down Expand Up @@ -109,7 +108,6 @@ type Ingress struct {
XForwardedPrefix string
SSLCiphers string
Logs log.Config
LuaRestyWAF luarestywaf.Config
InfluxDB influxdb.Config
ModSecurity modsecurity.Config
Mirror mirror.Config
Expand Down Expand Up @@ -157,7 +155,6 @@ func NewAnnotationExtractor(cfg resolver.Resolver) Extractor {
"XForwardedPrefix": xforwardedprefix.NewParser(cfg),
"SSLCiphers": sslcipher.NewParser(cfg),
"Logs": log.NewParser(cfg),
"LuaRestyWAF": luarestywaf.NewParser(cfg),
"InfluxDB": influxdb.NewParser(cfg),
"BackendProtocol": backendprotocol.NewParser(cfg),
"ModSecurity": modsecurity.NewParser(cfg),
Expand Down
126 changes: 0 additions & 126 deletions internal/ingress/annotations/luarestywaf/main.go

This file was deleted.

86 changes: 0 additions & 86 deletions internal/ingress/annotations/luarestywaf/main_test.go

This file was deleted.

4 changes: 0 additions & 4 deletions internal/ingress/controller/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -610,10 +610,6 @@ type Configuration struct {
// +optional
GlobalExternalAuth GlobalExternalAuth `json:"global-external-auth"`

// DisableLuaRestyWAF disables lua-resty-waf globally regardless
// of whether there's an ingress that has enabled the WAF using annotation
DisableLuaRestyWAF bool `json:"disable-lua-resty-waf"`

// EnableInfluxDB enables the nginx InfluxDB extension
// http://github.com/influxdata/nginx-influxdb-module/
// By default this is disabled
Expand Down
1 change: 0 additions & 1 deletion internal/ingress/controller/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -1177,7 +1177,6 @@ func locationApplyAnnotations(loc *ingress.Location, anns *annotations.Ingress)
loc.UsePortInRedirects = anns.UsePortInRedirects
loc.Connection = anns.Connection
loc.Logs = anns.Logs
loc.LuaRestyWAF = anns.LuaRestyWAF
loc.InfluxDB = anns.InfluxDB
loc.DefaultBackend = anns.DefaultBackend
loc.BackendProtocol = anns.BackendProtocol
Expand Down
36 changes: 3 additions & 33 deletions internal/ingress/controller/template/template.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,11 +93,6 @@ func (t *Template) Write(conf config.TemplateConfig) ([]byte, error) {
outCmdBuf := t.bp.Get()
defer t.bp.Put(outCmdBuf)

// TODO: remove once we found a fix for coredump running luarocks install lrexlib
if runtime.GOARCH == "arm" {
conf.Cfg.DisableLuaRestyWAF = true
}

if klog.V(3) {
b, err := json.Marshal(conf)
if err != nil {
Expand Down Expand Up @@ -134,7 +129,6 @@ var (
return true
},
"escapeLiteralDollar": escapeLiteralDollar,
"shouldConfigureLuaRestyWAF": shouldConfigureLuaRestyWAF,
"buildLuaSharedDictionaries": buildLuaSharedDictionaries,
"luaConfigurationRequestBodySize": luaConfigurationRequestBodySize,
"buildLocation": buildLocation,
Expand Down Expand Up @@ -225,23 +219,16 @@ func quote(input interface{}) string {
return fmt.Sprintf("%q", inputStr)
}

func shouldConfigureLuaRestyWAF(disableLuaRestyWAF bool, mode string) bool {
if !disableLuaRestyWAF && len(mode) > 0 {
return true
}

return false
}

func buildLuaSharedDictionaries(c interface{}, s interface{}, disableLuaRestyWAF bool) string {
func buildLuaSharedDictionaries(c interface{}, s interface{}) string {
var out []string

cfg, ok := c.(config.Configuration)
if !ok {
klog.Errorf("expected a 'config.Configuration' type but %T was returned", c)
return ""
}
servers, ok := s.([]*ingress.Server)

_, ok = s.([]*ingress.Server)
if !ok {
klog.Errorf("expected an '[]*ingress.Server' type but %T was returned", s)
return ""
Expand All @@ -251,23 +238,6 @@ func buildLuaSharedDictionaries(c interface{}, s interface{}, disableLuaRestyWAF
out = append(out, fmt.Sprintf("lua_shared_dict %s %dM", name, size))
}

// TODO: there must be a better place for this
if _, ok := cfg.LuaSharedDicts["waf_storage"]; !ok && !disableLuaRestyWAF {
luaRestyWAFEnabled := func() bool {
for _, server := range servers {
for _, location := range server.Locations {
if len(location.LuaRestyWAF.Mode) > 0 {
return true
}
}
}
return false
}()
if luaRestyWAFEnabled {
out = append(out, "lua_shared_dict waf_storage 64M")
}
}

sort.Strings(out)

return strings.Join(out, ";\n") + ";\n"
Expand Down
20 changes: 5 additions & 15 deletions internal/ingress/controller/template/template_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ import (
"k8s.io/ingress-nginx/internal/ingress"
"k8s.io/ingress-nginx/internal/ingress/annotations/authreq"
"k8s.io/ingress-nginx/internal/ingress/annotations/influxdb"
"k8s.io/ingress-nginx/internal/ingress/annotations/luarestywaf"
"k8s.io/ingress-nginx/internal/ingress/annotations/modsecurity"
"k8s.io/ingress-nginx/internal/ingress/annotations/ratelimit"
"k8s.io/ingress-nginx/internal/ingress/annotations/rewrite"
Expand Down Expand Up @@ -189,7 +188,7 @@ func TestBuildLuaSharedDictionaries(t *testing.T) {
"configuration_data": 10, "certificate_data": 20,
},
}
actual := buildLuaSharedDictionaries(cfg, invalidType, true)
actual := buildLuaSharedDictionaries(cfg, invalidType)

if !reflect.DeepEqual(expected, actual) {
t.Errorf("Expected '%v' but returned '%v'", expected, actual)
Expand All @@ -198,32 +197,23 @@ func TestBuildLuaSharedDictionaries(t *testing.T) {
servers := []*ingress.Server{
{
Hostname: "foo.bar",
Locations: []*ingress.Location{{Path: "/", LuaRestyWAF: luarestywaf.Config{}}},
Locations: []*ingress.Location{{Path: "/"}},
},
{
Hostname: "another.host",
Locations: []*ingress.Location{{Path: "/", LuaRestyWAF: luarestywaf.Config{}}},
Locations: []*ingress.Location{{Path: "/"}},
},
}
// returns value from config
configuration := buildLuaSharedDictionaries(cfg, servers, false)
configuration := buildLuaSharedDictionaries(cfg, servers)
if !strings.Contains(configuration, "lua_shared_dict configuration_data 10M;\n") {
t.Errorf("expected to include 'configuration_data' but got %s", configuration)
}
if !strings.Contains(configuration, "lua_shared_dict certificate_data 20M;\n") {
t.Errorf("expected to include 'certificate_data' but got %s", configuration)
}
if strings.Contains(configuration, "waf_storage") {
t.Errorf("expected to not include 'waf_storage' but got %s", configuration)
}

servers[1].Locations[0].LuaRestyWAF = luarestywaf.Config{Mode: "ACTIVE"}
configuration = buildLuaSharedDictionaries(cfg, servers, false)
if !strings.Contains(configuration, "lua_shared_dict waf_storage") {
t.Errorf("expected to configure 'waf_storage', but got %s", configuration)
}
// test invalid config
configuration = buildLuaSharedDictionaries(invalidType, servers, false)
configuration = buildLuaSharedDictionaries(invalidType, servers)
if configuration != "" {
t.Errorf("expected an empty string, but got %s", configuration)
}
Expand Down
3 changes: 0 additions & 3 deletions internal/ingress/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ import (
"k8s.io/ingress-nginx/internal/ingress/annotations/influxdb"
"k8s.io/ingress-nginx/internal/ingress/annotations/ipwhitelist"
"k8s.io/ingress-nginx/internal/ingress/annotations/log"
"k8s.io/ingress-nginx/internal/ingress/annotations/luarestywaf"
"k8s.io/ingress-nginx/internal/ingress/annotations/mirror"
"k8s.io/ingress-nginx/internal/ingress/annotations/modsecurity"
"k8s.io/ingress-nginx/internal/ingress/annotations/proxy"
Expand Down Expand Up @@ -307,8 +306,6 @@ type Location struct {
// Logs allows to enable or disable the nginx logs
// By default access logs are enabled and rewrite logs are disabled
Logs log.Config `json:"logs,omitempty"`
// LuaRestyWAF contains parameters to configure lua-resty-waf
LuaRestyWAF luarestywaf.Config `json:"luaRestyWAF"`
// InfluxDB allows to monitor the incoming request by sending them to an influxdb database
// +optional
InfluxDB influxdb.Config `json:"influxDB,omitempty"`
Expand Down
Loading