diff --git a/pkg/apis/metal/types_cloudprofile.go b/pkg/apis/metal/types_cloudprofile.go index 9c315f5e..ed3cf524 100644 --- a/pkg/apis/metal/types_cloudprofile.go +++ b/pkg/apis/metal/types_cloudprofile.go @@ -75,7 +75,6 @@ type Partition struct { // NetworkIsolation defines configuration for restricted or forbidden clusters. type NetworkIsolation struct { // AllowedNetworks is a list of networks which are allowed to connect in restricted or forbidden NetworkIsolated clusters. - // if empty all destinations are allowed. AllowedNetworks AllowedNetworks // DNSServers DNSServers []string diff --git a/pkg/apis/metal/validation/cloudprofile.go b/pkg/apis/metal/validation/cloudprofile.go index bd183079..6acb1372 100644 --- a/pkg/apis/metal/validation/cloudprofile.go +++ b/pkg/apis/metal/validation/cloudprofile.go @@ -52,76 +52,129 @@ func ValidateCloudProfileConfig(cloudProfileConfig *apismetal.CloudProfileConfig networkIsolationField := mcpField.Child(partitionName, "networkIsolation") - if len(partition.NetworkIsolation.DNSServers) > 3 { - dnsField := networkIsolationField.Child("dnsServers") - allErrs = append(allErrs, field.Invalid(dnsField, partition.NetworkIsolation.DNSServers, "only up to 3 dns servers are allowed")) - } - for index, ip := range partition.NetworkIsolation.DNSServers { - ipField := networkIsolationField.Child("dnsServers").Index(index) - if _, err := netip.ParseAddr(ip); err != nil { - allErrs = append(allErrs, field.Invalid(ipField, ip, "invalid ip address")) - } - } - for index, ip := range partition.NetworkIsolation.NTPServers { - ipField := networkIsolationField.Child("ntpServers").Index(index) - if _, err := netip.ParseAddr(ip); err != nil { - allErrs = append(allErrs, field.Invalid(ipField, ip, "invalid ip address")) - } - } - for index, cidr := range partition.NetworkIsolation.AllowedNetworks.Egress { - ipField := networkIsolationField.Child("allowedNetworks", "egress").Index(index) - if _, err := netip.ParsePrefix(cidr); err != nil { - allErrs = append(allErrs, field.Invalid(ipField, cidr, "invalid cidr")) - } - } - for index, cidr := range partition.NetworkIsolation.AllowedNetworks.Ingress { - ipField := networkIsolationField.Child("allowedNetworks", "ingress").Index(index) - if _, err := netip.ParsePrefix(cidr); err != nil { - allErrs = append(allErrs, field.Invalid(ipField, cidr, "invalid cidr")) - } - } + dnsServers := partition.NetworkIsolation.DNSServers + dnsServersField := networkIsolationField.Child("dnsServers") + allErrs = append(allErrs, validateDNSServers(dnsServers, dnsServersField)...) - for mirrIndex, mirr := range partition.NetworkIsolation.RegistryMirrors { - mirrorField := networkIsolationField.Child("registryMirrors").Index(mirrIndex) - if mirr.Name == "" { - allErrs = append(allErrs, field.Invalid(mirrorField.Child("name"), mirr.Name, "name of mirror may not be empty")) - } - endpointUrl, err := url.Parse(mirr.Endpoint) - if err != nil { - allErrs = append(allErrs, field.Invalid(mirrorField.Child("endpoint"), mirr.Endpoint, "not a valid url")) - } else if endpointUrl.Scheme != "http" && endpointUrl.Scheme != "https" { - allErrs = append(allErrs, field.Invalid(mirrorField.Child("endpoint"), mirr.Endpoint, "url must have the scheme http/s")) - } - if _, err := netip.ParseAddr(mirr.IP); err != nil { - allErrs = append(allErrs, field.Invalid(mirrorField.Child("ip"), mirr.IP, "invalid ip address")) - } - if mirr.Port == 0 { - allErrs = append(allErrs, field.Invalid(mirrorField.Child("port"), mirr.Port, "must be a valid port")) - } - if len(mirr.MirrorOf) == 0 { - allErrs = append(allErrs, field.Invalid(mirrorField.Child("mirrorOf"), mirr.MirrorOf, "registry mirror must replace existing registries")) - } + ntpServers := partition.NetworkIsolation.NTPServers + ntpServersField := networkIsolationField.Child("ntpServers") + allErrs = append(allErrs, validateNTPServers(ntpServers, ntpServersField)...) - for regIndex, reg := range mirr.MirrorOf { - regField := mirrorField.Child("mirrorOf").Index(regIndex) - if reg == "" { - allErrs = append(allErrs, field.Invalid(regField, reg, "cannot be empty")) - } - regUrl, err := url.Parse("https://" + reg + "/") - if err != nil { - allErrs = append(allErrs, field.Invalid(regField, reg, "invalid registry")) - } - if regUrl.Host != reg { - allErrs = append(allErrs, field.Invalid(regField, reg, "not a valid registry host")) - } - } - } + allowedNetworks := partition.NetworkIsolation.AllowedNetworks + allowedNetworksField := networkIsolationField.Child("allowedNetworks") + allErrs = append(allErrs, validateAllowedNetworks(allowedNetworksField, allowedNetworks)...) + + registryMirrors := partition.NetworkIsolation.RegistryMirrors + registryMirrorsField := networkIsolationField.Child("registryMirrors") + allErrs = append(allErrs, validateRegistryMirrors(registryMirrors, registryMirrorsField)...) } } return allErrs } +func validateDNSServers(dnsServers []string, dnsField *field.Path) field.ErrorList { + errs := field.ErrorList{} + if len(dnsServers) == 0 { + errs = append(errs, field.Invalid(dnsField, dnsServers, "may not be empty")) + } + if len(dnsServers) > 3 { + errs = append(errs, field.Invalid(dnsField, dnsServers, "only up to 3 dns servers are allowed")) + } + for index, ip := range dnsServers { + ipField := dnsField.Index(index) + if _, err := netip.ParseAddr(ip); err != nil { + errs = append(errs, field.Invalid(ipField, ip, "invalid ip address")) + } + } + return errs +} + +func validateNTPServers(ntpServers []string, ntpField *field.Path) field.ErrorList { + errs := field.ErrorList{} + if len(ntpServers) == 0 { + errs = append(errs, field.Invalid(ntpField, ntpServers, "may not be empty")) + } + for index, ip := range ntpServers { + ipField := ntpField.Index(index) + if _, err := netip.ParseAddr(ip); err != nil { + errs = append(errs, field.Invalid(ipField, ip, "invalid ip address")) + } + } + return errs +} + +func validateAllowedNetworks(allowedNetworksField *field.Path, allowedNetworks apismetal.AllowedNetworks) field.ErrorList { + errs := field.ErrorList{} + + egress := allowedNetworks.Egress + egressField := allowedNetworksField.Child("egress") + if len(egress) == 0 { + errs = append(errs, field.Invalid(egressField, egress, "may not be empty")) + } + for index, cidr := range egress { + ipField := egressField.Index(index) + if _, err := netip.ParsePrefix(cidr); err != nil { + errs = append(errs, field.Invalid(ipField, cidr, "invalid cidr")) + } + } + ingress := allowedNetworks.Ingress + ingressField := allowedNetworksField.Child("ingress") + if len(ingress) == 0 { + errs = append(errs, field.Invalid(ingressField, ingress, "may not be empty")) + } + for index, cidr := range ingress { + ipField := ingressField.Index(index) + if _, err := netip.ParsePrefix(cidr); err != nil { + errs = append(errs, field.Invalid(ipField, cidr, "invalid cidr")) + } + } + return errs +} + +func validateRegistryMirrors(registryMirrors []apismetal.RegistryMirror, registryMirrorsField *field.Path) field.ErrorList { + errs := field.ErrorList{} + if len(registryMirrors) == 0 { + errs = append(errs, field.Invalid(registryMirrorsField, registryMirrors, "may not be empty")) + } + for mirrIndex, mirr := range registryMirrors { + mirrorField := registryMirrorsField.Index(mirrIndex) + if mirr.Name == "" { + errs = append(errs, field.Invalid(mirrorField.Child("name"), mirr.Name, "name of mirror may not be empty")) + } + endpointUrl, err := url.Parse(mirr.Endpoint) + if err != nil { + errs = append(errs, field.Invalid(mirrorField.Child("endpoint"), mirr.Endpoint, "not a valid url")) + } else if endpointUrl.Scheme != "http" && endpointUrl.Scheme != "https" { + errs = append(errs, field.Invalid(mirrorField.Child("endpoint"), mirr.Endpoint, "url must have the scheme http/s")) + } + if _, err := netip.ParseAddr(mirr.IP); err != nil { + errs = append(errs, field.Invalid(mirrorField.Child("ip"), mirr.IP, "invalid ip address")) + } + if mirr.Port == 0 { + errs = append(errs, field.Invalid(mirrorField.Child("port"), mirr.Port, "must be a valid port")) + } + if len(mirr.MirrorOf) == 0 { + errs = append(errs, field.Invalid(mirrorField.Child("mirrorOf"), mirr.MirrorOf, "registry mirror must replace existing registries")) + } + + for regIndex, reg := range mirr.MirrorOf { + regField := mirrorField.Child("mirrorOf").Index(regIndex) + if reg == "" { + errs = append(errs, field.Invalid(regField, reg, "cannot be empty")) + } + regUrl, err := url.Parse("https://" + reg + "/") + if err != nil { + errs = append(errs, field.Invalid(regField, reg, "invalid registry")) + } + if regUrl.Host != reg { + errs = append(errs, field.Invalid(regField, reg, "not a valid registry host")) + } + } + } + return errs +} + func ValidateImmutableCloudProfileConfig( newCloudProfileConfig *apismetal.CloudProfileConfig, oldCloudProfileConfig *apismetal.CloudProfileConfig, diff --git a/pkg/apis/metal/validation/cloudprofile_test.go b/pkg/apis/metal/validation/cloudprofile_test.go index efb6ae7f..0c1ea994 100644 --- a/pkg/apis/metal/validation/cloudprofile_test.go +++ b/pkg/apis/metal/validation/cloudprofile_test.go @@ -125,6 +125,67 @@ var _ = Describe("CloudProfileConfig validation", func() { Expect(errorList).To(BeEmpty()) }) + It("should pass with missing network isolation", func() { + cloudProfileConfig.MetalControlPlanes = map[string]apismetal.MetalControlPlane{ + "prod": { + Partitions: map[string]apismetal.Partition{ + "partition-b": {}, + }, + }, + } + + errorList := ValidateCloudProfileConfig(cloudProfileConfig, cloudProfile, path) + + Expect(errorList).To(BeEmpty()) + }) + + It("should fail with network isolation with empty values", func() { + cloudProfileConfig.MetalControlPlanes = map[string]apismetal.MetalControlPlane{ + "prod": { + Partitions: map[string]apismetal.Partition{ + "partition-b": { + NetworkIsolation: &apismetal.NetworkIsolation{}, + }, + }, + }, + } + + errorList := ValidateCloudProfileConfig(cloudProfileConfig, cloudProfile, path) + + Expect(errorList).To(ConsistOf( + PointTo(MatchFields(IgnoreExtras, Fields{ + "Type": Equal(field.ErrorTypeInvalid), + "Field": Equal("test.metalControlPlanes.prod.partition-b.networkIsolation.dnsServers"), + "BadValue": HaveLen(0), + "Detail": Equal("may not be empty"), + })), + PointTo(MatchFields(IgnoreExtras, Fields{ + "Type": Equal(field.ErrorTypeInvalid), + "Field": Equal("test.metalControlPlanes.prod.partition-b.networkIsolation.ntpServers"), + "BadValue": HaveLen(0), + "Detail": Equal("may not be empty"), + })), + PointTo(MatchFields(IgnoreExtras, Fields{ + "Type": Equal(field.ErrorTypeInvalid), + "Field": Equal("test.metalControlPlanes.prod.partition-b.networkIsolation.registryMirrors"), + "BadValue": HaveLen(0), + "Detail": Equal("may not be empty"), + })), + PointTo(MatchFields(IgnoreExtras, Fields{ + "Type": Equal(field.ErrorTypeInvalid), + "Field": Equal("test.metalControlPlanes.prod.partition-b.networkIsolation.allowedNetworks.egress"), + "BadValue": HaveLen(0), + "Detail": Equal("may not be empty"), + })), + PointTo(MatchFields(IgnoreExtras, Fields{ + "Type": Equal(field.ErrorTypeInvalid), + "Field": Equal("test.metalControlPlanes.prod.partition-b.networkIsolation.allowedNetworks.ingress"), + "BadValue": HaveLen(0), + "Detail": Equal("may not be empty"), + })), + )) + }) + It("should allow up to 3 dns servers", func() { cloudProfileConfig.MetalControlPlanes = map[string]apismetal.MetalControlPlane{ "prod": { @@ -166,24 +227,7 @@ var _ = Describe("CloudProfileConfig validation", func() { Partitions: map[string]apismetal.Partition{ "partition-b": { NetworkIsolation: &apismetal.NetworkIsolation{ - AllowedNetworks: apismetal.AllowedNetworks{ - Ingress: []string{"10.0.0.1/24"}, - Egress: []string{"100.0.0.1/24"}, - }, DNSServers: []string{"1.1.1.1", "1.0.0.1", "8.8.8.8", "8.8.4.4"}, - NTPServers: []string{"134.60.1.27", "134.60.111.110"}, - RegistryMirrors: []apismetal.RegistryMirror{ - { - Name: "metal-stack registry", - Endpoint: "https://some.registry", - IP: "1.2.3.4", - Port: 443, - MirrorOf: []string{ - "ghcr.io", - "quay.io", - }, - }, - }, }, }, }, @@ -192,7 +236,7 @@ var _ = Describe("CloudProfileConfig validation", func() { errorList := ValidateCloudProfileConfig(cloudProfileConfig, cloudProfile, path) - Expect(errorList).To(ConsistOf( + Expect(errorList).To(ContainElement( PointTo(MatchFields(IgnoreExtras, Fields{ "Type": Equal(field.ErrorTypeInvalid), "Field": Equal("test.metalControlPlanes.prod.partition-b.networkIsolation.dnsServers"), @@ -202,18 +246,12 @@ var _ = Describe("CloudProfileConfig validation", func() { )) }) - It("should prevent partitions with empty network isolation registry mirror", func() { + It("should prevent registry mirrors with empty values", func() { cloudProfileConfig.MetalControlPlanes = map[string]apismetal.MetalControlPlane{ "prod": { Partitions: map[string]apismetal.Partition{ "partition-b": { NetworkIsolation: &apismetal.NetworkIsolation{ - AllowedNetworks: apismetal.AllowedNetworks{ - Ingress: []string{"10.0.0.1/24"}, - Egress: []string{"100.0.0.1/24"}, - }, - DNSServers: []string{"1.1.1.1", "1.0.0.1"}, - NTPServers: []string{"134.60.1.27", "134.60.111.110"}, RegistryMirrors: []apismetal.RegistryMirror{ { Name: "", @@ -231,7 +269,7 @@ var _ = Describe("CloudProfileConfig validation", func() { errorList := ValidateCloudProfileConfig(cloudProfileConfig, cloudProfile, path) - Expect(errorList).To(ConsistOf( + Expect(errorList).To(ContainElements( PointTo(MatchFields(IgnoreExtras, Fields{ "Type": Equal(field.ErrorTypeInvalid), "Field": Equal("test.metalControlPlanes.prod.partition-b.networkIsolation.registryMirrors[0].name"), @@ -265,7 +303,7 @@ var _ = Describe("CloudProfileConfig validation", func() { )) }) - It("should prevent partitions with invalid network isolation", func() { + It("should prevent partition with invalid network isolation", func() { cloudProfileConfig.MetalControlPlanes = map[string]apismetal.MetalControlPlane{ "prod": { Partitions: map[string]apismetal.Partition{ diff --git a/pkg/apis/metal/validation/network_access.go b/pkg/apis/metal/validation/network_access.go index 45fc65ac..fd1831c9 100644 --- a/pkg/apis/metal/validation/network_access.go +++ b/pkg/apis/metal/validation/network_access.go @@ -32,46 +32,9 @@ func validateNetworkAccessFields(controlPlaneConfig *apismetal.ControlPlaneConfi if partition.NetworkIsolation == nil { allErrs = append(allErrs, - field.Invalid(natPath, controlPlaneConfig.NetworkAccessType, "network access type requires partition's networkAccess to be set"), + field.Invalid(natPath, controlPlaneConfig.NetworkAccessType, "network access type requires partition's networkIsolation to be set"), field.Required(partNiPath, "network isolation required if control plane config networkAccess is not baseline"), ) - return allErrs - } - - if len(partition.NetworkIsolation.DNSServers) == 0 { - allErrs = append(allErrs, field.Invalid( - partNiPath.Child("dnsServers"), - partition.NetworkIsolation.DNSServers, - "may not be empty", - )) - } - if len(partition.NetworkIsolation.NTPServers) == 0 { - allErrs = append(allErrs, field.Invalid( - partNiPath.Child("ntpServers"), - partition.NetworkIsolation.NTPServers, - "may not be empty", - )) - } - if len(partition.NetworkIsolation.RegistryMirrors) == 0 { - allErrs = append(allErrs, field.Invalid( - partNiPath.Child("registryMirrors"), - partition.NetworkIsolation.RegistryMirrors, - "may not be empty", - )) - } - if len(partition.NetworkIsolation.AllowedNetworks.Egress) == 0 { - allErrs = append(allErrs, field.Invalid( - partNiPath.Child("allowedNetworks", "egress"), - partition.NetworkIsolation.AllowedNetworks.Egress, - "may not be empty", - )) - } - if len(partition.NetworkIsolation.AllowedNetworks.Ingress) == 0 { - allErrs = append(allErrs, field.Invalid( - partNiPath.Child("allowedNetworks", "ingress"), - partition.NetworkIsolation.AllowedNetworks.Ingress, - "may not be empty", - )) } return allErrs diff --git a/pkg/apis/metal/validation/network_access_test.go b/pkg/apis/metal/validation/network_access_test.go index 67b67cd8..2a830ed2 100644 --- a/pkg/apis/metal/validation/network_access_test.go +++ b/pkg/apis/metal/validation/network_access_test.go @@ -80,7 +80,7 @@ var _ = Describe("CloudProfileConfig validation", func() { "Type": Equal(field.ErrorTypeInvalid), "Field": Equal("test.networkAccessType"), "BadValue": PointTo(Equal(apismetal.NetworkAccessForbidden)), - "Detail": Equal("network access type requires partition's networkAccess to be set"), + "Detail": Equal("network access type requires partition's networkIsolation to be set"), })), PointTo(MatchFields(IgnoreExtras, Fields{ "Type": Equal(field.ErrorTypeRequired), @@ -125,53 +125,6 @@ var _ = Describe("CloudProfileConfig validation", func() { Expect(errorList).To(BeEmpty()) }) - - It("should fail with empty network isolation", func() { - cloudProfileConfig.MetalControlPlanes = map[string]apismetal.MetalControlPlane{ - "prod": { - Partitions: map[string]apismetal.Partition{ - "partition-b": { - NetworkIsolation: &apismetal.NetworkIsolation{}, - }, - }, - }, - } - - errorList := ValidateControlPlaneConfigNetworkAccess(controlPlaneConfig, cloudProfileConfig, partitionName, path) - - Expect(errorList).To(ConsistOf( - PointTo(MatchFields(IgnoreExtras, Fields{ - "Type": Equal(field.ErrorTypeInvalid), - "Field": Equal("test.metalControlPlanes[prod].partitions[partition-b].networkIsolation.dnsServers"), - "BadValue": HaveLen(0), - "Detail": Equal("may not be empty"), - })), - PointTo(MatchFields(IgnoreExtras, Fields{ - "Type": Equal(field.ErrorTypeInvalid), - "Field": Equal("test.metalControlPlanes[prod].partitions[partition-b].networkIsolation.ntpServers"), - "BadValue": HaveLen(0), - "Detail": Equal("may not be empty"), - })), - PointTo(MatchFields(IgnoreExtras, Fields{ - "Type": Equal(field.ErrorTypeInvalid), - "Field": Equal("test.metalControlPlanes[prod].partitions[partition-b].networkIsolation.registryMirrors"), - "BadValue": HaveLen(0), - "Detail": Equal("may not be empty"), - })), - PointTo(MatchFields(IgnoreExtras, Fields{ - "Type": Equal(field.ErrorTypeInvalid), - "Field": Equal("test.metalControlPlanes[prod].partitions[partition-b].networkIsolation.allowedNetworks.egress"), - "BadValue": HaveLen(0), - "Detail": Equal("may not be empty"), - })), - PointTo(MatchFields(IgnoreExtras, Fields{ - "Type": Equal(field.ErrorTypeInvalid), - "Field": Equal("test.metalControlPlanes[prod].partitions[partition-b].networkIsolation.allowedNetworks.ingress"), - "BadValue": HaveLen(0), - "Detail": Equal("may not be empty"), - })), - )) - }) }) Describe("with network access type restricted", func() { @@ -195,7 +148,7 @@ var _ = Describe("CloudProfileConfig validation", func() { "Type": Equal(field.ErrorTypeInvalid), "Field": Equal("test.networkAccessType"), "BadValue": PointTo(Equal(apismetal.NetworkAccessRestricted)), - "Detail": Equal("network access type requires partition's networkAccess to be set"), + "Detail": Equal("network access type requires partition's networkIsolation to be set"), })), PointTo(MatchFields(IgnoreExtras, Fields{ "Type": Equal(field.ErrorTypeRequired), @@ -240,53 +193,6 @@ var _ = Describe("CloudProfileConfig validation", func() { Expect(errorList).To(BeEmpty()) }) - - It("should fail with empty network isolation", func() { - cloudProfileConfig.MetalControlPlanes = map[string]apismetal.MetalControlPlane{ - "prod": { - Partitions: map[string]apismetal.Partition{ - "partition-b": { - NetworkIsolation: &apismetal.NetworkIsolation{}, - }, - }, - }, - } - - errorList := ValidateControlPlaneConfigNetworkAccess(controlPlaneConfig, cloudProfileConfig, partitionName, path) - - Expect(errorList).To(ConsistOf( - PointTo(MatchFields(IgnoreExtras, Fields{ - "Type": Equal(field.ErrorTypeInvalid), - "Field": Equal("test.metalControlPlanes[prod].partitions[partition-b].networkIsolation.dnsServers"), - "BadValue": HaveLen(0), - "Detail": Equal("may not be empty"), - })), - PointTo(MatchFields(IgnoreExtras, Fields{ - "Type": Equal(field.ErrorTypeInvalid), - "Field": Equal("test.metalControlPlanes[prod].partitions[partition-b].networkIsolation.ntpServers"), - "BadValue": HaveLen(0), - "Detail": Equal("may not be empty"), - })), - PointTo(MatchFields(IgnoreExtras, Fields{ - "Type": Equal(field.ErrorTypeInvalid), - "Field": Equal("test.metalControlPlanes[prod].partitions[partition-b].networkIsolation.registryMirrors"), - "BadValue": HaveLen(0), - "Detail": Equal("may not be empty"), - })), - PointTo(MatchFields(IgnoreExtras, Fields{ - "Type": Equal(field.ErrorTypeInvalid), - "Field": Equal("test.metalControlPlanes[prod].partitions[partition-b].networkIsolation.allowedNetworks.egress"), - "BadValue": HaveLen(0), - "Detail": Equal("may not be empty"), - })), - PointTo(MatchFields(IgnoreExtras, Fields{ - "Type": Equal(field.ErrorTypeInvalid), - "Field": Equal("test.metalControlPlanes[prod].partitions[partition-b].networkIsolation.allowedNetworks.ingress"), - "BadValue": HaveLen(0), - "Detail": Equal("may not be empty"), - })), - )) - }) }) }) })