lzxuan commented on code in PR #278:
URL: https://github.com/apache/apisix-helm-chart/pull/278#discussion_r946561658


##########
charts/apisix/values.yaml:
##########
@@ -120,6 +120,14 @@ apisix:
 nameOverride: ""
 fullnameOverride: ""
 
+serviceAccount:
+  create: false  # Set to `true` to create ServiceAccount for Kubernetes 
service discovery.
+  annotations: {}
+  name: ""
+
+rbac:
+  create: false  # Set to `true` to create RBAC resources for Kubernetes 
service discovery.

Review Comment:
   I think `serviceAccount` can stay at the same section, as its behavior is 
the same as in other Helm charts. The comment in line 124 can be removed, since 
it is not strongly tied to this Kubernetes service discovery feature now; and I 
might need to update the comment on configuring `discovery.registry.kubernetes`.
   
   As for the `rbac`, is moving it to, like `discovery.kubernetesRegistryRBAC` 
better? I noticed 
[here](https://github.com/apache/apisix-helm-chart/blob/master/charts/apisix/templates/configmap.yaml#L244)
 the content of `discovery.registry.*` is directly render as part of APISIX 
config.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to