[capz] kubeadm bootstrap config contains raw secrets

alexeldeib commented 4 years ago

What steps did you take and what happened: Create any CAPZ cluster with CAPBK. The files object with azure.json contains a secret. In CAPBK, this probably needs to be changed so that files can be pulled from Kubernetes secrets and populated inside the CAPBK controller.

https://github.com/kubernetes-sigs/cluster-api/blob/9970a175c258395de4a601f96efb315c50db0656/bootstrap/kubeadm/controllers/kubeadmconfig_controller.go#L370

This code should likely retrieve files by secret if necessary much earlier and pass it down the pipeline, instead of passing the raw Files struct.

What did you expect to happen: No secrets should be contained in plain text in the spec of any objects.

Anything else you would like to add: This is CAPZ specific, but anyone who tries to put secrets in any sort of provisioning data will have the same problem. The fix also probably needs to be made here and not in CAPZ, which is why I opened the issue here.

We can debate which of these needs to be scrubbed, but at the very least aadClientSecret shouldn't be in raw text. The controller doesn't care about the content of these files so it's probably easier to handle sensitive content as secrets directly.

  files:
  - content: |
      {
        "cloud": "AzurePublicCloud",
        **"tenantId": "XXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX",
        "subscriptionId": "XXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX",
        "aadClientId": "XXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX",
        "aadClientSecret": "XXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX",**
        "resourceGroup": "capz-cluster",
        "securityGroupName": "capz-cluster-node-nsg",
        "location": "southcentralus",
        "vmType": "vmss",
        "vnetName": "capz-cluster-vnet",
        "vnetResourceGroup": "capz-cluster",
        "subnetName": "capz-cluster-node-subnet",
        "routeTableName": "capz-cluster-node-routetable",
        "loadBalancerSku": "standard",
        "maximumLoadBalancerRuleCount": 250,
        "useManagedIdentityExtension": false,
        "useInstanceMetadata": true
      }
    owner: root:root
    path: /etc/kubernetes/azure.json
    permissions: "0644"

Environment:

Cluster-api version: 9970a175c258395de4a601f96efb315c50db0656
Cluster-api-provider-azure version: 8fa937a970b1d513bdefd8bccf24ccf22b040736

/kind bug [One or more /area label. See https://github.com/kubernetes-sigs/cluster-api/labels?q=area for the list of labels]

cc @CecileRobertMichon @devigned

vincepri commented 4 years ago

There was https://github.com/kubernetes-sigs/cluster-api/pull/1860 open a while ago which had good progress on this issue, although there was a little back/forth, maybe we want to open a simple CAEP for it?

vincepri commented 4 years ago

I'm super +1 on getting Secret support for anything that may contain credentials or similar data

fabriziopandini commented 4 years ago

What about adding a new filed secretMounts of type []corev1.SecretVolumeSource (https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.18/#secretvolumesource-v1-core)

alexeldeib commented 4 years ago

I missed #1846, it's the request for the feature while I described it as a bug 🙃

deduping

kubernetes-sigs / cluster-api

[capz] kubeadm bootstrap config contains raw secrets #3030