Skip to content

Commit

Permalink
Store MAC as lowercase (#279)
Browse files Browse the repository at this point in the history
## Description

Always stores any MAC addresses provided as lowercase to avoid issues with comparisons later on.

## Why is this needed

boots is attempting to compare pxe booted systems using a lowercased MAC address, so if the user inputs the MAC addresses that are capitalized they will not match.

Fixes: #278 

## How Has This Been Tested?
Still in progress, will be testing against the Vagrant getting started guide, and manually changing the MAC used to be uppercased.

## How are existing users impacted? What migration steps/scripts do we need?

This should not effect existing users (at least ones that have functioning environments), since existing hardware definitions using upper cased MAC addresses would be failing already.

## Checklist:

I have:

- [ ] updated the documentation and/or roadmap (if required)
- [ ] added unit or e2e tests
- [ ] provided instructions on how to upgrade
  • Loading branch information
mergify[bot] authored Sep 16, 2020
2 parents 9de4dd5 + 48bcbe7 commit 966682b
Show file tree
Hide file tree
Showing 3 changed files with 216 additions and 10 deletions.
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,7 @@ github.com/prometheus/client_golang v0.9.3/go.mod h1:/TN21ttK/J9q6uSwhBd54HahCDf
github.com/prometheus/client_golang v1.0.0/go.mod h1:db9x61etRT2tGnBNRi70OPL5FsnadC4Ky3P0J6CfImo=
github.com/prometheus/client_golang v1.1.0 h1:BQ53HtBmfOitExawJ6LokA4x8ov/z0SYYb0+HxJfRI8=
github.com/prometheus/client_golang v1.1.0/go.mod h1:I1FGZT9+L76gKKOs5djB6ezCbFQP1xR9D75/vuwEF3g=
github.com/prometheus/client_golang v1.7.1 h1:NTGy1Ja9pByO+xAeH/qiWnLrKtr3hJPNjaVUwnjpdpA=
github.com/prometheus/client_model v0.0.0-20180712105110-5c3871d89910/go.mod h1:MbSGuTsp3dbXC40dX6PRTWyKYBIrTGTE9sqQNg2J8bo=
github.com/prometheus/client_model v0.0.0-20190129233127-fd36f4220a90/go.mod h1:xMI15A0UPsDsEKsMN9yxemIoYk6Tm2C1GtYGdfGttqA=
github.com/prometheus/client_model v0.0.0-20190812154241-14fe0d1b01d4 h1:gQz4mCbXsO+nc9n1hCxHcGA3Zx3Eo+UHZoInFGUIXNM=
Expand Down Expand Up @@ -211,6 +212,7 @@ github.com/spf13/viper v1.4.0 h1:yXHLWeravcrgGyFSyCgdYpXQ9dR9c/WED3pg1RhxqEU=
github.com/spf13/viper v1.4.0/go.mod h1:PTJ7Z/lr49W6bUbkmS1V3by4uWynFiR9p7+dSq/yZzE=
github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
github.com/stretchr/objx v0.1.1/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
github.com/stretchr/objx v0.2.0 h1:Hbg2NidpLE8veEBkEZTL3CvlkUIVzuU9jDplZO54c48=
github.com/stretchr/objx v0.2.0/go.mod h1:qt09Ya8vawLte6SNmTgCsAVtYtaKzEcn8ATUoHMkEqE=
github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs=
github.com/stretchr/testify v1.3.0 h1:TivCn/peBQ7UY8ooIcPgZFpTNSz0Q2U6UrFlUfqbe0Q=
Expand Down
45 changes: 35 additions & 10 deletions grpc-server/hardware.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"encoding/json"
"fmt"
"strings"
"time"

"github.com/pkg/errors"
Expand All @@ -29,15 +30,26 @@ func (s *server) Push(ctx context.Context, in *hardware.PushRequest) (*hardware.
// must be a copy so deferred cacheInFlight.Dec matches the Inc
labels = prometheus.Labels{"method": "Push", "op": ""}

hw := in.Data
if hw.Id == "" {
hw := in.GetData()
if hw == nil {
err := errors.New("expected data not to be nil")
logger.Error(err)
return &hardware.Empty{}, err
}

// we know hw is non-nil at this point, since we returned early above
// if it was nil
if hw.GetId() == "" {
metrics.CacheTotals.With(labels).Inc()
metrics.CacheErrors.With(labels).Inc()
err := errors.New("id must be set to a UUID, got id: " + hw.Id)
logger.Error(err)
return &hardware.Empty{}, err
}

// normalize data prior to storing in the database
normalizeHardwareData(hw)

// validate the hardware data to avoid duplicate mac address
err := s.validateHardwareData(ctx, hw)
if err != nil {
Expand Down Expand Up @@ -292,22 +304,35 @@ func (s *server) Delete(ctx context.Context, in *hardware.DeleteRequest) (*hardw
}

func (s *server) validateHardwareData(ctx context.Context, hw *hardware.Hardware) error {
interfaces := hw.GetNetwork().GetInterfaces()
for i := range hw.GetNetwork().GetInterfaces() {
data, _ := s.db.GetByMAC(ctx, interfaces[i].GetDhcp().GetMac())
if data != "" {
logger.With("MAC", interfaces[i].GetDhcp().GetMac()).Info(duplicateMAC)
for _, iface := range hw.GetNetwork().GetInterfaces() {
mac := iface.GetDhcp().GetMac()

if data, _ := s.db.GetByMAC(ctx, mac); data != "" {
logger.With("MAC", mac).Info(duplicateMAC)

newhw := hardware.Hardware{}
err := json.Unmarshal([]byte(data), &newhw)
if err != nil {
if err := json.Unmarshal([]byte(data), &newhw); err != nil {
logger.Error(err, "Failed to unmarshal hardware data")
return err
}

if newhw.Id == hw.Id {
return nil
}
return errors.New(fmt.Sprintf(conflictMACAddr, interfaces[i].GetDhcp().GetMac()))

return fmt.Errorf(conflictMACAddr, mac)
}
}

return nil
}

func normalizeHardwareData(hw *hardware.Hardware) {
// Ensure MAC is stored as lowercase
for _, iface := range hw.GetNetwork().GetInterfaces() {
dhcp := iface.GetDhcp()
if mac := dhcp.GetMac(); mac != "" {
dhcp.Mac = strings.ToLower(mac)
}
}
}
179 changes: 179 additions & 0 deletions grpc-server/hardware_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,179 @@
package grpcserver

import (
"testing"

"github.com/stretchr/testify/assert"
"github.com/tinkerbell/tink/protos/hardware"
)

func Test_server_normalizeHardwareData(t *testing.T) {
tests := []struct {
name string
given *hardware.Hardware
expected *hardware.Hardware
}{
{
name: "Expect MAC to be normalized to lowercase",
given: &hardware.Hardware{
Network: &hardware.Hardware_Network{
Interfaces: []*hardware.Hardware_Network_Interface{
{
Dhcp: &hardware.Hardware_DHCP{
Mac: "02:42:0E:D9:C7:53",
},
},
},
},
},
expected: &hardware.Hardware{
Network: &hardware.Hardware_Network{
Interfaces: []*hardware.Hardware_Network_Interface{
{
Dhcp: &hardware.Hardware_DHCP{
Mac: "02:42:0e:d9:c7:53",
},
},
},
},
},
},
{
name: "Expect MAC to be normalized to lowercase, multiple interfaces",
given: &hardware.Hardware{
Network: &hardware.Hardware_Network{
Interfaces: []*hardware.Hardware_Network_Interface{
{
Dhcp: &hardware.Hardware_DHCP{
Mac: "02:42:0E:D9:C7:53",
},
},
{
Dhcp: &hardware.Hardware_DHCP{
Mac: "02:4F:0E:D9:C7:5E",
},
},
},
},
},
expected: &hardware.Hardware{
Network: &hardware.Hardware_Network{
Interfaces: []*hardware.Hardware_Network_Interface{
{
Dhcp: &hardware.Hardware_DHCP{
Mac: "02:42:0e:d9:c7:53",
},
},
{
Dhcp: &hardware.Hardware_DHCP{
Mac: "02:4f:0e:d9:c7:5e",
},
},
},
},
},
},
{
name: "Expect MAC to be normalized to lowercase, nultiple interfaces, mixed casing",
given: &hardware.Hardware{
Network: &hardware.Hardware_Network{
Interfaces: []*hardware.Hardware_Network_Interface{
{
Dhcp: &hardware.Hardware_DHCP{
Mac: "02:42:0e:d9:c7:53",
},
},
{
Dhcp: &hardware.Hardware_DHCP{
Mac: "02:4F:0E:D9:C7:5E",
},
},
},
},
},
expected: &hardware.Hardware{
Network: &hardware.Hardware_Network{
Interfaces: []*hardware.Hardware_Network_Interface{
{
Dhcp: &hardware.Hardware_DHCP{
Mac: "02:42:0e:d9:c7:53",
},
},
{
Dhcp: &hardware.Hardware_DHCP{
Mac: "02:4f:0e:d9:c7:5e",
},
},
},
},
},
},
{
name: "Handle nil DHCP",
given: &hardware.Hardware{
Network: &hardware.Hardware_Network{
Interfaces: []*hardware.Hardware_Network_Interface{
{
Dhcp: nil,
},
},
},
},
expected: &hardware.Hardware{
Network: &hardware.Hardware_Network{
Interfaces: []*hardware.Hardware_Network_Interface{
{
Dhcp: nil,
},
},
},
},
},
{
name: "Handle nil Interface",
given: &hardware.Hardware{
Network: &hardware.Hardware_Network{
Interfaces: []*hardware.Hardware_Network_Interface{
nil,
},
},
},
expected: &hardware.Hardware{
Network: &hardware.Hardware_Network{
Interfaces: []*hardware.Hardware_Network_Interface{
nil,
},
},
},
},
{
name: "Handle nil Interfaces",
given: &hardware.Hardware{
Network: &hardware.Hardware_Network{
Interfaces: nil,
},
},
expected: &hardware.Hardware{
Network: &hardware.Hardware_Network{
Interfaces: nil,
},
},
},
{
name: "Handle nil Network",
given: &hardware.Hardware{Network: nil},
expected: &hardware.Hardware{Network: nil},
},
{
name: "Handle nil Hardware",
given: nil,
expected: nil,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
assert.NotPanics(t, func() { normalizeHardwareData(tt.given) })
assert.Equal(t, tt.expected, tt.given)
})
}
}

0 comments on commit 966682b

Please sign in to comment.