Improve secret copy + kubeconfig permissions #481
Conversation
caseydavenport
force-pushed
the
more-in-kubeconfig
branch
from
f01e774 to
c61f24f
Compare
caseydavenport
changed the title
caseydavenport
force-pushed
the
more-in-kubeconfig
branch
from
818d261 to
f6d7bdc
Compare
| @@ -29,7 +29,7 @@ if [ "$(ls ${SECRETS_MOUNT_DIR} 3>/dev/null)" ]; | |||
| then | |||
| echo "Installing any TLS assets from ${SECRETS_MOUNT_DIR}" | |||
| mkdir -p /host/etc/cni/net.d/calico-tls | |||
| cp ${SECRETS_MOUNT_DIR}/* /host/etc/cni/net.d/calico-tls/ | |||
| cp -p ${SECRETS_MOUNT_DIR}/* /host/etc/cni/net.d/calico-tls/ | |||
There was a problem hiding this comment.
I think we should have this change done on line 180 also.
k8s-install/scripts/install-cni.sh
Outdated
| cat > /host/etc/cni/net.d/calico-kubeconfig <<EOF | ||
| # Kubeconfig file for Calico CNI plugin. | ||
| apiVersion: v1 | ||
| kind: Config | ||
| clusters: | ||
| - name: local | ||
| cluster: | ||
| server: https://${KUBERNETES_SERVICE_HOST:-}:${KUBERNETES_SERVICE_PORT:-} |
There was a problem hiding this comment.
If those are not set I think this will result in server: https://:, should we have defaults for them?
Do we need to have https be configurable, would someone ever use http?
There was a problem hiding this comment.
I don't know if we can put a reasonable default here. It's all a bit awkward, but those envs really should be set whenever we're running as a k8s pod (but not say, mesos).
+1 on adding http/https configuration.
k8s-install/scripts/install-cni.sh
Outdated
| @@ -119,7 +127,6 @@ current-context: calico-context | |||
| EOF | |||
|
|
|||
| # Insert any of the supported "auto" parameters. | |||
| SERVICEACCOUNT_TOKEN=$(cat /var/run/secrets/kubernetes.io/serviceaccount/token) | |||
There was a problem hiding this comment.
The replacement below of SERVICEACCOUNT_TOKEN could be removed once it is removed from the CNI config, are we waiting until that is done to allow an easier transition. I think that makes sense, just making sure it has been considered.
There was a problem hiding this comment.
Yeah, I think we should wait a while. We should probably leave it there for compat purposes indefinitely. We can document it as deprecated if we want though.
|
@tmjd thanks for the review, I've pushed some changes, LMK what you think. |
caseydavenport
force-pushed
the
more-in-kubeconfig
branch
from
7403eba to
1889dc3
Compare
Description
A few improvements to our install CNI script:
-pto retain permissions (See https://github.com/projectcalico/cni-plugin/issues/459)calico-kubeconfigmode to0600by defaultTodos
Release Note