Skip to content

Commit 713fda4

Browse files
cypharcyphar
committed
merge branch 'automated-history-mutation'
Implements cyphar/umoci#36 Signed-off-by: Aleksa Sarai <[email protected]>
2 parents 4f24278 + 94a7d9a commit 713fda4

File tree

5 files changed

+243
-31
lines changed

5 files changed

+243
-31
lines changed

‎cmd/umoci/config.go‎

Lines changed: 49 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818
package main
1919

2020
import (
21-
"encoding/json"
2221
"fmt"
2322
"time"
2423

@@ -44,8 +43,7 @@ var configFlags = []cli.Flag{
4443
// FIXME: These aren't really safe to expose.
4544
//cli.StringFlag{Name: "rootfs.type"},
4645
//cli.StringSliceFlag{Name: "rootfs.diffids"},
47-
cli.StringSliceFlag{Name: "history"}, // FIXME: Implement this is a way that isn't super dodgy.
48-
cli.StringFlag{Name: "created"}, // FIXME: Implement TimeFlag.
46+
cli.StringFlag{Name: "created"}, // FIXME: Implement TimeFlag.
4947
cli.StringFlag{Name: "author"},
5048
cli.StringFlag{Name: "architecture"},
5149
cli.StringFlag{Name: "os"},
@@ -82,6 +80,24 @@ based.`,
8280
Name: "tag",
8381
Usage: "tag name for repacked image",
8482
},
83+
// XXX: These flags are replicated for umoci-repack. This should be
84+
// refactored.
85+
cli.StringFlag{
86+
Name: "history.comment",
87+
Usage: "comment for the history entry corresponding to the modified configuration",
88+
},
89+
cli.StringFlag{
90+
Name: "history.created_by",
91+
Usage: "created_by value for the history entry corresponding to the modified configuration",
92+
},
93+
cli.StringFlag{
94+
Name: "history.author",
95+
Usage: "author value for the history entry corresponding to the the modified configuration",
96+
},
97+
cli.StringFlag{
98+
Name: "history.created",
99+
Usage: "created value for the history entry corresponding to the the modified configuration",
100+
},
85101
},
86102

87103
Action: config,
@@ -101,8 +117,6 @@ func mutateConfig(g *igen.Generator, ctx *cli.Context) error {
101117
case "rootfs.diffids":
102118
//g.ClearRootfsDiffIDs()
103119
return fmt.Errorf("clear rootfs.diffids is not safe")
104-
case "history":
105-
g.ClearHistory()
106120
default:
107121
return fmt.Errorf("unknown set to clear: %s", key)
108122
}
@@ -174,17 +188,6 @@ func mutateConfig(g *igen.Generator, ctx *cli.Context) error {
174188
g.AddRootfsDiffID(diffid)
175189
}
176190
}
177-
// FIXME: Also implement this is a way that isn't broken (using string is broken).
178-
if ctx.IsSet("history") {
179-
// This is a JSON-encoded version of v1.History. I'm sorry.
180-
for _, historyJSON := range ctx.StringSlice("history") {
181-
var history v1.History
182-
if err := json.Unmarshal([]byte(historyJSON), &history); err != nil {
183-
return fmt.Errorf("error reading --history argument: %s", err)
184-
}
185-
g.AddHistory(history)
186-
}
187-
}
188191
return nil
189192
}
190193

@@ -265,6 +268,36 @@ func config(ctx *cli.Context) error {
265268
return err
266269
}
267270

271+
var (
272+
created = ctx.String("history.created")
273+
createdBy = ctx.String("history.created_by")
274+
author = ctx.String("history.author")
275+
comment = ctx.String("history.comment")
276+
)
277+
278+
if created == "" {
279+
// XXX: We really should make sure that the format of this is right.
280+
// Also, does this option _really_ make sense?
281+
created = time.Now().Format(igen.ISO8601)
282+
}
283+
if createdBy == "" {
284+
// XXX: Should we append argv to this?
285+
createdBy = "umoci repack"
286+
}
287+
if author == "" {
288+
author = g.Author()
289+
}
290+
291+
// Add a history entry about the fact we just changed the config.
292+
// FIXME: It should be possible to disable this.
293+
g.AddHistory(v1.History{
294+
Created: created,
295+
CreatedBy: createdBy,
296+
Author: author,
297+
Comment: comment,
298+
EmptyLayer: true,
299+
})
300+
268301
// Update config and create a new blob for it.
269302
*config = g.Image()
270303
newConfigDigest, newConfigSize, err := engine.PutBlobJSON(context.TODO(), config)

‎cmd/umoci/repack.go‎

Lines changed: 54 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,11 @@ import (
2525
"os"
2626
"path/filepath"
2727
"strings"
28+
"time"
2829

2930
"github.com/Sirupsen/logrus"
3031
"github.com/cyphar/umoci/image/cas"
31-
"github.com/cyphar/umoci/image/generator"
32+
igen "github.com/cyphar/umoci/image/generator"
3233
"github.com/cyphar/umoci/image/layer"
3334
"github.com/opencontainers/image-spec/specs-go/v1"
3435
"github.com/urfave/cli"
@@ -63,6 +64,24 @@ manifest and configuration information uses the new diff atop the old manifest.`
6364
Name: "tag",
6465
Usage: "tag name for repacked image",
6566
},
67+
// XXX: These flags are replicated for umoci-config. This should be
68+
// refactored.
69+
cli.StringFlag{
70+
Name: "history.comment",
71+
Usage: "comment for the history entry corresponding to the new layer",
72+
},
73+
cli.StringFlag{
74+
Name: "history.created_by",
75+
Usage: "created_by value for the history entry corresponding to the new layer",
76+
},
77+
cli.StringFlag{
78+
Name: "history.author",
79+
Usage: "author value for the history entry corresponding to the new layer",
80+
},
81+
cli.StringFlag{
82+
Name: "history.created",
83+
Usage: "created value for the history entry corresponding to the the modified configuration",
84+
},
6685
},
6786

6887
Action: repack,
@@ -229,14 +248,47 @@ func repack(ctx *cli.Context) error {
229248
return fmt.Errorf("config blob type not implemented: %s", configBlob.MediaType)
230249
}
231250

232-
g, err := generator.NewFromImage(*config)
251+
g, err := igen.NewFromImage(*config)
233252
if err != nil {
234253
return err
235254
}
236255

237256
// Append our new layer to the set of DiffIDs.
238257
g.AddRootfsDiffID(layerDiffID)
239258

259+
var (
260+
created = ctx.String("history.created")
261+
createdBy = ctx.String("history.created_by")
262+
author = ctx.String("history.author")
263+
comment = ctx.String("history.comment")
264+
)
265+
266+
if created == "" {
267+
// XXX: We really should make sure that the format of this is right.
268+
// Also, does this option _really_ make sense?
269+
created = time.Now().Format(igen.ISO8601)
270+
}
271+
if createdBy == "" {
272+
// XXX: Should we append argv to this?
273+
createdBy = "umoci repack"
274+
}
275+
if author == "" {
276+
author = g.Author()
277+
}
278+
if comment == "" {
279+
comment = fmt.Sprintf("repack diffid %s", layerDiffID)
280+
}
281+
282+
// We need to add a history entry here, since a lot of tooling depends on
283+
// the EmptyLayer == false semantics of Docker's history.
284+
g.AddHistory(v1.History{
285+
Created: created,
286+
CreatedBy: createdBy,
287+
Author: author,
288+
Comment: comment,
289+
EmptyLayer: false,
290+
})
291+
240292
// Update config and create a new blob for it.
241293
*config = g.Image()
242294
newConfigDigest, newConfigSize, err := engine.PutBlobJSON(context.TODO(), config)

‎test/config.bats‎

Lines changed: 68 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,20 @@ function teardown() {
5050
sane_run diff -u "$BATS_TMPDIR/a-config.json" "$BATS_TMPDIR/b-config.json"
5151
[ "$status" -eq 0 ]
5252
[ -z "$output" ]
53+
54+
# Make sure that the history was modified.
55+
umoci stat --image "$IMAGE" --tag "$TAG" --json
56+
[ "$status" -eq 0 ]
57+
numLinesA="$(echo "$output" | jq -SM '.history | length')"
58+
59+
umoci stat --image "$IMAGE" --tag "${TAG}-new" --json
60+
[ "$status" -eq 0 ]
61+
numLinesB="$(echo "$output" | jq -SM '.history | length')"
62+
63+
# Number of lines should be greater.
64+
[ "$numLinesB" -gt "$numLinesA" ]
65+
# The final layer should be an empty_layer now.
66+
[[ "$(echo "$output" | jq -SM '.history[-1].empty_layer')" == "true" ]]
5367
}
5468

5569
@test "umoci config --config.user 'user'" {
@@ -491,23 +505,64 @@ function teardown() {
491505

492506
# XXX: This doesn't do any actual testing of the results of any of these flags.
493507
# This needs to be fixed after we implement raw-cat or something like that.
494-
@test "umoci config --[author+created+history]" {
508+
@test "umoci config --[author+created]" {
495509
# Modify everything.
496-
umoci config --image "$IMAGE" --from "$TAG" --tag "${TAG}" --author="Aleksa Sarai <[email protected]>" --created="2016-03-25T12:34:02.655002+11:00" \
497-
--clear=history --history '{"created_by": "ls -la", "comment": "should work", "author": "me", "empty_layer": false, "created": "2016-03-25T12:34:02.655002+11:00"}' \
498-
--history '{"created_by": "ls -la", "author": "me", "empty_layer": false}'
510+
umoci config --image "$IMAGE" --from "$TAG" --tag "${TAG}-new" --author="Aleksa Sarai <[email protected]>" --created="2016-03-25T12:34:02.655002+11:00"
499511
[ "$status" -eq 0 ]
500512

501-
# Make sure that --history doesn't work with a random string.
502-
umoci config --image "$IMAGE" --from "$TAG" --tag "${TAG}" --history "some random string"
503-
[ "$status" -ne 0 ]
504-
# FIXME It turns out that Go's JSON parser will ignore unknown keys...
505-
#umoci config --image "$IMAGE" --from "$TAG" --tag "${TAG}" --history '{"unknown key": 12, "created_by": "ls -la", "comment": "should not work"}'
506-
#[ "$status" -ne 0 ]
507-
508513
# Make sure that --created doesn't work with a random string.
509-
umoci config --image "$IMAGE" --from "$TAG" --tag "${TAG}" --created="not a date"
514+
umoci config --image "$IMAGE" --from "$TAG" --tag "${TAG}-new" --created="not a date"
510515
[ "$status" -ne 0 ]
511-
umoci config --image "$IMAGE" --from "$TAG" --tag "${TAG}" --created="Jan 04 2004"
516+
umoci config --image "$IMAGE" --from "$TAG" --tag "${TAG}-new" --created="Jan 04 2004"
512517
[ "$status" -ne 0 ]
518+
519+
# Make sure that the history was modified and the author is now me.
520+
umoci stat --image "$IMAGE" --tag "$TAG" --json
521+
[ "$status" -eq 0 ]
522+
numLinesA="$(echo "$output" | jq -SMr '.history | length')"
523+
524+
umoci stat --image "$IMAGE" --tag "${TAG}-new" --json
525+
[ "$status" -eq 0 ]
526+
numLinesB="$(echo "$output" | jq -SMr '.history | length')"
527+
528+
# Number of lines should be greater.
529+
[ "$numLinesB" -gt "$numLinesA" ]
530+
# The final layer should be an empty_layer now.
531+
[[ "$(echo "$output" | jq -SMr '.history[-1].empty_layer')" == "true" ]]
532+
# The author should've changed.
533+
[[ "$(echo "$output" | jq -SMr '.history[-1].author')" == "Aleksa Sarai <[email protected]>" ]]
534+
}
535+
536+
# XXX: We don't do any testing of --author and that the config is changed properly.
537+
@test "umoci config --history.*" {
538+
# Modify something and set the history values.
539+
umoci config --image "$IMAGE" --from "$TAG" --tag "${TAG}-new" \
540+
--history.author="Not Aleksa <[email protected]>" \
541+
--history.comment="/* Not a real comment. */" \
542+
--history.created_by="-- <bats> integration test --" \
543+
--history.created="2016-12-09T04:45:40+11:00" \
544+
--author="Aleksa Sarai <[email protected]>"
545+
[ "$status" -eq 0 ]
546+
547+
# Make sure that the history was modified.
548+
umoci stat --image "$IMAGE" --tag "$TAG" --json
549+
[ "$status" -eq 0 ]
550+
numLinesA="$(echo "$output" | jq -SMr '.history | length')"
551+
552+
umoci stat --image "$IMAGE" --tag "${TAG}-new" --json
553+
[ "$status" -eq 0 ]
554+
numLinesB="$(echo "$output" | jq -SMr '.history | length')"
555+
556+
# Number of lines should be greater.
557+
[ "$numLinesB" -gt "$numLinesA" ]
558+
# The final layer should be an empty_layer now.
559+
[[ "$(echo "$output" | jq -SMr '.history[-1].empty_layer')" == "true" ]]
560+
# The author should've changed to --history.author.
561+
[[ "$(echo "$output" | jq -SMr '.history[-1].author')" == "Not Aleksa <[email protected]>" ]]
562+
# The comment should be added.
563+
[[ "$(echo "$output" | jq -SMr '.history[-1].comment')" == "/* Not a real comment. */" ]]
564+
# The created_by should be set.
565+
[[ "$(echo "$output" | jq -SMr '.history[-1].created_by')" == "-- <bats> integration test --" ]]
566+
# The created should be set.
567+
[[ "$(echo "$output" | jq -SMr '.history[-1].created')" == "2016-12-09T04:45:40+11:00" ]]
513568
}

‎test/create.bats‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,4 +86,10 @@ function teardown() {
8686
sane_run jq -SMr '.process.user.additionalGids' "$BUNDLE/config.json"
8787
[ "$status" -eq 0 ]
8888
[[ "$output" == "null" ]]
89+
90+
# Check that the history looks sane.
91+
umoci stat --image "$NEWIMAGE" --tag "latest" --json
92+
[ "$status" -eq 0 ]
93+
# There should be no non-empty_layers.
94+
[[ "$(echo "$output" | jq -SM '[.history[] | .empty_layer == false] | any')" == "false" ]]
8995
}

‎test/repack.bats‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,20 @@ function teardown() {
6464
[ -d "$BUNDLE_B/rootfs/newdir" ]
6565
[ -f "$BUNDLE_B/rootfs/newdir/anotherfile" ]
6666
[ -L "$BUNDLE_B/rootfs/newdir/link" ]
67+
68+
# Make sure we added a new layer.
69+
umoci stat --image "$IMAGE" --tag "$TAG" --json
70+
[ "$status" -eq 0 ]
71+
numLinesA="$(echo "$output" | jq -SM '.history | length')"
72+
73+
umoci stat --image "$IMAGE" --tag "${TAG}-new" --json
74+
[ "$status" -eq 0 ]
75+
numLinesB="$(echo "$output" | jq -SM '.history | length')"
76+
77+
# Number of lines should be greater.
78+
[ "$numLinesB" -gt "$numLinesA" ]
79+
# Make sure that the new layer is a non-empty_layer.
80+
[[ "$(echo "$output" | jq -SM '.history[-1].empty_layer')" == "false" ]]
6781
}
6882

6983
@test "umoci repack [whiteout]" {
@@ -102,6 +116,11 @@ function teardown() {
102116
! [ -e "$BUNDLE_A/rootfs/etc" ]
103117
! [ -e "$BUNDLE_A/rootfs/bin/sh" ]
104118
! [ -e "$BUNDLE_A/rootfs/usr/bin/env" ]
119+
120+
# Make sure that the new layer is a non-empty_layer.
121+
umoci stat --image "$IMAGE" --tag "${TAG}-new" --json
122+
[ "$status" -eq 0 ]
123+
[[ "$(echo "$output" | jq -SM '.history[-1].empty_layer')" == "false" ]]
105124
}
106125

107126
@test "umoci repack [replace]" {
@@ -144,6 +163,53 @@ function teardown() {
144163
[ -f "$BUNDLE_A/rootfs/etc" ]
145164
[ -d "$BUNDLE_A/rootfs/bin/sh" ]
146165
[ -L "$BUNDLE_A/rootfs/usr/bin/env" ]
166+
167+
# Make sure that the new layer is a non-empty_layer.
168+
umoci stat --image "$IMAGE" --tag "${TAG}" --json
169+
[ "$status" -eq 0 ]
170+
[[ "$(echo "$output" | jq -SM '.history[-1].empty_layer')" == "false" ]]
171+
}
172+
173+
@test "umoci repack --history.*" {
174+
BUNDLE="$(setup_bundle)"
175+
176+
# Unpack the image.
177+
umoci unpack --image "$IMAGE" --from "$TAG" --bundle "$BUNDLE"
178+
[ "$status" -eq 0 ]
179+
180+
# Make some small change.
181+
touch "$BUNDLE/a_small_change"
182+
now="$(date --iso-8601=seconds)"
183+
184+
# Repack the image, setting history values.
185+
umoci repack --image "$IMAGE" --bundle="$BUNDLE" --tag "${TAG}-new" \
186+
--history.author="Some Author <[email protected]>" \
187+
--history.comment="Made a_small_change." \
188+
--history.created_by="touch '$BUNDLE/a_small_change'" \
189+
--history.created="$now"
190+
[ "$status" -eq 0 ]
191+
192+
# Make sure that the history was modified.
193+
umoci stat --image "$IMAGE" --tag "$TAG" --json
194+
[ "$status" -eq 0 ]
195+
numLinesA="$(echo "$output" | jq -SMr '.history | length')"
196+
197+
umoci stat --image "$IMAGE" --tag "${TAG}-new" --json
198+
[ "$status" -eq 0 ]
199+
numLinesB="$(echo "$output" | jq -SMr '.history | length')"
200+
201+
# Number of lines should be greater.
202+
[ "$numLinesB" -gt "$numLinesA" ]
203+
# The final layer should not be an empty_layer now.
204+
[[ "$(echo "$output" | jq -SMr '.history[-1].empty_layer')" == "false" ]]
205+
# The author should've changed to --history.author.
206+
[[ "$(echo "$output" | jq -SMr '.history[-1].author')" == "Some Author <[email protected]>" ]]
207+
# The comment should be added.
208+
[[ "$(echo "$output" | jq -SMr '.history[-1].comment')" == "Made a_small_change." ]]
209+
# The created_by should be set.
210+
[[ "$(echo "$output" | jq -SMr '.history[-1].created_by')" == "touch '$BUNDLE/a_small_change'" ]]
211+
# The created should be set.
212+
[[ "$(echo "$output" | jq -SMr '.history[-1].created')" == "$now" ]]
147213
}
148214

149215
# TODO: Test hardlinks once we fix the hardlink issue. https://github.com/cyphar/umoci/issues/29

0 commit comments

Comments
 (0)