Adding Support for JAWS PDU devices#45
Conversation
|
Testing: PDU in SMD: Power status: Power off: Confirmed: Just need those commits signed. |
3745e96 to
6384bfd
Compare
Signed-off-by: Michael Buchmann <michael.buchmann@hpe.com>
Signed-off-by: Michael Buchmann <michael.buchmann@hpe.com>
6384bfd to
21eda97
Compare
alexlovelltroy
left a comment
There was a problem hiding this comment.
Looks good. Two minor issues:
-
I don't understand the need for the GODEBUG environment variable and am suspicious that we don't actually want it in the production container.
-
The JAWS implementation appears to be fairly well compartmentalized. Can we move it to
pkgso other tools can use it?
rainest
left a comment
There was a problem hiding this comment.
We can probably separate concerns more, but if nothing else we should make sure we're handling all errors, and aborting on them immediately if we cannot expect to successfully proceed.
Signed-off-by: Michael Buchmann <michael.buchmann@hpe.com>
rainest
left a comment
There was a problem hiding this comment.
Sorry I missed the TLS stuff previously. Do these devices usually have decent enough cert management that we can default to secure and provide a flag to skip JAWS TLS verification? Can we also limit the additional ciphers to them?
internal/domain/jaws.go
Outdated
| func JawsLoad(xname string, FQDN string, authUser string, authPass string) { | ||
| timeout := 20 | ||
| transport := &http.Transport{ | ||
| TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, |
There was a problem hiding this comment.
Missed this earlier, hard coding this off seems iffy. What's the typical certificate management of these devices like? I'm not surprised if it's not great and winds up using self-signed certs often, but we should at add comments here indicating why this is off if so.
Ideally we actually default it on and make insecure an explicit opt-in.
We can also set
CipherSuites: []uint16{...}
here rather than setting tlsrakex=1, correct? Ideally we don't enable those across the board.
Unfortunately Golang's helper list functions return cipher suite structs while configuration wants their int representation, so we'd need our own helper to generate the longer list:
func jawsSuites() []uint16 {
suites := []uint16{tls.TLS_RSA_WITH_AES_128_GCM_SHA256, tls.TLS_RSA_WITH_AES_256_GCM_SHA384, tls.TLS_RSA_WITH_AES_128_CBC_SHA, tls.TLS_RSA_WITH_AES_256_CBC_SHA}
for _, c := range tls.CipherSuites() {
suites = append(suites, c.ID)
}
return suites
}
|
@mbuchmann-hpe Can you address the concerns in this PR and/or close it? @cjh1 What do you think? |
|
Would definitely like to get this in, @rainest Have your concerns been address or could be addressed in a follow up? |
Summary and Scope
Adds JAWS PDU Support:
ServerTech PDUs do not use Redfish, but instead uses the JSON API Web Service (JAWS).
Instead of creating an interface which converts Redfish calls to JAWS, it was decided to add support for PCS to call JAWS directly.
Added the file jaws.go which takes care of the JAWS calls and monitoring of the PDU.
The URIs must be setup correctly in SMD (HSM) to detect these PDUs and make the correct calls.
Testing
Tested locally using the SMD setup on Tamarindo and the PDU from another local system. Able to detect PDUs, query and store power status and power on/off outlets.
Test Procedure
Using a ServerTech PDU, add the PDU to SMD/HSM. PCS should automatically discover the PDU and start to monitor it for power status.
Test using the following curl (these assume PCS is running on localhost:28007 and the PDU is x3000m0)
Risks and Mitigations
Changes where carefully done to leave all current functionality in tact.
Environment variables must be set to enable the PDU monitoring (see Dockerfile).