Feature cli (#56) #127

pxp928 · 2021-09-13T16:19:25Z

Added CLI support for in-toto-golang. Updated Readme and added Makefile support. Supports ITE-7 implementation for certificate constraints. Functionality testing of the CLI done via the Makefile. Second PR of the Boxboat fork.

Please fill in the fields below to submit a pull request. The more
information that is provided, the better.

Fixes issue #: #106

Description of pull request:

Please verify and check that the pull request fulfills the following
requirements:

Tests have been added for the bug fix or new feature
Docs have been added for the bug fix or new feature

shibumi

lgtm!

adityasaky

Thanks for the PR @pxp928! This looks great, and I think we should borrow some things from here for the CLI in https://github.com/in-toto/in-toto. However, I've marked some changes I think we need for consistency etc...

README.md

cmd/in-toto/run.go

adityasaky · 2021-09-14T16:23:32Z

cmd/in-toto/run.go

+func runPreRun(cmd *cobra.Command, args []string) error {
+	key = intoto.Key{}
+	cert = intoto.Key{}
+	if err := key.LoadKeyDefaults(keyPath); err != nil {


Can we better handle the error if neither key, nor cert is supplied? Right now, it looks like invalid key at : open : no such file or directory.

added check and error for file not found.

cmd/in-toto/record.go

cmd/in-toto/key.go

mikhailswift · 2021-09-14T18:59:52Z

I think the key layout command should be modified to exclude the private field of the keyval struct to be consistent with what the layout model expects

adityasaky · 2021-09-14T19:13:52Z

Good point @mikhailswift.

pxp928 · 2021-09-15T13:56:45Z

I will work through the fixes and changes today. Thanks for the feedback!

pxp928 · 2021-09-15T21:13:06Z

I think the key layout command should be modified to exclude the private field of the keyval struct to be consistent with what the layout model expects

@mikhailswift Should this just be changing the Private filed to string json:"private,omitempty"? Or did you have some other thoughts in mind?

mikhailswift · 2021-09-15T22:25:18Z

I think the key layout command should be modified to exclude the private field of the keyval struct to be consistent with what the layout model expects

@mikhailswift Should this just be changing the Private filed to string json:"private,omitempty"? Or did you have some other thoughts in mind?

I think that should do it :)

pxp928 · 2021-09-16T13:50:45Z

I think the key layout command should be modified to exclude the private field of the keyval struct to be consistent with what the layout model expects

@mikhailswift Should this just be changing the Private filed to string json:"private,omitempty"? Or did you have some other thoughts in mind?

I think that should do it :)

@mikhailswift I just did a replace to remove the empty private field. Using the omitempty caused many of the tests to fail. Is that what you were looking to do? The key layout is now properly formatted with the private field.

adityasaky · 2021-09-16T14:45:53Z

If in-toto key layout is pointed to a private key, it should still only print the public key. As it stands, it currently prints both, because private is not empty in this case. The command is meant to be used with the layout, where we only ever add public keys, so we don't want it to ever print the private key.

Also, I wonder if we can enhance it to output <KEYID>: <KEYOBJ> rather than just <KEYOBJ>. This makes it easier to paste into the pubkeys field of a layout.

adityasaky · 2021-09-16T14:46:54Z

I'll take another look at the PR as a whole today afternoon, thanks for addressing the comments @pxp928. 😄

cmd/in-toto/run.go

cmd/in-toto/record.go

README.md

pxp928 · 2021-09-16T19:00:39Z

If in-toto key layout is pointed to a private key, it should still only print the public key. As it stands, it currently prints both, because private is not empty in this case. The command is meant to be used with the layout, where we only ever add public keys, so we don't want it to ever print the private key.

Also, I wonder if we can enhance it to output <KEYID>: <KEYOBJ> rather than just <KEYOBJ>. This makes it easier to paste into the pubkeys field of a layout.

Updated such that in-toto key layout now does not print out the private field. Also changed the output to <KEYID>: <KEYOBJ> such that it can be added directly to the layout. Changed the Makefile to reflect this new substitution into the layout for testing.

adityasaky

Thanks @pxp928! This looks fine to me. I'm happy to merge tonight or so, but if you get a chance before then, could I ask you to squash some commits?

author pxp928 <[email protected]> 1631470550 -0400 committer pxp928 <[email protected]> 1631823796 -0400 go mod tidy using go version 1.17 updated record/run commands and readme added omitempty to private field removed omitempty as test fails added replace to remove empty private field in key layout updated key layout and added check for key or cert in run/record

adityasaky · 2021-09-17T13:57:30Z

Thanks for your work on this, @pxp928!

Parth Patel and others added 7 commits August 30, 2021 09:12

added cli

bcdb29d

added cli

9ea8854

added readme for the cli

d74b97d

updated go.sum entries

6ec1216

updated with upstream master

fee73c6

added certifcate back into cli, updated readme and added makefile

b13e099

moved layout and openssl into cert folder

4869a0a

adityasaky requested review from shibumi and adityasaky September 13, 2021 16:20

shibumi approved these changes Sep 13, 2021

View reviewed changes

adityasaky requested changes Sep 14, 2021

View reviewed changes

pxp928 mentioned this pull request Sep 15, 2021

Feature cli boxboat/in-toto-golang#57

Closed

2 tasks

adityasaky self-requested a review September 15, 2021 20:19

adityasaky requested changes Sep 16, 2021

View reviewed changes

cmd/in-toto/run.go Show resolved Hide resolved

cmd/in-toto/record.go Show resolved Hide resolved

cmd/in-toto/record.go Show resolved Hide resolved

README.md Show resolved Hide resolved

pxp928 requested a review from adityasaky September 16, 2021 19:01

adityasaky approved these changes Sep 16, 2021

View reviewed changes

pxp928 force-pushed the feature-cli branch from 882ae51 to 6862711 Compare September 16, 2021 20:29

adityasaky merged commit 6b6d786 into in-toto:master Sep 17, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature cli (#56) #127

Feature cli (#56) #127

pxp928 commented Sep 13, 2021 •

edited by adityasaky

Loading

shibumi left a comment

adityasaky left a comment

adityasaky Sep 14, 2021

pxp928 Sep 15, 2021

mikhailswift commented Sep 14, 2021

adityasaky commented Sep 14, 2021

pxp928 commented Sep 15, 2021

pxp928 commented Sep 15, 2021 •

edited

Loading

mikhailswift commented Sep 15, 2021

pxp928 commented Sep 16, 2021

adityasaky commented Sep 16, 2021 •

edited

Loading

adityasaky commented Sep 16, 2021

pxp928 commented Sep 16, 2021

adityasaky left a comment

adityasaky commented Sep 17, 2021

Feature cli (#56) #127

Feature cli (#56) #127

Conversation

pxp928 commented Sep 13, 2021 • edited by adityasaky Loading

shibumi left a comment

Choose a reason for hiding this comment

adityasaky left a comment

Choose a reason for hiding this comment

adityasaky Sep 14, 2021

Choose a reason for hiding this comment

pxp928 Sep 15, 2021

Choose a reason for hiding this comment

mikhailswift commented Sep 14, 2021

adityasaky commented Sep 14, 2021

pxp928 commented Sep 15, 2021

pxp928 commented Sep 15, 2021 • edited Loading

mikhailswift commented Sep 15, 2021

pxp928 commented Sep 16, 2021

adityasaky commented Sep 16, 2021 • edited Loading

adityasaky commented Sep 16, 2021

pxp928 commented Sep 16, 2021

adityasaky left a comment

Choose a reason for hiding this comment

adityasaky commented Sep 17, 2021

pxp928 commented Sep 13, 2021 •

edited by adityasaky

Loading

pxp928 commented Sep 15, 2021 •

edited

Loading

adityasaky commented Sep 16, 2021 •

edited

Loading