From a7dd02a6d1d9c2c322469b9b8427ec930d4dc050 Mon Sep 17 00:00:00 2001 From: Vishal Choudhary Date: Tue, 15 Oct 2024 12:29:18 +0530 Subject: [PATCH] feat: update engine response.generatedResources to support multiple resource (#11398) * fix: manually add generated_resources property Signed-off-by: Vishal Choudhary * fix: update engine response Signed-off-by: Vishal Choudhary * fix: nil check before deferences Signed-off-by: Vishal Choudhary * fix: outdated errors Signed-off-by: Vishal Choudhary * fix: linter Signed-off-by: Vishal Choudhary --------- Signed-off-by: Vishal Choudhary Co-authored-by: shuting --- .../kubectl-kyverno/commands/test/command.go | 5 +- .../kubectl-kyverno/commands/test/compare.go | 35 +++++++--- cmd/cli/kubectl-kyverno/processor/generate.go | 20 +++--- cmd/cli/kubectl-kyverno/resource/resource.go | 8 +-- pkg/background/generate/controller.go | 65 +++++++++++++------ pkg/engine/api/ruleresponse.go | 17 +++-- pkg/utils/report/results.go | 8 ++- .../generate/chainsaw-step-03-assert-1-1.yaml | 4 +- 8 files changed, 109 insertions(+), 53 deletions(-) diff --git a/cmd/cli/kubectl-kyverno/commands/test/command.go b/cmd/cli/kubectl-kyverno/commands/test/command.go index dc3252280c..e3d8fc8880 100644 --- a/cmd/cli/kubectl-kyverno/commands/test/command.go +++ b/cmd/cli/kubectl-kyverno/commands/test/command.go @@ -15,6 +15,7 @@ import ( "github.com/kyverno/kyverno/cmd/cli/kubectl-kyverno/test/filter" engineapi "github.com/kyverno/kyverno/pkg/engine/api" "github.com/spf13/cobra" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/client-go/tools/cache" ) @@ -158,7 +159,7 @@ func checkResult(test v1alpha1.TestResult, fs billy.Filesystem, resoucePath stri } // fallback on deprecated field if test.PatchedResource != "" { - equals, err := getAndCompareResource(response.PatchedResource, fs, filepath.Join(resoucePath, test.PatchedResource)) + equals, err := getAndCompareResource([]*unstructured.Unstructured{&response.PatchedResource}, fs, filepath.Join(resoucePath, test.PatchedResource)) if err != nil { return false, err.Error(), "Resource error" } @@ -167,7 +168,7 @@ func checkResult(test v1alpha1.TestResult, fs billy.Filesystem, resoucePath stri } } if test.GeneratedResource != "" { - equals, err := getAndCompareResource(rule.GeneratedResource(), fs, filepath.Join(resoucePath, test.GeneratedResource)) + equals, err := getAndCompareResource(rule.GeneratedResources(), fs, filepath.Join(resoucePath, test.GeneratedResource)) if err != nil { return false, err.Error(), "Resource error" } diff --git a/cmd/cli/kubectl-kyverno/commands/test/compare.go b/cmd/cli/kubectl-kyverno/commands/test/compare.go index 0fcb4f1dad..9452ab9c6c 100644 --- a/cmd/cli/kubectl-kyverno/commands/test/compare.go +++ b/cmd/cli/kubectl-kyverno/commands/test/compare.go @@ -8,16 +8,35 @@ import ( "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" ) -func getAndCompareResource(actualResource unstructured.Unstructured, fs billy.Filesystem, path string) (bool, error) { - expectedResource, err := resource.GetResourceFromPath(fs, path) +func getAndCompareResource(actualResources []*unstructured.Unstructured, fs billy.Filesystem, path string) (bool, error) { + expectedResources, err := resource.GetResourceFromPath(fs, path) if err != nil { return false, fmt.Errorf("error: failed to load resource (%s)", err) } - resource.FixupGenerateLabels(actualResource) - resource.FixupGenerateLabels(*expectedResource) - equals, err := resource.Compare(actualResource, *expectedResource, true) - if err != nil { - return false, fmt.Errorf("error: failed to compare resources (%s)", err) + + expectedResourcesMap := map[string]unstructured.Unstructured{} + for _, expectedResource := range expectedResources { + if expectedResource == nil { + continue + } + r := *expectedResource + resource.FixupGenerateLabels(r) + expectedResourcesMap[expectedResource.GetNamespace()+"/"+expectedResource.GetName()] = r } - return equals, nil + + for _, actualResource := range actualResources { + if actualResource == nil { + continue + } + r := *actualResource + resource.FixupGenerateLabels(r) + equals, err := resource.Compare(r, expectedResourcesMap[r.GetNamespace()+"/"+r.GetName()], true) + if err != nil { + return false, fmt.Errorf("error: failed to compare resources (%s)", err) + } + if !equals { + return false, nil + } + } + return true, nil } diff --git a/cmd/cli/kubectl-kyverno/processor/generate.go b/cmd/cli/kubectl-kyverno/processor/generate.go index 8510113456..c2ac60a6b2 100644 --- a/cmd/cli/kubectl-kyverno/processor/generate.go +++ b/cmd/cli/kubectl-kyverno/processor/generate.go @@ -5,6 +5,7 @@ import ( "io" "strings" + kyvernov1 "github.com/kyverno/kyverno/api/kyverno/v1" "github.com/kyverno/kyverno/cmd/cli/kubectl-kyverno/log" "github.com/kyverno/kyverno/cmd/cli/kubectl-kyverno/resource" "github.com/kyverno/kyverno/cmd/cli/kubectl-kyverno/store" @@ -78,21 +79,22 @@ func handleGeneratePolicy(out io.Writer, store *store.Store, generateResponse *e return nil, err } - var newRuleResponse []engineapi.RuleResponse + newRuleResponse := []engineapi.RuleResponse{} for _, rule := range generateResponse.PolicyResponse.Rules { - genResource, err := c.ApplyGeneratePolicy(log.Log.V(2), &policyContext, []string{rule.Name()}) + genResourceMap, err := c.ApplyGeneratePolicy(log.Log.V(2), &policyContext, []string{rule.Name()}) if err != nil { return nil, err } - - if genResource != nil { - unstrGenResource, err := c.GetUnstrResource(genResource[0]) - if err != nil { - return nil, err - } - newRuleResponse = append(newRuleResponse, *rule.WithGeneratedResource(*unstrGenResource)) + generatedResources := []kyvernov1.ResourceSpec{} + for _, v := range genResourceMap { + generatedResources = append(generatedResources, v...) } + unstrGenResources, err := c.GetUnstrResources(generatedResources) + if err != nil { + return nil, err + } + newRuleResponse = append(newRuleResponse, *rule.WithGeneratedResources(unstrGenResources)) } return newRuleResponse, nil diff --git a/cmd/cli/kubectl-kyverno/resource/resource.go b/cmd/cli/kubectl-kyverno/resource/resource.go index b80648415e..177dea42a4 100644 --- a/cmd/cli/kubectl-kyverno/resource/resource.go +++ b/cmd/cli/kubectl-kyverno/resource/resource.go @@ -59,7 +59,7 @@ func YamlToUnstructured(resourceYaml []byte) (*unstructured.Unstructured, error) return resource, nil } -func GetResourceFromPath(fs billy.Filesystem, path string) (*unstructured.Unstructured, error) { +func GetResourceFromPath(fs billy.Filesystem, path string) ([]*unstructured.Unstructured, error) { var resourceBytes []byte if fs == nil { data, err := GetFileBytes(path) @@ -83,10 +83,10 @@ func GetResourceFromPath(fs billy.Filesystem, path string) (*unstructured.Unstru if err != nil { return nil, err } - if len(resources) != 1 { - return nil, fmt.Errorf("exactly one resource expected, found %d", len(resources)) + if len(resources) == 0 { + return nil, fmt.Errorf("no resources found") } - return resources[0], nil + return resources, nil } func GetFileBytes(path string) ([]byte, error) { diff --git a/pkg/background/generate/controller.go b/pkg/background/generate/controller.go index 9b7eb0f07c..8cecc25eb5 100644 --- a/pkg/background/generate/controller.go +++ b/pkg/background/generate/controller.go @@ -236,11 +236,6 @@ func (c *GenerateController) applyGenerate(trigger unstructured.Unstructured, ur logger.V(4).Info(doesNotApply) return nil, errors.New(doesNotApply) } - if c.needsReports(trigger) { - if err := c.createReports(context.TODO(), policyContext.NewResource(), engineResponse); err != nil { - c.log.Error(err, "failed to create report") - } - } var applicableRules []string for _, r := range engineResponse.PolicyResponse.Rules { @@ -250,17 +245,40 @@ func (c *GenerateController) applyGenerate(trigger unstructured.Unstructured, ur } // Apply the generate rule on resource - genResources, err := c.ApplyGeneratePolicy(logger, policyContext, applicableRules) - if err == nil { - for _, res := range genResources { - e := event.NewResourceGenerationEvent(ur.Spec.Policy, ur.Spec.RuleContext[i].Rule, event.GeneratePolicyController, res) - c.eventGen.Add(e) - } - - e := event.NewBackgroundSuccessEvent(event.GeneratePolicyController, policy, genResources) - c.eventGen.Add(e...) + genResourcesMap, err := c.ApplyGeneratePolicy(logger, policyContext, applicableRules) + if err != nil { + return nil, err } + for i, v := range engineResponse.PolicyResponse.Rules { + if resources, ok := genResourcesMap[v.Name()]; ok { + unstResources, err := c.GetUnstrResources(resources) + if err != nil { + c.log.Error(err, "failed to get unst resource names report") + } + engineResponse.PolicyResponse.Rules[i] = *v.WithGeneratedResources(unstResources) + } + } + + if c.needsReports(trigger) { + if err := c.createReports(context.TODO(), policyContext.NewResource(), engineResponse); err != nil { + c.log.Error(err, "failed to create report") + } + } + + genResources := make([]kyvernov1.ResourceSpec, 0) + for _, v := range genResourcesMap { + genResources = append(genResources, v...) + } + + for _, res := range genResources { + e := event.NewResourceGenerationEvent(ur.Spec.Policy, ur.Spec.RuleContext[i].Rule, event.GeneratePolicyController, res) + c.eventGen.Add(e) + } + + e := event.NewBackgroundSuccessEvent(event.GeneratePolicyController, policy, genResources) + c.eventGen.Add(e...) + return genResources, err } @@ -285,7 +303,8 @@ func (c *GenerateController) getPolicyObject(ur kyvernov2.UpdateRequest) (kyvern return npolicyObj, nil } -func (c *GenerateController) ApplyGeneratePolicy(log logr.Logger, policyContext *engine.PolicyContext, applicableRules []string) (genResources []kyvernov1.ResourceSpec, err error) { +func (c *GenerateController) ApplyGeneratePolicy(log logr.Logger, policyContext *engine.PolicyContext, applicableRules []string) (map[string][]kyvernov1.ResourceSpec, error) { + genResources := make(map[string][]kyvernov1.ResourceSpec) policy := policyContext.Policy() resource := policyContext.NewResource() // To manage existing resources, we compare the creation time for the default resource to be generated and policy creation time @@ -348,7 +367,7 @@ func (c *GenerateController) ApplyGeneratePolicy(log logr.Logger, policyContext return nil, err } ruleNameToProcessingTime[rule.Name] = time.Since(startTime) - genResources = append(genResources, genResource...) + genResources[rule.Name] = genResource applyCount++ } @@ -365,12 +384,16 @@ func NewGenerateControllerWithOnlyClient(client dclient.Interface, engine engine } // GetUnstrResource converts ResourceSpec object to type Unstructured -func (c *GenerateController) GetUnstrResource(genResourceSpec kyvernov1.ResourceSpec) (*unstructured.Unstructured, error) { - resource, err := c.client.GetResource(context.TODO(), genResourceSpec.APIVersion, genResourceSpec.Kind, genResourceSpec.Namespace, genResourceSpec.Name) - if err != nil { - return nil, err +func (c *GenerateController) GetUnstrResources(genResourceSpecs []kyvernov1.ResourceSpec) ([]*unstructured.Unstructured, error) { + resources := []*unstructured.Unstructured{} + for _, genResourceSpec := range genResourceSpecs { + resource, err := c.client.GetResource(context.TODO(), genResourceSpec.APIVersion, genResourceSpec.Kind, genResourceSpec.Namespace, genResourceSpec.Name) + if err != nil { + return nil, err + } + resources = append(resources, resource) } - return resource, nil + return resources, nil } func (c *GenerateController) needsReports(trigger unstructured.Unstructured) bool { diff --git a/pkg/engine/api/ruleresponse.go b/pkg/engine/api/ruleresponse.go index 218cf02b80..632d4252f1 100644 --- a/pkg/engine/api/ruleresponse.go +++ b/pkg/engine/api/ruleresponse.go @@ -33,8 +33,8 @@ type RuleResponse struct { status RuleStatus // stats contains rule statistics stats ExecutionStats - // generatedResource is the generated by the generate rules of a policy - generatedResource unstructured.Unstructured + // generatedResources is the list of resources generated by the generate rules of a policy + generatedResources []*unstructured.Unstructured // patchedTarget is the patched resource for mutate.targets patchedTarget *unstructured.Unstructured // patchedTargetParentResourceGVR is the GVR of the parent resource of the PatchedTarget. This is only populated when PatchedTarget is a subresource. @@ -113,8 +113,8 @@ func (r RuleResponse) WithPatchedTarget(patchedTarget *unstructured.Unstructured return &r } -func (r RuleResponse) WithGeneratedResource(resource unstructured.Unstructured) *RuleResponse { - r.generatedResource = resource +func (r RuleResponse) WithGeneratedResources(resource []*unstructured.Unstructured) *RuleResponse { + r.generatedResources = resource return &r } @@ -128,6 +128,11 @@ func (r RuleResponse) WithEmitWarning(emitWarning bool) *RuleResponse { return &r } +func (r RuleResponse) WithProperties(m map[string]string) *RuleResponse { + r.properties = m + return &r +} + func (r *RuleResponse) Stats() ExecutionStats { return r.stats } @@ -152,8 +157,8 @@ func (r *RuleResponse) PatchedTarget() (*unstructured.Unstructured, metav1.Group return r.patchedTarget, r.patchedTargetParentResourceGVR, r.patchedTargetSubresourceName } -func (r *RuleResponse) GeneratedResource() unstructured.Unstructured { - return r.generatedResource +func (r *RuleResponse) GeneratedResources() []*unstructured.Unstructured { + return r.generatedResources } func (r *RuleResponse) Message() string { diff --git a/pkg/utils/report/results.go b/pkg/utils/report/results.go index 0715a34747..c3005750a8 100644 --- a/pkg/utils/report/results.go +++ b/pkg/utils/report/results.go @@ -219,8 +219,12 @@ func GenerationEngineResponseToReportResults(response engineapi.EngineResponse) results := make([]policyreportv1alpha2.PolicyReportResult, 0, len(response.PolicyResponse.Rules)) for _, ruleResult := range response.PolicyResponse.Rules { result := ToPolicyReportResult(policyType, policyName, ruleResult, annotations, nil) - if generatedResource := ruleResult.GeneratedResource(); generatedResource.GetName() != "" { - addProperty("generated-resource", getResourceInfo(generatedResource.GroupVersionKind(), generatedResource.GetName(), generatedResource.GetNamespace()), &result) + if generatedResources := ruleResult.GeneratedResources(); len(generatedResources) != 0 { + property := make([]string, 0) + for _, r := range generatedResources { + property = append(property, getResourceInfo(r.GroupVersionKind(), r.GetName(), r.GetNamespace())) + } + addProperty("generated-resources", strings.Join(property, "; "), &result) } results = append(results, result) } diff --git a/test/conformance/chainsaw/reports/background/generate/chainsaw-step-03-assert-1-1.yaml b/test/conformance/chainsaw/reports/background/generate/chainsaw-step-03-assert-1-1.yaml index f6abe75a88..13e66cf92b 100755 --- a/test/conformance/chainsaw/reports/background/generate/chainsaw-step-03-assert-1-1.yaml +++ b/test/conformance/chainsaw/reports/background/generate/chainsaw-step-03-assert-1-1.yaml @@ -13,6 +13,8 @@ results: rule: clone-secret scored: true source: kyverno + properties: + generated-resources: /v1, Kind=Secret Name=regcred Namespace=cpol-clone-nosync-create-ns scope: apiVersion: v1 kind: Namespace @@ -22,4 +24,4 @@ summary: fail: 0 pass: 1 skip: 0 - warn: 0 \ No newline at end of file + warn: 0