[ovs-dev] Read only versions of the *ctl binaries

[Ryan Moats]

Ben Pfaff <blp at ovn.org> wrote on 07/30/2016 01:38:27 PM:

> From: Ben Pfaff <blp at ovn.org>
> To: Ryan Moats/Omaha/IBM at IBMUS
> Cc: dev at openvswitch.org
> Date: 07/30/2016 01:38 PM
> Subject: Re: [ovs-dev] Read only versions of the *ctl binaries
>
> On Fri, Jul 29, 2016 at 05:35:31PM -0500, Ryan Moats wrote:
> > Ben Pfaff <blp at ovn.org> wrote on 07/29/2016 05:27:29 PM:
> >
> > > From: Ben Pfaff <blp at ovn.org>
> > > To: Ryan Moats/Omaha/IBM at IBMUS
> > > Cc: dev at openvswitch.org
> > > Date: 07/29/2016 05:27 PM
> > > Subject: Re: [ovs-dev] Read only versions of the *ctl binaries
> > >
> > > On Fri, Jul 29, 2016 at 04:11:00PM -0500, Ryan Moats wrote:
> > > >
> > > > We just received a new operational requirement that we have
> > > > to restrict access to all binaries that provide RW access to
> > > > infrastructure components, but yet still have the ability to
> > > > read current state from the infrastructure.
> > > >
> > > > For OVN/OVS, this means we won't be able to use the following
> > > > binaries in our production environment to read current state:
> > > > ovs-vsctl, ovs-dpctl, ovs-ofctl, ovs-appctl, ovn-nbctl, and
> > > > ovn-sbctl.
> > > >
> > > > I'm thinking of meeting this by creating new binaries
> > > > ovs-vsread, ovs-dpread, ovs-ofread, ovs-appread, ovn-nbread,
> > > > and ovn-sbread that would include the show, list, and search
> > > > commands from their RW brethren, but omit the various add
> > > > and del commands.
> > > >
> > > > Before I start crafting code, I wanted to see if folks can
> > > > think of a simpler way of meeting this new requirement...
> > >
> > > You could hard-code the 'dry_run' variable to true.
> > >
> >
> > Yes, that will certainly be quicker, and I can couple that with
> > some Makefile magic to allow the same source code to produce
> > both the *ctl and *read binaries (which lowers future
> > maintenance costs too)...
> >
> > The $64K question for the community is this idea acceptable?
> > The tl;dr; is that I'd rather not carry this type of change
> > around as a local patch, but I will if I have to...
>
> I'm not yet convinced that this is useful.  Is it a valuable feature or
> a bureaucratic requirement?

I'm going to say some of both (and hope that Kyle chimes in here
with better reasons if I forget something).

The bottom line is the interaction of two requirements for our
deployments (you can call these bureaucratic, but we've learned
the hard way that they are *absolute* when operating large
clouds, so we feel that they are valuable):

1) That all cloud deployments be "repeatable."

2) That all necessary telemetry information be collectable for
problem triage.

The upshot of the first is that an operator at a keyboard can't be
able to apply a "one-off" state change on a single machine, because
now the deployment of that machine (and therefore the cloud) is
"non-repeatable".  This practical aspect of this is that we aren't
going to give anyone access to any of the *ctl commands in their
current form.

On the other hand, the second means we have to give access to
the read only portions of all of the *ctl commands because, afaik,
they are the only things that can read the various state information
needed for triaging problems.

Now, rather than split the read write and read only portions of the
*ctl commands apart (because other parts of the community likely
operate their clouds differently), my thought was just to spin new
versions that contain just the read only pieces under a new name
and give our operation folks access to them.  But this also explains
my comment about carrying a local patch - we will *have* to do
something about this and I'd like the communities eyes on it because
(a) as I dig through the code, I'm finding places that I'm going to
question in terms of whether a command is RW or RO and (b) more
reviewing eyes are always better.

Hopefully the above explains the situation more clearly (and again,
Kyle, please jump in with more supporting details or other points
I may have forgotten).