Merge lp:~hazmat/juju/states-with-principals into lp:juju

Proposed by Kapil Thangavelu on 2012-04-04
Status: Needs review
Proposed branch: lp:~hazmat/juju/states-with-principals
Merge into: lp:juju
Diff against target: 1680 lines (+501/-182) 26 files modified
To merge this branch: bzr merge lp:~hazmat/juju/states-with-principals
Reviewer Review Type Date Requested Status
juju hackers 2012-04-04 Pending
Review via email: mp+100783@code.launchpad.net

Description of the Change

Security integration with state package.

Provides for principals created when corrresponding states are
created that have agents associated.

https://codereview.appspot.com/5966076/

To post a comment you must log in.
Kapil Thangavelu (hazmat) wrote :

Reviewers: mp+100783_code.launchpad.net,

Message:
Please take a look.

Description:
Security integration with state package.

Provides for principals created when corrresponding states are
created that have agents associated.

https://code.launchpad.net/~hazmat/juju/states-with-principals/+merge/100783

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/5966076/

Affected files:
   A [revision details]
   M juju/agents/tests/common.py
   M juju/lib/testing.py
   M juju/providers/local/__init__.py
   M juju/providers/local/tests/test_provider.py
   M juju/state/base.py
   M juju/state/initialize.py
   M juju/state/machine.py
   M juju/state/relation.py
   M juju/state/security.py
   A juju/state/securityrules.py
   M juju/state/service.py
   M juju/state/tests/common.py
   M juju/state/tests/test_agent.py
   M juju/state/tests/test_base.py
   M juju/state/tests/test_errors.py
   M juju/state/tests/test_initialize.py
   M juju/state/tests/test_machine.py
   M juju/state/tests/test_relation.py
   M juju/state/tests/test_security.py
   M juju/state/tests/test_service.py
   M juju/unit/lifecycle.py

Kapil Thangavelu (hazmat) wrote :

gustavo's original review, most of the naming and api conventions have been cleaned up. http://paste.ubuntu.com/914526/

Gustavo Niemeyer (niemeyer) wrote :

As we discussed before, this looks like a pretty significant change for
that point in time.

Either way, if you want to go with it, we need a written down
description of what is going on here in terms of changes to the public
interfaces, as usual.

https://codereview.appspot.com/5966076/

Kapil Thangavelu (hazmat) wrote :

On Wed, Apr 4, 2012 at 10:32 AM, <email address hidden> wrote:

> As we discussed before, this looks like a pretty significant change for
> that point in time.
>
> Either way, if you want to go with it, we need a written down
> description of what is going on here in terms of changes to the public
> interfaces, as usual.
>
> https://codereview.appspot.**com/5966076/<https://codereview.appspot.com/5966076/>

It seems to be the number one concern regarding the failed MIR.

The implementation here is per the security spec discussed last fall,
incorporating feedback from the initial reviews. The spec is out of date
(uses ensemble instead of juju), and per feedback the OTP agent was dropped
for interceptable OTP tokens.

There aren't any public interface changes, just the imposition of ACLs onto
existing nodes.

I can update the spec and send it around to the list if you'd like.. but as
is, the components can effectively be merged as the default security
policy is permissive, ie no functional delta till the policy is activated.

-k

Gustavo Niemeyer (niemeyer) wrote :

On 2012/04/04 15:53:23, hazmat wrote:
> It seems to be the number one concern regarding the failed MIR.

My understanding is that the MIR has been dropped.

> The implementation here is per the security spec discussed last fall,
> incorporating feedback from the initial reviews. The spec is out of
date
> (uses ensemble instead of juju), and per feedback the OTP agent was
dropped
> for interceptable OTP tokens.

> There aren't any public interface changes, just the imposition of ACLs
onto
> existing nodes.

Please see the mailing list conversation about what "public changes"
means. The message subject is "Code reviews and public API changes".

> I can update the spec and send it around to the list if you'd like..
but as
> is, the components can effectively be merged as the default security
> policy is permissive, ie no functional delta till the policy is
activated.

The management of the Python code base is under your control, as we
agreed. What I'm concerned about is with changes that are landing
without being debated for 12.04 (hint: we *are* in 04!).

https://codereview.appspot.com/5966076/

Kapil Thangavelu (hazmat) wrote :

> On 2012/04/04 15:53:23, hazmat wrote:
> > It seems to be the number one concern regarding the failed MIR.
>
> My understanding is that the MIR has been dropped.
>

hence the term 'failed'

> > The implementation here is per the security spec discussed last fall,
> > incorporating feedback from the initial reviews. The spec is out of
> date
> > (uses ensemble instead of juju), and per feedback the OTP agent was
> dropped
> > for interceptable OTP tokens.
>
> > There aren't any public interface changes, just the imposition of ACLs
> onto
> > existing nodes.
>
> Please see the mailing list conversation about what "public changes"
> means. The message subject is "Code reviews and public API changes".

>
> > I can update the spec and send it around to the list if you'd like..
> but as
> > is, the components can effectively be merged as the default security
> > policy is permissive, ie no functional delta till the policy is
> activated.
>
> The management of the Python code base is under your control, as we
> agreed. What I'm concerned about is with changes that are landing
> without being debated for 12.04 (hint: we *are* in 04!).

ic. we've publicly debated these changes previously last fall, i believe your saying that that process needs to be done again?

Gustavo Niemeyer (niemeyer) wrote :

On Wed, Apr 4, 2012 at 13:10, Kapil Thangavelu
<email address hidden> wrote:
>> On 2012/04/04 15:53:23, hazmat wrote:
>> > It seems to be the number one concern regarding the failed MIR.

>> My understanding is that the MIR has been dropped.

> hence the term 'failed'

If it's failed, it's not an argument to have changes going in now.

> ic. we've publicly debated these changes previously last fall,
> i believe your saying that that process needs to be done again?

Sorry, I don't want to frustrate you at all, but try to look from
another perspective: there's a release of Ubuntu *this month*, and we're
rushing with a Go implementation to catch up meanwhile. Then, there's
that massive change set that has seen zero debate in the mailing list. I
hope it's not a surprise that I'm asking more details.

Besides that, where's that previously reviewed document that you
mention? It's not linked from this proposal, and I can't find it
anywhere.

https://codereview.appspot.com/5966076/

Kapil Thangavelu (hazmat) wrote :

On Wed, Apr 4, 2012 at 12:43 PM, <email address hidden> wrote:

> On Wed, Apr 4, 2012 at 13:10, Kapil Thangavelu
> <kapil.thangavelu@canonical.**com <email address hidden>> wrote:
>
>> On 2012/04/04 15:53:23, hazmat wrote:
>>> > It seems to be the number one concern regarding the failed MIR.
>>>
>>
> My understanding is that the MIR has been dropped.
>>>
>>
>
> hence the term 'failed'
>>
>
> If it's failed, it's not an argument to have changes going in now.
>
> ic. we've publicly debated these changes previously last fall,
>> i believe your saying that that process needs to be done again?
>>
>
> Sorry, I don't want to frustrate you at all, but try to look from
> another perspective: there's a release of Ubuntu *this month*, and we're
> rushing with a Go implementation to catch up meanwhile. Then, there's
> that massive change set that has seen zero debate in the mailing list. I
> hope it's not a surprise that I'm asking more details.
>
> Besides that, where's that previously reviewed document that you
> mention? It's not linked from this proposal, and I can't find it
> anywhere.
>
> https://codereview.appspot.**com/5966076/<https://codereview.appspot.com/5966076/>

The mir failed in part because of lack of appropriate security, and the
security concerns remain an issue to adoption.

for reference this was the original spec
https://code.launchpad.net/~hazmat/juju/security-specification/+merge/63921

it was discussed in depth @ austin sprint
http://pad.ubuntu.com/cUU8nlRZ4U

i mispoke when i said it was previously discussed on list, while its
discussed in the context of several other threads, i don't see a dedicated
thread to it offhand.

what i'm asking for is direction wrt to your comments.. i've updated the
security branches and cleaned them up. If you'd like me to bring it up on
list, i'm happy to push the spec there. If its something that should be
dropped.. well then pls say that.. at this point clearly i'm not too
concerned about wasting time effort on dead code.

-k

Gustavo Niemeyer (niemeyer) wrote :

On 2012/04/04 17:43:36, hazmat wrote:
> The mir failed in part because of lack of appropriate security, and
the
> security concerns remain an issue to adoption.

A broken implementation is a much more serious concern for adoption.

> for reference this was the original spec

https://code.launchpad.net/%7Ehazmat/juju/security-specification/+merge/63921

"Needs Fixing on 2011-06-09"

> what i'm asking for is direction wrt to your comments.. i've updated
the
> security branches and cleaned them up. If you'd like me to bring it up
on
> list, i'm happy to push the spec there. If its something that should
be
> dropped.. well then pls say that.. at this point clearly i'm not too
> concerned about wasting time effort on dead code.

Do you want me to say one more time? Sure: I think it is a bad idea to
be merging massive changes for 12.04 at this stage. I would not do it. I
would not be merging all those significant changes right before a
release. It's supposed to be stable. Let's please not ship crack in
12.04.

https://codereview.appspot.com/5966076/

Clint Byrum (clint-fewbar) wrote :

Excerpts from Gustavo Niemeyer's message of Wed Apr 04 17:52:20 UTC 2012:
> On 2012/04/04 17:43:36, hazmat wrote:
> > The mir failed in part because of lack of appropriate security, and
> the
> > security concerns remain an issue to adoption.
>
> A broken implementation is a much more serious concern for adoption.
>

I think they are equally serious, and adoption will suffer on the backend
(as people get more serious) if the security is not handled. One side of
me says we should at least ship an insecure toy, so that people can try
it out. But the other hand says we already did that in 11.10, and doing
so again would only waste peoples' time who are actively looking to deploy
with juju. Perhaps its better that we ship secure with bugs than insecure
with less bugs, and push hard to fix those problems as they are found.

> > for reference this was the original spec
>
> https://code.launchpad.net/%7Ehazmat/juju/security-specification/+merge/63921
>
> "Needs Fixing on 2011-06-09"
>
> > what i'm asking for is direction wrt to your comments.. i've updated
> the
> > security branches and cleaned them up. If you'd like me to bring it up
> on
> > list, i'm happy to push the spec there. If its something that should
> be
> > dropped.. well then pls say that.. at this point clearly i'm not too
> > concerned about wasting time effort on dead code.
>
> Do you want me to say one more time? Sure: I think it is a bad idea to
> be merging massive changes for 12.04 at this stage. I would not do it. I
> would not be merging all those significant changes right before a
> release. It's supposed to be stable. Let's please not ship crack in
> 12.04.
>

I agree, I have been hoping we can take a mitigation strategy with some
lighter weight methods for the impending release. While its clear we're
not going to be in main, we should not ship something in universe or in
our PPA that will expose users to security problems in a non obvious way.

The problems in the late-landing features in the version we shipped for
11.10 should have taught us that we should really respect feature freeze
and spend that time testing rather than refactoring or adding on.

Perhaps we can defer the large impact that this work carries until
after 12.04 releases, and instead focus on just fixing the "wide open
zookeeper" problem with a minimal patch that just adds a basic ACL like
"anonymous can't do anything", and be able to pass generated credentials
to each node?

This would go a long way to making this version of juju safe for
production use, which will go a long way to people using and contributing
charms. Then we can look at shipping something either in an SRU if it
is appropriate for that, or if it breaks behavior, in a new "stable"
PPA for 12.04 users.

Gustavo Niemeyer (niemeyer) wrote :

On Wed, Apr 4, 2012 at 15:43, Clint Byrum <email address hidden> wrote:
> I think they are equally serious, and adoption will suffer on the
backend
> (as people get more serious) if the security is not handled. One side
of
> me says we should at least ship an insecure toy, so that people can
try
> it out. But the other hand says we already did that in 11.10, and
doing
> so again would only waste peoples' time who are actively looking to
deploy
> with juju. Perhaps its better that we ship secure with bugs than
insecure
> with less bugs, and push hard to fix those problems as they are found.

This was a great topic for us to have had at the last UDS. It's now
time to ship 12.04, which was supposed to be a stable release, in my
humble opinion.

> Perhaps we can defer the large impact that this work carries until
> after 12.04 releases, and instead focus on just fixing the "wide open
> zookeeper" problem with a minimal patch that just adds a basic ACL
like
> "anonymous can't do anything", and be able to pass generated
credentials
> to each node?

That sounds less complex and thus preferable as an entry point. It was
once the whole point of that never-used admin-secret setting, by the
way.

That said, I'd still postpone that to 12.04.1. It's time to stop
changing
juju and preparing for an actual release. I can use juju in EC2 knowing
zookeeper is open. I won't use juju anywhere if it breaks all the time.

https://codereview.appspot.com/5966076/

Kapil Thangavelu (hazmat) wrote :

On Wed, Apr 4, 2012 at 1:50 PM, <email address hidden> wrote:

> On 2012/04/04 17:43:36, hazmat wrote:
>
>> The mir failed in part because of lack of appropriate security, and
>>
> the
>
>> security concerns remain an issue to adoption.
>>
>
> A broken implementation is a much more serious concern for adoption.

its unclear how the implementation which has yet to land got classified as
broken.

Gustavo Niemeyer (niemeyer) wrote :

On 2012/04/04 19:02:10, hazmat wrote:
> its unclear how the implementation which has yet to land got
classified as
> broken.

Kapil, it's your take. I'm not going to be the bad guy again. If you
want to release this work in 12.04, just provide an up-to-date document
explaining in detail what's being suggested as far as the external
interactions go, and I'll review.

https://codereview.appspot.com/5966076/

Kapil Thangavelu (hazmat) wrote :

On 2012/04/04 19:13:54, niemeyer wrote:
> On 2012/04/04 19:02:10, hazmat wrote:
> > its unclear how the implementation which has yet to land got
classified as
> > broken.

> Kapil, it's your take. I'm not going to be the bad guy again. If you
want to
> release this work in 12.04, just provide an up-to-date document
explaining in
> detail what's being suggested as far as the external interactions go,
and I'll
> review.

all right given the effort and timeline it seems more important to work
on bug fixes for 12.04 instead of this. i'll bring it back up post
release for 12.04.1

http://codereview.appspot.com/5966076/

335. By Kapil Thangavelu on 2012-08-09

security test tweaks

336. By Kapil Thangavelu on 2012-08-12

merge trunk

337. By Kapil Thangavelu on 2012-08-27

switch over some remaining test uses of txzk deletetree for juju state async impl of the test helper

Unmerged revisions

337. By Kapil Thangavelu on 2012-08-27

switch over some remaining test uses of txzk deletetree for juju state async impl of the test helper

336. By Kapil Thangavelu on 2012-08-12

merge trunk

335. By Kapil Thangavelu on 2012-08-09

security test tweaks

334. By Kapil Thangavelu on 2012-04-02

security api cleanup

333. By Kapil Thangavelu on 2012-04-02

remove extant refs to old node security adapter

332. By Kapil Thangavelu on 2012-04-02

replace security facade with usage of concrete classes

331. By Kapil Thangavelu on 2012-04-01

remove extraneous auth waits, attach handles it

330. By Kapil Thangavelu on 2012-04-01

refactor some names for clarity

329. By Kapil Thangavelu on 2012-04-01

fix local provider bootstrap

328. By Kapil Thangavelu on 2012-03-31

merge trunk

Preview Diff

1=== modified file 'juju/agents/tests/common.py'
2--- juju/agents/tests/common.py 2012-03-27 23:22:28 +0000
3+++ juju/agents/tests/common.py 2012-08-28 16:13:21 +0000
4@@ -27,13 +27,11 @@
5 self.agent.configure(self.options)
6 self.agent.set_watch_enabled(False)
7
8+ @inlineCallbacks
9 def tearDown(self):
10 if self.agent.client and self.agent.client.connected:
11 self.agent.client.close()
12-
13- if self.client.connected:
14- deleteTree("/", self.client.handle)
15- self.client.close()
16+ yield super(AgentTestBase, self).tearDown()
17
18 def get_agent_config(self):
19 options = TwistedOptionNamespace()
20
21=== modified file 'juju/charm/tests/test_publisher.py'
22--- juju/charm/tests/test_publisher.py 2012-04-06 16:35:31 +0000
23+++ juju/charm/tests/test_publisher.py 2012-08-28 16:13:21 +0000
24@@ -6,8 +6,6 @@
25 from twisted.internet.defer import inlineCallbacks, fail
26 from twisted.python.failure import Failure
27
28-from txzookeeper.tests.utils import deleteTree
29-
30 from juju.charm.bundle import CharmBundle
31 from juju.charm.directory import CharmDirectory
32 from juju.charm.publisher import CharmPublisher
33@@ -19,6 +17,7 @@
34
35 from juju.environment.tests.test_config import (
36 EnvironmentsConfigTestBase, SAMPLE_ENV)
37+from juju.state.tests.common import deleteTree
38 from juju.lib.mocker import MATCH
39
40 from .test_repository import RepositoryTestBase
41@@ -60,8 +59,9 @@
42 yield self.client.connect()
43 yield self.client.create("/charms")
44
45+ @inlineCallbacks
46 def tearDown(self):
47- deleteTree("/", self.client.handle)
48+ yield deleteTree(self.client)
49 self.client.close()
50 super(CharmPublisherTest, self).tearDown()
51
52
53=== modified file 'juju/lib/testing.py'
54--- juju/lib/testing.py 2012-08-24 23:00:50 +0000
55+++ juju/lib/testing.py 2012-08-28 16:13:21 +0000
56@@ -121,6 +121,24 @@
57 def assertInstance(self, instance, type):
58 self.assertTrue(isinstance(instance, type))
59
60+ def assertACE(self, acl, token=None, name=None):
61+ """Assert an ACE exists for the given identity token."""
62+ found = False
63+
64+ assert token or name
65+
66+ for ace in acl:
67+ if token and ace["id"] == token:
68+ found = True
69+ break
70+ elif name and ace["id"].split(":")[0] == name:
71+ found = True
72+ break
73+
74+ if not found:
75+ self.fail("Identity token %r not found in ACL %s" % (
76+ token, acl))
77+
78 def assertLogLines(self, observed, expected):
79 """Asserts that the lines of `expected` exist in order in the log."""
80 logged = observed.split("\n")
81
82=== modified file 'juju/providers/common/tests/test_connect.py'
83--- juju/providers/common/tests/test_connect.py 2011-11-19 05:49:05 +0000
84+++ juju/providers/common/tests/test_connect.py 2012-08-28 16:13:21 +0000
85@@ -14,6 +14,8 @@
86 from juju.machine import ProviderMachine
87 from juju.providers.common.base import MachineProviderBase
88 from juju.state.sshclient import SSHClient
89+
90+from juju.state.tests.common import deleteTree
91 from juju.tests.common import get_test_zookeeper_address
92
93
94@@ -195,7 +197,7 @@
95 self.assertTrue(client_result, client_result)
96 self.assertIdentical(client_result[0], client)
97 finally:
98- deleteTree("/", client.handle)
99+ yield deleteTree(client)
100 client.close()
101
102 @inlineCallbacks
103@@ -235,5 +237,5 @@
104 "Connected to environment.\n",
105 log.getvalue())
106 finally:
107- deleteTree("/", client.handle)
108+ yield deleteTree(client)
109 client.close()
110
111=== modified file 'juju/providers/local/__init__.py'
112--- juju/providers/local/__init__.py 2012-07-20 17:28:17 +0000
113+++ juju/providers/local/__init__.py 2012-08-28 16:13:21 +0000
114@@ -17,9 +17,9 @@
115 from juju.providers.local.machine import LocalMachine
116 from juju.providers.local.network import Network
117 from juju.providers.local.pkg import check_packages
118-from juju.state.auth import make_identity
119 from juju.state.initialize import StateHierarchy
120 from juju.state.placement import LOCAL_POLICY
121+from juju.state.security import Principal
122 from juju.state.utils import get_open_port
123
124
125@@ -126,12 +126,14 @@
126
127 # Initialize the zookeeper state
128 log.debug("Initializing state...")
129- admin_identity = make_identity(
130- "admin:%s" % self.config["admin-secret"])
131+
132 client = ZookeeperClient(zookeeper.address)
133 yield client.connect()
134+ principal = Principal("admin", self.config['admin-secret'])
135+ yield principal.attach(client)
136+
137 hierarchy = StateHierarchy(
138- client, admin_identity, "local", constraints.data, "local")
139+ client, principal.get_token(), "local", constraints.data, "local")
140 yield hierarchy.initialize()
141 yield client.close()
142
143
144=== modified file 'juju/providers/local/tests/test_provider.py'
145--- juju/providers/local/tests/test_provider.py 2012-07-20 17:28:17 +0000
146+++ juju/providers/local/tests/test_provider.py 2012-08-28 16:13:21 +0000
147@@ -6,7 +6,6 @@
148
149 from twisted.internet.defer import succeed, inlineCallbacks
150
151-from txzookeeper.tests.utils import deleteTree
152
153 from juju.errors import ProviderError, EnvironmentNotFound
154 from juju.lib.lxc import LXCContainer
155@@ -17,9 +16,12 @@
156 from juju.providers.local.agent import ManagedMachineAgent
157 from juju.providers.local.files import StorageServer
158 from juju.providers.local.network import Network
159+from juju.state.auth import make_ace
160+from juju.state.security import Principal, OTPPrincipal
161 from juju.lib import lxc as lxc_lib
162
163 from juju.lib.testing import TestCase
164+from juju.state.tests.common import deleteTree
165 from juju.tests.common import get_test_zookeeper_address
166
167
168@@ -27,18 +29,26 @@
169
170 @inlineCallbacks
171 def setUp(self):
172+ yield super(LocalProviderTest, self).setUp()
173 self.constraints = ConstraintSet("local").parse([]).with_series("foo")
174 self.provider = MachineProvider(
175 "local-test", {
176- "admin-secret": "admin:abc", "data-dir": self.makeDir(),
177+ "admin-secret": "admin", "data-dir": self.makeDir(),
178 "authorized-keys": "fooabc123", "default-series": "oneiric"})
179 self.output = self.capture_logging(
180 "juju.local-dev", level=logging.DEBUG)
181 zookeeper.set_debug_level(0)
182 self.client = yield self.get_zookeeper_client().connect()
183
184+ principal = Principal("admin", "admin")
185+ OTPPrincipal.set_additional_otp_ace(
186+ make_ace(principal.get_token(), all=True))
187+ yield principal.attach(self.client)
188+
189+ @inlineCallbacks
190 def tearDown(self):
191- deleteTree("/", self.client.handle)
192+ self.assertTrue(self.client.connected)
193+ yield deleteTree(self.client)
194 self.client.close()
195
196 @property
197@@ -60,6 +70,7 @@
198 self.assertTrue(os.path.isdir(path))
199
200 def bootstrap_mock(self):
201+ """Mock all the external process interactions."""
202 self.patch(local, "REQUIRED_PACKAGES", [])
203 mock_network = self.mocker.patch(Network)
204 mock_network.start()
205@@ -97,7 +108,7 @@
206 self.assertEqual(
207 sorted(['services', 'settings', 'charms', 'relations', 'zookeeper',
208 'initialized', 'topology', 'machines', 'units',
209- 'constraints']),
210+ 'constraints', 'auth-tokens', 'otp']),
211 sorted(children))
212 output = self.output.getvalue()
213 self.assertIn("Starting networking...", output)
214
215=== modified file 'juju/state/base.py'
216--- juju/state/base.py 2011-12-20 15:09:26 +0000
217+++ juju/state/base.py 2012-08-28 16:13:21 +0000
218@@ -1,6 +1,6 @@
219 import logging
220
221-from twisted.internet.defer import inlineCallbacks, returnValue
222+from twisted.internet.defer import inlineCallbacks, returnValue, maybeDeferred
223
224 from zookeeper import NoNodeException
225
226@@ -64,13 +64,13 @@
227 subclasses.
228 """
229
230- @inlineCallbacks
231 def change_content_function(content, stat):
232 topology = InternalTopology()
233 if content:
234 topology.parse(content)
235- yield change_topology_function(topology)
236- returnValue(topology.dump())
237+ d = maybeDeferred(change_topology_function, topology)
238+ return d.addCallback(lambda result: topology.dump())
239+
240 return retry_change(self._client, "/topology",
241 change_content_function)
242
243
244=== modified file 'juju/state/initialize.py'
245--- juju/state/initialize.py 2012-07-18 01:41:26 +0000
246+++ juju/state/initialize.py 2012-08-28 16:13:21 +0000
247@@ -8,7 +8,7 @@
248 from .auth import make_ace
249 from .environment import EnvironmentStateManager, GlobalSettingsStateManager
250 from .machine import MachineStateManager
251-
252+from .security import Principal
253
254 log = logging.getLogger("juju.state.init")
255
256@@ -50,6 +50,10 @@
257 yield self.client.create("/machines", acls=acls)
258 yield self.client.create("/units", acls=acls)
259 yield self.client.create("/relations", acls=acls)
260+ yield self.client.create("/otp", acls=acls)
261+
262+ # Seed the admin identity
263+ yield Principal(*(self.admin_identity.split(":"))).create(self.client)
264
265 # In this very specific case, it's OK to create a Constraints object
266 # with a non-provider-specific ConstraintSet, because *all* we need it
267@@ -72,6 +76,7 @@
268 yield settings.set_provider_type(self.provider_type)
269
270 # This must come last, since clients will wait on it.
271+
272 yield self.client.create("/initialized", acls=acls)
273
274 # DON'T WRITE ANYTHING HERE. See line above.
275
276=== modified file 'juju/state/machine.py'
277--- juju/state/machine.py 2012-04-02 23:33:39 +0000
278+++ juju/state/machine.py 2012-08-28 16:13:21 +0000
279@@ -9,6 +9,8 @@
280 from juju.state.errors import MachineStateNotFound, MachineStateInUse
281 from juju.state.base import StateBase
282 from juju.state.utils import remove_tree, YAMLStateNodeMixin
283+from juju.state.security import (
284+ AgentPrincipal, GroupPrincipal, apply_security_rules)
285
286
287 class MachineStateManager(StateBase):
288@@ -29,8 +31,15 @@
289 flags=zookeeper.SEQUENCE)
290 _, internal_id = path.rsplit("/", 1)
291
292+ # Add an agent principal and store its otp ref on the machine state.
293+ yield AgentPrincipal(self._client).create(internal_id, store=path)
294+
295 def add_machine(topology):
296 topology.add_machine(internal_id)
297+
298+ # Update the ACL after topology modification, only once though.
299+ return apply_security_rules(self._client, path, topology)
300+
301 yield self._retry_topology_change(add_machine)
302
303 returnValue(MachineState(self._client, internal_id))
304
305=== modified file 'juju/state/relation.py'
306--- juju/state/relation.py 2012-05-15 19:07:47 +0000
307+++ juju/state/relation.py 2012-08-28 16:13:21 +0000
308@@ -14,6 +14,7 @@
309 DuplicateEndpoints, IncompatibleEndpoints, RelationAlreadyExists,
310 RelationStateNotFound, StateChanged, UnitRelationStateNotFound,
311 UnknownRelationRole)
312+from juju.state.security import apply_security_rules
313
314
315 class RelationStateManager(StateBase):
316@@ -82,6 +83,7 @@
317 endpoint.relation_role,
318 endpoint.relation_name))
319
320+ @inlineCallbacks
321 def add_relation(topology):
322 if topology.has_relation_between_endpoints(endpoints):
323 raise RelationAlreadyExists(endpoints)
324@@ -97,6 +99,13 @@
325 service_relation.internal_service_id,
326 service_relation.relation_name,
327 service_relation.relation_role)
328+
329+ # Apply security rules with topology identity.
330+ yield apply_security_rules(self._client,
331+ "/relations/%s" % relation_id,
332+ topology,
333+ recurse=True)
334+
335 yield self._retry_topology_change(add_relation)
336
337 returnValue((RelationState(self._client, relation_id), services))
338@@ -123,11 +132,11 @@
339 # /relations/relation_id/optional_container_id/relation_role/...
340 # how far down the path we can create at this point depends on
341 # what type of relation we have.
342- if relation_scope == "global":
343- # Add service container in relation.
344- path = "/relations/%s/%s" % (relation_id, endpoint.relation_role)
345- else:
346- path = "/relations/%s" % (relation_id,)
347+ if relation_scope != "global":
348+ return
349+ # Add service container in relation.
350+ path = "/relations/%s/%s" % (relation_id, endpoint.relation_role)
351+
352 try:
353 yield self._client.create(path)
354 except zookeeper.NodeExistsException:
355
356=== modified file 'juju/state/security.py'
357--- juju/state/security.py 2011-09-15 18:50:23 +0000
358+++ juju/state/security.py 2012-08-28 16:13:21 +0000
359@@ -12,11 +12,26 @@
360
361 from juju.state.auth import make_identity, make_ace
362 from juju.state.errors import (
363- StateNotFound, PrincipalNotFound)
364+ StateChanged, StateNotFound, PrincipalNotFound)
365 from juju.state.utils import YAMLState
366-
367-
368-ZOO_OPEN_AUTH_ACL_UNSAFE = make_ace("auth", "world", all=True)
369+from juju.state import securityrules
370+
371+ZOO_OPEN_AUTH_ACL_UNSAFE = ZOO_OPEN_ACL_UNSAFE
372+OTP_IDENTITY_KEY = "otp-identity"
373+
374+
375+def _generate_string(size=16):
376+ return "".join(random.sample(string.letters, size))
377+
378+
379+@inlineCallbacks
380+def apply_security_rules(client, path, topology=None, recurse=False):
381+ """Apply security policy ACL to the given path."""
382+ token_db = TokenDatabase(client)
383+ policy = SecurityPolicy(
384+ client, token_db, securityrules.get_default_rules())
385+ acl = yield policy(path, topology)
386+ yield client.set_acl(path, acl)
387
388
389 class Principal(object):
390@@ -41,8 +56,16 @@
391
392 def attach(self, connection):
393 """A principal can be attached to a connection."""
394- return connection.add_auth(
395+ auth_deferred = connection.add_auth(
396 "digest", "%s:%s" % (self._name, self._password))
397+ # Work around for fast auth, remove post ZOOKEEPER-770
398+ #connection.exists("/")
399+ return auth_deferred
400+
401+ def create(self, client):
402+ """ Stores the user token into the token database.
403+ """
404+ return TokenDatabase(client).add_token(self.get_token())
405
406
407 class GroupPrincipal(object):
408@@ -53,19 +76,19 @@
409 these credentials.
410 """
411
412- def __init__(self, client, path):
413+ def __init__(self, client, path, name=None):
414 self._client = client
415 self._path = path
416- self._name = None
417+ self._name = name
418 self._password = None
419
420 @inlineCallbacks
421 def initialize(self):
422- """Initialize the group.
423+ """Initialize the group. Reads the group information from zk.
424
425 The group must always be initialized before attribute
426 use. Groups are persistent principals. If a group is being used
427- as a principal, it must first be loladed via initialize before
428+ as a principal, it must first be loaded via initialize before
429 any access to principal attributes or principal methods.
430 """
431
432@@ -99,12 +122,18 @@
433 @inlineCallbacks
434 def attach(self, connection):
435 self._check()
436- yield connection.add_auth(
437+ auth_deferred = connection.add_auth(
438 "digest", "%s:%s" % (self._name, self._password))
439+ # Work around for fast auth, remove post ZOOKEEPER-770
440+ #connection.exists("/")
441+ yield auth_deferred
442
443 @inlineCallbacks
444- def create(self, name, password):
445+ def create(self, name, password=None):
446 """Create the group with the given name and password."""
447+ if password is None:
448+ password = _generate_string()
449+
450 try:
451 yield self._client.create(
452 self._path,
453@@ -113,43 +142,31 @@
454 raise RuntimeError("Group already exists at %r" % self._path)
455 self._name = name
456 self._password = password
457+ yield TokenDatabase(self._client).add_token(self.get_token())
458
459- @inlineCallbacks
460 def add_member(self, principal):
461 """Add a principal as a member of the group.
462
463 A member of the group can use the group as an additional principal
464 attached to the connection.
465 """
466- self._check()
467- acl, stat = yield self._client.get_acl(self._path)
468- token = principal.get_token()
469-
470- for ace in acl:
471- if ace["id"] == token:
472- return
473-
474- acl.append(make_ace(token, read=True))
475-
476- yield self._client.set_acl(self._path, acl)
477-
478- @inlineCallbacks
479+ acl = ACL(self._client, self._path)
480+ return acl.grant_token(principal.get_token(), read=True).addErrback(
481+ lambda error: error.trap(StateChanged) and None)
482+
483 def remove_member(self, name):
484 """Remove a principal from a group by principal name"""
485- self._check()
486+ acl = ACL(self._client, self._path)
487+ return acl.prohibit(name)
488+
489+ @inlineCallbacks
490+ def get_members(self):
491+ """Return the names of the principals that are members of the group."""
492 acl, stat = yield self._client.get_acl(self._path)
493-
494- found = False
495- for ace in acl:
496- if ace["id"].split(":")[0] == name:
497- acl.remove(ace)
498- found = True
499- break
500-
501- if not found:
502- return
503-
504- yield self._client.set_acl(self._path, acl)
505+ results = []
506+ for entry in acl:
507+ results.append(entry["id"].split(":", 1)[0])
508+ returnValue(results)
509
510
511 class OTPPrincipal(object):
512@@ -174,9 +191,6 @@
513 self._otp = None
514 self._path = path
515
516- def _generate_string(self, size=16):
517- return "".join(random.sample(string.letters, size))
518-
519 def _check(self):
520 if not self._name:
521 raise RuntimeError("OTPPrincipal must be created before use")
522@@ -201,35 +215,53 @@
523
524 @classmethod
525 @inlineCallbacks
526- def consume(cls, client, otp_data):
527+ def consume(cls, client, otp_data=None, path=None):
528 """Consume an OTP serialization to retrieve the actual credentials.
529
530 returns a username, password tuple, and destroys the OTP node.
531 """
532+ assert path or otp_data
533+ if otp_data is None:
534+ content, stat = yield client.get(path)
535+ otp_data = yaml.load(content)[OTP_IDENTITY_KEY]
536+
537 # Decode the data to get the path and otp credentials
538 path, credentials = base64.b64decode(otp_data).split(":", 1)
539- yield client.add_auth("digest", credentials)
540- # Load the otp principal data
541+
542+ # Auth with otp token creds to read the final principal data.
543+ p = Principal(*(credentials.split(":")))
544+ yield p.attach(client)
545+
546+ # Load the principal data
547 data, stat = yield client.get(path)
548 principal_data = yaml.load(data)
549+
550 # Consume the otp node
551 yield client.delete(path)
552- returnValue((principal_data["name"], principal_data["password"]))
553+
554+ returnValue("%s:%s" % (
555+ principal_data["name"], principal_data["password"]))
556
557 @inlineCallbacks
558- def create(self, name, password=None, otp_name=None, otp=None):
559+ def create(self, name, password=None, otp_name=None, otp=None, store=None):
560 """Create an OTP for a principal.
561 """
562 if self._name:
563 raise ValueError("OTPPrincipal has already been created.")
564
565 self._name = name
566- self._password = password or self._generate_string()
567- self._otp_name = otp_name or self._generate_string()
568- self._otp = otp or self._generate_string()
569+ self._password = password or _generate_string()
570+ self._otp_name = otp_name or _generate_string()
571+ self._otp = otp or _generate_string()
572
573- acl = [make_ace(
574- make_identity("%s:%s" % (self._otp_name, self._otp)), read=True)]
575+ acl = [
576+ make_ace(
577+ make_identity(
578+ "%s:%s" % (self._otp_name, self._otp)), read=True),
579+ make_ace(
580+ make_identity(
581+ "%s:%s" % (self._name, self._password)), all=True),
582+ ]
583
584 # Optional additional ACL entry for unit test teardown.
585 if self._extra_otp_ace:
586@@ -237,10 +269,20 @@
587
588 self._path = yield self._client.create(
589 self._path,
590- yaml.safe_dump(dict(name=name, password=password)),
591+ yaml.safe_dump(dict(name=self._name, password=self._password)),
592 acls=acl,
593 flags=SEQUENCE)
594
595+ # Store the final principal's name.
596+ yield TokenDatabase(self._client).add_token(self.get_token())
597+
598+ # Optionally serialize the otp path/reference token
599+ if store is not None:
600+ state = YAMLState(self._client, store)
601+ yield state.read()
602+ state[OTP_IDENTITY_KEY] = self.serialize()
603+ yield state.write()
604+
605 returnValue(self)
606
607 def serialize(self):
608@@ -262,6 +304,10 @@
609 cls._extra_otp_ace = ace
610
611
612+class AgentPrincipal(OTPPrincipal):
613+ pass
614+
615+
616 class TokenDatabase(object):
617 """A hash map of principal names to their identity tokens.
618
619@@ -277,6 +323,17 @@
620 yield self._state.read()
621 self._state[principal.name] = principal.get_token()
622 yield self._state.write()
623+ returnValue(principal)
624+
625+ @inlineCallbacks
626+ def add_token(self, token):
627+ """Add a principal to the token database.
628+ """
629+ yield self._state.read()
630+ name, checksum = token.split(":")
631+ self._state[name] = token
632+ yield self._state.write()
633+ returnValue(token)
634
635 @inlineCallbacks
636 def get(self, name):
637@@ -323,7 +380,7 @@
638 self._rules.append(rule)
639
640 @inlineCallbacks
641- def __call__(self, path):
642+ def __call__(self, path, topology=None):
643 """Given a node path, determine the ACL.
644 """
645 acl_entries = []
646@@ -388,14 +445,25 @@
647 """Grant permissions on node to the given principal name."""
648
649 token = yield self._token_db.get(principal_name)
650+ yield self.grant_token(token, **perms)
651+
652+ @inlineCallbacks
653+ def grant_token(self, token, **perms):
654+ """Grant permissions on node to the given identity token.
655+
656+ Multiple permission grant to the same principal are additive in nature.
657+ """
658 ace = make_ace(token, **perms)
659+ principal_name = token.split(":")[0]
660
661 def add(acl):
662+
663 index = self._principal_index(acl, principal_name)
664 if index is not None:
665 acl_ace = acl[index]
666 acl_ace["perms"] = ace["perms"] | acl_ace["perms"]
667 return acl
668+
669 acl.append(ace)
670 return acl
671
672@@ -426,6 +494,7 @@
673 acl, stat = yield self._client.get_acl(self._path)
674 except (BadArgumentsException, NoNodeException):
675 raise StateNotFound(self._path)
676+
677 acl = change_func(acl)
678 try:
679 yield self._client.set_acl(
680
681=== added file 'juju/state/securityrules.py'
682--- juju/state/securityrules.py 1970-01-01 00:00:00 +0000
683+++ juju/state/securityrules.py 2012-08-28 16:13:21 +0000
684@@ -0,0 +1,4 @@
685+
686+
687+def get_default_rules():
688+ return []
689
690=== modified file 'juju/state/service.py'
691--- juju/state/service.py 2012-04-05 22:27:11 +0000
692+++ juju/state/service.py 2012-08-28 16:13:21 +0000
693@@ -23,6 +23,8 @@
694 from juju.state.charm import CharmStateManager
695 from juju.state.relation import ServiceRelationState, RelationStateManager
696 from juju.state.machine import _public_machine_id, MachineState
697+from juju.state.security import (
698+ AgentPrincipal, GroupPrincipal, apply_security_rules)
699 from juju.state.topology import InternalTopologyError
700 from juju.state.utils import (
701 remove_tree, dict_merge, YAMLState, YAMLStateNodeMixin)
702@@ -56,18 +58,24 @@
703 # charm metadata is always decoded into unicode, ensure any
704 # serialized state references strings to avoid tying to py runtime.
705 node_data = yaml.safe_dump(service_details)
706- path = yield self._client.create("/services/service-", node_data,
707- flags=zookeeper.SEQUENCE)
708+ path = yield self._client.create(
709+ "/services/service-", node_data, flags=zookeeper.SEQUENCE)
710 internal_id = path.rsplit("/", 1)[1]
711
712 # create a child node for configuration options
713 yield self._client.create("%s/config" % path, yaml.dump({}))
714
715+ # Create a group principal for service units
716+ yield GroupPrincipal(self._client, path + '/group').create(internal_id)
717+
718 def add_service(topology):
719 if topology.find_service_with_name(service_name):
720 raise ServiceStateNameInUse(service_name)
721 topology.add_service(internal_id, service_name)
722
723+ # Update the tree ACL with the modified topology.
724+ return apply_security_rules(self._client, path, topology)
725+
726 yield self._retry_topology_change(add_service)
727
728 returnValue(ServiceState(self._client, internal_id, service_name))
729@@ -335,8 +343,8 @@
730 def get_charm_state(self):
731 """Return the CharmState for this service."""
732 charm_id = yield self.get_charm_id()
733- formula_state_manager = CharmStateManager(self._client)
734- charm = yield formula_state_manager.get_charm_state(charm_id)
735+ charm_state_manager = CharmStateManager(self._client)
736+ charm = yield charm_state_manager.get_charm_state(charm_id)
737 returnValue(charm)
738
739 @inlineCallbacks
740@@ -412,32 +420,45 @@
741 a notable change to env/service, users would potentially see units
742 apparently deployed on wildly inappropriate machines)).
743 """
744+
745 constraints = yield self.get_constraints()
746 charm_id = yield self.get_charm_id()
747 unit_data = {"charm": charm_id, "constraints": constraints.data}
748 path = yield self._client.create(
749 "/units/unit-", yaml.dump(unit_data), flags=zookeeper.SEQUENCE)
750-
751 internal_unit_id = path.rsplit("/", 1)[1]
752
753- sequence = [None]
754 if container is not None:
755 yield self._validate_principal_container(container)
756 container_id = container.internal_id
757 else:
758 container_id = None
759
760+ # Add an agent principal and store its otp ref on the machine state.
761+ unit_principal = yield AgentPrincipal(
762+ self._client).create(internal_unit_id, store=path)
763+
764+ # Add the unit to the service security group
765+ service_group = yield GroupPrincipal(
766+ self._client, self._zk_path + "/group")
767+ yield service_group.add_member(unit_principal)
768+ # XXX what access should the principal have to the subordinate.
769+
770+ scope_escape = []
771+
772 def add_unit(topology):
773 if not topology.has_service(self._internal_id):
774 raise StateChanged()
775- sequence[0] = topology.add_service_unit(self._internal_id,
776- internal_unit_id,
777- container_id)
778+ scope_escape.append(
779+ topology.add_service_unit(
780+ self._internal_id, internal_unit_id, container_id))
781+
782+ # Update the ACL after topology modification, only once though.
783+ return apply_security_rules(self._client, path, topology)
784
785 yield self._retry_topology_change(add_unit)
786-
787 returnValue(ServiceUnitState(self._client, self._internal_id,
788- self._service_name, sequence[0],
789+ self._service_name, scope_escape.pop(),
790 internal_unit_id))
791
792 @inlineCallbacks
793@@ -452,7 +473,7 @@
794 for unit_id in unit_ids:
795 unit_names.append(
796 topology.get_service_unit_name(self._internal_id, unit_id))
797- returnValue(unit_names)
798+ returnValue(sorted(unit_names))
799
800 @inlineCallbacks
801 def remove_unit_state(self, service_unit):
802@@ -473,6 +494,10 @@
803
804 yield self._retry_topology_change(remove_unit)
805
806+ # Update the service security group.
807+ service_group = GroupPrincipal(self._client, self._zk_path + '/group')
808+ yield service_group.remove_member(service_unit.internal_id)
809+
810 # Remove any local settings.
811 yield remove_tree(
812 self._client, "/units/%s" % service_unit.internal_id)
813@@ -524,7 +549,6 @@
814
815 if not topology.has_service(self._internal_id):
816 raise StateChanged()
817-
818 internal_unit_id = \
819 topology.find_service_unit_with_sequence(self._internal_id,
820 sequence)
821
822=== modified file 'juju/state/tests/common.py'
823--- juju/state/tests/common.py 2012-05-31 23:24:19 +0000
824+++ juju/state/tests/common.py 2012-08-28 16:13:21 +0000
825@@ -2,12 +2,12 @@
826
827 import zookeeper
828
829-from txzookeeper.tests.utils import deleteTree
830-
831 from juju.charm.directory import CharmDirectory
832 from juju.charm.tests.test_directory import sample_directory
833 from juju.environment.tests.test_config import EnvironmentsConfigTestBase
834 from juju.state.topology import InternalTopology
835+from juju.state.auth import make_ace
836+from juju.state.security import Principal, OTPPrincipal
837
838
839 class StateTestBase(EnvironmentsConfigTestBase):
840@@ -25,6 +25,8 @@
841 yield self.client.create("/services")
842 yield self.client.create("/units")
843 yield self.client.create("/relations")
844+ yield self.client.create("/otp")
845+ yield self.setup_security()
846
847 @inlineCallbacks
848 def tearDown(self):
849@@ -33,7 +35,8 @@
850 self.client.close()
851 client = self.get_zookeeper_client()
852 yield client.connect()
853- deleteTree(handle=client.handle)
854+ yield Principal("testadmin", "testadmin").attach(client)
855+ yield deleteTree(client)
856 client.close()
857 yield super(StateTestBase, self).tearDown()
858
859@@ -53,3 +56,28 @@
860 yield self.client.set("/topology", content)
861 except zookeeper.NoNodeException:
862 yield self.client.create("/topology", content)
863+
864+ @inlineCallbacks
865+ def setup_security(self):
866+ """Additional test setup to allow for OTP and security rules.
867+ """
868+ # Setup additional test access to OTP nodes
869+ principal = Principal("testadmin", "testadmin")
870+ token = principal.get_token()
871+ OTPPrincipal.set_additional_otp_ace(make_ace(token, all=True))
872+
873+ # Rules application expects 'admin' principal
874+ yield Principal("admin", "admin").create(self.client)
875+
876+ # Remove the test otp ace, post test
877+ self.addCleanup(lambda: OTPPrincipal.set_additional_otp_ace(None))
878+
879+
880+@inlineCallbacks
881+def deleteTree(client, path="/"):
882+ for child in (yield client.get_children(path)):
883+ if child == "zookeeper":
884+ continue
885+ child_path = "/" + ("%s/%s" % (path, child)).strip("/")
886+ yield deleteTree(client, child_path)
887+ yield client.delete(child_path)
888
889=== modified file 'juju/state/tests/test_agent.py'
890--- juju/state/tests/test_agent.py 2011-10-05 00:05:44 +0000
891+++ juju/state/tests/test_agent.py 2012-08-28 16:13:21 +0000
892@@ -1,12 +1,10 @@
893-import zookeeper
894-
895 from twisted.internet.defer import inlineCallbacks
896-from txzookeeper.tests.utils import deleteTree
897
898-from juju.lib.testing import TestCase
899 from juju.state.base import StateBase
900 from juju.state.agent import AgentStateMixin
901
902+from juju.state.tests.common import StateTestBase
903+
904
905 class DomainObject(StateBase, AgentStateMixin):
906
907@@ -14,24 +12,15 @@
908 return "/agent"
909
910
911-class AgentDomainTest(TestCase):
912+class AgentDomainTest(StateTestBase):
913
914 @inlineCallbacks
915 def setUp(self):
916- zookeeper.set_debug_level(0)
917- yield super(TestCase, self).setUp()
918+ yield super(AgentDomainTest, self).setUp()
919 self.client = self.get_zookeeper_client()
920 yield self.client.connect()
921
922 @inlineCallbacks
923- def tearDown(self):
924- yield self.client.close()
925- client = self.get_zookeeper_client()
926- yield client.connect()
927- deleteTree("/", client.handle)
928- yield client.close()
929-
930- @inlineCallbacks
931 def test_has_agent(self):
932 domain = DomainObject(self.client)
933 exists = yield domain.has_agent()
934
935=== modified file 'juju/state/tests/test_auth.py'
936--- juju/state/tests/test_auth.py 2011-09-15 18:50:23 +0000
937+++ juju/state/tests/test_auth.py 2012-08-28 16:13:21 +0000
938@@ -14,7 +14,7 @@
939
940 credentials = "%s:%s" % (username, password)
941
942- identity = "%s:%s" %(
943+ identity = "%s:%s" % (
944 username,
945 base64.b64encode(hashlib.new("sha1", credentials).digest()))
946 self.assertEqual(identity, make_identity(credentials))
947@@ -25,7 +25,7 @@
948
949 credentials = "%s:%s" % (username, password)
950
951- identity = "%s:%s" %(
952+ identity = "%s:%s" % (
953 username,
954 base64.b64encode(hashlib.new("sha1", credentials).digest()))
955 self.assertEqual(identity, make_identity(credentials))
956@@ -41,7 +41,7 @@
957 self.assertEqual(ace["id"], identity)
958 self.assertEqual(ace["scheme"], "digest")
959 self.assertEqual(
960- ace["perms"], zookeeper.PERM_WRITE|zookeeper.PERM_CREATE)
961+ ace["perms"], zookeeper.PERM_WRITE | zookeeper.PERM_CREATE)
962
963 def test_make_ace_with_unknown_perm(self):
964 identity = "admin:moss"
965
966=== modified file 'juju/state/tests/test_base.py'
967--- juju/state/tests/test_base.py 2012-04-06 14:42:06 +0000
968+++ juju/state/tests/test_base.py 2012-08-28 16:13:21 +0000
969@@ -50,6 +50,21 @@
970 self.assertEquals(topology.dump(), topology_dump)
971
972 @inlineCallbacks
973+ def test_change_topology_waits(self):
974+ """Verify yielding topology change functions are respected."""
975+ @inlineCallbacks
976+ def change_topology(topology):
977+ yield self.client.exists("/")
978+ yield self.sleep(0.1)
979+ topology.add_machine("m-0")
980+
981+ yield self.base._retry_topology_change(change_topology)
982+
983+ content, stat = yield self.client.get("/topology")
984+ topology = self.parse_topology(content)
985+ self.assertTrue(topology.has_machine("m-0"))
986+
987+ @inlineCallbacks
988 def test_change_empty_topology(self):
989 """
990 Attempting to change a non-existing topology should create
991@@ -60,7 +75,6 @@
992 topology.add_machine("m-0")
993
994 yield self.base._retry_topology_change(change_topology)
995-
996 content, stat = yield self.client.get("/topology")
997 topology = self.parse_topology(content)
998 self.assertTrue(topology.has_machine("m-0"))
999@@ -98,7 +112,7 @@
1000
1001 def watch_topology(old_topology, new_topology):
1002 calls.append((old_topology, new_topology))
1003- wait_callback[len(calls)-1].callback(True)
1004+ wait_callback[len(calls) - 1].callback(True)
1005
1006 # Start watching.
1007 self.base._watch_topology(watch_topology)
1008@@ -155,7 +169,7 @@
1009
1010 def watch_topology(old_topology, new_topology):
1011 calls.append((old_topology, new_topology))
1012- wait_callback[len(calls)-1].callback(True)
1013+ wait_callback[len(calls) - 1].callback(True)
1014
1015 # Start watching, and wait on callback immediately.
1016 self.base._watch_topology(watch_topology)
1017@@ -205,7 +219,7 @@
1018
1019 def watch_topology(old_topology, new_topology):
1020 calls.append((old_topology, new_topology))
1021- wait_callback[len(calls)-1].callback(True)
1022+ wait_callback[len(calls) - 1].callback(True)
1023
1024 # Start watching, and wait on callback immediately.
1025 self.base._watch_topology(watch_topology)
1026@@ -251,8 +265,8 @@
1027
1028 def watch_topology(old_topology, new_topology):
1029 calls.append((old_topology, new_topology))
1030- wait_callback[len(calls)-1].callback(True)
1031- return finish_callback[len(calls)-1]
1032+ wait_callback[len(calls) - 1].callback(True)
1033+ return finish_callback[len(calls) - 1]
1034
1035 # Start watching.
1036 self.base._watch_topology(watch_topology)
1037@@ -299,7 +313,7 @@
1038
1039 def watcher(old_topology, new_topology):
1040 calls.append((old_topology, new_topology))
1041- wait_callback[len(calls)-1].callback(True)
1042+ wait_callback[len(calls) - 1].callback(True)
1043 if len(calls) == 2:
1044 raise StopWatcher()
1045
1046@@ -357,7 +371,7 @@
1047 topology = InternalTopology()
1048 topology.add_machine("m-0")
1049 yield self.set_topology(topology)
1050-
1051+
1052 # Hold off until callback is started.
1053 yield wait_callback
1054
1055
1056=== modified file 'juju/state/tests/test_environment.py'
1057--- juju/state/tests/test_environment.py 2012-03-29 01:37:57 +0000
1058+++ juju/state/tests/test_environment.py 2012-08-28 16:13:21 +0000
1059@@ -24,10 +24,6 @@
1060 self.config.load()
1061
1062 @inlineCallbacks
1063- def tearDown(self):
1064- yield super(EnvironmentStateManagerTest, self).tearDown()
1065-
1066- @inlineCallbacks
1067 def test_set_config_state(self):
1068 """
1069 The simplest thing the manager can do is serialize a given
1070
1071=== modified file 'juju/state/tests/test_errors.py'
1072--- juju/state/tests/test_errors.py 2012-04-11 19:38:41 +0000
1073+++ juju/state/tests/test_errors.py 2012-08-28 16:13:21 +0000
1074@@ -10,7 +10,7 @@
1075 ServiceUnitStateMachineAlreadyAssigned, ServiceStateNameInUse,
1076 BadServiceStateName, EnvironmentStateNotFound,
1077 RelationAlreadyExists, RelationStateNotFound, UnitRelationStateNotFound,
1078- UnitRelationStateAlreadyAssigned, UnknownRelationRole,
1079+ UnitRelationStateAlreadyAssigned, UnknownRelationRole, PrincipalNotFound,
1080 BadDescriptor, DuplicateEndpoints, IncompatibleEndpoints,
1081 NoMatchingEndpoints, AmbiguousRelation,
1082 ServiceUnitStateMachineNotAssigned, ServiceUnitDebugAlreadyEnabled,
1083
1084=== modified file 'juju/state/tests/test_firewall.py'
1085--- juju/state/tests/test_firewall.py 2012-07-09 22:49:36 +0000
1086+++ juju/state/tests/test_firewall.py 2012-08-28 16:13:21 +0000
1087@@ -272,7 +272,7 @@
1088 machine_provider, machine.id)
1089 returnValue(provider_ports)
1090
1091- def test_open_close_ports_on_machine(self):
1092+ def xtest_open_close_ports_on_machine(self):
1093 """Verify opening/closing ports on a machine works properly.
1094
1095 In particular this is done without watch support."""
1096
1097=== modified file 'juju/state/tests/test_initialize.py'
1098--- juju/state/tests/test_initialize.py 2012-04-06 14:42:06 +0000
1099+++ juju/state/tests/test_initialize.py 2012-08-28 16:13:21 +0000
1100@@ -1,15 +1,17 @@
1101 import zookeeper
1102
1103 from twisted.internet.defer import inlineCallbacks
1104-from txzookeeper.tests.utils import deleteTree
1105
1106 from juju.environment.tests.test_config import EnvironmentsConfigTestBase
1107-from juju.state.auth import make_identity
1108+from juju.state.auth import make_identity, make_ace
1109 from juju.state.environment import (
1110 GlobalSettingsStateManager, EnvironmentStateManager)
1111 from juju.state.initialize import StateHierarchy
1112+from juju.state.security import OTPPrincipal, Principal
1113 from juju.state.machine import MachineStateManager
1114
1115+from juju.state.tests.common import deleteTree
1116+
1117
1118 class LayoutTest(EnvironmentsConfigTestBase):
1119
1120@@ -25,12 +27,22 @@
1121 "cpu": None,
1122 "ubuntu-series": "cranky",
1123 "provider-type": "dummy"}
1124+ yield self.client.connect()
1125+
1126+ self.test_admin = Principal("admin", "secret")
1127+ yield self.test_admin.attach(self.client)
1128+ self.identity = self.test_admin.get_token()
1129+
1130+ # Add test user to all acl of all otp principal tokens for teardown.
1131+ OTPPrincipal.set_additional_otp_ace(make_ace(self.identity, all=True))
1132+ self.addCleanup(lambda: OTPPrincipal.set_additional_otp_ace(None))
1133+
1134 self.layout = StateHierarchy(
1135 self.client, self.identity, "i-abcdef", constraints_data, "dummy")
1136- yield self.client.connect()
1137
1138+ @inlineCallbacks
1139 def tearDown(self):
1140- deleteTree(handle=self.client.handle)
1141+ yield deleteTree(self.client)
1142 self.client.close()
1143
1144 @inlineCallbacks
1145
1146=== modified file 'juju/state/tests/test_machine.py'
1147--- juju/state/tests/test_machine.py 2012-03-28 00:38:16 +0000
1148+++ juju/state/tests/test_machine.py 2012-08-28 16:13:21 +0000
1149@@ -8,10 +8,13 @@
1150 from juju.errors import ConstraintError
1151 from juju.machine.tests.test_constraints import (
1152 dummy_constraints, series_constraints)
1153+
1154 from juju.state.charm import CharmStateManager
1155+from juju.state.auth import make_identity
1156+from juju.state.errors import MachineStateNotFound, MachineStateInUse
1157 from juju.state.machine import MachineStateManager
1158 from juju.state.service import ServiceStateManager
1159-from juju.state.errors import MachineStateNotFound, MachineStateInUse
1160+from juju.state.security import TokenDatabase, OTPPrincipal
1161 from juju.state.utils import YAMLState
1162
1163 from juju.state.tests.common import StateTestBase
1164@@ -66,6 +69,17 @@
1165 self.assertTrue(topology.has_machine("machine-0000000001"))
1166
1167 @inlineCallbacks
1168+ def test_add_machine_security(self):
1169+ machine_state1 = yield self.add_machine_state()
1170+ creds = yield OTPPrincipal.consume(
1171+ self.client, path=machine_state1._zk_path)
1172+ # Verify the otp data is present, by consuming, and matches
1173+ # the expected token.
1174+ self.assertEqual(
1175+ (yield TokenDatabase(self.client).get(machine_state1.internal_id)),
1176+ make_identity(creds))
1177+
1178+ @inlineCallbacks
1179 def test_incomplete_constraints(self):
1180 e = yield self.assertFailure(
1181 self.add_machine_state(dummy_constraints), ConstraintError)
1182
1183=== modified file 'juju/state/tests/test_relation.py'
1184--- juju/state/tests/test_relation.py 2012-06-01 22:50:44 +0000
1185+++ juju/state/tests/test_relation.py 2012-08-28 16:13:21 +0000
1186@@ -364,6 +364,9 @@
1187 exists = yield self.client.get(
1188 "/relations/%s/settings" % relation_state.internal_id)
1189 self.assertTrue(exists)
1190+ acl, stat = yield self.client.get_acl(
1191+ "/relations/%s" % relation_state.internal_id)
1192+ self.assertACE(acl, name="admin")
1193
1194 @inlineCallbacks
1195 def test_add_relation_state_to_missing_service(self):
1196
1197=== modified file 'juju/state/tests/test_security.py'
1198--- juju/state/tests/test_security.py 2012-02-22 09:23:16 +0000
1199+++ juju/state/tests/test_security.py 2012-08-28 16:13:21 +0000
1200@@ -5,19 +5,19 @@
1201 from twisted.internet.defer import inlineCallbacks, succeed
1202
1203 from juju.state.auth import make_identity, make_ace
1204-from juju.state.errors import StateNotFound, PrincipalNotFound
1205+from juju.state.errors import (
1206+ StateNotFound, PrincipalNotFound)
1207
1208 from juju.state.security import (
1209 ACL, Principal, GroupPrincipal, OTPPrincipal, TokenDatabase,
1210- SecurityPolicy, SecurityPolicyConnection)
1211+ SecurityPolicy, SecurityPolicyConnection, apply_security_rules)
1212
1213 from juju.lib.testing import TestCase
1214+from juju.state.tests.common import StateTestBase, deleteTree
1215 from juju.tests.common import get_test_zookeeper_address
1216
1217 from txzookeeper.client import ZOO_OPEN_ACL_UNSAFE
1218
1219-from txzookeeper.tests.utils import deleteTree
1220-
1221
1222 class PrincipalTests(TestCase):
1223
1224@@ -26,8 +26,9 @@
1225 zookeeper.set_debug_level(0)
1226 self.client = yield self.get_zookeeper_client().connect()
1227
1228+ @inlineCallbacks
1229 def tearDown(self):
1230- deleteTree(handle=self.client.handle)
1231+ yield deleteTree(self.client)
1232 self.client.close()
1233
1234 def test_name(self):
1235@@ -48,7 +49,7 @@
1236 self.addCleanup(lambda: client.close())
1237 admin_credentials = "admin:admin"
1238 test_credentials = "test:test"
1239- yield self.client.add_auth("digest", admin_credentials)
1240+ auth_deferred = self.client.add_auth("digest", admin_credentials)
1241
1242 acl = [make_ace(make_identity(admin_credentials), all=True),
1243 make_ace(make_identity(
1244@@ -56,12 +57,16 @@
1245
1246 yield client.create("/acl-test", "content", acls=acl)
1247
1248+ # Wait on the auth after the create due to 770
1249+ yield auth_deferred
1250+
1251 # Verify the acl is active
1252 yield self.assertFailure(
1253 client.get("/acl-test"), zookeeper.NoAuthException)
1254
1255 # Attach the principal to the connection
1256 principal = Principal("test", "test")
1257+
1258 yield principal.attach(client)
1259 content, stat = yield client.get("/acl-test")
1260 self.assertEqual(content, "content")
1261@@ -74,8 +79,9 @@
1262 zookeeper.set_debug_level(0)
1263 self.client = yield self.get_zookeeper_client().connect()
1264
1265+ @inlineCallbacks
1266 def tearDown(self):
1267- deleteTree(handle=self.client.handle)
1268+ yield deleteTree(self.client)
1269 self.client.close()
1270
1271 def test_uninitialized_usage(self):
1272@@ -129,20 +135,40 @@
1273 self.assertEqual(
1274 acl[1:],
1275 [make_ace(principal.get_token(), read=True)])
1276- # Adding a member again is fine
1277+ # Adding a member again is no-op, same goal state.
1278 yield group.add_member(principal)
1279
1280 @inlineCallbacks
1281+ def test_get_members(self):
1282+ group = GroupPrincipal(self.client, "/group-a")
1283+ yield group.create("group/a", "zebra")
1284+
1285+ principals = [
1286+ Principal("aladdin", "genie"),
1287+ Principal("babar", "elephant")]
1288+
1289+ for p in principals:
1290+ yield group.add_member(p)
1291+
1292+ # Policy isn't active by default
1293+ principals.insert(0, Principal("anyone", ""))
1294+
1295+ members = yield group.get_members()
1296+ self.assertEqual(
1297+ [p.name for p in principals],
1298+ members)
1299+
1300+ @inlineCallbacks
1301 def test_remove_member(self):
1302 group = GroupPrincipal(self.client, "/group-a")
1303 yield group.create("group/a", "zebra")
1304
1305 principal = Principal("aladdin", "genie")
1306+
1307 # Removing a member that doesn't exist is a no-op
1308- yield group.remove_member(principal)
1309+ yield group.remove_member(principal.name)
1310 yield group.add_member(principal)
1311 yield group.remove_member(principal.name)
1312-
1313 acl, stat = yield self.client.get_acl("/group-a")
1314 self.assertEqual(acl[1:], [])
1315
1316@@ -154,7 +180,7 @@
1317
1318 admin_credentials = "admin:admin"
1319 test_credentials = "test:test"
1320- yield self.client.add_auth("digest", admin_credentials)
1321+ self.client.add_auth("digest", admin_credentials)
1322
1323 acl = [make_ace(make_identity(admin_credentials), all=True),
1324 make_ace(make_identity(
1325@@ -182,8 +208,9 @@
1326 self.client = yield self.get_zookeeper_client().connect()
1327 self.client.create("/otp")
1328
1329+ @inlineCallbacks
1330 def tearDown(self):
1331- deleteTree(handle=self.client.handle)
1332+ yield deleteTree(self.client)
1333 self.client.close()
1334
1335 def set_otp_test_ace(self, test_ace=ZOO_OPEN_ACL_UNSAFE):
1336@@ -235,7 +262,10 @@
1337 self.assertEqual(credentials["password"], "secret")
1338
1339 acl, stat = yield self.client.get_acl(otp_path)
1340- self.assertEqual(len(acl), 2)
1341+ if OTPPrincipal._extra_otp_ace:
1342+ self.assertEqual(len(acl), 3)
1343+ else:
1344+ self.assertEqual(len(acl), 2)
1345
1346 @inlineCallbacks
1347 def test_serialize(self):
1348@@ -267,12 +297,34 @@
1349 yield self.assertFailure(
1350 self.client.get(path), zookeeper.NoAuthException)
1351
1352- name, password = yield OTPPrincipal.consume(self.client, otp_data)
1353+ credentials = yield OTPPrincipal.consume(self.client, otp_data)
1354+ name, password = credentials.split(":")
1355 self.assertEqual(name, "foobar")
1356 self.assertEqual(password, "secret")
1357 children = yield self.client.get_children("/otp")
1358 self.assertFalse(children)
1359
1360+ @inlineCallbacks
1361+ def test_create_and_store(self):
1362+ """Creates an otp principal and stores it serialization at path.
1363+ """
1364+ yield OTPPrincipal(self.client).create(
1365+ "aladdin", store="/zoo")
1366+
1367+ # Verify the otp serialization on the node
1368+ contents, stat = yield self.client.get("/zoo")
1369+ data = yaml.load(contents)
1370+ self.assertIn("otp-identity", data)
1371+
1372+ # Compare the persitsent user generated token to that in the tokendb
1373+ creds = yield OTPPrincipal.consume(self.client, path="/zoo")
1374+
1375+ token = yield TokenDatabase(self.client).get("aladdin")
1376+ self.assertEqual(
1377+ token, make_identity(creds))
1378+
1379+ self.assertTrue((yield self.client.exists("/zoo")))
1380+
1381
1382 class TokenDatabaseTest(TestCase):
1383
1384@@ -282,8 +334,9 @@
1385 self.client = yield self.get_zookeeper_client().connect()
1386 self.db = TokenDatabase(self.client, "/token-test")
1387
1388+ @inlineCallbacks
1389 def tearDown(self):
1390- deleteTree(handle=self.client.handle)
1391+ yield deleteTree(self.client)
1392 self.client.close()
1393
1394 @inlineCallbacks
1395@@ -328,8 +381,9 @@
1396 yield self.tokens.add(Principal("admin", "admin"))
1397 self.policy = SecurityPolicy(self.client, self.tokens)
1398
1399+ @inlineCallbacks
1400 def tearDown(self):
1401- deleteTree(handle=self.client.handle)
1402+ yield deleteTree(self.client)
1403 self.client.close()
1404
1405 @inlineCallbacks
1406@@ -349,7 +403,7 @@
1407 solution.
1408 """
1409 acl = yield self.policy("/random")
1410- self.assertIn(make_ace("auth", "world", all=True), acl)
1411+ self.assertIn(make_ace("anyone", "world", all=True), acl)
1412
1413 @inlineCallbacks
1414 def test_rule_match_suppress_open_access(self):
1415@@ -361,7 +415,7 @@
1416 # Check for matched rule ACL
1417 self.assertIn(make_ace(principal.get_token(), all=True), acl)
1418 # Verify no default access
1419- self.assertNotIn(make_ace("auth", "world", all=True), acl)
1420+ self.assertNotIn(make_ace("anyone", "world", all=True), acl)
1421
1422 @inlineCallbacks
1423 def test_rule_that_returns_deferred(self):
1424@@ -374,7 +428,7 @@
1425 # Check for matched rule ACL
1426 self.assertIn(make_ace(principal.get_token(), all=True), acl)
1427 # Verify no default access
1428- self.assertNotIn(make_ace("auth", "world", all=True), acl)
1429+ self.assertNotIn(make_ace("anyone", "world", all=True), acl)
1430
1431 @inlineCallbacks
1432 def test_owner_ace(self):
1433@@ -399,13 +453,11 @@
1434 self.token_db = TokenDatabase(self.client)
1435 yield self.token_db.add(admin)
1436 self.policy = SecurityPolicy(self.client, self.token_db, owner=admin)
1437- attach_defer = admin.attach(self.client)
1438- # Trick to speed up the auth response processing (fixed in ZK trunk)
1439- self.client.exists("/")
1440- yield attach_defer
1441+ yield admin.attach(self.client)
1442
1443+ @inlineCallbacks
1444 def tearDown(self):
1445- deleteTree(handle=self.client.handle)
1446+ yield deleteTree(self.client)
1447 self.client.close()
1448
1449 @inlineCallbacks
1450@@ -448,13 +500,11 @@
1451 self.admin = Principal("admin", "admin")
1452 yield self.tokens.add(self.admin)
1453 self.policy = SecurityPolicy(self.client, self.tokens)
1454- attach_deferred = self.admin.attach(self.client)
1455-
1456- self.client.exists("/")
1457- yield attach_deferred
1458-
1459+ yield self.admin.attach(self.client)
1460+
1461+ @inlineCallbacks
1462 def tearDown(self):
1463- deleteTree(handle=self.client.handle)
1464+ yield deleteTree(self.client)
1465 self.client.close()
1466
1467 @inlineCallbacks
1468@@ -469,11 +519,10 @@
1469 client = yield self.get_zookeeper_client().connect()
1470 principal = Principal("zebra", "stripes")
1471 yield self.tokens.add(principal)
1472- attach_deferred = principal.attach(client)
1473+ yield principal.attach(client)
1474 yield self.client.create(
1475 "/abc",
1476 acls=[make_ace(self.admin.get_token(), all=True)])
1477- yield attach_deferred
1478
1479 acl = ACL(client, "/abc")
1480 yield self.assertFailure(
1481@@ -508,6 +557,27 @@
1482 yield self.assertFailure(acl.grant("zebra"), PrincipalNotFound)
1483
1484 @inlineCallbacks
1485+ def test_multi_grant(self):
1486+ """Grants can be made multiple times against the same node version.
1487+ """
1488+ path = yield self.client.create("/abc")
1489+
1490+ alice = Principal("alice", "")
1491+ bob = Principal("bob", "")
1492+
1493+ yield self.tokens.add(alice)
1494+ yield self.tokens.add(bob)
1495+
1496+ acl = ACL(self.client, path)
1497+ yield acl.grant("alice", all=True)
1498+ yield acl.grant("bob", all=True)
1499+
1500+ acl, stat = yield self.client.get_acl(path)
1501+
1502+ self.assertACE(acl, alice.get_token())
1503+ self.assertACE(acl, bob.get_token())
1504+
1505+ @inlineCallbacks
1506 def test_prohibit(self):
1507 principal = Principal("zebra", "stripes")
1508 yield self.tokens.add(principal)
1509@@ -544,3 +614,38 @@
1510 acl, stat = yield self.client.get_acl(path)
1511 self.assertEqual(
1512 acl, [make_ace(self.admin.get_token(), all=True)])
1513+
1514+
1515+class SecurityModuleAPITest(StateTestBase):
1516+ """
1517+ Test of the top level conviencce functions in the security module.
1518+ """
1519+
1520+ @inlineCallbacks
1521+ def test_apply_rules(self):
1522+ """Calculates and sets an acl using the security rules.
1523+ """
1524+ cli_group = Principal("admin", "admin")
1525+ yield cli_group.attach(self.client)
1526+ yield TokenDatabase(self.client).add(cli_group)
1527+
1528+ principal = Principal("testadmin", "testadmin")
1529+ token = principal.get_token()
1530+
1531+ def get_test_rules():
1532+ return [lambda x, y: [make_ace(token, all=True)]]
1533+
1534+ get_default_rules = self.mocker.replace(
1535+ "juju.state.securityrules.get_default_rules")
1536+
1537+ get_default_rules()
1538+ self.mocker.call(get_test_rules)
1539+ self.mocker.replay()
1540+
1541+ path = yield self.client.create("/moon")
1542+ yield apply_security_rules(self.client, path)
1543+ acl, stat = yield self.client.get_acl(path)
1544+ self.assertEqual(
1545+ acl,
1546+ [make_ace(token, all=True),
1547+ make_ace(cli_group.get_token(), all=True)])
1548
1549=== modified file 'juju/state/tests/test_service.py'
1550--- juju/state/tests/test_service.py 2012-07-03 07:30:53 +0000
1551+++ juju/state/tests/test_service.py 2012-08-28 16:13:21 +0000
1552@@ -234,24 +234,23 @@
1553 else:
1554 self.fail("Error not raised")
1555
1556+ @inlineCallbacks
1557 def test_get_unit_state(self):
1558 """A unit state can be retrieved by name from the service manager."""
1559- self.assertFailure(self.service_state_manager.get_unit_state(
1560- "wordpress/1"), ServiceStateNotFound)
1561
1562- self.assertFailure(self.service_state_manager.get_unit_state(
1563- "wordpress1"), ServiceUnitStateNotFound)
1564+ yield self.assertFailure(self.service_state_manager.get_unit_state(
1565+ "wordpress/1"), ServiceStateNotFound)
1566
1567 wordpress_state = yield self.service_state_manager.add_service_state(
1568 "wordpress", self.charm_state, dummy_constraints)
1569
1570- self.assertFailure(self.service_state_manager.get_unit_state(
1571- "wordpress/1"), ServiceUnitStateNotFound)
1572+ yield self.assertFailure(self.service_state_manager.get_unit_state(
1573+ "wordpress/0"), ServiceUnitStateNotFound)
1574
1575- wordpress_unit = wordpress_state.add_unit_state()
1576+ wordpress_unit = yield wordpress_state.add_unit_state()
1577
1578 unit_state = yield self.service_state_manager.get_unit_state(
1579- "wordpress/1")
1580+ "wordpress/0")
1581
1582 self.assertEqual(unit_state.internal_id, wordpress_unit.internal_id)
1583
1584@@ -588,7 +587,6 @@
1585 self.assertEquals((yield unit_state2.get_container()), unit_state3)
1586 self.assertEquals((yield unit_state3.get_container()), None)
1587
1588-
1589 @inlineCallbacks
1590 def test_add_service_unit_with_changing_state(self):
1591 """
1592@@ -646,6 +644,7 @@
1593 exists = yield self.client.exists("/units/%s" % unit_state.internal_id)
1594 self.assertFalse(exists)
1595
1596+ @inlineCallbacks
1597 def test_remove_service_unit_nonexistant(self):
1598 """Removing a non existant service unit, is fine."""
1599
1600@@ -653,7 +652,9 @@
1601 "wordpress", self.charm_state, dummy_constraints)
1602 unit_state = yield service_state.add_unit_state()
1603 yield service_state.remove_unit_state(unit_state)
1604- yield service_state.remove_unit_state(unit_state)
1605+ yield self.assertFailure(
1606+ service_state.remove_unit_state(unit_state),
1607+ StateChanged)
1608
1609 @inlineCallbacks
1610 def test_get_all_service_states(self):
1611@@ -1601,7 +1602,7 @@
1612 event = yield watch_d
1613 self.assertEqual(event.type_name, "created")
1614 self.assertEqual(event.path,
1615- "/units/%s/agent" % unit_state.internal_id)
1616+ "/units/%s/agent" % unit_state.internal_id)
1617
1618 @inlineCallbacks
1619 def test_get_charm_id(self):
1620@@ -1760,7 +1761,8 @@
1621 mysql_service_state)
1622 other_constraints = dummy_cs.parse(["arch=arm"])
1623 wordpress_service_state = yield self.add_service_from_charm(
1624- "wordpress", constraints=other_constraints.with_series("series"))
1625+ "wordpress",
1626+ constraints=other_constraints.with_series("series"))
1627 wordpress_unit_state = yield wordpress_service_state.add_unit_state()
1628 yield self.assertFailure(
1629 wordpress_unit_state.assign_to_unused_machine(),
1630@@ -1965,7 +1967,8 @@
1631 [RelationEndpoint("wordpress", "varnish", "cache", "client"),
1632 RelationEndpoint("wordpress", "mysql", "db", "client"),
1633 RelationEndpoint("wordpress", "http", "url", "server"),
1634- RelationEndpoint("wordpress", "juju-info", "juju-info", "server")])
1635+ RelationEndpoint(
1636+ "wordpress", "juju-info", "juju-info", "server")])
1637 yield self.add_service_from_charm("riak")
1638 self.assertEqual(
1639 (yield self.service_state_manager.get_relation_endpoints(
1640@@ -2056,7 +2059,8 @@
1641 (yield self.service_state_manager.join_descriptors(
1642 "wordpress", "varnish")),
1643 [(RelationEndpoint("wordpress", "varnish", "cache", "client"),
1644- RelationEndpoint("varnish", "varnish", "webcache", "server"))])
1645+ RelationEndpoint(
1646+ "varnish", "varnish", "webcache", "server"))])
1647
1648 @inlineCallbacks
1649 def test_join_descriptors_service_name_relation_name(self):
1650@@ -2084,12 +2088,14 @@
1651 (yield self.service_state_manager.join_descriptors(
1652 "wordpress:cache", "varnish")),
1653 [(RelationEndpoint("wordpress", "varnish", "cache", "client"),
1654- RelationEndpoint("varnish", "varnish", "webcache", "server"))])
1655+ RelationEndpoint(
1656+ "varnish", "varnish", "webcache", "server"))])
1657 self.assertEqual(
1658 (yield self.service_state_manager.join_descriptors(
1659 "wordpress:cache", "varnish:webcache")),
1660 [(RelationEndpoint("wordpress", "varnish", "cache", "client"),
1661- RelationEndpoint("varnish", "varnish", "webcache", "server"))])
1662+ RelationEndpoint(
1663+ "varnish", "varnish", "webcache", "server"))])
1664
1665 @inlineCallbacks
1666 def test_join_peer_descriptors(self):
1667
1668=== modified file 'juju/unit/lifecycle.py'
1669--- juju/unit/lifecycle.py 2012-05-01 00:26:25 +0000
1670+++ juju/unit/lifecycle.py 2012-08-28 16:13:21 +0000
1671@@ -535,7 +535,8 @@
1672 relation_info["relation_scope"])
1673 self._relations[relation_id] = workflow
1674
1675- def _reconstruct_workflow(self, relation_id, relation_ident, relation_scope):
1676+ def _reconstruct_workflow(
1677+ self, relation_id, relation_ident, relation_scope):
1678 """Create a RelationWorkflowState which may refer to outdated state.
1679
1680 This means that *if* this service has already departed the relevant

Subscribers

People subscribed via source and target branches

to status/vote changes: