Skip to content

Commit

Permalink
policy: Improve trace output
Browse files Browse the repository at this point in the history 
Make policy trace output more consistent and easier to follow.

* Track the number of selected rules vs. the total
* Track whether denies were caused by FromRequires constraints
* Print a simple summary of:
        "Found allow rule"
        "Found no allow rule"
        "Found unsatisfied FromRequires constraint"
* Use "selected" to determine rule selection by EndpointSelector, rather
  than using "matched"
* Print when no L4 rules match for ingress/egress L4 policies
* Improve whitespace

Signed-off-by: Joe Stringer <[email protected]>
  • Loading branch information
@joestringer @ianvernon
joestringer authored and ianvernon committed Oct 24, 2017
1 parent b69a052 commit c02a9cf
Show file tree
Hide file tree
Showing 7 changed files with 123 additions and 54 deletions.
17 changes: 11 additions & 6 deletions Documentation/policy.rst
View file
Original file line number Diff line number Diff line change
Expand Up @@ -763,23 +763,28 @@ endpoint with the label ``id.http`` on port 80:
$ cilium policy trace -s id.curl -d id.httpd --dport 80
Tracing From: [container:id.curl] => To: [container:id.httpd] Ports: [80/any]
* Rule {"matchLabels":{"any:id.httpd":""}}: match
* Rule {"matchLabels":{"any:id.httpd":""}}: selected
Allows from labels {"matchLabels":{"any:id.curl":""}}
Found all required labels
Rule restricts traffic to specific L4 destinations; deferring policy decision to L4 policy stage
1 rules matched
1/1 rules selected
Found no allow rule
Label verdict: undecided
Resolving egress port policy for [container:id.curl]
* Rule {"matchLabels":{"any:id.curl":""}}: match
* Rule {"matchLabels":{"any:id.curl":""}}: selected
Allows Egress port [{80 tcp}]
1 rules matched
Found all required labels
1/1 rules selected
Found allow rule
L4 egress verdict: allowed
Resolving ingress port policy for [container:id.httpd]
* Rule {"matchLabels":{"any:id.httpd":""}}: match
* Rule {"matchLabels":{"any:id.httpd":""}}: selected
Allows Ingress port [{80 tcp}]
1 rules matched
Found all required labels
1/1 rules selected
Found allow rule
L4 ingress verdict: allowed
Final verdict: ALLOWED
Expand Down
4 changes: 2 additions & 2 deletions daemon/policy.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -132,8 +132,8 @@ func (h *getPolicyResolve) Handle(params GetPolicyResolveParams) middleware.Resp
if !(d.policy.GetRulesMatching(labels.NewSelectLabelArrayFromModel(params.IdentityContext.From), true) ||
d.policy.GetRulesMatching(labels.NewSelectLabelArrayFromModel(params.IdentityContext.To), true)) {
policyEnforcementMsg = "Policy enforcement is disabled because " +
"no rules in the policy repository match either of the provided " +
"sets of labels."
"no rules in the policy repository match any endpoint selector " +
"from the provided destination sets of labels."
isPolicyEnforcementEnabled = false
}
}
Expand Down
44 changes: 32 additions & 12 deletions pkg/policy/repository.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,28 @@ type traceState struct {
// selectedRules is the number of rules with matching EndpointSelector
selectedRules int

// matchedRules is the number of rules that have allowed traffic
matchedRules int

// constrainedRules counts how many "FromRequires" constraints are
// unsatisfied
constrainedRules int

// ruleID is the rule ID currently being evaluated
ruleID int
}

func (state *traceState) trace(p *Repository, ctx *SearchContext) {
ctx.PolicyTrace("%d/%d rules selected\n", state.selectedRules, len(p.rules))
if state.constrainedRules > 0 {
ctx.PolicyTrace("Found unsatisfied FromRequires constraint\n")
} else if state.matchedRules > 0 {
ctx.PolicyTrace("Found allow rule\n")
} else {
ctx.PolicyTrace("Found no allow rule\n")
}
}

// CanReachRLocked evaluates the policy repository for the provided search
// context and returns the verdict or api.Undecided if no rule matches. The
// policy repository mutex must be held.
Expand All @@ -76,7 +94,7 @@ loop:
}
}

ctx.PolicyTrace("%d rules matched", state.selectedRules)
state.trace(p, ctx)

return decision
}
Expand All @@ -97,7 +115,7 @@ func (p *Repository) AllowsLabelAccess(ctx *SearchContext) api.Decision {
}
}

ctx.PolicyTrace("Label verdict: %s\n", decision.String())
ctx.PolicyTrace("Label verdict: %s", decision.String())

return decision
}
Expand All @@ -113,6 +131,7 @@ func (p *Repository) AllowsLabelAccess(ctx *SearchContext) api.Decision {
func (p *Repository) ResolveL4Policy(ctx *SearchContext) (*L4Policy, error) {
result := NewL4Policy()

ctx.PolicyTrace("\n")
if ctx.EgressL4Only {
ctx.PolicyTrace("Resolving egress port policy for %+v\n", ctx.To)
} else if ctx.IngressL4Only {
Expand All @@ -123,14 +142,17 @@ func (p *Repository) ResolveL4Policy(ctx *SearchContext) (*L4Policy, error) {

state := traceState{}
for _, r := range p.rules {
_, err := r.resolveL4Policy(ctx, &state, result)
found, err := r.resolveL4Policy(ctx, &state, result)
if err != nil {
return nil, err
}
state.ruleID++
if found != nil {
state.matchedRules++
}
}

ctx.PolicyTrace("%d rules matched\n", state.selectedRules)
state.trace(p, ctx)
return result, nil
}

Expand All @@ -149,7 +171,7 @@ func (p *Repository) ResolveL3Policy(ctx *SearchContext) *L3Policy {
state.ruleID++
}

ctx.PolicyTrace("%d rules matched\n", state.selectedRules)
state.trace(p, ctx)
return result
}

Expand All @@ -159,7 +181,6 @@ func (p *Repository) allowsL4Egress(searchCtx *SearchContext) api.Decision {
ctx.From = labels.LabelArray{}
ctx.EgressL4Only = true

ctx.PolicyTrace("\n")
policy, err := p.ResolveL4Policy(&ctx)
if err != nil {
log.WithError(err).Warning("Evaluation error while resolving L4 egress policy")
Expand All @@ -170,9 +191,9 @@ func (p *Repository) allowsL4Egress(searchCtx *SearchContext) api.Decision {
}

if len(ctx.DPorts) == 0 {
ctx.PolicyTrace("L4 egress verdict: [no port context specified]\n")
ctx.PolicyTrace("L4 egress verdict: [no port context specified]")
} else {
ctx.PolicyTrace("L4 egress verdict: %s\n", verdict.String())
ctx.PolicyTrace("L4 egress verdict: %s", verdict.String())
}

return verdict
Expand All @@ -181,7 +202,6 @@ func (p *Repository) allowsL4Egress(searchCtx *SearchContext) api.Decision {
func (p *Repository) allowsL4Ingress(ctx *SearchContext) api.Decision {
ctx.IngressL4Only = true

ctx.PolicyTrace("\n")
policy, err := p.ResolveL4Policy(ctx)
if err != nil {
log.WithError(err).Warning("Evaluation error while resolving L4 ingress policy")
Expand All @@ -192,9 +212,9 @@ func (p *Repository) allowsL4Ingress(ctx *SearchContext) api.Decision {
}

if len(ctx.DPorts) == 0 {
ctx.PolicyTrace("L4 ingress verdict: [no port context specified]\n")
ctx.PolicyTrace("L4 ingress verdict: [no port context specified]")
} else {
ctx.PolicyTrace("L4 ingress verdict: %s\n", verdict.String())
ctx.PolicyTrace("L4 ingress verdict: %s", verdict.String())
}

return verdict
Expand All @@ -207,7 +227,7 @@ func (p *Repository) allowsL4Ingress(ctx *SearchContext) api.Decision {
func (p *Repository) AllowsRLocked(ctx *SearchContext) api.Decision {
ctx.PolicyTrace("Tracing %s\n", ctx.String())
decision := p.CanReachRLocked(ctx)
ctx.PolicyTrace("Label verdict: %s\n", decision.String())
ctx.PolicyTrace("Label verdict: %s", decision.String())
if decision == api.Allowed {
return decision
}
Expand Down
3 changes: 2 additions & 1 deletion pkg/policy/repository_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,8 @@ func (ds *PolicyTestSuite) TestMinikubeGettingStarted(c *C) {
c.Assert(*l4policy, comparator.DeepEquals, *expected)

// L4 from app3 has no rules
expected = NewL4Policy()
l4policy, err = repo.ResolveL4Policy(fromApp3)
c.Assert(len(l4policy.Ingress), Equals, 1)
c.Assert(len(l4policy.Ingress), Equals, 0)
c.Assert(*l4policy, comparator.DeepEquals, *expected)
}
54 changes: 36 additions & 18 deletions pkg/policy/rule.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -172,23 +172,28 @@ func mergeL4Port(ctx *SearchContext, fromEndpoints []api.EndpointSelector, r api
func mergeL4(ctx *SearchContext, dir string, fromEndpoints []api.EndpointSelector, portRules []api.PortRule,
resMap L4PolicyMap) (int, error) {

if len(portRules) == 0 {
ctx.PolicyTrace(" No L4 rules\n")
return 0, nil
}

found := 0
var err error

for _, r := range portRules {
if fromEndpoints != nil {
ctx.PolicyTrace(" Allows %s port %v from endpoints %v\n", dir, r.Ports, fromEndpoints)
ctx.PolicyTrace(" Allows %s port %v from endpoints %v\n", dir, r.Ports, fromEndpoints)
} else {
ctx.PolicyTrace(" Allows %s port %v\n", dir, r.Ports)
ctx.PolicyTrace(" Allows %s port %v\n", dir, r.Ports)
}

if r.RedirectPort != 0 {
ctx.PolicyTrace(" Redirect-To: %d\n", r.RedirectPort)
ctx.PolicyTrace(" Redirect-To: %d\n", r.RedirectPort)
}

if r.Rules != nil {
for _, l7 := range r.Rules.HTTP {
ctx.PolicyTrace(" %+v\n", l7)
ctx.PolicyTrace(" %+v\n", l7)
}
}

Expand Down Expand Up @@ -234,17 +239,28 @@ func mergeL4(ctx *SearchContext, dir string, fromEndpoints []api.EndpointSelecto
return found, nil
}

func (state *traceState) selectRule(ctx *SearchContext, r *rule) {
ctx.PolicyTrace("* Rule %s: selected\n", r)
state.selectedRules++
}

func (state *traceState) unSelectRule(ctx *SearchContext, r *rule) {
ctx.PolicyTraceVerbose(" Rule %s: did not select %+v\n", r, ctx.To)
}

func (r *rule) resolveL4Policy(ctx *SearchContext, state *traceState, result *L4Policy) (*L4Policy, error) {
if !r.EndpointSelector.Matches(ctx.To) {
ctx.PolicyTraceVerbose(" Rule %s: no match\n", r)
state.unSelectRule(ctx, r)
return nil, nil
}

state.selectedRules++
ctx.PolicyTrace("* Rule %s: match\n", r)
state.selectRule(ctx, r)
found := 0

if !ctx.EgressL4Only {
if len(r.Ingress) == 0 {
ctx.PolicyTrace(" No L4 rules\n")
}
for _, r := range r.Ingress {
cnt, err := mergeL4(ctx, "Ingress", r.FromEndpoints, r.ToPorts, result.Ingress)
if err != nil {
Expand All @@ -255,6 +271,9 @@ func (r *rule) resolveL4Policy(ctx *SearchContext, state *traceState, result *L4
}

if !ctx.IngressL4Only {
if len(r.Egress) == 0 {
ctx.PolicyTrace(" No L4 rules\n")
}
for _, r := range r.Egress {
cnt, err := mergeL4(ctx, "Egress", nil, r.ToPorts, result.Egress)
if err != nil {
Expand All @@ -268,7 +287,6 @@ func (r *rule) resolveL4Policy(ctx *SearchContext, state *traceState, result *L4
return result, nil
}

ctx.PolicyTrace(" No L4 rules\n")
return nil, nil
}

Expand Down Expand Up @@ -311,12 +329,11 @@ func computeResultantCIDRSet(cidrs []api.CIDRRule) []api.CIDR {

func (r *rule) resolveL3Policy(ctx *SearchContext, state *traceState, result *L3Policy) *L3Policy {
if !r.EndpointSelector.Matches(ctx.To) {
ctx.PolicyTraceVerbose(" Rule %s: no match\n", r)
state.unSelectRule(ctx, r)
return nil
}

state.selectedRules++
ctx.PolicyTrace("* Rule %s: match\n", r)
state.selectRule(ctx, r)
found := 0

for _, r := range r.Ingress {
Expand Down Expand Up @@ -351,22 +368,20 @@ func (r *rule) canReach(ctx *SearchContext, state *traceState) api.Decision {

if !r.EndpointSelector.Matches(ctx.To) {
if entitiesDecision == api.Undecided {
ctx.PolicyTraceVerbose(" Rule %s: no match for %+v\n", r, ctx.To)
state.unSelectRule(ctx, r)
} else {
state.selectedRules++
ctx.PolicyTrace("* Rule %s: match\n", r)
state.selectRule(ctx, r)
}
return entitiesDecision
}

state.selectedRules++
ctx.PolicyTrace("* Rule %s: match\n", r)

state.selectRule(ctx, r)
for _, r := range r.Ingress {
for _, sel := range r.FromRequires {
ctx.PolicyTrace(" Requires from labels %+v", sel)
if !sel.Matches(ctx.From) {
ctx.PolicyTrace("- Labels %v not found\n", ctx.From)
state.constrainedRules++
return api.Denied
}
ctx.PolicyTrace("+ Found all required labels\n")
Expand All @@ -381,7 +396,8 @@ func (r *rule) canReach(ctx *SearchContext, state *traceState) api.Decision {
if sel.Matches(ctx.From) {
ctx.PolicyTrace(" Found all required labels")
if len(r.ToPorts) == 0 {
ctx.PolicyTrace("+ No L4 restrictions; allowing\n")
ctx.PolicyTrace("+ No L4 restrictions\n")
state.matchedRules++
return api.Allowed
}
ctx.PolicyTrace(" Rule restricts traffic to specific L4 destinations; deferring policy decision to L4 policy stage\n")
Expand All @@ -394,6 +410,7 @@ func (r *rule) canReach(ctx *SearchContext, state *traceState) api.Decision {
for _, entitySelector := range r.fromEntities {
if entitySelector.Matches(ctx.From) {
ctx.PolicyTrace("+ Found all required labels to match entity %s\n", entitySelector.String())
state.matchedRules++
return api.Allowed
}

Expand All @@ -406,6 +423,7 @@ func (r *rule) canReachEntities(ctx *SearchContext, state *traceState) api.Decis
for _, entitySelector := range r.toEntities {
if entitySelector.Matches(ctx.To) {
ctx.PolicyTrace("+ Found all required labels to match entity %s\n", entitySelector.String())
state.matchedRules++
return api.Allowed
}
}
Expand Down
Loading

0 comments on commit c02a9cf

Please sign in to comment.