Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add bundle validate as sub-command #20

Merged
merged 5 commits into from
Mar 8, 2016
Merged

add bundle validate as sub-command #20

merged 5 commits into from
Mar 8, 2016

Conversation

liangchenye
Copy link
Member

Signed-off-by: liangchenye liangchenye@huawei.com

Clean the worthless commits in (#4) and re-submit it.

Signed-off-by: liangchenye <liangchenye@huawei.com>
@mrunalp
Copy link
Contributor

mrunalp commented Feb 8, 2016

Thanks for the PR. I am on vacation and will be slow in responding till next week.

Signed-off-by: liangchenye <liangchenye@huawei.com>
Signed-off-by: liangchenye <liangchenye@huawei.com>
@liangchenye
Copy link
Member Author

@mrunalp bundle validate is updated to 0.3.0
I also update generate.go to 0.3.0 by commit '0fbe252'.

@mrunalp
Copy link
Contributor

mrunalp commented Feb 23, 2016

Build failure as you need to bump up the spec to 0.3.0 in Godeps. Thanks!

Signed-off-by: liangchenye <liangchenye@huawei.com>
@liangchenye
Copy link
Member Author

Godeps updated.

The config.json generated by ocitools could not pass the bundle validation, since three security fields are not noted as 'optional', I submit a PR here (opencontainers/runtime-spec#325)

@liangchenye
Copy link
Member Author

Opps, the codes under cmd directory should also need to be changed.

Signed-off-by: liangchenye <liangchenye@huawei.com>
@liangchenye
Copy link
Member Author

cmd updated.

@mrunalp
Copy link
Contributor

mrunalp commented Mar 8, 2016

Can you update the deps to pickup the change where selinuxlabel and apparmor got moved to proces?

@mrunalp
Copy link
Contributor

mrunalp commented Mar 8, 2016

Also, we need to make seccomp optional in the spec. Otherwise this seems to be working well.

@mrunalp
Copy link
Contributor

mrunalp commented Mar 8, 2016

I have created PR for seccomp opencontainers/runtime-spec#333

@mrunalp
Copy link
Contributor

mrunalp commented Mar 8, 2016

I think we can merge this for now and fix up things in follow up PRs. Thanks! LGTM

mrunalp pushed a commit that referenced this pull request Mar 8, 2016
add bundle validate as sub-command
@mrunalp mrunalp merged commit 07b481e into opencontainers:master Mar 8, 2016
@liangchenye
Copy link
Member Author

Yes, I'll update all of the selinuxlabel/apparmor/seccomp Godeps once seccomp PR checked in.
Thanks

wking added a commit to wking/ocitools-v2 that referenced this pull request May 20, 2016
These landed as CheckMounts in 647e355 (bundle validate update to
0.3.0, 2016-02-23, opencontainers#20), but both checks are too strict.

The first (destination exists in the rootfs) errors on valid cases
like:

  "mounts": [
    {
      "source": "users",
      "destination": "/home",
      "type": "bind"
    },
    {
      "source": "none",
      "destination": "/home/wking",
      "type": "tmpfs"
    }
  ]

Where the source 'users' directory already contained a 'wking'
subdirectory.  So by the time the tmpfs was setup, the destination
directory would exist, but at validation time (without having run the
bind mount) the tmpfs destination directory would not exist.

The second (destination is a directory) errors on valid cases like:

  "mounts": [
    {
      "source": "/etc/resolv.conf",
      "destination": "/etc/resolv.conf",
      "type": "bind"
    }
  ]

because binding files to files works.  In a shell:

  # touch test
  # mount --bind /etc/resolv.conf test
  # umount test

However binding directories to files does not work:

  # mount --bind /etc test
  mount: mount point /tmp/test is not a directory

Figuring out which mount configurations are valid and which aren't may
be possible, but I'm pretty sure it's more trouble than we want to get
into.  There may be room for other mount tests (e.g. comparing 'type'
against /proc/filesystems as a host-specific test), but I'm leaving
those to subsequent pull requests.

Signed-off-by: W. Trevor King <wking@tremily.us>
This was referenced May 20, 2016
mrunalp pushed a commit that referenced this pull request May 23, 2016
These landed as CheckMounts in 647e355 (bundle validate update to
0.3.0, 2016-02-23, #20), but both checks are too strict.

The first (destination exists in the rootfs) errors on valid cases
like:

  "mounts": [
    {
      "source": "users",
      "destination": "/home",
      "type": "bind"
    },
    {
      "source": "none",
      "destination": "/home/wking",
      "type": "tmpfs"
    }
  ]

Where the source 'users' directory already contained a 'wking'
subdirectory.  So by the time the tmpfs was setup, the destination
directory would exist, but at validation time (without having run the
bind mount) the tmpfs destination directory would not exist.

The second (destination is a directory) errors on valid cases like:

  "mounts": [
    {
      "source": "/etc/resolv.conf",
      "destination": "/etc/resolv.conf",
      "type": "bind"
    }
  ]

because binding files to files works.  In a shell:

  # touch test
  # mount --bind /etc/resolv.conf test
  # umount test

However binding directories to files does not work:

  # mount --bind /etc test
  mount: mount point /tmp/test is not a directory

Figuring out which mount configurations are valid and which aren't may
be possible, but I'm pretty sure it's more trouble than we want to get
into.  There may be room for other mount tests (e.g. comparing 'type'
against /proc/filesystems as a host-specific test), but I'm leaving
those to subsequent pull requests.

Signed-off-by: W. Trevor King <wking@tremily.us>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants