-
Notifications
You must be signed in to change notification settings - Fork 32
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
provide cli containerfile #329
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
FROM python:3.12-slim | ||
|
||
COPY pyproject.toml ./ | ||
COPY src ./src | ||
|
||
RUN pip install typing-extensions sigstore-protobuf-specs protobuf in-toto-attestation cryptography certifi pyOpenSSL sigstore | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there a better way to resolve the python dependencies? I've not much exp. with python and didn't see a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The sub-dependencies are defined here: https://github.com/sigstore/model-transparency/blob/main/pyproject.toml#L29-L34 It might be better to just install https://pypi.org/project/model-signing/ instead? Or does this need to always be built from the source repo? Running There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. +1 to just installing the library. Alternatively, since we use I was actually thinking of making There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, seems like just installing the library is not going to be enough: #330.
I think it's probably fine to require There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sounds good. I think we didn't add the CLI scripts to the library, but we'll do once we rewrite them to use the higher level API. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Hi @di, I tried this. But it fails. So I changed it to
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is interesting. I'll try to debug this on Monday. |
||
|
||
RUN echo '#!/bin/bash\n\ | ||
cd "/src" && python sign.py' > /usr/local/bin/sign | ||
|
||
RUN echo '#!/bin/bash\n\ | ||
cd "/src" && python verify.py' > /usr/local/bin/verify | ||
|
||
RUN echo '#!/bin/bash\n\ | ||
echo "Usage:"\n\ | ||
echo " verify - Runs the verify.py Python script"\n\ | ||
echo " sign - Runs the sign.py Python script"\n\ | ||
echo " help - Displays this help message"' > /usr/local/bin/help | ||
|
||
RUN chmod +x /usr/local/bin/sign /usr/local/bin/verify /usr/local/bin/help | ||
|
||
CMD ["help"] | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should it be named
Dockerfile
?Also, can you add the standard license header please? (the one with sigstore authors, see https://github.com/sigstore/model-transparency/blob/main/src/model_signing/__init__.py)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @mihaimaruseac, I added the license header. 😃
Regarding the file, I named it
Containerfile
overDockerfile
since its agnostic to container engines like Podman, Buildah (got recently donated to CNCF) or Docker. Podman/Buildah seem to pick Containerfile over Dockerfile by default. We could still change it if you think it makes more sense - wdyt? :)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the context! This makes a lot of sense (and TIL!)