Skip to content
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 Unmarshal(cfg string) to VendorConfigManager #17

Merged
merged 18 commits into from
Jul 23, 2024
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
405a457
Add supermicro.Unmarshal() to config
splaspood May 30, 2024
22ec803
config/supermicro: Add config normalization functions
splaspood Jun 3, 2024
d7c5278
config/supermicro: Enforce ISO-8859-1/UTF-8 XML decoding
splaspood Jun 3, 2024
3400aa0
FS-1123: Enable line numbering for lint in GHA
splaspood Jun 5, 2024
e4d8a1e
FS-1123: Uncuddle my if
splaspood Jun 5, 2024
a070a21
FS-1123: Flesh out generic config options for supermicro a bit
splaspood Jun 5, 2024
cfd9f1a
FS-1123: Add generic configuration functions to interface
splaspood Jun 5, 2024
9b517fc
config/asrockrack.go: Explicitly return err from Unmarshal()
splaspood Jul 18, 2024
bd1910e
config/dell.go: Explicitly return err from Unmarshal()
splaspood Jul 18, 2024
dc11699
config/supermicro.go: Implement Menu/Setting finder functions
splaspood Jul 18, 2024
48975f6
config/asrockrack.go, config/dell.go: Use explict returns
splaspood Jul 18, 2024
1eff505
config/supermicro.go: Describe reasoning for use of for loops in Boot…
splaspood Jul 18, 2024
4150ec4
config/asrockrack.go, config/dell.go: Add dummy Unimplemented BootOrd…
splaspood Jul 18, 2024
a8e48f6
config/asrockrack.go, config/dell.go: Add dummy Unimplemented BootMod…
splaspood Jul 18, 2024
f23f69e
config/asrockrack.go, config/dell.go: Add dummy Unimplemented for rem…
splaspood Jul 18, 2024
fc87035
config/asrockrack.go, config/dell.go: Properly return nil for unimple…
splaspood Jul 18, 2024
e7cce54
config/supermicro.go: Correcting lint
splaspood Jul 18, 2024
bac336d
config/supermicro.go: Refactor FindOrCreateSetting to avoid premature…
splaspood Jul 18, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -18,5 +18,5 @@ jobs:
- name: Run golangci-lint
uses: golangci/golangci-lint-action@v3
with:
args: -v --config .golangci.yml --timeout=5m
args: -v --config .golangci.yml --timeout=5m --out-format=colored-line-number
version: latest
9 changes: 9 additions & 0 deletions config/asrockrack.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package config

Check failure on line 1 in config/asrockrack.go

View workflow job for this annotation

GitHub Actions / lint

: # github.com/bmc-toolbox/common/config

Check failure on line 1 in config/asrockrack.go

View workflow job for this annotation

GitHub Actions / lint

: # github.com/bmc-toolbox/common/config

import (
"encoding/xml"
Expand Down Expand Up @@ -48,7 +48,7 @@
BiosCfg: &asrockrackBiosCfg{},
}

return asrr, nil

Check failure on line 51 in config/asrockrack.go

View workflow job for this annotation

GitHub Actions / lint

cannot use asrr (variable of type *asrockrackVendorConfig) as type VendorConfigManager in return statement:

Check failure on line 51 in config/asrockrack.go

View workflow job for this annotation

GitHub Actions / lint

cannot use asrr (variable of type *asrockrackVendorConfig) as type VendorConfigManager in return statement:

Check failure on line 51 in config/asrockrack.go

View workflow job for this annotation

GitHub Actions / lint

cannot use asrr (variable of type *asrockrackVendorConfig) as type VendorConfigManager in return statement:

Check failure on line 51 in config/asrockrack.go

View workflow job for this annotation

GitHub Actions / lint

cannot use asrr (variable of type *asrockrackVendorConfig) as type VendorConfigManager in return statement:
}

// FindMenu locates an existing asrockrackBiosCfgMenu if one exists in the ConfigData, if not
Expand Down Expand Up @@ -105,6 +105,15 @@
}
}

func (cm *asrockrackVendorConfig) Unmarshal(cfgData string) (err error) {
err = xml.Unmarshal([]byte(cfgData), cm.ConfigData)
splaspood marked this conversation as resolved.
Show resolved Hide resolved
return
}

func (cm *asrockrackVendorConfig) StandardConfig() (biosConfig map[string]string, err error) {
return
splaspood marked this conversation as resolved.
Show resolved Hide resolved
}

// Generic config options

func (cm *asrockrackVendorConfig) EnableTPM() {
Expand Down
9 changes: 9 additions & 0 deletions config/dell.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@

dell.setSystemConfiguration(vendorOptions["model"], vendorOptions["servicetag"])

return dell, nil

Check failure on line 57 in config/dell.go

View workflow job for this annotation

GitHub Actions / lint

cannot use dell (variable of type *dellVendorConfig) as type VendorConfigManager in return statement:

Check failure on line 57 in config/dell.go

View workflow job for this annotation

GitHub Actions / lint

cannot use dell (variable of type *dellVendorConfig) as type VendorConfigManager in return statement:

Check failure on line 57 in config/dell.go

View workflow job for this annotation

GitHub Actions / lint

cannot use dell (variable of type *dellVendorConfig) as type VendorConfigManager in return statement:

Check failure on line 57 in config/dell.go

View workflow job for this annotation

GitHub Actions / lint

cannot use dell (variable of type *dellVendorConfig) as type VendorConfigManager in return statement:
}

func (cm *dellVendorConfig) setSystemConfiguration(model, servicetag string) {
Expand Down Expand Up @@ -129,6 +129,15 @@
}
}

func (cm *dellVendorConfig) Unmarshal(cfgData string) (err error) {
err = xml.Unmarshal([]byte(cfgData), cm.ConfigData)
splaspood marked this conversation as resolved.
Show resolved Hide resolved
return
}

func (cm *dellVendorConfig) StandardConfig() (biosConfig map[string]string, err error) {
return
splaspood marked this conversation as resolved.
Show resolved Hide resolved
}

// Generic config options

func (cm *dellVendorConfig) EnableTPM() {
Expand Down
16 changes: 16 additions & 0 deletions config/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,27 @@ import (

var errUnknownConfigFormat = errors.New("unknown config format")
var errUnknownVendor = errors.New("unknown/unsupported vendor")
var errUnknownSettingType = errors.New("unknown setting type")

var errInvalidBootModeOption = errors.New("invalid BootMode option <LEGACY|UEFI|DUAL>")
var errInvalidSGXOption = errors.New("invalid SGX option <Enabled|Disabled|Software Controlled>")

func UnknownConfigFormatError(format string) error {
return fmt.Errorf("unknown config format %w : %s", errUnknownConfigFormat, format)
}

func UnknownSettingType(t string) error {
return fmt.Errorf("unknown setting type %w : %s", errUnknownSettingType, t)
}

func UnknownVendorError(vendorName string) error {
return fmt.Errorf("unknown/unsupported vendor %w : %s", errUnknownVendor, vendorName)
}

func InvalidBootModeOption(mode string) error {
return fmt.Errorf("%w : %s", errInvalidBootModeOption, mode)
}

func InvalidSGXOption(mode string) error {
return fmt.Errorf("%w : %s", errInvalidSGXOption, mode)
}
13 changes: 10 additions & 3 deletions config/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,18 @@ import (
)

type VendorConfigManager interface {
EnableTPM()
EnableSRIOV()

Raw(name, value string, menuPath []string)
Marshal() (string, error)
Unmarshal(cfgData string) (err error)
StandardConfig() (biosConfig map[string]string, err error)

BootMode(mode string) error
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would recommend smaller interfaces, so not all vendor implementations are forced to implement all of these

BootOrder(mode string) error
IntelSGX(mode string) error
SecureBoot(enable bool) error
TPM(enable bool) error
SMT(enable bool) error
SRIOV(enable bool) error
}

func NewVendorConfigManager(configFormat, vendorName string, vendorOptions map[string]string) (VendorConfigManager, error) {
Expand Down
218 changes: 212 additions & 6 deletions config/supermicro.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,17 @@
package config

import (
"bytes"
"encoding/xml"
"strings"

"golang.org/x/net/html/charset"
)

const (
// enabledValue and disabledValue are utilized for bios setting value normalization
enabledValue = "Enabled"
disabledValue = "Disabled"
)

type supermicroVendorConfig struct {
Expand All @@ -28,10 +37,12 @@ type supermicroBiosCfgMenu struct {

type supermicroBiosCfgSetting struct {
XMLName xml.Name `xml:"Setting"`
Name string `xml:"Name,attr"`
Name string `xml:"name,attr"`
Order string `xml:"order,attr"`
SelectedOption string `xml:"selectedOption,attr"`
Type string `xml:"type,attr"`
CheckedStatus string `xml:"checkedStatus,attr"`
NumericValue string `xml:"numericValue,attr"`
}

func NewSupermicroVendorConfigManager(configFormat string, vendorOptions map[string]string) (VendorConfigManager, error) {
Expand Down Expand Up @@ -114,13 +125,208 @@ func (cm *supermicroVendorConfig) Marshal() (string, error) {
}
}

func (cm *supermicroVendorConfig) Unmarshal(cfgData string) (err error) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you include test cases and some fixture data for these

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to produce a future PR with test cases for all the config stuff and not block this one for now.

// the xml exported by sum is ISO-8859-1 encoded
decoder := xml.NewDecoder(bytes.NewReader([]byte(cfgData)))
// convert characters from non-UTF-8 to UTF-8
decoder.CharsetReader = charset.NewReaderLabel

err = decoder.Decode(cm.ConfigData.BiosCfg)
splaspood marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
return err
}

return
}

func (cm *supermicroVendorConfig) StandardConfig() (biosConfig map[string]string, err error) {
biosConfig = make(map[string]string)

for _, menu := range cm.ConfigData.BiosCfg.Menus {
for _, s := range menu.Settings {
switch s.Name {
// We want to drop this list of settings
case "NewSetupPassword", "NewSysPassword", "OldSetupPassword", "OldSysPassword":
// All others get normalized
default:
var k, v string
k, v, err = normalizeSetting(s)

if err != nil {
return
}

biosConfig[k] = v
}
}
}

return
}

func normalizeSetting(s *supermicroBiosCfgSetting) (k, v string, err error) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test cases for this method would help maintain this over time

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment re a future PR.

switch s.Type {
case "CheckBox":
k = normalizeName(s.Name)
v = normalizeValue(k, s.CheckedStatus)
case "Option":
k = normalizeName(s.Name)
v = normalizeValue(k, s.SelectedOption)
case "Password":
k = normalizeName(s.Name)
v = ""
case "Numeric":
k = normalizeName(s.Name)
v = normalizeValue(k, s.NumericValue)
default:
err = UnknownSettingType(s.Type)
return
}

return
}

func normalizeName(k string) string {
switch k {
case "CpuMinSevAsid":
return "amd_sev"
case "BootMode", "Boot mode select":
return "boot_mode"
case "IntelTxt":
return "intel_txt"
case "Software Guard Extensions (SGX)":
return "intel_sgx"
case "SecureBoot", "Secure Boot":
return "secure_boot"
case "Hyper-Threading", "Hyper-Threading [ALL]", "LogicalProc":
return "smt"
case "SriovGlobalEnable":
return "sr_iov"
case "TpmSecurity", "Security Device Support":
return "tpm"
default:
// When we don't normalize the key prepend "raw:"
return "raw:" + k
}
}

func normalizeBootMode(v string) string {
switch strings.ToLower(v) {
case "legacy":
return "BIOS"
default:
return strings.ToUpper(v)
}
}

func normalizeValue(k, v string) string {
if k == "boot_mode" {
return normalizeBootMode(v)
}

switch strings.ToLower(v) {
case "disable":
return disabledValue
case "disabled":
return disabledValue
case "enable":
return enabledValue
case "enabled":
return enabledValue
case "off":
return disabledValue
case "on":
return enabledValue
default:
return v
}
}

// Generic config options

func (cm *supermicroVendorConfig) EnableTPM() {
cm.Raw(" Security Device Support", "Enable", []string{"Trusted Computing"})
cm.Raw(" SHA-1 PCR Bank", "Enabled", []string{"Trusted Computing"})
func (cm *supermicroVendorConfig) BootMode(mode string) error {
switch strings.ToUpper(mode) {
case "LEGACY", "UEFI", "DUAL":
cm.Raw("Boot mode select", strings.ToUpper(mode), []string{"Boot"})
default:
return InvalidBootModeOption(strings.ToUpper(mode))
}

return nil
}

func (cm *supermicroVendorConfig) EnableSRIOV() {
cm.Raw("SR-IOV Support", "Enabled", []string{"Advanced", "PCIe/PCI/PnP Configuration"})
func (cm *supermicroVendorConfig) BootOrder(mode string) error {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test case here would help

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And here

switch strings.ToUpper(mode) {
case "LEGACY":
cm.Raw("Legacy Boot Option #1", "Hard Disk", []string{"Boot"})
cm.Raw("Legacy Boot Option #2", "Network", []string{"Boot"})
for i := 3; i < 8; i++ {
splaspood marked this conversation as resolved.
Show resolved Hide resolved
cm.Raw("Legacy Boot Option #"+string(i), "Disabled", []string{"Boot"})
}
case "UEFI":
cm.Raw("UEFI Boot Option #1", "UEFI Hard Disk", []string{"Boot"})
cm.Raw("UEFI Boot Option #2", "UEFI Network", []string{"Boot"})
for i := 3; i < 9; i++ {
splaspood marked this conversation as resolved.
Show resolved Hide resolved
cm.Raw("UEFI Boot Option #"+string(i), "Disabled", []string{"Boot"})
}
case "DUAL":
// TODO(jwb) Is this just both sets?
default:
return InvalidBootModeOption(strings.ToUpper(mode))
}

return nil
}

func (cm *supermicroVendorConfig) IntelSGX(mode string) error {
switch mode {
case "Disabled", "Enabled", "Software Controlled":
// TODO(jwb) Path needs to be determined.
cm.Raw("Software Guard Extensions (SGX)", mode, []string{"Advanced", "PCIe/PCI/PnP Configuration"})
default:
return InvalidSGXOption(mode)
}

return nil
}

func (cm *supermicroVendorConfig) SecureBoot(enable bool) error {
if enable {
cm.Raw("Secure Boot", "Enabled", []string{"SMC Secure Boot Configuration"})
// cm.Raw("Secure Boot Mode", "Setup", []string{"SMC Secure Boot Configuration"})
} else {
cm.Raw("Secure Boot", "Disabled", []string{"SMC Secure Boot Configuration"})
}

return nil
}

func (cm *supermicroVendorConfig) TPM(enable bool) error {
if enable {
// Note, this is actually 'Enable' not 'Enabled' like everything else.
cm.Raw(" Security Device Support", "Enable", []string{"Trusted Computing"})
cm.Raw(" SHA-1 PCR Bank", "Enabled", []string{"Trusted Computing"})
} else {
// Note, this is actually 'Disable' not 'Disabled' like everything else.
cm.Raw(" Security Device Support", "Disable", []string{"Trusted Computing"})
cm.Raw(" SHA-1 PCR Bank", "Disabled", []string{"Trusted Computing"})
}

return nil
}

func (cm *supermicroVendorConfig) SMT(enable bool) error {
if enable {
cm.Raw("Hyper-Threading", "Enabled", []string{"Advanced", "CPU Configuration"})
} else {
cm.Raw("Hyper-Threading", "Disabled", []string{"Advanced", "CPU Configuration"})
}

return nil
}

func (cm *supermicroVendorConfig) SRIOV(enable bool) error {
// TODO(jwb) Need to figure out how we do this on platforms that support it...

return nil
}
4 changes: 4 additions & 0 deletions go.mod
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
module github.com/bmc-toolbox/common

go 1.17

require golang.org/x/net v0.25.0

require golang.org/x/text v0.15.0 // indirect
4 changes: 4 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
golang.org/x/net v0.25.0 h1:d/OCCoBEUq33pjydKrGQhw7IlUPI2Oylr+8qLx49kac=
golang.org/x/net v0.25.0/go.mod h1:JkAGAh7GEvH74S6FOH42FLoXpXbE/aqXSrIQjXgsiwM=
golang.org/x/text v0.15.0 h1:h1V/4gjBv8v9cjcR6+AR5+/cIYK5N/WAgiv4xlsEtAk=
golang.org/x/text v0.15.0/go.mod h1:18ZOQIKpY8NJVqYksKHtTdi31H5itFRjB5/qKTNYzSU=
Loading