Add gramine-manifest to GSC build

[DukeDavis12]

Description of the changes

Fixes #249, #251.

How to test this PR?

---

This change is 

[woju]

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

---

a discussion (no related file):
Directories are problematic far beyond this, from cursory look you'll need to account for symlinks, you didn't check for / as the last character and possibly other problems.

We already solved those in gramine-manifest. I think you need to either reuse that tool, or if you can't, you'll need to require that people have installed gramine and then you can import gramine.manifest and reuse gramine.manifest.Manifest class or gramine.manifest.TrustedFile. Lack of code reuse in this area is becoming untenable.

Otherwise you'll be condemned to repeat all our previous mistakes in this area yourself.

[DukeDavis12]

@woju

you'll need to require that people have installed gramine and then you can import gramine.manifest and reuse gramine.manifest.Manifest class or gramine.manifest.TrustedFile. Lack of code reuse in this area is becoming untenable.

Thanks for the suggestion. Reusing gramine-manifest tool.

Can you review the changes ?

[woju]

Reviewed 2 of 2 files at r2, 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 (2 more required, approved so far: ), one-liner is not in imperative mood

---

-- commits line 14 at r2:
(n/b) All of this history probably needs to be squashed and reworded, possibly at merge.

---

templates/Dockerfile.common.build.template line 66 at r2 (raw file):

RUN {% block path %}{% endblock %} \
    && gramine-manifest /gramine/app_files/entrypoint.manifest \
    /gramine/app_files/entrypoint.manifest \

This depends on an edge behaviour of click that the file is opened for writing on first write, not when main() is called. If not for it, having same file for input and output would truncate(2) it before it is even read, resulting in empty file and likely parse error. Click has it documented, but I think it's quirky: https://click.palletsprojects.com/en/stable/handling-files/#file-opening-behaviors.

@mkow do we want to leave it like that? It works, it's officially documented and it's reasonably robust (I think it won't ever break), but it is surprising. Alternatives: 1) we can have separate file as the manifest, 2) we can comment (here and/or in main repo, python/gramine-manifest to uphold an invariant that we need to fully read manifest before calling manifest.dump(file)).

---

templates/Dockerfile.common.build.template line 67 at r2 (raw file):

    && gramine-manifest /gramine/app_files/entrypoint.manifest \
    /gramine/app_files/entrypoint.manifest \
    && gramine-manifest-check /gramine/app_files/entrypoint.manifest

If calling gramine-manifest directly, you can remove gramine-manifest-check, because the check is called internally. Alternatively you can add --no-check to gramine-manifest and leave this explicit call, if you want it for some reason.

[DukeDavis12]

Reviewable status: 1 of 2 files reviewed, 3 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (2 more required, approved so far: ), one-liner is not in imperative mood (waiting on @woju)

---

a discussion (no related file):

Previously, woju (Wojtek Porczyk) wrote…

Directories are problematic far beyond this, from cursory look you'll need to account for symlinks, you didn't check for / as the last character and possibly other problems.

We already solved those in gramine-manifest. I think you need to either reuse that tool, or if you can't, you'll need to require that people have installed gramine and then you can import gramine.manifest and reuse gramine.manifest.Manifest class or gramine.manifest.TrustedFile. Lack of code reuse in this area is becoming untenable.

Otherwise you'll be condemned to repeat all our previous mistakes in this area yourself.

Using gramine-manifest tool now instead of handling it in code.

---

-- commits line 14 at r2:

Previously, woju (Wojtek Porczyk) wrote…

(n/b) All of this history probably needs to be squashed and reworded, possibly at merge.

Will squash once I get approval from your side.

---

templates/Dockerfile.common.build.template line 67 at r2 (raw file):

Previously, woju (Wojtek Porczyk) wrote…

If calling gramine-manifest directly, you can remove gramine-manifest-check, because the check is called internally. Alternatively you can add --no-check to gramine-manifest and leave this explicit call, if you want it for some reason.

Removed gramine-manifest-check.

[DukeDavis12]

Reviewable status: 1 of 2 files reviewed, 3 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (2 more required, approved so far: ), one-liner is not in imperative mood (waiting on @mkow and @woju)

---

templates/Dockerfile.common.build.template line 66 at r2 (raw file):

Previously, woju (Wojtek Porczyk) wrote…

This depends on an edge behaviour of click that the file is opened for writing on first write, not when main() is called. If not for it, having same file for input and output would truncate(2) it before it is even read, resulting in empty file and likely parse error. Click has it documented, but I think it's quirky: https://click.palletsprojects.com/en/stable/handling-files/#file-opening-behaviors.

@mkow do we want to leave it like that? It works, it's officially documented and it's reasonably robust (I think it won't ever break), but it is surprising. Alternatives: 1) we can have separate file as the manifest, 2) we can comment (here and/or in main repo, python/gramine-manifest to uphold an invariant that we need to fully read manifest before calling manifest.dump(file)).

@mkow What's your take ?

[mkow]

Reviewed 1 of 1 files at r3.
Reviewable status: all files reviewed, 3 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (2 more required, approved so far: ), one-liner is not in imperative mood (waiting on @woju)

---

templates/Dockerfile.common.build.template line 66 at r2 (raw file):

Previously, DukeDavis12 (Davis Benny) wrote…

@mkow What's your take ?

We can keep it, but we'd need to document that on the python/gramine-manifest side + add a test there. It's IMO not obvious otherwise that it works.

[DukeDavis12]

@woju @mkow Please review.

[mkow]

Reviewed 1 of 2 files at r2, 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 (2 more required, approved so far: ), one-liner is not in imperative mood (waiting on @woju)

[kailun-qin]

@kailun-qin made 1 comment.
Reviewable status: all files reviewed, 3 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (2 more required, approved so far: ), one-liner is not in imperative mood (waiting on woju).

---

a discussion (no related file):
@DukeDavis12: Same question here, Davis -- do you plan to continue working on this PR? Otherwise, I can take care of it.

[DukeDavis12]

@kailun-qin made 1 comment.
Reviewable status: all files reviewed, 3 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (2 more required, approved so far: ), one-liner is not in imperative mood (waiting on woju).

a discussion (no related file): @DukeDavis12: Same question here, Davis -- do you plan to continue working on this PR? Otherwise, I can take care of it.

I am working on a different Project now. @kailun-qin Can you take care of it ?

[kailun-qin]

@kailun-qin reviewed 5 files and all commit messages, and made 3 comments.
Reviewable status: all files reviewed, 3 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (2 more required, approved so far: ) (waiting on mkow and woju).

---

a discussion (no related file):

Previously, DukeDavis12 (Davis Benny) wrote…

I am working on a different Project now. @kailun-qin Can you take care of it ?

Thanks! Updated.

---

-- commits line 14 at r2:

Previously, DukeDavis12 (Davis Benny) wrote…

Will squash once I get approval from your side.

Done.

---

templates/Dockerfile.common.build.template line 66 at r2 (raw file):
Done.

but we'd need to document that on the python/gramine-manifest side + add a test there

I'll submit a PR to the core Gramine repo later. For now, I'm adding a note here as suggested.