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

[Docs] Change "GSC" page to "Container integration" #749

Merged
merged 1 commit into from
Aug 4, 2022

Conversation

dimakuv
Copy link
Contributor

@dimakuv dimakuv commented Jul 12, 2022

Description of the changes

We now have not only GSC, but also the Gramine Docker image hosted on DockerHub. Update our documentation to mention both tools and prepare for more tools/workflows in the future. Also, GSC was split from the core repository a long time ago and there is no need to emphasize that, so we rephrase its description.

How to test this PR?

N/A.


This change is Reviewable

Copy link
Contributor

@boryspoplawski boryspoplawski left a comment

Choose a reason for hiding this comment

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

Reviewed 3 of 3 files at r1, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (1 more required, approved so far: Intel) (waiting on @dimakuv)


Documentation/container-integration.rst line 20 at r1 (raw file):

applications under Gramine. The only currently available Gramine image is based
on Ubuntu 20.04. The only requirement on the host system is a Linux kernel
version 5.11 or higher (with the in-kernel SGX driver).

I would remove parens, this is actually the most important part - if somebody had in-kernel SGX driver in 5.0 (e.g. manual port) that would work too.
Or even rephrase to: The only requirement on the host system is a Linux kernel with in-kernel SGX driver (available from version 5.11 onward).


Documentation/container-integration.rst line 30 at r1 (raw file):

    docker run --device /dev/sgx_enclave \
        --security-opt seccomp=unconfined \

Recommended way is to download our seccomp profile and use it.
And if users do not intend to use Linux PAL (non SGX), then they can just use default docker seccomp profile.


Documentation/container-integration.rst line 31 at r1 (raw file):

    docker run --device /dev/sgx_enclave \
        --security-opt seccomp=unconfined \
        -it --entrypoint /bin/bash gramineproject/gramine

You can remove --entrypoint /bin/bash it's default in this image.

Copy link
Member

@mkow mkow left a comment

Choose a reason for hiding this comment

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

Reviewed 3 of 3 files at r1, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (1 more required, approved so far: Intel) (waiting on @dimakuv)


-- commits line 4 at r1:

Suggestion:

a Gramine Docker image

@dimakuv dimakuv force-pushed the dimakuv/docs-gsc-to-container-integration branch from 82244dd to ff1f2c2 Compare July 12, 2022 11:56
Copy link
Contributor Author

@dimakuv dimakuv left a comment

Choose a reason for hiding this comment

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

Reviewable status: 2 of 3 files reviewed, 5 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (1 more required, approved so far: Intel) (waiting on @boryspoplawski and @mkow)

a discussion (no related file):
Let me block this PR until we have a proper Gramine Docker image available.



-- commits line 4 at r1:
Done.


Documentation/container-integration.rst line 20 at r1 (raw file):

Previously, boryspoplawski (Borys Popławski) wrote…

I would remove parens, this is actually the most important part - if somebody had in-kernel SGX driver in 5.0 (e.g. manual port) that would work too.
Or even rephrase to: The only requirement on the host system is a Linux kernel with in-kernel SGX driver (available from version 5.11 onward).

Done.


Documentation/container-integration.rst line 30 at r1 (raw file):

Previously, boryspoplawski (Borys Popławski) wrote…

Recommended way is to download our seccomp profile and use it.
And if users do not intend to use Linux PAL (non SGX), then they can just use default docker seccomp profile.

Done. Not sure if giving a link is the best way here.


Documentation/container-integration.rst line 31 at r1 (raw file):

Previously, boryspoplawski (Borys Popławski) wrote…

You can remove --entrypoint /bin/bash it's default in this image.

Done.

Copy link
Contributor

@boryspoplawski boryspoplawski left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (1 more required, approved so far: Intel) (waiting on @dimakuv and @mkow)


Documentation/container-integration.rst line 35 at r2 (raw file):

See
https://gramine.readthedocs.io/projects/gsc/en/latest/#execute-with-linux-pal-instead-of-linux-sgx-pal
for details.

This is link to GSC which talks about some GSC specific details. I would just put:

If you want to run :program:`gramine-direct` in addition to command:`gramine-sgx`,
then you should run Docker with a our custom seccomp profile using `--security-opt seccomp=<profile_file>`.
You can download the profile file from https://github.com/gramineproject/gramine/blob/master/scripts/docker_seccomp.json.
Alternatively you can disable seccomp completely (`--security-opt seccomp=unconfined`).

Copy link
Contributor Author

@dimakuv dimakuv left a comment

Choose a reason for hiding this comment

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

Reviewable status: 2 of 3 files reviewed, 4 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (1 more required, approved so far: Intel), "fixup! " found in commit messages' one-liners (waiting on @boryspoplawski, @dimakuv, and @mkow)


Documentation/container-integration.rst line 35 at r2 (raw file):

Previously, boryspoplawski (Borys Popławski) wrote…

This is link to GSC which talks about some GSC specific details. I would just put:

If you want to run :program:`gramine-direct` in addition to command:`gramine-sgx`,
then you should run Docker with a our custom seccomp profile using `--security-opt seccomp=<profile_file>`.
You can download the profile file from https://github.com/gramineproject/gramine/blob/master/scripts/docker_seccomp.json.
Alternatively you can disable seccomp completely (`--security-opt seccomp=unconfined`).

Done

Copy link
Contributor

@boryspoplawski boryspoplawski left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r3, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (1 more required, approved so far: Intel), "fixup! " found in commit messages' one-liners (waiting on @dimakuv and @mkow)

mkow
mkow previously approved these changes Jul 12, 2022
Copy link
Member

@mkow mkow left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r3, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion, not enough approvals from maintainers (1 more required), "fixup! " found in commit messages' one-liners (waiting on @dimakuv)

We now have not only GSC, but also a Gramine Docker image hosted on
DockerHub. Update our documentation to mention both tools and prepare
for more tools/workflows in the future. Also, GSC was split from the
core repository a long time ago and there is no need to emphasize that,
so we rephrase its description.

Signed-off-by: Dmitrii Kuvaiskii <dmitrii.kuvaiskii@intel.com>
@dimakuv dimakuv force-pushed the dimakuv/docs-gsc-to-container-integration branch from ec5b143 to 57eaa9d Compare August 4, 2022 07:49
Copy link
Contributor Author

@dimakuv dimakuv left a comment

Choose a reason for hiding this comment

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

Reviewable status: 2 of 3 files reviewed, 1 unresolved discussion, not enough approvals from maintainers (1 more required), not enough approvals from different teams (1 more required, approved so far: Intel) (waiting on @boryspoplawski and @mkow)

a discussion (no related file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Let me block this PR until we have a proper Gramine Docker image available.

And the proper Gramine Docker image is publicly available: https://hub.docker.com/r/gramineproject/gramine. Rebased, ready for merge.


Copy link
Contributor

@boryspoplawski boryspoplawski left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r4, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

@dimakuv dimakuv merged commit 57eaa9d into master Aug 4, 2022
@dimakuv dimakuv deleted the dimakuv/docs-gsc-to-container-integration branch August 4, 2022 12:04
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.

3 participants