[Freeipa-devel] [PATCHES 0015-0017] consolidation of various Kerberos auth methods in FreeIPA code

[Martin Kosek]

On 03/13/2015 05:37 PM, Martin Babinsky wrote:
> Attaching the next iteration of patches.
> 
> I have tried my best to reword the ipa-client-install man page bit about the
> new option. Any suggestions to further improve it are welcome.
> 
> I have also slightly modified the 'kinit_keytab' function so that in Kerberos
> errors are reported for each attempt and the text of the last error is retained
> when finally raising exception.

The approach looks very good. I think that my only concern with this patch is
this part:

+            ccache.init_creds_keytab(keytab=ktab, principal=princ)
...
+        except krbV.Krb5Error as e:
+            last_exc = str(e)
+            root_logger.debug("Attempt %d/%d: failed: %s"
+                              % (attempt, attempts, last_exc))
+            time.sleep(1)
+
+    root_logger.debug("Maximum number of attempts (%d) reached"
+                      % attempts)
+    raise StandardError("Error initializing principal %s: %s"
+                        % (principal, last_exc))

The problem here is that this function will raise the super-generic
StandardError instead of the proper with all the context and information about
the error that the caller can then process.

I think that

    except krbV.Krb5Error as e:
        if attempt == max_attempts:
            log something
            raise

would be better.