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

Configure DNS and NTP in machine allocation #571

Merged
merged 24 commits into from
Nov 8, 2024
Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
df41e53
Configure DNS servers in machine allocation optionally
simcod Sep 11, 2024
4a18873
Return DNSServers in MachineResponse
simcod Sep 11, 2024
ffa8877
Add ntp servers field
simcod Sep 13, 2024
77f0843
NTP and DNS validation
simcod Sep 13, 2024
cbf9f21
Regenerate metal-api
simcod Sep 13, 2024
fcba3b4
Merge branch 'master' into machine-allocation-with-dns-and-ntp
majst01 Oct 4, 2024
139ca1b
Merge remote-tracking branch 'origin' into machine-allocation-with-dn…
simcod Oct 16, 2024
fe45192
Use new struct for DNS and NTP servers
simcod Oct 17, 2024
9973d7c
Check for empty ntp servers configuration
simcod Oct 22, 2024
f57775b
Add NTP and DNS server field to partition
simcod Oct 22, 2024
7abf175
Add new metal-api JSON
simcod Oct 22, 2024
fd58302
Add dns and ntp servers to partition base to include it in response
simcod Oct 23, 2024
a8f9111
Add DNS and NTP servers to fix integration test
simcod Oct 23, 2024
21bf0c5
Fix typo
simcod Oct 24, 2024
6a6bf09
Set partition default for machine allocation spec
simcod Oct 24, 2024
e82d7d0
Add DNS and NTP servers to partition response
simcod Oct 24, 2024
2df7e17
Fix: Specify a minimum of three NTP servers
simcod Oct 28, 2024
451813f
Integrate review
simcod Oct 29, 2024
b5b0adc
Regenerate spec
simcod Oct 29, 2024
14d08b3
Merge branch 'master' into machine-allocation-with-dns-and-ntp
Gerrit91 Nov 7, 2024
b5236c3
Implement review
simcod Nov 8, 2024
5f678e1
Remove if condition
simcod Nov 8, 2024
5f71f78
Review with Simon.
Gerrit91 Nov 8, 2024
d01ce2d
Fix.
Gerrit91 Nov 8, 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
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,12 @@ func (_ *machineTestable) defaultBody(m *metal.Machine) *metal.Machine {
if m.Allocation.SSHPubKeys == nil {
m.Allocation.SSHPubKeys = []string{}
}
if m.Allocation.DNSServers == nil {
m.Allocation.DNSServers = metal.DNSServers{}
}
if m.Allocation.NTPServers == nil {
m.Allocation.NTPServers = metal.NTPServers{}
}
for i := range m.Allocation.MachineNetworks {
n := m.Allocation.MachineNetworks[i]
if n.Prefixes == nil {
Expand Down
14 changes: 14 additions & 0 deletions cmd/metal-api/internal/metal/machine.go
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,8 @@ type MachineAllocation struct {
VPN *MachineVPN `rethinkdb:"vpn" json:"vpn"`
UUID string `rethinkdb:"uuid" json:"uuid"`
FirewallRules *FirewallRules `rethinkdb:"firewall_rules" json:"firewall_rules"`
DNSServers DNSServers `rethinkdb:"dns_servers" json:"dns_servers"`
NTPServers NTPServers `rethinkdb:"ntp_servers" json:"ntp_servers"`
}

type FirewallRules struct {
Expand All @@ -175,6 +177,18 @@ type IngressRule struct {
Comment string `rethinkdb:"comment" json:"comment"`
}

type DNSServers []DNSServer

type DNSServer struct {
IP string `rethinkdb:"ip" json:"ip" description:"ip address of this dns server"`
}

type NTPServers []NTPServer

type NTPServer struct {
Address string `address:"address" json:"address" description:"ip address or dns hostname of this ntp server"`
}

type Protocol string

const (
Expand Down
2 changes: 2 additions & 0 deletions cmd/metal-api/internal/metal/partition.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ type Partition struct {
MgmtServiceAddress string `rethinkdb:"mgmtserviceaddr" json:"mgmtserviceaddr"`
PrivateNetworkPrefixLength uint8 `rethinkdb:"privatenetworkprefixlength" json:"privatenetworkprefixlength"`
Labels map[string]string `rethinkdb:"labels" json:"labels"`
DNSServers DNSServers `rethinkdb:"dns_servers" json:"dns_servers"`
NTPServers NTPServers `rethinkdb:"ntp_servers" json:"ntp_servers"`
}

// BootConfiguration defines the metal-hammer initrd, kernel and commandline
Expand Down
53 changes: 53 additions & 0 deletions cmd/metal-api/internal/service/machine-service.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,12 @@ import (
"log/slog"
"net"
"net/http"
"net/netip"
"strconv"
"strings"
"time"

"github.com/asaskevich/govalidator"
"github.com/google/uuid"
"github.com/metal-stack/metal-api/cmd/metal-api/internal/headscale"
"github.com/metal-stack/metal-api/cmd/metal-api/internal/issues"
Expand Down Expand Up @@ -78,6 +80,8 @@ type machineAllocationSpec struct {
PlacementTags []string
EgressRules []metal.EgressRule
IngressRules []metal.IngressRule
DNSServers metal.DNSServers
NTPServers metal.NTPServers
}

// allocationNetwork is intermediate struct to create machine networks from regular networks during machine allocation
Expand Down Expand Up @@ -1143,6 +1147,51 @@ func createMachineAllocationSpec(ds *datastore.RethinkStore, machineRequest v1.M
return nil, fmt.Errorf("size:%s not found err:%w", sizeID, err)
}

partition, err := ds.FindPartition(partitionID)
if err != nil {
return nil, fmt.Errorf("partition:%s not found err:%w", partitionID, err)
}
var (
dnsServers metal.DNSServers
ntpServers metal.NTPServers
Copy link
Contributor

Choose a reason for hiding this comment

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

This would render the else branches unnecessary:

Suggested change
dnsServers metal.DNSServers
ntpServers metal.NTPServers
dnsServers = partition.DNSServers
ntpServers = partition.NTPServers

)
if len(machineRequest.DNSServers) != 0 {
if len(machineRequest.DNSServers) > 3 {
return nil, errors.New("please specify a maximum of three dns servers")
}
dnsServers = machineRequest.DNSServers
} else {
dnsServers = partition.DNSServers
}
for _, dnsServer := range dnsServers {
_, err := netip.ParseAddr(dnsServer.IP)
if err != nil {
return nil, fmt.Errorf("ip: %s for dns server not correct err: %w", dnsServer, err)
}
}

if len(machineRequest.NTPServers) != 0 {
if len(machineRequest.NTPServers) < 3 || len(machineRequest.NTPServers) > 5 {
return nil, errors.New("please specify a minimum of 3 and a maximum of 5 ntp servers")
}
ntpServers = machineRequest.NTPServers
} else {
ntpServers = partition.NTPServers
}

for _, ntpserver := range ntpServers {
if net.ParseIP(ntpserver.Address) != nil {
_, err := netip.ParseAddr(ntpserver.Address)
if err != nil {
return nil, fmt.Errorf("ip: %s for ntp server not correct err: %w", ntpserver, err)
}
} else {
if !govalidator.IsDNSName(ntpserver.Address) {
return nil, fmt.Errorf("dns name: %s for ntp server not correct err: %w", ntpserver, err)
}
}
}

return &machineAllocationSpec{
Creator: user.EMail,
UUID: uuid,
Expand All @@ -1164,6 +1213,8 @@ func createMachineAllocationSpec(ds *datastore.RethinkStore, machineRequest v1.M
PlacementTags: machineRequest.PlacementTags,
EgressRules: egress,
IngressRules: ingress,
DNSServers: dnsServers,
NTPServers: ntpServers,
}, nil
}

Expand Down Expand Up @@ -1247,6 +1298,8 @@ func allocateMachine(ctx context.Context, logger *slog.Logger, ds *datastore.Ret
VPN: allocationSpec.VPN,
FirewallRules: firewallRules,
UUID: uuid.New().String(),
DNSServers: allocationSpec.DNSServers,
NTPServers: allocationSpec.NTPServers,
}
rollbackOnError := func(err error) error {
if err != nil {
Expand Down
12 changes: 12 additions & 0 deletions cmd/metal-api/internal/service/partition-service.go
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,16 @@ func (r *partitionResource) createPartition(request *restful.Request, response *
commandLine = *requestPayload.PartitionBootConfiguration.CommandLine
}

var dnsServers metal.DNSServers
if len(requestPayload.DNSServers) > 0 {
dnsServers = requestPayload.DNSServers
}

var ntpServers metal.NTPServers
if len(requestPayload.NTPServers) > 0 {
ntpServers = requestPayload.NTPServers
}
Copy link
Contributor

@Gerrit91 Gerrit91 Nov 7, 2024

Choose a reason for hiding this comment

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

Same validations as in the machine spec should be implemented here. Otherwise machines might default to invalid partition defaults.

Maybe you can factor out the validation into a dedicated func?


p := &metal.Partition{
Base: metal.Base{
ID: requestPayload.ID,
Expand All @@ -219,6 +229,8 @@ func (r *partitionResource) createPartition(request *restful.Request, response *
KernelURL: kernelURL,
CommandLine: commandLine,
},
DNSServers: dnsServers,
NTPServers: ntpServers,
}

fqn := metal.TopicMachine.GetFQN(p.GetID())
Expand Down
6 changes: 6 additions & 0 deletions cmd/metal-api/internal/service/v1/machine.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ type MachineAllocation struct {
VPN *MachineVPN `json:"vpn" description:"vpn connection info for machine" optional:"true"`
AllocationUUID string `json:"allocationuuid" description:"a unique identifier for this machine allocation, can be used to distinguish between machine allocations over time."`
FirewallRules *FirewallRules `json:"firewall_rules,omitempty" description:"a set of firewall rules to apply" optional:"true"`
DNSServers metal.DNSServers `json:"dns_servers,omitempty" description:"the dns servers used for the machine" optional:"true"`
NTPServers metal.NTPServers `json:"ntp_servers,omitempty" description:"the ntp servers used for the machine" optional:"true"`
}

type FirewallRules struct {
Expand Down Expand Up @@ -229,6 +231,8 @@ type MachineAllocateRequest struct {
Networks MachineAllocationNetworks `json:"networks" description:"the networks that this machine will be placed in." optional:"true"`
IPs []string `json:"ips" description:"the ips to attach to this machine additionally" optional:"true"`
PlacementTags []string `json:"placement_tags,omitempty" description:"by default machines are spread across the racks inside a partition for every project. if placement tags are provided, the machine candidate has an additional anti-affinity to other machines having the same tags"`
DNSServers metal.DNSServers `json:"dns_servers,omitempty" description:"the dns servers used for the machine" optional:"true"`
NTPServers metal.NTPServers `json:"ntp_servers,omitempty" description:"the ntp servers used for the machine" optional:"true"`
}

type MachineAllocationNetworks []MachineAllocationNetwork
Expand Down Expand Up @@ -597,6 +601,8 @@ func NewMachineResponse(m *metal.Machine, s *metal.Size, p *metal.Partition, i *
VPN: NewMachineVPN(m.Allocation.VPN),
AllocationUUID: m.Allocation.UUID,
FirewallRules: firewallRules,
DNSServers: m.Allocation.DNSServers,
NTPServers: m.Allocation.NTPServers,
}

allocation.Reinstall = m.Allocation.Reinstall
Expand Down
4 changes: 4 additions & 0 deletions cmd/metal-api/internal/service/v1/partition.go
majst01 marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ type PartitionBase struct {
MgmtServiceAddress *string `json:"mgmtserviceaddress" description:"the address to the management service of this partition" optional:"true"`
PrivateNetworkPrefixLength *int `json:"privatenetworkprefixlength" description:"the length of private networks for the machine's child networks in this partition, default 22" optional:"true" minimum:"16" maximum:"30"`
Labels map[string]string `json:"labels" description:"free labels that you associate with this partition" optional:"true"`
DNSServers metal.DNSServers `json:"dns_servers" description:"the dns servers for this partition" optional:"true"`
NTPServers metal.NTPServers `json:"ntp_servers" description:"the ntp servers for this partition" optional:"true"`
}

type PartitionBootConfiguration struct {
Expand Down Expand Up @@ -117,6 +119,8 @@ func NewPartitionResponse(p *metal.Partition) *PartitionResponse {
PartitionBase: PartitionBase{
MgmtServiceAddress: &p.MgmtServiceAddress,
PrivateNetworkPrefixLength: &prefixLength,
DNSServers: p.DNSServers,
NTPServers: p.NTPServers,
},
PartitionBootConfiguration: PartitionBootConfiguration{
ImageURL: &p.BootConfiguration.ImageURL,
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ require (
github.com/Azure/go-ansiterm v0.0.0-20230124172434-306776ec8161 // indirect
github.com/Microsoft/go-winio v0.6.2 // indirect
github.com/andybalholm/brotli v1.1.0 // indirect
github.com/asaskevich/govalidator v0.0.0-20230301143203-a9d515a09cc2 // indirect
github.com/asaskevich/govalidator v0.0.0-20230301143203-a9d515a09cc2
github.com/beorn7/perks v1.0.1 // indirect
github.com/cenkalti/backoff/v4 v4.3.0 // indirect
github.com/cespare/xxhash/v2 v2.3.0 // indirect
Expand Down
Loading
Loading