Add nginx-status-allow-cidrs and service externalIPs to Helm chart
#398
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* Add nginxStatusAllowCidrs cli argument to deployment template * Add externalIPs to service template
* Add Note about nginxStatusAllowCidrs cli argument * Add Note about service externalIPs
Dean-Coakley
added
the
enhancement
isaachawley
approved these changes
Oct 16, 2018
pleshakov
suggested changes
Oct 18, 2018
deployments/helm-chart/README.md
Outdated
| `controller.nginxStatus.port` | Set the port where the NGINX stub_status or the NGINX Plus API is exposed. | 8080 | ||
| `controller.reportIngressStatus.enable` | Update the address field in the status of Ingresses resources with an external address of the Ingress controller. You must also specify the source of the external address either through an external service via `controller.reportIngressStatus.externalService` or the `external-status-address` entry in the ConfigMap via `controller.config.entries`. **Note:** `controller.config.entries.external-status-address` takes precedence if both are set. | true | ||
| `controller.reportIngressStatus.externalService` | Specifies the name of the service with the type LoadBalancer through which the Ingress controller is exposed externally. The external address of the service is used when reporting the status of Ingress resources. `controller.reportIngressStatus.enable` must be set to `true`. | nginx-ingress | ||
| `controller.reportIngressStatus.enableLeaderElection` | Enable Leader election to avoid multiple replicas of the controller reporting the status of Ingress resources. `controller.reportIngressStatus.enable` must be set to `true`. | true | ||
| `controller.nginxStatusAllowCidrs` | Whitelist IPv4 IP/CIDR blocks to allow access to NGINX stub_status or the NGINX Plus API. Separate multiple IP/CIDR by commas. | 127.0.0.1 |
There was a problem hiding this comment.
were you able to successfully pass multiple IP/CIDRs? For example, "192.168.1.3,10.0.0.0/8"
There was a problem hiding this comment.
Yeah, this example appears in the manifest in this format:
-nginx-status-allow-cidrs=192.168.1.3,10.0.0.0/8
nginxStatusAllowCidrs -> nginxStatus.allowCidrs * Update Helm documentation to match allowCidrs key change * Add missing template value to daemonset template * Update values to match allowCidrs key change
pleshakov
suggested changes
Oct 22, 2018
| @@ -80,6 +80,9 @@ spec: | |||
| - -external-service={{ .Values.controller.reportIngressStatus.externalService }} | |||
| - -enable-leader-election={{ .Values.controller.reportIngressStatus.enableLeaderElection }} | |||
| {{- end }} | |||
| {{- if .Values.controller.nginxStatus.allowCidrs }} | |||
There was a problem hiding this comment.
please move it above, so it depends on {{- if .Values.controller.reportIngressStatus.enable }}
no need for a separate condition for {{- if .Values.controller.nginxStatus.allowCidrs }}
There was a problem hiding this comment.
same for controller-deployment.yaml
pleshakov
approved these changes
Oct 24, 2018
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Proposed changes
nginx-status-allow-cidrscli argument support to helm chart. Implemented in: Add CLI option-nginx-status-allow-cidrsto allow easily restricting access to sensitive endpoints via CLI argument. #387Checklist
Before creating a PR, run through this checklist and mark each as complete.