Merge lp:~jtv/launchpad/bug-822640 into lp:launchpad

Proposed by Jeroen T. Vermeulen
Status: Merged
Approved by: Jeroen T. Vermeulen
Approved revision: no longer in the source branch.
Merged at revision: 13629
Proposed branch: lp:~jtv/launchpad/bug-822640
Merge into: lp:launchpad
Diff against target: 22 lines (+4/-0)
1 file modified
database/schema/security.cfg (+4/-0)
To merge this branch: bzr merge lp:~jtv/launchpad/bug-822640
Reviewer Review Type Date Requested Status
Benji York (community) code Approve
Review via email: mp+70730@code.launchpad.net

Commit message

[r=benji][bug=822640] Extra DB privileges publish-ftpmaster needs.

Description of the change

= Summary =

The new publish-ftpmaster script runs process-accepted and publish-distro in-process now, so needs the same privileges. Some of them are fairly obscure, hidden away in massive complexity and deep code paths so testing for them in advance was not a realistic proposition.

== Proposed fix ==

Four tables needed extra access privileges for the archivepublisher user.

== Pre-implementation notes ==

William was kind enough to run through the database roles and figure out what the differences amounted to. This was not easy because of structural differences in how their security configs were arranged.

Managing this kind of thing has gotten far out of hand.

== Tests ==

It should be possible to reconstruct test scenarios where these privileges are used, but to do so would probably incur significant costs in both development and in test run times. I don't believe such relatively small configuration changes are worth it; the worry is not that this might regress, but that we may have forgotten about another obscure code path that reads from another table.

== Demo and Q/A ==

Observe the publish-ftpmaster script runnin in production, at three minutes past every hour. Dogfood testing was not enough to turn up the missing SELECT privilege on TranslationGroup that we observed.

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  database/schema/security.cfg

To post a comment you must log in.
Revision history for this message
Benji York (benji) wrote :

Looks good.

review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'database/schema/security.cfg'
2--- database/schema/security.cfg 2011-08-02 09:57:41 +0000
3+++ database/schema/security.cfg 2011-08-08 12:48:34 +0000
4@@ -857,6 +857,7 @@
5 public.cve = SELECT, INSERT
6 public.distributionjob = SELECT, INSERT, DELETE
7 public.distributionsourcepackage = SELECT, INSERT, UPDATE
8+public.distroseriesdifference = SELECT
9 public.distroseriesparent = SELECT, INSERT, UPDATE
10 public.flatpackagesetinclusion = SELECT, INSERT, UPDATE, DELETE
11 public.gpgkey = SELECT, INSERT, UPDATE
12@@ -885,7 +886,10 @@
13 public.questionjob = SELECT, INSERT
14 public.questionsubscription = SELECT
15 public.sourcepackagepublishinghistory = SELECT, INSERT, UPDATE, DELETE
16+public.sourcepackagerecipebuild = SELECT
17+public.sourcepackagerecipebuildjob = SELECT, INSERT, UPDATE
18 public.structuralsubscription = SELECT
19+public.translationgroup = SELECT
20 public.validpersoncache = SELECT
21 public.validpersonorteamcache = SELECT
22 type=user