asterfusion commit pki2.0#21
Conversation
Larry199203
requested review from
amoljawale2,
binnyjeshan,
jaspreetsachdev and
liutao0316
binnyjeshan
requested review from
Cahb,
chengchao23041,
jacky1100002,
mikehansen1970,
oleksandr-kholodnyi,
phwhite and
taraschornyiplv
|
Could you please add relevant folks from the OLS SONiC side to review as well, thanks. |
| // save to operational.pem | ||
| file, err := os.Create(operationalPath) | ||
| if err != nil { | ||
| logger.Error("cannot create opreational.pem, err is %s", err.Error()) |
There was a problem hiding this comment.
minor mistake .. typo error "operational" it must have been.
| Bytes: cert.Raw, | ||
| }) | ||
| if err != nil { | ||
| logger.Error("write opreational.pem failed, err is %s", err.Error()) |
| if ControllerAddr == "" { | ||
| logger.Error("Could not get ControllerAddr") | ||
| estServerList = []string{estHost} | ||
| updateOperationalPem() |
There was a problem hiding this comment.
return if it was false, then its not handled ?
| cmd.Stdout = &out | ||
| err := cmd.Run() | ||
| if err != nil { | ||
| logger.Error("command dig failed, err is %s", err.Error()) |
There was a problem hiding this comment.
Logging the domain context may also be additionally helpful in this log.
| caUrl := "https://" + estServer + "/cacerts" | ||
| resp, err := client.Get(caUrl) | ||
| if err != nil { | ||
| logger.Error("request failed, err is %s", err.Error()) |
There was a problem hiding this comment.
additionally printing the url attempted may be helpful.
There was a problem hiding this comment.
the err info contains the url.But It's more readable when expressed in code. Thank you!
| } | ||
|
|
||
| // parse private key | ||
| privateKey, err := x509.ParsePKCS1PrivateKey(block.Bytes) |
There was a problem hiding this comment.
getSignatureAlgorithm function supports both RSA and ECDSA, rest is erred.
In tihs code, there is a single key type parsed. Is that enough?
I'm not sure if my comment is right, but thinking why only this is called here and returned failed below, without checking if the key type is something else?
| return true | ||
| } | ||
|
|
||
| estServerList = getEstServer(ControllerAddr) |
There was a problem hiding this comment.
What if this was returned empty from getEstServer ?
Guess that case to be handled?
There was a problem hiding this comment.
If It's empty from getEstServer, the code will use production est or qs est based on the birth certificate.
2 participants
https://telecominfraproject.atlassian.net/browse/OLS-828