crossplane-contrib / provider-helm

Crossplane Helm Provider
Apache License 2.0
111 stars 65 forks source link

identity section for AzureAD auth in ProviderConfig should not be processed when the supplied kubeconfig does not require AzureAD auth #206

Open erhancagirici opened 11 months ago

erhancagirici commented 11 months ago

What happened?

provider-helm v0.16.0 (https://github.com/crossplane-contrib/provider-helm/pull/205) introduced support for AzureAD authentication for AKS clusters and lets you specify the principal to authenticate to AKS through identity section in ProviderConfig spec, when kubeconfigs relying on kubelogin are supplied via ProviderConfig. These clusters typically has kubeconfigs with execProvider section, which consists of kubelogin command and arguments.

However, when user supplies both of the following in ProviderConfig:

This causes a runtime panic, because of a nil pointer dereference. It is recovered and does not halt the execution.

2023-12-14T11:22:57.415Z    DEBUG    provider-helm    Connecting    {"request": "test-env-xkb5l-jqvkl"}                                           │
│ E1214 11:22:57.416651       1 runtime.go:79] Observed a panic: "invalid memory address or nil pointer dereference" (runtime error: invalid memory │
│  address or nil pointer dereference)                                                                                                              │
│ goroutine 105 [running]:                                                                                                                          │
│ k8s.io/apimachinery/pkg/util/runtime.logPanic({0x1f5d6c0?, 0x36745f0})                                                                            │
│     k8s.io/apimachinery@v0.28.3/pkg/util/runtime/runtime.go:75 +0x99                                                                              │
│ sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Reconcile.func1()                                                            │
│     sigs.k8s.io/controller-runtime@v0.16.3/pkg/internal/controller/controller.go:108 +0xc5                                                        │
│ panic({0x1f5d6c0, 0x36745f0})                                                                                                                     │
│     runtime/panic.go:884 +0x213                                                                                                                   │
│ github.com/crossplane-contrib/provider-helm/pkg/clients/azure.WrapRESTConfig({0x25b51c0?, 0xc0003f73b0?}, 0xc00079f200, {0xc0005ec000, 0x28a, 0x2 │
│ 8a}, {0x0?, 0x0?, 0xc0003f7dd0?})                                                                                                                 │
│     github.com/crossplane-contrib/provider-helm/pkg/clients/azure/azure.go:36 +0x1e7

By design, when such kubeconfigs are supplied, the identity section must not exist. However, such cases should be handled gracefully when users provide such configs, as reported by a community member in the Slack thread

How can we reproduce it?

The following ProviderConfig that references a "standalone" kubeconfig and a non-empty identity section with type AzureServicePrincipalCredentials

apiVersion: helm.crossplane.io/v1beta1
kind: ProviderConfig
metadata:
  name: helm
spec:
  credentials:
    source: Secret
    secretRef:
      name: helm-aks-config
      namespace: crossplane-system
      key: kubeconfig
  identity:
    type: AzureServicePrincipalCredentials
    source: Secret
    secretRef:
      name: crossplane-azure-secret
      namespace: crossplane-system
      key: creds

the kubeconfig in helm-aks-config Secret. This kubeconfig has client cert key and token for authentication and has no execProvider section

apiVersion: v1
clusters:
- cluster:
    certificate-authority-data: MASKED-BASE64string
    server: [https://aks-hostname.hcp.westeurope.azmk8s.io:443](https://aks-hostname.hcp.westeurope.azmk8s.io/)
  name: aks-ciliumenv
contexts:
- context:
    cluster: aks-ciliumenv
    user: clusterAdmin_rg-MASKED_aks-ciliumenv
  name: aks-ciliumenv
current-context: aks-ciliumenv
kind: Config
preferences: {}
users:
- name: clusterAdmin_rg-MASKED_aks-ciliumenv
  user:
    client-certificate-data: MASKED-BASE64string
    client-key-data: MASKED-BASE64string
    token: MASKED-BASE64string

What environment did it happen in?

Crossplane version: v1.13.2 provider-helm: v0.16.0