ShiningRush commented on a change in pull request #4:
URL: https://github.com/apache/apisix-helm-chart/pull/4#discussion_r534920448
##########
File path: chart/apisix/templates/tests/test-connection.yaml
##########
@@ -18,6 +18,7 @@ apiVersion: v1
kind: Pod
metadata:
name: "{{ include "apisix.fullname" . }}-test-connection"
+ namespace: {{ .Release.Namespace }}
Review comment:
We really need this pod? I think the health check is enough.
##########
File path: chart/apisix/templates/configmap.yaml
##########
@@ -72,9 +73,14 @@ data:
# disk_path: "/tmp/disk_cache_two"
# cache_levels: "1:2"
- # allow_admin: #
http://nginx.org/en/docs/http/ngx_http_access_module.html#allow
- # - 127.0.0.0/24 # If we don't set any IP list, then any
IP access is allowed by default.
- # - 172.17.0.0/24
+ allow_admin: #
http://nginx.org/en/docs/http/ngx_http_access_module.html#allow
Review comment:
It is not a good design, we can not limit user config by predefined
parameter.
Define a values parameter names `config`, then transform it's content to
`config.yaml` would be better, it can give user full control to config.
##########
File path: chart/apisix/templates/ingress.yaml
##########
@@ -25,6 +25,7 @@ apiVersion: extensions/v1beta1
kind: Ingress
metadata:
name: {{ $fullName }}
+ namespace: {{ .Release.Namespace }}
Review comment:
`Ingress` required cluster install `ingress-controller`, k8s cluster
does not install any implement about it by default, so this CRD will not work.
##########
File path: chart/apisix/templates/service-gateway.yaml
##########
@@ -18,6 +18,7 @@ apiVersion: v1
kind: Service
Review comment:
Can this service work well?I found that there are some problems:
- it not config `selector`, it will not bind to any pods
- no ports specified, it will not proxy any traffic
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
