Skip to content

Commit

Permalink
fix #267,#200,#168
Browse files Browse the repository at this point in the history
  • Loading branch information
@derailed
derailed committed Dec 30, 2024
1 parent 360e011 commit d23937a
Show file tree
Hide file tree
Showing 21 changed files with 371 additions and 74 deletions.
2 changes: 1 addition & 1 deletion internal/lint/cronjob.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ func (s *CronJob) Lint(ctx context.Context) error {
cj := o.(*batchv1.CronJob)
fqn := client.FQN(cj.Namespace, cj.Name)
s.InitOutcome(fqn)
ctx = internal.WithSpec(ctx, SpecFor(fqn, cj))
ctx = internal.WithSpec(ctx, coSpecFor(fqn, cj, cj.Spec.JobTemplate.Spec.Template.Spec))
s.checkCronJob(ctx, fqn, cj)
s.checkContainers(ctx, fqn, cj.Spec.JobTemplate.Spec.Template.Spec)
s.checkUtilization(ctx, over, fqn)
Expand Down
2 changes: 1 addition & 1 deletion internal/lint/dp.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ func (s *Deployment) Lint(ctx context.Context) error {
dp := o.(*appsv1.Deployment)
fqn := client.FQN(dp.Namespace, dp.Name)
s.InitOutcome(fqn)
ctx = internal.WithSpec(ctx, SpecFor(fqn, dp))
ctx = internal.WithSpec(ctx, coSpecFor(fqn, dp, dp.Spec.Template.Spec))
s.checkDeployment(ctx, dp)
s.checkContainers(ctx, fqn, dp.Spec.Template.Spec)
s.checkUtilization(ctx, over, dp)
Expand Down
2 changes: 1 addition & 1 deletion internal/lint/ds.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ func (s *DaemonSet) Lint(ctx context.Context) error {
ds := o.(*appsv1.DaemonSet)
fqn := client.FQN(ds.Namespace, ds.Name)
s.InitOutcome(fqn)
ctx = internal.WithSpec(ctx, SpecFor(fqn, ds))
ctx = internal.WithSpec(ctx, coSpecFor(fqn, ds, ds.Spec.Template.Spec))

s.checkDaemonSet(ctx, ds)
s.checkContainers(ctx, fqn, ds.Spec.Template.Spec)
Expand Down
21 changes: 20 additions & 1 deletion internal/lint/helper.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,25 @@ const (

type qos = int

func coSpecFor(fqn string, o metav1.ObjectMetaAccessor, spec v1.PodSpec) rules.Spec {
rule := SpecFor(fqn, o)
rule.Containers = fetchContainers(spec)

return rule
}

func fetchContainers(podTemplate v1.PodSpec) []string {
containers := make([]string, 0, len(podTemplate.InitContainers)+len(podTemplate.Containers))
for _, co := range podTemplate.InitContainers {
containers = append(containers, co.Name)
}
for _, co := range podTemplate.Containers {
containers = append(containers, co.Name)
}

return containers
}

// SpecFor construct a new run spec for a given resource.
func SpecFor(fqn string, o metav1.ObjectMetaAccessor) rules.Spec {
spec := rules.Spec{
Expand Down Expand Up @@ -146,7 +165,7 @@ func asMB(q resource.Quantity) string {
return fmt.Sprintf("%vMi", toMB(q))
}

// PodResources computes pod resouces as sum of containers allocations.
// PodResources computes pod resources as sum of containers allocations.
func podResources(spec v1.PodSpec) (cpu, mem resource.Quantity) {
for _, co := range spec.InitContainers {
c, m, _ := containerResources(co)
Expand Down
2 changes: 1 addition & 1 deletion internal/lint/job.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ func (s *Job) Lint(ctx context.Context) error {
j := o.(*batchv1.Job)
fqn := client.FQN(j.Namespace, j.Name)
s.InitOutcome(fqn)
ctx = internal.WithSpec(ctx, SpecFor(fqn, j))
ctx = internal.WithSpec(ctx, coSpecFor(fqn, j, j.Spec.Template.Spec))
s.checkJob(ctx, fqn, j)
s.checkContainers(ctx, fqn, j.Spec.Template.Spec)
s.checkUtilization(ctx, over, fqn)
Expand Down
28 changes: 15 additions & 13 deletions internal/lint/np.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,13 @@ type direction string
const (
dirIn direction = "Ingress"
dirOut direction = "Egress"
bothPols = "All"
bothPols = "all"
noPols = ""
ingress = "ingress"
egress = "egress"
)

// NetworkPolicy tracks NetworkPolicy sanitizatios.
// NetworkPolicy tracks NetworkPolicy linting.
type NetworkPolicy struct {
*issues.Collector

Expand Down Expand Up @@ -57,13 +59,13 @@ func (s *NetworkPolicy) Lint(ctx context.Context) error {
s.checkSelector(ctx, fqn, np.Spec.PodSelector)
s.checkIngresses(ctx, fqn, np.Spec.Ingress)
s.checkEgresses(ctx, fqn, np.Spec.Egress)
s.checkRuleType(ctx, fqn, &np.Spec)
s.checkRuleType(ctx, &np.Spec)
}

return nil
}

func (s *NetworkPolicy) checkRuleType(ctx context.Context, fqn string, spec *netv1.NetworkPolicySpec) {
func (s *NetworkPolicy) checkRuleType(ctx context.Context, spec *netv1.NetworkPolicySpec) {
if spec.PodSelector.Size() > 0 {
return
}
Expand All @@ -72,15 +74,15 @@ func (s *NetworkPolicy) checkRuleType(ctx context.Context, fqn string, spec *net
case isAllowAll(spec):
s.AddCode(ctx, 1203, "Allow", bothPols)
case isAllowAllIngress(spec):
s.AddCode(ctx, 1203, "Allow All", dirIn)
s.AddCode(ctx, 1203, "Allow all", ingress)
case isAllowAllEgress(spec):
s.AddCode(ctx, 1203, "Allow All", dirOut)
s.AddCode(ctx, 1203, "Allow all", egress)
case isDenyAll(spec):
s.AddCode(ctx, 1203, "Deny", bothPols)
case isDenyAllIngress(spec):
s.AddCode(ctx, 1203, "Deny All", dirIn)
s.AddCode(ctx, 1203, "Deny all", ingress)
case isDenyAllEgress(spec):
s.AddCode(ctx, 1203, "Deny All", dirOut)
s.AddCode(ctx, 1203, "Deny all", egress)
}
}

Expand Down Expand Up @@ -157,7 +159,7 @@ func (s *NetworkPolicy) checkIPBlocks(ctx context.Context, fqn string, b *netv1.
s.AddErr(ctx, err)
}
if !s.matchPips(ns, ipnet) {
s.AddCode(ctx, 1206, d, b.CIDR)
s.AddCode(ctx, 1206, strings.ToLower(string(d)), b.CIDR)
}
for _, ex := range b.Except {
_, ipnet, err := net.ParseCIDR(ex)
Expand All @@ -166,7 +168,7 @@ func (s *NetworkPolicy) checkIPBlocks(ctx context.Context, fqn string, b *netv1.
continue
}
if !s.matchPips(ns, ipnet) {
s.AddCode(ctx, 1207, d, ex)
s.AddCode(ctx, 1207, strings.ToLower(string(d)), ex)
}
}
}
Expand Down Expand Up @@ -210,16 +212,16 @@ func (s *NetworkPolicy) checkPodSelector(ctx context.Context, nss []*v1.Namespac
}
if !found {
if len(nn) > 0 {
s.AddCode(ctx, 1208, d, dumpSel(sel), strings.Join(nn, ","))
s.AddCode(ctx, 1208, strings.ToLower(string(d)), dumpSel(sel), strings.Join(nn, ","))
} else {
s.AddCode(ctx, 1202, d, dumpSel(sel))
s.AddCode(ctx, 1202, strings.ToLower(string(d)), dumpSel(sel))
}
}
}

func (s *NetworkPolicy) checkNSSelector(ctx context.Context, sel *metav1.LabelSelector, nss []*v1.Namespace, d direction) bool {
if len(nss) == 0 {
s.AddCode(ctx, 1201, d, dumpSel(sel))
s.AddCode(ctx, 1201, strings.ToLower(string(d)), dumpSel(sel))
return false
}

Expand Down
36 changes: 18 additions & 18 deletions internal/lint/np_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,46 +31,46 @@ func TestNPLintDenyAll(t *testing.T) {

ii := np.Outcome()["default/deny-all"]
assert.Equal(t, 1, len(ii))
assert.Equal(t, `[POP-1203] Deny All policy in effect`, ii[0].Message)
assert.Equal(t, `[POP-1203] Deny all policy in effect`, ii[0].Message)
assert.Equal(t, rules.InfoLevel, ii[0].Level)

ii = np.Outcome()["default/deny-all-ing"]
assert.Equal(t, 1, len(ii))
assert.Equal(t, `[POP-1203] Deny All Ingress policy in effect`, ii[0].Message)
assert.Equal(t, `[POP-1203] Deny all ingress policy in effect`, ii[0].Message)
assert.Equal(t, rules.InfoLevel, ii[0].Level)

ii = np.Outcome()["default/deny-all-eg"]
assert.Equal(t, 1, len(ii))
assert.Equal(t, `[POP-1203] Deny All Egress policy in effect`, ii[0].Message)
assert.Equal(t, `[POP-1203] Deny all egress policy in effect`, ii[0].Message)
assert.Equal(t, rules.InfoLevel, ii[0].Level)

ii = np.Outcome()["default/allow-all"]
assert.Equal(t, 1, len(ii))
assert.Equal(t, `[POP-1203] Allow All policy in effect`, ii[0].Message)
assert.Equal(t, `[POP-1203] Allow all policy in effect`, ii[0].Message)
assert.Equal(t, rules.InfoLevel, ii[0].Level)

ii = np.Outcome()["default/allow-all-ing"]
assert.Equal(t, 1, len(ii))
assert.Equal(t, `[POP-1203] Allow All Ingress policy in effect`, ii[0].Message)
assert.Equal(t, `[POP-1203] Allow all ingress policy in effect`, ii[0].Message)
assert.Equal(t, rules.InfoLevel, ii[0].Level)

ii = np.Outcome()["default/allow-all-eg"]
assert.Equal(t, 1, len(ii))
assert.Equal(t, `[POP-1203] Allow All Egress policy in effect`, ii[0].Message)
assert.Equal(t, `[POP-1203] Allow all egress policy in effect`, ii[0].Message)
assert.Equal(t, rules.InfoLevel, ii[0].Level)

ii = np.Outcome()["default/ip-block-all-ing"]
assert.Equal(t, 2, len(ii))
assert.Equal(t, `[POP-1206] No pods matched Egress IPBlock 172.2.0.0/24`, ii[0].Message)
assert.Equal(t, `[POP-1206] No pods matched egress IPBlock 172.2.0.0/24`, ii[0].Message)
assert.Equal(t, rules.WarnLevel, ii[0].Level)
assert.Equal(t, `[POP-1203] Deny All Ingress policy in effect`, ii[1].Message)
assert.Equal(t, `[POP-1203] Deny all ingress policy in effect`, ii[1].Message)
assert.Equal(t, rules.InfoLevel, ii[1].Level)

ii = np.Outcome()["default/ip-block-all-eg"]
assert.Equal(t, 2, len(ii))
assert.Equal(t, `[POP-1206] No pods matched Ingress IPBlock 172.2.0.0/24`, ii[0].Message)
assert.Equal(t, `[POP-1206] No pods matched ingress IPBlock 172.2.0.0/24`, ii[0].Message)
assert.Equal(t, rules.WarnLevel, ii[0].Level)
assert.Equal(t, `[POP-1203] Deny All Egress policy in effect`, ii[1].Message)
assert.Equal(t, `[POP-1203] Deny all egress policy in effect`, ii[1].Message)
assert.Equal(t, rules.InfoLevel, ii[1].Level)
}

Expand All @@ -93,26 +93,26 @@ func TestNPLint(t *testing.T) {

ii = np.Outcome()["default/np2"]
assert.Equal(t, 3, len(ii))
assert.Equal(t, `[POP-1207] No pods matched except Ingress IPBlock 172.1.1.0/24`, ii[0].Message)
assert.Equal(t, `[POP-1207] No pods matched except ingress IPBlock 172.1.1.0/24`, ii[0].Message)
assert.Equal(t, rules.WarnLevel, ii[0].Level)
assert.Equal(t, `[POP-1208] No pods match Ingress pod selector: app=p2 in namespace: ns2`, ii[1].Message)
assert.Equal(t, `[POP-1208] No pods match ingress pod selector: app=p2 in namespace: ns2`, ii[1].Message)
assert.Equal(t, rules.WarnLevel, ii[1].Level)
assert.Equal(t, `[POP-1206] No pods matched Egress IPBlock 172.0.0.0/24`, ii[2].Message)
assert.Equal(t, `[POP-1206] No pods matched egress IPBlock 172.0.0.0/24`, ii[2].Message)
assert.Equal(t, rules.WarnLevel, ii[2].Level)

ii = np.Outcome()["default/np3"]
assert.Equal(t, 6, len(ii))
assert.Equal(t, `[POP-1200] No pods match pod selector: app=p-bozo`, ii[0].Message)
assert.Equal(t, rules.WarnLevel, ii[0].Level)
assert.Equal(t, `[POP-1206] No pods matched Ingress IPBlock 172.2.0.0/16`, ii[1].Message)
assert.Equal(t, `[POP-1206] No pods matched ingress IPBlock 172.2.0.0/16`, ii[1].Message)
assert.Equal(t, rules.WarnLevel, ii[1].Level)
assert.Equal(t, `[POP-1207] No pods matched except Ingress IPBlock 172.2.1.0/24`, ii[2].Message)
assert.Equal(t, `[POP-1207] No pods matched except ingress IPBlock 172.2.1.0/24`, ii[2].Message)
assert.Equal(t, rules.WarnLevel, ii[2].Level)
assert.Equal(t, `[POP-1201] No namespaces match Ingress namespace selector: app-In-ns-bozo`, ii[3].Message)
assert.Equal(t, `[POP-1201] No namespaces match ingress namespace selector: app-In-ns-bozo`, ii[3].Message)
assert.Equal(t, rules.WarnLevel, ii[3].Level)
assert.Equal(t, `[POP-1202] No pods match Ingress pod selector: app=pod-bozo`, ii[4].Message)
assert.Equal(t, `[POP-1202] No pods match ingress pod selector: app=pod-bozo`, ii[4].Message)
assert.Equal(t, rules.WarnLevel, ii[4].Level)
assert.Equal(t, `[POP-1208] No pods match Egress pod selector: app=p1-missing in namespace: default`, ii[5].Message)
assert.Equal(t, `[POP-1208] No pods match egress pod selector: app=p1-missing in namespace: default`, ii[5].Message)
assert.Equal(t, rules.WarnLevel, ii[5].Level)
}

Expand Down
48 changes: 32 additions & 16 deletions internal/lint/pod.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ func NewPod(co *issues.Collector, db *db.DB) *Pod {

// Lint cleanse the resource..
func (s *Pod) Lint(ctx context.Context) error {
boundSA := boundDefaultSA(s.db)
txn, it := s.db.MustITFor(internal.Glossary[internal.PO])
defer txn.Abort()
for o := it.Next(); o != nil; o = it.Next() {
Expand All @@ -59,7 +60,7 @@ func (s *Pod) Lint(ctx context.Context) error {
s.InitOutcome(fqn)
defer s.CloseOutcome(ctx, fqn, nil)

ctx = internal.WithSpec(ctx, SpecFor(fqn, po))
ctx = internal.WithSpec(ctx, coSpecFor(fqn, po, po.Spec))
s.checkStatus(ctx, po)
s.checkContainerStatus(ctx, fqn, po)
s.checkContainers(ctx, fqn, po)
Expand All @@ -69,7 +70,7 @@ func (s *Pod) Lint(ctx context.Context) error {
s.checkPdb(ctx, po.ObjectMeta.Labels)
}
s.checkForMultiplePdbMatches(ctx, po.Namespace, po.ObjectMeta.Labels)
s.checkSecure(ctx, fqn, po.Spec)
s.checkSecure(ctx, fqn, po.Spec, boundSA)

pmx, err := s.db.FindPMX(fqn)
if err != nil {
Expand All @@ -96,41 +97,52 @@ func (s *Pod) checkNPs(ctx context.Context, pod *v1.Pod) {
txn, it := s.db.MustITForNS(internal.Glossary[internal.NP], pod.Namespace)
defer txn.Abort()

const (
in = 0
out = 1
)
matches := [2]int{}
for o := it.Next(); o != nil; o = it.Next() {
np := o.(*netv1.NetworkPolicy)
if isDenyAll(&np.Spec) || isAllowAll(&np.Spec) {
return
}
if isDenyAllIngress(&np.Spec) || isAllowAllIngress(&np.Spec) {
matches[0]++
matches[in]++
if s.checkEgresses(ctx, pod, np.Spec.Egress) {
matches[1]++
matches[out]++
}
continue
}
if isDenyAllEgress(&np.Spec) || isAllowAllEgress(&np.Spec) {
matches[1]++
matches[out]++
if s.checkIngresses(ctx, pod, np.Spec.Ingress) {
matches[0]++
matches[in]++
}
continue
}
if labelsMatch(&np.Spec.PodSelector, pod.Labels) {
if polInclude(np.Spec.PolicyTypes, dirIn) {
matches[in]++
}
if polInclude(np.Spec.PolicyTypes, dirOut) {
matches[out]++
}
} else {
if s.checkIngresses(ctx, pod, np.Spec.Ingress) {
matches[0]++
matches[out]++
}
if s.checkEgresses(ctx, pod, np.Spec.Egress) {
matches[1]++
matches[in]++
}
}
}

if matches[0] == 0 {
s.AddCode(ctx, 1204, dirIn)
if matches[in] == 0 {
s.AddCode(ctx, 1204, ingress)
}
if matches[1] == 0 {
s.AddCode(ctx, 1204, dirOut)
if matches[out] == 0 {
s.AddCode(ctx, 1204, egress)
}
}

Expand Down Expand Up @@ -285,17 +297,21 @@ func (s *Pod) checkUtilization(ctx context.Context, fqn string, po *v1.Pod, cmx
}
}

func (s *Pod) checkSecure(ctx context.Context, fqn string, spec v1.PodSpec) {
if err := s.checkSA(ctx, fqn, spec); err != nil {
func (s *Pod) checkSecure(ctx context.Context, fqn string, spec v1.PodSpec, boundSA bool) {
if err := s.checkSA(ctx, fqn, spec, boundSA); err != nil {
s.AddErr(ctx, err)
}
s.checkSecContext(ctx, fqn, spec)
}

func (s *Pod) checkSA(ctx context.Context, fqn string, spec v1.PodSpec) error {
func (s *Pod) checkSA(ctx context.Context, fqn string, spec v1.PodSpec, boundSA bool) error {
ns, _ := namespaced(fqn)
if spec.ServiceAccountName == "default" {
s.AddCode(ctx, 300)
if boundSA {
s.AddCode(ctx, 308)
} else {
s.AddCode(ctx, 300)
}
}

txn := s.db.Txn(false)
Expand Down
Loading

0 comments on commit d23937a

Please sign in to comment.