| Message ID | 20221202162145.1870691-1-frode.nordahl@canonical.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [ovs-dev] northd: Add missing RBAC rules for BFD table. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/github-robot-_ovn-kubernetes | success | github build: passed |
On 12/2/22 17:21, Frode Nordahl wrote: > If a OVN deployment has OVN RBAC enabled for the southbound > database, enabling BFD would lead to permission errors. > > The data in the entries in the BFD table do not belong to any > given chassis and no column can provide authentication, but the > rules still need to be there for successful operation. > > Fixes: 117203584d98 ("controller: introduce BFD tx path in ovn-controller.") > Reported-at: https://bugs.launchpad.net/ubuntu/+source/ovn/+bug/1995771 > Signed-off-by: Frode Nordahl <frode.nordahl@canonical.com> > --- Hi Frode, Thanks for the patch! > northd/ovn-northd.c | 13 +++++++++++++ > 1 file changed, 13 insertions(+) > > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c > index 965353cd7..89d8c7172 100644 > --- a/northd/ovn-northd.c > +++ b/northd/ovn-northd.c > @@ -125,6 +125,11 @@ static const char *rbac_igmp_group_auth[] = > {""}; > static const char *rbac_igmp_group_update[] = > {"address", "chassis", "datapath", "ports"}; > +static const char *rbac_bfd_auth[] = > + {""}; > +static const char *rbac_bfd_update[] = > + {"src_port", "disc", "logical_port", "dst_ip", "min_tx", "min_rx", > + "detect_mult", "status", "external_ids", "options"}; I didn't try it out but isn't it enough if we list "status" here? ovn-controller never tries to write to any of the others. Regards, Dumitru
On Tue, Dec 6, 2022 at 1:47 PM Dumitru Ceara <dceara@redhat.com> wrote: > > On 12/2/22 17:21, Frode Nordahl wrote: > > If a OVN deployment has OVN RBAC enabled for the southbound > > database, enabling BFD would lead to permission errors. > > > > The data in the entries in the BFD table do not belong to any > > given chassis and no column can provide authentication, but the > > rules still need to be there for successful operation. > > > > Fixes: 117203584d98 ("controller: introduce BFD tx path in ovn-controller.") > > Reported-at: https://bugs.launchpad.net/ubuntu/+source/ovn/+bug/1995771 > > Signed-off-by: Frode Nordahl <frode.nordahl@canonical.com> > > --- > > Hi Frode, > > Thanks for the patch! > > > northd/ovn-northd.c | 13 +++++++++++++ > > 1 file changed, 13 insertions(+) > > > > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c > > index 965353cd7..89d8c7172 100644 > > --- a/northd/ovn-northd.c > > +++ b/northd/ovn-northd.c > > @@ -125,6 +125,11 @@ static const char *rbac_igmp_group_auth[] = > > {""}; > > static const char *rbac_igmp_group_update[] = > > {"address", "chassis", "datapath", "ports"}; > > +static const char *rbac_bfd_auth[] = > > + {""}; > > +static const char *rbac_bfd_update[] = > > + {"src_port", "disc", "logical_port", "dst_ip", "min_tx", "min_rx", > > + "detect_mult", "status", "external_ids", "options"}; > > I didn't try it out but isn't it enough if we list "status" here? > ovn-controller never tries to write to any of the others. Ha, thank you for pointing that out, It ought to be sufficient then, I'll check and send a v2!
diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c index 965353cd7..89d8c7172 100644 --- a/northd/ovn-northd.c +++ b/northd/ovn-northd.c @@ -125,6 +125,11 @@ static const char *rbac_igmp_group_auth[] = {""}; static const char *rbac_igmp_group_update[] = {"address", "chassis", "datapath", "ports"}; +static const char *rbac_bfd_auth[] = + {""}; +static const char *rbac_bfd_update[] = + {"src_port", "disc", "logical_port", "dst_ip", "min_tx", "min_rx", + "detect_mult", "status", "external_ids", "options"}; static struct rbac_perm_cfg { const char *table; @@ -207,6 +212,14 @@ static struct rbac_perm_cfg { .update = rbac_igmp_group_update, .n_update = ARRAY_SIZE(rbac_igmp_group_update), .row = NULL + },{ + .table = "BFD", + .auth = rbac_bfd_auth, + .n_auth = ARRAY_SIZE(rbac_bfd_auth), + .insdel = true, + .update = rbac_bfd_update, + .n_update = ARRAY_SIZE(rbac_bfd_update), + .row = NULL },{ .table = NULL, .auth = NULL,
If a OVN deployment has OVN RBAC enabled for the southbound database, enabling BFD would lead to permission errors. The data in the entries in the BFD table do not belong to any given chassis and no column can provide authentication, but the rules still need to be there for successful operation. Fixes: 117203584d98 ("controller: introduce BFD tx path in ovn-controller.") Reported-at: https://bugs.launchpad.net/ubuntu/+source/ovn/+bug/1995771 Signed-off-by: Frode Nordahl <frode.nordahl@canonical.com> --- northd/ovn-northd.c | 13 +++++++++++++ 1 file changed, 13 insertions(+)