| Message ID | 20170721204450.29536-1-mmichels@redhat.com |
|---|---|
| State | Changes Requested |
| Headers | show |
> From: "Mark Michelson" <mmichels@redhat.com> > To: dev@openvswitch.org > 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 <mmichels@redhat.com> > Reported-by: Lance Richardson <lrichard@redhat.com> > --- > 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", > - "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"}}}, > "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 > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > Applied the patch, tried a few things, works as expected. LGTM! Acked-by: Lance Richardson <lrichard@redhat.com>
A couple of suggestions inline -- On Wed, Jul 26, 2017 at 9:07 AM, Lance Richardson <lrichard@redhat.com> wrote: >> From: "Mark Michelson" <mmichels@redhat.com> >> To: dev@openvswitch.org >> 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 <mmichels@redhat.com> >> Reported-by: Lance Richardson <lrichard@redhat.com> >> --- >> 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 >> dev@openvswitch.org >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >> > > Applied the patch, tried a few things, works as expected. LGTM! > > Acked-by: Lance Richardson <lrichard@redhat.com> > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
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", - "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"}}}, "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);
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 <mmichels@redhat.com> Reported-by: Lance Richardson <lrichard@redhat.com> --- 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(-)