Skip to content

Commit

Permalink
Fix apply endpoint NPE (#193)
Browse files Browse the repository at this point in the history
* Fix null pointer in ApplyEndpoint()

* Improve logging output

Do not return error if an unknown inventory shows up, just print the info

* Always set inventory map low case, no matter the configuration casing
  • Loading branch information
damyan authored Sep 27, 2024
1 parent e8ccf83 commit b525015
Showing 1 changed file with 13 additions and 10 deletions.
23 changes: 13 additions & 10 deletions plugins/metal/plugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ import (
"os"
"strings"

"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"

"github.com/coredhcp/coredhcp/handler"
"github.com/coredhcp/coredhcp/logger"
"github.com/coredhcp/coredhcp/plugins"
Expand All @@ -23,7 +25,6 @@ import (
"github.com/mdlayher/netx/eui64"
"gopkg.in/yaml.v3"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
controllerruntime "sigs.k8s.io/controller-runtime"
)

var log = logger.GetLogger("plugins/metal")
Expand Down Expand Up @@ -56,13 +57,12 @@ func setup6(args ...string) (handler.Handler6, error) {
}

func loadConfig(args ...string) (map[string]string, error) {
log.Info("Loading metal config")
path, err := parseArgs(args...)
if err != nil {
return nil, fmt.Errorf("invalid configuration: %v", err)
}

log.Info("Reading metal config file", "ConfigFile", path)
log.Infof("Reading metal config file %s", path)
configData, err := os.ReadFile(path)
if err != nil {
return nil, fmt.Errorf("failed to read config file: %v", err)
Expand All @@ -76,11 +76,11 @@ func loadConfig(args ...string) (map[string]string, error) {
inventories := make(map[string]string)
for _, i := range config {
if i.MacAddress != "" && i.Name != "" {
inventories[i.MacAddress] = i.Name
inventories[strings.ToLower(i.MacAddress)] = i.Name
}
}

log.Info("Loaded metal config", "Inventories", len(inventories))
log.Infof("Loaded metal config with %d inventories", len(inventories))
return inventories, nil
}

Expand Down Expand Up @@ -133,10 +133,11 @@ func handler4(req, resp *dhcpv4.DHCPv4) (*dhcpv4.DHCPv4, bool) {
}

func applyEndpointForMACAddress(mac net.HardwareAddr, subnetFamily ipamv1alpha1.SubnetAddressType) error {
inventoryName, ok := inventoryMap[mac.String()]
inventoryName, ok := inventoryMap[strings.ToLower(mac.String())]
if !ok {
// done here, next plugin
return fmt.Errorf("unknown inventory MAC address: %s", mac.String())
// done here, return no error, next plugin
log.Printf("Unknown inventory MAC address: %s", mac.String())
return nil
}

ip, err := GetIPForMACAddress(mac, subnetFamily)
Expand All @@ -147,6 +148,8 @@ func applyEndpointForMACAddress(mac net.HardwareAddr, subnetFamily ipamv1alpha1.
if ip != nil {
if err := ApplyEndpointForInventory(inventoryName, mac, ip); err != nil {
return fmt.Errorf("could not apply endpoint for inventory: %s", err)
} else {
log.Infof("Successfully applied endpoint for inventory %s[%s]", inventoryName, mac.String())
}
} else {
log.Infof("Could not find IP for MAC address %s", mac.String())
Expand Down Expand Up @@ -179,7 +182,7 @@ func ApplyEndpointForInventory(name string, mac net.HardwareAddr, ip *netip.Addr
return fmt.Errorf("kubernetes client not initialized")
}

if _, err := controllerruntime.CreateOrUpdate(ctx, cl, endpoint, func() error { return nil }); err != nil {
if _, err := controllerutil.CreateOrPatch(ctx, cl, endpoint, nil); err != nil {
return fmt.Errorf("failed to apply endpoint: %v", err)
}

Expand All @@ -200,7 +203,7 @@ func GetIPForMACAddress(mac net.HardwareAddr, subnetFamily ipamv1alpha1.SubnetAd
return nil, fmt.Errorf("failed to list IPs: %v", err)
}

sanitizedMAC := strings.Replace(mac.String(), ":", "", -1)
sanitizedMAC := strings.Replace(strings.ToLower(mac.String()), ":", "", -1)
for _, ip := range ips.Items {
if ip.Labels["mac"] == sanitizedMAC && ipFamilyMatches(ip, subnetFamily) {
return &ip.Status.Reserved.Net, nil
Expand Down

0 comments on commit b525015

Please sign in to comment.