Fixed issue #1200 (buildctl: add --tlsdir) #1284
Conversation
AkihiroSuda
changed the title
|
@nvpandeti (Nikhil Varma Pandeti ) the signer (Jeffrey Huang) seems another person |
|
@AkihiroSuda I should have mentioned this earlier, @jeffreyhuang23 and I are members of the same Virtualization course at UT Austin. We collaborated on fixes for issues #1200 #1230 and #1240 . I can have him confirm this ASAP. |
cmd/buildctl/common/common.go
Outdated
| // Look for cert.pem and, if it exists, set key to that | ||
| // Look for key.pem and, if it exists, set tlsDir to that | ||
| for _, v := range [3]string{"/ca.pem", "/cert.pem", "/key.pem"} { | ||
| matches, _ := filepath.Glob(tlsDir + v) |
There was a problem hiding this comment.
This looks like a weird construction to me.
if _, err := os.Stat(filepath.Join(tlsDir, v)); err != nil {
switch v {
seems more readable to me. Also, I think we need to error if one of the files can't be found under tlsdir
There was a problem hiding this comment.
Gotcha, applying your changes and adding an error if one of the files are not found in the specified TLS directory
jeffreyhuang23
force-pushed
the
issue-1200
branch
from
5f46ed2
to
f39baa9
Compare
cmd/buildctl/common/common.go
Outdated
| // Look for ca.pem and, if it exists, set caCert to that | ||
| // Look for cert.pem and, if it exists, set key to that | ||
| // Look for key.pem and, if it exists, set tlsDir to that | ||
| for _, v := range [3]string{"/ca.pem", "/cert.pem", "/key.pem"} { |
There was a problem hiding this comment.
Gotcha, removing those slashes
cmd/buildctl/common/common.go
Outdated
| key = file | ||
| } | ||
| } else { | ||
| return nil, errors.New(v + " not found in specified TLS directory") |
There was a problem hiding this comment.
if error is not os.IsNotExist() we should not try to hide the original error. You may just wrap the error with the specific filename as well.
There was a problem hiding this comment.
Returning the error given by os.Stat(file) then. Not wrapping the error with the specific filename though because the error already returns the absolute path of the file with issues.
Added a command-line flag (tlsdir) to buildctl that allowed for specifying a directory that contains a ca.pem, cert.pem, and key.pem. This command-line flag acts an alias of --tlscacert, --tlscert, and --tlskey. --tlsdir cannot be used at the same time as those flags, and will cause an error if done. The pkg/errors package has been added to cmd/buildctl/common/common.go to allow for the creation of this error message. Signed-off-by: Jeffrey Huang <[email protected]>
jeffreyhuang23
force-pushed
the
issue-1200
branch
from
f39baa9
to
783a723
Compare
Added a command-line flag (tlsdir) to buildctl that allowed for specifying a directory that contains a ca.pem, cert.pem, and key.pem. This command-line flag acts an alias of --tlscacert, --tlscert, and --tlskey. --tlsdir cannot be used at the same time as those flags, and will cause an error if done. The pkg/errors package has been added to cmd/buildctl/common/common.go to allow for the creation of this error message.
fix #1200
Signed-off-by: Jeffrey Huang [email protected]