Code review comment for lp:~wallyworld/juju-core/instance-default-secgroup

On 2014/01/09 19:48:46, fwereade wrote:
> On 2014/01/09 12:56:35, gz wrote:
> > I added a comment to bug 1129720 about the general approach.
> >
> > I agree with William that adding more config here isn't really
desirable, but
> > just putting the default group on all machines juju creates is a
step back in
> > our general isolation level. The per-environment security group is a
much
> > cleaner place to add custom rules, even if we don't supply a juju
command for
> > doing that (though we could).
> >
> > Code itself looks fine.

> I dunno, I think there's quite a neat separation between the default
security
> group -- with whatever cloud-specific rules, considered appropriate by
the
> administrator, that are kinda outside our purview -- and our own
config. What's
> the use case for *not* including the default security group? Nobody's
needed it
> before, sure; but the default behaviour doesn't look harmful, and if
that's been
> changed it seems reasonable to assume it's been done for a reason.
Right?

> Sorry for the delay, Ian, I had an all-day power cut -- can we chat
about this
> tomorrow morning? I'm fine with the code, indeed, but I feel like
including that
> security group is the correct default, and I'd be happiest of all if
we just
> skipped the config, but I'll accept pretty much any plausible use case
that
> requires its falsity as justification for including the setting.

LGTM after discussion -- we've been fine without it so far, so let's
stick with the existing default. Thanks for bearing with me.

https://codereview.appspot.com/44770044/