-
Notifications
You must be signed in to change notification settings - Fork 606
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 cosign in compose #1508
Add cosign in compose #1508
Conversation
cc @AkihiroSuda @developer-guy @Dentrax @ktock. I think the functionality should work now, but thanks for any comments&suggestions as I don't have much experience with cosign :) Also any suggestion on how to check Thanks! (I'll update related docs after the implementation looks okay :)) |
Thanks!
Yes, should be added in Options |
// 4. compose run | ||
const sttyPartialOutput = "speed 38400 baud" | ||
// unbuffer(1) emulates tty, which is required by `nerdctl run -t`. | ||
// unbuffer(1) can be installed with `apt-get install expect`. |
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.
Any reason to check the TTY functionality here?
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.
I followed compose run
testcases in the same folder. Seems they all use this scenario. Should I change to something else? thanks.
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.
Just "echo hi" should suffice, unless the cosign stuff relates to tty
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.
I tested echo hi
but seems have some issue in test(below, maybe due to some unimplemented feature in compose run
as mentioned in #1340).
If it's okay I'll keep this test consistent with other compose run
testcases (i.e., use unbuffer
) in this file.
time="2022-11-17T01:49:58Z" level=fatal msg="error while creating container nerdctl-compose-test2565186661_svc0_run_6af98b73d3b0: exit status 2"
069aa92
to
2a47031
Compare
If people are going to try that feature in lima VM, they will fail as we did not add cosign binary into the VM image yet 👇 |
Signed-off-by: Jin Dong <jindon@amazon.com> Add compose cosign test Signed-off-by: Jin Dong <jindon@amazon.com> Fix naming bug Signed-off-by: Jin Dong <jindon@amazon.com> Add compose cosign test [pass] Signed-off-by: Jin Dong <jindon@amazon.com> Add compose experimental flag Signed-off-by: Jin Dong <jindon@amazon.com> Refactor cosign func Signed-off-by: Jin Dong <jindon@amazon.com> Add compose cosign doc Signed-off-by: Jin Dong <jindon@amazon.com>
Signed-off-by: Jin Dong <jindon@amazon.com>
48d64ef
to
cfa27df
Compare
Thanks for pointing this out! This will be the same for cosign support in other commands right? (e.g. |
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.
Thanks
Fix #607.
Similar to ##556, #606, #628, this PR adds cosign support to compose:
compose run
,compose up
,compose push
,compose pull
.nerdctl
andnerdctl compose
.--experimental
for cosign support incompose
commands.I'm more towards the below method from #607 discussion, since it's less error-prone than cli args and more flexibile (my understanding is we should provide the capability to sign/verify individual images per services and cannot assume user will use the same pair for all images in a compose yaml).
A running example:
Signed-off-by: Jin Dong jindon@amazon.com