-
Notifications
You must be signed in to change notification settings - Fork 784
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add envFrom to container specs; sanitize comment with account info #1312
Conversation
… irsa comment in serviceaccount-csi-controller template
|
Welcome @jebbens! |
Hi @jebbens. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
/ok-to-test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you bump up the Helm chart version here and update the CHANGELOG?
Rendered Helm chart, lgtm. Thanks for your contribution.
values.yaml
controller:
# If arbitrary args like "--aws-sdk-debug-log=true" need to be passed, use this value
additionalArgs: []
affinity: {}
# The default filesystem type of the volume to provision when fstype is unspecified in the StorageClass.
# If the default is not set and fstype is unset in the StorageClass, then no fstype will be set
defaultFsType: ext4
env: []
envFrom:
- configMapRef:
name: env-configmap
# If set, add pv/pvc metadata to plugin create requests as parameters.
extraCreateMetadata: true
$ helm template ./ | grep -A 20 envFrom
envFrom:
- configMapRef:
name: env-configmap
volumeMounts:
- name: kubelet-dir
mountPath: /var/lib/kubelet
mountPropagation: "Bidirectional"
- name: plugin-dir
mountPath: /csi
- name: device-dir
mountPath: /dev
ports:
- name: healthz
containerPort: 9808
protocol: TCP
livenessProbe:
httpGet:
path: /healthz
port: healthz
initialDelaySeconds: 10
timeoutSeconds: 3
--
envFrom:
- configMapRef:
name: env-configmap
volumeMounts:
- name: plugin-dir
mountPath: /csi
- name: registration-dir
mountPath: /registration
- name: liveness-probe
image: k8s.gcr.io/sig-storage/livenessprobe:v2.5.0
imagePullPolicy: IfNotPresent
args:
- --csi-address=/csi/csi.sock
envFrom:
- configMapRef:
name: env-configmap
volumeMounts:
- name: plugin-dir
mountPath: /csi
volumes:
- name: kubelet-dir
hostPath:
path: /var/lib/kubelet
type: Directory
- name: plugin-dir
hostPath:
path: /var/lib/kubelet/plugins/ebs.csi.aws.com/
type: DirectoryOrCreate
- name: registration-dir
hostPath:
path: /var/lib/kubelet/plugins_registry/
type: Directory
- name: device-dir
hostPath:
--
envFrom:
- configMapRef:
name: env-configmap
volumeMounts:
- name: socket-dir
mountPath: /var/lib/csi/sockets/pluginproxy/
ports:
- name: healthz
containerPort: 9808
protocol: TCP
livenessProbe:
httpGet:
path: /healthz
port: healthz
initialDelaySeconds: 10
timeoutSeconds: 3
periodSeconds: 10
failureThreshold: 5
readinessProbe:
httpGet:
path: /healthz
--
envFrom:
- configMapRef:
name: env-configmap
volumeMounts:
- name: socket-dir
mountPath: /var/lib/csi/sockets/pluginproxy/
- name: csi-attacher
image: k8s.gcr.io/sig-storage/csi-attacher:v3.4.0
imagePullPolicy: IfNotPresent
args:
- --csi-address=$(ADDRESS)
- --v=2
- --leader-election=true
env:
- name: ADDRESS
value: /var/lib/csi/sockets/pluginproxy/csi.sock
envFrom:
- configMapRef:
name: env-configmap
volumeMounts:
- name: socket-dir
mountPath: /var/lib/csi/sockets/pluginproxy/
- name: csi-resizer
image: k8s.gcr.io/sig-storage/csi-resizer:v1.4.0
imagePullPolicy: IfNotPresent
args:
- --csi-address=$(ADDRESS)
- --v=2
- --handle-volume-inuse-error=false
env:
- name: ADDRESS
value: /var/lib/csi/sockets/pluginproxy/csi.sock
envFrom:
- configMapRef:
name: env-configmap
volumeMounts:
- name: socket-dir
mountPath: /var/lib/csi/sockets/pluginproxy/
- name: liveness-probe
image: k8s.gcr.io/sig-storage/livenessprobe:v2.5.0
imagePullPolicy: IfNotPresent
args:
- --csi-address=/csi/csi.sock
envFrom:
- configMapRef:
name: env-configmap
volumeMounts:
- name: socket-dir
mountPath: /csi
volumes:
- name: socket-dir
emptyDir: {}
@@ -77,6 +77,7 @@ controller: | |||
# If the default is not set and fstype is unset in the StorageClass, then no fstype will be set | |||
defaultFsType: ext4 | |||
env: [] | |||
envFrom: [] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you include a comment here for clarity? Something like "envFrom provides a way to reference configMaps across multiple resources".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added "Use envFrom to reference ConfigMaps and Secrets across all containers in the deployment"
/lgtm |
/lgtm |
/lgtm Thanks for the contribution @jebbens ! |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jebbens, torredil The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Is this a bug fix or adding new feature?
Adding envFrom property to container specs in the controller, node, and values templates. Sanitize comment related to EKS IRSA in the serviceaccount-csi-controller template.
What is this PR about? / Why do we need it?
envFrom provides a simple way to reference configMaps across multiple resources. Sanitizing the comment in the template removes sensitive AWS account information and also makes the value more generic.
What testing is done?
Deployed the AWS EBS CSI driver using Helm with these changes made to the templates and referencing a configMap with values for http/s_proxy and no_proxy, and successfully ran a test using the dynamic provisioning example.