Skip to content

Commit 731b4c5

Browse files
committed
cmd: *: unify --image and --from
The usage for --image and --from was quite frustrating and also was confusing (since the image path and the tag are quite related concepts). So switch to an --image <path>[:<tag>] layout. In addition, a bunch of changes to help pages, categorisation of commands and --layout flags (which are added to differentiate between "layout" commands and "image" commands) were done just to make everything a bit more consistent. Signed-off-by: Aleksa Sarai <[email protected]>
1 parent 8f75982 commit 731b4c5

File tree

8 files changed

+188
-153
lines changed

8 files changed

+188
-153
lines changed

cmd/umoci/config.go

Lines changed: 17 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -60,25 +60,21 @@ func init() {
6060
var configCommand = cli.Command{
6161
Name: "config",
6262
Usage: "modifies the image configuration of an OCI image",
63-
ArgsUsage: `--image <image-path> --from <reference>
63+
ArgsUsage: `--image <image-path>[:<tag>] [--tag <new-tag>]
6464
65-
Where "<image-path>" is the path to the OCI image, and "<reference>" is the
66-
name of the reference descriptor from which the config modifications will be
67-
based.`,
65+
Where "<image-path>" is the path to the OCI image, and "<tag>" is the name of
66+
the tagged image from which the config modifications will be based (if not
67+
specified, it defaults to "latest"). "<new-tag>" is the new reference name to
68+
save the new image as, if this is not specified then umoci will replace the old
69+
image.`,
70+
71+
// config modifies a particular image manifest.
72+
Category: "image",
6873

6974
Flags: []cli.Flag{
70-
// FIXME: This really should be a global option.
71-
cli.StringFlag{
72-
Name: "image",
73-
Usage: "path to OCI image bundle",
74-
},
75-
cli.StringFlag{
76-
Name: "from",
77-
Usage: "reference descriptor name to modify",
78-
},
7975
cli.StringFlag{
8076
Name: "tag",
81-
Usage: "tag name for repacked image",
77+
Usage: "tag name for repacked image (if not specified then the original tag will be clobbered)",
8278
},
8379
// XXX: These flags are replicated for umoci-repack. This should be
8480
// refactored.
@@ -192,14 +188,13 @@ func mutateConfig(g *igen.Generator, ctx *cli.Context) error {
192188
}
193189

194190
func config(ctx *cli.Context) error {
195-
// FIXME: Is there a nicer way of dealing with mandatory arguments?
196-
imagePath := ctx.String("image")
197-
if imagePath == "" {
198-
return fmt.Errorf("image path cannot be empty")
199-
}
200-
fromName := ctx.String("from")
201-
if fromName == "" {
202-
return fmt.Errorf("reference name cannot be empty")
191+
imagePath := ctx.App.Metadata["layout"].(string)
192+
fromName := ctx.App.Metadata["tag"].(string)
193+
194+
tagName := ctx.String("tag")
195+
if tagName == "" {
196+
// By default we clobber the old tag.
197+
tagName = fromName
203198
}
204199

205200
// Get a reference to the CAS.
@@ -337,11 +332,6 @@ func config(ctx *cli.Context) error {
337332
"size": newDescriptor.Size,
338333
}).Infof("created new image")
339334

340-
tagName := ctx.String("tag")
341-
if tagName == "" {
342-
return nil
343-
}
344-
345335
// We have to clobber the old reference.
346336
// XXX: Should we output some warning if we actually did remove an old
347337
// reference?

cmd/umoci/create.go

Lines changed: 14 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -32,25 +32,25 @@ import (
3232
"golang.org/x/net/context"
3333
)
3434

35+
// TODO: This command needs to be split into umoci-init(1) and umoci-create(1).
36+
3537
var createCommand = cli.Command{
3638
Name: "create",
37-
Usage: "creates an OCI image and/or base manifest",
38-
ArgsUsage: `--image <image-path> [--tag <new-manifest>]
39+
Usage: "creates an OCI layout and/or base manifest",
40+
ArgsUsage: `--layout <image-path> [--tag <new-tag>]
3941
40-
Where "<image-path>" is the path to the OCI image, and "<new-manifest>" is an
42+
Where "<image-path>" is the path to the OCI image, and "<new-tag>" is an
4143
optional tag which will be linked to a new empty manifest.
4244
4345
If --tag is specified, the empty manifest created is such that you can use
4446
umoci-unpack(1), umoci-repack(1), and umoci-config(1) to modify the new
4547
manifest as you see fit. This allows you to create entirely new images without
4648
needing a base image to start from.`,
4749

50+
// create modifies an image layout.
51+
Category: "layout",
52+
4853
Flags: []cli.Flag{
49-
// FIXME: This really should be a global option.
50-
cli.StringFlag{
51-
Name: "image",
52-
Usage: "path to OCI image bundle",
53-
},
5454
cli.StringFlag{
5555
Name: "tag",
5656
Usage: "tag name for new manifest",
@@ -61,14 +61,10 @@ needing a base image to start from.`,
6161
}
6262

6363
func create(ctx *cli.Context) error {
64-
// FIXME: Is there a nicer way of dealing with mandatory arguments?
65-
imagePath := ctx.String("image")
66-
if imagePath == "" {
67-
return fmt.Errorf("image path cannot be empty")
68-
}
64+
imagePath := ctx.App.Metadata["layout"].(string)
6965

7066
if fi, err := os.Stat(imagePath); os.IsNotExist(err) {
71-
logrus.Infof("creating non-existent image")
67+
logrus.Infof("creating non-existent layout")
7268
if err := cas.CreateLayout(imagePath); err != nil {
7369
return err
7470
}
@@ -100,8 +96,10 @@ func create(ctx *cli.Context) error {
10096
g.SetCreated(createTime)
10197
g.SetOS(runtime.GOOS)
10298
g.SetArchitecture(runtime.GOARCH)
99+
// XXX: Should we include this?
103100
g.AddHistory(v1.History{
104-
CreatedBy: fmt.Sprintf("umoci create [at '%s']", createTime),
101+
CreatedBy: fmt.Sprintf("umoci create"),
102+
Created: createTime.Format(igen.ISO8601),
105103
EmptyLayer: true,
106104
})
107105

@@ -157,7 +155,7 @@ func create(ctx *cli.Context) error {
157155
"mediatype": descriptor.MediaType,
158156
"digest": descriptor.Digest,
159157
"size": descriptor.Size,
160-
}).Infof("created new image")
158+
}).Infof("created new layout")
161159

162160
// We have to clobber the old reference.
163161
// XXX: Should we output some warning if we actually did remove an old

cmd/umoci/gc.go

Lines changed: 5 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,6 @@
1818
package main
1919

2020
import (
21-
"fmt"
22-
2321
"github.com/cyphar/umoci/image/cas"
2422
"github.com/urfave/cli"
2523
"golang.org/x/net/context"
@@ -28,31 +26,21 @@ import (
2826
var gcCommand = cli.Command{
2927
Name: "gc",
3028
Usage: "garbage-collects an OCI image's blobs",
31-
ArgsUsage: `--image <image-path>
29+
ArgsUsage: `--layout <image-path>
3230
3331
Where "<image-path>" is the path to the OCI image.
3432
3533
This command will do a mark-and-sweep garbage collection of the provided OCI
3634
image, only retaining blobs which can be reached by a descriptor path from the
3735
root set of references. All other blobs will be removed.`,
3836

39-
Flags: []cli.Flag{
40-
// FIXME: This really should be a global option.
41-
cli.StringFlag{
42-
Name: "image",
43-
Usage: "path to OCI image bundle",
44-
},
45-
},
46-
47-
Action: gc,
37+
// create modifies an image layout.
38+
Category: "layout",
39+
Action: gc,
4840
}
4941

5042
func gc(ctx *cli.Context) error {
51-
// FIXME: Is there a nicer way of dealing with mandatory arguments?
52-
imagePath := ctx.String("image")
53-
if imagePath == "" {
54-
return fmt.Errorf("image path cannot be empty")
55-
}
43+
imagePath := ctx.App.Metadata["layout"].(string)
5644

5745
// Get a reference to the CAS.
5846
engine, err := cas.Open(imagePath)

cmd/umoci/main.go

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ package main
2020
import (
2121
"fmt"
2222
"os"
23+
"regexp"
24+
"strings"
2325

2426
"github.com/Sirupsen/logrus"
2527
"github.com/urfave/cli"
@@ -33,6 +35,9 @@ var version = ""
3335
// populated on build by make.
3436
var gitCommit = ""
3537

38+
// refRegexp defines the regexp that a given OCI tag must obey.
39+
var refRegexp = regexp.MustCompile(`^([A-Za-z0-9._-]+)+$`)
40+
3641
const (
3742
usage = `umoci modifies Open Container images`
3843
)
@@ -83,6 +88,103 @@ func main() {
8388
statCommand,
8489
}
8590

91+
app.Metadata = map[string]interface{}{}
92+
93+
// In order to consolidate a lot of the --image and --layout handling, we
94+
// have to do some monkey-patching of commands. In particular, we set up
95+
// the --image and --layout flags (for image and layout category commands)
96+
// and then add parsing code to cmd.Before so that we can validate and
97+
// parse the required arguments. It's definitely not pretty, but it's the
98+
// best we can do without making them all global flags and then having odd
99+
// semantics.
100+
101+
for idx, cmd := range app.Commands {
102+
var flag cli.Flag
103+
oldBefore := cmd.Before
104+
105+
switch cmd.Category {
106+
case "image":
107+
// Does the command modify images (manifests)?
108+
flag = cli.StringFlag{
109+
Name: "image",
110+
Usage: "OCI image URI of the form 'path[:tag]'",
111+
}
112+
113+
// Add BeforeFunc that will verify code.
114+
cmd.Before = func(ctx *cli.Context) error {
115+
// Parse --image.
116+
image := ctx.String("image")
117+
118+
var dir, tag string
119+
sep := strings.LastIndex(image, ":")
120+
if sep == -1 {
121+
dir = image
122+
tag = "latest"
123+
} else {
124+
dir = image[:sep]
125+
tag = image[sep+1:]
126+
}
127+
128+
// Verify directory value.
129+
if strings.Contains(dir, ":") {
130+
return fmt.Errorf("invalid --image: directory contains ':' character: '%s'", dir)
131+
}
132+
if dir == "" {
133+
return fmt.Errorf("invalid --image: directory is empty")
134+
}
135+
136+
// Verify tag value.
137+
if !refRegexp.MatchString(tag) {
138+
return fmt.Errorf("invalid --image: tag contains invalid characters: '%s'", tag)
139+
}
140+
if tag == "" {
141+
return fmt.Errorf("invalid --image: tag is empty")
142+
}
143+
144+
ctx.App.Metadata["layout"] = dir
145+
ctx.App.Metadata["tag"] = tag
146+
147+
if oldBefore != nil {
148+
return oldBefore(ctx)
149+
}
150+
return nil
151+
}
152+
153+
case "layout":
154+
// Does the command modify an OCI image layout itself?
155+
flag = cli.StringFlag{
156+
Name: "layout",
157+
Usage: "OCI image URI of the form 'path'",
158+
}
159+
160+
// Add BeforeFunc that will verify code.
161+
cmd.Before = func(ctx *cli.Context) error {
162+
dir := ctx.String("layout")
163+
// Verify directory value.
164+
if strings.Contains(dir, ":") {
165+
return fmt.Errorf("invalid --layout: directory contains ':' character: '%s'", dir)
166+
}
167+
if dir == "" {
168+
return fmt.Errorf("invalid --layout: directory is empty")
169+
}
170+
171+
ctx.App.Metadata["layout"] = dir
172+
173+
if oldBefore != nil {
174+
return oldBefore(ctx)
175+
}
176+
return nil
177+
}
178+
default:
179+
// This is a programming error. All umoci commands should fall into
180+
// one of the above categories.
181+
panic("Unknown command category: " + cmd.Category)
182+
}
183+
184+
cmd.Flags = append([]cli.Flag{flag}, cmd.Flags...)
185+
app.Commands[idx] = cmd
186+
}
187+
86188
// Actually run umoci.
87189
if err := app.Run(os.Args); err != nil {
88190
logrus.Fatal(err)

0 commit comments

Comments
 (0)