Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions cmd/ocitools/generate.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ var generateCommand = cli.Command{
}
}

err := setupSpec(specgen, context)
err := setupSpec(&specgen, context)
if err != nil {
return err
}
Expand All @@ -96,8 +96,12 @@ var generateCommand = cli.Command{
},
}

func setupSpec(g generate.Generator, context *cli.Context) error {
spec := g.GetSpec()
func setupSpec(g *generate.Generator, context *cli.Context) error {
if context.GlobalBool("host-specific") {
g.HostSpecific = true
}

spec := g.Spec()

if len(spec.Version) == 0 {
g.SetVersion(rspec.Version)
Expand Down Expand Up @@ -369,7 +373,7 @@ func checkNs(nsMaps map[string]string, nsName string) bool {
return true
}

func setupLinuxNamespaces(g generate.Generator, needsNewUser bool, nsMaps map[string]string) {
func setupLinuxNamespaces(g *generate.Generator, needsNewUser bool, nsMaps map[string]string) {
for _, nsName := range generate.Namespaces {
if !checkNs(nsMaps, nsName) && !(needsNewUser && nsName == "user") {
continue
Expand Down
4 changes: 4 additions & 0 deletions cmd/ocitools/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@ func main() {
Value: "error",
Usage: "Log level (panic, fatal, error, warn, info, or debug)",
},
cli.BoolFlag{
Name: "host-specific",
Usage: "generate host-specific configs or do host-specific validations",
},
}

app.Commands = []cli.Command{
Expand Down
3 changes: 1 addition & 2 deletions cmd/ocitools/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ type configCheck func(rspec.Spec, string, bool) []string

var bundleValidateFlags = []cli.Flag{
cli.StringFlag{Name: "path", Value: ".", Usage: "path to a bundle"},
cli.BoolFlag{Name: "host-specific", Usage: "Check host specific configs."},
}

var (
Expand Down Expand Up @@ -99,7 +98,7 @@ var bundleValidateCommand = cli.Command{
return fmt.Errorf("The root path %q is not a directory.", rootfsPath)
}

hostCheck := context.Bool("host-specific")
hostCheck := context.GlobalBool("host-specific")

checks := []configCheck{
checkMandatoryFields,
Expand Down
31 changes: 22 additions & 9 deletions generate/generate.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ var (

// Generator represents a generator for a container spec.
type Generator struct {
spec *rspec.Spec
spec *rspec.Spec
HostSpecific bool
}

// New creates a spec Generator with the default spec.
Expand Down Expand Up @@ -139,12 +140,16 @@ func New() Generator {
Devices: []rspec.Device{},
},
}
return Generator{&spec}
return Generator{
spec: &spec,
}
}

// NewFromSpec creates a spec Generator from a given spec.
func NewFromSpec(spec *rspec.Spec) Generator {
return Generator{spec}
return Generator{
spec: spec,
}
}

// NewFromFile loads the template specifed in a file into a spec Generator.
Expand All @@ -166,16 +171,18 @@ func NewFromTemplate(r io.Reader) (Generator, error) {
if err := json.NewDecoder(r).Decode(&spec); err != nil {
return Generator{}, err
}
return Generator{&spec}, nil
return Generator{
spec: &spec,
}, nil
}

// SetSpec sets the spec in the Generator g.
func (g *Generator) SetSpec(spec *rspec.Spec) {
g.spec = spec
}

// GetSpec gets the spec in the Generator g.
func (g *Generator) GetSpec() *rspec.Spec {
// Spec gets the spec in the Generator g.
func (g *Generator) Spec() *rspec.Spec {
return g.spec
}

Expand Down Expand Up @@ -953,6 +960,9 @@ func (g *Generator) SetupPrivileged(privileged bool) {
// Add all capabilities in privileged mode.
var finalCapList []string
for _, cap := range capability.List() {
if g.HostSpecific && cap > capability.CAP_LAST_CAP {
continue
}
finalCapList = append(finalCapList, fmt.Sprintf("CAP_%s", strings.ToUpper(cap.String())))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This section should probably be using AddProcessCapability and passing hostSpecific through too that function. AddProcessCapability (or maybe checkCap) should actually be doing the conditional CAP_LAST_CAP comparison.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wking , Thanks for pointing out.
I would rather keep this function as it is for code efficiency.
I will fix AddProcessCapability and other related functions.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On Thu, Jul 21, 2016 at 03:03:04PM -0700, hmeng-19 wrote:

I would rather keep this function as it is for code efficiency.

Are we really worried about performance for compiling a capability
list? Can you spell out your concern in more detail or give a dummy
benchmark or something?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could be addressed in a different PR.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On Thu, Jul 21, 2016 at 03:39:31PM -0700, Mrunal Patel wrote:

    // Add all capabilities in privileged mode.
    var finalCapList []string
    for _, cap := range capability.List() {
  •       if hostSpecific && cap > capability.CAP_LAST_CAP {
    
  •           continue
    
  •       }
        finalCapList = append(finalCapList, fmt.Sprintf("CAP_%s", strings.ToUpper(cap.String())))
    

This could be addressed in a different PR.

Using AddProcessCapability here? I guess, if we can't decide in this
one. I'm happy to file that PR if you want to land this without
addressing that point (although I still think we want to handle 1
here).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wking Yes, you can open another PR to use AddProcessCapability here.

}
g.initSpecLinux()
Expand All @@ -963,12 +973,15 @@ func (g *Generator) SetupPrivileged(privileged bool) {
}
}

func checkCap(c string) error {
func checkCap(c string, hostSpecific bool) error {
isValid := false
cp := strings.ToUpper(c)

for _, cap := range capability.List() {
if cp == strings.ToUpper(cap.String()) {
if hostSpecific && cap > capability.CAP_LAST_CAP {
return fmt.Errorf("CAP_%s is not supported on the current host", cp)
}
isValid = true
break
}
Expand All @@ -990,7 +1003,7 @@ func (g *Generator) ClearProcessCapabilities() {

// AddProcessCapability adds a process capability into g.spec.Process.Capabilities.
func (g *Generator) AddProcessCapability(c string) error {
if err := checkCap(c); err != nil {
if err := checkCap(c, g.HostSpecific); err != nil {
return err
}

Expand All @@ -1009,7 +1022,7 @@ func (g *Generator) AddProcessCapability(c string) error {

// DropProcessCapability drops a process capability from g.spec.Process.Capabilities.
func (g *Generator) DropProcessCapability(c string) error {
if err := checkCap(c); err != nil {
if err := checkCap(c, g.HostSpecific); err != nil {
return err
}

Expand Down
10 changes: 0 additions & 10 deletions man/ocitools-validate.1.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,16 +18,6 @@ Validate an OCI bundle
**--path=PATH
Path to bundle

**--host-specific**
Check host specific configs.
By default, validation only tests for compatibility with a hypothetical host.
With this flag, validation will also run more specific tests to see whether
the current host is capable of launching a container from the configuration.
For example, validating a compliant Windows configuration on a Linux machine
will pass without this flag ("there may be a Windows host capable of
launching this container"), but will fail with it ("this host is not capable
of launching this container").

# SEE ALSO
**ocitools**(1)

Expand Down
17 changes: 16 additions & 1 deletion man/ocitools.1.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,26 @@ ocitools is a collection of tools for working with the [OCI runtime specificatio

# OPTIONS
**--help**
Print usage statement
Print usage statement.

**-v**, **--version**
Print version information.

**--log-level**
Log level (panic, fatal, error, warn, info, or debug) (default: "error").

**--host-specific**
Generate host-specific configs or do host-specific validations.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This section (including the following paragraph) needs to be genericized now that it's covering both generate and validate.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment has been addressed.


By default, generator generates configs without checking whether they are
supported on the current host. With this flag, generator will first check
whether each config is supported on the current host, and only add it into
the config file if it passes the checking.

By default, validation only tests for compatibility with a hypothetical host.
With this flag, validation will also run more specific tests to see whether
the current host is capable of launching a container from the configuration.

# COMMANDS
**validate**
Validating OCI bundle
Expand Down