Deprecate GSC_PAL and use instead GRAMINE_MODE

[dimakuv]

Description of the changes

Previously, to run gramine-direct, one had to specify docker run ... --env GSC_PAL=Linux. This was cumbersome because (1) Gramine users don't need to know the meaning of word "PAL", (2) the value "Linux" doesn't correspond to known-to-users gramine-direct, (3) it requires special logic in apploader code.

This PR introduces instead GSC_GRAMINE_BINARY GRAMINE_BINARY GRAMINE_MODE envvar with easier semantics: the value is the mode (direct or sgx) which user wants to invoke.

Extracted from #199.

How to test this PR?

Manually run e.g. the HelloWorld example. After GSC build & sign, you can try:

$ docker run \
  --security-opt seccomp=unconfined \
  --env GRAMINE_MODE=direct \
  gsc-ubuntu20.04-hello-world
"Hello World! Let's check escaped symbols: < & > "

sdp@sdp:~/tdx/gsc$ docker run \
  --device=/dev/sgx_enclave -v /var/run/aesmd/aesm.socket:/var/run/aesmd/aesm.socket \
  --env GRAMINE_MODE=sgx \
  gsc-ubuntu20.04-hello-world
Gramine is starting. Parsing TOML manifest file, this may take some time...
"Hello World! Let's check escaped symbols: < & > "

sdp@sdp:~/tdx/gsc$ docker run \
  --device=/dev/sgx_enclave \
  -v /var/run/aesmd/aesm.socket:/var/run/aesmd/aesm.socket \
  gsc-ubuntu20.04-hello-world
Gramine is starting. Parsing TOML manifest file, this may take some time...
"Hello World! Let's check escaped symbols: < & > "

sdp@sdp:~/tdx/gsc$ docker run \
  --env GRAMINE_MODE=bla \
  gsc-ubuntu20.04-hello-world
/gramine/app_files/apploader.sh: line 16: exec: gramine-bla: not found

---

This change is 

[jkr0103]

Reviewable status: 0 of 2 files reviewed, 1 unresolved discussion, 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/index.rst line 408 at r1 (raw file):

command-line option to :command:`docker run`.

.. envvar:: GSC_GRAMINE_BINARY

We don't have a different Gramine binary for GSC, it'll still be a gramine binary, hence I would suggest keeping GRAMINE_BINARY instead of GSC_GRAMINE_BINARY envvar.

[dimakuv]

Reviewable status: 0 of 2 files reviewed, all discussions resolved, not enough approvals from maintainers (2 more required), not enough approvals from different teams (1 more required, approved so far: Intel)

---

Documentation/index.rst line 408 at r1 (raw file):

Previously, jkr0103 (Jitender Kumar) wrote…

We don't have a different Gramine binary for GSC, it'll still be a gramine binary, hence I would suggest keeping GRAMINE_BINARY instead of GSC_GRAMINE_BINARY envvar.

Done

[mkow]

Reviewable status: 0 of 2 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 @dimakuv)

a discussion (no related file):

(1) Gramine users don't need to know the meaning of word "PAL"
(2) the value "Linux" doesn't correspond to known-to-users gramine-direct
(3) it requires special logic in apploader code.

I think I agree with these, but I'm not sure about the new name, the thing you're supposed to pass there is actually not a binary and you aren't supposed to provide a full path. Maybe GRAMINE_BACKEND or GRAMINE_MODE? And then just pass direct or sgx.

---

[dimakuv]

Reviewable status: 0 of 2 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 @mkow)

a discussion (no related file):

Previously, mkow (Michał Kowalczyk) wrote…

(1) Gramine users don't need to know the meaning of word "PAL"
(2) the value "Linux" doesn't correspond to known-to-users gramine-direct
(3) it requires special logic in apploader code.

I think I agree with these, but I'm not sure about the new name, the thing you're supposed to pass there is actually not a binary and you aren't supposed to provide a full path. Maybe GRAMINE_BACKEND or GRAMINE_MODE? And then just pass direct or sgx.

Done, changed to GRAMINE_MODE

---

[mkow]

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

---

Documentation/index.rst line 436 at r3 (raw file):

This is deprecated in GSC v1.8

Didn't you remove it completely in this PR?

---

templates/apploader.common.template line 12 at r3 (raw file):

# Note: default to SGX if Gramine mode (`direct`, `sgx`) wasn't specified
exec gramine-${GRAMINE_MODE:-sgx} /gramine/app_files/entrypoint \

I don't like it, passing wrong value in this env will cause a weird failure (instead of a user-friendly error).

Code quote:

gramine-${GRAMINE_MODE:-sgx}

[dimakuv]

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

---

Documentation/index.rst line 436 at r3 (raw file):

Previously, mkow (Michał Kowalczyk) wrote…

This is deprecated in GSC v1.8

Didn't you remove it completely in this PR?

Oops, true :) Should I keep GSC_PAL logic in apploader.common.template? Or it's so infrequently used that I can just remove it completely?

Well, it's in the documentation, so I guess I should keep that logic but add a warning.

---

templates/apploader.common.template line 12 at r3 (raw file):

Previously, mkow (Michał Kowalczyk) wrote…

I don't like it, passing wrong value in this env will cause a weird failure (instead of a user-friendly error).

The failure message is not that weird:

/gramine/app_files/apploader.sh: line 16: exec: gramine-bla: not found

You still don't like it and would like to have a proper if-then-else check in this bash script?

[mkow]

Reviewable status: all files reviewed, 2 unresolved discussions, not enough approvals from maintainers (1 more required), not enough approvals from different teams (1 more required, approved so far: Intel) (waiting on @dimakuv)

---

Documentation/index.rst line 436 at r3 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Oops, true :) Should I keep GSC_PAL logic in apploader.common.template? Or it's so infrequently used that I can just remove it completely?

Well, it's in the documentation, so I guess I should keep that logic but add a warning.

There should at least be a warning (but ideally keep it working + a warning for v1.8). Right now it will just run the SGX version and silently ignore user-provided GSC_PAL=Linux.

---

templates/apploader.common.template line 12 at r3 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

The failure message is not that weird:

/gramine/app_files/apploader.sh: line 16: exec: gramine-bla: not found

You still don't like it and would like to have a proper if-then-else check in this bash script?

Yeah, I think I'd prefer a more explicit error pointing to the env variable which was set incorrectly. It's always nicer for the users, this mistake is rather easy to make and it's just one simple if in Bash to improve it.

[dimakuv]

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

---

Documentation/index.rst line 436 at r3 (raw file):

Previously, mkow (Michał Kowalczyk) wrote…

There should at least be a warning (but ideally keep it working + a warning for v1.8). Right now it will just run the SGX version and silently ignore user-provided GSC_PAL=Linux.

Done.

---

templates/apploader.common.template line 12 at r3 (raw file):

Previously, mkow (Michał Kowalczyk) wrote…

Yeah, I think I'd prefer a more explicit error pointing to the env variable which was set incorrectly. It's always nicer for the users, this mistake is rather easy to make and it's just one simple if in Bash to improve it.

Done.

[mkow]

Reviewed 1 of 1 files at r4, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion, "fixup! " found in commit messages' one-liners (waiting on @dimakuv)

---

templates/apploader.common.template line 22 at r4 (raw file):

if [ -n "$GSC_PAL" ]; then
    echo "WARNING: GSC_PAL environment variable is deprecated in v1.8 and will be removed in v1.9."
    echo "         Use instead the GRAMINE_MODE={direct|sgx} environment variable."

Suggestion:

Instead, use GRAMINE_MODE={direct|sgx}.

[dimakuv]

Reviewable status: 1 of 2 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 @mkow)

---

templates/apploader.common.template line 22 at r4 (raw file):

if [ -n "$GSC_PAL" ]; then
    echo "WARNING: GSC_PAL environment variable is deprecated in v1.8 and will be removed in v1.9."
    echo "         Use instead the GRAMINE_MODE={direct|sgx} environment variable."

Done.

[mkow]

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