From 23e471168a77fb1eeec4dc911bb3d7ab5350db1e Mon Sep 17 00:00:00 2001 From: Shaun Crampton Date: Thu, 21 Sep 2017 11:46:14 +0100 Subject: [PATCH] Make validation of policy types more permissive. Allow policies to specify ingress/egress rules even if the corresponding type isn't enabled. This allows for smoother upgrades since we can have policy that works for old and new Felix versions. --- lib/validator/validator.go | 17 ----------------- lib/validator/validator_test.go | 14 ++++++++++---- 2 files changed, 10 insertions(+), 21 deletions(-) diff --git a/lib/validator/validator.go b/lib/validator/validator.go index cca9a0652..cd85f68e3 100644 --- a/lib/validator/validator.go +++ b/lib/validator/validator.go @@ -552,21 +552,4 @@ func validatePolicySpec(v *validator.Validate, structLevel *validator.StructLeve mp[t] = true } } - - // When Types is explicitly specified: - if len(m.Types) > 0 { - var exists bool - // 'ingress' type must be there if Policy has any ingress rules. - _, exists = mp[api.PolicyTypeIngress] - if len(m.IngressRules) > 0 && !exists { - structLevel.ReportError(reflect.ValueOf(m.Types), - "PolicySpec.Types", "", reason("'ingress' must be specified when policy has ingress rules")) - } - // 'egress' type must be there if Policy has any egress rules. - _, exists = mp[api.PolicyTypeEgress] - if len(m.EgressRules) > 0 && !exists { - structLevel.ReportError(reflect.ValueOf(m.Types), - "PolicySpec.Types", "", reason("'egress' must be specified when policy has egress rules")) - } - } } diff --git a/lib/validator/validator_test.go b/lib/validator/validator_test.go index 674868113..bb31e1b08 100644 --- a/lib/validator/validator_test.go +++ b/lib/validator/validator_test.go @@ -674,16 +674,22 @@ func init() { Entry("allow ingress+egress Types", api.PolicySpec{Types: []api.PolicyType{api.PolicyTypeIngress, api.PolicyTypeEgress}}, true), Entry("disallow repeated egress Types", api.PolicySpec{Types: []api.PolicyType{api.PolicyTypeEgress, api.PolicyTypeEgress}}, false), Entry("disallow unexpected value", api.PolicySpec{Types: []api.PolicyType{"unexpected"}}, false), - Entry("disallow Types without ingress when IngressRules present", + + // In the initial implementation, we validated against the following two cases but we found + // that prevented us from doing a smooth upgrade from type-less to typed policy since we + // couldn't write a policy that would work for back-level Felix instances while also + // specifying the type for up-level Felix instances. + Entry("allow Types without ingress when IngressRules present", api.PolicySpec{ IngressRules: []api.Rule{{Action: "allow"}}, Types: []api.PolicyType{api.PolicyTypeEgress}, - }, false), - Entry("disallow Types without egress when EgressRules present", + }, true), + Entry("allow Types without egress when EgressRules present", api.PolicySpec{ EgressRules: []api.Rule{{Action: "allow"}}, Types: []api.PolicyType{api.PolicyTypeIngress}, - }, false), + }, true), + Entry("allow Types with ingress when IngressRules present", api.PolicySpec{ IngressRules: []api.Rule{{Action: "allow"}},