Various LDAP fixes (#1209)

- update: `examples/kustomization/tenant-external-idp-ldap` deployment
  example to use LDAP Lookup-Bind mode
- fix: create buckets during tenant creation when LDAP is enabled
- fix: tenant stuck in provisioning users during tenant creation when
  LDAP is enabled and tenant configuration is readed from configuration
  file

Signed-off-by: Lenin Alevski <alevsk.8772@gmail.com>

examples/kustomization/tenant-external-idp-ldap/openldap.yaml

@@ -1,44 +1,4 @@
 ---
-apiVersion: v1
-kind: ConfigMap
-metadata:
-  name: billy-ldif
-data:
-  billy.ldif: |
-    # LDIF fragment to create group branch under root
-    dn: uid=billy,dc=example,dc=org
-    uid: billy
-    cn: billy
-    sn: 3
-    objectClass: top
-    objectClass: posixAccount
-    objectClass: inetOrgPerson
-    loginShell: /bin/bash
-    homeDirectory: /home/billy
-    uidNumber: 14583102
-    gidNumber: 14564100
-    userPassword: billy123
-    mail: billy@example.org
-    gecos: Billy User
-
-    # Create base group
-    dn: ou=groups,dc=example,dc=org
-    objectclass:organizationalunit
-    ou: groups
-    description: generic groups branch
-
-    # create consoleAdmin group (this already exists on minio and have a policy of s3::*)
-    dn: cn=consoleAdmin,ou=groups,dc=example,dc=org
-    objectClass: top
-    objectClass: posixGroup
-    gidNumber: 678
-
-    # Assing group to new user
-    dn: cn=consoleAdmin,ou=groups,dc=example,dc=org
-    changetype: modify
-    add: memberuid
-    memberuid: billy
----
 apiVersion: apps/v1
 kind: Deployment
 metadata:
@@ -53,23 +13,22 @@ spec:
       labels:
         app: openldap
     spec:
-      volumes:
-        - name: billy-configuration
-          configMap:
-            name: billy-ldif
       containers:
         - name: openldap
-          image: osixia/openldap:1.3.0
+          image: quay.io/minio/openldap:latest
+          env:
+            - name: LDAP_ORGANIZATION
+              value: "MinIO Inc."
+            - name: LDAP_DOMAIN
+              value: "min.io"
+            - name: LDAP_ADMIN_PASSWORD
+              value: "admin"
           imagePullPolicy: "IfNotPresent"
           ports:
             - containerPort: 389
               name: tcp-ldap
             - containerPort: 636
               name: tcp-ldap2
-          volumeMounts:
-            - name: billy-configuration
-              mountPath: /tmp/billy.ldif
-              subPath: billy.ldif
 ---
 apiVersion: v1
 kind: Service

examples/kustomization/tenant-external-idp-ldap/storage-user.yaml

@@ -1,6 +1,6 @@
 apiVersion: v1
 data:
-  CONSOLE_ACCESS_KEY: dWlkPWJpbGx5LGRjPWV4YW1wbGUsZGM9b3Jn # "uid=billy,dc=example,dc=org"
+  CONSOLE_ACCESS_KEY: dWlkPWRpbGxvbixvdT1wZW9wbGUsb3U9c3dlbmdnLGRjPW1pbixkYz1pbw== # "uid=dillon,ou=people,ou=swengg,dc=min,dc=io"
   CONSOLE_SECRET_KEY: ""
 kind: Secret
 metadata:

examples/kustomization/tenant-external-idp-ldap/tenant.yaml

@@ -10,11 +10,17 @@ spec:
   env:
     - name: MINIO_IDENTITY_LDAP_SERVER_ADDR
       value: "openldap.tenant-external-idp-ldap.svc.cluster.local:389"
-    - name: MINIO_IDENTITY_LDAP_USERNAME_FORMAT
-      value: "uid=%s,dc=example,dc=org"
-    - name: MINIO_IDENTITY_LDAP_USERNAME_SEARCH_FILTER
-      value: "(|(objectclass=posixAccount)(uid=%s))"
-    - name: MINIO_IDENTITY_LDAP_TLS_SKIP_VERIFY
-      value: "on"
+    - name: MINIO_IDENTITY_LDAP_LOOKUP_BIND_DN
+      value: "cn=admin,dc=min,dc=io"
+    - name: MINIO_IDENTITY_LDAP_LOOKUP_BIND_PASSWORD
+      value: "admin"
+    - name: MINIO_IDENTITY_LDAP_USER_DN_SEARCH_BASE_DN
+      value: "dc=min,dc=io"
+    - name: MINIO_IDENTITY_LDAP_USER_DN_SEARCH_FILTER
+      value: "(uid=%s)"
+    - name: MINIO_IDENTITY_LDAP_GROUP_SEARCH_BASE_DN
+      value: "ou=swengg,dc=min,dc=io"
+    - name: MINIO_IDENTITY_LDAP_GROUP_SEARCH_FILTER
+      value: "(&(objectclass=groupOfNames)(member=%d))"
     - name: MINIO_IDENTITY_LDAP_SERVER_INSECURE
       value: "on"

pkg/apis/minio.min.io/v2/helper.go

@@ -736,49 +736,26 @@ func (t *Tenant) getMinIOTenantDetails(address string, minioSecret map[string][]
 }
 
 // NewMinIOUser initializes a new console user
-func (t *Tenant) NewMinIOUser(userCredentialSecrets []*corev1.Secret, tr *http.Transport) (*minio.Client, error) {
-	return t.NewMinIOUserForAddress("", userCredentialSecrets, tr)
+func (t *Tenant) NewMinIOUser(minioSecret map[string][]byte, tr *http.Transport) (*minio.Client, error) {
+	return t.NewMinIOUserForAddress("", minioSecret, tr)
 }
 
 // NewMinIOUserForAddress initializes a new console user
-func (t *Tenant) NewMinIOUserForAddress(address string, userCredentialSecrets []*corev1.Secret, tr *http.Transport) (*minio.Client, error) {
-	host := address
-	if host == "" {
-		host = t.MinIOServerHostAddress()
-		if host == "" {
-			return nil, errors.New("MinIO server host is empty")
-		}
+func (t *Tenant) NewMinIOUserForAddress(address string, minioSecret map[string][]byte, tr *http.Transport) (*minio.Client, error) {
+	host, accessKey, secretKey, err := t.getMinIOTenantDetails(address, minioSecret)
+	if err != nil {
+		return nil, err
 	}
-
-	for _, cred := range userCredentialSecrets {
-		consoleAccessKey, ok := cred.Data["CONSOLE_ACCESS_KEY"]
-		if !ok {
-			return nil, errors.New("CONSOLE_ACCESS_KEY not provided")
-		}
-		// remove spaces and line breaks from access key
-		userAccessKey := strings.TrimSpace(string(consoleAccessKey))
-		consoleSecretKey, ok := cred.Data["CONSOLE_SECRET_KEY"]
-		// remove spaces and line breaks from secret key
-		userSecretKey := strings.TrimSpace(string(consoleSecretKey))
-		if !ok {
-			return nil, errors.New("CONSOLE_SECRET_KEY not provided")
-		}
-
-		opts := &minio.Options{
-			Transport: tr,
-			Secure:    t.TLS(),
-			Creds:     credentials.NewStaticV4(userAccessKey, userSecretKey, ""),
-		}
-
-		minioClnt, err := minio.New(host, opts)
-		if err != nil {
-			return nil, err
-		}
-
-		return minioClnt, nil
+	opts := &minio.Options{
+		Transport: tr,
+		Secure:    t.TLS(),
+		Creds:     credentials.NewStaticV4(string(accessKey), string(secretKey), ""),
 	}
-
-	return nil, errors.New("no user credentials specified to initialize")
+	minioClient, err := minio.New(host, opts)
+	if err != nil {
+		return nil, err
+	}
+	return minioClient, nil
 }
 
 // MustGetSystemCertPool - return system CAs or empty pool in case of error (or windows)
@@ -790,20 +767,12 @@ func MustGetSystemCertPool() *x509.CertPool {
 	return pool
 }
 
-// IsLDAPEnabled ldap enabled
-func (t *Tenant) IsLDAPEnabled() bool {
-	for _, env := range t.GetEnvVars() {
-		if env.Name == "MINIO_IDENTITY_LDAP_SERVER_ADDR" && env.Value != "" {
-			return true
-		}
-	}
-	return false
-}
-
 // CreateUsers creates a list of admin users on MinIO, optionally creating users is disabled.
-func (t *Tenant) CreateUsers(madmClnt *madmin.AdminClient, userCredentialSecrets []*corev1.Secret) error {
-	skipCreateUser := t.IsLDAPEnabled() // Skip creating users if LDAP is enabled.
-
+func (t *Tenant) CreateUsers(madmClnt *madmin.AdminClient, userCredentialSecrets []*corev1.Secret, tenantConfiguration map[string][]byte) error {
+	var skipCreateUser bool // Skip creating users if LDAP is enabled.
+	if ldapAddress, ok := tenantConfiguration["MINIO_IDENTITY_LDAP_SERVER_ADDR"]; ok {
+		skipCreateUser = string(ldapAddress) != ""
+	}
 	// add user with a 20 seconds timeout
 	ctx, cancel := context.WithTimeout(context.Background(), time.Second*20)
 	defer cancel()

pkg/controller/cluster/main-controller.go

@@ -1222,7 +1222,7 @@ func (c *Controller) syncHandler(key string) error {
 		c.RegisterEvent(ctx, tenant, corev1.EventTypeNormal, "UsersCreated", "Users created")
 	}
 
-	// Ensure we are only creating the bucket once using the console user
+	// Ensure we are only creating the bucket
 	if !tenant.Status.ProvisionedBuckets && len(tenant.Spec.Buckets) > 0 {
 		if err := c.createBuckets(ctx, tenant, tenantConfiguration); err != nil {
 			klog.V(2).Infof("Unable to create MinIO buckets: %v", err)

pkg/controller/cluster/operator.go

@@ -359,7 +359,7 @@ func (c *Controller) createUsers(ctx context.Context, tenant *miniov2.Tenant, te
 	}
 
 	// get a new admin client
-	adminClnt, err := tenant.NewMinIOAdmin(tenantConfiguration, c.getTransport())
+	adminClient, err := tenant.NewMinIOAdmin(tenantConfiguration, c.getTransport())
 	if err != nil {
 		// show the error and continue
 		klog.Errorf("Error instantiating madmin: %v", err)
@@ -368,7 +368,7 @@ func (c *Controller) createUsers(ctx context.Context, tenant *miniov2.Tenant, te
 
 	// configuration that means MinIO is running with LDAP enabled
 	// and we need to skip the console user creation
-	if err = tenant.CreateUsers(adminClnt, userCredentials); err != nil {
+	if err = tenant.CreateUsers(adminClient, userCredentials, tenantConfiguration); err != nil {
 		klog.V(2).Infof("Unable to create MinIO users: %v", err)
 	}
 
@@ -384,25 +384,24 @@ func (c *Controller) createBuckets(ctx context.Context, tenant *miniov2.Tenant,
 		}
 	}()
 
-	if tenant.IsLDAPEnabled() {
+	tenantBuckets := tenant.Spec.Buckets
+	if len(tenantBuckets) == 0 {
 		return nil
 	}
 
 	if _, err := c.updateTenantStatus(ctx, tenant, StatusProvisioningDefaultBuckets, 0); err != nil {
 		return err
 	}
 
-	userCredentials := c.fetchUserCredentials(ctx, tenant)
-
-	// Create bucket using the console user
-	minioClnt, err := tenant.NewMinIOUser(userCredentials, c.getTransport())
+	// get a new admin client
+	minioClient, err := tenant.NewMinIOUser(tenantConfiguration, c.getTransport())
 	if err != nil {
 		// show the error and continue
-		klog.Errorf("Error instantiating minio Client: %v, no buckets will be provisioned please verify if you have 'spec.users' set", err)
+		klog.Errorf("Error instantiating minio Client: %v ", err)
 		return err
 	}
 
-	if err := tenant.CreateBuckets(minioClnt, tenant.Spec.Buckets...); err != nil {
+	if err := tenant.CreateBuckets(minioClient, tenantBuckets...); err != nil {
 		klog.V(2).Infof("Unable to create MinIO buckets: %v", err)
 		return err
 	}