Skip to content

Commit

Permalink
chore(e2e): code review
Browse files Browse the repository at this point in the history
Signed-off-by: Marcin Skalski <[email protected]>
  • Loading branch information
Automaat committed Jan 29, 2025
1 parent 08eb3d8 commit 1bbc414
Show file tree
Hide file tree
Showing 18 changed files with 29 additions and 46 deletions.
5 changes: 2 additions & 3 deletions pkg/core/resources/apis/mesh/validators.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import (

common_api "github.com/kumahq/kuma/api/common/v1alpha1"
mesh_proto "github.com/kumahq/kuma/api/mesh/v1alpha1"
core_model "github.com/kumahq/kuma/pkg/core/resources/model"
"github.com/kumahq/kuma/pkg/core/validators"
util_proto "github.com/kumahq/kuma/pkg/util/proto"
)
Expand Down Expand Up @@ -65,7 +64,7 @@ type ValidateTargetRefOpts struct {
// includes a forward slash, but it's allowed as an exception to
// handle unresolved references.
AllowedInvalidNames []string
Descriptor core_model.ResourceTypeDescriptor
IsInboundPolicy bool
}

func ValidateSelectors(path validators.PathBuilder, sources []*mesh_proto.Selector, opts ValidateSelectorsOpts) validators.ValidationError {
Expand Down Expand Up @@ -385,7 +384,7 @@ func ValidateTargetRef(
if len(ref.Labels) > 0 && (ref.Name != "" || ref.Namespace != "") {
err.AddViolation("labels", "either labels or name and namespace must be specified")
}
if !opts.Descriptor.HasFromTargetRef && !opts.Descriptor.HasRulesTargetRef && ref.SectionName != "" {
if !opts.IsInboundPolicy && ref.SectionName != "" {
err.AddViolation("sectionName", "can only be used with inbound policies")
}
case common_api.MeshSubset:
Expand Down
5 changes: 1 addition & 4 deletions pkg/core/resources/apis/mesh/validators_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (

common_api "github.com/kumahq/kuma/api/common/v1alpha1"
. "github.com/kumahq/kuma/pkg/core/resources/apis/mesh"
"github.com/kumahq/kuma/pkg/core/resources/model"
"github.com/kumahq/kuma/pkg/core/validators"
)

Expand Down Expand Up @@ -208,9 +207,7 @@ sectionName: http-port
SupportedKinds: []common_api.TargetRefKind{
common_api.Dataplane,
},
Descriptor: model.ResourceTypeDescriptor{
HasRulesTargetRef: true,
},
IsInboundPolicy: true,
},
}),
Entry("MeshGateway", testCase{
Expand Down
6 changes: 3 additions & 3 deletions pkg/plugins/policies/meshaccesslog/api/v1alpha1/validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import (
func (r *MeshAccessLogResource) validate() error {
var verr validators.ValidationError
path := validators.RootedAt("spec")
verr.AddErrorAt(path.Field("targetRef"), r.validateTop(r.Spec.GetTargetRef()))
verr.AddErrorAt(path.Field("targetRef"), r.validateTop(r.Spec.GetTargetRef(), len(r.Spec.From) > 0))
if len(r.Spec.To) == 0 && len(r.Spec.From) == 0 {
verr.AddViolationAt(path, "at least one of 'from', 'to' has to be defined")
}
Expand All @@ -22,7 +22,7 @@ func (r *MeshAccessLogResource) validate() error {
return verr.OrNil()
}

func (r *MeshAccessLogResource) validateTop(targetRef common_api.TargetRef) validators.ValidationError {
func (r *MeshAccessLogResource) validateTop(targetRef common_api.TargetRef, isInboundPolicy bool) validators.ValidationError {
targetRefErr := mesh.ValidateTargetRef(targetRef, &mesh.ValidateTargetRefOpts{
SupportedKinds: []common_api.TargetRefKind{
common_api.Mesh,
Expand All @@ -33,7 +33,7 @@ func (r *MeshAccessLogResource) validateTop(targetRef common_api.TargetRef) vali
common_api.Dataplane,
},
GatewayListenerTagsAllowed: true,
Descriptor: r.Descriptor(),
IsInboundPolicy: isInboundPolicy,
})
return targetRefErr
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import (
func (r *MeshCircuitBreakerResource) validate() error {
var verr validators.ValidationError
path := validators.RootedAt("spec")
verr.AddErrorAt(path.Field("targetRef"), r.validateTop(r.Spec.TargetRef))
verr.AddErrorAt(path.Field("targetRef"), r.validateTop(r.Spec.TargetRef, len(r.Spec.From) > 0))
if len(r.Spec.To) == 0 && len(r.Spec.From) == 0 {
verr.AddViolationAt(path, "at least one of 'from', 'to' has to be defined")
}
Expand All @@ -21,7 +21,7 @@ func (r *MeshCircuitBreakerResource) validate() error {
return verr.OrNil()
}

func (r *MeshCircuitBreakerResource) validateTop(targetRef *common_api.TargetRef) validators.ValidationError {
func (r *MeshCircuitBreakerResource) validateTop(targetRef *common_api.TargetRef, isInboundPolicy bool) validators.ValidationError {
if targetRef == nil {
return validators.ValidationError{}
}
Expand All @@ -35,7 +35,7 @@ func (r *MeshCircuitBreakerResource) validateTop(targetRef *common_api.TargetRef
common_api.Dataplane,
},
GatewayListenerTagsAllowed: true,
Descriptor: r.Descriptor(),
IsInboundPolicy: isInboundPolicy,
})
return targetRefErr
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,14 @@ import (
func (r *MeshFaultInjectionResource) validate() error {
var verr validators.ValidationError
path := validators.RootedAt("spec")
verr.AddErrorAt(path.Field("targetRef"), r.validateTop(r.Spec.TargetRef))
verr.AddErrorAt(path.Field("targetRef"), r.validateTop(r.Spec.TargetRef, len(r.Spec.From) > 0))
topLevel := pointer.DerefOr(r.Spec.TargetRef, common_api.TargetRef{Kind: common_api.Mesh})
verr.AddErrorAt(path, validateFrom(topLevel, r.Spec.From))
verr.AddErrorAt(path, validateTo(topLevel, r.Spec.To))
return verr.OrNil()
}

func (r *MeshFaultInjectionResource) validateTop(targetRef *common_api.TargetRef) validators.ValidationError {
func (r *MeshFaultInjectionResource) validateTop(targetRef *common_api.TargetRef, isInboundPolicy bool) validators.ValidationError {
if targetRef == nil {
return validators.ValidationError{}
}
Expand All @@ -35,7 +35,7 @@ func (r *MeshFaultInjectionResource) validateTop(targetRef *common_api.TargetRef
common_api.Dataplane,
},
GatewayListenerTagsAllowed: true,
Descriptor: r.Descriptor(),
IsInboundPolicy: isInboundPolicy,
})
default:
return mesh.ValidateTargetRef(*targetRef, &mesh.ValidateTargetRefOpts{
Expand All @@ -46,7 +46,7 @@ func (r *MeshFaultInjectionResource) validateTop(targetRef *common_api.TargetRef
common_api.MeshServiceSubset,
common_api.Dataplane,
},
Descriptor: r.Descriptor(),
IsInboundPolicy: isInboundPolicy,
})
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ func (r *MeshHealthCheckResource) validateTop(targetRef *common_api.TargetRef) v
common_api.Dataplane,
},
GatewayListenerTagsAllowed: true,
Descriptor: r.Descriptor(),
})
return targetRefErr
}
Expand Down
2 changes: 0 additions & 2 deletions pkg/plugins/policies/meshhttproute/api/v1alpha1/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ func (r *MeshHTTPRouteResource) validateTop(targetRef *common_api.TargetRef) val
common_api.Dataplane,
},
GatewayListenerTagsAllowed: true,
Descriptor: r.Descriptor(),
})
default:
return mesh.ValidateTargetRef(*targetRef, &mesh.ValidateTargetRefOpts{
Expand All @@ -52,7 +51,6 @@ func (r *MeshHTTPRouteResource) validateTop(targetRef *common_api.TargetRef) val
common_api.MeshServiceSubset,
common_api.Dataplane,
},
Descriptor: r.Descriptor(),
})
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ func (r *MeshLoadBalancingStrategyResource) validateTop(targetRef *common_api.Ta
common_api.Dataplane,
},
GatewayListenerTagsAllowed: true,
Descriptor: r.Descriptor(),
})
return targetRefErr
}
Expand Down
2 changes: 0 additions & 2 deletions pkg/plugins/policies/meshmetric/api/v1alpha1/validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ func (r *MeshMetricResource) validateTop(targetRef *common_api.TargetRef) valida
common_api.Dataplane,
},
GatewayListenerTagsAllowed: true,
Descriptor: r.Descriptor(),
})
default:
return mesh.ValidateTargetRef(*targetRef, &mesh.ValidateTargetRefOpts{
Expand All @@ -49,7 +48,6 @@ func (r *MeshMetricResource) validateTop(targetRef *common_api.TargetRef) valida
common_api.Dataplane,
common_api.MeshServiceSubset,
},
Descriptor: r.Descriptor(),
})
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ func (r *MeshPassthroughResource) validateTop(targetRef *common_api.TargetRef) v
common_api.MeshSubset,
common_api.Dataplane,
},
Descriptor: r.Descriptor(),
})
return targetRefErr
}
Expand Down
2 changes: 0 additions & 2 deletions pkg/plugins/policies/meshproxypatch/api/v1alpha1/validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ func (r *MeshProxyPatchResource) validateTop(targetRef *common_api.TargetRef) va
common_api.Dataplane,
},
GatewayListenerTagsAllowed: false,
Descriptor: r.Descriptor(),
})
default:
return mesh.ValidateTargetRef(*targetRef, &mesh.ValidateTargetRefOpts{
Expand All @@ -58,7 +57,6 @@ func (r *MeshProxyPatchResource) validateTop(targetRef *common_api.TargetRef) va
common_api.MeshService,
common_api.MeshServiceSubset,
},
Descriptor: r.Descriptor(),
})
}
}
Expand Down
8 changes: 4 additions & 4 deletions pkg/plugins/policies/meshratelimit/api/v1alpha1/validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,14 @@ import (
func (r *MeshRateLimitResource) validate() error {
var verr validators.ValidationError
path := validators.RootedAt("spec")
verr.AddErrorAt(path.Field("targetRef"), r.validateTop(r.Spec.TargetRef))
verr.AddErrorAt(path.Field("targetRef"), r.validateTop(r.Spec.TargetRef, len(r.Spec.From) > 0))
topLevel := pointer.DerefOr(r.Spec.TargetRef, common_api.TargetRef{Kind: common_api.Mesh})
verr.AddErrorAt(path, validateFrom(topLevel, r.Spec.From))
verr.AddErrorAt(path, validateTo(topLevel, r.Spec.To))
return verr.OrNil()
}

func (r *MeshRateLimitResource) validateTop(targetRef *common_api.TargetRef) validators.ValidationError {
func (r *MeshRateLimitResource) validateTop(targetRef *common_api.TargetRef, isInboundPolicy bool) validators.ValidationError {
if targetRef == nil {
return validators.ValidationError{}
}
Expand All @@ -38,7 +38,7 @@ func (r *MeshRateLimitResource) validateTop(targetRef *common_api.TargetRef) val
common_api.Dataplane,
},
GatewayListenerTagsAllowed: true,
Descriptor: r.Descriptor(),
IsInboundPolicy: isInboundPolicy,
})
default:
return mesh.ValidateTargetRef(*targetRef, &mesh.ValidateTargetRefOpts{
Expand All @@ -49,7 +49,7 @@ func (r *MeshRateLimitResource) validateTop(targetRef *common_api.TargetRef) val
common_api.MeshServiceSubset,
common_api.Dataplane,
},
Descriptor: r.Descriptor(),
IsInboundPolicy: isInboundPolicy,
})
}
}
Expand Down
2 changes: 0 additions & 2 deletions pkg/plugins/policies/meshretry/api/v1alpha1/validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ func (r *MeshRetryResource) validateTop(targetRef *common_api.TargetRef) validat
common_api.Dataplane,
},
GatewayListenerTagsAllowed: true,
Descriptor: r.Descriptor(),
})
default:
return mesh.ValidateTargetRef(*targetRef, &mesh.ValidateTargetRefOpts{
Expand All @@ -52,7 +51,6 @@ func (r *MeshRetryResource) validateTop(targetRef *common_api.TargetRef) validat
common_api.MeshServiceSubset,
common_api.Dataplane,
},
Descriptor: r.Descriptor(),
})
}
}
Expand Down
2 changes: 0 additions & 2 deletions pkg/plugins/policies/meshtcproute/api/v1alpha1/validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ func (r *MeshTCPRouteResource) validateTop(targetRef *common_api.TargetRef) vali
common_api.Dataplane,
},
GatewayListenerTagsAllowed: true,
Descriptor: r.Descriptor(),
})
default:
return mesh.ValidateTargetRef(*targetRef, &mesh.ValidateTargetRefOpts{
Expand All @@ -47,7 +46,6 @@ func (r *MeshTCPRouteResource) validateTop(targetRef *common_api.TargetRef) vali
common_api.MeshServiceSubset,
common_api.Dataplane,
},
Descriptor: r.Descriptor(),
})
}
}
Expand Down
8 changes: 4 additions & 4 deletions pkg/plugins/policies/meshtimeout/api/v1alpha1/validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import (
func (r *MeshTimeoutResource) validate() error {
var verr validators.ValidationError
path := validators.RootedAt("spec")
verr.AddErrorAt(path.Field("targetRef"), r.validateTop(r.Spec.TargetRef))
verr.AddErrorAt(path.Field("targetRef"), r.validateTop(r.Spec.TargetRef, len(r.Spec.From) > 0))
if len(r.Spec.Rules) > 0 && (len(r.Spec.To) > 0 || len(r.Spec.From) > 0) {
verr.AddViolationAt(path, "fields 'to' and 'from' must be empty when 'rules' is defined")
}
Expand All @@ -25,7 +25,7 @@ func (r *MeshTimeoutResource) validate() error {
return verr.OrNil()
}

func (r *MeshTimeoutResource) validateTop(targetRef *common_api.TargetRef) validators.ValidationError {
func (r *MeshTimeoutResource) validateTop(targetRef *common_api.TargetRef, isInboundPolicy bool) validators.ValidationError {
if targetRef == nil {
return validators.ValidationError{}
}
Expand All @@ -42,7 +42,7 @@ func (r *MeshTimeoutResource) validateTop(targetRef *common_api.TargetRef) valid
common_api.MeshHTTPRoute,
},
GatewayListenerTagsAllowed: true,
Descriptor: r.Descriptor(),
IsInboundPolicy: isInboundPolicy,
})
default:
return mesh.ValidateTargetRef(*targetRef, &mesh.ValidateTargetRefOpts{
Expand All @@ -53,7 +53,7 @@ func (r *MeshTimeoutResource) validateTop(targetRef *common_api.TargetRef) valid
common_api.MeshService,
common_api.MeshServiceSubset,
},
Descriptor: r.Descriptor(),
IsInboundPolicy: isInboundPolicy,
})
}
}
Expand Down
6 changes: 3 additions & 3 deletions pkg/plugins/policies/meshtls/api/v1alpha1/validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,13 @@ import (
func (r *MeshTLSResource) validate() error {
var verr validators.ValidationError
path := validators.RootedAt("spec")
verr.AddErrorAt(path.Field("targetRef"), r.validateTop(r.Spec.TargetRef))
verr.AddErrorAt(path.Field("targetRef"), r.validateTop(r.Spec.TargetRef, len(r.Spec.From) > 0))
topLevel := pointer.DerefOr(r.Spec.TargetRef, common_api.TargetRef{Kind: common_api.Mesh, UsesSyntacticSugar: true})
verr.AddErrorAt(path.Field("from"), validateFrom(r.Spec.From, topLevel.Kind))
return verr.OrNil()
}

func (r *MeshTLSResource) validateTop(targetRef *common_api.TargetRef) validators.ValidationError {
func (r *MeshTLSResource) validateTop(targetRef *common_api.TargetRef, isInboundPolicy bool) validators.ValidationError {
if targetRef == nil {
return validators.ValidationError{}
}
Expand All @@ -29,7 +29,7 @@ func (r *MeshTLSResource) validateTop(targetRef *common_api.TargetRef) validator
common_api.MeshSubset,
common_api.Dataplane,
},
Descriptor: r.Descriptor(),
IsInboundPolicy: isInboundPolicy,
})
return targetRefErr
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/plugins/policies/meshtrace/api/v1alpha1/validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,9 @@ func (r *MeshTraceResource) validateTop(targetRef *common_api.TargetRef) validat
common_api.MeshGateway,
common_api.MeshService,
common_api.MeshServiceSubset,
common_api.Dataplane,
},
GatewayListenerTagsAllowed: false,
Descriptor: r.Descriptor(),
})
default:
return mesh.ValidateTargetRef(*targetRef, &mesh.ValidateTargetRefOpts{
Expand All @@ -50,8 +50,8 @@ func (r *MeshTraceResource) validateTop(targetRef *common_api.TargetRef) validat
common_api.MeshSubset,
common_api.MeshService,
common_api.MeshServiceSubset,
common_api.Dataplane,
},
Descriptor: r.Descriptor(),
})
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,15 @@ import (
func (r *MeshTrafficPermissionResource) validate() error {
var verr validators.ValidationError
path := validators.RootedAt("spec")
verr.AddErrorAt(path.Field("targetRef"), r.validateTop(r.Spec.TargetRef))
verr.AddErrorAt(path.Field("targetRef"), r.validateTop(r.Spec.TargetRef, len(r.Spec.From) > 0))
if len(r.Spec.From) == 0 {
verr.AddViolationAt(path.Field("from"), "needs at least one item")
}
verr.AddErrorAt(path, validateFrom(r.Spec.From))
return verr.OrNil()
}

func (r *MeshTrafficPermissionResource) validateTop(targetRef *common_api.TargetRef) validators.ValidationError {
func (r *MeshTrafficPermissionResource) validateTop(targetRef *common_api.TargetRef, isInboundPolicy bool) validators.ValidationError {
if targetRef == nil {
return validators.ValidationError{}
}
Expand All @@ -29,7 +29,7 @@ func (r *MeshTrafficPermissionResource) validateTop(targetRef *common_api.Target
common_api.MeshServiceSubset,
common_api.Dataplane,
},
Descriptor: r.Descriptor(),
IsInboundPolicy: isInboundPolicy,
})
return targetRefErr
}
Expand Down

0 comments on commit 1bbc414

Please sign in to comment.