Skip to content

Commit

Permalink
refactor: reduce generic policy interface (kyverno#11977)
Browse files Browse the repository at this point in the history
Signed-off-by: Charles-Edouard Brétéché <[email protected]>
  • Loading branch information
eddycharly authored Jan 22, 2025
1 parent 61d69c9 commit f5467fc
Show file tree
Hide file tree
Showing 27 changed files with 153 additions and 275 deletions.
17 changes: 6 additions & 11 deletions cmd/cli/kubectl-kyverno/commands/apply/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,6 @@ import (
"k8s.io/client-go/kubernetes"
)

const divider = "----------------------------------------------------------------------"

type SkippedInvalidPolicies struct {
skipped []string
invalid []string
Expand Down Expand Up @@ -113,9 +111,9 @@ func Command() *cobra.Command {
}
if rule.RuleType() == engineapi.Mutation {
if rule.Status() == engineapi.RuleStatusSkip {
fmt.Fprintln(out, "\nskipped mutate policy", response.Policy().MetaObject().GetName(), "->", "resource", resPath)
fmt.Fprintln(out, "\nskipped mutate policy", response.Policy().GetName(), "->", "resource", resPath)
} else if rule.Status() == engineapi.RuleStatusError {
fmt.Fprintln(out, "\nerror while applying mutate policy", response.Policy().MetaObject().GetName(), "->", "resource", resPath, "\nerror: ", rule.Message())
fmt.Fprintln(out, "\nerror while applying mutate policy", response.Policy().GetName(), "->", "resource", resPath, "\nerror: ", rule.Message())
}
}
}
Expand All @@ -125,9 +123,9 @@ func Command() *cobra.Command {
auditWarn = true
}
if auditWarn {
fmt.Fprintln(out, "policy", response.Policy().MetaObject().GetName(), "->", "resource", resPath, "failed as audit warning:")
fmt.Fprintln(out, "policy", response.Policy().GetName(), "->", "resource", resPath, "failed as audit warning:")
} else {
fmt.Fprintln(out, "policy", response.Policy().MetaObject().GetName(), "->", "resource", resPath, "failed:")
fmt.Fprintln(out, "policy", response.Policy().GetName(), "->", "resource", resPath, "failed:")
}
for i, rule := range failedRules {
fmt.Fprintln(out, i+1, "-", rule.Name(), rule.Message())
Expand Down Expand Up @@ -348,7 +346,7 @@ func (c *ApplyCommandConfig) applyValidatingPolicies(
Rules: r.Rules,
},
}
engineResponse = engineResponse.WithPolicy(engineapi.NewValidatingPolicy(r.Policy))
engineResponse = engineResponse.WithPolicy(engineapi.NewValidatingPolicy(&r.Policy))
responses = append(responses, engineResponse)
}
}
Expand Down Expand Up @@ -515,10 +513,7 @@ func (c *ApplyCommandConfig) loadPolicies() (
return policies, vaps, vapBindings, vps, nil
}

func (c *ApplyCommandConfig) initStoreAndClusterClient(store *store.Store, targetResources ...*unstructured.Unstructured) (
dclient.Interface,
error,
) {
func (c *ApplyCommandConfig) initStoreAndClusterClient(store *store.Store, targetResources ...*unstructured.Unstructured) (dclient.Interface, error) {
store.SetLocal(true)
store.SetRegistryAccess(c.RegistryAccess)
if c.Cluster {
Expand Down
2 changes: 2 additions & 0 deletions cmd/cli/kubectl-kyverno/commands/apply/print.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ import (
"sigs.k8s.io/yaml"
)

const divider = "----------------------------------------------------------------------"

func printSkippedAndInvalidPolicies(out io.Writer, skipInvalidPolicies SkippedInvalidPolicies) {
if len(skipInvalidPolicies.skipped) > 0 {
fmt.Fprintln(out, divider)
Expand Down
9 changes: 4 additions & 5 deletions cmd/cli/kubectl-kyverno/commands/apply/table.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,9 @@ func printTable(out io.Writer, compact, auditWarn bool, engineResponses ...engin
id := 1
for _, engineResponse := range engineResponses {
policy := engineResponse.Policy()
policyMeta := policy.MetaObject()
policyName := policyMeta.GetName()
policyNamespace := policyMeta.GetNamespace()
scored := annotations.Scored(policyMeta.GetAnnotations())
policyName := policy.GetName()
policyNamespace := policy.GetNamespace()
scored := annotations.Scored(policy.GetAnnotations())
resourceKind := engineResponse.Resource.GetKind()
resourceNamespace := engineResponse.Resource.GetNamespace()
resourceName := engineResponse.Resource.GetName()
Expand All @@ -27,7 +26,7 @@ func printTable(out io.Writer, compact, auditWarn bool, engineResponses ...engin
row.ID = id
id++
row.Policy = color.Policy(policyNamespace, policyName)
if policy.GetType() == engineapi.KyvernoPolicyType {
if policy.AsKyvernoPolicy() != nil {
row.Rule = color.Rule(ruleResponse.Name())
}
row.Resource = color.Resource(resourceKind, resourceNamespace, resourceName)
Expand Down
8 changes: 4 additions & 4 deletions cmd/cli/kubectl-kyverno/commands/test/output.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ func printCheckResult(
if check.Match.Policy != nil {
var filtered []engineapi.EngineResponse
for _, response := range matchingEngineResponses {
data, err := runtime.DefaultUnstructuredConverter.ToUnstructured(response.Policy().MetaObject())
data, err := runtime.DefaultUnstructuredConverter.ToUnstructured(response.Policy().AsObject())
if err != nil {
return err
}
Expand Down Expand Up @@ -105,7 +105,7 @@ func printCheckResult(
row := table.Row{
RowCompact: table.RowCompact{
ID: testCount,
Policy: color.Policy("", response.Policy().MetaObject().GetName()),
Policy: color.Policy("", response.Policy().GetName()),
Rule: color.Rule(rule.Name()),
Resource: color.Resource(response.Resource.GetKind(), response.Resource.GetNamespace(), response.Resource.GetName()),
IsFailure: len(errs) != 0,
Expand Down Expand Up @@ -136,7 +136,7 @@ func printCheckResult(
row := table.Row{
RowCompact: table.RowCompact{
ID: testCount,
Policy: color.Policy("", response.Policy().MetaObject().GetName()),
Policy: color.Policy("", response.Policy().GetName()),
Rule: color.Rule(rule.Name()),
Resource: color.Resource(response.Resource.GetKind(), response.Resource.GetNamespace(), response.Resource.GetName()),
IsFailure: len(errs) != 0,
Expand Down Expand Up @@ -240,7 +240,7 @@ func printTestResult(
if _, ok := responses.Trigger[resource]; ok {
for _, response := range responses.Trigger[resource] {
polNameNs := strings.Split(test.Policy, "/")
if response.Policy().MetaObject().GetName() != polNameNs[len(polNameNs)-1] {
if response.Policy().GetName() != polNameNs[len(polNameNs)-1] {
continue
}
for _, rule := range lookupRuleResponses(test, response.PolicyResponse.Rules...) {
Expand Down
4 changes: 2 additions & 2 deletions cmd/cli/kubectl-kyverno/processor/policy_processor.go
Original file line number Diff line number Diff line change
Expand Up @@ -399,7 +399,7 @@ func (p *PolicyProcessor) printOutput(resource interface{}, response engineapi.E
resource := string(yamlEncodedResource) + string("\n---")
if len(strings.TrimSpace(resource)) > 0 {
if !p.Stdin {
fmt.Fprintf(p.Out, "\npolicy %s applied to %s:", response.Policy().MetaObject().GetName(), resourcePath)
fmt.Fprintf(p.Out, "\npolicy %s applied to %s:", response.Policy().GetName(), resourcePath)
}
fmt.Fprintf(p.Out, "\n"+resource+"\n") //nolint:govet
if len(yamlEncodedTargetResources) > 0 {
Expand All @@ -416,7 +416,7 @@ func (p *PolicyProcessor) printOutput(resource interface{}, response engineapi.E
mutateLogPath := filepath.Clean(p.MutateLogPath)
filename := p.Resource.GetName() + "-mutated"
if isGenerate {
filename = response.Policy().MetaObject().GetName() + "-generated"
filename = response.Policy().GetName() + "-generated"
}

file, err = os.OpenFile(filepath.Join(mutateLogPath, filename+".yaml"), os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0o600) // #nosec G304
Expand Down
6 changes: 3 additions & 3 deletions cmd/cli/kubectl-kyverno/processor/result.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ func (rc *ResultCounts) addEngineResponses(auditWarn bool, responses ...engineap
func (rc *ResultCounts) addEngineResponse(auditWarn bool, response engineapi.EngineResponse) {
if !response.IsEmpty() {
genericPolicy := response.Policy()
if polType := genericPolicy.GetType(); polType == engineapi.ValidatingAdmissionPolicyType {
if genericPolicy.AsKyvernoPolicy() == nil {
return
}
policy := genericPolicy.AsKyvernoPolicy()
Expand Down Expand Up @@ -65,7 +65,7 @@ func (rc *ResultCounts) addEngineResponse(auditWarn bool, response engineapi.Eng

func (rc *ResultCounts) addGenerateResponse(response engineapi.EngineResponse) {
genericPolicy := response.Policy()
if polType := genericPolicy.GetType(); polType == engineapi.ValidatingAdmissionPolicyType {
if genericPolicy.AsKyvernoPolicy() == nil {
return
}
policy := genericPolicy.AsKyvernoPolicy()
Expand All @@ -85,7 +85,7 @@ func (rc *ResultCounts) addGenerateResponse(response engineapi.EngineResponse) {

func (rc *ResultCounts) addMutateResponse(response engineapi.EngineResponse) bool {
genericPolicy := response.Policy()
if polType := genericPolicy.GetType(); polType == engineapi.ValidatingAdmissionPolicyType {
if genericPolicy.AsKyvernoPolicy() == nil {
return false
}
policy := genericPolicy.AsKyvernoPolicy()
Expand Down
17 changes: 5 additions & 12 deletions cmd/cli/kubectl-kyverno/report/report.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,9 @@ 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 {
policy := engineResponse.Policy()
policyType := policy.GetType()
policyMeta := policy.MetaObject()
policyName := cache.MetaObjectToName(policyMeta).String()
resource := engineResponse.Resource
resorceRef := &corev1.ObjectReference{
Kind: resource.GetKind(),
Expand All @@ -23,8 +18,7 @@ func ComputePolicyReportResult(auditWarn bool, engineResponse engineapi.EngineRe
APIVersion: resource.GetAPIVersion(),
ResourceVersion: resource.GetResourceVersion(),
}

result := reportutils.ToPolicyReportResult(policyType, policyName, ruleResponse, policyMeta.GetAnnotations(), resorceRef)
result := reportutils.ToPolicyReportResult(engineResponse.Policy(), ruleResponse, resorceRef)
if result.Result == policyreportv1alpha2.StatusFail {
audit := engineResponse.GetValidationFailureAction().Audit()
if audit && auditWarn {
Expand Down Expand Up @@ -61,8 +55,7 @@ func ComputePolicyReports(auditWarn bool, engineResponses ...engineapi.EngineRes
var namespaced []policyreportv1alpha2.PolicyReport
perPolicyResults := ComputePolicyReportResultsPerPolicy(auditWarn, engineResponses...)
for policy, results := range perPolicyResults {
policyMeta := policy.MetaObject()
if policyMeta.GetNamespace() == "" {
if policy.GetNamespace() == "" {
report := policyreportv1alpha2.ClusterPolicyReport{
TypeMeta: metav1.TypeMeta{
APIVersion: policyreportv1alpha2.SchemeGroupVersion.String(),
Expand All @@ -71,7 +64,7 @@ func ComputePolicyReports(auditWarn bool, engineResponses ...engineapi.EngineRes
Results: results,
Summary: reportutils.CalculateSummary(results),
}
report.SetName(policy.MetaObject().GetName())
report.SetName(policy.GetName())
clustered = append(clustered, report)
} else {
report := policyreportv1alpha2.PolicyReport{
Expand All @@ -82,8 +75,8 @@ func ComputePolicyReports(auditWarn bool, engineResponses ...engineapi.EngineRes
Results: results,
Summary: reportutils.CalculateSummary(results),
}
report.SetName(policy.MetaObject().GetName())
report.SetNamespace(policyMeta.GetNamespace())
report.SetName(policy.GetName())
report.SetNamespace(policy.GetNamespace())
namespaced = append(namespaced, report)
}
}
Expand Down
1 change: 0 additions & 1 deletion cmd/cli/kubectl-kyverno/utils/common/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,6 @@ func GetResourceAccordingToResourcePath(
resourcePaths = listOfFiles
}
}

resources, err = GetResources(out, policies, validatingAdmissionPolicies, resourcePaths, dClient, cluster, namespace, policyReport)
if err != nil {
return resources, err
Expand Down
2 changes: 1 addition & 1 deletion pkg/admissionpolicy/mutate.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ func mutateResource(
) (engineapi.EngineResponse, error) {
startTime := time.Now()

engineResponse := engineapi.NewEngineResponse(resource, engineapi.NewMutatingAdmissionPolicy(policy), nil)
engineResponse := engineapi.NewEngineResponse(resource, engineapi.NewMutatingAdmissionPolicy(&policy), nil)
policyResp := engineapi.NewPolicyResponse()

gvk := resource.GroupVersionKind()
Expand Down
4 changes: 2 additions & 2 deletions pkg/admissionpolicy/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ func Validate(
resPath := fmt.Sprintf("%s/%s/%s", resource.GetNamespace(), resource.GetKind(), resource.GetName())
policy := policyData.definition
bindings := policyData.bindings
engineResponse := engineapi.NewEngineResponse(resource, engineapi.NewValidatingAdmissionPolicy(policy), nil)
engineResponse := engineapi.NewEngineResponse(resource, engineapi.NewValidatingAdmissionPolicy(&policy), nil)

gvk := resource.GroupVersionKind()
gvr := schema.GroupVersionResource{
Expand Down Expand Up @@ -186,7 +186,7 @@ func validateResource(
) (engineapi.EngineResponse, error) {
startTime := time.Now()

engineResponse := engineapi.NewEngineResponse(resource, engineapi.NewValidatingAdmissionPolicy(policy), nil)
engineResponse := engineapi.NewEngineResponse(resource, engineapi.NewValidatingAdmissionPolicy(&policy), nil)
policyResp := engineapi.NewPolicyResponse()
var ruleResp *engineapi.RuleResponse

Expand Down
7 changes: 3 additions & 4 deletions pkg/cel/engine/engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,9 @@ func (e *engine) Handle(ctx context.Context, request EngineRequest) (EngineRespo
return response, nil
}

func (e *engine) handlePolicy(ctx context.Context, policy policy.CompiledPolicy, resource *unstructured.Unstructured, namespace *unstructured.Unstructured) PolicyResponse {
func (e *engine) handlePolicy(ctx context.Context, policy CompiledPolicy, resource *unstructured.Unstructured, namespace *unstructured.Unstructured) PolicyResponse {
var rules []engineapi.RuleResponse
results, err := policy.Evaluate(ctx, resource, namespace)
results, err := policy.CompiledPolicy.Evaluate(ctx, resource, namespace)
// TODO: error is about match conditions here ?
if err != nil {
rules = handlers.WithResponses(engineapi.RuleError("evaluation", engineapi.Validation, "failed to load context", err, nil))
Expand All @@ -93,8 +93,7 @@ func (e *engine) handlePolicy(ctx context.Context, policy policy.CompiledPolicy,
}
}
return PolicyResponse{
// TODO
Policy: kyvernov2alpha1.ValidatingPolicy{},
Policy: policy.Policy,
Rules: rules,
}
}
20 changes: 14 additions & 6 deletions pkg/cel/engine/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,26 +8,34 @@ import (
"github.com/kyverno/kyverno/pkg/cel/policy"
)

type CompiledPolicy struct {
Policy kyvernov2alpha1.ValidatingPolicy
CompiledPolicy policy.CompiledPolicy
}

type Provider interface {
CompiledPolicies(context.Context) ([]policy.CompiledPolicy, error)
CompiledPolicies(context.Context) ([]CompiledPolicy, error)
}

type ProviderFunc func(context.Context) ([]policy.CompiledPolicy, error)
type ProviderFunc func(context.Context) ([]CompiledPolicy, error)

func (f ProviderFunc) CompiledPolicies(ctx context.Context) ([]policy.CompiledPolicy, error) {
func (f ProviderFunc) CompiledPolicies(ctx context.Context) ([]CompiledPolicy, error) {
return f(ctx)
}

func NewProvider(compiler policy.Compiler, policies ...kyvernov2alpha1.ValidatingPolicy) (ProviderFunc, error) {
compiled := make([]policy.CompiledPolicy, 0, len(policies))
compiled := make([]CompiledPolicy, 0, len(policies))
for _, vp := range policies {
policy, err := compiler.Compile(&vp)
if err != nil {
return nil, fmt.Errorf("failed to compile policy %s (%w)", vp.GetName(), err.ToAggregate())
}
compiled = append(compiled, policy)
compiled = append(compiled, CompiledPolicy{
Policy: vp,
CompiledPolicy: policy,
})
}
provider := func(context.Context) ([]policy.CompiledPolicy, error) {
provider := func(context.Context) ([]CompiledPolicy, error) {
return compiled, nil
}
return provider, nil
Expand Down
15 changes: 0 additions & 15 deletions pkg/cel/policy/policy.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,21 +72,6 @@ func (p *compiledPolicy) Evaluate(ctx context.Context, resource resource, namesp
Result: out,
Error: err,
})
// // check error
// if err != nil {
// results = append(results, EvaluationResult{
// Error: err,
// })
// }
// response, err := utils.ConvertToNative[bool](out)
// // check error
// if err != nil {
// return false, err
// }
// // if response is false, return
// if !response {
// return false, nil
// }
}
return results, nil
}
Expand Down
14 changes: 7 additions & 7 deletions pkg/controllers/report/background/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,7 @@ func (c *controller) needsReconcile(namespace, name, hash string, exceptions []k
// if a policy or an exception changed, we need a partial reconcile
expected := map[string]string{}
for _, policy := range policies {
expected[reportutils.PolicyLabel(policy)] = policy.MetaObject().GetResourceVersion()
expected[reportutils.PolicyLabel(policy)] = policy.GetResourceVersion()
}
for _, exception := range exceptions {
expected[reportutils.PolicyExceptionLabel(exception)] = exception.GetResourceVersion()
Expand Down Expand Up @@ -342,7 +342,7 @@ func (c *controller) reconcileReport(
// build desired report
expected := map[string]string{}
for _, policy := range policies {
expected[reportutils.PolicyLabel(policy)] = policy.MetaObject().GetResourceVersion()
expected[reportutils.PolicyLabel(policy)] = policy.GetResourceVersion()
}
for _, exception := range exceptions {
expected[reportutils.PolicyExceptionLabel(exception)] = exception.GetResourceVersion()
Expand All @@ -362,9 +362,9 @@ func (c *controller) reconcileReport(
for _, policy := range policies {
var key string
var err error
if policy.GetType() == engineapi.KyvernoPolicyType {
if policy.AsKyvernoPolicy() != nil {
key, err = cache.MetaNamespaceKeyFunc(policy.AsKyvernoPolicy())
} else {
} else if policy.AsValidatingAdmissionPolicy() != nil {
key, err = cache.MetaNamespaceKeyFunc(policy.AsValidatingAdmissionPolicy())
}
if err != nil {
Expand Down Expand Up @@ -412,7 +412,7 @@ func (c *controller) reconcileReport(
// calculate necessary results
for _, policy := range policies {
reevaluate := false
if policy.GetType() == engineapi.KyvernoPolicyType {
if policy.AsKyvernoPolicy() != nil {
for _, polex := range exceptions {
if actual[reportutils.PolicyExceptionLabel(polex)] != polex.GetResourceVersion() {
reevaluate = true
Expand All @@ -427,7 +427,7 @@ func (c *controller) reconcileReport(
}
}
}
if full || reevaluate || actual[reportutils.PolicyLabel(policy)] != policy.MetaObject().GetResourceVersion() {
if full || reevaluate || actual[reportutils.PolicyLabel(policy)] != policy.GetResourceVersion() {
scanner := utils.NewScanner(logger, c.engine, c.config, c.jp, c.client, c.reportsConfig)
for _, result := range scanner.ScanResource(ctx, *target, nsLabels, bindings, policy) {
if result.Error != nil {
Expand Down Expand Up @@ -541,7 +541,7 @@ func (c *controller) reconcile(ctx context.Context, log logr.Logger, key, namesp
return err
}
for _, pol := range vapPolicies {
policies = append(policies, engineapi.NewValidatingAdmissionPolicy(pol))
policies = append(policies, engineapi.NewValidatingAdmissionPolicy(&pol))
}
}
var vapBindings []admissionregistrationv1beta1.ValidatingAdmissionPolicyBinding
Expand Down
Loading

0 comments on commit f5467fc

Please sign in to comment.