A couple of suggestions inline --
On Wed, Jul 26, 2017 at 9:07 AM, Lance Richardson <[email protected]> wrote:
>> From: "Mark Michelson" <[email protected]>
>> To: [email protected]
>> Sent: Friday, 21 July, 2017 4:44:50 PM
>> Subject: [ovs-dev] [PATCH] ovn: Restrict encap modification to its creating
>> chassis
>>
>> This patch extends RBAC restrictiveness of the encap table in
>> the ovn southbound database by only allowing modification by the
>> chassis that created the encap.
>>
>> Signed-off-by: Mark Michelson <[email protected]>
>> Reported-by: Lance Richardson <[email protected]>
>> ---
>> ovn/controller/chassis.c | 1 +
>> ovn/northd/ovn-northd.c | 2 +-
>> ovn/ovn-sb.ovsschema | 5 +++--
>> ovn/ovn-sb.xml | 3 +++
>> ovn/utilities/ovn-sbctl.c | 1 +
>> 5 files changed, 9 insertions(+), 3 deletions(-)
>>
>> diff --git a/ovn/controller/chassis.c b/ovn/controller/chassis.c
>> index d38bc949a..640ebbc40 100644
>> --- a/ovn/controller/chassis.c
>> +++ b/ovn/controller/chassis.c
>> @@ -222,6 +222,7 @@ chassis_run(struct controller_ctx *ctx, const char
>> *chassis_id,
>> sbrec_encap_set_type(encaps[i], type);
>> sbrec_encap_set_ip(encaps[i], encap_ip);
>> sbrec_encap_set_options(encaps[i], &options);
>> + sbrec_encap_set_name(encaps[i], chassis_id);
>> }
>> sbrec_chassis_set_encaps(chassis_rec, encaps, n_encaps);
>> free(encaps);
>> diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
>> index a3f138d44..473221a70 100644
>> --- a/ovn/northd/ovn-northd.c
>> +++ b/ovn/northd/ovn-northd.c
>> @@ -6025,7 +6025,7 @@ static const char *rbac_chassis_update[] =
>> {"nb_cfg", "external_ids", "encaps", "vtep_logical_switches"};
>>
>> static const char *rbac_encap_auth[] =
>> - {""};
>> + {"name"};
>> static const char *rbac_encap_update[] =
>> {"type", "options", "ip"};
>>
>> diff --git a/ovn/ovn-sb.ovsschema b/ovn/ovn-sb.ovsschema
>> index 264364095..8ee6a4519 100644
>> --- a/ovn/ovn-sb.ovsschema
>> +++ b/ovn/ovn-sb.ovsschema
>> @@ -1,7 +1,7 @@
>> {
>> "name": "OVN_Southbound",
>> "version": "1.14.0",
Since this patch is updating the schema, we should bump the version
number. Since this adds a column, but is backwards compatible, let's
make it 1.15.0.
>> - "cksum": "3613553908 13275",
>> + "cksum": "1622387373 13319",
>> "tables": {
>> "SB_Global": {
>> "columns": {
>> @@ -45,7 +45,8 @@
>> "value": "string",
>> "min": 0,
>> "max": "unlimited"}},
>> - "ip": {"type": "string"}}},
>> + "ip": {"type": "string"},
>> + "name": {"type": "string"}}},
Instead of "name", how about "chassis_name" since it's not really the
name of the encap.
>> "Address_Set": {
>> "columns": {
>> "name": {"type": "string"},
>> diff --git a/ovn/ovn-sb.xml b/ovn/ovn-sb.xml
>> index c1731d284..4a12f3eaf 100644
>> --- a/ovn/ovn-sb.xml
>> +++ b/ovn/ovn-sb.xml
>> @@ -353,6 +353,9 @@
>> <column name="ip">
>> The IPv4 address of the encapsulation tunnel endpoint.
>> </column>
>> + <column name="name">
>> + The name of the chassis that created this encap.
>> + </column>
>> </table>
>>
>> <table name="Address_Set" title="Address Sets">
>> diff --git a/ovn/utilities/ovn-sbctl.c b/ovn/utilities/ovn-sbctl.c
>> index 5c3403266..669d47495 100644
>> --- a/ovn/utilities/ovn-sbctl.c
>> +++ b/ovn/utilities/ovn-sbctl.c
>> @@ -571,6 +571,7 @@ cmd_chassis_add(struct ctl_context *ctx)
>> sbrec_encap_set_type(encaps[i], encap_type);
>> sbrec_encap_set_ip(encaps[i], encap_ip);
>> sbrec_encap_set_options(encaps[i], &options);
>> + sbrec_encap_set_name(encaps[i], ch_name);
>> i++;
>> }
>> sset_destroy(&encap_set);
>> --
>> 2.13.3
>>
>> _______________________________________________
>> dev mailing list
>> [email protected]
>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>>
>
> Applied the patch, tried a few things, works as expected. LGTM!
>
> Acked-by: Lance Richardson <[email protected]>
> _______________________________________________
> dev mailing list
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
--
Russell Bryant
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
