feat: add extra env/volumes/volumeMounts to implement the ability to …#513
feat: add extra env/volumes/volumeMounts to implement the ability to …#513toyhoshi wants to merge 1 commit into
Conversation
|
@venkateshjayagopal any feedback on this one? |
| projected: | ||
| defaultMode: 420 | ||
| sources: | ||
| - configMap: |
There was a problem hiding this comment.
I was really looking forward to this functionality. Nice PR! Should we set it to a secret rather than a configmap since the ca bundle can be potentially sensitive information ?
There was a problem hiding this comment.
Should we set it to a secret rather than a configmap since the ca bundle can be potentially sensitive information ?
CA Bundles cannot contain sensitive information. Certificate data is inherently public (your browser receives this from all TLS sites)
There was a problem hiding this comment.
Having said that, defaulting the module's values for this feels odd. Might be better to move the core values to an example in a doc? Or comment it out here and leave extraVolumeMounts: []
There was a problem hiding this comment.
Same comment as the manager section; shouldn't this be set to the following as the default?
extraEnv: []
extraVolumes: []
extraVolumeMounts: []| projected: | ||
| defaultMode: 420 | ||
| sources: | ||
| - configMap: |
There was a problem hiding this comment.
same note as above swapping from configMap to secret
There was a problem hiding this comment.
Should we set it to a secret rather than a configmap since the ca bundle can be potentially sensitive information ?
CA Bundles cannot contain sensitive information. Certificate data is inherently public (your browser receives this from all TLS sites)
There was a problem hiding this comment.
Shouldn't this be set to the following as the default?
extraEnv: []
extraVolumes: []
extraVolumeMounts: []
rodneyxr
left a comment
rodneyxr
left a comment
There was a problem hiding this comment.
I think the values.yaml should include empty defaults. This is a very common pattern in helm charts and don't feel examples are necessary but if they are, they should probably be commented out.
| projected: | ||
| defaultMode: 420 | ||
| sources: | ||
| - configMap: |
There was a problem hiding this comment.
Shouldn't this be set to the following as the default?
extraEnv: []
extraVolumes: []
extraVolumeMounts: []| projected: | ||
| defaultMode: 420 | ||
| sources: | ||
| - configMap: |
There was a problem hiding this comment.
Same comment as the manager section; shouldn't this be set to the following as the default?
extraEnv: []
extraVolumes: []
extraVolumeMounts: []|
|
||
| # To add your own CA set a ConfigMap with your CA (i.e. ca-bundle) | ||
| extraEnv: | ||
| - name: SSL_CERT_FILE | ||
| value: /etc/ssl/certs/ca-bundle.pem | ||
| extraVolumes: | ||
| - name: ca-bundle | ||
| projected: | ||
| defaultMode: 420 | ||
| sources: | ||
| - configMap: | ||
| name: ca-bundle | ||
| extraVolumeMounts: | ||
| - name: ca-bundle | ||
| mountPath: /etc/ssl/certs/ca-bundle.pem | ||
| subPath: ca-bundle.pem # remember that subPath field must exactly match the key name in the ConfigMap. | ||
| readOnly: true |
There was a problem hiding this comment.
| # To add your own CA set a ConfigMap with your CA (i.e. ca-bundle) | |
| extraEnv: | |
| - name: SSL_CERT_FILE | |
| value: /etc/ssl/certs/ca-bundle.pem | |
| extraVolumes: | |
| - name: ca-bundle | |
| projected: | |
| defaultMode: 420 | |
| sources: | |
| - configMap: | |
| name: ca-bundle | |
| extraVolumeMounts: | |
| - name: ca-bundle | |
| mountPath: /etc/ssl/certs/ca-bundle.pem | |
| subPath: ca-bundle.pem # remember that subPath field must exactly match the key name in the ConfigMap. | |
| readOnly: true | |
| extraEnv: [] | |
| # Additional volumes on the manager deployment | |
| extraVolumes: [] | |
| # - name: ca-bundle | |
| # configMap: | |
| # defaultMode: 420 | |
| # name: ca-bundle | |
| # Additional volumeMounts on the manager deployment | |
| extraVolumeMounts: [] | |
| # - mountPath: /etc/ssl/certs/ca-certificates.crt | |
| # name: custom-ca | |
| # subPath: ca-bundle.pem | |
| # readOnly: true |
| # To add your own CA set a ConfigMap with your CA (i.e. ca-bundle) | ||
| extraEnv: | ||
| - name: SSL_CERT_FILE | ||
| value: /etc/ssl/certs/ca-bundle.pem | ||
| extraVolumes: | ||
| - name: ca-bundle | ||
| projected: | ||
| defaultMode: 420 | ||
| sources: | ||
| - configMap: | ||
| name: ca-bundle | ||
| extraVolumeMounts: | ||
| - name: ca-bundle | ||
| mountPath: /etc/ssl/certs/ca-bundle.pem | ||
| subPath: ca-bundle.pem # remember that subPath field must exactly match the key name in the ConfigMap. | ||
| readOnly: true |
There was a problem hiding this comment.
| # To add your own CA set a ConfigMap with your CA (i.e. ca-bundle) | |
| extraEnv: | |
| - name: SSL_CERT_FILE | |
| value: /etc/ssl/certs/ca-bundle.pem | |
| extraVolumes: | |
| - name: ca-bundle | |
| projected: | |
| defaultMode: 420 | |
| sources: | |
| - configMap: | |
| name: ca-bundle | |
| extraVolumeMounts: | |
| - name: ca-bundle | |
| mountPath: /etc/ssl/certs/ca-bundle.pem | |
| subPath: ca-bundle.pem # remember that subPath field must exactly match the key name in the ConfigMap. | |
| readOnly: true | |
| extraEnv: [] | |
| # Additional volumes on the controller deployment | |
| extraVolumes: [] | |
| # - name: ca-bundle | |
| # configMap: | |
| # defaultMode: 420 | |
| # name: ca-bundle | |
| # Additional volumeMounts on the controller deployment | |
| extraVolumeMounts: [] | |
| # - mountPath: /etc/ssl/certs/ca-certificates.crt | |
| # name: custom-ca | |
| # subPath: ca-bundle.pem | |
| # readOnly: true |
4 participants
this change resolves #193 by adding extraEnv, extraVolume and extraVolumeMounts.
Signed-off by: Marco Varagnolo marco.varagnolo@gmail.com