Skip to content

Commit

Permalink
Fix 1351 - policy report (kyverno#1359)
Browse files Browse the repository at this point in the history 
* ignore Kyverno CRDs existence check when server is not available

* clean up cluster / reportChangeRequest

* resolve PR comments

* - fixes kyverno#1351; - clean up code

* fo fmt
  • Loading branch information
@realshuting
realshuting authored Dec 4, 2020
1 parent 630a9cc commit 624b481
Show file tree
Hide file tree
Showing 11 changed files with 70 additions and 91 deletions.
12 changes: 5 additions & 7 deletions pkg/engine/validation.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -174,8 +174,6 @@ func validateResource(log logr.Logger, ctx context.EvalInterface, policy kyverno
}

// check if the resource satisfies the filter conditions defined in the rule
// TODO: this needs to be extracted, to filter the resource so that we can avoid passing resources that
// dont satisfy a policy rule resource description
if err := MatchesResourceDescription(resource, rule, admissionInfo, excludeResource); err != nil {
log.V(4).Info("resource fails the match description", "reason", err.Error())
continue
Expand All @@ -189,7 +187,7 @@ func validateResource(log logr.Logger, ctx context.EvalInterface, policy kyverno
// operate on the copy of the conditions, as we perform variable substitution
preconditionsCopy := copyConditions(rule.Conditions)
// evaluate pre-conditions
// - handle variable subsitutions
// - handle variable substitutions
if !variables.EvaluateConditions(log, ctx, preconditionsCopy) {
log.V(4).Info("resource fails the preconditions")
continue
Expand Down Expand Up @@ -265,7 +263,7 @@ func validatePatterns(log logr.Logger, ctx context.EvalInterface, resource unstr
pattern := validationRule.Pattern
var err error
if pattern, err = variables.SubstituteVars(logger, ctx, pattern); err != nil {
// variable subsitution failed
// variable substitution failed
resp.Success = false
resp.Message = fmt.Sprintf("Validation error: %s; Validation rule '%s' failed. '%s'",
rule.Validation.Message, rule.Name, err)
Expand Down Expand Up @@ -295,13 +293,13 @@ func validatePatterns(log logr.Logger, ctx context.EvalInterface, resource unstr
anyPatterns, err := rule.Validation.DeserializeAnyPattern()
if err != nil {
resp.Success = false
resp.Message = fmt.Sprintf("Failed to deserialze anyPattern, expect type array: %v", err)
resp.Message = fmt.Sprintf("Failed to deserialize anyPattern, expect type array: %v", err)
return resp
}

for idx, pattern := range anyPatterns {
if pattern, err = variables.SubstituteVars(logger, ctx, pattern); err != nil {
// variable subsitution failed
// variable substitution failed
failedSubstitutionsErrors = append(failedSubstitutionsErrors, err)
continue
}
Expand All @@ -316,7 +314,7 @@ func validatePatterns(log logr.Logger, ctx context.EvalInterface, resource unstr
failedAnyPatternsErrors = append(failedAnyPatternsErrors, patternErr)
}

// Subsitution falures
// Substitution failures
if len(failedSubstitutionsErrors) > 0 {
resp.Success = false
resp.Message = fmt.Sprintf("Substitutions failed: %v", failedSubstitutionsErrors)
Expand Down
66 changes: 32 additions & 34 deletions pkg/generate/generate.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
dclient "github.com/kyverno/kyverno/pkg/dclient"
"github.com/kyverno/kyverno/pkg/engine"
"github.com/kyverno/kyverno/pkg/engine/context"
"github.com/kyverno/kyverno/pkg/engine/utils"
"github.com/kyverno/kyverno/pkg/engine/validate"
"github.com/kyverno/kyverno/pkg/engine/variables"
apierrors "k8s.io/apimachinery/pkg/api/errors"
Expand Down Expand Up @@ -257,12 +258,32 @@ func updateGenerateExecutionTime(newTime time.Duration, oldAverageTimeString str
return time.Duration(newAverageTimeInNanoSeconds) * time.Nanosecond
}

func getResourceInfo(object map[string]interface{}) (kind, name, namespace, apiversion string, err error) {
if kind, _, err = unstructured.NestedString(object, "kind"); err != nil {
return "", "", "", "", err
}

if name, _, err = unstructured.NestedString(object, "name"); err != nil {
return "", "", "", "", err
}

if namespace, _, err = unstructured.NestedString(object, "namespace"); err != nil {
return "", "", "", "", err
}

if apiversion, _, err = unstructured.NestedString(object, "apiVersion"); err != nil {
return "", "", "", "", err
}

return
}

func applyRule(log logr.Logger, client *dclient.Client, rule kyverno.Rule, resource unstructured.Unstructured, ctx context.EvalInterface, policy string, gr kyverno.GenerateRequest, processExisting bool) (kyverno.ResourceSpec, error) {
var rdata map[string]interface{}
var err error
var mode ResourceMode
var noGenResource kyverno.ResourceSpec
// convert to unstructured Resource

genUnst, err := getUnstrRule(rule.Generation.DeepCopy())
if err != nil {
return noGenResource, err
Expand All @@ -275,40 +296,31 @@ func applyRule(log logr.Logger, client *dclient.Client, rule kyverno.Rule, resou
if err != nil {
return noGenResource, err
}
genUnst.Object, _ = object.(map[string]interface{})

genKind, _, err := unstructured.NestedString(genUnst.Object, "kind")
if err != nil {
return noGenResource, err
}
genName, _, err := unstructured.NestedString(genUnst.Object, "name")
if err != nil {
return noGenResource, err
}
genNamespace, _, err := unstructured.NestedString(genUnst.Object, "namespace")
genUnst.Object, _ = object.(map[string]interface{})
genKind, genName, genNamespace, genAPIVersion, err := getResourceInfo(genUnst.Object)
if err != nil {
return noGenResource, err
}

genAPIVersion, _, err := unstructured.NestedString(genUnst.Object, "apiVersion")
if err != nil {
return noGenResource, err
}
// Resource to be generated
newGenResource := kyverno.ResourceSpec{
APIVersion: genAPIVersion,
Kind: genKind,
Namespace: genNamespace,
Name: genName,
}

genData, _, err := unstructured.NestedMap(genUnst.Object, "data")
if err != nil {
return noGenResource, err
}

genCopy, _, err := unstructured.NestedMap(genUnst.Object, "clone")
if err != nil {
return noGenResource, err
}

if genData != nil {
rdata, mode, err = manageData(log, genAPIVersion, genKind, genNamespace, genName, genData, client, resource)
} else {
Expand All @@ -317,10 +329,6 @@ func applyRule(log logr.Logger, client *dclient.Client, rule kyverno.Rule, resou

logger := log.WithValues("genKind", genKind, "genAPIVersion", genAPIVersion, "genNamespace", genNamespace, "genName", genName)

if err != nil {
return noGenResource, err
}

if rdata == nil {
// existing resource contains the configuration
return newGenResource, nil
Expand Down Expand Up @@ -373,15 +381,17 @@ func applyRule(log logr.Logger, client *dclient.Client, rule kyverno.Rule, resou
isUpdate = true
}
} else {
if label["policy.kyverno.io/synchronize"] == "enable" {
isUpdate = true
if label["policy.kyverno.io/synchronize"] == "disable" {
isUpdate = false
}
}

if rule.Generation.Synchronize {
label["policy.kyverno.io/synchronize"] = "enable"
} else {
label["policy.kyverno.io/synchronize"] = "disable"
}

if isUpdate {
logger.V(4).Info("updating existing resource")
newResource.SetLabels(label)
Expand All @@ -402,8 +412,6 @@ func applyRule(log logr.Logger, client *dclient.Client, rule kyverno.Rule, resou
}
logger.V(2).Info("updated generated resource")
}

logger.V(2).Info("Synchronize resource is disabled")
}
return newGenResource, nil
}
Expand Down Expand Up @@ -493,15 +501,5 @@ func getUnstrRule(rule *kyverno.Generation) (*unstructured.Unstructured, error)
if err != nil {
return nil, err
}
return ConvertToUnstructured(ruleData)
}

//ConvertToUnstructured converts the resource to unstructured format
func ConvertToUnstructured(data []byte) (*unstructured.Unstructured, error) {
resource := &unstructured.Unstructured{}
err := resource.UnmarshalJSON(data)
if err != nil {
return nil, err
}
return resource, nil
return utils.ConvertToUnstructured(ruleData)
}
2 changes: 1 addition & 1 deletion pkg/policy/background.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ func ContainsVariablesOtherThanObject(policy kyverno.ClusterPolicy) error {
ctx := context.NewContext(filterVars...)
for condIdx, condition := range rule.Conditions {
if condition.Key, err = variables.SubstituteVars(log.Log, ctx, condition.Key); !checkNotFoundErr(err) {
return fmt.Errorf("invalid variable %s used at spec/rules[%d]/condition[%d]/key",condition.Key, idx, condIdx)
return fmt.Errorf("invalid variable %s used at spec/rules[%d]/condition[%d]/key", condition.Key, idx, condIdx)
}

if condition.Value, err = variables.SubstituteVars(log.Log, ctx, condition.Value); !checkNotFoundErr(err) {
Expand Down
10 changes: 2 additions & 8 deletions pkg/policyreport/builder.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -137,18 +137,12 @@ func (builder *requestBuilder) build(info Info) (req *unstructured.Unstructured,
}

// deletion of a result entry
// - on resource deleteion:
// - info.Rules == 0 && info.PolicyName == ""
// - set label delete.resource=resourceKind-resourceNamespace-resourceName
// - on policy deleteion:
// - info.PolicyName != "" && info.Resource == {}
// - set label delete.policy=policyName
if len(info.Rules) == 0 && info.PolicyName == "" {
if len(info.Rules) == 0 && info.PolicyName == "" { // on resource deleteion
req.SetLabels(map[string]string{
resourceLabelNamespace: info.Resource.GetNamespace(),
deletedLabelResource: info.Resource.GetName(),
deletedLabelResourceKind: info.Resource.GetKind()})
} else if info.PolicyName != "" && reflect.DeepEqual(info.Resource, unstructured.Unstructured{}) {
} else if info.PolicyName != "" && reflect.DeepEqual(info.Resource, unstructured.Unstructured{}) { // on policy deleteion
req.SetKind("ReportChangeRequest")

if len(info.Rules) == 0 {
Expand Down
24 changes: 12 additions & 12 deletions pkg/policyreport/reportcontroller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ func (g *ReportGenerator) handleErr(err error, key interface{}) {
}

// syncHandler reconciles clusterPolicyReport if namespace == ""
// otherwise it updates policyrReport
// otherwise it updates policyReport
func (g *ReportGenerator) syncHandler(key string) error {
if policy, rule, ok := isDeletedPolicyKey(key); ok {
return g.removePolicyEntryFromReport(policy, rule)
Expand All @@ -251,7 +251,7 @@ func (g *ReportGenerator) syncHandler(key string) error {
return err
}

g.cleanupReportRequets(aggregatedRequests)
g.cleanupReportRequests(aggregatedRequests)
return nil
}

Expand All @@ -274,7 +274,7 @@ func (g *ReportGenerator) createReportIfNotPresent(namespace string, new *unstru
}

log.V(2).Info("successfully created policyReport", "namespace", new.GetNamespace(), "name", new.GetName())
g.cleanupReportRequets(aggregatedRequests)
g.cleanupReportRequests(aggregatedRequests)
return nil, nil
}

Expand All @@ -290,7 +290,7 @@ func (g *ReportGenerator) createReportIfNotPresent(namespace string, new *unstru
}

log.V(2).Info("successfully created ClusterPolicyReport")
g.cleanupReportRequets(aggregatedRequests)
g.cleanupReportRequests(aggregatedRequests)
return nil, nil
}
return nil, nil
Expand Down Expand Up @@ -373,15 +373,15 @@ func (g *ReportGenerator) removePolicyEntryFromReport(policyName, ruleName strin
return err
}

g.cleanupReportRequets(aggregatedRequests)
g.cleanupReportRequests(aggregatedRequests)
return nil
}

func (g *ReportGenerator) aggregateReports(namespace string) (
report *unstructured.Unstructured, aggregatedRequests interface{}, err error) {

if namespace == "" {
requests, err := g.clusterReportLister.List(labels.Everything())
requests, err := g.clusterReportChangeRequestLister.List(labels.Everything())
if err != nil {
return nil, nil, fmt.Errorf("unable to list ClusterReportChangeRequests within: %v", err)
}
Expand Down Expand Up @@ -505,14 +505,14 @@ func (g *ReportGenerator) updateReport(old interface{}, new *unstructured.Unstru
return nil
}

oldUnstructed := make(map[string]interface{})
oldUnstructured := make(map[string]interface{})

if oldTyped, ok := old.(*report.ClusterPolicyReport); ok {
if oldTyped.GetDeletionTimestamp() != nil {
return g.dclient.DeleteResource(oldTyped.APIVersion, "ClusterPolicyReport", oldTyped.Namespace, oldTyped.Name, false)
}

if oldUnstructed, err = runtime.DefaultUnstructuredConverter.ToUnstructured(oldTyped); err != nil {
if oldUnstructured, err = runtime.DefaultUnstructuredConverter.ToUnstructured(oldTyped); err != nil {
return fmt.Errorf("unable to convert clusterPolicyReport: %v", err)
}
new.SetUID(oldTyped.GetUID())
Expand All @@ -522,21 +522,21 @@ func (g *ReportGenerator) updateReport(old interface{}, new *unstructured.Unstru
return g.dclient.DeleteResource(oldTyped.APIVersion, "PolicyReport", oldTyped.Namespace, oldTyped.Name, false)
}

if oldUnstructed, err = runtime.DefaultUnstructuredConverter.ToUnstructured(oldTyped); err != nil {
if oldUnstructured, err = runtime.DefaultUnstructuredConverter.ToUnstructured(oldTyped); err != nil {
return fmt.Errorf("unable to convert policyReport: %v", err)
}

new.SetUID(oldTyped.GetUID())
new.SetResourceVersion(oldTyped.GetResourceVersion())
}

obj, err := updateResults(oldUnstructed, new.UnstructuredContent(), aggregatedRequests)
obj, err := updateResults(oldUnstructured, new.UnstructuredContent(), aggregatedRequests)
if err != nil {
return fmt.Errorf("failed to update results entry: %v", err)
}
new.Object = obj

if !hasResultsChanged(oldUnstructed, new.UnstructuredContent()) {
if !hasResultsChanged(oldUnstructured, new.UnstructuredContent()) {
g.log.V(4).Info("unchanged policy report", "namespace", new.GetNamespace(), "name", new.GetName())
return nil
}
Expand All @@ -549,7 +549,7 @@ func (g *ReportGenerator) updateReport(old interface{}, new *unstructured.Unstru
return
}

func (g *ReportGenerator) cleanupReportRequets(requestsGeneral interface{}) {
func (g *ReportGenerator) cleanupReportRequests(requestsGeneral interface{}) {
defer g.log.V(5).Info("successfully cleaned up report requests")
if requests, ok := requestsGeneral.([]*changerequest.ReportChangeRequest); ok {
for _, request := range requests {
Expand Down
8 changes: 4 additions & 4 deletions pkg/policyreport/reportrequest.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -310,23 +310,23 @@ func (gen *Generator) sync(reportReq *unstructured.Unstructured, info Info) erro
}

func updateReportChangeRequest(dClient *client.Client, old interface{}, new *unstructured.Unstructured, log logr.Logger) (err error) {
oldUnstructed := make(map[string]interface{})
oldUnstructured := make(map[string]interface{})
if oldTyped, ok := old.(*changerequest.ReportChangeRequest); ok {
if oldUnstructed, err = runtime.DefaultUnstructuredConverter.ToUnstructured(oldTyped); err != nil {
if oldUnstructured, err = runtime.DefaultUnstructuredConverter.ToUnstructured(oldTyped); err != nil {
return fmt.Errorf("unable to convert reportChangeRequest: %v", err)
}
new.SetResourceVersion(oldTyped.GetResourceVersion())
new.SetUID(oldTyped.GetUID())
} else {
oldTyped := old.(*changerequest.ClusterReportChangeRequest)
if oldUnstructed, err = runtime.DefaultUnstructuredConverter.ToUnstructured(oldTyped); err != nil {
if oldUnstructured, err = runtime.DefaultUnstructuredConverter.ToUnstructured(oldTyped); err != nil {
return fmt.Errorf("unable to convert clusterReportChangeRequest: %v", err)
}
new.SetUID(oldTyped.GetUID())
new.SetResourceVersion(oldTyped.GetResourceVersion())
}

if !hasResultsChanged(oldUnstructed, new.UnstructuredContent()) {
if !hasResultsChanged(oldUnstructured, new.UnstructuredContent()) {
log.V(4).Info("unchanged report request", "name", new.GetName())
return nil
}
Expand Down
1 change: 0 additions & 1 deletion pkg/webhookconfig/common.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,6 @@ func generateDebugValidatingWebhook(name, url string, caData []byte, validate bo
}
}


// mutating webhook
func generateMutatingWebhook(name, servicePath string, caData []byte, validation bool, timeoutSeconds int32, resources []string, apiGroups, apiVersions string, operationTypes []admregapi.OperationType) admregapi.MutatingWebhook {
sideEffect := admregapi.SideEffectClassNoneOnDryRun
Expand Down
8 changes: 3 additions & 5 deletions pkg/webhookconfig/registration.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ const (
// 4. Resource Mutation
// 5. Webhook Status Mutation
type Register struct {
client *client.Client
clientConfig *rest.Config
client *client.Client
clientConfig *rest.Config
serverIP string // when running outside a cluster
timeoutSeconds int32
log logr.Logger
Expand Down Expand Up @@ -293,7 +293,7 @@ func (wrc *Register) removePolicyMutatingWebhookConfiguration(wg *sync.WaitGroup
if errorsapi.IsNotFound(err) {
logger.V(5).Info("policy mutating webhook configuration not found")
return
}
}

if err != nil {
logger.Error(err, "failed to delete policy mutating webhook configuration")
Expand Down Expand Up @@ -423,8 +423,6 @@ func (wrc *Register) getVerifyWebhookMutatingWebhookName() string {
return mutatingConfig
}



// GetWebhookTimeOut returns the value of webhook timeout
func (wrc *Register) GetWebhookTimeOut() time.Duration {
return time.Duration(wrc.timeoutSeconds)
Expand Down
Loading

0 comments on commit 624b481

Please sign in to comment.