diff --git a/docs/user-guide/nginx-configuration/configmap.md b/docs/user-guide/nginx-configuration/configmap.md index 83eaae2a7c..fe550c8cca 100755 --- a/docs/user-guide/nginx-configuration/configmap.md +++ b/docs/user-guide/nginx-configuration/configmap.md @@ -148,6 +148,7 @@ The following table shows a configuration option's name, type, and the default v |[skip-access-log-urls](#skip-access-log-urls)|[]string|[]string{}| |[limit-rate](#limit-rate)|int|0| |[limit-rate-after](#limit-rate-after)|int|0| +|[lua-shared-dicts](#lua-shared-dicts)|string|""| |[http-redirect-code](#http-redirect-code)|int|308| |[proxy-buffering](#proxy-buffering)|string|"off"| |[limit-req-status-code](#limit-req-status-code)|int|503| @@ -847,6 +848,21 @@ _References:_ Sets the initial amount after which the further transmission of a response to a client will be rate limited. +## lua-shared-dicts + +Customize default Lua shared dictionaries or define more. You can use the following syntax to do so: + +``` +lua-shared-dicts: ": , [: ], ..." +``` + +For example following will set default `certificate_data` dictionary to `100M` and will introduce a new dictionary called +`my_custom_plugin`: + +``` +lua-shared-dicts: "certificate_data: 100, my_custom_plugin: 5" +``` + _References:_ [http://nginx.org/en/docs/http/ngx_http_core_module.html#limit_rate_after](http://nginx.org/en/docs/http/ngx_http_core_module.html#limit_rate_after) diff --git a/internal/ingress/controller/template/configmap.go b/internal/ingress/controller/template/configmap.go index 76492180bf..58957bb654 100644 --- a/internal/ingress/controller/template/configmap.go +++ b/internal/ingress/controller/template/configmap.go @@ -59,11 +59,20 @@ const ( globalAuthSnippet = "global-auth-snippet" globalAuthCacheKey = "global-auth-cache-key" globalAuthCacheDuration = "global-auth-cache-duration" - luaSharedDicts = "lua-shared-dicts" + luaSharedDictsKey = "lua-shared-dicts" ) var ( - validRedirectCodes = sets.NewInt([]int{301, 302, 307, 308}...) + validRedirectCodes = sets.NewInt([]int{301, 302, 307, 308}...) + defaultLuaSharedDicts = map[string]int{ + "configuration_data": 20, + "certificate_data": 20, + } +) + +const ( + maxAllowedLuaDictSize = 200 + maxNumberOfLuaDicts = 100 ) // ReadConfig obtains the configuration defined by the user merged with the defaults. @@ -88,20 +97,38 @@ func ReadConfig(src map[string]string) config.Configuration { blockUserAgentList := make([]string, 0) blockRefererList := make([]string, 0) responseHeaders := make([]string, 0) - luaSharedDict := make(map[string]int) + luaSharedDicts := make(map[string]int) //parse lua shared dict values - if val, ok := conf[luaSharedDicts]; ok { - delete(conf, luaSharedDicts) + if val, ok := conf[luaSharedDictsKey]; ok { + delete(conf, luaSharedDictsKey) lsd := strings.Split(val, ",") for _, v := range lsd { v = strings.Replace(v, " ", "", -1) results := strings.SplitN(v, ":", 2) - val, err := strconv.Atoi(results[1]) + dictName := results[0] + size, err := strconv.Atoi(results[1]) if err != nil { - klog.Warningf("%v is not a valid lua entry: %v", v, err) + klog.Errorf("Ignoring non integer value %v for Lua dictionary %v: %v.", results[1], dictName, err) + continue + } + if size > maxAllowedLuaDictSize { + klog.Errorf("Ignoring %v for Lua dictionary %v: maximum size is %v.", size, dictName, maxAllowedLuaDictSize) + continue } - luaSharedDict[results[0]] = val + if len(luaSharedDicts)+1 > maxNumberOfLuaDicts { + klog.Errorf("Ignoring %v for Lua dictionary %v: can not configure more than %v dictionaries.", + size, dictName, maxNumberOfLuaDicts) + continue + } + + luaSharedDicts[dictName] = size + } + } + // set default Lua shared dicts + for k, v := range defaultLuaSharedDicts { + if _, ok := luaSharedDicts[k]; !ok { + luaSharedDicts[k] = v } } if val, ok := conf[customHTTPErrors]; ok { @@ -321,7 +348,7 @@ func ReadConfig(src map[string]string) config.Configuration { to.HideHeaders = hideHeadersList to.ProxyStreamResponses = streamResponses to.DisableIpv6DNS = !ing_net.IsIPv6Enabled() - to.LuaSharedDicts = luaSharedDict + to.LuaSharedDicts = luaSharedDicts config := &mapstructure.DecoderConfig{ Metadata: nil, diff --git a/internal/ingress/controller/template/configmap_test.go b/internal/ingress/controller/template/configmap_test.go index 21bbe0ee1b..d6f40797e5 100644 --- a/internal/ingress/controller/template/configmap_test.go +++ b/internal/ingress/controller/template/configmap_test.go @@ -124,6 +124,7 @@ func TestMergeConfigMapToStruct(t *testing.T) { } def = config.NewDefault() + def.LuaSharedDicts = map[string]int{"configuration_data": 20, "certificate_data": 20} def.DisableIpv6DNS = true hash, err = hashstructure.Hash(def, &hashstructure.HashOptions{ @@ -142,6 +143,7 @@ func TestMergeConfigMapToStruct(t *testing.T) { } def = config.NewDefault() + def.LuaSharedDicts = map[string]int{"configuration_data": 20, "certificate_data": 20} def.WhitelistSourceRange = []string{"1.1.1.1/32"} def.DisableIpv6DNS = true @@ -303,50 +305,48 @@ func TestGlobalExternalAuthCacheDurationParsing(t *testing.T) { } } -func TestLuaSharedDict(t *testing.T) { - +func TestLuaSharedDictsParsing(t *testing.T) { testsCases := []struct { name string entry map[string]string expect map[string]int }{ { - name: "lua valid entry", - entry: map[string]string{"lua-shared-dicts": "configuration_data:5,certificate_data:5"}, - expect: map[string]int{"configuration_data": 5, "certificate_data": 5}, + name: "default dicts configured when lua-shared-dicts is not set", + entry: make(map[string]string), + expect: defaultLuaSharedDicts, }, - { - name: "lua invalid entry", - entry: map[string]string{"lua-shared-dict": "configuration_data:5,certificate_data:5"}, - expect: map[string]int{}, + name: "configuration_data only", + entry: map[string]string{"lua-shared-dicts": "configuration_data:5"}, + expect: map[string]int{"configuration_data": 5, "certificate_data": 20}, }, { - name: "lua mixed entry", - entry: map[string]string{"lua-shared-dicts": "configuration_data:10,certificate_data:5"}, - expect: map[string]int{"configuration_data": 10, "certificate_data": 5}, + name: "certificate_data only", + entry: map[string]string{"lua-shared-dicts": "certificate_data: 4"}, + expect: map[string]int{"configuration_data": 20, "certificate_data": 4}, }, { - name: "lua valid entry - configuration_data only", - entry: map[string]string{"lua-shared-dicts": "configuration_data:5"}, - expect: map[string]int{"configuration_data": 5}, + name: "custom dicts", + entry: map[string]string{"lua-shared-dicts": "configuration_data: 10, my_random_dict:15 , another_example:2"}, + expect: map[string]int{"configuration_data": 10, "certificate_data": 20, "my_random_dict": 15, "another_example": 2}, }, { - name: "lua valid entry certificate_data only", - entry: map[string]string{"lua-shared-dicts": "certificate_data:5"}, - expect: map[string]int{"certificate_data": 5}, + name: "invalid size value should be ignored", + entry: map[string]string{"lua-shared-dicts": "mydict: 10, invalid_dict: 1a"}, + expect: map[string]int{"configuration_data": 20, "certificate_data": 20, "mydict": 10}, }, { - name: "lua valid entry certificate_data only", - entry: map[string]string{"lua-shared-dicts": "configuration_data:10, my_random_dict:15,another_example:2"}, - expect: map[string]int{"configuration_data": 10, "my_random_dict": 15, "another_example": 2}, + name: "dictionary size can not be larger than 200", + entry: map[string]string{"lua-shared-dicts": "mydict: 10, invalid_dict: 201"}, + expect: map[string]int{"configuration_data": 20, "certificate_data": 20, "mydict": 10}, }, } - for n, tc := range testsCases { + for _, tc := range testsCases { cfg := ReadConfig(tc.entry) if !reflect.DeepEqual(cfg.LuaSharedDicts, tc.expect) { - t.Errorf("Testing %v. Expected \"%v\" but \"%v\" was returned", n, tc.expect, cfg.LuaSharedDicts) + t.Errorf("Testing %v. Expected \"%v\" but \"%v\" was returned", tc.name, tc.expect, cfg.LuaSharedDicts) } } } diff --git a/internal/ingress/controller/template/template.go b/internal/ingress/controller/template/template.go index 5fe2d78ea4..72d84cf279 100644 --- a/internal/ingress/controller/template/template.go +++ b/internal/ingress/controller/template/template.go @@ -133,36 +133,37 @@ var ( } return true }, - "escapeLiteralDollar": escapeLiteralDollar, - "shouldConfigureLuaRestyWAF": shouldConfigureLuaRestyWAF, - "buildLuaSharedDictionaries": buildLuaSharedDictionaries, - "buildLocation": buildLocation, - "buildAuthLocation": buildAuthLocation, - "shouldApplyGlobalAuth": shouldApplyGlobalAuth, - "buildAuthResponseHeaders": buildAuthResponseHeaders, - "buildProxyPass": buildProxyPass, - "filterRateLimits": filterRateLimits, - "buildRateLimitZones": buildRateLimitZones, - "buildRateLimit": buildRateLimit, - "configForLua": configForLua, - "locationConfigForLua": locationConfigForLua, - "buildResolvers": buildResolvers, - "buildUpstreamName": buildUpstreamName, - "isLocationInLocationList": isLocationInLocationList, - "isLocationAllowed": isLocationAllowed, - "buildLogFormatUpstream": buildLogFormatUpstream, - "buildDenyVariable": buildDenyVariable, - "getenv": os.Getenv, - "contains": strings.Contains, - "hasPrefix": strings.HasPrefix, - "hasSuffix": strings.HasSuffix, - "trimSpace": strings.TrimSpace, - "toUpper": strings.ToUpper, - "toLower": strings.ToLower, - "formatIP": formatIP, - "quote": quote, - "buildNextUpstream": buildNextUpstream, - "getIngressInformation": getIngressInformation, + "escapeLiteralDollar": escapeLiteralDollar, + "shouldConfigureLuaRestyWAF": shouldConfigureLuaRestyWAF, + "buildLuaSharedDictionaries": buildLuaSharedDictionaries, + "luaConfigurationRequestBodySize": luaConfigurationRequestBodySize, + "buildLocation": buildLocation, + "buildAuthLocation": buildAuthLocation, + "shouldApplyGlobalAuth": shouldApplyGlobalAuth, + "buildAuthResponseHeaders": buildAuthResponseHeaders, + "buildProxyPass": buildProxyPass, + "filterRateLimits": filterRateLimits, + "buildRateLimitZones": buildRateLimitZones, + "buildRateLimit": buildRateLimit, + "configForLua": configForLua, + "locationConfigForLua": locationConfigForLua, + "buildResolvers": buildResolvers, + "buildUpstreamName": buildUpstreamName, + "isLocationInLocationList": isLocationInLocationList, + "isLocationAllowed": isLocationAllowed, + "buildLogFormatUpstream": buildLogFormatUpstream, + "buildDenyVariable": buildDenyVariable, + "getenv": os.Getenv, + "contains": strings.Contains, + "hasPrefix": strings.HasPrefix, + "hasSuffix": strings.HasSuffix, + "trimSpace": strings.TrimSpace, + "toUpper": strings.ToUpper, + "toLower": strings.ToLower, + "formatIP": formatIP, + "quote": quote, + "buildNextUpstream": buildNextUpstream, + "getIngressInformation": getIngressInformation, "serverConfig": func(all config.TemplateConfig, server *ingress.Server) interface{} { return struct{ First, Second interface{} }{all, server} }, @@ -236,7 +237,7 @@ func shouldConfigureLuaRestyWAF(disableLuaRestyWAF bool, mode string) bool { func buildLuaSharedDictionaries(c interface{}, s interface{}, disableLuaRestyWAF bool) string { var out []string - // Load config + cfg, ok := c.(config.Configuration) if !ok { klog.Errorf("expected a 'config.Configuration' type but %T was returned", c) @@ -247,20 +248,13 @@ func buildLuaSharedDictionaries(c interface{}, s interface{}, disableLuaRestyWAF klog.Errorf("expected an '[]*ingress.Server' type but %T was returned", s) return "" } - // check if config contains lua "lua_configuration_data" value otherwise, use default - cfgData, ok := cfg.LuaSharedDicts["configuration_data"] - if !ok { - cfgData = 15 - } - out = append(out, fmt.Sprintf("lua_shared_dict configuration_data %dM", cfgData)) - // check if config contains "lua_certificate_data" value otherwise, use default - certData, ok := cfg.LuaSharedDicts["certificate_data"] - if !ok { - certData = 16 + for name, size := range cfg.LuaSharedDicts { + out = append(out, fmt.Sprintf("lua_shared_dict %s %dM", name, size)) } - out = append(out, fmt.Sprintf("lua_shared_dict certificate_data %dM", certData)) - if !disableLuaRestyWAF { + + // 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 { @@ -275,7 +269,24 @@ func buildLuaSharedDictionaries(c interface{}, s interface{}, disableLuaRestyWAF out = append(out, "lua_shared_dict waf_storage 64M") } } - return strings.Join(out, ";\n\r") + ";" + + return strings.Join(out, ";\n") + ";\n" +} + +func luaConfigurationRequestBodySize(c interface{}) string { + cfg, ok := c.(config.Configuration) + if !ok { + klog.Errorf("expected a 'config.Configuration' type but %T was returned", c) + return "100" // just a default number + } + + size := cfg.LuaSharedDicts["configuration_data"] + if size < cfg.LuaSharedDicts["certificate_data"] { + size = cfg.LuaSharedDicts["certificate_data"] + } + size = size + 1 + + return fmt.Sprintf("%d", size) } // configForLua returns some general configuration as Lua table represented as string diff --git a/internal/ingress/controller/template/template_test.go b/internal/ingress/controller/template/template_test.go index 16b56a8bbb..08e07d6813 100644 --- a/internal/ingress/controller/template/template_test.go +++ b/internal/ingress/controller/template/template_test.go @@ -203,9 +203,12 @@ func TestBuildLuaSharedDictionaries(t *testing.T) { } // returns value from config configuration := buildLuaSharedDictionaries(cfg, servers, false) - if !strings.Contains(configuration, "lua_shared_dict configuration_data 10M;\n\rlua_shared_dict certificate_data 20M;") { + 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) } @@ -222,6 +225,19 @@ func TestBuildLuaSharedDictionaries(t *testing.T) { } } +func TestLuaConfigurationRequestBodySize(t *testing.T) { + cfg := config.Configuration{ + LuaSharedDicts: map[string]int{ + "configuration_data": 10, "certificate_data": 20, + }, + } + + size := luaConfigurationRequestBodySize(cfg) + if "21" != size { + t.Errorf("expected the size to be 20 but got: %v", size) + } +} + func TestFormatIP(t *testing.T) { cases := map[string]struct { Input, Output string diff --git a/rootfs/etc/nginx/template/nginx.tmpl b/rootfs/etc/nginx/template/nginx.tmpl index adc4e550c7..172fdd9c63 100755 --- a/rootfs/etc/nginx/template/nginx.tmpl +++ b/rootfs/etc/nginx/template/nginx.tmpl @@ -580,9 +580,8 @@ http { } location /configuration { - # this should be equals to configuration_data dict - client_max_body_size 10m; - client_body_buffer_size 10m; + client_max_body_size {{ luaConfigurationRequestBodySize $cfg }}m; + client_body_buffer_size {{ luaConfigurationRequestBodySize $cfg }}m; proxy_buffering off; content_by_lua_block { diff --git a/test/e2e/lua/dynamic_configuration.go b/test/e2e/lua/dynamic_configuration.go index f3903fe8ab..4b924a00a6 100644 --- a/test/e2e/lua/dynamic_configuration.go +++ b/test/e2e/lua/dynamic_configuration.go @@ -62,30 +62,6 @@ var _ = framework.IngressNginxDescribe("Dynamic Configuration", func() { }) }) - Context("Lua shared dict", func() { - It("update config", func() { - - host := "foo.com" - ingress := framework.NewSingleIngress(host, "/", host, f.Namespace, "http-svc", 80, nil) - f.EnsureIngress(ingress) - - lkey := "lua-shared-dicts" - lval := "configuration_data:100,certificate_data:200" - - By("update shared dict") - - f.UpdateNginxConfigMapData(lkey, lval) - - var nginxConfig string - f.WaitForNginxConfiguration(func(cfg string) bool { - nginxConfig = cfg - return true - }) - - Expect(strings.ContainsAny(nginxConfig, "configuration_data:100M,certificate_data:200M"), true) - }) - }) - Context("when only backends change", func() { It("handles endpoints only changes", func() { var nginxConfig string diff --git a/test/e2e/settings/lua_shared_dicts.go b/test/e2e/settings/lua_shared_dicts.go index 5ebe05041c..f00891463b 100644 --- a/test/e2e/settings/lua_shared_dicts.go +++ b/test/e2e/settings/lua_shared_dicts.go @@ -17,14 +17,13 @@ limitations under the License. package settings import ( - "strings" - . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + "k8s.io/ingress-nginx/test/e2e/framework" ) var _ = framework.IngressNginxDescribe("LuaSharedDict", func() { - f := framework.NewDefaultFramework("lua-shared-dicts") host := "lua-shared-dicts" @@ -35,13 +34,20 @@ var _ = framework.IngressNginxDescribe("LuaSharedDict", func() { AfterEach(func() { }) - It("update lua shared dict", func() { + It("configures lua shared dicts", func() { ingress := framework.NewSingleIngress(host, "/", host, f.Namespace, "http-svc", 80, nil) f.EnsureIngress(ingress) - By("update shared dict") - f.UpdateNginxConfigMapData("lua-shared-dicts", "configuration_data:123,certificate_data:456") + + f.UpdateNginxConfigMapData("lua-shared-dicts", "configuration_data:60,certificate_data:300, my_dict: 15 , invalid: 1a") + + ngxCfg := "" f.WaitForNginxConfiguration(func(cfg string) bool { - return strings.Contains(cfg, "lua_shared_dict configuration_data 123M; lua_shared_dict certificate_data 456M;") + ngxCfg = cfg + return true }) + + Expect(ngxCfg).Should(ContainSubstring("lua_shared_dict configuration_data 60M;")) + Expect(ngxCfg).Should(ContainSubstring("lua_shared_dict certificate_data 20M;")) + Expect(ngxCfg).Should(ContainSubstring("lua_shared_dict my_dict 15M;")) }) })