389-ds-base

Clone
Source Code
GIT

#50316 Ticket 49390, 50019 - support cn=config compare operations
Closed 3 years ago by spichugi. Opened 5 years ago by firstyear.
firstyear/389-ds-base 49390-ldap-cn-config-compare-server  into  master

file modified
+1 -1 
@@ -1301,6 +1301,7 @@ 
 

  	ldap/servers/slapd/ch_malloc.c \
 

  	ldap/servers/slapd/computed.c \
 

  	ldap/servers/slapd/control.c \
 

+ 	ldap/servers/slapd/configdse.c \
 

  	ldap/servers/slapd/counters.c \
 

  	ldap/servers/slapd/csn.c \
 

  	ldap/servers/slapd/csngen.c \
 
@@ -2056,7 +2057,6 @@ 
 

  	ldap/servers/slapd/bind.c  \
 

  	ldap/servers/slapd/compare.c \
 

  	ldap/servers/slapd/config.c \
 

- 	ldap/servers/slapd/configdse.c \
 

  	ldap/servers/slapd/connection.c \
 

  	ldap/servers/slapd/conntable.c \
 

  	ldap/servers/slapd/daemon.c \

file modified
-2 
@@ -16,7 +16,6 @@ 
 

  

      :expectedresults: 1. It should be the same (excluding unique id attrs)
 

      """
 

- 

      st1_config = topology_i2.ins.get('standalone1').config
 

      st2_config = topology_i2.ins.get('standalone2').config
 

      # 'nsslapd-port' attribute is expected to be same in cn=config comparison,
 
@@ -30,7 +29,6 @@ 
 

  

      assert Config.compare(st1_config, st2_config)
 

  

- 

  if __name__ == '__main__':
 

      # Run isolated
 

      # -s for DEBUG mode

file modified
+48 
@@ -117,7 +117,55 @@ 
 

              frame_iter = map(DSIdleFilterDecorator, frame_iter)
 

          return frame_iter
 

  

+ class DSEntryPrint (gdb.Command):
 

+     """Display a Slapi_Entry"""
 

+     def __init__(self):
 

+         super (DSEntryPrint, self).__init__("ds-entry-print", gdb.COMMAND_DATA)
 

+ 

+     def _display_values(self, a_name, va_ptr, num):
 

+         inum = int(num)
 

+ 

+         for i in range(0, inum):
 

+             value = va_ptr[i].dereference()
 

+             bv = value['bv']['bv_val']
 

+             if bv == 0:
 

+                 print("%s: X 0" % a_name)
 

+             else:
 

+                 print("%s: %s" % (a_name, bv.string()))
 

+ 

+ 

+     def _display_attrs(self, start):
 

+         if start.address == 0:
 

+             return
 

+         attr = start.dereference()
 

+         while True:
 

+             name = attr['a_type'].string()
 

+             # print(dir(name))
 

+             va = attr['a_present_values']['va']
 

+             num = attr['a_present_values']['num']
 

+             self._display_values(name, va, num)
 

+             # Now loop
 

+             n = attr['a_next']
 

+             if n == 0:
 

+                 return
 

+             attr = n.dereference()
 

+ 

+ 

+     def invoke (self, arg, from_tty):
 

+         gdb.newest_frame()
 

+         cur_frame = gdb.selected_frame()
 

+         entry_val = cur_frame.read_var(arg)
 

+         entry_root = entry_val.dereference()
 

+         entry_sdn = entry_root['e_sdn']['ndn']
 

+         # Display the SDN
 

+         print("Display Slapi_Entry: %s" % entry_sdn.string())
 

+         # Display the attributes.
 

+         entry_attrs = entry_root['e_attrs']
 

+         self._display_attrs(entry_attrs)
 

+ 

  DSAccessLog()
 

  DSBacktrace()
 

  DSIdleFilter()
 

+ DSEntryPrint()
 

+

file modified
+1 -1 
@@ -113,7 +113,7 @@ 
 

      be->be_database->plg_next_search_entry = &dse_next_search_entry;
 

      be->be_database->plg_search_results_release = &dse_search_set_release;
 

      be->be_database->plg_prev_search_results = &dse_prev_search_results;
 

-     be->be_database->plg_compare = &be_plgfn_unwillingtoperform;
 

+     be->be_database->plg_compare = &dse_compare;
 

      be->be_database->plg_modify = &dse_modify;
 

      be->be_database->plg_modrdn = &be_plgfn_unwillingtoperform;
 

      be->be_database->plg_add = &dse_add;

file modified
+89 -2 
@@ -39,6 +39,8 @@ 
 

  #include <prcountr.h>
 

  #include "slap.h"
 

  #include <pwd.h>
 

+ /* Needed to access read_config_dse */
 

+ #include "proto-slap.h"
 

  

  #include <unistd.h> /* provides fsync/close */
 

  
@@ -186,16 +188,18 @@ 
 

      Slapi_Entry *e = NULL;
 

      struct dse_node *n;
 

  

-     if (use_lock == DSE_USE_LOCK && pdse->dse_rwlock)
 

+     if (use_lock == DSE_USE_LOCK && pdse->dse_rwlock) {
 

          slapi_rwlock_rdlock(pdse->dse_rwlock);
 

+     }
 

  

      n = dse_find_node(pdse, dn);
 

      if (n != NULL) {
 

          e = slapi_entry_dup(n->entry);
 

      }
 

  

-     if (use_lock == DSE_USE_LOCK && pdse->dse_rwlock)
 

+     if (use_lock == DSE_USE_LOCK && pdse->dse_rwlock) {
 

          slapi_rwlock_unlock(pdse->dse_rwlock);
 

+     }
 

  

      return e;
 

  }
 
@@ -1678,6 +1682,89 @@ 
 

      return 0;
 

  }
 

  

+ int32_t
 

+ dse_compare(Slapi_PBlock *pb)
 

+ {
 

+     /*
 

+      * Inspired largely by ldbm_compare.c. Allow schema aware comparison
 

+      * of entries in the DSE, including cn=config.
 

+      */
 

+     backend *be = NULL;
 

+     char *type = NULL;
 

+     struct berval *bval = NULL;
 

+     Slapi_DN *sdn = NULL;
 

+     struct dse *pdse = NULL;
 

+     Slapi_Entry *ec = NULL;
 

+     Slapi_Value compare_value = {0};
 

+ 

+     slapi_pblock_get(pb, SLAPI_BACKEND, &be);
 

+     slapi_pblock_get(pb, SLAPI_PLUGIN_PRIVATE, &pdse);
 

+     slapi_pblock_get(pb, SLAPI_TARGET_SDN, &sdn);
 

+     slapi_pblock_get(pb, SLAPI_COMPARE_TYPE, &type);
 

+     slapi_pblock_get(pb, SLAPI_COMPARE_VALUE, &bval);
 

+ 

+     /* get the entry */
 

+     ec = dse_get_entry_copy(pdse, sdn, DSE_USE_LOCK);
 

+     if (ec == NULL) {
 

+         slapi_send_ldap_result(pb, LDAP_NO_SUCH_OBJECT, NULL, NULL, 0, NULL);
 

+         return -1;
 

+     }
 

+ 

+     /* Access control check */
 

+     int32_t err = slapi_access_allowed(pb, ec, type, bval, SLAPI_ACL_COMPARE);
 

+     if (err != LDAP_SUCCESS) {
 

+         slapi_entry_free(ec);
 

+         slapi_send_ldap_result(pb, err, NULL, NULL, 0, NULL);
 

+         return -1;
 

+     }
 

+ 

+     /* If cn=config, setup the entry with ALL values we could check from defaults */
 

+     Slapi_DN config_dn;
 

+     slapi_sdn_init_ndn_byref(&config_dn, "cn=config");
 

+     if (slapi_sdn_compare(&config_dn, sdn) == 0) {
 

+         read_config_dse(pb, ec, NULL, &err, NULL, NULL);
 

+         /*
 

+          * read_config_dse, and in turn, config_set_entry always returns
 

+          * a 1 here, which is probably dse_callback related.
 

+          */
 

+         if (err != 1) {
 

+             slapi_send_ldap_result(pb, LDAP_OPERATIONS_ERROR, NULL, NULL, 0, NULL);
 

+             return -1;
 

+         }
 

+         /*
 

+          * cn=config is now populated
 

+          */
 

+     }
 

+     slapi_sdn_done(&config_dn);
 

+ 

+     /* Do the schema aware check. */
 

+     slapi_value_init_berval(&compare_value, bval);
 

+ 

+     int32_t result = 0;
 

+     err = slapi_vattr_value_compare(ec, type, &compare_value, &result, 0);
 

+ 

+     /* We have the results, now free and then send. */
 

+     slapi_entry_free(ec);
 

+     value_done(&compare_value);
 

+ 

+     /* Format the result as expected. */
 

+     if (err != LDAP_SUCCESS) {
 

+         if (SLAPI_VIRTUALATTRS_NOT_FOUND == err) {
 

+             slapi_send_ldap_result(pb, LDAP_NO_SUCH_ATTRIBUTE, NULL, NULL, 0, NULL);
 

+         } else {
 

+             /* Some other problem, call it an operations error */
 

+             slapi_send_ldap_result(pb, LDAP_OPERATIONS_ERROR, NULL, NULL, 0, NULL);
 

+         }
 

+         return -1;
 

+     } else {
 

+         if (result != 0) {
 

+             slapi_send_ldap_result(pb, LDAP_COMPARE_TRUE, NULL, NULL, 0, NULL);
 

+         } else {
 

+             slapi_send_ldap_result(pb, LDAP_COMPARE_FALSE, NULL, NULL, 0, NULL);
 

+         }
 

+     }
 

+     return 0;
 

+ }
 

  

  /*
 

   * -1 means something went wrong.

file modified
+1 
@@ -701,6 +701,7 @@ 
 

  int dse_bind(Slapi_PBlock *pb);
 

  int dse_unbind(Slapi_PBlock *pb);
 

  int dse_search(Slapi_PBlock *pb);
 

+ int32_t dse_compare(Slapi_PBlock *pb);
 

  int dse_modify(Slapi_PBlock *pb);
 

  int dse_add(Slapi_PBlock *pb);
 

  int dse_delete(Slapi_PBlock *pb);

file modified
+12 -3 
@@ -415,7 +415,7 @@ 
 

                  raise ValueError('Too many arguments in the mod op')
 

          return self._instance.modify_ext_s(self._dn, mod_list, serverctrls=self._server_controls, clientctrls=self._client_controls, escapehatch='i am sure')
 

  

-     def _unsafe_compare_attribute(self, other):
 

+     def _unsafe_compare_attribute(self, attr, values):
 

          """Compare two attributes from two objects. This is currently marked unsafe as it's
 

          not complete yet.
 

  
@@ -430,7 +430,12 @@ 
 

  

          To allow schema aware checking, we need to call ldap compare extop here.
 

          """
 

-         pass
 

+         return all([
 

+             self._instance.compare_ext_s(self._dn, attr, value, serverctrls=self._server_controls,
 

+                 clientctrls=self._client_controls, escapehatch='i am sure')
 

+             for value in values
 

+         ])
 

+ 

  

      @classmethod
 

      def compare(cls, obj1, obj2):
 
@@ -469,10 +474,14 @@ 
 

          if set(obj1_attrs.keys()) != set(obj2_attrs.keys()):
 

              obj1._log.debug("%s != %s" % (obj1_attrs.keys(), obj2_attrs.keys()))
 

              return False
 

+         obj1._log.debug(sorted(obj1_attrs.keys()))
 

          # Check the values of each key
 

          # using obj1_attrs.keys() because obj1_attrs.iterkleys() is not supported in python3
 

          for key in obj1_attrs.keys():
 

-             if set(obj1_attrs[key]) != set(obj2_attrs[key]):
 

+             # Check if they are offline/online?
 

+             # if set(obj1_attrs[key]) != set(obj2_attrs[key]):
 

+             obj1._log.debug("checking %s: %s ..." % (key, obj2_attrs[key]))
 

+             if not obj1._unsafe_compare_attribute(key, obj2_attrs[key]):
 

                  obj1._log.debug("  v-- %s != %s" % (key, key))
 

                  obj1._log.debug("%s != %s" % (obj1_attrs[key], obj2_attrs[key]))
 

                  return False

Bug Description: Ansible will attempt to check the state of a value
before it makes an alteration on the ldap server. To do this in a
correct and schema aware fashion, it will use the ldapcompare operation.

It's a request that people want to manage their cn=config with ansible,
however dse.c didn't support ldapcompare on these backends.

Fix Description: Add support for ldapcompare operations on dse.c,
including the ability to correctly generate the cn=config defaults
into the entry for comparison.

This also adds support for ldapcompare as the default comparitor in
lib389.

https://pagure.io/389-ds-base/issue/49390
https://pagure.io/389-ds-base/issue/50019

Author: William Brown william@blackhats.net.au

Review by: ???

If I understood the PR correctly, you changed the ldapcompare behavior for working with cn=config. Is it possible to add some simple tests for that case?

Also, I am a bit new to this. How can I try ldap/admin/src/scripts/ns-slapd-gdb.py ?
I've run the instance with gdb --args /usr/sbin/ns-slapd -D /etc/dirsrv/slapd-localhost -i /var/run/dirsrv/slapd-localhost.pid and typed source /mnt/tests/rhds/tests/upstream/ds/ldap/admin/src/scripts/ns-slapd-gdb.py afterwards. And then run.
Nothing special has happened.

What is the right use for this tool?

There are already test cases? That's what the compare tests I was working on recently do ...

To use the script with the rpm, installed, IE /usr/sbin/ns-slapd, it autoloads. so just use the ds-* commands in gdb.

Remember, sourcing just bring the commands in, you need to run them like "ds-backtrace" or whatever.

There are already test cases? That's what the compare tests I was working on recently do ...

Right. By some reason, I remembered that you've added some directory entry compare tests.
But now I see you've added the cn=config one.
It passes with the PR, BTW.

To use the script with the rpm, installed, IE /usr/sbin/ns-slapd, it autoloads. so just use the ds-* commands in gdb.

Yeah, I've already discussed it with guys on the scrum. But thank you anyway :)

Remember, sourcing just bring the commands in, you need to run them like "ds-backtrace" or whatever.

For sure, I remember. :) I was just describing my initial steps.

You have my ack! The code works and I don't see any issues with the logic.

rebased onto 56373fb
5 years ago

Pull-Request has been merged by firstyear
5 years ago

389-ds-base is moving from Pagure to Github. This means that new issues and pull requests
will be accepted only in 389-ds-base's github repository.

This pull request has been cloned to Github as issue and is available here:
- https://github.com/389ds/389-ds-base/issues/3375

If you want to continue to work on the PR, please navigate to the github issue,
download the patch from the attachments and file a new pull request.

Thank you for understanding. We apologize for all inconvenience.

Pull-Request has been closed by spichugi
3 years ago
Metadata
None
No Tags
Changes Summary 7
+1 -1
file changed
Makefile.am
+0 -2
file changed
dirsrvtests/tests/suites/lib389/config_compare_test.py
+48 -0
file changed
ldap/admin/src/scripts/ns-slapd-gdb.py
+1 -1
file changed
ldap/servers/slapd/backend_manager.c
+89 -2
file changed
ldap/servers/slapd/dse.c
+1 -0
file changed
ldap/servers/slapd/proto-slap.h
+12 -3
file changed
src/lib389/lib389/_mapped_object.py
Powered by Pagure 5.13.3
Documentation • File an Issue • About • SSH Hostkey/Fingerprint
© Red Hat, Inc. and others.