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

feat(ldap): Support reading bind username and password from secret #149

Conversation

jsirianni
Copy link
Member

@jsirianni jsirianni commented Aug 20, 2024

Description of Changes

Changes

  • Fixed an issue where basic auth username and password is expected even when LDAP is selected for auth
  • Added three new options for reading LDAP / AD bind username and password from K8s secret.

New Options

| Key | Type | Default | Description |
|-----|------|---------|-------------|
| auth.ldap.bindCredentialSecret.name | string | `""` | Kubernetes secret name that contains the bind username and password. |
| auth.ldap.bindCredentialSecret.passwordKey | string | `""` | The secret's subPath which contains the bind password. |
| auth.ldap.bindCredentialSecret.usernameKey | string | `""` | The secret's subPath which contains the bind username. |

When .Values.auth.ldap.bindCredentialSecret.name is set, it is assumed that auth.ldap.bindCredentialSecret.passwordKey and auth.ldap.bindCredentialSecret.usernameKey are also set. Instead of reading the plain text credentials, the environment variables will be set by reading from the k8s secret.

Testing

Fire up minikube. I always delete and re-create. Up to you.

minikube delete
minikube start

First, deploy the chart using the existing options. We will update to the secret after.

  1. Create the license secret
kubectl create secret generic bindplane \
  --from-literal=license=$BINDPLANE_LICENSE
  1. Create values.yaml, Reach out to me for a values file as it contains sensitive values related to our ldap server.
  2. Connect to our VPN to access our dev LDAP server
  3. Deploy the Helm chart
helm upgrade \
  --install bindplane \
  ./charts/bindplane \
  --values values.yaml

Once the pods are ready, try logging into the server.

  1. kubectl port-forward bindplane-0 3011:3001
  2. http://localhost:3011

If it is working, try switching to a secret. Use the secret command I gave you, it will look similar to this but contain the real credentials for the dev ldap server.

kubectl create secret generic bindplane-ldap-bind \
  --from-literal=username=REDACTED \
  --from-literal=password='REDACTED'

Update the values.yaml file to use a secret.

...
auth:
...
  ldap:
...
    bindCredentialSecret:
      name: bindplane-ldap-bind
      usernameKey: username
      passwordKey: password
...

Update the deployment. You should see the pods cycle.

helm upgrade \
  --install bindplane \
  ./charts/bindplane \
  --values values.yaml

Once the new BindPlane pod is running, inspect its environment and make sure it is referencing a secret for BINDPLANE_LDAP_BIND_USER and BINDPLANE_LDAP_BIND_PASSWORD.

kubectl get pod bindplane-0 \
  -o json | jq ".spec.containers[0].env"  

It should look like this

  {
    "name": "BINDPLANE_LDAP_BIND_USER",
    "valueFrom": {
      "secretKeyRef": {
        "key": "username",
        "name": "bindplane-ldap-bind",
        "optional": false
      }
    }
  },
  {
    "name": "BINDPLANE_LDAP_BIND_PASSWORD",
    "valueFrom": {
      "secretKeyRef": {
        "key": "password",
        "name": "bindplane-ldap-bind",
        "optional": false
      }
    }
  },

Log into BindPlane again using the port-forwarding command and http://localhost:3011.

Please check that the PR fulfills these requirements

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)
  • CI passes
  • Changes to ports, services, or other networking have been tested with istio

@jsirianni jsirianni marked this pull request as ready for review August 20, 2024 14:59
@jsirianni jsirianni requested a review from tbm48813 as a code owner August 20, 2024 14:59
@jsirianni jsirianni requested a review from algchoo August 20, 2024 14:59
@jsirianni jsirianni force-pushed the joesirianni/bpop-612-helm-ldap-read-bind-credentials-from-secret branch from f4546ad to cc6ce67 Compare August 26, 2024 17:37
@jsirianni jsirianni merged commit 7731313 into main Aug 26, 2024
19 checks passed
@jsirianni jsirianni deleted the joesirianni/bpop-612-helm-ldap-read-bind-credentials-from-secret branch August 26, 2024 18:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants