juju: update missing secrets if required

juju: update missing secrets if required

Update any missing secrets iff no secrets are present in the environment.

https://code.launchpad.net/~dave-cheney/juju-core/084--conn-update-secrets-only-if-needed/+merge/122228

(do not edit description out of merge proposal)

[dave_cheney.net, 2012-08-31 10:15:12]

Please take a look.

[dave_cheney.net, 2012-09-03 02:21:01]

Please take a look.

[niemeyer, 2012-09-04 15:09:55]

https://codereview.appspot.com/6496069/diff/2002/juju/conn.go
File juju/conn.go (right):

https://codereview.appspot.com/6496069/diff/2002/juju/conn.go#newcode95
juju/conn.go:95: delete(secrets, k)
I'm sorry, but this still doesn't look right. The point, as we talked about on
IRC, is to send the secrets iff they're missing. No need to delete things from
the secrets map, nor to apply a mutated (?) selection of secrets either.

Anything from secrets seen? Bail. Nothing at all? Set.

[dave_cheney.net, 2012-09-05 00:54:27]

https://codereview.appspot.com/6496069/diff/2002/juju/conn.go
File juju/conn.go (right):

https://codereview.appspot.com/6496069/diff/2002/juju/conn.go#newcode95
juju/conn.go:95: delete(secrets, k)
On 2012/09/04 15:09:55, niemeyer wrote:
> I'm sorry, but this still doesn't look right. The point, as we talked about on
> IRC, is to send the secrets iff they're missing. No need to delete things from
> the secrets map, nor to apply a mutated (?) selection of secrets either.
> 
> Anything from secrets seen? Bail. Nothing at all? Set.

I'm sorry I don't understand the business logic here. Are you saying that if any
keys from 

c.Environ.Provider().SecretAttrs(c.Environ.Config())

exist in c.state.EnvironConfig() we should not make any change in the
environment configuration ?

[niemeyer, 2012-09-05 01:04:31]

On 2012/09/05 00:54:27, dfc wrote:
> I'm sorry I don't understand the business logic here. Are you saying that if
any
> keys from 
> 
> c.Environ.Provider().SecretAttrs(c.Environ.Config())
> 
> exist in c.state.EnvironConfig() we should not make any change in the
> environment configuration ?

Right, we should only push secrets when they are not yet set. This is a one-time
initialization procedure to workaround the fact we can't push secrets safely at
bootstrap time. We shouldn't be blindly pushing secrets every time a connection
is made.

[dave_cheney.net, 2012-09-05 01:07:15]

What if some of the secrets are present but others are not?

On 05/09/2012, at 11:04, n13m3y3r@gmail.com wrote:

> On 2012/09/05 00:54:27, dfc wrote:
>> I'm sorry I don't understand the business logic here. Are you saying
> that if any
>> keys from
> 
>> c.Environ.Provider().SecretAttrs(c.Environ.Config())
> 
>> exist in c.state.EnvironConfig() we should not make any change in the
>> environment configuration ?
> 
> Right, we should only push secrets when they are not yet set. This is a
> one-time initialization procedure to workaround the fact we can't push
> secrets safely at bootstrap time. We shouldn't be blindly pushing
> secrets every time a connection is made.
> 
> https://codereview.appspot.com/6496069/

[niemeyer, 2012-09-05 01:29:44]

On Tue, Sep 4, 2012 at 10:07 PM, Dave Cheney <dave@cheney.net> wrote:
> What if some of the secrets are present but others are not?

This is a one-time only initialization procedure. What having secrets
there would indicate?

[dave_cheney.net, 2012-09-05 01:33:15]

If you aren't worried about this edge case, that will make the logic
simpler. Let me resubmit.

On Wed, Sep 5, 2012 at 11:29 AM, Gustavo Niemeyer <n13m3y3r@gmail.com> wrote:
> On Tue, Sep 4, 2012 at 10:07 PM, Dave Cheney <dave@cheney.net> wrote:
>> What if some of the secrets are present but others are not?
>
> This is a one-time only initialization procedure. What having secrets
> there would indicate?

[niemeyer, 2012-09-05 01:36:36]

On Tue, Sep 4, 2012 at 10:33 PM, Dave Cheney <dave@cheney.net> wrote:
> If you aren't worried about this edge case, that will make the logic
> simpler. Let me resubmit.

Yeah, it is super simple. If there's anything resembling a secret at
all, we've done our job and shouldn't bother again.

[dave_cheney.net, 2012-09-06 02:07:16]

Please take a look.

[niemeyer, 2012-09-06 02:19:23]

LGTM, thanks.

https://codereview.appspot.com/6496069/diff/8001/juju/conn.go
File juju/conn.go (right):

https://codereview.appspot.com/6496069/diff/8001/juju/conn.go#newcode82
juju/conn.go:82: // from the local configuration.
// updateSecrets writes secrets into the environment when there are none.
// This is done because environments such as ec2 offer no way to securely
// deliver the secrets onto the machine, so the bootstrap is done with the
// whole environment configuration but without secrets, and then secrets
// are delivered on the first communication with the running environment.

https://codereview.appspot.com/6496069/diff/8001/juju/conn.go#newcode94
juju/conn.go:94: if _, exists := attrs[k]; exists {
// Environment already has secrets. Won't send again.

[dave_cheney.net, 2012-09-06 02:44:08]

Thanks. Sorry there is a large merge conflict so I will need to repropose.

https://codereview.appspot.com/6496069/diff/8001/juju/conn.go
File juju/conn.go (right):

https://codereview.appspot.com/6496069/diff/8001/juju/conn.go#newcode82
juju/conn.go:82: // from the local configuration.
On 2012/09/06 02:19:23, niemeyer wrote:
> // updateSecrets writes secrets into the environment when there are none.
> // This is done because environments such as ec2 offer no way to securely
> // deliver the secrets onto the machine, so the bootstrap is done with the
> // whole environment configuration but without secrets, and then secrets
> // are delivered on the first communication with the running environment.

Done.

https://codereview.appspot.com/6496069/diff/8001/juju/conn.go#newcode94
juju/conn.go:94: if _, exists := attrs[k]; exists {
On 2012/09/06 02:19:23, niemeyer wrote:
> // Environment already has secrets. Won't send again.

Done.

[dave_cheney.net, 2012-09-06 03:24:54]

Please take a look.

[dave_cheney.net, 2012-09-06 03:29:11]

*** Submitted:

juju: update missing secrets if required

Update any missing secrets iff no secrets are present in the environment.

R=niemeyer
CC=
https://codereview.appspot.com/6496069