You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by "flylan (via GitHub)" <gi...@apache.org> on 2023/04/13 08:30:44 UTC
[GitHub] [apisix-helm-chart] flylan opened a new pull request, #537: Optimize admin.allow_admin configuration generation logic
flylan opened a new pull request, #537:
URL: https://github.com/apache/apisix-helm-chart/pull/537
1. modify admin.allow_admin configured IP list logic to reduce the generation of duplicate IP segment configurations
2. when ingress-controller or dashboard is enabled, all private IP segments are added by default instead of 0.0.0.0/0 to improve security
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-helm-chart] tao12345666333 commented on a diff in pull request #537: Optimize admin.allow_admin configuration generation logic
Posted by "tao12345666333 (via GitHub)" <gi...@apache.org>.
tao12345666333 commented on code in PR #537:
URL: https://github.com/apache/apisix-helm-chart/pull/537#discussion_r1259138364
##########
charts/apisix/templates/configmap.yaml:
##########
@@ -287,16 +287,16 @@ data:
admin:
allow_admin: # http://nginx.org/en/docs/http/ngx_http_access_module.html#allow
+ {{- $ipList := list "0.0.0.0/0" -}}
{{- if .Values.admin.allow.ipList }}
- {{- range $ips := .Values.admin.allow.ipList }}
- - {{ $ips }}
+ {{- $ipList = .Values.admin.allow.ipList -}}
{{- end }}
- {{- else }}
- - 0.0.0.0/0
- {{- end}}
- {{- if or (index .Values "ingress-controller" "enabled") .Values.dashboard.enabled }}
- - 0.0.0.0/0
+ {{- if and (or (index .Values "ingress-controller" "enabled") .Values.dashboard.enabled) (not (has "0.0.0.0/0" $ipList)) }}
+ {{- $ipList = concat $ipList ( list "127.0.0.1/32" "10.0.0.0/8" "172.16.0.0/12" "192.168.0.0/16" ) | uniq -}}
Review Comment:
If security is a concern, we use NetworkPolicy better, right？
##########
charts/apisix/templates/configmap.yaml:
##########
@@ -287,16 +287,16 @@ data:
admin:
allow_admin: # http://nginx.org/en/docs/http/ngx_http_access_module.html#allow
+ {{- $ipList := list "0.0.0.0/0" -}}
{{- if .Values.admin.allow.ipList }}
- {{- range $ips := .Values.admin.allow.ipList }}
- - {{ $ips }}
+ {{- $ipList = .Values.admin.allow.ipList -}}
{{- end }}
- {{- else }}
- - 0.0.0.0/0
- {{- end}}
- {{- if or (index .Values "ingress-controller" "enabled") .Values.dashboard.enabled }}
- - 0.0.0.0/0
+ {{- if and (or (index .Values "ingress-controller" "enabled") .Values.dashboard.enabled) (not (has "0.0.0.0/0" $ipList)) }}
+ {{- $ipList = concat $ipList ( list "127.0.0.1/32" "10.0.0.0/8" "172.16.0.0/12" "192.168.0.0/16" ) | uniq -}}
Review Comment:
If security is a concern, we use NetworkPolicy better, right？
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-helm-chart] tao12345666333 commented on a diff in pull request #537: Optimize admin.allow_admin configuration generation logic
Posted by "tao12345666333 (via GitHub)" <gi...@apache.org>.
tao12345666333 commented on code in PR #537:
URL: https://github.com/apache/apisix-helm-chart/pull/537#discussion_r1267629894
##########
charts/apisix/templates/configmap.yaml:
##########
@@ -287,16 +287,16 @@ data:
admin:
allow_admin: # http://nginx.org/en/docs/http/ngx_http_access_module.html#allow
+ {{- $ipList := list "0.0.0.0/0" -}}
{{- if .Values.admin.allow.ipList }}
- {{- range $ips := .Values.admin.allow.ipList }}
- - {{ $ips }}
+ {{- $ipList = .Values.admin.allow.ipList -}}
{{- end }}
- {{- else }}
- - 0.0.0.0/0
- {{- end}}
- {{- if or (index .Values "ingress-controller" "enabled") .Values.dashboard.enabled }}
- - 0.0.0.0/0
+ {{- if and (or (index .Values "ingress-controller" "enabled") .Values.dashboard.enabled) (not (has "0.0.0.0/0" $ipList)) }}
+ {{- $ipList = concat $ipList ( list "127.0.0.1/32" "10.0.0.0/8" "172.16.0.0/12" "192.168.0.0/16" ) | uniq -}}
Review Comment:
And in some companies that I know of, there are indeed cases where non-private network segments are used as private networks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-helm-chart] flylan closed pull request #537: Optimize admin.allow_admin configuration generation logic
Posted by "flylan (via GitHub)" <gi...@apache.org>.
flylan closed pull request #537: Optimize admin.allow_admin configuration generation logic
URL: https://github.com/apache/apisix-helm-chart/pull/537
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-helm-chart] Sn0rt commented on a diff in pull request #537: Optimize admin.allow_admin configuration generation logic
Posted by "Sn0rt (via GitHub)" <gi...@apache.org>.
Sn0rt commented on code in PR #537:
URL: https://github.com/apache/apisix-helm-chart/pull/537#discussion_r1267429589
##########
charts/apisix/templates/configmap.yaml:
##########
@@ -287,16 +287,16 @@ data:
admin:
allow_admin: # http://nginx.org/en/docs/http/ngx_http_access_module.html#allow
+ {{- $ipList := list "0.0.0.0/0" -}}
{{- if .Values.admin.allow.ipList }}
- {{- range $ips := .Values.admin.allow.ipList }}
- - {{ $ips }}
+ {{- $ipList = .Values.admin.allow.ipList -}}
{{- end }}
- {{- else }}
- - 0.0.0.0/0
- {{- end}}
- {{- if or (index .Values "ingress-controller" "enabled") .Values.dashboard.enabled }}
- - 0.0.0.0/0
+ {{- if and (or (index .Values "ingress-controller" "enabled") .Values.dashboard.enabled) (not (has "0.0.0.0/0" $ipList)) }}
+ {{- $ipList = concat $ipList ( list "127.0.0.1/32" "10.0.0.0/8" "172.16.0.0/12" "192.168.0.0/16" ) | uniq -}}
Review Comment:
@flylan PTAL
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-helm-chart] flylan commented on pull request #537: Optimize admin.allow_admin configuration generation logic
Posted by "flylan (via GitHub)" <gi...@apache.org>.
flylan commented on PR #537:
URL: https://github.com/apache/apisix-helm-chart/pull/537#issuecomment-1506564814
relate：https://github.com/apache/apisix-helm-chart/issues/536
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-helm-chart] flylan commented on a diff in pull request #537: Optimize admin.allow_admin configuration generation logic
Posted by "flylan (via GitHub)" <gi...@apache.org>.
flylan commented on code in PR #537:
URL: https://github.com/apache/apisix-helm-chart/pull/537#discussion_r1270187707
##########
charts/apisix/templates/configmap.yaml:
##########
@@ -287,16 +287,16 @@ data:
admin:
allow_admin: # http://nginx.org/en/docs/http/ngx_http_access_module.html#allow
+ {{- $ipList := list "0.0.0.0/0" -}}
{{- if .Values.admin.allow.ipList }}
- {{- range $ips := .Values.admin.allow.ipList }}
- - {{ $ips }}
+ {{- $ipList = .Values.admin.allow.ipList -}}
{{- end }}
- {{- else }}
- - 0.0.0.0/0
- {{- end}}
- {{- if or (index .Values "ingress-controller" "enabled") .Values.dashboard.enabled }}
- - 0.0.0.0/0
+ {{- if and (or (index .Values "ingress-controller" "enabled") .Values.dashboard.enabled) (not (has "0.0.0.0/0" $ipList)) }}
+ {{- $ipList = concat $ipList ( list "127.0.0.1/32" "10.0.0.0/8" "172.16.0.0/12" "192.168.0.0/16" ) | uniq -}}
Review Comment:
Indeed, some companies may directly use public IP as the K8S networking environment, such as when multiple subsidiaries are distributed in different places
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org