changing trusted dkim domains requires code deployment

Bug #643224 reported by Martin Pool
20
This bug affects 3 people
Affects Status Importance Assigned to Milestone
Launchpad itself
Won't Fix
Low
Unassigned

Bug Description

At the moment there is a hardcoded list _trusted_dkim_domains in mail/incoming.py. It might be better if this was taken from a dynamic configuration source rather than the source. (It's probably only worth changing if the new location is substantially easier to change than the source tree.)

Revision history for this message
Robert Collins (lifeless) wrote : Re: [Bug 643224] [NEW] dkim whitelist should be in configuration

/me whispers ...flags...

Revision history for this message
Gary Poster (gary) wrote : Re: dkim whitelist should be in configuration

How often, and how quickly, do we need to change this? Do we have tests associated with these domains?

If doing it dynamically is in fact a good idea, then yeah, flags sound good.

Changed in launchpad-foundations:
status: Confirmed → Incomplete
Revision history for this message
Martin Pool (mbp) wrote :

I think it's plausible we'd want to change them dynamically. On the positive side, scottk said "please activate this from my domain" and it would be nice to be able to turn it on easily. On the other hand if there turns out to be a problem with any domain or with dkim in general it would be good to let us quickly turn it off.

A code rollout to change configuration seems pretty laborious to me so I think we should put it into dynamic configuration unless there's a good reason not to. If there's a perceived/actual downside to using flags let's describe that and see if it can be addressed. I can see two:

 * if we configure many things as flags, the current ui for controlling them may get unwieldy (but it can be improved)
 * defaulting to empty may cause variation on new deployments: but I guess we could always have the code say "if nothing is configured, trust these hardcoded domains".

I don't think this impacts testing: I think the tests stub this out and in any case there are facilities to run tests with particular flags set (bug 645768).

tags: added: feature-flags
Changed in launchpad-foundations:
status: Incomplete → Confirmed
Curtis Hovey (sinzui)
Changed in launchpad:
status: Confirmed → Triaged
Revision history for this message
Martin Pool (mbp) wrote :

If/when I get to this, I will probably take this approach: have a
hardcoded list of "generally ok" domains, and allow that to be
overridden by a feature flag. Then we have a scram switch in the case
that we need to suddenly stop trusting one domain, or if we want to
try out a new one.

Revision history for this message
Jonathan Lange (jml) wrote :

Sounds good to me, especially if the defaults are visible in the documentation for the flag.

Revision history for this message
Robert Collins (lifeless) wrote :

My 2c: don't have two places to configure the same thing. Thats confusing and liable to lead to errors. E.g. does the flag config supplement or entirely replace the configured setting. If the former, we can't disable a configured domain. If the latter sysadmins need to copy the *entire* config in when setting a flag. That sort of thing.

Revision history for this message
Martin Pool (mbp) wrote :

After discussing this with Robert, we'll remove it from the code, add a flag, and deploy a flag setting that will keep the currently trusted domains. Then we have just one place to change it.

I was planning to do this by adding a scope that means "mail from domain X", then we can use that to say both whether dkim is trusted, and potentially other things in future.

Martin Pool (mbp)
Changed in launchpad:
status: Triaged → In Progress
importance: Low → High
assignee: nobody → Martin Pool (mbp)
Revision history for this message
Gavin Panella (allenap) wrote :

What are the advantages of using flags as opposed to, say, a db table,
administered via the web service?

Revision history for this message
Julian Edwards (julian-edwards) wrote :

I was also surprised that flags were mentioned instead of storing this in a table.

Revision history for this message
Robert Collins (lifeless) wrote : Re: [Bug 643224] Re: dkim whitelist should be in configuration

On Wed, Feb 23, 2011 at 2:25 AM, Gavin Panella
<email address hidden> wrote:
> What are the advantages of using flags as opposed to, say, a db table,
> administered via the web service?

I think its 6/1 1/2 dozen of the other. No reason flags can't be
exposed on the web service, and theres not much reason to model this
as an explicit table - nor much reason not to.

Flags are a table themselves ;)

Revision history for this message
Martin Pool (mbp) wrote :

On 23 February 2011 00:25, Gavin Panella <email address hidden> wrote:
> What are the advantages of using flags as opposed to, say, a db table,
> administered via the web service?

+ it doesn't need a new db table, and doesn't need a rollout
+ seems like a smaller change
+ don't need to write a special gui or client tool to set them
+ we get an audit trail and a web view of it for free from sinzui's work
+ by making all types of dynamic configuration be in the same place we
perhaps make lp easier to change and administer

disadvantages:

- storing them as text in a more generic table is a bit denormalized
- arguably this is a different kind of configuration to "turn this
feature on or off in this scope" because it will probably never be
appropriate to turn it on across the board, so perhaps it ought to
live somewhere different
- the editor gui has no domain knowledge or validation — but it could
grow it in future based on the scope/rule metadata

Revision history for this message
Martin Pool (mbp) wrote : Re: dkim whitelist should be in configuration
Changed in launchpad:
status: In Progress → Confirmed
Changed in launchpad:
status: Confirmed → In Progress
Revision history for this message
Martin Pool (mbp) wrote :

I asked offline if this would be better off in zcml or other kinds of configuration other than flags, and Robert's answer (for the record) was they're much harder and slower to update, so no.

Martin Pool (mbp)
Changed in launchpad:
importance: High → Low
Revision history for this message
Martin Pool (mbp) wrote :
Revision history for this message
Martin Pool (mbp) wrote :

unassigning: I don't have any code towards this that's not already covered by other flags or dkim branches; it's not at the top of my queue; it's also not critical to finishing off the dkim story.

Changed in launchpad:
assignee: Martin Pool (mbp) → nobody
status: In Progress → Triaged
Martin Pool (mbp)
summary: - dkim whitelist should be in configuration
+ changing trusted dkim domains requires code deployment
Jürgen Gmach (jugmac00)
Changed in launchpad:
status: Triaged → Won't Fix
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Related questions

Remote bug watches

Bug watches keep track of this bug in other bug trackers.