Skip to content

Commit

Permalink
Add metadata to annotation-derived l7 policy
Browse files Browse the repository at this point in the history
This change adds metadata labels to L7 DNS allow-all visibility policies
derived from pod annotation in order to inform user about which pod
annotations caused policy creation.

Signed-off-by: Maciej Kwiek <maciej@isovalent.com>
  • Loading branch information
nebril committed Apr 24, 2024
1 parent d176dd6 commit 20cc159
Show file tree
Hide file tree
Showing 4 changed files with 40 additions and 27 deletions.
2 changes: 1 addition & 1 deletion pkg/endpoint/events.go
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ func (ev *EndpointPolicyVisibilityEvent) Handle(res chan interface{}) {
return
}
e.getLogger().Debug("creating visibility policy")
nvp, err = policy.NewVisibilityPolicy(proxyVisibility)
nvp, err = policy.NewVisibilityPolicy(proxyVisibility, e.K8sNamespace, e.K8sPodName)
if err != nil {
e.getLogger().WithError(err).Warning("unable to parse annotations into visibility policy; disabling visibility policy for endpoint")
e.visibilityPolicy = &policy.VisibilityPolicy{
Expand Down
2 changes: 1 addition & 1 deletion pkg/policy/selectorcache_selector.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ type CachedSelectionUser interface {
// new identitySelectors are pre-populated from the set of currently
// known identities.
//
// 2. When reachacble identities appear or disappear, either via local
// 2. When reachable identities appear or disappear, either via local
// allocation (CIDRs), or via the KV-store (remote endpoints). In this
// case all existing identitySelectors are walked through and their
// cached selections are updated as necessary.
Expand Down
14 changes: 11 additions & 3 deletions pkg/policy/visibility.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import (
"strconv"
"strings"

ciliumio "github.com/cilium/cilium/pkg/k8s/apis/cilium.io"
"github.com/cilium/cilium/pkg/labels"
"github.com/cilium/cilium/pkg/policy/api"
"github.com/cilium/cilium/pkg/u8proto"
)
Expand Down Expand Up @@ -42,7 +44,7 @@ func validateL7ProtocolWithDirection(dir string, proto L7ParserType) error {
// format for a visibility annotation.
// - if there is a conflict between the state encoded in the annotation (e.g.,
// different L7 protocols for the same L4 port / protocol / traffic direction.
func NewVisibilityPolicy(anno string) (*VisibilityPolicy, error) {
func NewVisibilityPolicy(anno, namespace, pod string) (*VisibilityPolicy, error) {
if !annotationRegex.MatchString(anno) {
return nil, fmt.Errorf("annotation for proxy visibility did not match expected format %s", annotationRegex.String())
}
Expand Down Expand Up @@ -112,7 +114,7 @@ func NewVisibilityPolicy(anno string) (*VisibilityPolicy, error) {
}
}

l7Meta := generateL7AllowAllRules(l7Protocol)
l7Meta := generateL7AllowAllRules(l7Protocol, namespace, pod)

dvp[pp] = &VisibilityMetadata{
Parser: l7Protocol,
Expand All @@ -127,13 +129,19 @@ func NewVisibilityPolicy(anno string) (*VisibilityPolicy, error) {
return nvp, nil
}

func generateL7AllowAllRules(parser L7ParserType) L7DataMap {
func generateL7AllowAllRules(parser L7ParserType, namespace, pod string) L7DataMap {
var m L7DataMap
switch parser {
case ParserTypeDNS:
m = L7DataMap{}
// Create an entry to explicitly allow all at L7 for DNS.
emptyL3Selector := &identitySelector{source: &labelIdentitySelector{selector: api.WildcardEndpointSelector}, key: wildcardSelectorKey}
emptyL3Selector.metadataLbls = labels.LabelArray{
labels.NewLabel(ciliumio.PolicyLabelDerivedFrom, "PodVisibilityAnnotation", labels.LabelSourceK8s),
labels.NewLabel(ciliumio.PodNamespaceLabel, namespace, labels.LabelSourceK8s),
labels.NewLabel(ciliumio.PodNameLabel, pod, labels.LabelSourceK8s),
}

m[emptyL3Selector] = &PerSelectorPolicy{
L7Rules: api.L7Rules{
DNS: []api.PortRuleDNS{
Expand Down
49 changes: 27 additions & 22 deletions pkg/policy/visibility_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,35 +7,40 @@ import (
. "github.com/cilium/checkmate"

"github.com/cilium/cilium/pkg/checker"
ciliumio "github.com/cilium/cilium/pkg/k8s/apis/cilium.io"
"github.com/cilium/cilium/pkg/labels"
"github.com/cilium/cilium/pkg/policy/api"
"github.com/cilium/cilium/pkg/u8proto"
)

func (ds *PolicyTestSuite) TestGenerateL7RulesByParser(c *C) {
m := generateL7AllowAllRules(ParserTypeHTTP)
m := generateL7AllowAllRules(ParserTypeHTTP, "", "")
c.Assert(m, IsNil)

m = generateL7AllowAllRules(ParserTypeKafka)
m = generateL7AllowAllRules(ParserTypeKafka, "", "")
c.Assert(m, IsNil)

m = generateL7AllowAllRules(ParserTypeDNS)
m = generateL7AllowAllRules(ParserTypeDNS, "ns-name", "pod-name")
c.Assert(m, Not(IsNil))
c.Assert(len(m), Equals, 1)

l7Rules := make([]*PerSelectorPolicy, 0, len(m))
for _, v := range m {
l7Rules = append(l7Rules, v)
}
for k, v := range m {
// Check that we allow all at L7 for DNS for the one rule we should have
// generated.
c.Assert(v, checker.DeepEquals, &PerSelectorPolicy{L7Rules: api.L7Rules{DNS: []api.PortRuleDNS{{MatchPattern: "*"}}}})

// Check that we allow all at L7 for DNS for the one rule we should have
// generated.
c.Assert(l7Rules[0], checker.DeepEquals, &PerSelectorPolicy{L7Rules: api.L7Rules{DNS: []api.PortRuleDNS{{MatchPattern: "*"}}}})
c.Assert(k.GetMetadataLabels(), checker.DeepEquals, labels.LabelArray{
labels.NewLabel(ciliumio.PolicyLabelDerivedFrom, "PodVisibilityAnnotation", labels.LabelSourceK8s),
labels.NewLabel(ciliumio.PodNamespaceLabel, "ns-name", labels.LabelSourceK8s),
labels.NewLabel(ciliumio.PodNameLabel, "pod-name", labels.LabelSourceK8s),
})
}
}

func (ds *PolicyTestSuite) TestVisibilityPolicyCreation(c *C) {

anno := "<Ingress/80/TCP/HTTP>"
vp, err := NewVisibilityPolicy(anno)
vp, err := NewVisibilityPolicy(anno, "", "")
c.Assert(vp, Not(IsNil))
c.Assert(err, IsNil)

Expand All @@ -48,7 +53,7 @@ func (ds *PolicyTestSuite) TestVisibilityPolicyCreation(c *C) {
})

anno = "<Ingress/80/TCP/HTTP>,<Ingress/8080/TCP/HTTP>"
vp, err = NewVisibilityPolicy(anno)
vp, err = NewVisibilityPolicy(anno, "", "")
c.Assert(vp, Not(IsNil))
c.Assert(err, IsNil)

Expand All @@ -67,7 +72,7 @@ func (ds *PolicyTestSuite) TestVisibilityPolicyCreation(c *C) {
})

anno = "<Ingress/80/TCP/HTTP>,<Ingress/80/TCP/HTTP>"
vp, err = NewVisibilityPolicy(anno)
vp, err = NewVisibilityPolicy(anno, "", "")
c.Assert(vp, Not(IsNil))
c.Assert(err, IsNil)

Expand All @@ -80,49 +85,49 @@ func (ds *PolicyTestSuite) TestVisibilityPolicyCreation(c *C) {
})

anno = "<Ingress/80/TCP/HTTP>,<Ingress/80/TCP/Kafka>"
vp, err = NewVisibilityPolicy(anno)
vp, err = NewVisibilityPolicy(anno, "", "")
c.Assert(vp, IsNil)
c.Assert(err, Not(IsNil))

anno = "asdf"
vp, err = NewVisibilityPolicy(anno)
vp, err = NewVisibilityPolicy(anno, "", "")
c.Assert(vp, IsNil)
c.Assert(err, Not(IsNil))

anno = "<Ingress/65536/TCP/HTTP>"
vp, err = NewVisibilityPolicy(anno)
vp, err = NewVisibilityPolicy(anno, "", "")
c.Assert(vp, IsNil)
c.Assert(err, Not(IsNil))

anno = "<Ingress/65535/TCP/HTTP>"
vp, err = NewVisibilityPolicy(anno)
vp, err = NewVisibilityPolicy(anno, "", "")
c.Assert(vp, Not(IsNil))
c.Assert(err, IsNil)

anno = "<Ingress/99999/TCP/HTTP>"
vp, err = NewVisibilityPolicy(anno)
vp, err = NewVisibilityPolicy(anno, "", "")
c.Assert(vp, IsNil)
c.Assert(err, Not(IsNil))

anno = "<Ingress/0/TCP/HTTP>"
vp, err = NewVisibilityPolicy(anno)
vp, err = NewVisibilityPolicy(anno, "", "")
c.Assert(vp, IsNil)
c.Assert(err, Not(IsNil))

// Do not allow > 5 digits.
anno = "<Ingress/123456/TCP/HTTP"
vp, err = NewVisibilityPolicy(anno)
vp, err = NewVisibilityPolicy(anno, "", "")
c.Assert(vp, IsNil)
c.Assert(err, Not(IsNil))

// Do not allow leading zeroes.
anno = "<Ingress/02345/TCP/HTTP"
vp, err = NewVisibilityPolicy(anno)
vp, err = NewVisibilityPolicy(anno, "", "")
c.Assert(vp, IsNil)
c.Assert(err, Not(IsNil))

anno = "<Egress/53/ANY/DNS>"
vp, err = NewVisibilityPolicy(anno)
vp, err = NewVisibilityPolicy(anno, "", "")
c.Assert(err, IsNil)
c.Assert(vp.Egress, HasLen, 3)
udp, ok := vp.Egress["53/UDP"]
Expand Down

0 comments on commit 20cc159

Please sign in to comment.