From e1899c062b89892d06ba3d00433f7c2dce41a961 Mon Sep 17 00:00:00 2001 From: Kajol Asabe Date: Mon, 17 Jun 2024 17:38:50 +0530 Subject: [PATCH 1/2] Add validations for NetworkPeeringClaimRef fields of Network type --- go.mod | 1 + go.sum | 2 + .../apis/networking/validation/network.go | 53 ++++++++++++++++++ .../networking/validation/network_test.go | 56 +++++++++++++++++++ 4 files changed, 112 insertions(+) diff --git a/go.mod b/go.mod index 196cda794..500da5b46 100644 --- a/go.mod +++ b/go.mod @@ -56,6 +56,7 @@ require ( github.com/go-openapi/jsonreference v0.20.2 // indirect github.com/go-openapi/swag v0.22.3 // indirect github.com/go-task/slim-sprig/v3 v3.0.0 // indirect + github.com/gofrs/uuid v4.4.0+incompatible github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect github.com/golang/protobuf v1.5.4 // indirect github.com/google/cel-go v0.17.7 // indirect diff --git a/go.sum b/go.sum index 387414266..d1fcfe429 100644 --- a/go.sum +++ b/go.sum @@ -68,6 +68,8 @@ github.com/go-openapi/swag v0.22.3/go.mod h1:UzaqsxGiab7freDnrUUra0MwWfN/q7tE4j+ github.com/go-task/slim-sprig/v3 v3.0.0 h1:sUs3vkvUymDpBKi3qH1YSqBQk9+9D/8M2mN1vB6EwHI= github.com/go-task/slim-sprig/v3 v3.0.0/go.mod h1:W848ghGpv3Qj3dhTPRyJypKRiqCdHZiAzKg9hl15HA8= github.com/godbus/dbus/v5 v5.0.4/go.mod h1:xhWf0FNVPg57R7Z0UbKHbJfkEywrmjJnf7w5xrFpKfA= +github.com/gofrs/uuid v4.4.0+incompatible h1:3qXRTX8/NbyulANqlc0lchS1gqAVxRgsuW1YrTJupqA= +github.com/gofrs/uuid v4.4.0+incompatible/go.mod h1:b2aQJv3Z4Fp6yNu3cdSllBxTCLRxnplIgP/c0N/04lM= github.com/gogo/protobuf v1.3.2 h1:Ov1cvc58UF3b5XjBnZv7+opcTcQFZebYjWzi34vdm4Q= github.com/gogo/protobuf v1.3.2/go.mod h1:P1XiOD3dCwIKUDQYPy72D8LYyHL2YPYrpS2s69NZV8Q= github.com/golang-jwt/jwt/v4 v4.5.0 h1:7cYmW1XlMY7h7ii7UhUyChSgS5wUJEnm9uZVTGqOWzg= diff --git a/internal/apis/networking/validation/network.go b/internal/apis/networking/validation/network.go index a0c35449e..7469b28b5 100644 --- a/internal/apis/networking/validation/network.go +++ b/internal/apis/networking/validation/network.go @@ -4,6 +4,7 @@ package validation import ( + "github.com/gofrs/uuid" ironcorevalidation "github.com/ironcore-dev/ironcore/internal/api/validation" "github.com/ironcore-dev/ironcore/internal/apis/networking" apivalidation "k8s.io/apimachinery/pkg/api/validation" @@ -54,6 +55,29 @@ func validateNetworkSpec(namespace, name string, spec *networking.NetworkSpec, f allErrs = append(allErrs, validateNetworkPeering(peering, fldPath)...) } + seenPeeringClaimRefKeys := sets.New[client.ObjectKey]() + + for i, peeringClaimRef := range spec.PeeringClaimRefs { + fldPath := fldPath.Child("incomingPeerings").Index(i) + + peeringClaimRefNamespace := peeringClaimRef.Namespace + if peeringClaimRefNamespace == "" { + peeringClaimRefNamespace = namespace + } + + peeringClaimRefkKey := client.ObjectKey{Namespace: peeringClaimRefNamespace, Name: peeringClaimRef.Name} + + if name != "" && (client.ObjectKey{Namespace: namespace, Name: name}) == peeringClaimRefkKey { + allErrs = append(allErrs, field.Forbidden(fldPath, "cannot claim itself")) + } else if seenPeeringClaimRefKeys.Has(peeringClaimRefkKey) { + allErrs = append(allErrs, field.Duplicate(fldPath, peeringClaimRef)) + } else { + seenPeeringClaimRefKeys.Insert(peeringClaimRefkKey) + } + + allErrs = append(allErrs, validatePeeringClaimRef(peeringClaimRef, fldPath)...) + } + return allErrs } @@ -77,6 +101,35 @@ func validateNetworkPeering(peering networking.NetworkPeering, fldPath *field.Pa return allErrs } +func validatePeeringClaimRef(peeringClaimRef networking.NetworkPeeringClaimRef, fldPath *field.Path) field.ErrorList { + var allErrs field.ErrorList + + if len(peeringClaimRef.Name) == 0 { + allErrs = append(allErrs, field.Required(fldPath.Child("name"), "name is required")) + } else { + for _, msg := range apivalidation.NameIsDNSLabel(peeringClaimRef.Name, false) { + allErrs = append(allErrs, field.Invalid(fldPath.Child("name"), peeringClaimRef.Name, msg)) + } + } + + if peeringClaimRef.Namespace != "" { + for _, msg := range apivalidation.NameIsDNSLabel(peeringClaimRef.Namespace, false) { + allErrs = append(allErrs, field.Invalid(fldPath.Child("namespace"), peeringClaimRef.Namespace, msg)) + } + } + + if peeringClaimRef.UID != "" && !isValidUID(string(peeringClaimRef.UID)) { + allErrs = append(allErrs, field.Invalid(fldPath.Child("uid"), peeringClaimRef.UID, "invalid uid")) + } + + return allErrs +} + +func isValidUID(u string) bool { + _, err := uuid.FromString(u) + return err == nil +} + // ValidateNetworkUpdate validates a Network object before an update. func ValidateNetworkUpdate(newNetwork, oldNetwork *networking.Network) field.ErrorList { var allErrs field.ErrorList diff --git a/internal/apis/networking/validation/network_test.go b/internal/apis/networking/validation/network_test.go index ce293118f..b435ab4c8 100644 --- a/internal/apis/networking/validation/network_test.go +++ b/internal/apis/networking/validation/network_test.go @@ -68,6 +68,62 @@ var _ = Describe("Network", func() { }, ContainElement(DuplicateField("spec.peerings[1].networkRef")), ), + Entry("peering claim references itself", + &networking.Network{ + ObjectMeta: metav1.ObjectMeta{Namespace: "ns", Name: "foo"}, + Spec: networking.NetworkSpec{ + PeeringClaimRefs: []networking.NetworkPeeringClaimRef{ + { + Name: "foo", + Namespace: "ns", + }, + }, + }, + }, + ContainElement(ForbiddenField("spec.incomingPeerings[0]")), + ), + Entry("duplicate peering claim ref", + &networking.Network{ + Spec: networking.NetworkSpec{ + PeeringClaimRefs: []networking.NetworkPeeringClaimRef{ + {Name: "bar"}, + {Name: "bar"}, + }, + }, + }, + ContainElement(DuplicateField("spec.incomingPeerings[1]")), + ), + Entry("invalid peering claim ref name", + &networking.Network{ + Spec: networking.NetworkSpec{ + PeeringClaimRefs: []networking.NetworkPeeringClaimRef{ + {Name: "bar*"}, + }, + }, + }, + ContainElement(InvalidField("spec.incomingPeerings[0].name")), + ), + Entry("invalid peering claim ref namespace", + &networking.Network{ + Spec: networking.NetworkSpec{ + PeeringClaimRefs: []networking.NetworkPeeringClaimRef{ + {Namespace: "ns*"}, + }, + }, + }, + ContainElements(InvalidField("spec.incomingPeerings[0].namespace"), + RequiredField("spec.incomingPeerings[0].name")), + ), + Entry("invalid peering claim ref uid", + &networking.Network{ + Spec: networking.NetworkSpec{ + PeeringClaimRefs: []networking.NetworkPeeringClaimRef{ + {UID: "978978-dsfdff"}, + }, + }, + }, + ContainElements(InvalidField("spec.incomingPeerings[0].uid")), + ), ) DescribeTable("ValidateNetworkUpdate", From 0fbebc38d35c697f5ff5846d51649f406c98c66b Mon Sep 17 00:00:00 2001 From: Kajol Asabe Date: Tue, 18 Jun 2024 18:15:18 +0530 Subject: [PATCH 2/2] remove uid validation as its been handled internally --- go.mod | 1 - go.sum | 2 -- internal/apis/networking/validation/network.go | 10 ---------- internal/apis/networking/validation/network_test.go | 10 ---------- 4 files changed, 23 deletions(-) diff --git a/go.mod b/go.mod index 500da5b46..196cda794 100644 --- a/go.mod +++ b/go.mod @@ -56,7 +56,6 @@ require ( github.com/go-openapi/jsonreference v0.20.2 // indirect github.com/go-openapi/swag v0.22.3 // indirect github.com/go-task/slim-sprig/v3 v3.0.0 // indirect - github.com/gofrs/uuid v4.4.0+incompatible github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect github.com/golang/protobuf v1.5.4 // indirect github.com/google/cel-go v0.17.7 // indirect diff --git a/go.sum b/go.sum index d1fcfe429..387414266 100644 --- a/go.sum +++ b/go.sum @@ -68,8 +68,6 @@ github.com/go-openapi/swag v0.22.3/go.mod h1:UzaqsxGiab7freDnrUUra0MwWfN/q7tE4j+ github.com/go-task/slim-sprig/v3 v3.0.0 h1:sUs3vkvUymDpBKi3qH1YSqBQk9+9D/8M2mN1vB6EwHI= github.com/go-task/slim-sprig/v3 v3.0.0/go.mod h1:W848ghGpv3Qj3dhTPRyJypKRiqCdHZiAzKg9hl15HA8= github.com/godbus/dbus/v5 v5.0.4/go.mod h1:xhWf0FNVPg57R7Z0UbKHbJfkEywrmjJnf7w5xrFpKfA= -github.com/gofrs/uuid v4.4.0+incompatible h1:3qXRTX8/NbyulANqlc0lchS1gqAVxRgsuW1YrTJupqA= -github.com/gofrs/uuid v4.4.0+incompatible/go.mod h1:b2aQJv3Z4Fp6yNu3cdSllBxTCLRxnplIgP/c0N/04lM= github.com/gogo/protobuf v1.3.2 h1:Ov1cvc58UF3b5XjBnZv7+opcTcQFZebYjWzi34vdm4Q= github.com/gogo/protobuf v1.3.2/go.mod h1:P1XiOD3dCwIKUDQYPy72D8LYyHL2YPYrpS2s69NZV8Q= github.com/golang-jwt/jwt/v4 v4.5.0 h1:7cYmW1XlMY7h7ii7UhUyChSgS5wUJEnm9uZVTGqOWzg= diff --git a/internal/apis/networking/validation/network.go b/internal/apis/networking/validation/network.go index 7469b28b5..9631d40b7 100644 --- a/internal/apis/networking/validation/network.go +++ b/internal/apis/networking/validation/network.go @@ -4,7 +4,6 @@ package validation import ( - "github.com/gofrs/uuid" ironcorevalidation "github.com/ironcore-dev/ironcore/internal/api/validation" "github.com/ironcore-dev/ironcore/internal/apis/networking" apivalidation "k8s.io/apimachinery/pkg/api/validation" @@ -118,18 +117,9 @@ func validatePeeringClaimRef(peeringClaimRef networking.NetworkPeeringClaimRef, } } - if peeringClaimRef.UID != "" && !isValidUID(string(peeringClaimRef.UID)) { - allErrs = append(allErrs, field.Invalid(fldPath.Child("uid"), peeringClaimRef.UID, "invalid uid")) - } - return allErrs } -func isValidUID(u string) bool { - _, err := uuid.FromString(u) - return err == nil -} - // ValidateNetworkUpdate validates a Network object before an update. func ValidateNetworkUpdate(newNetwork, oldNetwork *networking.Network) field.ErrorList { var allErrs field.ErrorList diff --git a/internal/apis/networking/validation/network_test.go b/internal/apis/networking/validation/network_test.go index b435ab4c8..e9166bcb7 100644 --- a/internal/apis/networking/validation/network_test.go +++ b/internal/apis/networking/validation/network_test.go @@ -114,16 +114,6 @@ var _ = Describe("Network", func() { ContainElements(InvalidField("spec.incomingPeerings[0].namespace"), RequiredField("spec.incomingPeerings[0].name")), ), - Entry("invalid peering claim ref uid", - &networking.Network{ - Spec: networking.NetworkSpec{ - PeeringClaimRefs: []networking.NetworkPeeringClaimRef{ - {UID: "978978-dsfdff"}, - }, - }, - }, - ContainElements(InvalidField("spec.incomingPeerings[0].uid")), - ), ) DescribeTable("ValidateNetworkUpdate",