From 23071fd2e6b421f0f5fcd6e7e4985c6900e5405c Mon Sep 17 00:00:00 2001 From: Alex Jones Date: Thu, 13 Apr 2023 21:25:41 +0100 Subject: [PATCH 1/6] chore: additional analyzers Signed-off-by: Alex Jones --- go.mod | 1 + go.sum | 2 + pkg/analyzer/analyzer.go | 2 + pkg/analyzer/cronjob.go | 76 +++++++++++++++++++ pkg/analyzer/cronjob_test.go | 130 ++++++++++++++++++++++++++++++++ pkg/analyzer/deployment.go | 46 +++++++++++ pkg/analyzer/deployment_test.go | 64 ++++++++++++++++ pkg/analyzer/netpol.go | 66 ++++++++++++++++ pkg/analyzer/netpol_test.go | 122 ++++++++++++++++++++++++++++++ pkg/common/types.go | 1 + pkg/util/util.go | 17 +++++ 11 files changed, 527 insertions(+) create mode 100644 pkg/analyzer/cronjob.go create mode 100644 pkg/analyzer/cronjob_test.go create mode 100644 pkg/analyzer/deployment.go create mode 100644 pkg/analyzer/deployment_test.go create mode 100644 pkg/analyzer/netpol.go create mode 100644 pkg/analyzer/netpol_test.go diff --git a/go.mod b/go.mod index 01a3541..7a3178c 100644 --- a/go.mod +++ b/go.mod @@ -119,6 +119,7 @@ require ( github.com/prometheus/common v0.37.0 // indirect github.com/prometheus/procfs v0.8.0 // indirect github.com/rivo/uniseg v0.4.4 // indirect + github.com/robfig/cron/v3 v3.0.1 github.com/rubenv/sql-migrate v1.3.1 // indirect github.com/russross/blackfriday/v2 v2.1.0 // indirect github.com/samber/lo v1.37.0 // indirect diff --git a/go.sum b/go.sum index 6050187..846f5f0 100644 --- a/go.sum +++ b/go.sum @@ -607,6 +607,8 @@ github.com/prometheus/tsdb v0.7.1/go.mod h1:qhTCs0VvXwvX/y3TZrWD7rabWM+ijKTux40T github.com/rivo/uniseg v0.2.0/go.mod h1:J6wj4VEh+S6ZtnVlnTBMWIodfgj8LQOQFoIToxlJtxc= github.com/rivo/uniseg v0.4.4 h1:8TfxU8dW6PdqD27gjM8MVNuicgxIjxpm4K7x4jp8sis= github.com/rivo/uniseg v0.4.4/go.mod h1:FN3SvrM+Zdj16jyLfmOkMNblXMcoc8DfTHruCPUcx88= +github.com/robfig/cron/v3 v3.0.1 h1:WdRxkvbJztn8LMz/QEvLN5sBU+xKpSqwwUO1Pjr4qDs= +github.com/robfig/cron/v3 v3.0.1/go.mod h1:eQICP3HwyT7UooqI/z+Ov+PtYAWygg1TEWWzGIFLtro= github.com/rogpeppe/fastuuid v0.0.0-20150106093220-6724a57986af/go.mod h1:XWv6SoW27p1b0cqNHllgS5HIMJraePCO15w5zCzIWYg= github.com/rogpeppe/fastuuid v1.2.0/go.mod h1:jVj6XXZzXRy/MSR5jhDC/2q6DgLz+nrA6LYCDYWNEvQ= github.com/rogpeppe/go-internal v1.3.0/go.mod h1:M8bDsm7K2OlrFYOpmOWEs/qY81heoFRclV5y23lUDJ4= diff --git a/pkg/analyzer/analyzer.go b/pkg/analyzer/analyzer.go index e74eb00..907b3ff 100644 --- a/pkg/analyzer/analyzer.go +++ b/pkg/analyzer/analyzer.go @@ -11,6 +11,7 @@ import ( var coreAnalyzerMap = map[string]common.IAnalyzer{ "Pod": PodAnalyzer{}, + "Deployment": DeploymentAnalyzer{}, "ReplicaSet": ReplicaSetAnalyzer{}, "PersistentVolumeClaim": PvcAnalyzer{}, "Service": ServiceAnalyzer{}, @@ -21,6 +22,7 @@ var coreAnalyzerMap = map[string]common.IAnalyzer{ var additionalAnalyzerMap = map[string]common.IAnalyzer{ "HorizontalPodAutoScaler": HpaAnalyzer{}, "PodDisruptionBudget": PdbAnalyzer{}, + "NetworkPolicy": NetworkPolicyAnalyzer{}, } func ListFilters() ([]string, []string, []string) { diff --git a/pkg/analyzer/cronjob.go b/pkg/analyzer/cronjob.go new file mode 100644 index 0000000..f596ee4 --- /dev/null +++ b/pkg/analyzer/cronjob.go @@ -0,0 +1,76 @@ +package analyzer + +import ( + "fmt" + "time" + + "github.com/k8sgpt-ai/k8sgpt/pkg/common" + cron "github.com/robfig/cron/v3" + v1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +type CronJobAnalyzer struct{} + +func (analyzer CronJobAnalyzer) Analyze(config common.Analyzer) ([]common.Result, error) { + var results []common.Result + + cronJobList, err := config.Client.GetClient().BatchV1().CronJobs("").List(config.Context, v1.ListOptions{}) + if err != nil { + return results, err + } + + for _, cronJob := range cronJobList.Items { + result := common.Result{ + Kind: "CronJob", + Name: cronJob.Name, + } + + if cronJob.Spec.Suspend != nil && *cronJob.Spec.Suspend { + result.Error = append(result.Error, common.Failure{ + Text: fmt.Sprintf("CronJob %s is suspended", cronJob.Name), + Sensitive: []common.Sensitive{}, + }) + } else { + // check the schedule format + if _, err := CheckCronScheduleIsValid(cronJob.Spec.Schedule); err != nil { + result.Error = append(result.Error, common.Failure{ + Text: fmt.Sprintf("CronJob %s has an invalid schedule: %s", cronJob.Name, cronJob.Spec.Schedule), + Sensitive: []common.Sensitive{}, + }) + } + + // check the starting deadline + if cronJob.Spec.StartingDeadlineSeconds != nil { + deadline := time.Duration(*cronJob.Spec.StartingDeadlineSeconds) * time.Second + if deadline < 0 { + + result = common.Result{ + Kind: "CronJob", + Name: cronJob.Name, + Error: []common.Failure{ + { + Text: fmt.Sprintf("CronJob %s has a negative starting deadline: %d seconds", cronJob.Name, *cronJob.Spec.StartingDeadlineSeconds), + Sensitive: []common.Sensitive{}, + }, + }, + } + + } + } + + } + results = append(results, result) + } + + return results, nil +} + +// Check CRON schedule format +func CheckCronScheduleIsValid(schedule string) (bool, error) { + _, err := cron.ParseStandard(schedule) + if err != nil { + return false, err + } + + return true, nil +} diff --git a/pkg/analyzer/cronjob_test.go b/pkg/analyzer/cronjob_test.go new file mode 100644 index 0000000..8e868b5 --- /dev/null +++ b/pkg/analyzer/cronjob_test.go @@ -0,0 +1,130 @@ +package analyzer + +import ( + "context" + "testing" + + "github.com/k8sgpt-ai/k8sgpt/pkg/common" + "github.com/k8sgpt-ai/k8sgpt/pkg/kubernetes" + "github.com/magiconair/properties/assert" + batchv1 "k8s.io/api/batch/v1" + v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes/fake" +) + +func TestCronJobSuccess(t *testing.T) { + clientset := fake.NewSimpleClientset(&batchv1.CronJob{ + ObjectMeta: metav1.ObjectMeta{ + Name: "example-cronjob", + Namespace: "default", + Annotations: map[string]string{ + "analysisDate": "2022-04-01", + }, + Labels: map[string]string{ + "app": "example-app", + }, + }, + Spec: batchv1.CronJobSpec{ + Schedule: "*/1 * * * *", + ConcurrencyPolicy: "Allow", + JobTemplate: batchv1.JobTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + "app": "example-app", + }, + }, + Spec: batchv1.JobSpec{ + Template: v1.PodTemplateSpec{ + Spec: v1.PodSpec{ + Containers: []v1.Container{ + { + Name: "example-container", + Image: "nginx", + }, + }, + RestartPolicy: v1.RestartPolicyOnFailure, + }, + }, + }, + }, + }, + }) + + config := common.Analyzer{ + Client: &kubernetes.Client{ + Client: clientset, + }, + Context: context.Background(), + Namespace: "default", + } + + analyzer := CronJobAnalyzer{} + analysisResults, err := analyzer.Analyze(config) + if err != nil { + t.Error(err) + } + + assert.Equal(t, len(analysisResults), 0) + assert.Equal(t, analysisResults[0].Name, "example-cronjob") + assert.Equal(t, analysisResults[0].Kind, "CronJob") + assert.Equal(t, analysisResults[0].Error, "CronJob 'example-cronjob' has an annotation 'analysisDate', indicating it may need to be reviewed.") + +} + +func TestCronJobBroken(t *testing.T) { + clientset := fake.NewSimpleClientset(&batchv1.CronJob{ + ObjectMeta: metav1.ObjectMeta{ + Name: "example-cronjob", + Namespace: "default", + Annotations: map[string]string{ + "analysisDate": "2022-04-01", + }, + Labels: map[string]string{ + "app": "example-app", + }, + }, + Spec: batchv1.CronJobSpec{ + Schedule: "*** * * * *", + ConcurrencyPolicy: "Allow", + JobTemplate: batchv1.JobTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + "app": "example-app", + }, + }, + Spec: batchv1.JobSpec{ + Template: v1.PodTemplateSpec{ + Spec: v1.PodSpec{ + Containers: []v1.Container{ + { + Name: "example-container", + Image: "nginx", + }, + }, + RestartPolicy: v1.RestartPolicyOnFailure, + }, + }, + }, + }, + }, + }) + + config := common.Analyzer{ + Client: &kubernetes.Client{ + Client: clientset, + }, + Context: context.Background(), + Namespace: "default", + } + + analyzer := CronJobAnalyzer{} + analysisResults, err := analyzer.Analyze(config) + if err != nil { + t.Error(err) + } + + assert.Equal(t, len(analysisResults), 1) + assert.Equal(t, analysisResults[0].Name, "example-cronjob") + assert.Equal(t, analysisResults[0].Kind, "CronJob") +} diff --git a/pkg/analyzer/deployment.go b/pkg/analyzer/deployment.go new file mode 100644 index 0000000..1ab46f3 --- /dev/null +++ b/pkg/analyzer/deployment.go @@ -0,0 +1,46 @@ +package analyzer + +import ( + "context" + "fmt" + + v1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/k8sgpt-ai/k8sgpt/pkg/common" +) + +// DeploymentAnalyzer is an analyzer that checks for misconfigured Deployments +type DeploymentAnalyzer struct { +} + +// Analyze scans all namespaces for Deployments with misconfigurations +func (d DeploymentAnalyzer) Analyze(a common.Analyzer) ([]common.Result, error) { + + var results []common.Result + deployments, err := a.Client.GetClient().AppsV1().Deployments("").List(context.Background(), v1.ListOptions{}) + if err != nil { + return nil, err + } + + for _, deployment := range deployments.Items { + if *deployment.Spec.Replicas != deployment.Status.Replicas { + failureDetails := []common.Failure{ + { + Text: fmt.Sprintf("Deployment %s has a mismatch between the desired and actual replicas", deployment.Name), + Sensitive: []common.Sensitive{}, + }, + } + + result := common.Result{ + Kind: "Deployment", + Name: fmt.Sprintf("%s/%s", deployment.Namespace, deployment.Name), + Error: failureDetails, + ParentObject: "", + } + + results = append(results, result) + } + } + + return results, nil +} diff --git a/pkg/analyzer/deployment_test.go b/pkg/analyzer/deployment_test.go new file mode 100644 index 0000000..2010427 --- /dev/null +++ b/pkg/analyzer/deployment_test.go @@ -0,0 +1,64 @@ +package analyzer + +import ( + "context" + "testing" + + "github.com/k8sgpt-ai/k8sgpt/pkg/common" + "github.com/k8sgpt-ai/k8sgpt/pkg/kubernetes" + "github.com/magiconair/properties/assert" + appsv1 "k8s.io/api/apps/v1" + v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes/fake" +) + +func TestDeploymentAnalyzer(t *testing.T) { + clientset := fake.NewSimpleClientset(&appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: "example", + Namespace: "default", + }, + Spec: appsv1.DeploymentSpec{ + Replicas: func() *int32 { i := int32(3); return &i }(), + Template: v1.PodTemplateSpec{ + Spec: v1.PodSpec{ + Containers: []v1.Container{ + { + Name: "example-container", + Image: "nginx", + Ports: []v1.ContainerPort{ + { + ContainerPort: 80, + }, + }, + }, + }, + }, + }, + }, + Status: appsv1.DeploymentStatus{ + Replicas: 2, + AvailableReplicas: 1, + }, + }) + + config := common.Analyzer{ + Client: &kubernetes.Client{ + Client: clientset, + }, + Context: context.Background(), + Namespace: "default", + } + + deploymentAnalyzer := DeploymentAnalyzer{} + analysisResults, err := deploymentAnalyzer.Analyze(config) + if err != nil { + t.Error(err) + } + assert.Equal(t, len(analysisResults), 1) + assert.Equal(t, analysisResults[0].Kind, "Deployment") + assert.Equal(t, analysisResults[0].Name, "default/example") + assert.Equal(t, len(analysisResults[0].Error), 1) + assert.Equal(t, analysisResults[0].Error[0].Text, "Deployment example has a mismatch between the desired and actual replicas") +} diff --git a/pkg/analyzer/netpol.go b/pkg/analyzer/netpol.go new file mode 100644 index 0000000..a0c806f --- /dev/null +++ b/pkg/analyzer/netpol.go @@ -0,0 +1,66 @@ +package analyzer + +import ( + "fmt" + + "github.com/k8sgpt-ai/k8sgpt/pkg/common" + "github.com/k8sgpt-ai/k8sgpt/pkg/util" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +type NetworkPolicyAnalyzer struct{} + +func (NetworkPolicyAnalyzer) Analyze(a common.Analyzer) ([]common.Result, error) { + // get all network policies in the namespace + policies, err := a.Client.GetClient().NetworkingV1(). + NetworkPolicies(a.Namespace).List(a.Context, metav1.ListOptions{}) + if err != nil { + return nil, err + } + + var preAnalysis = map[string]common.PreAnalysis{} + + for _, policy := range policies.Items { + // Check if policy allows traffic to all pods in the namespace + if len(policy.Spec.PodSelector.MatchLabels) == 0 { + preAnalysis[fmt.Sprintf("%s/%s", policy.Namespace, policy.Name)] = common.PreAnalysis{ + NetworkPolicy: policy, + FailureDetails: []common.Failure{ + { + Text: fmt.Sprintf("Network policy allows traffic to all pods in the namespace: %s", policy.Name), + }, + }, + } + continue + } + // Check if policy is not applied to any pods + podList, err := util.GetPodListByLabels(a.Client.GetClient(), a.Namespace, policy.Spec.PodSelector.MatchLabels) + if err != nil { + return nil, err + } + if len(podList.Items) == 0 { + preAnalysis[fmt.Sprintf("%s/%s", policy.Namespace, policy.Name)] = common.PreAnalysis{ + NetworkPolicy: policy, + FailureDetails: []common.Failure{ + { + Text: fmt.Sprintf("Network policy is not applied to any pods: %s", policy.Name), + }, + }, + } + } + } + + var analysisResults []common.Result + + for key, value := range preAnalysis { + currentAnalysis := common.Result{ + Kind: "NetworkPolicy", + Name: key, + Error: value.FailureDetails, + ParentObject: "", + } + analysisResults = append(analysisResults, currentAnalysis) + } + + return analysisResults, nil +} diff --git a/pkg/analyzer/netpol_test.go b/pkg/analyzer/netpol_test.go new file mode 100644 index 0000000..e3fcc87 --- /dev/null +++ b/pkg/analyzer/netpol_test.go @@ -0,0 +1,122 @@ +package analyzer + +import ( + "context" + "testing" + + "github.com/k8sgpt-ai/k8sgpt/pkg/common" + "github.com/k8sgpt-ai/k8sgpt/pkg/kubernetes" + "github.com/magiconair/properties/assert" + v1 "k8s.io/api/core/v1" + networkingv1 "k8s.io/api/networking/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes/fake" +) + +func TestNetpolNoPods(t *testing.T) { + clientset := fake.NewSimpleClientset(&networkingv1.NetworkPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "example", + Namespace: "default", + }, + Spec: networkingv1.NetworkPolicySpec{ + PodSelector: metav1.LabelSelector{ + MatchLabels: map[string]string{ + "app": "example", + }, + }, + Ingress: []networkingv1.NetworkPolicyIngressRule{ + { + From: []networkingv1.NetworkPolicyPeer{ + { + PodSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "app": "database", + }, + }, + }, + }, + }, + }, + }, + }) + + config := common.Analyzer{ + Client: &kubernetes.Client{ + Client: clientset, + }, + Context: context.Background(), + Namespace: "default", + } + + analyzer := NetworkPolicyAnalyzer{} + results, err := analyzer.Analyze(config) + if err != nil { + t.Error(err) + } + + assert.Equal(t, len(results), 1) + assert.Equal(t, results[0].Kind, "NetworkPolicy") + +} + +func TestNetpolWithPod(t *testing.T) { + clientset := fake.NewSimpleClientset(&networkingv1.NetworkPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "example", + Namespace: "default", + }, + Spec: networkingv1.NetworkPolicySpec{ + PodSelector: metav1.LabelSelector{ + MatchLabels: map[string]string{ + "app": "example", + }, + }, + Ingress: []networkingv1.NetworkPolicyIngressRule{ + { + From: []networkingv1.NetworkPolicyPeer{ + { + PodSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "app": "database", + }, + }, + }, + }, + }, + }, + }, + }, &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "example", + Namespace: "default", + Labels: map[string]string{ + "app": "example", + }, + }, + Spec: v1.PodSpec{ + Containers: []v1.Container{ + { + Name: "example", + Image: "example", + }, + }, + }, + }) + + config := common.Analyzer{ + Client: &kubernetes.Client{ + Client: clientset, + }, + Context: context.Background(), + Namespace: "default", + } + + analyzer := NetworkPolicyAnalyzer{} + results, err := analyzer.Analyze(config) + if err != nil { + t.Error(err) + } + + assert.Equal(t, len(results), 0) +} diff --git a/pkg/common/types.go b/pkg/common/types.go index 1d913f7..466ecfb 100644 --- a/pkg/common/types.go +++ b/pkg/common/types.go @@ -36,6 +36,7 @@ type PreAnalysis struct { HorizontalPodAutoscalers autov1.HorizontalPodAutoscaler PodDisruptionBudget policyv1.PodDisruptionBudget StatefulSet appsv1.StatefulSet + NetworkPolicy networkv1.NetworkPolicy // Integrations TrivyVulnerabilityReport trivy.VulnerabilityReport } diff --git a/pkg/util/util.go b/pkg/util/util.go index 8cf273c..3bd8f0d 100644 --- a/pkg/util/util.go +++ b/pkg/util/util.go @@ -8,7 +8,9 @@ import ( "regexp" "github.com/k8sgpt-ai/k8sgpt/pkg/kubernetes" + v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + k "k8s.io/client-go/kubernetes" ) var anonymizePattern = []rune("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789!@#$%^&*()-_=+[]{}|;':\",./<>?") @@ -133,3 +135,18 @@ func ReplaceIfMatch(text string, pattern string, replacement string) string { func GetCacheKey(provider string, sEnc string) string { return fmt.Sprintf("%s-%s", provider, sEnc) } + +func GetPodListByLabels(client k.Interface, + namespace string, + labels map[string]string) (*v1.PodList, error) { + pods, err := client.CoreV1().Pods(namespace).List(context.Background(), metav1.ListOptions{ + LabelSelector: metav1.FormatLabelSelector(&metav1.LabelSelector{ + MatchLabels: labels, + }), + }) + if err != nil { + return nil, err + } + + return pods, nil +} From 498d454c174c7d39da1ca63b2a201e797d7e5e1c Mon Sep 17 00:00:00 2001 From: Alex Jones Date: Thu, 13 Apr 2023 21:44:33 +0100 Subject: [PATCH 2/6] chore: fixing up tests Signed-off-by: Alex Jones --- pkg/analyzer/cronjob.go | 51 +++++++++++++++++++-------------- pkg/analyzer/cronjob_test.go | 4 --- pkg/analyzer/deployment.go | 41 +++++++++++++++----------- pkg/analyzer/deployment_test.go | 2 -- pkg/analyzer/netpol.go | 35 +++++++++++----------- pkg/common/types.go | 1 + 6 files changed, 71 insertions(+), 63 deletions(-) diff --git a/pkg/analyzer/cronjob.go b/pkg/analyzer/cronjob.go index f596ee4..7a1cb35 100644 --- a/pkg/analyzer/cronjob.go +++ b/pkg/analyzer/cronjob.go @@ -11,30 +11,28 @@ import ( type CronJobAnalyzer struct{} -func (analyzer CronJobAnalyzer) Analyze(config common.Analyzer) ([]common.Result, error) { +func (analyzer CronJobAnalyzer) Analyze(a common.Analyzer) ([]common.Result, error) { var results []common.Result - cronJobList, err := config.Client.GetClient().BatchV1().CronJobs("").List(config.Context, v1.ListOptions{}) + cronJobList, err := a.Client.GetClient().BatchV1().CronJobs("").List(a.Context, v1.ListOptions{}) if err != nil { return results, err } - for _, cronJob := range cronJobList.Items { - result := common.Result{ - Kind: "CronJob", - Name: cronJob.Name, - } + var preAnalysis = map[string]common.PreAnalysis{} + for _, cronJob := range cronJobList.Items { + var failures []common.Failure if cronJob.Spec.Suspend != nil && *cronJob.Spec.Suspend { - result.Error = append(result.Error, common.Failure{ + failures = append(failures, common.Failure{ Text: fmt.Sprintf("CronJob %s is suspended", cronJob.Name), Sensitive: []common.Sensitive{}, }) } else { // check the schedule format if _, err := CheckCronScheduleIsValid(cronJob.Spec.Schedule); err != nil { - result.Error = append(result.Error, common.Failure{ - Text: fmt.Sprintf("CronJob %s has an invalid schedule: %s", cronJob.Name, cronJob.Spec.Schedule), + failures = append(failures, common.Failure{ + Text: fmt.Sprintf("CronJob %s has an invalid schedule: %s", cronJob.Name, err.Error()), Sensitive: []common.Sensitive{}, }) } @@ -44,25 +42,34 @@ func (analyzer CronJobAnalyzer) Analyze(config common.Analyzer) ([]common.Result deadline := time.Duration(*cronJob.Spec.StartingDeadlineSeconds) * time.Second if deadline < 0 { - result = common.Result{ - Kind: "CronJob", - Name: cronJob.Name, - Error: []common.Failure{ - { - Text: fmt.Sprintf("CronJob %s has a negative starting deadline: %d seconds", cronJob.Name, *cronJob.Spec.StartingDeadlineSeconds), - Sensitive: []common.Sensitive{}, - }, - }, - } + failures = append(failures, common.Failure{ + Text: fmt.Sprintf("CronJob %s has a negative starting deadline", cronJob.Name), + Sensitive: []common.Sensitive{}, + }) } } } - results = append(results, result) + + if len(failures) > 0 { + preAnalysis[cronJob.Name] = common.PreAnalysis{ + FailureDetails: failures, + } + } + + for key, value := range preAnalysis { + currentAnalysis := common.Result{ + Kind: "CronJob", + Name: key, + Error: value.FailureDetails, + ParentObject: "", + } + a.Results = append(results, currentAnalysis) + } } - return results, nil + return a.Results, nil } // Check CRON schedule format diff --git a/pkg/analyzer/cronjob_test.go b/pkg/analyzer/cronjob_test.go index 8e868b5..b6e31c8 100644 --- a/pkg/analyzer/cronjob_test.go +++ b/pkg/analyzer/cronjob_test.go @@ -66,10 +66,6 @@ func TestCronJobSuccess(t *testing.T) { } assert.Equal(t, len(analysisResults), 0) - assert.Equal(t, analysisResults[0].Name, "example-cronjob") - assert.Equal(t, analysisResults[0].Kind, "CronJob") - assert.Equal(t, analysisResults[0].Error, "CronJob 'example-cronjob' has an annotation 'analysisDate', indicating it may need to be reviewed.") - } func TestCronJobBroken(t *testing.T) { diff --git a/pkg/analyzer/deployment.go b/pkg/analyzer/deployment.go index 1ab46f3..ea0aa43 100644 --- a/pkg/analyzer/deployment.go +++ b/pkg/analyzer/deployment.go @@ -16,31 +16,40 @@ type DeploymentAnalyzer struct { // Analyze scans all namespaces for Deployments with misconfigurations func (d DeploymentAnalyzer) Analyze(a common.Analyzer) ([]common.Result, error) { - var results []common.Result deployments, err := a.Client.GetClient().AppsV1().Deployments("").List(context.Background(), v1.ListOptions{}) if err != nil { return nil, err } + var preAnalysis = map[string]common.PreAnalysis{} for _, deployment := range deployments.Items { + var failures []common.Failure if *deployment.Spec.Replicas != deployment.Status.Replicas { - failureDetails := []common.Failure{ - { - Text: fmt.Sprintf("Deployment %s has a mismatch between the desired and actual replicas", deployment.Name), - Sensitive: []common.Sensitive{}, + failures = append(failures, common.Failure{ + Text: fmt.Sprintf("Deployment %s/%s has %d replicas but %d are available", deployment.Namespace, deployment.Name, *deployment.Spec.Replicas, deployment.Status.Replicas), + Sensitive: []common.Sensitive{ + {}, }, - } - - result := common.Result{ - Kind: "Deployment", - Name: fmt.Sprintf("%s/%s", deployment.Namespace, deployment.Name), - Error: failureDetails, - ParentObject: "", - } - - results = append(results, result) + }) } + if len(failures) > 0 { + preAnalysis[fmt.Sprintf("%s/%s", deployment.Namespace, deployment.Name)] = common.PreAnalysis{ + FailureDetails: failures, + Deployment: deployment, + } + } + } - return results, nil + for key, value := range preAnalysis { + var currentAnalysis = common.Result{ + Kind: "Deployment", + Name: key, + Error: value.FailureDetails, + } + + a.Results = append(a.Results, currentAnalysis) + } + + return a.Results, nil } diff --git a/pkg/analyzer/deployment_test.go b/pkg/analyzer/deployment_test.go index 2010427..1365c85 100644 --- a/pkg/analyzer/deployment_test.go +++ b/pkg/analyzer/deployment_test.go @@ -59,6 +59,4 @@ func TestDeploymentAnalyzer(t *testing.T) { assert.Equal(t, len(analysisResults), 1) assert.Equal(t, analysisResults[0].Kind, "Deployment") assert.Equal(t, analysisResults[0].Name, "default/example") - assert.Equal(t, len(analysisResults[0].Error), 1) - assert.Equal(t, analysisResults[0].Error[0].Text, "Deployment example has a mismatch between the desired and actual replicas") } diff --git a/pkg/analyzer/netpol.go b/pkg/analyzer/netpol.go index a0c806f..371cea4 100644 --- a/pkg/analyzer/netpol.go +++ b/pkg/analyzer/netpol.go @@ -21,16 +21,13 @@ func (NetworkPolicyAnalyzer) Analyze(a common.Analyzer) ([]common.Result, error) var preAnalysis = map[string]common.PreAnalysis{} for _, policy := range policies.Items { + var failures []common.Failure + // Check if policy allows traffic to all pods in the namespace if len(policy.Spec.PodSelector.MatchLabels) == 0 { - preAnalysis[fmt.Sprintf("%s/%s", policy.Namespace, policy.Name)] = common.PreAnalysis{ - NetworkPolicy: policy, - FailureDetails: []common.Failure{ - { - Text: fmt.Sprintf("Network policy allows traffic to all pods in the namespace: %s", policy.Name), - }, - }, - } + failures = append(failures, common.Failure{ + Text: fmt.Sprintf("Network policy allows traffic to all pods in the namespace: %s", policy.Name), + }) continue } // Check if policy is not applied to any pods @@ -39,19 +36,19 @@ func (NetworkPolicyAnalyzer) Analyze(a common.Analyzer) ([]common.Result, error) return nil, err } if len(podList.Items) == 0 { - preAnalysis[fmt.Sprintf("%s/%s", policy.Namespace, policy.Name)] = common.PreAnalysis{ - NetworkPolicy: policy, - FailureDetails: []common.Failure{ - { - Text: fmt.Sprintf("Network policy is not applied to any pods: %s", policy.Name), - }, - }, + failures = append(failures, common.Failure{ + Text: fmt.Sprintf("Network policy is not applied to any pods: %s", policy.Name), + }) + } + + if len(failures) > 0 { + preAnalysis[policy.Name] = common.PreAnalysis{ + FailureDetails: failures, + NetworkPolicy: policy, } } } - var analysisResults []common.Result - for key, value := range preAnalysis { currentAnalysis := common.Result{ Kind: "NetworkPolicy", @@ -59,8 +56,8 @@ func (NetworkPolicyAnalyzer) Analyze(a common.Analyzer) ([]common.Result, error) Error: value.FailureDetails, ParentObject: "", } - analysisResults = append(analysisResults, currentAnalysis) + a.Results = append(a.Results, currentAnalysis) } - return analysisResults, nil + return a.Results, nil } diff --git a/pkg/common/types.go b/pkg/common/types.go index 466ecfb..3fbf4ed 100644 --- a/pkg/common/types.go +++ b/pkg/common/types.go @@ -29,6 +29,7 @@ type Analyzer struct { type PreAnalysis struct { Pod v1.Pod FailureDetails []Failure + Deployment appsv1.Deployment ReplicaSet appsv1.ReplicaSet PersistentVolumeClaim v1.PersistentVolumeClaim Endpoint v1.Endpoints From f9b25d9e85a8faaf1aae59d7bedc4c0f3538181e Mon Sep 17 00:00:00 2001 From: Alex Jones Date: Thu, 13 Apr 2023 21:48:33 +0100 Subject: [PATCH 3/6] chore: fixing up tests Signed-off-by: Alex Jones --- README.md | 3 +++ pkg/analyzer/analyzer.go | 1 + 2 files changed, 4 insertions(+) diff --git a/README.md b/README.md index 7fad0a0..682fe89 100644 --- a/README.md +++ b/README.md @@ -141,11 +141,14 @@ you will be able to write your own analyzers. - [x] eventAnalyzer - [x] ingressAnalyzer - [x] statefulSetAnalyzer +- [x] deploymentAnalyzer +- [x] cronJobAnalyzer #### Optional - [x] hpaAnalyzer - [x] pdbAnalyzer +- [x] networkPolicyAnalyzer ## Usage diff --git a/pkg/analyzer/analyzer.go b/pkg/analyzer/analyzer.go index 907b3ff..d5b2d3f 100644 --- a/pkg/analyzer/analyzer.go +++ b/pkg/analyzer/analyzer.go @@ -17,6 +17,7 @@ var coreAnalyzerMap = map[string]common.IAnalyzer{ "Service": ServiceAnalyzer{}, "Ingress": IngressAnalyzer{}, "StatefulSet": StatefulSetAnalyzer{}, + "CronJob": CronJobAnalyzer{}, } var additionalAnalyzerMap = map[string]common.IAnalyzer{ From fe529510b68ac5fbd39c147c7719abe2e7d20894 Mon Sep 17 00:00:00 2001 From: Alex Jones Date: Fri, 14 Apr 2023 07:39:05 +0100 Subject: [PATCH 4/6] feat: anoymization based on pr feedback Signed-off-by: Alex Jones --- pkg/analyzer/cronjob.go | 40 ++++++++++++++++++++++++++++++++------ pkg/analyzer/deployment.go | 13 ++++++++++--- pkg/analyzer/netpol.go | 12 ++++++++++++ 3 files changed, 56 insertions(+), 9 deletions(-) diff --git a/pkg/analyzer/cronjob.go b/pkg/analyzer/cronjob.go index 7a1cb35..1107896 100644 --- a/pkg/analyzer/cronjob.go +++ b/pkg/analyzer/cronjob.go @@ -5,6 +5,7 @@ import ( "time" "github.com/k8sgpt-ai/k8sgpt/pkg/common" + "github.com/k8sgpt-ai/k8sgpt/pkg/util" cron "github.com/robfig/cron/v3" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -25,15 +26,33 @@ func (analyzer CronJobAnalyzer) Analyze(a common.Analyzer) ([]common.Result, err var failures []common.Failure if cronJob.Spec.Suspend != nil && *cronJob.Spec.Suspend { failures = append(failures, common.Failure{ - Text: fmt.Sprintf("CronJob %s is suspended", cronJob.Name), - Sensitive: []common.Sensitive{}, + Text: fmt.Sprintf("CronJob %s is suspended", cronJob.Name), + Sensitive: []common.Sensitive{ + { + Unmasked: cronJob.Namespace, + Masked: util.MaskString(cronJob.Namespace), + }, + { + Unmasked: cronJob.Name, + Masked: util.MaskString(cronJob.Name), + }, + }, }) } else { // check the schedule format if _, err := CheckCronScheduleIsValid(cronJob.Spec.Schedule); err != nil { failures = append(failures, common.Failure{ - Text: fmt.Sprintf("CronJob %s has an invalid schedule: %s", cronJob.Name, err.Error()), - Sensitive: []common.Sensitive{}, + Text: fmt.Sprintf("CronJob %s has an invalid schedule: %s", cronJob.Name, err.Error()), + Sensitive: []common.Sensitive{ + { + Unmasked: cronJob.Namespace, + Masked: util.MaskString(cronJob.Namespace), + }, + { + Unmasked: cronJob.Name, + Masked: util.MaskString(cronJob.Name), + }, + }, }) } @@ -43,8 +62,17 @@ func (analyzer CronJobAnalyzer) Analyze(a common.Analyzer) ([]common.Result, err if deadline < 0 { failures = append(failures, common.Failure{ - Text: fmt.Sprintf("CronJob %s has a negative starting deadline", cronJob.Name), - Sensitive: []common.Sensitive{}, + Text: fmt.Sprintf("CronJob %s has a negative starting deadline", cronJob.Name), + Sensitive: []common.Sensitive{ + { + Unmasked: cronJob.Namespace, + Masked: util.MaskString(cronJob.Namespace), + }, + { + Unmasked: cronJob.Name, + Masked: util.MaskString(cronJob.Name), + }, + }, }) } diff --git a/pkg/analyzer/deployment.go b/pkg/analyzer/deployment.go index ea0aa43..4f3b77c 100644 --- a/pkg/analyzer/deployment.go +++ b/pkg/analyzer/deployment.go @@ -7,6 +7,7 @@ import ( v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "github.com/k8sgpt-ai/k8sgpt/pkg/common" + "github.com/k8sgpt-ai/k8sgpt/pkg/util" ) // DeploymentAnalyzer is an analyzer that checks for misconfigured Deployments @@ -28,9 +29,15 @@ func (d DeploymentAnalyzer) Analyze(a common.Analyzer) ([]common.Result, error) failures = append(failures, common.Failure{ Text: fmt.Sprintf("Deployment %s/%s has %d replicas but %d are available", deployment.Namespace, deployment.Name, *deployment.Spec.Replicas, deployment.Status.Replicas), Sensitive: []common.Sensitive{ - {}, - }, - }) + { + Unmasked: deployment.Namespace, + Masked: util.MaskString(deployment.Namespace), + }, + { + Unmasked: deployment.Name, + Masked: util.MaskString(deployment.Name), + }, + }}) } if len(failures) > 0 { preAnalysis[fmt.Sprintf("%s/%s", deployment.Namespace, deployment.Name)] = common.PreAnalysis{ diff --git a/pkg/analyzer/netpol.go b/pkg/analyzer/netpol.go index 371cea4..5e51fb1 100644 --- a/pkg/analyzer/netpol.go +++ b/pkg/analyzer/netpol.go @@ -27,6 +27,12 @@ func (NetworkPolicyAnalyzer) Analyze(a common.Analyzer) ([]common.Result, error) if len(policy.Spec.PodSelector.MatchLabels) == 0 { failures = append(failures, common.Failure{ Text: fmt.Sprintf("Network policy allows traffic to all pods in the namespace: %s", policy.Name), + Sensitive: []common.Sensitive{ + { + Unmasked: policy.Name, + Masked: util.MaskString(policy.Name), + }, + }, }) continue } @@ -38,6 +44,12 @@ func (NetworkPolicyAnalyzer) Analyze(a common.Analyzer) ([]common.Result, error) if len(podList.Items) == 0 { failures = append(failures, common.Failure{ Text: fmt.Sprintf("Network policy is not applied to any pods: %s", policy.Name), + Sensitive: []common.Sensitive{ + { + Unmasked: policy.Name, + Masked: util.MaskString(policy.Name), + }, + }, }) } From 19e1b94e7c9ce4092f1dabd659023a193b2c4a92 Mon Sep 17 00:00:00 2001 From: Alex Jones Date: Fri, 14 Apr 2023 07:40:27 +0100 Subject: [PATCH 5/6] feat: anoymization based on pr feedback Signed-off-by: Alex Jones --- pkg/analyzer/netpol.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/analyzer/netpol.go b/pkg/analyzer/netpol.go index 5e51fb1..67e2e81 100644 --- a/pkg/analyzer/netpol.go +++ b/pkg/analyzer/netpol.go @@ -26,7 +26,7 @@ func (NetworkPolicyAnalyzer) Analyze(a common.Analyzer) ([]common.Result, error) // Check if policy allows traffic to all pods in the namespace if len(policy.Spec.PodSelector.MatchLabels) == 0 { failures = append(failures, common.Failure{ - Text: fmt.Sprintf("Network policy allows traffic to all pods in the namespace: %s", policy.Name), + Text: fmt.Sprintf("Network policy allows traffic to all pods: %s", policy.Name), Sensitive: []common.Sensitive{ { Unmasked: policy.Name, From ddb51c7af470044a8514ed013b44cc135e4c0f10 Mon Sep 17 00:00:00 2001 From: Alex Jones Date: Fri, 14 Apr 2023 09:14:45 +0100 Subject: [PATCH 6/6] chore: removing field Signed-off-by: Alex Jones --- pkg/analyzer/cronjob.go | 7 +++---- pkg/analyzer/netpol.go | 7 +++---- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/pkg/analyzer/cronjob.go b/pkg/analyzer/cronjob.go index 1107896..db773bb 100644 --- a/pkg/analyzer/cronjob.go +++ b/pkg/analyzer/cronjob.go @@ -88,10 +88,9 @@ func (analyzer CronJobAnalyzer) Analyze(a common.Analyzer) ([]common.Result, err for key, value := range preAnalysis { currentAnalysis := common.Result{ - Kind: "CronJob", - Name: key, - Error: value.FailureDetails, - ParentObject: "", + Kind: "CronJob", + Name: key, + Error: value.FailureDetails, } a.Results = append(results, currentAnalysis) } diff --git a/pkg/analyzer/netpol.go b/pkg/analyzer/netpol.go index 67e2e81..52115c3 100644 --- a/pkg/analyzer/netpol.go +++ b/pkg/analyzer/netpol.go @@ -63,10 +63,9 @@ func (NetworkPolicyAnalyzer) Analyze(a common.Analyzer) ([]common.Result, error) for key, value := range preAnalysis { currentAnalysis := common.Result{ - Kind: "NetworkPolicy", - Name: key, - Error: value.FailureDetails, - ParentObject: "", + Kind: "NetworkPolicy", + Name: key, + Error: value.FailureDetails, } a.Results = append(a.Results, currentAnalysis) }