[freeside-devel] Re: svc_broadband merge to HEAD

[ivan at 420.am]

On Thu, Sep 12, 2002 at 10:50:40AM -0700, Mark Wells wrote:
> 
> On Wed, 11 Sep 2002 ivan at 420.am wrote:
> 
> > Hmmmmmmmmmmmm....
> >
> > I agree svc_* tables could really benefit from "add your own" fields.
> >
> > I'm not sure about tying the custom fields to exports (part_acct_field).
> > Can't they just be defined as global custom fields for a particular
> > svc_* table?
> 
> They might not be appropriate to all the things that that svc_* table is
> used for.

That's okay.  They can be enabled or disabled in the service definition.

>  Look at svc_acct: we're using it for dialup, email 'boxes',
> email aliases, shell accounts, and, any day now, broadband services that
> use RADIUS for authentication.  All of these functions need different sets
> of pseudo-fields.  It doesn't make any sense to assign a circuit ID to a
> shell account or a RADIUS option of any kind to an email alias.

Agreed, but that's a problem with many solutions.  I'm not sure tying
svc_* pseudo-fields to exports is a good one.

> Attaching the pseudo-fields to exports provides some way to make this
> distinction.

I'm not sure it is a good way to make the distinction.  Can't we let
users decide which fields apply at the service definition level?

> If this particular svc_acct is only going to be exported to
> a shell host, the irrelevant fields simply aren't available.

This introduces a *lot* of complexity in the cases where you add and
remove exports from service definitions.  The possibility of
username/domain/uid here is already going to be a serious pain to handle
- I'd rather not handle the complexity introduced by having exports
define pseudo-fields...

What are the advantages of tying svc_* pseudo-fields to exports, as
opposed to making them globals that are enabled/disabled per service
definition?

> Since the pseudo-fields are user-configurable, they're not guaranteed to
> be there, or to be of the correct types.  They're not in the schema, like
> real fields.  So it seems that any code that uses them should be
> encapsulated, so that we can say (in the documentation if nowhere else)
> "If you want to use FS::part_export::alcatel_dslam you must create the
> following extended fields in svc_acct: ..."

This could be automated.  I wouldn't be against a weaker association
between exports and svc_* pseudo-fields, where adding an export may
enable some necessary pseudo-fields... but I wouldn't want removing an
export to change anything, or prevent folks from enabling fields anyway.

> > > It really seems that ALL of the fields in svc_acct (except svcnum and
> > > maybe username) are export fields.  uid and gid, for example, don't make
> > > much sense for any kind of svc_acct except one that represents a shell
> > > account.
> >
> > I'm don't think I'm ready to move existing fields (uid, gid, etc.) to
> > such a mechanism yet...  It would be nice if we designed the API so that
> > this would be an internal change, though (i.e. make all custom fields
> > available as "pseudo-fields" in svc_whatever.pm).
> 
> That's what I was hoping to do.  The existing fields don't have to move
> immediately.  If they're inappropriate for, say, a broadband svcpart, they
> can be given fixed values and ignored.
>
> > > What I'm proposing is to move the export fields into a separate table
> > > (acct_field), like we were going to do with sbp_field.  This subsumes the
> > > functions of both sbp_field and the RADIUS attribute table.
> >
> > RADIUS attributes feel wrong mixed up with the custom fields, for some
> > reason, though.... For some reason it still feels like we should have
> > RADIUS attributes in their own table, like the schema I described in my
> > other email...  unlike other custom fields, they have a specific meaning
> > and come out of a dictionary.
> 
> They shouldn't be 'mixed up' with the other fields. That's one of the
> advantages of tying them to the export.  Only svcparts that get exported
> to RADIUS can have RADIUS attributes.

"mixed up" as in "in the same table with".  I think RADIUS attributes
should go in a table that has nothing but RADIUS attributes.  They're
specifically indexed by FS::raddb unlike other pseudo-fields.  FS::raddb
should become a real database table.  See ticket #456:
http://pouncequick.420.am/rt/Ticket/Display.html?id=456

> As for the enumerated values that many RADIUS attributes have, Kristian
> and I were thinking that the field definitions (part_acct_field, in this
> case) would have a 'check' field, which would be eval'd during
> svc_acct::check().  We could do something similar to get the list of
> allowed values; if that field was set, the pseudo-field would appear on
> forms as a drop-down box.  (That would make the form more complicated, as
> it would have to refresh after the user selected the pseudo-field name to
> get the appropriate list of values.)  This would work for RADIUS
> attributes or any other type of pseudo-field.

A worthwhile idea, but not what I'm talking about wrt RADIUS attributes.
I"m talking about the "keys", i.e. the attribute names, not the
"values" being an enum-type structure.

FWIW, I'm also not find of pages that refresh unexpectedly when you make
a form change.  It's bad UI.  Hence the HTML::Widgets::SelectLayers
stuff.

> > > It makes
> > > svc_broadband completely unnecessary; a broadband service is just a
> > > svc_acct that exports to a broadband device of some kind.
> >
> > I don't think this would make svc_broadband unnecessary... svc_broadband
> > seems to be appropriate for "network connections", and especially if
> > they aren't provisioned with a username in RADIUS, you want someplace to
> > keep their info, IP/netblock, and so on.  svc_broadband is for "network
> > connection things" whereas svc_acct is for "username things".
> 
> Those seem to be orthogonal concepts.

What do you mean?

> I don't understand the rationale for giving "whether it has a username" as
> much weight as it currently has in the classification of services.

It's about identifying common structure among the different things an
ISP provisions, and modeling them in a reasonable fashion.

We could just make the entire svc_* a free-for-all of user-defined
fields and exports, but I don't feel that would be as useful as
identifying common data elements and storing them in standard places.

So, yes, "anything with a username", i.e. "accounts" - RADIUS, mailbox,
or otherwise, is an svc_acct record.  If we have to add lots of fields
to svc_acct (pseudo- or otherwise) that aren't used in many cases, I
don't consider that a problem.  They can always be hidden in the UI.

> Web hosting services have usernames-- which are stored in svc_acct, because
> they're usernames.

No!  Web hosting services (svc_www) do NOT have usernames - they
represent web space, not accounts.  Web hosting services are USED in
conjunction with _accounts_ that have usernames (and passwords), but
they don't have usernames of their own.

In fact, there probably should be a many-to-many relationship between
accounts and hosting (multiple "owners" of a web site) instead of the
current one-to-many "one master account per web site".

> But the web hosting services themselves can't be,
> because svc_acct doesn't have a 'recnum' field.

Web hosting services are not stored in svc_acct because they're
fundamentally a different "thing" than accounts.

> Email forwarding services can have _two_ usernames, and they're
> stored as _two_ records in svc_acct, plus the one record in
> svc_forward because, again, svc_acct doesn't have enough fields.

Hmm.... email forwarding has always seemed like something "separate",
but it might be reasonable to just add dstsvc and dst fields to svc_acct
and drop svc_forward entirely.  I'm uncertain about that...

> We need _some place_ to store additional information about a broadband
> connection (like IP address, possibly MAC address or circuit ID, etc.).
> With svc_acct pseudo-fields, we can create a place, while avoiding
> gratuitous schema changes ("At our site we have to keep track of the
> password for each customer's router, so we want a svc_acct.router_password
> field") and proliferation of tables.

As I said, I'm all for svc_* pseudo-fields.  I just want them to be
globally defined for a particular svc_* table and enabled/disabed at the
service definition level, not tied up with exports.

-- 
_ivan