Merge lp:~jml/canonical-identity-provider/clean-up-temp into lp:canonical-identity-provider/release

Proposed by Jonathan Lange
Status: Merged
Approved by: Jonathan Lange
Approved revision: no longer in the source branch.
Merged at revision: 616
Proposed branch: lp:~jml/canonical-identity-provider/clean-up-temp
Merge into: lp:canonical-identity-provider/release
Diff against target: 70 lines (+41/-0)
2 files modified
scripts/run-acceptance-tests (+20/-0)
tarmac_tests.sh (+21/-0)
To merge this branch: bzr merge lp:~jml/canonical-identity-provider/clean-up-temp
Reviewer Review Type Date Requested Status
Natalia Bidart (community) Approve
James Westby (community) Approve
Review via email: mp+143537@code.launchpad.net

Commit message

Control the creation and deletion of /tmp/djopenid_consumer_store

Description of the change

Changes the tarmac_tests.sh and run-acceptance-tests scripts to control
the creation and deletion of /tmp/djopenid_consumer_store, which is used
by webui for storing openid consumer information.

The problem is that it gets created implicitly when webui gets a startOpenID
or finishOpenID request, and when it does, it is owned by whichever user
does this.

Thus, this changes the test scripts at least to make sure it's owned by the
'tarmac' group and that anyone in the group can at least clear out the
contents.

To post a comment you must log in.
Revision history for this message
Natalia Bidart (nataliabidart) wrote :

14:31 < nessita> jml: I'm looking at https://code.launchpad.net/~jml/canonical-identity-provider/clean-up-temp/+merge/143537, I was wondering if we can somehow avoid the duplication
                 of the code

review: Needs Information
Revision history for this message
James Westby (james-w) wrote :

Hi,

This looks ok to me, though I'm a little confused about why it's ok to remove the
dir at the end if it isn't at the beginning.

Natalia had a comment on IRC:

<nessita> jml: I'm looking at https://code.launchpad.net/~jml/canonical-identity-provider/clean-up-temp/+merge/143537, I was wondering if we can somehow avoid the duplication of the code

Thanks,

James

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

On Wed, Jan 16, 2013 at 11:28 PM, James Westby
<email address hidden> wrote:
> Review: Approve
>
> Hi,
>
> This looks ok to me, though I'm a little confused about why it's ok to remove the
> dir at the end if it isn't at the beginning.
>

That was me being confused. Fixed.

> Natalia had a comment on IRC:
>
> <nessita> jml: I'm looking at https://code.launchpad.net/~jml/canonical-identity-provider/clean-up-temp/+merge/143537, I was wondering if we can somehow avoid the duplication of the code
>

* We could write a couple of shell functions and source the file.
That would still require us to duplicate the structure, but would mean
slightly less duplicated code

* We could come up with something fundamentally different.

I'm really disinclined to do the first.

jml

Revision history for this message
Ubuntu One Server Tarmac Bot (ubuntuone-server-tarmac) wrote :

Voting does not meet specified criteria. Required: Approve >= 1, Disapprove == 0, Needs Fixing == 0, Needs Information == 0, Resubmit == 0, Pending == 0. Got: 1 Approve, 1 Needs Information.

Revision history for this message
Natalia Bidart (nataliabidart) wrote :

Fair enough, approving.

review: Approve
Revision history for this message
Ubuntu One Server Tarmac Bot (ubuntuone-server-tarmac) wrote :

The attempt to merge lp:~jml/canonical-identity-provider/clean-up-temp into lp:canonical-identity-provider failed. Below is the output from the failed tests.

./tarmac_tests.sh: 10: ./tarmac_tests.sh: [-d: not found
chgrp: changing group of `/tmp/djopenid_consumer_store': Operation not permitted

Revision history for this message
Ubuntu One Server Tarmac Bot (ubuntuone-server-tarmac) wrote :

The attempt to merge lp:~jml/canonical-identity-provider/clean-up-temp into lp:canonical-identity-provider failed. Below is the output from the failed tests.

rm: cannot remove `/tmp/djopenid_consumer_store/associations/http-localhost_3A16418-Vh0ConNmIqGOigGmK.Mc00BOquk-_dwT2ZqvZFHEYRQyRgJe733FXkc': Permission denied
rm: cannot remove `/tmp/djopenid_consumer_store/associations/http-localhost_3A16544-hZZgJmFgbPHilxfRyYrgzy4G.sA-vnuzwQDpYrcMNDU4mnTFdSrEFBQ': Permission denied
rm: cannot remove `/tmp/djopenid_consumer_store/nonces/50f7461d-http-localhost_3A16418-Vh0ConNmIqGOigGmK.Mc00BOquk-4RLxV0IAVc27Z4NI.abxwzU7eH8': Permission denied
rm: cannot remove `/tmp/djopenid_consumer_store/nonces/50f741d8-http-localhost_3A16418-Vh0ConNmIqGOigGmK.Mc00BOquk-62XAfFo4G1fJhy4XhyGfpPNI_Aw': Permission denied
rm: cannot remove `/tmp/djopenid_consumer_store/nonces/50f6b1bd-http-localhost_3A16544-hZZgJmFgbPHilxfRyYrgzy4G.sA-XWcEKQ3ErjqQ3OAGU.GcuHLSnO4': Permission denied
rm: cannot remove `/tmp/djopenid_consumer_store/nonces/50f6b1c5-http-localhost_3A16544-hZZgJmFgbPHilxfRyYrgzy4G.sA-qil8w_hsfI3Lz6XX1kfP5Zp.qPU': Permission denied
rm: cannot remove `/tmp/djopenid_consumer_store/nonces/50f741d0-http-localhost_3A16418-Vh0ConNmIqGOigGmK.Mc00BOquk-OqZkNnb8g4wU2VzJHGWpu1DpaVk': Permission denied
rm: cannot remove `/tmp/djopenid_consumer_store/nonces/50f6b1b5-http-localhost_3A16544-hZZgJmFgbPHilxfRyYrgzy4G.sA-M05K1MSX6s6ffkV5lwgjuis.lOA': Permission denied
rm: cannot remove `/tmp/djopenid_consumer_store/nonces/50f741e7-http-localhost_3A16418-Vh0ConNmIqGOigGmK.Mc00BOquk-aVEjEygZlMlHo1y_HxIOrvgHWxs': Permission denied
rm: cannot remove `/tmp/djopenid_consumer_store/nonces/50f6b1ac-http-localhost_3A16544-hZZgJmFgbPHilxfRyYrgzy4G.sA-mkn2y6v98L9EXqClqk4hjHR4aZI': Permission denied
rm: cannot remove `/tmp/djopenid_consumer_store/nonces/50f741be-http-localhost_3A16418-Vh0ConNmIqGOigGmK.Mc00BOquk-Xp1OmfbzFAVsGCt4.ricO9BQVhU': Permission denied
rm: cannot remove `/tmp/djopenid_consumer_store/nonces/50f741e0-http-localhost_3A16418-Vh0ConNmIqGOigGmK.Mc00BOquk-QzMSsEI32C65Afj7si2dKoRqauE': Permission denied
rm: cannot remove `/tmp/djopenid_consumer_store/nonces/50f6b1d4-http-localhost_3A16544-hZZgJmFgbPHilxfRyYrgzy4G.sA-QFyeJTs8nh3KaW189ixTe_ykbaE': Permission denied
rm: cannot remove `/tmp/djopenid_consumer_store/nonces/50f6b1dc-http-localhost_3A16544-hZZgJmFgbPHilxfRyYrgzy4G.sA-j_clEpqQhwLqSyccghkM6zQDiRQ': Permission denied
rm: cannot remove `/tmp/djopenid_consumer_store/nonces/50f6b601-http-localhost_3A16544-hZZgJmFgbPHilxfRyYrgzy4G.sA-rDnGe.tWHgAbtthKDaLD0B7NJbA': Permission denied
rm: cannot remove `/tmp/djopenid_consumer_store/nonces/50f6b1cd-http-localhost_3A16544-hZZgJmFgbPHilxfRyYrgzy4G.sA-fHWcxwVs7yd26sZNPIpzXHreJC8': Permission denied
rm: cannot remove `/tmp/djopenid_consumer_store/nonces/50f741f0-http-localhost_3A16418-Vh0ConNmIqGOigGmK.Mc00BOquk-GKdaLqUvThenfw51ylTq2WDEBcY': Permission denied
rm: cannot remove `/tmp/djopenid_consumer_store/nonces/50f741c7-http-localhost_3A16418-Vh0ConNmIqGOigGmK.Mc00BOquk-ly0OZnWFRzxd0f.hAK6AyHgts9w': Permission denied
rm: cannot remove `/tmp/djopenid_consumer_store/temp': Permission denied

Revision history for this message
Ubuntu One Server Tarmac Bot (ubuntuone-server-tarmac) wrote :

The attempt to merge lp:~jml/canonical-identity-provider/clean-up-temp into lp:canonical-identity-provider failed. Below is the output from the failed tests.

rm: cannot remove `/tmp/djopenid_consumer_store/associations/http-localhost_3A16418-Vh0ConNmIqGOigGmK.Mc00BOquk-_dwT2ZqvZFHEYRQyRgJe733FXkc': Permission denied
rm: cannot remove `/tmp/djopenid_consumer_store/associations/http-localhost_3A16544-hZZgJmFgbPHilxfRyYrgzy4G.sA-vnuzwQDpYrcMNDU4mnTFdSrEFBQ': Permission denied
rm: cannot remove `/tmp/djopenid_consumer_store/nonces/50f7461d-http-localhost_3A16418-Vh0ConNmIqGOigGmK.Mc00BOquk-4RLxV0IAVc27Z4NI.abxwzU7eH8': Permission denied
rm: cannot remove `/tmp/djopenid_consumer_store/nonces/50f741d8-http-localhost_3A16418-Vh0ConNmIqGOigGmK.Mc00BOquk-62XAfFo4G1fJhy4XhyGfpPNI_Aw': Permission denied
rm: cannot remove `/tmp/djopenid_consumer_store/nonces/50f6b1bd-http-localhost_3A16544-hZZgJmFgbPHilxfRyYrgzy4G.sA-XWcEKQ3ErjqQ3OAGU.GcuHLSnO4': Permission denied
rm: cannot remove `/tmp/djopenid_consumer_store/nonces/50f6b1c5-http-localhost_3A16544-hZZgJmFgbPHilxfRyYrgzy4G.sA-qil8w_hsfI3Lz6XX1kfP5Zp.qPU': Permission denied
rm: cannot remove `/tmp/djopenid_consumer_store/nonces/50f741d0-http-localhost_3A16418-Vh0ConNmIqGOigGmK.Mc00BOquk-OqZkNnb8g4wU2VzJHGWpu1DpaVk': Permission denied
rm: cannot remove `/tmp/djopenid_consumer_store/nonces/50f6b1b5-http-localhost_3A16544-hZZgJmFgbPHilxfRyYrgzy4G.sA-M05K1MSX6s6ffkV5lwgjuis.lOA': Permission denied
rm: cannot remove `/tmp/djopenid_consumer_store/nonces/50f741e7-http-localhost_3A16418-Vh0ConNmIqGOigGmK.Mc00BOquk-aVEjEygZlMlHo1y_HxIOrvgHWxs': Permission denied
rm: cannot remove `/tmp/djopenid_consumer_store/nonces/50f6b1ac-http-localhost_3A16544-hZZgJmFgbPHilxfRyYrgzy4G.sA-mkn2y6v98L9EXqClqk4hjHR4aZI': Permission denied
rm: cannot remove `/tmp/djopenid_consumer_store/nonces/50f741be-http-localhost_3A16418-Vh0ConNmIqGOigGmK.Mc00BOquk-Xp1OmfbzFAVsGCt4.ricO9BQVhU': Permission denied
rm: cannot remove `/tmp/djopenid_consumer_store/nonces/50f741e0-http-localhost_3A16418-Vh0ConNmIqGOigGmK.Mc00BOquk-QzMSsEI32C65Afj7si2dKoRqauE': Permission denied
rm: cannot remove `/tmp/djopenid_consumer_store/nonces/50f6b1d4-http-localhost_3A16544-hZZgJmFgbPHilxfRyYrgzy4G.sA-QFyeJTs8nh3KaW189ixTe_ykbaE': Permission denied
rm: cannot remove `/tmp/djopenid_consumer_store/nonces/50f6b1dc-http-localhost_3A16544-hZZgJmFgbPHilxfRyYrgzy4G.sA-j_clEpqQhwLqSyccghkM6zQDiRQ': Permission denied
rm: cannot remove `/tmp/djopenid_consumer_store/nonces/50f6b601-http-localhost_3A16544-hZZgJmFgbPHilxfRyYrgzy4G.sA-rDnGe.tWHgAbtthKDaLD0B7NJbA': Permission denied
rm: cannot remove `/tmp/djopenid_consumer_store/nonces/50f6b1cd-http-localhost_3A16544-hZZgJmFgbPHilxfRyYrgzy4G.sA-fHWcxwVs7yd26sZNPIpzXHreJC8': Permission denied
rm: cannot remove `/tmp/djopenid_consumer_store/nonces/50f741f0-http-localhost_3A16418-Vh0ConNmIqGOigGmK.Mc00BOquk-GKdaLqUvThenfw51ylTq2WDEBcY': Permission denied
rm: cannot remove `/tmp/djopenid_consumer_store/nonces/50f741c7-http-localhost_3A16418-Vh0ConNmIqGOigGmK.Mc00BOquk-ly0OZnWFRzxd0f.hAK6AyHgts9w': Permission denied
rm: cannot remove `/tmp/djopenid_consumer_store/temp': Permission denied

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'scripts/run-acceptance-tests'
2--- scripts/run-acceptance-tests 2013-01-14 19:04:29 +0000
3+++ scripts/run-acceptance-tests 2013-01-17 14:57:20 +0000
4@@ -16,6 +16,21 @@
5 CONFIG_BRANCH=$2
6 FLAGS=$3
7
8+# This gets created during the test run by webui, and needs to be deleted (or
9+# at least, delete-able) when the test run finishes. Thus, make it group
10+# writeable.
11+OPENID_TMP_DIR=/tmp/djopenid_consumer_store
12+if [ -d $OPENID_TMP_DIR ]; then
13+ # Delete the contents but not the directory because we might
14+ # not have been the ones who made it and /tmp is sticky by
15+ # default.
16+ rm -rf $OPENID_TMP_DIR/*
17+else
18+ mkdir -p $OPENID_TMP_DIR --mode 2775
19+ if id -g tarmac >/dev/null 2>&1; then
20+ chgrp tarmac $OPENID_TMP_DIR
21+ fi
22+fi
23
24 if [ "$TARGET" = "production" ]; then
25 SST_BASE_URL=https://login.ubuntu.com
26@@ -54,3 +69,8 @@
27 else
28 SST_BASE_URL=$SST_BASE_URL fab acceptance:screenshot=true,report=xml,extended=true,flags=$FLAGS
29 fi
30+
31+# Delete the contents but not the directory because we might
32+# not have been the ones who made it and /tmp is sticky by
33+# default.
34+rm -rf $OPENID_TMP_DIR/*
35
36=== modified file 'tarmac_tests.sh'
37--- tarmac_tests.sh 2013-01-04 16:50:54 +0000
38+++ tarmac_tests.sh 2013-01-17 14:57:20 +0000
39@@ -3,6 +3,22 @@
40
41 set -e
42
43+# This gets created during the test run by webui, and needs to be deleted (or
44+# at least, delete-able) when the test run finishes. Thus, make it group
45+# writeable.
46+OPENID_TMP_DIR=/tmp/djopenid_consumer_store
47+if [ -d $OPENID_TMP_DIR ]; then
48+ # Delete the contents but not the directory because we might
49+ # not have been the ones who made it and /tmp is sticky by
50+ # default.
51+ rm -rf $OPENID_TMP_DIR/*
52+else
53+ mkdir -p $OPENID_TMP_DIR --mode 2775
54+ if id -g tarmac >/dev/null 2>&1; then
55+ chgrp tarmac $OPENID_TMP_DIR
56+ fi
57+fi
58+
59 LOG_FILE=log
60
61 echo "Running canonical-identity-provider tests in tarmac"
62@@ -15,3 +31,8 @@
63
64 # run tests
65 fab test
66+
67+# Delete the contents but not the directory because we might
68+# not have been the ones who made it and /tmp is sticky by
69+# default.
70+rm -rf $OPENID_TMP_DIR/*