search

juju / persistent-cookiejar

cookiejar is a fork of net/http/cookiejar that allows serialisation of the stored cookies
BSD 3-Clause "New" or "Revised" License
113 stars 76 forks source link

Need a migration from serialization format that was used in Juju 1.25... #8

Closed cmars closed 8 years ago

cmars commented 9 years ago

See https://bugs.launchpad.net/juju-core/+bug/1511717 for background. It appears that we've broken serialization compatibility in ~/.go-cookies. The version that was built into Juju 1.25 writes an object, the version in 1.26 (current master) writes a slice of objects.

Since 1.25 is now released, we will need to provide a migration to avoid breaking users. I've confirmed with manual testing that in a 1.25 -> 1.26 client upgrade scenario, with a .go-cookies written by 1.25 and then read by a 1.26 client, the user will get a very misleading error to the effect of:

ubuntu@jujudev:~/juju/gopath/src/github.com/juju/juju/cmd/juju$ ./juju status
ERROR Unable to connect to environment "local".
Please check your credentials or use 'juju bootstrap' to create a new environment.

Error details:
cannot load cookies: json: cannot unmarshal object into Go value of type []cookiejar.entry

ubuntu@jujudev:~/juju/gopath/src/github.com/juju/juju/cmd/juju$ rm ~/.go-cookies 
ubuntu@jujudev:~/juju/gopath/src/github.com/juju/juju/cmd/juju$ ./juju status                                                                          
environment: local
machines:
...

I think if persistent-cookiejar were tolerant of reading the older format, we'd be ok.

cmars commented 9 years ago

@rogpeppe @mhilton Please take a look at this on Monday.. The linked bug above is currently blocking juju master. Thanks!

rogpeppe commented 9 years ago

Oops, thought cookies weren't used until chicago-cubs merged last week (so backward compatibility not an issue), but it seems they've been around for ages. Will fix asap.

abentley commented 9 years ago

CI has multiple version under test simultaneously, so if the solution is a migration, old versions will be tested with new cookie files, and presumably fail. CI will still be broken. I suggest using a different file location, so that there is no conflict.

cmars commented 9 years ago

Thanks @rogpeppe !

cmars commented 9 years ago

@abentley That sounds like a CI requirement, not a UX one. Our priority is to first make sure users aren't negatively impacted in a client upgrade scenario. Changing the file path does not seem warranted here. We don't guarantee client downgrade compatibility for the client files in $JUJU_HOME, for example.

When we CI test this, the client home directory should be intentionally reset to a known state prior to testing, for a specific case: client upgrading from 1.25 -> 1.26. Otherwise, the workspace should be cleaned up.

Let's discuss this further on Monday, I'll try to drop in to one of the QA calls.

anastasiamac commented 8 years ago

@cmars The bug you have mentioned above was Released. Is this issue now addressed and can be closed?