[augeas-devel] adjustment to the krb5.aug lense

[Dominic Cleal]

On 13/08/12 16:30, Pat Riehecky wrote:
> I've got this typechecked for real this time and would love some feedback.
> 
> Its amazing when you check the code you changed rather than the known 
> good reference, suddenly your errors show up!

Makes life easier, doesn't it? :-)

I'm pretty happy with that, just a few bits below that could be improved
if you can.  The use of the test_*.aug lenses is spot on, that's much
more powerful than the script you'd created last time round.

> --- lenses/krb5.aug.orig	2012-08-07 13:01:20.000000000 -0500
> +++ lenses/krb5.aug	2012-08-13 10:24:21.584557595 -0500
> +(*
> +  For the enctypes this appears to be a list of the valid entries:
> +       c4-hmac arcfour-hmac aes128-cts rc4-hmac
> +       arcfour-hmac-md5 des3-cbc-sha1 des-cbc-md5 des-cbc-crc
> +*)
> +let enctype_re = /[a-zA-Z0-9]{3,8}-[a-zA-Z0-9]{3,5}[a-zA-Z0-9-]*/ 

You could probably simplify this to [a-zA-Z0-9-]{3,} which is likely to
be more robust.  I don't think the specificity in the above regexp is
helpful.

> +let enctypes = /(permitted_enctypes|default_tgs_enctypes|default_tkt_enctypes)/

This line needs to be case insensitive I think, so add an "i" after the
end "/".  Reason below..

> --- lenses/tests/test_krb5.aug.orig	2012-08-07 13:01:20.000000000 -0500
> +++ lenses/tests/test_krb5.aug	2012-08-13 10:26:02.758574747 -0500
> @@ -1,6 +1,6 @@
>  module Test_krb5 =
>  
> -  (* Krb5.conf from Fermi labs *)
> +  (* Krb5.conf from Fermilab *)
>    let fermi_str = "###
>  ### This krb5.conf template is intended for use with Fermi
>  ### Kerberos v1_2 and later.  Earlier versions may choke on the
> @@ -13,8 +13,9 @@ module Test_krb5 =
>  	ticket_lifetime = 1560m
>  	default_realm = FNAL.GOV
>  	ccache_type = 4
> -	default_tgs_enCtypes = des-cbc-crc
> +	default_tgs_enctypes = des-cbc-crc

.. this line probably used to test case insensitivity.  If you put
"default_tgs_ENCTYPES" in your config file versus "default_tgs_enctypes"
you'd get different trees created.  It looks like the original intention
of the test and lens was to be case insensitive, as I _assume_ krb5 is.

> +  let enctype_list = [ indent . key enctypes . eq
> +      . Build.opt_list ([label "enctype" . store enctype_re]) comma_or_space_sep
> +      . (comment|eol)] in

I'd suggest using the "seq" lens in this instance, which creates
numbered nodes rather than relying on the special constant "enctype".
You use it in combination with "counter" to initialise it outside your
opt_list, then put "seq" inside the inner lens used in the list, e.g.

let enctype_list = [ indent . key enctypes . eq . counter "enctype"
    . Build.opt_list ([seq "enctype" . store enctype_re]) comma_or_space_sep
    . (comment|eol)] in

The tree created should then look like:

  { "default_tgs_enctypes"
    { "1" = "des-cbc-crc" }
    { "2" = "aes128-cts" } }

Cheers,

-- 
Dominic Cleal
Red Hat Consulting
m: +44 (0)7817 878113