diff --git a/cmd/cli/kubectl-kyverno/commands/apply/command.go b/cmd/cli/kubectl-kyverno/commands/apply/command.go index 6d085bcd16..0deb8e9e9a 100644 --- a/cmd/cli/kubectl-kyverno/commands/apply/command.go +++ b/cmd/cli/kubectl-kyverno/commands/apply/command.go @@ -447,12 +447,15 @@ func printSkippedAndInvalidPolicies(skipInvalidPolicies SkippedInvalidPolicies) } func printReport(engineResponses []engineapi.EngineResponse, auditWarn bool) { - clustered, namespaced := report.ComputePolicyReports(auditWarn, engineResponses...) + clustered, namespaced, err := report.ComputePolicyReports(auditWarn, engineResponses...) + if err != nil { + fmt.Println("Error: failed to compute policy reports") + } if len(clustered) > 0 || len(namespaced) > 0 { fmt.Println(divider) fmt.Println("POLICY REPORT:") fmt.Println(divider) - report := report.MergeClusterReports(clustered, namespaced) + report := report.MergeClusterReports(clustered) yamlReport, _ := yaml.Marshal(report) fmt.Println(string(yamlReport)) } else { diff --git a/cmd/cli/kubectl-kyverno/commands/apply/command_test.go b/cmd/cli/kubectl-kyverno/commands/apply/command_test.go index d83c3b7b7b..a9cfdee0b1 100644 --- a/cmd/cli/kubectl-kyverno/commands/apply/command_test.go +++ b/cmd/cli/kubectl-kyverno/commands/apply/command_test.go @@ -317,10 +317,10 @@ func Test_Apply(t *testing.T) { _, _, _, responses, err := tc.config.applyCommandHelper() assert.NilError(t, err, desc) - clustered, _ := report.ComputePolicyReports(tc.config.AuditWarn, responses...) + clustered, _, _ := report.ComputePolicyReports(tc.config.AuditWarn, responses...) assert.Assert(t, len(clustered) > 0, "policy reports should not be empty: %s", desc) combined := []policyreportv1alpha2.ClusterPolicyReport{ - report.MergeClusterReports(clustered, nil), + report.MergeClusterReports(clustered), } assert.Equal(t, len(combined), len(tc.expectedPolicyReports)) for i, resp := range combined { diff --git a/cmd/cli/kubectl-kyverno/commands/test/command.go b/cmd/cli/kubectl-kyverno/commands/test/command.go index 1264cdf879..c13c08d99f 100644 --- a/cmd/cli/kubectl-kyverno/commands/test/command.go +++ b/cmd/cli/kubectl-kyverno/commands/test/command.go @@ -179,7 +179,10 @@ func checkResult(test testapi.TestResults, fs billy.Filesystem, resoucePath stri return false, "Generated resource didn't match the generated resource in the test result", "Resource diff" } } - result := report.ComputePolicyReportResult(false, response, rule) + result, err := report.ComputePolicyReportResult(false, response, rule) + if err != nil { + return false, err.Error(), "Error" + } if result.Result != expected { return false, result.Message, fmt.Sprintf("Want %s, got %s", expected, result.Result) } diff --git a/cmd/cli/kubectl-kyverno/output/table/table_test.go b/cmd/cli/kubectl-kyverno/output/table/table_test.go index 2151cbad4a..7c79461965 100644 --- a/cmd/cli/kubectl-kyverno/output/table/table_test.go +++ b/cmd/cli/kubectl-kyverno/output/table/table_test.go @@ -185,9 +185,7 @@ func TestTable_Rows(t *testing.T) { }, Message: "message2", }}, - }, - // TODO: Add test cases. - } + }} for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { tr := &Table{ diff --git a/cmd/cli/kubectl-kyverno/policy/load.go b/cmd/cli/kubectl-kyverno/policy/load.go index 18ad6aafb8..0a3f34e6f8 100644 --- a/cmd/cli/kubectl-kyverno/policy/load.go +++ b/cmd/cli/kubectl-kyverno/policy/load.go @@ -28,7 +28,7 @@ func Load(fs billy.Filesystem, resourcePath string, paths ...string) ([]kyvernov } pols = append(pols, p...) vaps = append(vaps, v...) - } else if fs != nil /* && source.IsGit(path) */ { // TODO source.IsGit(path) + } else if fs != nil { p, v, err := gitLoad(fs, filepath.Join(resourcePath, path)) if err != nil { return nil, nil, err diff --git a/cmd/cli/kubectl-kyverno/report/report.go b/cmd/cli/kubectl-kyverno/report/report.go index 19dcf2af0d..254b3bc0f3 100644 --- a/cmd/cli/kubectl-kyverno/report/report.go +++ b/cmd/cli/kubectl-kyverno/report/report.go @@ -8,17 +8,20 @@ import ( reportutils "github.com/kyverno/kyverno/pkg/utils/report" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/tools/cache" ) -func ComputePolicyReportResult(auditWarn bool, engineResponse engineapi.EngineResponse, ruleResponse engineapi.RuleResponse) policyreportv1alpha2.PolicyReportResult { +func ComputePolicyReportResult(auditWarn bool, engineResponse engineapi.EngineResponse, ruleResponse engineapi.RuleResponse) (policyreportv1alpha2.PolicyReportResult, error) { policy := engineResponse.Policy() - policyName := policy.GetName() + policyName, err := cache.MetaNamespaceKeyFunc(policy.MetaObject()) + if err != nil { + return policyreportv1alpha2.PolicyReportResult{}, err + } audit := engineResponse.GetValidationFailureAction().Audit() scored := annotations.Scored(policy.GetAnnotations()) category := annotations.Category(policy.GetAnnotations()) severity := annotations.Severity(policy.GetAnnotations()) result := policyreportv1alpha2.PolicyReportResult{ - // TODO policy name looks wrong, it should consider the namespace too Policy: policyName, Resources: []corev1.ObjectReference{ { @@ -54,10 +57,10 @@ func ComputePolicyReportResult(auditWarn bool, engineResponse engineapi.EngineRe result.Message = ruleResponse.Message() result.Source = kyverno.ValueKyvernoApp result.Timestamp = metav1.Timestamp{Seconds: ruleResponse.Stats().Timestamp()} - return result + return result, nil } -func ComputePolicyReportResultsPerPolicy(auditWarn bool, engineResponses ...engineapi.EngineResponse) map[engineapi.GenericPolicy][]policyreportv1alpha2.PolicyReportResult { +func ComputePolicyReportResultsPerPolicy(auditWarn bool, engineResponses ...engineapi.EngineResponse) (map[engineapi.GenericPolicy][]policyreportv1alpha2.PolicyReportResult, error) { results := map[engineapi.GenericPolicy][]policyreportv1alpha2.PolicyReportResult{} for _, engineResponse := range engineResponses { if len(engineResponse.PolicyResponse.Rules) == 0 { @@ -69,20 +72,26 @@ func ComputePolicyReportResultsPerPolicy(auditWarn bool, engineResponses ...engi // if ruleResponse.RuleType() != engineapi.Validation && ruleResponse.RuleType() != engineapi.ImageVerify { // continue // } - result := ComputePolicyReportResult(auditWarn, engineResponse, ruleResponse) + result, err := ComputePolicyReportResult(auditWarn, engineResponse, ruleResponse) + if err != nil { + return nil, err + } results[policy] = append(results[policy], result) } } if len(results) == 0 { - return nil + return nil, nil } - return results + return results, nil } -func ComputePolicyReports(auditWarn bool, engineResponses ...engineapi.EngineResponse) ([]policyreportv1alpha2.ClusterPolicyReport, []policyreportv1alpha2.PolicyReport) { +func ComputePolicyReports(auditWarn bool, engineResponses ...engineapi.EngineResponse) ([]policyreportv1alpha2.ClusterPolicyReport, []policyreportv1alpha2.PolicyReport, error) { var clustered []policyreportv1alpha2.ClusterPolicyReport var namespaced []policyreportv1alpha2.PolicyReport - perPolicyResults := ComputePolicyReportResultsPerPolicy(auditWarn, engineResponses...) + perPolicyResults, err := ComputePolicyReportResultsPerPolicy(auditWarn, engineResponses...) + if err != nil { + return nil, nil, err + } for policy, results := range perPolicyResults { if policy.GetNamespace() == "" { report := policyreportv1alpha2.ClusterPolicyReport{ @@ -109,21 +118,14 @@ func ComputePolicyReports(auditWarn bool, engineResponses ...engineapi.EngineRes namespaced = append(namespaced, report) } } - return clustered, namespaced + return clustered, namespaced, nil } -func MergeClusterReports(clustered []policyreportv1alpha2.ClusterPolicyReport, namespaced []policyreportv1alpha2.PolicyReport) policyreportv1alpha2.ClusterPolicyReport { +func MergeClusterReports(clustered []policyreportv1alpha2.ClusterPolicyReport) policyreportv1alpha2.ClusterPolicyReport { var results []policyreportv1alpha2.PolicyReportResult for _, report := range clustered { results = append(results, report.Results...) } - // TODO why this ? - for _, report := range namespaced { - if report.GetNamespace() != "" { - continue - } - results = append(results, report.Results...) - } return policyreportv1alpha2.ClusterPolicyReport{ TypeMeta: metav1.TypeMeta{ Kind: "ClusterPolicyReport", diff --git a/cmd/cli/kubectl-kyverno/report/report_test.go b/cmd/cli/kubectl-kyverno/report/report_test.go index da9b26fb1b..4fabccc3ed 100644 --- a/cmd/cli/kubectl-kyverno/report/report_test.go +++ b/cmd/cli/kubectl-kyverno/report/report_test.go @@ -34,7 +34,8 @@ func TestComputeClusterPolicyReports(t *testing.T) { "validation rule 'pods-require-limits' passed.", ), ) - clustered, namespaced := ComputePolicyReports(false, er) + clustered, namespaced, err := ComputePolicyReports(false, er) + assert.NilError(t, err) assert.Equal(t, len(clustered), 1) assert.Equal(t, len(namespaced), 0) { @@ -68,7 +69,8 @@ func TestComputePolicyReports(t *testing.T) { "validation rule 'pods-require-limits' passed.", ), ) - clustered, namespaced := ComputePolicyReports(false, er) + clustered, namespaced, err := ComputePolicyReports(false, er) + assert.NilError(t, err) assert.Equal(t, len(clustered), 0) assert.Equal(t, len(namespaced), 1) { @@ -102,7 +104,8 @@ func TestComputePolicyReportResultsPerPolicyOld(t *testing.T) { "validation rule 'pods-require-limits' passed.", ), ) - results := ComputePolicyReportResultsPerPolicy(false, er) + results, err := ComputePolicyReportResultsPerPolicy(false, er) + assert.NilError(t, err) for _, result := range results { assert.Equal(t, len(result), 2) for _, r := range result { @@ -146,73 +149,18 @@ func TestMergeClusterReport(t *testing.T) { }, }, }} - namespaced := []policyreportv1alpha2.PolicyReport{{ - TypeMeta: metav1.TypeMeta{ - Kind: "PolicyReport", - APIVersion: report.SchemeGroupVersion.String(), - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "ns-polr-1", - Namespace: "ns-polr", - }, - Results: []policyreportv1alpha2.PolicyReportResult{ - { - Policy: "ns-polr-1", - Result: report.StatusPass, - Resources: make([]corev1.ObjectReference, 10), - }, - }, - }, { - TypeMeta: metav1.TypeMeta{ - Kind: "PolicyReport", - APIVersion: report.SchemeGroupVersion.String(), - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "ns-polr-2", - }, - Results: []policyreportv1alpha2.PolicyReportResult{ - { - Policy: "ns-polr-2", - Result: report.StatusPass, - Resources: make([]corev1.ObjectReference, 5), - }, - }, - }, { - TypeMeta: metav1.TypeMeta{ - Kind: "PolicyReport", - APIVersion: report.SchemeGroupVersion.String(), - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "polr-3", - }, - Results: []policyreportv1alpha2.PolicyReportResult{ - { - Policy: "polr-3", - Result: report.StatusPass, - Resources: make([]corev1.ObjectReference, 1), - }, - }, - }} expectedResults := []policyreportv1alpha2.PolicyReportResult{{ Policy: "cpolr-4", Result: report.StatusFail, }, { Policy: "cpolr-5", Result: report.StatusFail, - }, { - Policy: "ns-polr-2", - Result: report.StatusPass, - Resources: make([]corev1.ObjectReference, 5), - }, { - Policy: "polr-3", - Result: report.StatusPass, - Resources: make([]corev1.ObjectReference, 1), }} - cpolr := MergeClusterReports(clustered, namespaced) + cpolr := MergeClusterReports(clustered) assert.Equal(t, cpolr.APIVersion, report.SchemeGroupVersion.String()) assert.Equal(t, cpolr.Kind, "ClusterPolicyReport") assert.DeepEqual(t, cpolr.Results, expectedResults) - assert.Equal(t, cpolr.Summary.Pass, 2) + assert.Equal(t, cpolr.Summary.Pass, 0) assert.Equal(t, cpolr.Summary.Fail, 2) } @@ -326,7 +274,8 @@ func TestComputePolicyReportResult(t *testing.T) { }} for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got := ComputePolicyReportResult(tt.auditWarn, tt.engineResponse, tt.ruleResponse) + got, err := ComputePolicyReportResult(tt.auditWarn, tt.engineResponse, tt.ruleResponse) + assert.NilError(t, err) got.Timestamp = metav1.Timestamp{} if !reflect.DeepEqual(got, tt.want) { t.Errorf("ComputePolicyReportResult() = %v, want %v", got, tt.want) @@ -351,7 +300,9 @@ func TestComputePolicyReportResultsPerPolicy(t *testing.T) { }} for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if got := ComputePolicyReportResultsPerPolicy(tt.auditWarn, tt.engineResponses...); !reflect.DeepEqual(got, tt.want) { + got, err := ComputePolicyReportResultsPerPolicy(tt.auditWarn, tt.engineResponses...) + assert.NilError(t, err) + if !reflect.DeepEqual(got, tt.want) { t.Errorf("ComputePolicyReportResultsPerPolicy() = %v, want %v", got, tt.want) } }) diff --git a/cmd/cli/kubectl-kyverno/variables/parse.go b/cmd/cli/kubectl-kyverno/variables/parse.go index d52f71c28a..9e6f9eab4c 100644 --- a/cmd/cli/kubectl-kyverno/variables/parse.go +++ b/cmd/cli/kubectl-kyverno/variables/parse.go @@ -2,6 +2,8 @@ package variables import ( "strings" + + "github.com/kyverno/kyverno/cmd/cli/kubectl-kyverno/log" ) func parse(vars ...string) map[string]string { @@ -10,21 +12,21 @@ func parse(vars ...string) map[string]string { variable = strings.TrimSpace(variable) kvs := strings.Split(variable, "=") if len(kvs) != 2 { - // TODO warning + log.Log.Info("ignored variable", "variable", variable) continue } key := strings.TrimSpace(kvs[0]) value := strings.TrimSpace(kvs[1]) if len(value) == 0 || len(key) == 0 { - // TODO log + log.Log.Info("ignored variable", "variable", variable) continue } if strings.Contains(key, "request.object.") { - // TODO log + log.Log.Info("ignored variable (contains `request.object.`)", "variable", variable) continue } if result[key] != "" { - // TODO log + log.Log.Info("ignored variable (duplicated)", "variable", variable) continue } result[key] = value diff --git a/pkg/engine/api/policy.go b/pkg/engine/api/policy.go index b283333798..038015dd14 100644 --- a/pkg/engine/api/policy.go +++ b/pkg/engine/api/policy.go @@ -3,6 +3,7 @@ package api import ( kyvernov1 "github.com/kyverno/kyverno/api/kyverno/v1" "k8s.io/api/admissionregistration/v1alpha1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) // PolicyType represents the type of a policy @@ -34,6 +35,8 @@ type GenericPolicy interface { GetAnnotations() map[string]string // IsNamespaced indicates if the policy is namespace scoped IsNamespaced() bool + // MetaObject provides an object compatible with metav1.Object + MetaObject() metav1.Object } type KyvernoPolicy struct { @@ -72,6 +75,10 @@ func (p *KyvernoPolicy) IsNamespaced() bool { return p.policy.IsNamespaced() } +func (p *KyvernoPolicy) MetaObject() metav1.Object { + return p.policy +} + func NewKyvernoPolicy(pol kyvernov1.PolicyInterface) GenericPolicy { return &KyvernoPolicy{ policy: pol, @@ -114,6 +121,10 @@ func (p *ValidatingAdmissionPolicy) IsNamespaced() bool { return false } +func (p *ValidatingAdmissionPolicy) MetaObject() metav1.Object { + return &p.policy +} + func NewValidatingAdmissionPolicy(pol v1alpha1.ValidatingAdmissionPolicy) GenericPolicy { return &ValidatingAdmissionPolicy{ policy: pol,