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.

Sign up for GitHub

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

Jump to bottom

Ignore case of MAC addresses and fix postgres log messages #1

Merged
merged 2 commits into from
Jan 19, 2024
Merged

Ignore case of MAC addresses and fix postgres log messages #1

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
50041b4
Store MAC addresses lower case; convert when comparing
synackd Oct 23, 2023
bb0901a
Remove period from error messages; fix comment typo
synackd Oct 23, 2023
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
68 changes: 49 additions & 19 deletions internal/postgres/postgres.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,11 +112,12 @@ func Connect(host string, port uint, user, password string, ssl bool) (BootDataD
return bddb, err
}

// NewNode creates a new Node and populates it with the boot MAC address, XName, and NID specified.
// Before returning the Node, its ID is populated with a new unique identifier.
// NewNode creates a new Node and populates it with the boot MAC address (converts to lower case),
// XName, and NID specified. Before returning the Node, its ID is populated with a new unique
// identifier.
func NewNode(mac, xname string, nid int32) (n Node) {
n.Id = uuid.Generate().String()
n.BootMac = mac
n.BootMac = strings.ToLower(mac)
n.Xname = xname
n.Nid = nid
return n
Expand Down Expand Up @@ -267,7 +268,12 @@ func (bddb BootDataDatabase) GetNodesByItems(macs, xnames []string, nids []int32
}
switch i {
case 0:
qstr += fmt.Sprintf(` boot_mac IN %s`, stringSliceToSql(macs))
// Ignore case when searching by MAC.
var macsLower []string
for _, mac := range macs {
macsLower = append(macsLower, strings.ToLower(mac))
}
qstr += fmt.Sprintf(` boot_mac IN %s`, stringSliceToSql(macsLower))
case 1:
qstr += fmt.Sprintf(` xname IN %s`, stringSliceToSql(xnames))
case 2:
Expand Down Expand Up @@ -579,7 +585,7 @@ func (bddb BootDataDatabase) addBootConfigByGroup(groupNames []string, kernelUri
results := make(map[string]string)

if len(groupNames) == 0 {
return results, fmt.Errorf("No group names specified to add.")
return results, fmt.Errorf("No group names specified to add")
}

// See if group name exists, if passed.
Expand Down Expand Up @@ -707,7 +713,7 @@ func (bddb BootDataDatabase) addBootConfigByNode(nodeList []Node, kernelUri, ini
bgaList []BootGroupAssignment
)
if len(nodeList) == 0 {
return result, fmt.Errorf("No nodes specified to add boot configurations for.")
return result, fmt.Errorf("No nodes specified to add boot configurations for")
}
existingBgList, existingBcList, numResults, err = bddb.GetBootConfigsByItems(kernelUri, initrdUri, cmdline)
if err != nil {
Expand Down Expand Up @@ -798,7 +804,7 @@ func (bddb BootDataDatabase) addBootConfigByNode(nodeList []Node, kernelUri, ini
// with any of the SQL queries, it is returned.
func (bddb BootDataDatabase) deleteBootGroupsByName(names []string) (bgList []BootGroup, err error) {
if len(names) == 0 {
err = fmt.Errorf("No boot group names specified to delete.")
err = fmt.Errorf("No boot group names specified to delete")
return bgList, err
}
// "RETURNING *" is Postgres-specific.
Expand Down Expand Up @@ -834,7 +840,7 @@ func (bddb BootDataDatabase) deleteBootGroupsByName(names []string) (bgList []Bo
// an error occurs with any of the SQL queries, it is returned.
func (bddb BootDataDatabase) deleteBootGroupsById(bgIds []string) (bgList []BootGroup, err error) {
if len(bgIds) == 0 {
err = fmt.Errorf("No boot group IDs specified to delete.")
err = fmt.Errorf("No boot group IDs specified to delete")
return bgList, err
}
// "RETURNING *" is Postgres-specific.
Expand Down Expand Up @@ -870,7 +876,7 @@ func (bddb BootDataDatabase) deleteBootGroupsById(bgIds []string) (bgList []Boot
// with any of the SQL queries, it is returned.
func (bddb BootDataDatabase) deleteBootConfigsById(bcIds []string) (bcList []BootConfig, err error) {
if len(bcIds) == 0 {
err = fmt.Errorf("No boot config IDs specified to delete.")
err = fmt.Errorf("No boot config IDs specified to delete")
return bcList, err
}
// "RETURNING *" is Postgres-specific.
Expand Down Expand Up @@ -1035,7 +1041,7 @@ func (bddb BootDataDatabase) deleteBootConfigsByItems(kernelUri, initrdUri, cmdl
// were deleted. If an error occurs with any of the SQL queries, it is returned.
func (bddb BootDataDatabase) deleteBootGroupAssignmentsByGroupId(bgIds []string) (bgaList []BootGroupAssignment, err error) {
if len(bgIds) == 0 {
err = fmt.Errorf("No boot group IDs specified for deleting boot group assignments.")
err = fmt.Errorf("No boot group IDs specified for deleting boot group assignments")
return bgaList, err
}
// "RETURNING *" is Postgres-specific.
Expand Down Expand Up @@ -1071,7 +1077,7 @@ func (bddb BootDataDatabase) deleteBootGroupAssignmentsByGroupId(bgIds []string)
// occurs with any of the SQL queries, it is returned.
func (bddb BootDataDatabase) deleteBootGroupAssignmentsByNodeId(nodeIds []string) (bgaList []BootGroupAssignment, err error) {
if len(nodeIds) == 0 {
err = fmt.Errorf("No node IDs specified for deleting boot group assignments.")
err = fmt.Errorf("No node IDs specified for deleting boot group assignments")
return bgaList, err
}
// "RETURNING *" is Postgres-specific.
Expand Down Expand Up @@ -1106,7 +1112,7 @@ func (bddb BootDataDatabase) deleteBootGroupAssignmentsByNodeId(nodeIds []string
// an error occurs with any of the SQL queries, it is returned.
func (bddb BootDataDatabase) deleteNodesById(nodeIds []string) (nodeList []Node, err error) {
if len(nodeIds) == 0 {
err = fmt.Errorf("No node IDs specified for deletion.")
err = fmt.Errorf("No node IDs specified for deletion")
return nodeList, err
}
// "RETURNING *" is Postgres-specific.
Expand Down Expand Up @@ -1142,7 +1148,7 @@ func (bddb BootDataDatabase) deleteNodesById(nodeIds []string) (nodeList []Node,
// nodes is returned. If an error occurs with any of the SQL queries, it is returned.
func (bddb BootDataDatabase) deleteNodesByItems(hosts, macs []string, nids []int32) (nodeList []Node, err error) {
if len(hosts) == 0 && len(macs) == 0 && len(nids) == 0 {
err = fmt.Errorf("No hosts, MAC addresses, or NIDs specified to delete nodes.")
err = fmt.Errorf("No hosts, MAC addresses, or NIDs specified to delete nodes")
return nodeList, err
}
qstr := `DELETE FROM nodes WHERE`
Expand All @@ -1156,7 +1162,12 @@ func (bddb BootDataDatabase) deleteNodesByItems(hosts, macs []string, nids []int
case 0:
qstr += fmt.Sprintf(` xname IN %s`, stringSliceToSql(hosts))
case 1:
qstr += fmt.Sprintf(` boot_mac IN %s`, stringSliceToSql(macs))
// Ignore case when matching MAC addresses.
var macsLower []string
for _, mac := range macs {
macsLower = append(macsLower, strings.ToLower(mac))
}
qstr += fmt.Sprintf(` boot_mac IN %s`, stringSliceToSql(macsLower))
case 2:
qstr += fmt.Sprintf(` nid IN %s`, int32SliceToSql(nids))
}
Expand Down Expand Up @@ -1197,7 +1208,7 @@ func (bddb BootDataDatabase) deleteNodesByItems(hosts, macs []string, nids []int
// SQL queries occurs, it is returned.
func (bddb BootDataDatabase) deleteBootConfigByGroup(groupNames []string) (nodeList []Node, bcList []BootConfig, err error) {
if len(groupNames) == 0 {
return nodeList, bcList, fmt.Errorf("No group names specified for deletion.")
return nodeList, bcList, fmt.Errorf("No group names specified for deletion")
}

// Delete matching boot groups, store deleted ones.
Expand Down Expand Up @@ -1257,6 +1268,7 @@ func (bddb BootDataDatabase) deleteBootConfigByGroup(groupNames []string) (nodeL
// BootConfig are also deleted. A slice of deleted Node items and a slice of deleted BootConfig
// items are returned. If an error occurs with any of the SQL queries, an error is returned.
func (bddb BootDataDatabase) deleteNodesWithBootConfigs(hosts, macs []string, nids []int32) (nodeList []Node, bcList []BootConfig, err error) {
// MAC address comparison is case-insensitive.
nodeList, err = bddb.deleteNodesByItems(hosts, macs, nids)
if err != nil {
err = fmt.Errorf("Error deleting Node(s): %v", err)
Expand Down Expand Up @@ -1404,7 +1416,12 @@ func (bddb BootDataDatabase) Add(bp bssTypes.BootParams) (result map[string]stri

// Store list of nodes to add.
for _, mac := range bp.Macs {
if existingNodeMap[mac] == (Node{}) {
if _, ok := existingNodeMap[strings.ToLower(mac)]; !ok {
// Store using lower case version of MAC address. When the MAC
// address is compared for retrieval/deletion/update, it will be
// converted into lower case before comparison, effectively ignoring
// case. This will prevent duplicate MAC addresses due to case
// difference.
nodesToAdd = append(nodesToAdd, NewNode(mac, "", 0))
}
}
Expand All @@ -1423,6 +1440,11 @@ func (bddb BootDataDatabase) Add(bp bssTypes.BootParams) (result map[string]stri
}
}

if len(nodesToAdd) == 0 {
err = fmt.Errorf("postgres.Add: No nodes to add (possible duplicate(s))")
return result, err
}

// Add any nonexisting nodes, plus their boot config as needed.
result, err = bddb.addBootConfigByNode(nodesToAdd, bp.Kernel, bp.Initrd, bp.Params)
if err != nil {
Expand Down Expand Up @@ -1468,7 +1490,7 @@ func (bddb BootDataDatabase) Delete(bp bssTypes.BootParams) (nodesDeleted, bcsDe
// Group name(s) specified, add boot config by group.
delNodes, delBcs, err = bddb.deleteBootConfigByGroup(groupNames)
if err != nil {
err = fmt.Errorf("postgres.Add: %v", err)
err = fmt.Errorf("postgres.Delete: %v", err)
return nodesDeleted, bcsDeleted, err
}
} else if len(xNames) > 0 {
Expand All @@ -1480,6 +1502,8 @@ func (bddb BootDataDatabase) Delete(bp bssTypes.BootParams) (nodesDeleted, bcsDe
}
}
case len(bp.Macs) > 0:
// This deletion function will ignore the case of the passed MAC addresses by first
// converting them to lower case before comparison.
delNodes, delBcs, err = bddb.deleteNodesWithBootConfigs([]string{}, bp.Macs, []int32{})
if err != nil {
err = fmt.Errorf("postgres.Delete: %v", err)
Expand Down Expand Up @@ -1661,16 +1685,22 @@ func (bddb BootDataDatabase) GetBootParamsByName(names []string) ([]bssTypes.Boo
func (bddb BootDataDatabase) GetBootParamsByMac(macs []string) ([]bssTypes.BootParams, error) {
var results []bssTypes.BootParams

// If inout is empty, so is the output.
// If input is empty, so is the output.
if len(macs) == 0 {
return results, nil
}

// Ignore case for MAC addresses.
var macsLower []string
for _, mac := range macs {
macsLower = append(macsLower, strings.ToLower(mac))
}

qstr := "SELECT n.boot_mac, bc.kernel_uri, bc.initrd_uri, bc.cmdline FROM nodes AS n" +
" LEFT JOIN boot_group_assignments AS bga ON n.id=bga.node_id" +
" JOIN boot_groups AS bg on bga.boot_group_id=bg.id" +
" JOIN boot_configs AS bc ON bg.boot_config_id=bc.id" +
" WHERE n.boot_mac IN " + stringSliceToSql(macs) +
" WHERE n.boot_mac IN " + stringSliceToSql(macsLower) +
";"
rows, err := bddb.DB.Query(qstr)
if err != nil {
Expand Down
5 changes: 3 additions & 2 deletions test/ct/postgres/tests/mac/04-bss-add-macs.hurl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,13 @@ POST http://bss:27778/boot/v1/bootparameters
HTTP 201

# Add another MAC address to BSS (to see if same boot config got used instead of
# creating another).
# creating another). Also, use an upper-case MAC address. BSS should store this
# as lower case.
POST http://bss:27778/boot/v1/bootparameters
{
"kernel": "https://testkerneluri2.tld",
"initrd": "https://testinitrduri2.tld",
"macs": ["02:0b:b8:00:30:04"],
"macs": ["02:0B:B8:00:30:04"],
"params": "param3,param4"
}

Expand Down
19 changes: 13 additions & 6 deletions ...postgres/tests/mac/05-bss-delete-mac.hurl → ...s/tests/mac/05-bss-add-duplicate-mac.hurl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,19 +1,25 @@
# Delete a MAC address.
DELETE http://bss:27778/boot/v1/bootparameters
# Try to add a duplicate MAC address using upper case. This should not add anything.
POST http://bss:27778/boot/v1/bootparameters
{
"macs": ["02:0b:b8:00:30:02"]
"kernel": "https://testkerneluri0.tld",
"initrd": "https://testinitrduri0.tld",
"macs": ["02:0B:B8:00:30:04"],
"params": "param0"
}

HTTP 200
HTTP 400
[Asserts]
jsonpath "$.detail" == "Bad Request: postgres.Add: No nodes to add (possible duplicate(s))"

GET http://bss:27778/boot/v1/bootparameters

# Response should be:
#
# [
# {
# "hosts": [
# "macs": [
# "02:0b:b8:00:30:00",
# "02:0b:b8:00:30:02",
# "02:0b:b8:00:30:04"
# ],
# "params": "param3,param4",
Expand All @@ -36,8 +42,9 @@ GET http://bss:27778/boot/v1/bootparameters
HTTP 200
Content-Type: application/json; charset=UTF-8
[Asserts]
jsonpath "$[0].macs" count == 2
jsonpath "$[0].macs" count == 3
jsonpath "$[0].macs[*]" includes "02:0b:b8:00:30:00"
jsonpath "$[0].macs[*]" includes "02:0b:b8:00:30:02"
jsonpath "$[0].macs[*]" includes "02:0b:b8:00:30:04"
jsonpath "$[0].params" == "param3,param4"
jsonpath "$[0].kernel" == "https://testkerneluri2.tld"
Expand Down
Loading
Loading