Merge lp:~gandelman-a/ubuntu/precise/cobbler-enlist/868492 into lp:ubuntu/precise/cobbler-enlist

[Adam Gandelman]

  Register all valid network interfaces when enlisting (LP: #868492)

  This converts enlister->netif to a linked-list (enlister->netifs). Each node
  of the LL is of struct netif, which contains an interface name and macaddr.

  Any number of interfaces can be supplied via the command line. Each interface
  is appended to the link list as a struct netif with the macaddr left NULL.

  If no interfaces are supplied via argv, the list is populated with every
  interface on the system (the macaddr for each left NULL).

  What is contained in the enlister->netifs list is then verified. In the event
  a non-existent interface was supplied via argv, it is removed from the list.
  If enlister->netifs is empty (ie, none of the interfaces supplied actually
  exist), die.

  Nodes remaining in enlister->netifs get their mac addr's populated via ioctl.
  Non-standard devices (ie, bridges not backed by any device) may contain a bad
  mac addr, these are skipped. After populating, error out if the list does not
  contain at least one valid mac address.

  During system registration, loop over enlister->netifs and register each device.

  On the cobbler side, the system ends up with something like:

  # cobbler system dumpvars --name=testnode | grep mac_address_
  mac_address_eth0 : 90:e6:ba:54:93:b9
  mac_address_eth1 : 00:04:5a:5e:62:5f
  mac_address_virbr1 : 52:54:00:c2:5d:94
  mac_address_vnet0 : fe:16:3e:3e:a9:1a
  mac_address_vnet1 : fe:54:00:4d:7e:a0

  Cobbler will serve the profile via netboot to any of the interfaces.

[C de-Avillez (hggdh2)]

We should keep an option to select specific interfaces, both on install and run-time. There are environments where strict separation of interface usage is enforced, and selecting all interfaces will trigger an audit finding.

[Dave Walker (davewalker)]

@Carlos, If i read the comment correctly.. It should default to sending all mac addresses, but support comma separated list of interfaces it should use. This should also work via preseed (the real target), but I am concerned how this will work with cobbler generic preseeds (as it was my intention to ship default of register all interfaces)

thoughts?

[Adam Gandelman (gandelman-a)]

To be clear, the default behavior after this patch is to register ALL valid interfaces+mac addrs on the system. If one or multiple interfaces is specified on the command line, those (and only those) interfaces+macaddrs will be registered.

It seems we're still determining how best to gather system information and post back. This change set provides the necessary internals to register multiple NICs + mac addrs, which is something that will be desired regardless of how decide to gather them. I predict we'll move from obtaining mac addrs via ioctl() to parsing it outside of cobbler-enlist. While the front end argv handling will likely change over time, the internal enlister->netifs changes here (which is the bulk of the patch) should meet our needs regardless.

[lp:~gandelman-a/ubuntu/precise/cobbler-enlist/868492]

9. By Adam Gandelman on 2011-11-15

Fix malloc() casting and memory initialization

[C de-Avillez (hggdh2)]

@Daviey: A generic cobbler preseed could (and should, I think) default to all interfaces; those that need to enforce separation of usage can then adjust the preseed(s) as needed (I believe this is possible ;-). I would also expect that these customers would probably restrict to one, at most two interfaces.

Hum. We do not support, anymore, d-i selection of interfaces?

[lp:~gandelman-a/ubuntu/precise/cobbler-enlist/868492]

10. By Adam Gandelman on 2011-11-21

Add support for multiple network interfaces (convert enlister->netif
to linked list enlister->netifs). When no interfaces specified, defaults
to registering all interfaces on the system. (LP: #868492)

11. By Adam Gandelman on 2011-11-21

Merge trunk

[Dave Walker (davewalker)]

Changed the pocket and version, and uploaded

Thanks.

review: Approve

=== modified file 'cobbler-enlist.c'
--- cobbler-enlist.c	2011-08-02 15:49:55 +0000
+++ cobbler-enlist.c	2011-11-21 19:59:24 +0000
@@ -48,13 +48,14 @@
 main (int const argc, char **argv)
 {
   int rc;
-
   struct cobbler_client *enlister;
   enlister = (struct cobbler_client *) malloc(sizeof(struct cobbler_client));
+  if (enlister == NULL)
+    die("malloc() failed for enlister- main()");
+  memset(enlister, 0, sizeof(struct cobbler_client));
   init_client(enlister);
   int c;
 
-  //int this_option_optind = optind ? optind : 1;
   int option_index = 0;
 
   const char* const short_options = "hds:u:p:n:P:i:m:";
@@ -67,7 +68,6 @@
     {"netif", 1, 0, 'i'},
     {"manclass", 1, 0, 'm'},
     {"help", 0, 0, 'h'},
-    //{"default-interface", 1, 0, 0},
     {0, 0, 0, 0}
   };
 
@@ -111,9 +111,8 @@
           break;
 
         case 'i':
-          enlister->netif = optarg;
+          append_netif(enlister, optarg);
           break;
-
         case 'm':
           enlister->mgmt_class = optarg;
           break;
@@ -145,16 +144,30 @@
       (enlister->name == NULL) ||
       (enlister->profile == NULL))
   {
-    fprintf(stderr, "[%s] ERROR: not all required parameters were passed\n", argv[0]);
     help(argv[0]);
-    exit (1);
-  }
-
-
-  if ((rc = get_mac_address(enlister)) != 0) {
-    printf("ERROR: Could not determine mac address for any interface on system.\n");
-    exit(1);
-  }
+    die("Not all required parameters were passed");
+  }
+
+  /* if no interfaces were specified on command line, add all interfaces on
+     the system */
+  if (!enlister->netifs) {
+    if (debug)
+      printf("[DEBUG] Populating netifs list with all interfaces on system.\n");
+    struct if_nameindex *ifs = if_nameindex();
+    while (ifs && ifs->if_name) {
+      if (strcasecmp(ifs->if_name, "lo") != 0)
+        append_netif(enlister, ifs->if_name);
+        ifs++;
+    }
+  }
+
+  /* ensure list isn't empty and all interfaces exist */
+  if (verify_netifs(enlister) != 0)
+    die("Could not verify any of specified interfaces");
+
+  /* get mac addresses for interfaces contained in enlister->netifs */
+  if (get_mac_addresses(enlister) != 0)
+    die("Could not determine mac address for any interface on system.");
 
   if (debug)
     display_config(enlister);
@@ -162,12 +175,16 @@
   /* Initialize our error-handling environment. */
   if ((rc = init_xmlrpc_env(enlister)) != 0)
     goto out;
+  /* Initialize the client */
   if ((rc = init_xmlrpc_client(enlister)) != 0)
     goto out;
+  /* Login to cobbler */
   if ((rc = cobbler_login(enlister)) != 0)
     goto out;
-  if ((rc =  cobbler_register_system(enlister)) != 0)
+  /* Register new system with everything contained in the enlister */
+  if ((rc = cobbler_register_system(enlister)) != 0)
     goto out;
+  /* Save the system */
   if ((rc = cobbler_save_system(enlister)) != 0)
     goto out;
 

=== modified file 'cobbler-enlist.h'
--- cobbler-enlist.h	2011-09-28 00:43:36 +0000
+++ cobbler-enlist.h	2011-11-21 19:59:24 +0000
@@ -38,6 +38,8 @@
 #define OPT_NETIF    5
 #define OPT_HELP     6
 
+#define NO_MACADDR "00:00:00:00:00:00"
+
 static int debug =  0;
 
 struct cobbler_client {
@@ -54,7 +56,16 @@
   const char *netif;
   const char *mgmt_class;
   const char *macaddr;
-};
+  struct netif *netifs;
+};
+
+struct netif {
+  const char *interface;
+  const char *macaddr;
+  struct netif *next;
+};
+
+void get_netifs(struct cobbler_client *enlister);
 
 int init_client(struct cobbler_client *enlister) {
   enlister->client = NULL;
@@ -68,6 +79,7 @@
   enlister->netif = NULL;
   enlister->mgmt_class = NULL;
   enlister->macaddr = NULL;
+  enlister->netifs = NULL;
   return 0;
 }
 
@@ -78,11 +90,80 @@
   printf("\tpassword:   %s\n", enlister->password);
   printf("\tname:       %s\n", enlister->name);
   printf("\tprofile:    %s\n", enlister->profile);
-  if (enlister->netif)
-    printf("\tnetif:      %s\n", enlister->netif);
+  if (enlister->netifs) {
+    get_netifs(enlister);
+  }
   if (enlister->mgmt_class)
     printf("\tmgmt class: %s\n", enlister->mgmt_class);
-  printf("\tmac addr:   %s\n", enlister->macaddr);
+}
+
+void get_netifs(struct cobbler_client *enlister) {
+  struct netif *current = enlister->netifs;
+  if (current == NULL) {
+    printf("Interface list is empty\n");
+    return;
+  }
+  int i = 0;
+  while (current != NULL) {
+    printf("\tinterface %d: %s - %s\n", i, current->interface, current->macaddr);
+    i++;
+    current = current->next;
+  }
+}
+
+void die(char *msg) {
+  fprintf(stderr, "[ERROR] %s\n", msg);
+  exit(1);
+}
+
+/* append a new interface to the netifs list. macaddr remains empty
+   until populated by get_macaddresses() */
+int append_netif(struct cobbler_client *enlister, char *netif) {
+  if (debug)
+    printf("[DEBUG] Appending interface %s to enlister's netif list\n", netif);
+  struct netif *new_netif, *current;
+  new_netif = (struct netif *) malloc(sizeof(struct netif));
+  if (new_netif == NULL)
+    die("malloc() - append_netif()");
+  memset(new_netif, 0, sizeof(struct netif));
+  new_netif->interface = netif;
+  new_netif->macaddr = NULL;
+  new_netif->next = NULL;
+  if (enlister->netifs == NULL) {
+    enlister->netifs = new_netif;
+    return 0;
+  }
+  int i;
+  current = enlister->netifs;
+  while (current->next != NULL) {
+    current = current->next;
+    i++;
+  }
+  current->next = new_netif;
+  return 0;
+}
+
+/* delete an interface from the netifs list */
+int remove_netif(struct cobbler_client *enlister, const char *interface) {
+  if (debug)
+    printf("[DEBUG] Removing from interface list: %s\n", interface);
+  struct netif *current = enlister->netifs;
+  int i = 0;
+  if (strcasecmp(current->interface, interface) == 0) {
+    enlister->netifs = current->next;
+    return 0;
+  }
+  i++;
+  while (current->next) {
+    if (strcasecmp(current->next->interface, interface) == 0) {
+      current->next = current->next->next;
+      return 0;
+    }
+    current = current->next;
+    i++;
+  }
+  printf("ERROR: NEtif: %s doesn't exist in enlister->netifs\n", interface);
+  return 1;
 }
 
 // Overide ether_ntoa_r because stock decides the convention of leading zeros is silly.
@@ -96,46 +177,93 @@
   return buf;
 }
 
-int get_mac_address(struct cobbler_client *enlister) {
-  if (debug)
-    printf("[DEBUG] get_mac_address()\n");
-
-  int fd;
-  struct if_nameindex *curif, *ifs;
-  char mac_str[25];
-  char interface[255];
-  char *macaddr = malloc(sizeof(mac_str));
-
+int interface_exists(const char *interface) {
+  if (debug)
+    printf("[DEBUG] Checking for interface: %s\n", interface);
+  struct if_nameindex *curif, *ifs = if_nameindex();
+  for (curif = ifs; curif && curif->if_name; curif++) {
+    if (strcasecmp(interface, curif->if_name) == 0)
+     return 0;
+  }
+  return 1;
+}
+
+/* inspect each enlister->netifs.  Make sure the interface exists on the system
+   Remove it from the list if it doesn't.
+   If we've cleaned the entire list because none of them exist, return 1. 
+   Can/should be extended to do more verification?
+*/
+int verify_netifs(struct cobbler_client *enlister) {
+  struct netif *current = enlister->netifs;
+  while (current) {
+    if (interface_exists(current->interface) != 0) {
+      printf("[WARNING] Interface %s doesn't exist, removing.\n", current->interface);
+      remove_netif(enlister, current->interface);
+    }
+    current = current->next;
+  }
+  if (enlister->netifs == NULL)
+    return 1;
+  return 0;
+}
+
+/* utility function to test whether the netifs list contains at least
+   one mac address 
+*/
+int contains_one_macaddr(struct cobbler_client *enlister) {
+  struct netif *current = enlister->netifs;
+  while (current) {
+    if ((current->macaddr != NULL) &&
+        (strcasecmp(current->macaddr, NO_MACADDR) != 0)) {
+          return 0;
+    }
+  }
+  return 1;
+}
+
+char *get_interface_macaddr(const char *interface) {
+  if (debug)
+    printf("[DEBUG] Getting mac addr for interface %s\n", interface);
+  char mac_str[255];
+  char *macaddr;
   int ioctl_data = 0;
   struct ifreq ifr;
-
-  if ((fd = socket (PF_INET, SOCK_DGRAM, 0)) != -1) {
-    ifs = if_nameindex ();
-    for (curif = ifs; curif && curif->if_name; curif++) {
-      if (((enlister->netif == NULL) ||
-          (strcasecmp(enlister->netif, curif->if_name) == 0))
-           && (strcasecmp(curif->if_name, "lo") !=0)) {
-
-          ioctl_data = socket (PF_INET, SOCK_STREAM, 0);
-          memset (&ifr, 0, sizeof (ifr));
-          strncpy (ifr.ifr_name, curif->if_name, sizeof (ifr.ifr_name));
-          strncpy (interface, curif->if_name, sizeof (interface));
-          ioctl (ioctl_data, SIOCGIFHWADDR, &ifr);
-          ether_ntoa_r( (const struct ether_addr *)
-                        &(ifr.ifr_hwaddr.sa_data), mac_str);
-          if (debug)
-            printf("[DEBUG] Using interface: %s Addr: %s\n", interface, mac_str);
-          if (enlister->netif == NULL)
-            enlister->netif = curif->if_name;
-          strncpy(macaddr, mac_str, sizeof(mac_str));
-          enlister->macaddr = macaddr;
-          return 0;
-      }
-    }
+  ioctl_data = socket(PF_INET, SOCK_STREAM, 0);
+  memset(&ifr, 0, sizeof(ifr));
+  strncpy(ifr.ifr_name, interface, sizeof(interface));
+  ioctl(ioctl_data, SIOCGIFHWADDR, &ifr);
+  ether_ntoa_r((const struct ether_addr *) &(ifr.ifr_hwaddr.sa_data), mac_str);
+  macaddr = malloc(sizeof(mac_str));
+  if (macaddr == NULL)
+    die("malloc() - get_interface_macaddr()");
+  strncpy(macaddr, mac_str, sizeof(mac_str));
+  return macaddr;
+}
+
+/* iterate through enlister->netifs, populating macaddr for each.
+   if we can't determine a valid mac addr, remove from list and continue
+   returns non-zero if the resulting enlister->netifs does not contain at least
+   one mac addr.
+*/
+int get_mac_addresses(struct cobbler_client *enlister) {
+  if (debug)
+    printf("[DEBUG] get_mac_address()\n");
+  const char *addr;
+  struct netif *current = enlister->netifs;
+  while (current != NULL) {
+    addr = get_interface_macaddr(current->interface);
+    if (strcasecmp(addr, NO_MACADDR) == 0) {
+      printf("[WARNING] Could not determine mac addr for %s, skipping.\n",
+             current->interface);
+      remove_netif(enlister, current->interface);
+    } else {
+      current->macaddr = addr;
+    };
+    current = current->next;
   }
+  return contains_one_macaddr(enlister);
+}
 
-  return 1;
-}
 void help(char *progname)
 {
   printf("Usage: %s --help  -- provides this help\n" , progname);

=== modified file 'cobbler-xmlrpc.h'
--- cobbler-xmlrpc.h	2011-08-02 15:49:55 +0000
+++ cobbler-xmlrpc.h	2011-11-21 19:59:24 +0000
@@ -109,7 +109,7 @@
   xmlrpc_value *token = xmlrpc_string_new(&enlister->env, enlister->token);
   xmlrpc_value *name = xmlrpc_string_new(&enlister->env, enlister->name);
   xmlrpc_value *profile = xmlrpc_string_new(&enlister->env, enlister->profile);
-  xmlrpc_value *macaddr = xmlrpc_string_new(&enlister->env, enlister->macaddr);
+//  xmlrpc_value *macaddr = xmlrpc_string_new(&enlister->env, enlister->macaddr);
 
   /* used for forming modify_interface request */
   xmlrpc_value *macaddr_struct = xmlrpc_struct_new(&enlister->env);
@@ -162,23 +162,35 @@
   if ((rc = make_xmlrpc_call(enlister, "modify_system", argument_array)) != 0)
     goto out;
   if (debug)
-  printf("[DEBUG] Modified system profile: %d\n", rc);
-
-  /* set its mac address */
-  sprintf(macaddr_form_item, "macaddress-%s", enlister->netif);
-  xmlrpc_struct_set_value(&enlister->env, macaddr_struct,
-                          macaddr_form_item, macaddr);
-  argument_array = xmlrpc_array_new(&enlister->env);
-  xmlrpc_array_append_item(&enlister->env, argument_array, system);
-  xmlrpc_array_append_item(&enlister->env, argument_array,
-                           xmlrpc_string_new(&enlister->env, "modify_interface"));
-  xmlrpc_array_append_item(&enlister->env, argument_array, macaddr_struct);
-  xmlrpc_array_append_item(&enlister->env, argument_array, token);
-  if ((rc = make_xmlrpc_call(enlister, "modify_system", argument_array)) != 0)
-    goto out;
-  if (debug)
-    printf("[DEBUG] Modified interface: %d\n", rc);
-
+    printf("[DEBUG] Modified system profile: %d\n", rc);
+
+  /* set its mac addresses */
+  struct netif *current_netif = enlister->netifs;
+  while (current_netif != NULL) {
+    if (debug) {
+      printf("[DEBUG] Registering interface and macaddr: %s %s\n",
+              current_netif->interface, current_netif->macaddr);
+    }
+    xmlrpc_value *macaddr = xmlrpc_string_new(&enlister->env,
+                                              current_netif->macaddr);
+    sprintf(macaddr_form_item, "macaddress-%s", current_netif->interface);
+    xmlrpc_struct_set_value(&enlister->env, macaddr_struct,
+                            macaddr_form_item, macaddr);
+    argument_array = xmlrpc_array_new(&enlister->env);
+    xmlrpc_array_append_item(&enlister->env, argument_array, system);
+    xmlrpc_array_append_item(&enlister->env, argument_array,
+                             xmlrpc_string_new(&enlister->env, "modify_interface"));
+    xmlrpc_array_append_item(&enlister->env, argument_array, macaddr_struct);
+    xmlrpc_array_append_item(&enlister->env, argument_array, token);
+    if ((rc = make_xmlrpc_call(enlister, "modify_system", argument_array)) != 0) {
+      xmlrpc_DECREF(macaddr);
+      xmlrpc_DECREF(macaddr_struct);
+      goto out;
+    }
+    if (debug)
+      printf("[DEBUG] Modified interface: %d\n", rc);
+    current_netif = current_netif->next;
+  }
   /* assign a mgmt class */
   if (enlister->mgmt_class) {
     xmlrpc_value *mgmt_class = xmlrpc_string_new(&enlister->env,
@@ -202,8 +214,6 @@
   xmlrpc_DECREF(name);
   xmlrpc_DECREF(profile);
   xmlrpc_DECREF(system);
-  xmlrpc_DECREF(macaddr);
-  xmlrpc_DECREF(macaddr_struct);
   xmlrpc_DECREF(argument_array);
   return rc;
 }

=== modified file 'debian/changelog'
--- debian/changelog	2011-11-12 11:07:07 +0000
+++ debian/changelog	2011-11-21 19:59:24 +0000
@@ -1,3 +1,11 @@
+cobbler-enlist (0.2-1ubuntu1) oneiric; urgency=low
+
+  * Add support for multiple network interfaces (convert enlister->netif
+    to linked list enlister->netifs).  When no interfaces specified, defaults
+    to registering all interfaces on the system. (LP: #868492)
+
+ -- Adam Gandelman <[e-mail]>  Mon, 21 Nov 2011 11:40:37 -0800
+
 cobbler-enlist (0.2-1build2) precise; urgency=low
 
   * Rebuild for libxmlrpc-core-c3-udeb.