crossplane / terrajet

Generate Crossplane Providers from any Terraform Provider
https://crossplane.io
Apache License 2.0
290 stars 38 forks source link

Cannot use Paved path as a key in Kubernetes secret #93

Closed ulucinar closed 2 years ago

ulucinar commented 2 years ago

What happened?

While trying to store sensitive data from a v1alpha1.KubernetesCluster in provider-tf-azure, after setting the writeConnectionSecretToRef field, I observed the following logs:

2021-10-07T23:33:55.924+0300    DEBUG   controller-runtime.manager.events       Warning {"object": {"kind":"KubernetesCluster","name":"example","uid":"1a8fbb21-9823-4869-8c3a-37b76bd9e1bc","apiVersion":"kubernetes.azure.tf.crossplane.io/v1alpha1","resourceVersion":"643827"}, "reason": "CannotPublishConnectionDetails", "message": "cannot create or update connection secret: cannot create object: Secret \"test-k8s\" is invalid: [data[kube_config[0].password]: Invalid value: \"kube_config[0].password\": a valid config key must consist of alphanumeric characters, '-', '_' or '.' (e.g. 'key.name',  or 'KEY_NAME',  or 'key-name', regex used for validation is '[-._a-zA-Z0-9]+'), data[kube_config[0].client_certificate]: Invalid value: \"kube_config[0].client_certificate\": a valid config key must consist of alphanumeric characters, '-', '_' or '.' (e.g. 'key.name',  or 'KEY_NAME',  or 'key-name', regex used for validation is '[-._a-zA-Z0-9]+'), data[kube_config[0].client_key]: Invalid value: \"kube_config[0].client_key\": a valid config key must consist of alphanumeric characters, '-', '_' or '.' (e.g. 'key.name',  or 'KEY_NAME',  or 'key-name', regex used for validation is '[-._a-zA-Z0-9]+'), data[kube_config[0].cluster_ca_certificate]: Invalid value: \"kube_config[0].cluster_ca_certificate\": a valid config key must consist of alphanumeric characters, '-', '_' or '.' (e.g. 'key.name',  or 'KEY_NAME',  or 'key-name', regex used for validation is '[-._a-zA-Z0-9]+')]"}

How can we reproduce it?

Using provider-tf-azure (from a local build):

apiVersion: kubernetes.azure.tf.crossplane.io/v1alpha1
kind: KubernetesCluster
metadata:
  name: example
spec:
  writeConnectionSecretToRef:
    name: example-kubeconfig
    namespace: crossplane-system
  forProvider:
    name: "kubernetesCluster1"
    location: "East US"
    resourceGroupName: "example-rg"
    dnsPrefix: "exampleaks1"
    defaultNodePool:
      - name: default
        nodeCount: 1
        vmSize: "Standard_D2_v2"
    identity:
      - type: "SystemAssigned"
    tags:
      purpose: example
  providerConfigRef:
    name: example
turkenh commented 2 years ago

Thanks for reporting @ulucinar!

Unfortunately, this case where an indexed field goes to status hence into the connection details secret was not something that I've tested end to end and indeed there is no such resource in the current subset of AWS resources:

Sensitive field "customer_key" in resource "S3ObjectCopy"
Sensitive field "kms_encryption_context" in resource "S3ObjectCopy"
Sensitive field "kms_key_id" in resource "S3ObjectCopy"
Sensitive field "source_customer_key" in resource "S3ObjectCopy"
Sensitive field "default_action[*].authenticate_oidc[*].client_secret" in resource "LbListener"
Sensitive field "action[*].authenticate_oidc[*].client_secret" in resource "LbListenerRule"
Sensitive field "master_password" in resource "RdsCluster"
Sensitive field "secret" in resource "IamAccessKey"
Sensitive field "ses_smtp_password_v4" in resource "IamAccessKey"
Sensitive field "private_key" in resource "IamServerCertificate"

I think we need some other unique representation which would still allow us to get back to original path while restoring the state back. The first solution coming to my mind is to convert kube_config[0].password to kube_config.0.password in the connection details secret. This would work except if there are keys with dots inside (not sure if there is a case for this).

ulucinar commented 2 years ago

I wouldn't expect too much trouble from Terraform attribute names, I think they are mostly alphanumeric snake case keys. Seems like we need to handle Paved syntax. We can also scan for the remaining sensitive fields of the Azure resources in the planned release.

turkenh commented 2 years ago

@ulucinar it would be great if you could verify the fix on your side 🙏