You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by GitBox <gi...@apache.org> on 2021/10/26 06:31:14 UTC
[GitHub] [apisix-helm-chart] tokers opened a new pull request #161: feat: support to mount 3rd plugins from configmap
tokers opened a new pull request #161:
URL: https://github.com/apache/apisix-helm-chart/pull/161
Signed-off-by: Chao Zhang <to...@apache.org>
--
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] gxthrj commented on a change in pull request #161: feat: support to mount 3rd plugins from configmap
Posted by GitBox <gi...@apache.org>.
gxthrj commented on a change in pull request #161:
URL: https://github.com/apache/apisix-helm-chart/pull/161#discussion_r737446849
##########
File path: charts/apisix/templates/deployment.yaml
##########
@@ -94,6 +103,13 @@ spec:
- configMap:
name: {{ include "apisix.fullname" . }}
name: apisix-config
+ {{- if .Values.customPlugins.enabled }}
+ {{- range $plugin := .Values.customPlugins.plugins }}
+ - name: plugin-{{ $plugin.configMap.name }}
+ configMap:
+ name: {{ $plugin.configMap.name }}
Review comment:
> put all plugins codes into the same configmap might cause the key confictions.
I mean you can use different filed names to distinguish the configuration of different plugins. The field name is here https://github.com/apache/apisix-helm-chart/blob/master/charts/apisix-ingress-controller/templates/configmap.yaml#L19
This is the same as the configmap name of each plug-in, except that the name is stored in one configmap as a key.
This has another advantage, we can write an example for users to refer to, and will not lead to flooding due to too much configmaps.
--
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] gxthrj commented on a change in pull request #161: feat: support to mount 3rd plugins from configmap
Posted by GitBox <gi...@apache.org>.
gxthrj commented on a change in pull request #161:
URL: https://github.com/apache/apisix-helm-chart/pull/161#discussion_r737438381
##########
File path: charts/apisix/templates/deployment.yaml
##########
@@ -94,6 +103,13 @@ spec:
- configMap:
name: {{ include "apisix.fullname" . }}
name: apisix-config
+ {{- if .Values.customPlugins.enabled }}
+ {{- range $plugin := .Values.customPlugins.plugins }}
+ - name: plugin-{{ $plugin.configMap.name }}
+ configMap:
+ name: {{ $plugin.configMap.name }}
Review comment:
> It's not a good idea, since keys in configmap are flat but custom plugins may have its own directory structure, only users know how to mount files to configmap
The config in configmap also can have structure. such as https://github.com/apache/apisix-helm-chart/blob/master/charts/apisix/templates/configmap.yaml#L41
--
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] nic-6443 commented on a change in pull request #161: feat: support to mount 3rd plugins from configmap
Posted by GitBox <gi...@apache.org>.
nic-6443 commented on a change in pull request #161:
URL: https://github.com/apache/apisix-helm-chart/pull/161#discussion_r736208284
##########
File path: charts/apisix/templates/deployment.yaml
##########
@@ -82,6 +82,15 @@ spec:
- mountPath: /usr/local/apisix/conf/config.yaml
name: apisix-config
subPath: config.yaml
+ {{- if .Values.customPlugins.enabled }}
+ {{- range $plugin := .Values.customPlugins.plugins }}
+ {{- range $mount := $plugin.configMap.mounts }}
+ - mountPath: {{ $mount.path }}
Review comment:
How about replace `- mountPath: {{ $mount.path }}` with `- mountPath: {{ $mount.path }}/{{ $mount.key }}`, make `mount.path` more simpler?
##########
File path: charts/apisix/templates/deployment.yaml
##########
@@ -82,6 +82,15 @@ spec:
- mountPath: /usr/local/apisix/conf/config.yaml
name: apisix-config
subPath: config.yaml
+ {{- if .Values.customPlugins.enabled }}
+ {{- range $plugin := .Values.customPlugins.plugins }}
+ {{- range $mount := $plugin.configMap.mounts }}
+ - mountPath: {{ $mount.path }}
Review comment:
`- mountPath: {{ $mount.path }}` -> `- mountPath: {{ $mount.path }}/{{ $mount.key }}`
##########
File path: charts/apisix/values.yaml
##########
@@ -178,6 +178,28 @@ stream_plugins:
- ip-restriction
- limit-conn
+# customPlugins allows you to mount your own HTTP plugins.
+customPlugins:
+ enabled: false
+ # the lua_path that tells APISIX where it can find plugins,
+ # note the last ';' is required.
+ luaPath: "/opts/custom_plugins/?.lua;"
Review comment:
The last ';' is not required, [apisix will append ';' while not found](https://github.com/api7/api7/blob/baae2c52064578e602a9abb1412b9d06e8537ed9/apisix/cli/ops.lua#L139)
--
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] nic-6443 commented on a change in pull request #161: feat: support to mount 3rd plugins from configmap
Posted by GitBox <gi...@apache.org>.
nic-6443 commented on a change in pull request #161:
URL: https://github.com/apache/apisix-helm-chart/pull/161#discussion_r736205743
##########
File path: charts/apisix/templates/deployment.yaml
##########
@@ -82,6 +82,15 @@ spec:
- mountPath: /usr/local/apisix/conf/config.yaml
name: apisix-config
subPath: config.yaml
+ {{- if .Values.customPlugins.enabled }}
+ {{- range $plugin := .Values.customPlugins.plugins }}
+ {{- range $mount := $plugin.configMap.mounts }}
+ - mountPath: {{ $mount.path }}
Review comment:
`- mountPath: {{ $mount.path }}` -> `- mountPath: {{ $mount.path }}/{{ $mount.key }}`
--
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] gxthrj commented on a change in pull request #161: feat: support to mount 3rd plugins from configmap
Posted by GitBox <gi...@apache.org>.
gxthrj commented on a change in pull request #161:
URL: https://github.com/apache/apisix-helm-chart/pull/161#discussion_r737953501
##########
File path: charts/apisix/templates/deployment.yaml
##########
@@ -94,6 +103,13 @@ spec:
- configMap:
name: {{ include "apisix.fullname" . }}
name: apisix-config
+ {{- if .Values.customPlugins.enabled }}
+ {{- range $plugin := .Values.customPlugins.plugins }}
+ - name: plugin-{{ $plugin.configMap.name }}
+ configMap:
+ name: {{ $plugin.configMap.name }}
Review comment:
I do not understand, the key is the same as the name of `configmap`, they are both flat.
--
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] tokers commented on a change in pull request #161: feat: support to mount 3rd plugins from configmap
Posted by GitBox <gi...@apache.org>.
tokers commented on a change in pull request #161:
URL: https://github.com/apache/apisix-helm-chart/pull/161#discussion_r737952387
##########
File path: charts/apisix/templates/deployment.yaml
##########
@@ -94,6 +103,13 @@ spec:
- configMap:
name: {{ include "apisix.fullname" . }}
name: apisix-config
+ {{- if .Values.customPlugins.enabled }}
+ {{- range $plugin := .Values.customPlugins.plugins }}
+ - name: plugin-{{ $plugin.configMap.name }}
+ configMap:
+ name: {{ $plugin.configMap.name }}
Review comment:
> I mean you can use different filed names to distinguish the configuration of different plugins. The field name is here https://github.com/apache/apisix-helm-chart/blob/master/charts/apisix-ingress-controller/templates/configmap.yaml#L19
Yes, we can decide the key name by ourselves, but then how did we decide the mapping relationship between the key name and the actual mount path? how can you know file should be mounted in which way, also, please know that a plugin might have multiple files and these files may be in their own directories, the organization of directories will affect how they import the codes in the plugin.
--
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] gxthrj commented on a change in pull request #161: feat: support to mount 3rd plugins from configmap
Posted by GitBox <gi...@apache.org>.
gxthrj commented on a change in pull request #161:
URL: https://github.com/apache/apisix-helm-chart/pull/161#discussion_r737446849
##########
File path: charts/apisix/templates/deployment.yaml
##########
@@ -94,6 +103,13 @@ spec:
- configMap:
name: {{ include "apisix.fullname" . }}
name: apisix-config
+ {{- if .Values.customPlugins.enabled }}
+ {{- range $plugin := .Values.customPlugins.plugins }}
+ - name: plugin-{{ $plugin.configMap.name }}
+ configMap:
+ name: {{ $plugin.configMap.name }}
Review comment:
> put all plugins codes into the same configmap might cause the key confictions.
I mean you can use different filed names to distinguish the configuration of different plugins. The field name is here https://github.com/apache/apisix-helm-chart/blob/master/charts/apisix-ingress-controller/templates/configmap.yaml#L19
This is the same as the configmap name of each plug-in, except that the name is stored in a configmap as a key.
This has another advantage, we can write an example for users to refer to, and will not lead to flooding due to too much configmaps.
--
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] gxthrj commented on a change in pull request #161: feat: support to mount 3rd plugins from configmap
Posted by GitBox <gi...@apache.org>.
gxthrj commented on a change in pull request #161:
URL: https://github.com/apache/apisix-helm-chart/pull/161#discussion_r737970515
##########
File path: charts/apisix/templates/deployment.yaml
##########
@@ -94,6 +103,13 @@ spec:
- configMap:
name: {{ include "apisix.fullname" . }}
name: apisix-config
+ {{- if .Values.customPlugins.enabled }}
+ {{- range $plugin := .Values.customPlugins.plugins }}
+ - name: plugin-{{ $plugin.configMap.name }}
+ configMap:
+ name: {{ $plugin.configMap.name }}
Review comment:
> a plugin might have multiple files and these files may be in their own directories, the organization of directories will affect how they import the codes in the plugin.
OK
--
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] tokers commented on a change in pull request #161: feat: support to mount 3rd plugins from configmap
Posted by GitBox <gi...@apache.org>.
tokers commented on a change in pull request #161:
URL: https://github.com/apache/apisix-helm-chart/pull/161#discussion_r737950297
##########
File path: charts/apisix/templates/deployment.yaml
##########
@@ -94,6 +103,13 @@ spec:
- configMap:
name: {{ include "apisix.fullname" . }}
name: apisix-config
+ {{- if .Values.customPlugins.enabled }}
+ {{- range $plugin := .Values.customPlugins.plugins }}
+ - name: plugin-{{ $plugin.configMap.name }}
+ configMap:
+ name: {{ $plugin.configMap.name }}
Review comment:
No. I mean the **key** space is flat. I don't mean the value inside a configmap.
--
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] tokers merged pull request #161: feat: support to mount 3rd plugins from configmap
Posted by GitBox <gi...@apache.org>.
tokers merged pull request #161:
URL: https://github.com/apache/apisix-helm-chart/pull/161
--
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] gxthrj commented on a change in pull request #161: feat: support to mount 3rd plugins from configmap
Posted by GitBox <gi...@apache.org>.
gxthrj commented on a change in pull request #161:
URL: https://github.com/apache/apisix-helm-chart/pull/161#discussion_r737068756
##########
File path: charts/apisix/templates/deployment.yaml
##########
@@ -94,6 +103,13 @@ spec:
- configMap:
name: {{ include "apisix.fullname" . }}
name: apisix-config
+ {{- if .Values.customPlugins.enabled }}
+ {{- range $plugin := .Values.customPlugins.plugins }}
+ - name: plugin-{{ $plugin.configMap.name }}
+ configMap:
+ name: {{ $plugin.configMap.name }}
Review comment:
If the `configMap` does not exist, APISIX will be shut down.
How about unifying the custom plug-in configuration to `charts/apisix/templates/configmap.yaml`, distinguished with different field names.
--
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] tokers commented on a change in pull request #161: feat: support to mount 3rd plugins from configmap
Posted by GitBox <gi...@apache.org>.
tokers commented on a change in pull request #161:
URL: https://github.com/apache/apisix-helm-chart/pull/161#discussion_r737952633
##########
File path: charts/apisix/templates/deployment.yaml
##########
@@ -94,6 +103,13 @@ spec:
- configMap:
name: {{ include "apisix.fullname" . }}
name: apisix-config
+ {{- if .Values.customPlugins.enabled }}
+ {{- range $plugin := .Values.customPlugins.plugins }}
+ - name: plugin-{{ $plugin.configMap.name }}
+ configMap:
+ name: {{ $plugin.configMap.name }}
Review comment:
> This has another advantage, we can write an example for users to refer to, and will not lead to flooding due to too much configmaps
It won't have too many configmaps actually.
--
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] tokers commented on a change in pull request #161: feat: support to mount 3rd plugins from configmap
Posted by GitBox <gi...@apache.org>.
tokers commented on a change in pull request #161:
URL: https://github.com/apache/apisix-helm-chart/pull/161#discussion_r737281083
##########
File path: charts/apisix/templates/deployment.yaml
##########
@@ -94,6 +103,13 @@ spec:
- configMap:
name: {{ include "apisix.fullname" . }}
name: apisix-config
+ {{- if .Values.customPlugins.enabled }}
+ {{- range $plugin := .Values.customPlugins.plugins }}
+ - name: plugin-{{ $plugin.configMap.name }}
+ configMap:
+ name: {{ $plugin.configMap.name }}
Review comment:
It's not a good idea, since keys in configmap are flat but custom plugins may have its own directory structure, only users know how to mount files to configmap, and, put all plugins codes into the same configmap might cause the key confictions.
--
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] tokers commented on a change in pull request #161: feat: support to mount 3rd plugins from configmap
Posted by GitBox <gi...@apache.org>.
tokers commented on a change in pull request #161:
URL: https://github.com/apache/apisix-helm-chart/pull/161#discussion_r737281412
##########
File path: charts/apisix/templates/deployment.yaml
##########
@@ -94,6 +103,13 @@ spec:
- configMap:
name: {{ include "apisix.fullname" . }}
name: apisix-config
+ {{- if .Values.customPlugins.enabled }}
+ {{- range $plugin := .Values.customPlugins.plugins }}
+ - name: plugin-{{ $plugin.configMap.name }}
+ configMap:
+ name: {{ $plugin.configMap.name }}
Review comment:
> If the configMap does not exist, APISIX will be shut down.
It won't cause APISIX to shut down but will cause the Pod always in the Pending state.
--
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] tokers commented on pull request #161: feat: support to mount 3rd plugins from configmap
Posted by GitBox <gi...@apache.org>.
tokers commented on pull request #161:
URL: https://github.com/apache/apisix-helm-chart/pull/161#issuecomment-951604417
@nic-6443 @gxthrj Please take a look when you have time, thanks!
--
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] tokers commented on a change in pull request #161: feat: support to mount 3rd plugins from configmap
Posted by GitBox <gi...@apache.org>.
tokers commented on a change in pull request #161:
URL: https://github.com/apache/apisix-helm-chart/pull/161#discussion_r737952898
##########
File path: charts/apisix/templates/deployment.yaml
##########
@@ -82,6 +82,15 @@ spec:
- mountPath: /usr/local/apisix/conf/config.yaml
name: apisix-config
subPath: config.yaml
+ {{- if .Values.customPlugins.enabled }}
+ {{- range $plugin := .Values.customPlugins.plugins }}
+ {{- range $mount := $plugin.configMap.mounts }}
+ - mountPath: {{ $mount.path }}
Review comment:
Well, it's same as the way that a Pod mounts a configmap: `.spec.containers[].volumeMounts[].mountPath`, the mountPath also contain the base filename.
--
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] tokers commented on a change in pull request #161: feat: support to mount 3rd plugins from configmap
Posted by GitBox <gi...@apache.org>.
tokers commented on a change in pull request #161:
URL: https://github.com/apache/apisix-helm-chart/pull/161#discussion_r737953524
##########
File path: charts/apisix/values.yaml
##########
@@ -178,6 +178,28 @@ stream_plugins:
- ip-restriction
- limit-conn
+# customPlugins allows you to mount your own HTTP plugins.
+customPlugins:
+ enabled: false
+ # the lua_path that tells APISIX where it can find plugins,
+ # note the last ';' is required.
+ luaPath: "/opts/custom_plugins/?.lua;"
Review comment:
Changed.
--
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] tokers commented on a change in pull request #161: feat: support to mount 3rd plugins from configmap
Posted by GitBox <gi...@apache.org>.
tokers commented on a change in pull request #161:
URL: https://github.com/apache/apisix-helm-chart/pull/161#discussion_r737281412
##########
File path: charts/apisix/templates/deployment.yaml
##########
@@ -94,6 +103,13 @@ spec:
- configMap:
name: {{ include "apisix.fullname" . }}
name: apisix-config
+ {{- if .Values.customPlugins.enabled }}
+ {{- range $plugin := .Values.customPlugins.plugins }}
+ - name: plugin-{{ $plugin.configMap.name }}
+ configMap:
+ name: {{ $plugin.configMap.name }}
Review comment:
> If the configMap does not exist, APISIX will be shut down.
It won't cause APISIX to shut down but will cause the Pod to always be in the Pending state, and the result of `kubectl describe` will tell people the reason.
--
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