diff --git a/docs/user-guide/nginx-configuration/configmap.md b/docs/user-guide/nginx-configuration/configmap.md index 54449df7c..850354105 100755 --- a/docs/user-guide/nginx-configuration/configmap.md +++ b/docs/user-guide/nginx-configuration/configmap.md @@ -1046,6 +1046,12 @@ For example following will set default `certificate_data` dictionary to `100M` a lua-shared-dicts: "certificate_data: 100, my_custom_plugin: 5" ``` +You can optionally set a size unit to allow for kilobyte-granularity. Allowed units are 'm' or 'k' (case-insensitive), and it defaults to MB if no unit is provided. Here is a similar example, but the `my_custom_plugin` dict is only 512KB. + +``` +lua-shared-dicts: "certificate_data: 100, my_custom_plugin: 512k" +``` + _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/nginx.go b/internal/ingress/controller/nginx.go index e5c47c9aa..11864ae6b 100644 --- a/internal/ingress/controller/nginx.go +++ b/internal/ingress/controller/nginx.go @@ -229,7 +229,7 @@ type NGINXController struct { // runningConfig contains the running configuration in the Backend runningConfig *ingress.Configuration - t ngx_template.TemplateWriter + t ngx_template.Writer resolver []net.IP diff --git a/internal/ingress/controller/template/configmap.go b/internal/ingress/controller/template/configmap.go index 93a6b430d..4f3aca444 100644 --- a/internal/ingress/controller/template/configmap.go +++ b/internal/ingress/controller/template/configmap.go @@ -19,6 +19,7 @@ package template import ( "fmt" "net" + "regexp" "strconv" "strings" "time" @@ -67,21 +68,22 @@ const ( var ( validRedirectCodes = sets.NewInt([]int{301, 302, 307, 308}...) + dictSizeRegex = regexp.MustCompile(`^(\d+)([kKmM])?$`) defaultLuaSharedDicts = map[string]int{ - "configuration_data": 20, - "certificate_data": 20, - "balancer_ewma": 10, - "balancer_ewma_last_touched_at": 10, - "balancer_ewma_locks": 1, - "certificate_servers": 5, - "ocsp_response_cache": 5, // keep this same as certificate_servers - "global_throttle_cache": 10, + "configuration_data": 20480, + "certificate_data": 20480, + "balancer_ewma": 10240, + "balancer_ewma_last_touched_at": 10240, + "balancer_ewma_locks": 1024, + "certificate_servers": 5120, + "ocsp_response_cache": 5120, // keep this same as certificate_servers + "global_throttle_cache": 10240, } defaultGlobalAuthRedirectParam = "rd" ) const ( - maxAllowedLuaDictSize = 200 + maxAllowedLuaDictSize = 204800 maxNumberOfLuaDicts = 100 ) @@ -117,18 +119,18 @@ func ReadConfig(src map[string]string) config.Configuration { v = strings.Replace(v, " ", "", -1) results := strings.SplitN(v, ":", 2) dictName := results[0] - size, err := strconv.Atoi(results[1]) - if err != nil { - klog.Errorf("Ignoring non integer value %v for Lua dictionary %v: %v.", results[1], dictName, err) + size := dictStrToKb(results[1]) + if size < 0 { + klog.Errorf("Ignoring poorly formatted value %v for Lua dictionary %v", results[1], dictName) continue } if size > maxAllowedLuaDictSize { - klog.Errorf("Ignoring %v for Lua dictionary %v: maximum size is %v.", size, dictName, maxAllowedLuaDictSize) + klog.Errorf("Ignoring %v for Lua dictionary %v: maximum size is %vk.", results[1], dictName, maxAllowedLuaDictSize) continue } if len(luaSharedDicts)+1 > maxNumberOfLuaDicts { klog.Errorf("Ignoring %v for Lua dictionary %v: can not configure more than %v dictionaries.", - size, dictName, maxNumberOfLuaDicts) + results[1], dictName, maxNumberOfLuaDicts) continue } @@ -427,3 +429,22 @@ func splitAndTrimSpace(s, sep string) []string { return values } + +func dictStrToKb(sizeStr string) int { + sizeMatch := dictSizeRegex.FindStringSubmatch(sizeStr) + if sizeMatch == nil { + return -1 + } + size, _ := strconv.Atoi(sizeMatch[1]) // validated already with regex + if sizeMatch[2] == "" || strings.ToLower(sizeMatch[2]) == "m" { + size *= 1024 + } + return size +} + +func dictKbToStr(size int) string { + if size%1024 == 0 { + return fmt.Sprintf("%dM", size/1024) + } + return fmt.Sprintf("%dK", size) +} diff --git a/internal/ingress/controller/template/configmap_test.go b/internal/ingress/controller/template/configmap_test.go index 65bb485ea..1f6ce2fe6 100644 --- a/internal/ingress/controller/template/configmap_test.go +++ b/internal/ingress/controller/template/configmap_test.go @@ -344,27 +344,32 @@ func TestLuaSharedDictsParsing(t *testing.T) { { name: "configuration_data only", entry: map[string]string{"lua-shared-dicts": "configuration_data:5"}, - expect: map[string]int{"configuration_data": 5}, + expect: map[string]int{"configuration_data": 5120}, }, { name: "certificate_data only", entry: map[string]string{"lua-shared-dicts": "certificate_data: 4"}, - expect: map[string]int{"certificate_data": 4}, + expect: map[string]int{"certificate_data": 4096}, }, { 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, "my_random_dict": 15, "another_example": 2}, + expect: map[string]int{"configuration_data": 10240, "my_random_dict": 15360, "another_example": 2048}, }, { name: "invalid size value should be ignored", - entry: map[string]string{"lua-shared-dicts": "mydict: 10, invalid_dict: 1a"}, - expect: map[string]int{"mydict": 10}, + entry: map[string]string{"lua-shared-dicts": "mydict: 10, invalid_dict: 1a, bad_mb_dict:10mb"}, + expect: map[string]int{"mydict": 10240}, }, { 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{"mydict": 10}, + entry: map[string]string{"lua-shared-dicts": "mydict: 10, invalid_dict: 201, invalid_kb: 204801k"}, + expect: map[string]int{"mydict": 10240}, + }, + { + name: "specified units are interpreted properly", + entry: map[string]string{"lua-shared-dicts": "kb_dict_a: 512k, mb_dict_a: 30m, kb_dict_b:16K, mb_dict_b:4M"}, + expect: map[string]int{"kb_dict_a": 512, "mb_dict_a": 30720, "kb_dict_b": 16, "mb_dict_b": 4096}, }, } @@ -418,3 +423,76 @@ func TestSplitAndTrimSpace(t *testing.T) { } } } + +func TestDictStrToKb(t *testing.T) { + testCases := []struct { + name string + input string + expect int + }{ + { + name: "unitless int size converted to kb", + input: "50", + expect: 51200, + }, + { + name: "lowercase k accepted", + input: "512k", + expect: 512, + }, + { + name: "uppercase K accepted", + input: "512K", + expect: 512, + }, + { + name: "lowercase m accepted", + input: "10m", + expect: 10240, + }, + { + name: "uppercase M accepted", + input: "10M", + expect: 10240, + }, + { + name: "trailing characters fail", + input: "50kb", + expect: -1, + }, + { + name: "leading characters fail", + input: " 50k", + expect: -1, + }, + } + for _, tc := range testCases { + if size := dictStrToKb(tc.input); size != tc.expect { + t.Errorf("Testing %v. Expected \"%v\" but \"%v\" was returned", tc.name, tc.expect, size) + } + } +} + +func TestDictKbToStr(t *testing.T) { + testCases := []struct { + name string + input int + expect string + }{ + { + name: "mod 1024 reports as M", + input: 5120, + expect: "5M", + }, + { + name: "non-mod 1024 reports as K", + input: 5001, + expect: "5001K", + }, + } + for _, tc := range testCases { + if sizeStr := dictKbToStr(tc.input); sizeStr != tc.expect { + t.Errorf("Testing %v. Expected \"%v\" but \"%v\" was returned", tc.name, tc.expect, sizeStr) + } + } +} diff --git a/internal/ingress/controller/template/template.go b/internal/ingress/controller/template/template.go index 522407a2f..3ba46681e 100644 --- a/internal/ingress/controller/template/template.go +++ b/internal/ingress/controller/template/template.go @@ -60,8 +60,8 @@ const ( stateComment ) -// TemplateWriter is the interface to render a template -type TemplateWriter interface { +// Writer is the interface to render a template +type Writer interface { Write(conf config.TemplateConfig) ([]byte, error) } @@ -329,7 +329,8 @@ func buildLuaSharedDictionaries(c interface{}, s interface{}) string { } for name, size := range cfg.LuaSharedDicts { - out = append(out, fmt.Sprintf("lua_shared_dict %s %dM", name, size)) + sizeStr := dictKbToStr(size) + out = append(out, fmt.Sprintf("lua_shared_dict %s %s", name, sizeStr)) } sort.Strings(out) @@ -341,16 +342,16 @@ 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 + return "100M" // just a default number } size := cfg.LuaSharedDicts["configuration_data"] if size < cfg.LuaSharedDicts["certificate_data"] { size = cfg.LuaSharedDicts["certificate_data"] } - size = size + 1 + size = size + 1024 - return fmt.Sprintf("%d", size) + return dictKbToStr(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 abe7049b0..cb2d20b9a 100644 --- a/internal/ingress/controller/template/template_test.go +++ b/internal/ingress/controller/template/template_test.go @@ -214,7 +214,7 @@ func TestBuildLuaSharedDictionaries(t *testing.T) { // config lua dict cfg := config.Configuration{ LuaSharedDicts: map[string]int{ - "configuration_data": 10, "certificate_data": 20, + "configuration_data": 10240, "certificate_data": 20480, }, } actual := buildLuaSharedDictionaries(cfg, invalidType) @@ -255,13 +255,13 @@ func TestBuildLuaSharedDictionaries(t *testing.T) { func TestLuaConfigurationRequestBodySize(t *testing.T) { cfg := config.Configuration{ LuaSharedDicts: map[string]int{ - "configuration_data": 10, "certificate_data": 20, + "configuration_data": 10240, "certificate_data": 20480, }, } size := luaConfigurationRequestBodySize(cfg) - if size != "21" { - t.Errorf("expected the size to be 20 but got: %v", size) + if size != "21M" { + t.Errorf("expected the size to be 21M but got: %v", size) } } diff --git a/rootfs/etc/nginx/template/nginx.tmpl b/rootfs/etc/nginx/template/nginx.tmpl index 4bb5fe18c..b4c91ef54 100755 --- a/rootfs/etc/nginx/template/nginx.tmpl +++ b/rootfs/etc/nginx/template/nginx.tmpl @@ -676,8 +676,8 @@ http { } location /configuration { - client_max_body_size {{ luaConfigurationRequestBodySize $cfg }}m; - client_body_buffer_size {{ luaConfigurationRequestBodySize $cfg }}m; + client_max_body_size {{ luaConfigurationRequestBodySize $cfg }}; + client_body_buffer_size {{ luaConfigurationRequestBodySize $cfg }}; proxy_buffering off; content_by_lua_block {