Skip to content
This repository has been archived by the owner on Nov 1, 2022. It is now read-only.

Commit

Permalink
Fix multiline lock reason yaml corruption
Browse files Browse the repository at this point in the history
Lock reason messages can be spread over multiple lines. YAML's multiline
output omits indentation for empty lines. The regex looking for a
fragment to replace did not include these lines and therefore was not
removing the full multiline content but just until the first empty line.

One implication was that the `flux.weave.works/locked_user` annotation was
never cleared and the original locker wrongfully returned as the author of
the current lock.

This PR expands matching for existing annotations to include these empty
lines.

User locked with multiline and empty line:
```
metadata:
  annotations:
    flux.weave.works/locked: "true"
    flux.weave.works/locked_msg: |-
      first

      second
    flux.weave.works/locked_user: test <test@test.test>
```

After removing lock:
```
metadata:
  annotations:

      second
    flux.weave.works/locked_user: test <test@test.test>
```

Another person locks:
```
metadata:
  annotations:
    flux.weave.works/locked: "true"
    flux.weave.works/locked_msg: Do not update.
    flux.weave.works/locked_user: someone <else@test.test>

      second
    flux.weave.works/locked_user: test <test@test.test>
```
  • Loading branch information
rndstr committed Mar 6, 2018
1 parent 6aced8e commit 57ae351
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 4 deletions.
2 changes: 1 addition & 1 deletion cluster/kubernetes/policies.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ func updateAnnotations(def []byte, tagAll string, f func(map[string]string) map[
// TODO: This should handle potentially different indentation.
// TODO: There's probably a more elegant regex-ey way to do this in one pass.
replaced := false
annotationsRE := regexp.MustCompile(`(?m:\n annotations:\s*(?:#.*)*(?:\n .*)*$)`)
annotationsRE := regexp.MustCompile(`(?m:\n annotations:\s*(?:#.*)*(?:\n .*|\n)*$)`)
newDef := annotationsRE.ReplaceAllStringFunc(string(def), func(found string) string {
if !replaced {
replaced = true
Expand Down
30 changes: 27 additions & 3 deletions cluster/kubernetes/policies_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ package kubernetes

import (
"bytes"
"fmt"
"strings"
"testing"
"text/template"

Expand Down Expand Up @@ -96,6 +98,22 @@ func TestUpdatePolicies(t *testing.T) {
Remove: policy.Set{policy.Automated: "true"},
},
},
{
name: "multiline",
in: map[string]string{"flux.weave.works/locked_msg": "|-\n first\n second"},
out: nil,
update: policy.Update{
Remove: policy.Set{policy.LockedMsg: "foo"},
},
},
{
name: "multiline with empty line",
in: map[string]string{"flux.weave.works/locked_msg": "|-\n first\n\n third"},
out: nil,
update: policy.Update{
Remove: policy.Set{policy.LockedMsg: "foo"},
},
},
} {
caseIn := templToString(t, annotationsTemplate, c.in)
caseOut := templToString(t, annotationsTemplate, c.out)
Expand All @@ -113,7 +131,7 @@ apiVersion: extensions/v1beta1
kind: Deployment
metadata: # comment really close to the war zone
{{with .}}annotations:{{range $k, $v := .}}
{{$k}}: {{printf "%q" $v}}{{end}}
{{$k}}: {{printf "%s" $v}}{{end}}
{{end}}name: nginx
spec:
replicas: 1
Expand All @@ -129,9 +147,15 @@ spec:
- containerPort: 80
`))

func templToString(t *testing.T, templ *template.Template, data interface{}) string {
func templToString(t *testing.T, templ *template.Template, annotations map[string]string) string {
for k, v := range annotations {
// Don't wrap multilines
if !strings.HasPrefix(v, "|") {
annotations[k] = fmt.Sprintf("%q", v)
}
}
out := &bytes.Buffer{}
err := templ.Execute(out, data)
err := templ.Execute(out, annotations)
if err != nil {
t.Fatal(err)
}
Expand Down

0 comments on commit 57ae351

Please sign in to comment.