From df41e532fe5b71166d9402a8f7357b1166279599 Mon Sep 17 00:00:00 2001 From: Simon Mayer Date: Wed, 11 Sep 2024 14:42:01 +0200 Subject: [PATCH 01/21] Configure DNS servers in machine allocation optionally --- cmd/metal-api/internal/metal/machine.go | 1 + cmd/metal-api/internal/service/machine-service.go | 3 +++ cmd/metal-api/internal/service/v1/machine.go | 2 ++ 3 files changed, 6 insertions(+) diff --git a/cmd/metal-api/internal/metal/machine.go b/cmd/metal-api/internal/metal/machine.go index 0f6eabfa..225b80bc 100644 --- a/cmd/metal-api/internal/metal/machine.go +++ b/cmd/metal-api/internal/metal/machine.go @@ -153,6 +153,7 @@ type MachineAllocation struct { VPN *MachineVPN `rethinkdb:"vpn" json:"vpn"` UUID string `rethinkdb:"uuid" json:"uuid"` FirewallRules *FirewallRules `rethinkdb:"firewall_rules" json:"firewall_rules"` + DNSServers []string `rethinkdb:"dns_servers" json:"dns_servers"` } type FirewallRules struct { diff --git a/cmd/metal-api/internal/service/machine-service.go b/cmd/metal-api/internal/service/machine-service.go index a9511218..b1963b65 100644 --- a/cmd/metal-api/internal/service/machine-service.go +++ b/cmd/metal-api/internal/service/machine-service.go @@ -78,6 +78,7 @@ type machineAllocationSpec struct { PlacementTags []string EgressRules []metal.EgressRule IngressRules []metal.IngressRule + DNSServers []string } // allocationNetwork is intermediate struct to create machine networks from regular networks during machine allocation @@ -1154,6 +1155,7 @@ func createMachineAllocationSpec(ds *datastore.RethinkStore, machineRequest v1.M PlacementTags: machineRequest.PlacementTags, EgressRules: egress, IngressRules: ingress, + DNSServers: machineRequest.DNSServers, }, nil } @@ -1237,6 +1239,7 @@ func allocateMachine(ctx context.Context, logger *slog.Logger, ds *datastore.Ret VPN: allocationSpec.VPN, FirewallRules: firewallRules, UUID: uuid.New().String(), + DNSServers: allocationSpec.DNSServers, } rollbackOnError := func(err error) error { if err != nil { diff --git a/cmd/metal-api/internal/service/v1/machine.go b/cmd/metal-api/internal/service/v1/machine.go index 13b92436..2a4237d6 100644 --- a/cmd/metal-api/internal/service/v1/machine.go +++ b/cmd/metal-api/internal/service/v1/machine.go @@ -45,6 +45,7 @@ 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 []string `json:"dns_servers,omitempty" description:"the dns servers used for the machine" optional:"true"` } type FirewallRules struct { @@ -218,6 +219,7 @@ 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 []string `json:"dns_servers,omitempty" description:"the dns servers used for the machine" optional:"true"` } type MachineAllocationNetworks []MachineAllocationNetwork From 4a18873057f13f34d2308d22755afd684ab49294 Mon Sep 17 00:00:00 2001 From: Simon Mayer Date: Wed, 11 Sep 2024 15:08:10 +0200 Subject: [PATCH 02/21] Return DNSServers in MachineResponse --- cmd/metal-api/internal/service/v1/machine.go | 1 + 1 file changed, 1 insertion(+) diff --git a/cmd/metal-api/internal/service/v1/machine.go b/cmd/metal-api/internal/service/v1/machine.go index 2a4237d6..f8c1c8e3 100644 --- a/cmd/metal-api/internal/service/v1/machine.go +++ b/cmd/metal-api/internal/service/v1/machine.go @@ -567,6 +567,7 @@ 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, } allocation.Reinstall = m.Allocation.Reinstall From ffa88777b442275a5abc8ab29b8c69a7869f03e1 Mon Sep 17 00:00:00 2001 From: Simon Mayer Date: Fri, 13 Sep 2024 10:58:21 +0200 Subject: [PATCH 03/21] Add ntp servers field --- cmd/metal-api/internal/metal/machine.go | 1 + cmd/metal-api/internal/service/v1/machine.go | 3 +++ 2 files changed, 4 insertions(+) diff --git a/cmd/metal-api/internal/metal/machine.go b/cmd/metal-api/internal/metal/machine.go index 225b80bc..8ce443c8 100644 --- a/cmd/metal-api/internal/metal/machine.go +++ b/cmd/metal-api/internal/metal/machine.go @@ -154,6 +154,7 @@ type MachineAllocation struct { UUID string `rethinkdb:"uuid" json:"uuid"` FirewallRules *FirewallRules `rethinkdb:"firewall_rules" json:"firewall_rules"` DNSServers []string `rethinkdb:"dns_servers" json:"dns_servers"` + NTPServers []string `rethinkdb:"ntp_servers" json:"ntp_servers"` } type FirewallRules struct { diff --git a/cmd/metal-api/internal/service/v1/machine.go b/cmd/metal-api/internal/service/v1/machine.go index f8c1c8e3..3e46cbb7 100644 --- a/cmd/metal-api/internal/service/v1/machine.go +++ b/cmd/metal-api/internal/service/v1/machine.go @@ -46,6 +46,7 @@ type MachineAllocation struct { 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 []string `json:"dns_servers,omitempty" description:"the dns servers used for the machine" optional:"true"` + NTPServers []string `json:"ntp_servers,omitempty" description:"the ntp servers used for the machine" optional:"true"` } type FirewallRules struct { @@ -220,6 +221,7 @@ type MachineAllocateRequest struct { 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 []string `json:"dns_servers,omitempty" description:"the dns servers used for the machine" optional:"true"` + NTPServers []string `json:"ntp_servers,omitempty" description:"the ntp servers used for the machine" optional:"true"` } type MachineAllocationNetworks []MachineAllocationNetwork @@ -568,6 +570,7 @@ func NewMachineResponse(m *metal.Machine, s *metal.Size, p *metal.Partition, i * AllocationUUID: m.Allocation.UUID, FirewallRules: firewallRules, DNSServers: m.Allocation.DNSServers, + NTPServers: m.Allocation.NTPServers, } allocation.Reinstall = m.Allocation.Reinstall From 77f0843d51db7aaceae582615f8148ea69ce227e Mon Sep 17 00:00:00 2001 From: Simon Mayer Date: Fri, 13 Sep 2024 10:58:38 +0200 Subject: [PATCH 04/21] NTP and DNS validation --- .../internal/service/machine-service.go | 31 +++++++++++++++++++ go.mod | 2 +- 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/cmd/metal-api/internal/service/machine-service.go b/cmd/metal-api/internal/service/machine-service.go index b1963b65..e20b48fb 100644 --- a/cmd/metal-api/internal/service/machine-service.go +++ b/cmd/metal-api/internal/service/machine-service.go @@ -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" @@ -79,6 +81,7 @@ type machineAllocationSpec struct { EgressRules []metal.EgressRule IngressRules []metal.IngressRule DNSServers []string + NTPServers []string } // allocationNetwork is intermediate struct to create machine networks from regular networks during machine allocation @@ -1134,6 +1137,32 @@ func createMachineAllocationSpec(ds *datastore.RethinkStore, machineRequest v1.M return nil, fmt.Errorf("size:%s not found err:%w", sizeID, err) } + if len(machineRequest.DNSServers) > 3 { + return nil, errors.New("please specify a maximum of three dns servers") + } + for _, dnsip := range machineRequest.DNSServers { + _, err := netip.ParseAddr(dnsip) + if err != nil { + return nil, fmt.Errorf("IP: %s for DNS server not correct err: %w", dnsip, err) + } + } + + 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") + } + for _, ntpserver := range machineRequest.NTPServers { + if net.ParseIP(ntpserver) != nil { + _, err := netip.ParseAddr(ntpserver) + if err != nil { + return nil, fmt.Errorf("IP: %s for NTP server not correct err: %w", ntpserver, err) + } + } else { + if !govalidator.IsDNSName(ntpserver) { + return nil, fmt.Errorf("DNS name: %s for NTP server not correct err: %w", ntpserver, err) + } + } + } + return &machineAllocationSpec{ Creator: user.EMail, UUID: uuid, @@ -1156,6 +1185,7 @@ func createMachineAllocationSpec(ds *datastore.RethinkStore, machineRequest v1.M EgressRules: egress, IngressRules: ingress, DNSServers: machineRequest.DNSServers, + NTPServers: machineRequest.NTPServers, }, nil } @@ -1240,6 +1270,7 @@ func allocateMachine(ctx context.Context, logger *slog.Logger, ds *datastore.Ret FirewallRules: firewallRules, UUID: uuid.New().String(), DNSServers: allocationSpec.DNSServers, + NTPServers: allocationSpec.NTPServers, } rollbackOnError := func(err error) error { if err != nil { diff --git a/go.mod b/go.mod index b738718f..94d5b13b 100644 --- a/go.mod +++ b/go.mod @@ -59,7 +59,7 @@ require ( github.com/akutz/memconn v0.1.0 // indirect github.com/alexbrainman/sspi v0.0.0-20231016080023-1a75b4708caa // 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 From cbf9f21e7cda5992d1407187ad68944bee53a2bc Mon Sep 17 00:00:00 2001 From: Simon Mayer Date: Fri, 13 Sep 2024 10:59:00 +0200 Subject: [PATCH 05/21] Regenerate metal-api --- spec/metal-api.json | 42 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/spec/metal-api.json b/spec/metal-api.json index 4c1da80c..78948a6b 100644 --- a/spec/metal-api.json +++ b/spec/metal-api.json @@ -1009,6 +1009,13 @@ "description": "a description for this entity", "type": "string" }, + "dns_servers": { + "description": "the dns servers used for the machine", + "items": { + "type": "string" + }, + "type": "array" + }, "filesystemlayoutid": { "description": "the filesystemlayout id to assign to this machine", "type": "string" @@ -1043,6 +1050,13 @@ }, "type": "array" }, + "ntp_servers": { + "description": "the ntp servers used for the machine", + "items": { + "type": "string" + }, + "type": "array" + }, "partitionid": { "description": "the partition id to assign this machine to", "type": "string" @@ -1984,6 +1998,13 @@ "description": "a description for this entity", "type": "string" }, + "dns_servers": { + "description": "the dns servers used for the machine", + "items": { + "type": "string" + }, + "type": "array" + }, "filesystemlayoutid": { "description": "the filesystemlayout id to assign to this machine", "type": "string" @@ -2014,6 +2035,13 @@ }, "type": "array" }, + "ntp_servers": { + "description": "the ntp servers used for the machine", + "items": { + "type": "string" + }, + "type": "array" + }, "partitionid": { "description": "the partition id to assign this machine to", "type": "string" @@ -2087,6 +2115,13 @@ "description": "a description for this machine", "type": "string" }, + "dns_servers": { + "description": "the dns servers used for the machine", + "items": { + "type": "string" + }, + "type": "array" + }, "filesystemlayout": { "$ref": "#/definitions/v1.FilesystemLayoutResponse", "description": "filesystemlayout to create on this machine" @@ -2115,6 +2150,13 @@ }, "type": "array" }, + "ntp_servers": { + "description": "the ntp servers used for the machine", + "items": { + "type": "string" + }, + "type": "array" + }, "project": { "description": "the project id that this machine is assigned to", "type": "string" From fe4519232f99de70a2e10a34ad3aeac73c07e80c Mon Sep 17 00:00:00 2001 From: Simon Mayer Date: Thu, 17 Oct 2024 13:42:39 +0200 Subject: [PATCH 06/21] Use new struct for DNS and NTP servers --- cmd/metal-api/internal/metal/machine.go | 16 ++++++++-- .../internal/service/machine-service.go | 12 +++---- cmd/metal-api/internal/service/v1/machine.go | 8 ++--- spec/metal-api.json | 32 +++++++++++++++---- 4 files changed, 50 insertions(+), 18 deletions(-) diff --git a/cmd/metal-api/internal/metal/machine.go b/cmd/metal-api/internal/metal/machine.go index 86597355..083d3cf1 100644 --- a/cmd/metal-api/internal/metal/machine.go +++ b/cmd/metal-api/internal/metal/machine.go @@ -153,8 +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 []string `rethinkdb:"dns_servers" json:"dns_servers"` - NTPServers []string `rethinkdb:"ntp_servers" json:"ntp_servers"` + DNSServers DNSServers `rethinkdb:"dns_servers" json:"dns_servers"` + NTPServers NTPServers `rethinkdb:"ntp_servers" json:"ntp_servers"` } type FirewallRules struct { @@ -177,6 +177,18 @@ type IngressRule struct { Comment string `rethinkdb:"comment" json:"comment"` } +type DNSServers []DNSServer + +type DNSServer struct { + IP string `rethinkdb:"ip" json:"ip"` +} + +type NTPServers []NTPServer + +type NTPServer struct { + Address string `address:"address" json:"address"` +} + type Protocol string const ( diff --git a/cmd/metal-api/internal/service/machine-service.go b/cmd/metal-api/internal/service/machine-service.go index 4c85c96a..8cb5c73b 100644 --- a/cmd/metal-api/internal/service/machine-service.go +++ b/cmd/metal-api/internal/service/machine-service.go @@ -80,8 +80,8 @@ type machineAllocationSpec struct { PlacementTags []string EgressRules []metal.EgressRule IngressRules []metal.IngressRule - DNSServers []string - NTPServers []string + DNSServers metal.DNSServers + NTPServers metal.NTPServers } // allocationNetwork is intermediate struct to create machine networks from regular networks during machine allocation @@ -1151,7 +1151,7 @@ func createMachineAllocationSpec(ds *datastore.RethinkStore, machineRequest v1.M return nil, errors.New("please specify a maximum of three dns servers") } for _, dnsip := range machineRequest.DNSServers { - _, err := netip.ParseAddr(dnsip) + _, err := netip.ParseAddr(dnsip.IP) if err != nil { return nil, fmt.Errorf("IP: %s for DNS server not correct err: %w", dnsip, err) } @@ -1161,13 +1161,13 @@ func createMachineAllocationSpec(ds *datastore.RethinkStore, machineRequest v1.M return nil, errors.New("please specify a minimum of 3 and a maximum of 5 ntp servers") } for _, ntpserver := range machineRequest.NTPServers { - if net.ParseIP(ntpserver) != nil { - _, err := netip.ParseAddr(ntpserver) + 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) { + if !govalidator.IsDNSName(ntpserver.Address) { return nil, fmt.Errorf("DNS name: %s for NTP server not correct err: %w", ntpserver, err) } } diff --git a/cmd/metal-api/internal/service/v1/machine.go b/cmd/metal-api/internal/service/v1/machine.go index 07de84cd..4d83a375 100644 --- a/cmd/metal-api/internal/service/v1/machine.go +++ b/cmd/metal-api/internal/service/v1/machine.go @@ -45,8 +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 []string `json:"dns_servers,omitempty" description:"the dns servers used for the machine" optional:"true"` - NTPServers []string `json:"ntp_servers,omitempty" description:"the ntp servers used for the machine" 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 { @@ -231,8 +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 []string `json:"dns_servers,omitempty" description:"the dns servers used for the machine" optional:"true"` - NTPServers []string `json:"ntp_servers,omitempty" description:"the ntp servers used for the machine" 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 MachineAllocationNetworks []MachineAllocationNetwork diff --git a/spec/metal-api.json b/spec/metal-api.json index d1485386..577dad30 100644 --- a/spec/metal-api.json +++ b/spec/metal-api.json @@ -348,6 +348,26 @@ "type": "HTTPErrorResponse" } }, + "metal.DNSServer": { + "properties": { + "ip": { + "type": "string" + } + }, + "required": [ + "ip" + ] + }, + "metal.NTPServer": { + "properties": { + "address": { + "type": "string" + } + }, + "required": [ + "address" + ] + }, "rest.HealthResponse": { "properties": { "message": { @@ -1012,7 +1032,7 @@ "dns_servers": { "description": "the dns servers used for the machine", "items": { - "type": "string" + "$ref": "#/definitions/metal.DNSServer" }, "type": "array" }, @@ -1053,7 +1073,7 @@ "ntp_servers": { "description": "the ntp servers used for the machine", "items": { - "type": "string" + "$ref": "#/definitions/metal.NTPServer" }, "type": "array" }, @@ -2001,7 +2021,7 @@ "dns_servers": { "description": "the dns servers used for the machine", "items": { - "type": "string" + "$ref": "#/definitions/metal.DNSServer" }, "type": "array" }, @@ -2038,7 +2058,7 @@ "ntp_servers": { "description": "the ntp servers used for the machine", "items": { - "type": "string" + "$ref": "#/definitions/metal.NTPServer" }, "type": "array" }, @@ -2118,7 +2138,7 @@ "dns_servers": { "description": "the dns servers used for the machine", "items": { - "type": "string" + "$ref": "#/definitions/metal.DNSServer" }, "type": "array" }, @@ -2153,7 +2173,7 @@ "ntp_servers": { "description": "the ntp servers used for the machine", "items": { - "type": "string" + "$ref": "#/definitions/metal.NTPServer" }, "type": "array" }, From 9973d7cc93b1eb765e8609b9597acca097375cb4 Mon Sep 17 00:00:00 2001 From: Simon Mayer Date: Tue, 22 Oct 2024 15:55:04 +0200 Subject: [PATCH 07/21] Check for empty ntp servers configuration --- cmd/metal-api/internal/service/machine-service.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/metal-api/internal/service/machine-service.go b/cmd/metal-api/internal/service/machine-service.go index 8cb5c73b..f65ca8b0 100644 --- a/cmd/metal-api/internal/service/machine-service.go +++ b/cmd/metal-api/internal/service/machine-service.go @@ -1157,7 +1157,7 @@ func createMachineAllocationSpec(ds *datastore.RethinkStore, machineRequest v1.M } } - if len(machineRequest.NTPServers) <= 3 || len(machineRequest.NTPServers) > 5 { + if len(machineRequest.NTPServers) > 0 && (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") } for _, ntpserver := range machineRequest.NTPServers { From f57775b8ced7a194eee1a605ed60465985c852ee Mon Sep 17 00:00:00 2001 From: Simon Mayer Date: Tue, 22 Oct 2024 15:55:39 +0200 Subject: [PATCH 08/21] Add NTP and DNS server field to partition --- cmd/metal-api/internal/metal/partition.go | 2 ++ cmd/metal-api/internal/service/partition-service.go | 12 ++++++++++++ cmd/metal-api/internal/service/v1/partition.go | 2 ++ 3 files changed, 16 insertions(+) diff --git a/cmd/metal-api/internal/metal/partition.go b/cmd/metal-api/internal/metal/partition.go index 2db7c2f8..288addc8 100644 --- a/cmd/metal-api/internal/metal/partition.go +++ b/cmd/metal-api/internal/metal/partition.go @@ -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"` + DNSServer DNSServers `rethinkdb:"dns_servers" json:"dns_servers"` + NTPServers NTPServers `rethinkdb:"ntp_servers" json:"ntp_servers"` } // BootConfiguration defines the metal-hammer initrd, kernel and commandline diff --git a/cmd/metal-api/internal/service/partition-service.go b/cmd/metal-api/internal/service/partition-service.go index 5f5dfd7c..c43c2512 100644 --- a/cmd/metal-api/internal/service/partition-service.go +++ b/cmd/metal-api/internal/service/partition-service.go @@ -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 + } + p := &metal.Partition{ Base: metal.Base{ ID: requestPayload.ID, @@ -219,6 +229,8 @@ func (r *partitionResource) createPartition(request *restful.Request, response * KernelURL: kernelURL, CommandLine: commandLine, }, + DNSServer: dnsServers, + NTPServers: ntpServers, } fqn := metal.TopicMachine.GetFQN(p.GetID()) diff --git a/cmd/metal-api/internal/service/v1/partition.go b/cmd/metal-api/internal/service/v1/partition.go index f3304241..10adf135 100644 --- a/cmd/metal-api/internal/service/v1/partition.go +++ b/cmd/metal-api/internal/service/v1/partition.go @@ -20,6 +20,8 @@ type PartitionCreateRequest struct { Common PartitionBase PartitionBootConfiguration PartitionBootConfiguration `json:"bootconfig" description:"the boot configuration of this partition"` + 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 PartitionUpdateRequest struct { From 7abf175a09c99d99d18b8a96e8b1fb8c59f7a5ad Mon Sep 17 00:00:00 2001 From: Simon Mayer Date: Tue, 22 Oct 2024 15:57:40 +0200 Subject: [PATCH 09/21] Add new metal-api JSON --- spec/metal-api.json | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/spec/metal-api.json b/spec/metal-api.json index 577dad30..c8df2c7a 100644 --- a/spec/metal-api.json +++ b/spec/metal-api.json @@ -4171,6 +4171,13 @@ "description": "a description for this entity", "type": "string" }, + "dns_servers": { + "description": "the dns servers for this partition", + "items": { + "$ref": "#/definitions/metal.DNSServer" + }, + "type": "array" + }, "id": { "description": "the unique ID of this entity", "type": "string" @@ -4190,6 +4197,13 @@ "description": "a readable name for this entity", "type": "string" }, + "ntp_servers": { + "description": "the ntp servers for this partition", + "items": { + "$ref": "#/definitions/metal.NTPServer" + }, + "type": "array" + }, "privatenetworkprefixlength": { "description": "the length of private networks for the machine's child networks in this partition, default 22", "format": "int32", From fd583028be889d7291d9456f31b8b31988f3cccc Mon Sep 17 00:00:00 2001 From: Simon Mayer Date: Wed, 23 Oct 2024 11:48:54 +0200 Subject: [PATCH 10/21] Add dns and ntp servers to partition base to include it in response --- .../internal/service/v1/partition.go | 4 +-- spec/metal-api.json | 28 +++++++++++++++++++ 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/cmd/metal-api/internal/service/v1/partition.go b/cmd/metal-api/internal/service/v1/partition.go index 10adf135..7c07def5 100644 --- a/cmd/metal-api/internal/service/v1/partition.go +++ b/cmd/metal-api/internal/service/v1/partition.go @@ -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 { @@ -20,8 +22,6 @@ type PartitionCreateRequest struct { Common PartitionBase PartitionBootConfiguration PartitionBootConfiguration `json:"bootconfig" description:"the boot configuration of this partition"` - 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 PartitionUpdateRequest struct { diff --git a/spec/metal-api.json b/spec/metal-api.json index c8df2c7a..24cb3fa4 100644 --- a/spec/metal-api.json +++ b/spec/metal-api.json @@ -4085,6 +4085,13 @@ }, "v1.PartitionBase": { "properties": { + "dns_servers": { + "description": "the dns servers for this partition", + "items": { + "$ref": "#/definitions/metal.DNSServer" + }, + "type": "array" + }, "labels": { "additionalProperties": { "type": "string" @@ -4096,6 +4103,13 @@ "description": "the address to the management service of this partition", "type": "string" }, + "ntp_servers": { + "description": "the ntp servers for this partition", + "items": { + "$ref": "#/definitions/metal.NTPServer" + }, + "type": "array" + }, "privatenetworkprefixlength": { "description": "the length of private networks for the machine's child networks in this partition, default 22", "format": "int32", @@ -4239,6 +4253,13 @@ "description": "a description for this entity", "type": "string" }, + "dns_servers": { + "description": "the dns servers for this partition", + "items": { + "$ref": "#/definitions/metal.DNSServer" + }, + "type": "array" + }, "id": { "description": "the unique ID of this entity", "type": "string" @@ -4258,6 +4279,13 @@ "description": "a readable name for this entity", "type": "string" }, + "ntp_servers": { + "description": "the ntp servers for this partition", + "items": { + "$ref": "#/definitions/metal.NTPServer" + }, + "type": "array" + }, "privatenetworkprefixlength": { "description": "the length of private networks for the machine's child networks in this partition, default 22", "format": "int32", From a8f91114a4b52b243ec4f1041b01a4a0895baa58 Mon Sep 17 00:00:00 2001 From: Simon Mayer Date: Wed, 23 Oct 2024 13:42:11 +0200 Subject: [PATCH 11/21] Add DNS and NTP servers to fix integration test --- .../internal/datastore/machine_integration_test.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/cmd/metal-api/internal/datastore/machine_integration_test.go b/cmd/metal-api/internal/datastore/machine_integration_test.go index b95dc5ff..e45a7681 100644 --- a/cmd/metal-api/internal/datastore/machine_integration_test.go +++ b/cmd/metal-api/internal/datastore/machine_integration_test.go @@ -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 { From 21bf0c5210d36f46314720adf39c3a1326c5633e Mon Sep 17 00:00:00 2001 From: Simon Mayer Date: Thu, 24 Oct 2024 08:06:19 +0200 Subject: [PATCH 12/21] Fix typo --- cmd/metal-api/internal/metal/partition.go | 2 +- cmd/metal-api/internal/service/partition-service.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/metal-api/internal/metal/partition.go b/cmd/metal-api/internal/metal/partition.go index 288addc8..de732c04 100644 --- a/cmd/metal-api/internal/metal/partition.go +++ b/cmd/metal-api/internal/metal/partition.go @@ -7,7 +7,7 @@ type Partition struct { MgmtServiceAddress string `rethinkdb:"mgmtserviceaddr" json:"mgmtserviceaddr"` PrivateNetworkPrefixLength uint8 `rethinkdb:"privatenetworkprefixlength" json:"privatenetworkprefixlength"` Labels map[string]string `rethinkdb:"labels" json:"labels"` - DNSServer DNSServers `rethinkdb:"dns_servers" json:"dns_servers"` + DNSServers DNSServers `rethinkdb:"dns_servers" json:"dns_servers"` NTPServers NTPServers `rethinkdb:"ntp_servers" json:"ntp_servers"` } diff --git a/cmd/metal-api/internal/service/partition-service.go b/cmd/metal-api/internal/service/partition-service.go index c43c2512..5172490e 100644 --- a/cmd/metal-api/internal/service/partition-service.go +++ b/cmd/metal-api/internal/service/partition-service.go @@ -229,7 +229,7 @@ func (r *partitionResource) createPartition(request *restful.Request, response * KernelURL: kernelURL, CommandLine: commandLine, }, - DNSServer: dnsServers, + DNSServers: dnsServers, NTPServers: ntpServers, } From 6a6bf09f8470b25501d4dc5c8d30e338c965fc11 Mon Sep 17 00:00:00 2001 From: Simon Mayer Date: Thu, 24 Oct 2024 08:07:06 +0200 Subject: [PATCH 13/21] Set partition default for machine allocation spec --- .../internal/service/machine-service.go | 35 ++++++++++++++----- 1 file changed, 27 insertions(+), 8 deletions(-) diff --git a/cmd/metal-api/internal/service/machine-service.go b/cmd/metal-api/internal/service/machine-service.go index f65ca8b0..af8ee715 100644 --- a/cmd/metal-api/internal/service/machine-service.go +++ b/cmd/metal-api/internal/service/machine-service.go @@ -1147,20 +1147,39 @@ func createMachineAllocationSpec(ds *datastore.RethinkStore, machineRequest v1.M return nil, fmt.Errorf("size:%s not found err:%w", sizeID, err) } - if len(machineRequest.DNSServers) > 3 { - return nil, errors.New("please specify a maximum of three dns servers") + partition, err := ds.FindPartition(partitionID) + if err != nil { + return nil, fmt.Errorf("partition:%s not found err:%w", partitionID, err) } - for _, dnsip := range machineRequest.DNSServers { + var ( + dnsServers metal.DNSServers + ntpServers metal.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 _, dnsip := range dnsServers { _, err := netip.ParseAddr(dnsip.IP) if err != nil { return nil, fmt.Errorf("IP: %s for DNS server not correct err: %w", dnsip, err) } } - if len(machineRequest.NTPServers) > 0 && (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") + 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 machineRequest.NTPServers { + + for _, ntpserver := range ntpServers { if net.ParseIP(ntpserver.Address) != nil { _, err := netip.ParseAddr(ntpserver.Address) if err != nil { @@ -1194,8 +1213,8 @@ func createMachineAllocationSpec(ds *datastore.RethinkStore, machineRequest v1.M PlacementTags: machineRequest.PlacementTags, EgressRules: egress, IngressRules: ingress, - DNSServers: machineRequest.DNSServers, - NTPServers: machineRequest.NTPServers, + DNSServers: dnsServers, + NTPServers: ntpServers, }, nil } From e82d7d0ecc3eb103d3b214eae8be7cc8c926e728 Mon Sep 17 00:00:00 2001 From: Simon Mayer Date: Thu, 24 Oct 2024 08:34:28 +0200 Subject: [PATCH 14/21] Add DNS and NTP servers to partition response --- cmd/metal-api/internal/service/v1/partition.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/cmd/metal-api/internal/service/v1/partition.go b/cmd/metal-api/internal/service/v1/partition.go index 7c07def5..531d8fbb 100644 --- a/cmd/metal-api/internal/service/v1/partition.go +++ b/cmd/metal-api/internal/service/v1/partition.go @@ -119,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, From 2df7e17d4fa1d53bfdae14e9e7703fa8c58e166c Mon Sep 17 00:00:00 2001 From: Simon Mayer Date: Mon, 28 Oct 2024 16:02:16 +0100 Subject: [PATCH 15/21] Fix: Specify a minimum of three NTP servers --- cmd/metal-api/internal/service/machine-service.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/metal-api/internal/service/machine-service.go b/cmd/metal-api/internal/service/machine-service.go index af8ee715..40f02158 100644 --- a/cmd/metal-api/internal/service/machine-service.go +++ b/cmd/metal-api/internal/service/machine-service.go @@ -1171,7 +1171,7 @@ func createMachineAllocationSpec(ds *datastore.RethinkStore, machineRequest v1.M } if len(machineRequest.NTPServers) != 0 { - if len(machineRequest.NTPServers) <= 3 || len(machineRequest.NTPServers) > 5 { + 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 From 451813fc85859c02b60a18eea7220b14de4450f0 Mon Sep 17 00:00:00 2001 From: Simon Mayer Date: Tue, 29 Oct 2024 08:26:00 +0100 Subject: [PATCH 16/21] Integrate review --- cmd/metal-api/internal/metal/machine.go | 4 ++-- cmd/metal-api/internal/service/machine-service.go | 10 +++++----- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/cmd/metal-api/internal/metal/machine.go b/cmd/metal-api/internal/metal/machine.go index 083d3cf1..32254971 100644 --- a/cmd/metal-api/internal/metal/machine.go +++ b/cmd/metal-api/internal/metal/machine.go @@ -180,13 +180,13 @@ type IngressRule struct { type DNSServers []DNSServer type DNSServer struct { - IP string `rethinkdb:"ip" json:"ip"` + 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"` + Address string `address:"address" json:"address" description:"ip address or dns hostname of this ntp server"` } type Protocol string diff --git a/cmd/metal-api/internal/service/machine-service.go b/cmd/metal-api/internal/service/machine-service.go index 40f02158..4c86f966 100644 --- a/cmd/metal-api/internal/service/machine-service.go +++ b/cmd/metal-api/internal/service/machine-service.go @@ -1163,10 +1163,10 @@ func createMachineAllocationSpec(ds *datastore.RethinkStore, machineRequest v1.M } else { dnsServers = partition.DNSServers } - for _, dnsip := range dnsServers { - _, err := netip.ParseAddr(dnsip.IP) + 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", dnsip, err) + return nil, fmt.Errorf("ip: %s for dns server not correct err: %w", dnsServer, err) } } @@ -1183,11 +1183,11 @@ func createMachineAllocationSpec(ds *datastore.RethinkStore, machineRequest v1.M 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) + 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 nil, fmt.Errorf("dns name: %s for ntp server not correct err: %w", ntpserver, err) } } } From b5b0adcbf90b9a7462e1af15447297870bbd0483 Mon Sep 17 00:00:00 2001 From: Simon Mayer Date: Tue, 29 Oct 2024 08:38:52 +0100 Subject: [PATCH 17/21] Regenerate spec --- spec/metal-api.json | 2 ++ 1 file changed, 2 insertions(+) diff --git a/spec/metal-api.json b/spec/metal-api.json index 24cb3fa4..6f9e7ded 100644 --- a/spec/metal-api.json +++ b/spec/metal-api.json @@ -351,6 +351,7 @@ "metal.DNSServer": { "properties": { "ip": { + "description": "ip address of this dns server", "type": "string" } }, @@ -361,6 +362,7 @@ "metal.NTPServer": { "properties": { "address": { + "description": "ip address or dns hostname of this ntp server", "type": "string" } }, From b5236c3ca4bff1a0c5c706c91f150372ece0987e Mon Sep 17 00:00:00 2001 From: Simon Mayer Date: Fri, 8 Nov 2024 08:29:10 +0100 Subject: [PATCH 18/21] Implement review --- .../internal/service/machine-service.go | 55 +++++++++++-------- .../internal/service/partition-service.go | 18 ++++-- 2 files changed, 47 insertions(+), 26 deletions(-) diff --git a/cmd/metal-api/internal/service/machine-service.go b/cmd/metal-api/internal/service/machine-service.go index 4c86f966..78e098b7 100644 --- a/cmd/metal-api/internal/service/machine-service.go +++ b/cmd/metal-api/internal/service/machine-service.go @@ -1152,22 +1152,18 @@ func createMachineAllocationSpec(ds *datastore.RethinkStore, machineRequest v1.M return nil, fmt.Errorf("partition:%s not found err:%w", partitionID, err) } var ( - 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 err := ValidateDNSServers(dnsServers); err != nil { + return nil, err } if len(machineRequest.NTPServers) != 0 { @@ -1175,21 +1171,10 @@ func createMachineAllocationSpec(ds *datastore.RethinkStore, machineRequest v1.M 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) - } - } + if err := ValidateNTPServers(ntpServers); err != nil { + return nil, err } return &machineAllocationSpec{ @@ -2527,3 +2512,29 @@ func (s machineAllocationSpec) noautoNetworkN() int { func (s machineAllocationSpec) autoNetworkN() int { return len(s.Networks) - s.noautoNetworkN() } + +func ValidateDNSServers(d metal.DNSServers) error { + for _, dnsServer := range d { + _, err := netip.ParseAddr(dnsServer.IP) + if err != nil { + return fmt.Errorf("ip: %s for dns server not correct err: %w", dnsServer, err) + } + } + return nil +} + +func ValidateNTPServers(dnsServers metal.NTPServers) error { + for _, ntpserver := range dnsServers { + if net.ParseIP(ntpserver.Address) != nil { + _, err := netip.ParseAddr(ntpserver.Address) + if err != nil { + return fmt.Errorf("ip: %s for ntp server not correct err: %w", ntpserver, err) + } + } else { + if !govalidator.IsDNSName(ntpserver.Address) { + return fmt.Errorf("dns name: %s for ntp server not correct", ntpserver) + } + } + } + return nil +} diff --git a/cmd/metal-api/internal/service/partition-service.go b/cmd/metal-api/internal/service/partition-service.go index 5172490e..d31e8e61 100644 --- a/cmd/metal-api/internal/service/partition-service.go +++ b/cmd/metal-api/internal/service/partition-service.go @@ -206,13 +206,23 @@ func (r *partitionResource) createPartition(request *restful.Request, response * } var dnsServers metal.DNSServers - if len(requestPayload.DNSServers) > 0 { - dnsServers = requestPayload.DNSServers + reqDNSServers := requestPayload.DNSServers + if len(reqDNSServers) > 0 { + if err := ValidateDNSServers(reqDNSServers); err != nil { + r.sendError(request, response, httperrors.BadRequest(err)) + return + } + + dnsServers = reqDNSServers } var ntpServers metal.NTPServers - if len(requestPayload.NTPServers) > 0 { - ntpServers = requestPayload.NTPServers + reqNtpServers := requestPayload.NTPServers + if len(ntpServers) > 0 { + if err := ValidateNTPServers(reqNtpServers); err != nil { + r.sendError(request, response, httperrors.BadRequest(err)) + } + ntpServers = reqNtpServers } p := &metal.Partition{ From 5f678e18e9d014dcce9ea2d3214172045b91c7f3 Mon Sep 17 00:00:00 2001 From: Simon Mayer Date: Fri, 8 Nov 2024 13:00:30 +0100 Subject: [PATCH 19/21] Remove if condition --- .../internal/service/partition-service.go | 20 ++++++++----------- 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/cmd/metal-api/internal/service/partition-service.go b/cmd/metal-api/internal/service/partition-service.go index d31e8e61..b1fc0ba9 100644 --- a/cmd/metal-api/internal/service/partition-service.go +++ b/cmd/metal-api/internal/service/partition-service.go @@ -207,23 +207,19 @@ func (r *partitionResource) createPartition(request *restful.Request, response * var dnsServers metal.DNSServers reqDNSServers := requestPayload.DNSServers - if len(reqDNSServers) > 0 { - if err := ValidateDNSServers(reqDNSServers); err != nil { - r.sendError(request, response, httperrors.BadRequest(err)) - return - } - - dnsServers = reqDNSServers + if err := ValidateDNSServers(reqDNSServers); err != nil { + r.sendError(request, response, httperrors.BadRequest(err)) + return } + dnsServers = reqDNSServers var ntpServers metal.NTPServers reqNtpServers := requestPayload.NTPServers - if len(ntpServers) > 0 { - if err := ValidateNTPServers(reqNtpServers); err != nil { - r.sendError(request, response, httperrors.BadRequest(err)) - } - ntpServers = reqNtpServers + if err := ValidateNTPServers(reqNtpServers); err != nil { + r.sendError(request, response, httperrors.BadRequest(err)) + return } + ntpServers = reqNtpServers p := &metal.Partition{ Base: metal.Base{ From 5f71f78ba37ea89ccaca1b34560a608485b61c71 Mon Sep 17 00:00:00 2001 From: Gerrit Date: Fri, 8 Nov 2024 13:27:36 +0100 Subject: [PATCH 20/21] Review with Simon. --- cmd/metal-api/internal/metal/machine.go | 45 ++++++++++++ .../internal/service/machine-service.go | 56 +++++---------- .../internal/service/partition-service.go | 25 +++++-- cmd/metal-api/internal/service/v1/machine.go | 36 ++++++++-- .../internal/service/v1/partition.go | 24 +++++-- spec/metal-api.json | 68 +++++++++---------- 6 files changed, 163 insertions(+), 91 deletions(-) diff --git a/cmd/metal-api/internal/metal/machine.go b/cmd/metal-api/internal/metal/machine.go index 32254971..5f51bd0f 100644 --- a/cmd/metal-api/internal/metal/machine.go +++ b/cmd/metal-api/internal/metal/machine.go @@ -1,8 +1,10 @@ package metal import ( + "errors" "fmt" "log/slog" + "net" "net/netip" "os" "path/filepath" @@ -10,6 +12,7 @@ import ( "strings" "time" + "github.com/asaskevich/govalidator" "github.com/dustin/go-humanize" mn "github.com/metal-stack/metal-lib/pkg/net" "github.com/samber/lo" @@ -731,3 +734,45 @@ func (i *MachineIPMISuperUser) User() string { func DisabledIPMISuperUser() MachineIPMISuperUser { return MachineIPMISuperUser{} } + +func (d DNSServers) Validate() error { + if d == nil { + return nil + } + + if len(d) > 3 { + return errors.New("please specify a maximum of three dns servers") + } + + for _, dnsServer := range d { + _, err := netip.ParseAddr(dnsServer.IP) + if err != nil { + return fmt.Errorf("ip: %s for dns server not correct err: %w", dnsServer, err) + } + } + return nil +} + +func (n NTPServers) Validate() error { + if n == nil { + return nil + } + + if len(n) < 3 || len(n) > 5 { + return errors.New("please specify a minimum of 3 and a maximum of 5 ntp servers") + } + + for _, ntpserver := range n { + if net.ParseIP(ntpserver.Address) != nil { + _, err := netip.ParseAddr(ntpserver.Address) + if err != nil { + return fmt.Errorf("ip: %s for ntp server not correct err: %w", ntpserver, err) + } + } else { + if !govalidator.IsDNSName(ntpserver.Address) { + return fmt.Errorf("dns name: %s for ntp server not correct", ntpserver) + } + } + } + return nil +} diff --git a/cmd/metal-api/internal/service/machine-service.go b/cmd/metal-api/internal/service/machine-service.go index 78e098b7..338b2a7c 100644 --- a/cmd/metal-api/internal/service/machine-service.go +++ b/cmd/metal-api/internal/service/machine-service.go @@ -7,12 +7,10 @@ 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" @@ -1151,29 +1149,33 @@ func createMachineAllocationSpec(ds *datastore.RethinkStore, machineRequest v1.M if err != nil { return nil, fmt.Errorf("partition:%s not found err:%w", partitionID, err) } + var ( 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 = metal.DNSServers{} + for _, s := range machineRequest.DNSServers { + dnsServers = append(dnsServers, metal.DNSServer{ + IP: s.IP, + }) } - dnsServers = machineRequest.DNSServers } - - if err := ValidateDNSServers(dnsServers); err != nil { - return nil, 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 = []metal.NTPServer{} + for _, s := range machineRequest.NTPServers { + ntpServers = append(ntpServers, metal.NTPServer{ + Address: s.Address, + }) } - ntpServers = machineRequest.NTPServers } - if err := ValidateNTPServers(ntpServers); err != nil { + if err := dnsServers.Validate(); err != nil { + return nil, err + } + + if err := ntpServers.Validate(); err != nil { return nil, err } @@ -2512,29 +2514,3 @@ func (s machineAllocationSpec) noautoNetworkN() int { func (s machineAllocationSpec) autoNetworkN() int { return len(s.Networks) - s.noautoNetworkN() } - -func ValidateDNSServers(d metal.DNSServers) error { - for _, dnsServer := range d { - _, err := netip.ParseAddr(dnsServer.IP) - if err != nil { - return fmt.Errorf("ip: %s for dns server not correct err: %w", dnsServer, err) - } - } - return nil -} - -func ValidateNTPServers(dnsServers metal.NTPServers) error { - for _, ntpserver := range dnsServers { - if net.ParseIP(ntpserver.Address) != nil { - _, err := netip.ParseAddr(ntpserver.Address) - if err != nil { - return fmt.Errorf("ip: %s for ntp server not correct err: %w", ntpserver, err) - } - } else { - if !govalidator.IsDNSName(ntpserver.Address) { - return fmt.Errorf("dns name: %s for ntp server not correct", ntpserver) - } - } - } - return nil -} diff --git a/cmd/metal-api/internal/service/partition-service.go b/cmd/metal-api/internal/service/partition-service.go index b1fc0ba9..81fae26f 100644 --- a/cmd/metal-api/internal/service/partition-service.go +++ b/cmd/metal-api/internal/service/partition-service.go @@ -206,20 +206,31 @@ func (r *partitionResource) createPartition(request *restful.Request, response * } var dnsServers metal.DNSServers - reqDNSServers := requestPayload.DNSServers - if err := ValidateDNSServers(reqDNSServers); err != nil { + if len(requestPayload.DNSServers) != 0 { + for _, s := range requestPayload.DNSServers { + dnsServers = append(dnsServers, metal.DNSServer{ + IP: s.IP, + }) + } + } + var ntpServers metal.NTPServers + if len(requestPayload.NTPServers) != 0 { + for _, s := range requestPayload.NTPServers { + ntpServers = append(ntpServers, metal.NTPServer{ + Address: s.Address, + }) + } + } + + if err := dnsServers.Validate(); err != nil { r.sendError(request, response, httperrors.BadRequest(err)) return } - dnsServers = reqDNSServers - var ntpServers metal.NTPServers - reqNtpServers := requestPayload.NTPServers - if err := ValidateNTPServers(reqNtpServers); err != nil { + if err := ntpServers.Validate(); err != nil { r.sendError(request, response, httperrors.BadRequest(err)) return } - ntpServers = reqNtpServers p := &metal.Partition{ Base: metal.Base{ diff --git a/cmd/metal-api/internal/service/v1/machine.go b/cmd/metal-api/internal/service/v1/machine.go index 4d83a375..ee137fd5 100644 --- a/cmd/metal-api/internal/service/v1/machine.go +++ b/cmd/metal-api/internal/service/v1/machine.go @@ -45,8 +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"` + DNSServers []DNSServer `json:"dns_servers,omitempty" description:"the dns servers used for the machine" optional:"true"` + NTPServers []NTPServer `json:"ntp_servers,omitempty" description:"the ntp servers used for the machine" optional:"true"` } type FirewallRules struct { @@ -231,8 +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"` + DNSServers []DNSServer `json:"dns_servers,omitempty" description:"the dns servers used for the machine" optional:"true"` + NTPServers []NTPServer `json:"ntp_servers,omitempty" description:"the ntp servers used for the machine" optional:"true"` } type MachineAllocationNetworks []MachineAllocationNetwork @@ -336,6 +336,14 @@ type MachineIssue struct { Details string `json:"details" description:"details of the issue"` } +type DNSServer struct { + IP string `json:"ip" description:"ip address of this dns server"` +} + +type NTPServer struct { + Address string `json:"address" description:"ip address or dns hostname of this ntp server"` +} + func NewMetalIPMI(r *MachineIPMI) metal.IPMI { var chassisPartNumber string if r.Fru.ChassisPartNumber != nil { @@ -584,6 +592,22 @@ func NewMachineResponse(m *metal.Machine, s *metal.Size, p *metal.Partition, i * } } + var ( + dnsServers []DNSServer + ntpServers []NTPServer + ) + + for _, s := range m.Allocation.DNSServers { + dnsServers = append(dnsServers, DNSServer{ + IP: s.IP, + }) + } + for _, s := range m.Allocation.NTPServers { + ntpServers = append(ntpServers, NTPServer{ + Address: s.Address, + }) + } + allocation = &MachineAllocation{ Creator: m.Allocation.Creator, Created: m.Allocation.Created, @@ -601,8 +625,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, + DNSServers: dnsServers, + NTPServers: ntpServers, } allocation.Reinstall = m.Allocation.Reinstall diff --git a/cmd/metal-api/internal/service/v1/partition.go b/cmd/metal-api/internal/service/v1/partition.go index 531d8fbb..0e5d3cec 100644 --- a/cmd/metal-api/internal/service/v1/partition.go +++ b/cmd/metal-api/internal/service/v1/partition.go @@ -8,8 +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"` + DNSServers []DNSServer `json:"dns_servers" description:"the dns servers for this partition" optional:"true"` + NTPServers []NTPServer `json:"ntp_servers" description:"the ntp servers for this partition" optional:"true"` } type PartitionBootConfiguration struct { @@ -106,6 +106,22 @@ func NewPartitionResponse(p *metal.Partition) *PartitionResponse { labels = p.Labels } + var ( + dnsServers []DNSServer + ntpServers []NTPServer + ) + + for _, s := range p.DNSServers { + dnsServers = append(dnsServers, DNSServer{ + IP: s.IP, + }) + } + for _, s := range p.NTPServers { + ntpServers = append(ntpServers, NTPServer{ + Address: s.Address, + }) + } + return &PartitionResponse{ Common: Common{ Identifiable: Identifiable{ @@ -119,8 +135,8 @@ func NewPartitionResponse(p *metal.Partition) *PartitionResponse { PartitionBase: PartitionBase{ MgmtServiceAddress: &p.MgmtServiceAddress, PrivateNetworkPrefixLength: &prefixLength, - DNSServers: p.DNSServers, - NTPServers: p.NTPServers, + DNSServers: dnsServers, + NTPServers: ntpServers, }, PartitionBootConfiguration: PartitionBootConfiguration{ ImageURL: &p.BootConfiguration.ImageURL, diff --git a/spec/metal-api.json b/spec/metal-api.json index 7b801933..79df1274 100644 --- a/spec/metal-api.json +++ b/spec/metal-api.json @@ -348,28 +348,6 @@ "type": "HTTPErrorResponse" } }, - "metal.DNSServer": { - "properties": { - "ip": { - "description": "ip address of this dns server", - "type": "string" - } - }, - "required": [ - "ip" - ] - }, - "metal.NTPServer": { - "properties": { - "address": { - "description": "ip address or dns hostname of this ntp server", - "type": "string" - } - }, - "required": [ - "address" - ] - }, "rest.HealthResponse": { "properties": { "message": { @@ -640,6 +618,17 @@ "id" ] }, + "v1.DNSServer": { + "properties": { + "ip": { + "description": "ip address of this dns server", + "type": "string" + } + }, + "required": [ + "ip" + ] + }, "v1.Describable": { "properties": { "description": { @@ -1034,7 +1023,7 @@ "dns_servers": { "description": "the dns servers used for the machine", "items": { - "$ref": "#/definitions/metal.DNSServer" + "$ref": "#/definitions/v1.DNSServer" }, "type": "array" }, @@ -1075,7 +1064,7 @@ "ntp_servers": { "description": "the ntp servers used for the machine", "items": { - "$ref": "#/definitions/metal.NTPServer" + "$ref": "#/definitions/v1.NTPServer" }, "type": "array" }, @@ -2023,7 +2012,7 @@ "dns_servers": { "description": "the dns servers used for the machine", "items": { - "$ref": "#/definitions/metal.DNSServer" + "$ref": "#/definitions/v1.DNSServer" }, "type": "array" }, @@ -2060,7 +2049,7 @@ "ntp_servers": { "description": "the ntp servers used for the machine", "items": { - "$ref": "#/definitions/metal.NTPServer" + "$ref": "#/definitions/v1.NTPServer" }, "type": "array" }, @@ -2140,7 +2129,7 @@ "dns_servers": { "description": "the dns servers used for the machine", "items": { - "$ref": "#/definitions/metal.DNSServer" + "$ref": "#/definitions/v1.DNSServer" }, "type": "array" }, @@ -2175,7 +2164,7 @@ "ntp_servers": { "description": "the ntp servers used for the machine", "items": { - "$ref": "#/definitions/metal.NTPServer" + "$ref": "#/definitions/v1.NTPServer" }, "type": "array" }, @@ -3624,6 +3613,17 @@ "vendor" ] }, + "v1.NTPServer": { + "properties": { + "address": { + "description": "ip address or dns hostname of this ntp server", + "type": "string" + } + }, + "required": [ + "address" + ] + }, "v1.NetworkAllocateRequest": { "properties": { "description": { @@ -4090,7 +4090,7 @@ "dns_servers": { "description": "the dns servers for this partition", "items": { - "$ref": "#/definitions/metal.DNSServer" + "$ref": "#/definitions/v1.DNSServer" }, "type": "array" }, @@ -4108,7 +4108,7 @@ "ntp_servers": { "description": "the ntp servers for this partition", "items": { - "$ref": "#/definitions/metal.NTPServer" + "$ref": "#/definitions/v1.NTPServer" }, "type": "array" }, @@ -4190,7 +4190,7 @@ "dns_servers": { "description": "the dns servers for this partition", "items": { - "$ref": "#/definitions/metal.DNSServer" + "$ref": "#/definitions/v1.DNSServer" }, "type": "array" }, @@ -4216,7 +4216,7 @@ "ntp_servers": { "description": "the ntp servers for this partition", "items": { - "$ref": "#/definitions/metal.NTPServer" + "$ref": "#/definitions/v1.NTPServer" }, "type": "array" }, @@ -4258,7 +4258,7 @@ "dns_servers": { "description": "the dns servers for this partition", "items": { - "$ref": "#/definitions/metal.DNSServer" + "$ref": "#/definitions/v1.DNSServer" }, "type": "array" }, @@ -4284,7 +4284,7 @@ "ntp_servers": { "description": "the ntp servers for this partition", "items": { - "$ref": "#/definitions/metal.NTPServer" + "$ref": "#/definitions/v1.NTPServer" }, "type": "array" }, From d01ce2dd1e4a9b3f0319a9228533e39dd0d2e488 Mon Sep 17 00:00:00 2001 From: Gerrit Date: Fri, 8 Nov 2024 13:34:34 +0100 Subject: [PATCH 21/21] Fix. --- cmd/metal-api/internal/metal/machine.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/metal-api/internal/metal/machine.go b/cmd/metal-api/internal/metal/machine.go index 5f51bd0f..baa11328 100644 --- a/cmd/metal-api/internal/metal/machine.go +++ b/cmd/metal-api/internal/metal/machine.go @@ -736,7 +736,7 @@ func DisabledIPMISuperUser() MachineIPMISuperUser { } func (d DNSServers) Validate() error { - if d == nil { + if len(d) == 0 { return nil } @@ -754,7 +754,7 @@ func (d DNSServers) Validate() error { } func (n NTPServers) Validate() error { - if n == nil { + if len(n) == 0 { return nil }