Skip to content
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

AWS VPC CNI Chart updates for 1.7.3 release #253

Merged
merged 10 commits into from
Sep 28, 2020
4 changes: 2 additions & 2 deletions stable/aws-vpc-cni/Chart.yaml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
apiVersion: v1
name: aws-vpc-cni
version: 1.0.9
appVersion: "v1.6.3"
version: 1.1.0
appVersion: "v1.7.3"
description: A Helm chart for the AWS VPC CNI
icon: https://raw.githubusercontent.com/aws/eks-charts/master/docs/logo/aws.png
home: https://github.com/aws/amazon-vpc-cni-k8s
Expand Down
14 changes: 11 additions & 3 deletions stable/aws-vpc-cni/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,24 +29,32 @@ The following table lists the configurable parameters for this chart and their d
| Parameter | Description | Default |
| ------------------------|---------------------------------------------------------|-------------------------------------|
| `affinity` | Map of node/pod affinities | `{}` |
| `env` | List of environment variables. See [here](https://github.com/aws/amazon-vpc-cni-k8s#cni-configuration-variables) for options | `[AWS_VPC_K8S_CNI_LOGLEVEL: DEBUG, AWS_VPC_K8S_CNI_VETHPREFIX: eni, AWS_VPC_ENI_MTU: "9001"]` |
| `env` | List of environment variables. See [here](https://github.com/aws/amazon-vpc-cni-k8s#cni-configuration-variables) for options | (see `values.yaml`) |
| `fullnameOverride` | Override the fullname of the chart | `aws-node` |
| `image.region` | ECR repository region to use. Should match your cluster | `us-west-2` |
| `image.tag` | Image tag | `v1.6.3` |
| `image.tag` | Image tag | `v1.7.3` |
| `image.pullPolicy` | Container pull policy | `IfNotPresent` |
| `image.override` | A custom docker image to use | `nil` |
| `imagePullSecrets` | Docker registry pull secret | `[]` |
| `init.image.region` | ECR repository region to use. Should match your cluster | `us-west-2` |
| `init.image.tag` | Image tag | `v1.7.3` |
| `init.image.pullPolicy` | Container pull policy | `IfNotPresent` |
| `init.image.override` | A custom docker image to use | `nil` |
| `init.env` | List of init container environment variables. See [here](https://github.com/aws/amazon-vpc-cni-k8s#cni-configuration-variables) for options | (see `values.yaml`) |
| `init.securityContext` | Init container Security context | `privileged: true` |
| `originalMatchLabels` | Use the original daemonset matchLabels | `false` |
| `nameOverride` | Override the name of the chart | `aws-node` |
| `nodeSelector` | Node labels for pod assignment | `{}` |
| `podSecurityContext` | Pod Security Context | `{}` |
| `podAnnotations` | annotations to add to each pod | `{}` |
| `priorityClassName` | Name of the priorityClass | `system-node-critical` |
| `resources` | Resources for the pods | `requests.cpu: 10m` |
| `securityContext` | Container Security context | `privileged: true` |
| `securityContext` | Container Security context | `capabilities: add: - "NET_ADMIN"` |
| `serviceAccount.name` | The name of the ServiceAccount to use | `nil` |
| `serviceAccount.create` | Specifies whether a ServiceAccount should be created | `true` |
| `serviceAccount.annotations` | Specifies the annotations for ServiceAccount | `{}` |
| `livenessProbe` | Livenness probe settings for daemonset | (see `values.yaml`) |
| `readinessProbe` | Readiness probe settings for daemonset | (see `values.yaml`) |
| `crd.create` | Specifies whether to create the VPC-CNI CRD | `true` |
| `tolerations` | Optional deployment tolerations | `[]` |
| `updateStrategy` | Optional update strategy | `type: RollingUpdate` |
Expand Down
13 changes: 7 additions & 6 deletions stable/aws-vpc-cni/templates/clusterrole.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -8,17 +8,18 @@ rules:
- apiGroups:
- crd.k8s.amazonaws.com
resources:
- "*"
- namespaces
verbs:
- "*"
- eniconfigs
verbs: ["list", "watch", "get"]
- apiGroups: [""]
resources:
- pods
- nodes
- namespaces
verbs: ["list", "watch", "get"]
- apiGroups: [""]
resources:
- nodes
verbs: ["list", "watch", "get", "update"]
- apiGroups: ["extensions"]
resources:
- daemonsets
- '*'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are there any specific resources?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this change was made to match the config in the CNI repo, v1.7/aws-k8s-cni.yaml. The change was added in aws/amazon-vpc-cni-k8s#1082 ping @jayanthvn

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we know for sure which resources are needed, we can update the helm chart independently of the cni repo. By specifying * we might be allowing more than necessary access.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It has been a while since I've had to write policies and not entirely sure where to get the entire list outside of kubectl cache but here are the resources I came up with for the extensions api group:

DaemonSet
Deployment
DeploymentRollback
Ingress
NetworkPolicy
PodSecurityPolicy
ReplicaSet
Scale

I might go try to figure out what ipamd change sparked the new resource requirement from that pr.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Kishorb @mogren I wasn't able to figure out what specific resources inside extension would be required outside of DaemonSet though I do know there are deprecation changes post 1.16 to the extensions group. I see my changes proposed here as orthogonal to the granularity of clusterrole policies as they represent upstream definitions more closely related to the vpc cni project. I do however agree with the least privileged approach and think this should be figured out rather soon. I'm not sure what other help I can provide, please let me know!

Copy link
Contributor

@mogren mogren Sep 28, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kishorj I agree that we should scope this down, but I guess for now it would be ok that the Helm chart is matching the current CNI config sample. That said, I opened aws/amazon-vpc-cni-k8s#1232 to address this in the CNI repo, and we should follow up with a PR here to match.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sounds good.

verbs: ["list", "watch"]
86 changes: 52 additions & 34 deletions stable/aws-vpc-cni/templates/daemonset.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ spec:
operator: In
values:
- amd64
- arm64
- key: "eks.amazonaws.com/compute-type"
operator: NotIn
values:
Expand All @@ -55,13 +56,28 @@ spec:
operator: In
values:
- amd64
- arm64
- key: "eks.amazonaws.com/compute-type"
operator: NotIn
values:
- fargate
serviceAccountName: {{ template "aws-vpc-cni.serviceAccountName" . }}
automountServiceAccountToken: true
hostNetwork: true
initContainers:
- name: aws-vpc-cni-init
image: "{{- if .Values.init.image.override }}{{- .Values.init.image.override }}{{- else }}602401143452.dkr.ecr.{{- .Values.init.image.region }}.amazonaws.com/amazon-k8s-cni-init:{{- .Values.init.image.tag }}{{- end}}"
imagePullPolicy: {{ .Values.init.image.pullPolicy }}
env:
{{- range $key, $value := .Values.init.env }}
- name: {{ $key }}
value: {{ $value | quote }}
{{- end }}
securityContext:
{{- toYaml .Values.init.securityContext | nindent 12 }}
volumeMounts:
- mountPath: /host/opt/cni/bin
name: cni-bin-dir
terminationGracePeriodSeconds: 10
tolerations:
- operator: Exists
{{- with .Values.imagePullSecrets }}
Expand All @@ -77,14 +93,10 @@ spec:
ports:
- containerPort: 61678
name: metrics
readinessProbe:
exec:
command: ["/app/grpc-health-probe", "-addr=:50051"]
initialDelaySeconds: 35
livenessProbe:
exec:
command: ["/app/grpc-health-probe", "-addr=:50051"]
initialDelaySeconds: 35
{{ toYaml .Values.livenessProbe | indent 12 }}
readinessProbe:
{{ toYaml .Values.readinessProbe | indent 12 }}
env:
{{- range $key, $value := .Values.env }}
- name: {{ $key }}
Expand All @@ -99,33 +111,39 @@ spec:
securityContext:
{{- toYaml .Values.securityContext | nindent 12 }}
volumeMounts:
- mountPath: /host/opt/cni/bin
name: cni-bin-dir
- mountPath: /host/etc/cni/net.d
name: cni-net-dir
- mountPath: /host/var/log
name: log-dir
- mountPath: /var/run/docker.sock
name: dockersock
- mountPath: /var/run/dockershim.sock
name: dockershim
- mountPath: /host/opt/cni/bin
name: cni-bin-dir
- mountPath: /host/etc/cni/net.d
name: cni-net-dir
- mountPath: /host/var/log/aws-routed-eni
name: log-dir
- mountPath: /var/run/dockershim.sock
name: dockershim
- mountPath: /var/run/aws-node
name: run-dir
- mountPath: /run/xtables.lock
name: xtables-lock
volumes:
- name: cni-bin-dir
hostPath:
path: /opt/cni/bin
- name: cni-net-dir
hostPath:
path: /etc/cni/net.d
- name: log-dir
hostPath:
path: /var/log
- name: dockersock
hostPath:
path: /var/run/docker.sock
- name: dockershim
hostPath:
path: /var/run/dockershim.sock

- name: cni-bin-dir
hostPath:
path: /opt/cni/bin
- name: cni-net-dir
hostPath:
path: /etc/cni/net.d
- name: dockershim
hostPath:
path: /var/run/dockershim.sock
- name: log-dir
hostPath:
path: /var/log/aws-routed-eni
type: DirectoryOrCreate
- name: run-dir
hostPath:
path: /var/run/aws-node
type: DirectoryOrCreate
- name: xtables-lock
hostPath:
path: /run/xtables.lock
{{- with .Values.nodeSelector }}
nodeSelector:
{{- toYaml . | nindent 8 }}
Expand Down
49 changes: 46 additions & 3 deletions stable/aws-vpc-cni/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -6,19 +6,44 @@
# existing naming
nameOverride: aws-node

init:
image:
tag: v1.7.3
region: us-west-2
pullPolicy: Always
# Set to use custom image
# override: "repo/org/image:tag"
env:
DISABLE_TCP_EARLY_DEMUX: "false"
securityContext:
privileged: true

image:
region: us-west-2
tag: v1.6.3
tag: v1.7.3
pullPolicy: Always
# Set to use custom image
# override: "repo/org/image:tag"

# The CNI supports a number of environment variable settings
# See https://github.com/aws/amazon-vpc-cni-k8s#cni-configuration-variables
env:
ADDITIONAL_ENI_TAGS: "{}"
AWS_VPC_CNI_NODE_PORT_SUPPORT: "true"
AWS_VPC_ENI_MTU: "9001"
AWS_VPC_K8S_CNI_CONFIGURE_RPFILTER: "false"
AWS_VPC_K8S_CNI_CUSTOM_NETWORK_CFG: "false"
AWS_VPC_K8S_CNI_EXTERNALSNAT: "false"
AWS_VPC_K8S_CNI_LOG_FILE: "/host/var/log/aws-routed-eni/ipamd.log"
AWS_VPC_K8S_CNI_LOGLEVEL: DEBUG
AWS_VPC_K8S_CNI_RANDOMIZESNAT: "prng"
AWS_VPC_K8S_CNI_VETHPREFIX: eni
AWS_VPC_ENI_MTU: "9001"
AWS_VPC_K8S_PLUGIN_LOG_FILE: "/var/log/aws-routed-eni/plugin.log"
AWS_VPC_K8S_PLUGIN_LOG_LEVEL: DEBUG
DISABLE_INTROSPECTION: "false"
DISABLE_METRICS: "false"
ENABLE_POD_ENI: "false"
WARM_ENI_TARGET: "1"

# this flag enables you to use the match label that was present in the original daemonset deployed by EKS
# You can then annotate and label the original aws-node resources and 'adopt' them into a helm release
Expand All @@ -35,7 +60,9 @@ podSecurityContext: {}
podAnnotations: {}

securityContext:
privileged: true
capabilities:
add:
- "NET_ADMIN"

crd:
create: true
Expand All @@ -49,12 +76,28 @@ serviceAccount:
annotations: {}
# eks.amazonaws.com/role-arn: arn:aws:iam::AWS_ACCOUNT_ID:role/IAM_ROLE_NAME

livenessProbe:
exec:
command:
- /app/grpc-health-probe
- '-addr=:50051'
initialDelaySeconds: 60

readinessProbe:
exec:
command:
- /app/grpc-health-probe
- '-addr=:50051'
initialDelaySeconds: 1

resources:
requests:
cpu: 10m

updateStrategy:
type: RollingUpdate
rollingUpdate:
maxUnavailable: "10%"

nodeSelector: {}

Expand Down