New upgrade step to rename addmodel permission #6435

Merged
merged 1 commit into from Oct 12, 2016

Conversation

Projects
None yet
3 participants
Owner

wallyworld commented Oct 12, 2016

addmodel permission was recently renamed to add-model.
This PR adds upgrade steps to convert any existing values.

QA: bootstrap older juju. Add addmodel permission to a user. upgrade. Check permission updated.

mjs approved these changes Oct 12, 2016

Looks fine. Just a few suggestions.

state/upgrades.go
+ var doc bson.M
+ for iter.Next(&doc) {
+ fieldVal, ok := doc["access"]
+ if !ok || fieldVal != "addmodel" {
@mjs

mjs Oct 12, 2016

Contributor

You could avoid this and make the set of documents shorter by querying for permissions which use addmodel.

state/upgrades.go
+ return errors.New("no id found in permission doc")
+ }
+
+ update := bson.DocElem{"$set", bson.D{{"access", "add-model"}}}
@mjs

mjs Oct 12, 2016

Contributor

Why not bson.D{{""$set", bson.D{{"access", "add-model"}}}}? It's shorter and saves having to wrap the bson.D later.

Also, there's no need to define this in the loop. Given the update is always the same, just define it above.

Owner

wallyworld commented Oct 12, 2016

$$merge$$

Contributor

jujubot commented Oct 12, 2016

Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju

@jujubot jujubot merged commit 52b1091 into juju:master Oct 12, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment