Skip to content

Commit

Permalink
fix(reports-controller): Allow UPDATE operation in validation
Browse files Browse the repository at this point in the history
Signed-off-by: Bhargav Ravuri <[email protected]>
  • Loading branch information
Bhargav-InfraCloud committed Feb 1, 2024
1 parent f1cf6ef commit d258fa8
Show file tree
Hide file tree
Showing 8 changed files with 839 additions and 103 deletions.
2 changes: 1 addition & 1 deletion pkg/controllers/report/utils/scanner.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ func (s *scanner) validateResource(ctx context.Context, resource unstructured.Un
WithNewResource(resource).
WithPolicy(policy).
WithNamespaceLabels(nsLabels)
response := s.engine.Validate(ctx, policyCtx)
response := s.engine.Validate(ctx, policyCtx, []kyvernov1.AdmissionOperation{kyvernov1.Create, kyvernov1.Update}...)
return &response, nil
}

Expand Down
1 change: 1 addition & 0 deletions pkg/engine/api/engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ type Engine interface {
Validate(
ctx context.Context,
policyContext PolicyContext,
operations ...kyvernov1.AdmissionOperation,
) EngineResponse

// Mutate performs mutation. Overlay first and then mutation patches
Expand Down
4 changes: 2 additions & 2 deletions pkg/engine/background.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,10 +87,10 @@ func (e *engine) filterRule(
policy := policyContext.Policy()
gvk, subresource := policyContext.ResourceKind()

if err := engineutils.MatchesResourceDescription(newResource, rule, admissionInfo, namespaceLabels, policy.GetNamespace(), gvk, subresource, policyContext.Operation()); err != nil {
if err := engineutils.MatchesResourceDescription(newResource, rule, admissionInfo, namespaceLabels, policy.GetNamespace(), gvk, subresource, []kyvernov1.AdmissionOperation{policyContext.Operation()}); err != nil {
if ruleType == engineapi.Generation {
// if the oldResource matched, return "false" to delete GR for it
if err = engineutils.MatchesResourceDescription(oldResource, rule, admissionInfo, namespaceLabels, policy.GetNamespace(), gvk, subresource, policyContext.Operation()); err == nil {
if err = engineutils.MatchesResourceDescription(oldResource, rule, admissionInfo, namespaceLabels, policy.GetNamespace(), gvk, subresource, []kyvernov1.AdmissionOperation{policyContext.Operation()}); err == nil {
return engineapi.RuleFail(rule.Name, ruleType, "")
}
}
Expand Down
11 changes: 7 additions & 4 deletions pkg/engine/engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,12 +87,13 @@ func NewEngine(
func (e *engine) Validate(
ctx context.Context,
policyContext engineapi.PolicyContext,
operations ...kyvernov1.AdmissionOperation,
) engineapi.EngineResponse {
startTime := time.Now()
response := engineapi.NewEngineResponseFromPolicyContext(policyContext)
logger := internal.LoggerWithPolicyContext(logging.WithName("engine.validate"), policyContext)
if internal.MatchPolicyContext(logger, policyContext, e.configuration) {
policyResponse := e.validate(ctx, logger, policyContext)
policyResponse := e.validate(ctx, logger, policyContext, operations)
response = response.WithPolicyResponse(policyResponse)
}
response = response.WithStats(engineapi.NewExecutionStats(startTime, time.Now()))
Expand Down Expand Up @@ -191,6 +192,7 @@ func (e *engine) matches(
rule kyvernov1.Rule,
policyContext engineapi.PolicyContext,
resource unstructured.Unstructured,
operations []kyvernov1.AdmissionOperation,
) error {
if policyContext.AdmissionOperation() {
request := policyContext.AdmissionInfo()
Expand All @@ -207,7 +209,7 @@ func (e *engine) matches(
policyContext.Policy().GetNamespace(),
gvk,
subresource,
policyContext.Operation(),
operations,
)
if err == nil {
return nil
Expand All @@ -222,7 +224,7 @@ func (e *engine) matches(
policyContext.Policy().GetNamespace(),
gvk,
subresource,
policyContext.Operation(),
operations,
)
if err == nil {
return nil
Expand All @@ -239,14 +241,15 @@ func (e *engine) invokeRuleHandler(
resource unstructured.Unstructured,
rule kyvernov1.Rule,
ruleType engineapi.RuleType,
operations ...kyvernov1.AdmissionOperation,
) (unstructured.Unstructured, []engineapi.RuleResponse) {
return tracing.ChildSpan2(
ctx,
"pkg/engine",
fmt.Sprintf("RULE %s", rule.Name),
func(ctx context.Context, span trace.Span) (patchedResource unstructured.Unstructured, results []engineapi.RuleResponse) {
// check if resource and rule match
if err := e.matches(rule, policyContext, resource); err != nil {
if err := e.matches(rule, policyContext, resource, operations); err != nil {
logger.V(4).Info("rule not matched", "reason", err.Error())
return resource, nil
}
Expand Down
35 changes: 19 additions & 16 deletions pkg/engine/utils/match.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,13 +57,16 @@ func doesResourceMatchConditionBlock(
namespaceLabels map[string]string,
gvk schema.GroupVersionKind,
subresource string,
operation kyvernov1.AdmissionOperation,
operations []kyvernov1.AdmissionOperation,
) []error {
if len(conditionBlock.Operations) > 0 {
if !slices.Contains(conditionBlock.Operations, operation) {
// if operation does not match, return immediately
err := fmt.Errorf("operation does not match")
return []error{err}
if !slices.ContainsFunc(operations, func(op kyvernov1.AdmissionOperation) bool {
return slices.Contains(conditionBlock.Operations, op)
}) {
// If none of the input operations match the operations from condition block, return immediately
return []error{fmt.Errorf("one or more operations from %v doesn't match any from %v",

Check failure on line 67 in pkg/engine/utils/match.go

View workflow job for this annotation

GitHub Actions / tests

File is not `gofumpt`-ed (gofumpt)
operations, conditionBlock.Operations),
}
}
}

Expand Down Expand Up @@ -173,7 +176,7 @@ func MatchesResourceDescription(
policyNamespace string,
gvk schema.GroupVersionKind,
subresource string,
operation kyvernov1.AdmissionOperation,
operations []kyvernov1.AdmissionOperation,
) error {
if resource.Object == nil {
return fmt.Errorf("resource is empty")
Expand All @@ -190,7 +193,7 @@ func MatchesResourceDescription(
oneMatched := false
for _, rmr := range rule.MatchResources.Any {
// if there are no errors it means it was a match
if len(matchesResourceDescriptionMatchHelper(rmr, admissionInfo, resource, namespaceLabels, gvk, subresource, operation)) == 0 {
if len(matchesResourceDescriptionMatchHelper(rmr, admissionInfo, resource, namespaceLabels, gvk, subresource, operations)) == 0 {
oneMatched = true
break
}
Expand All @@ -201,27 +204,27 @@ func MatchesResourceDescription(
} else if len(rule.MatchResources.All) > 0 {
// include object if ALL of the criteria match
for _, rmr := range rule.MatchResources.All {
reasonsForFailure = append(reasonsForFailure, matchesResourceDescriptionMatchHelper(rmr, admissionInfo, resource, namespaceLabels, gvk, subresource, operation)...)
reasonsForFailure = append(reasonsForFailure, matchesResourceDescriptionMatchHelper(rmr, admissionInfo, resource, namespaceLabels, gvk, subresource, operations)...)
}
} else {
rmr := kyvernov1.ResourceFilter{UserInfo: rule.MatchResources.UserInfo, ResourceDescription: rule.MatchResources.ResourceDescription}
reasonsForFailure = append(reasonsForFailure, matchesResourceDescriptionMatchHelper(rmr, admissionInfo, resource, namespaceLabels, gvk, subresource, operation)...)
reasonsForFailure = append(reasonsForFailure, matchesResourceDescriptionMatchHelper(rmr, admissionInfo, resource, namespaceLabels, gvk, subresource, operations)...)
}

// check exlude conditions only if match succeeds
if len(reasonsForFailure) == 0 {
if len(rule.ExcludeResources.Any) > 0 {
// exclude the object if ANY of the criteria match
for _, rer := range rule.ExcludeResources.Any {
reasonsForFailure = append(reasonsForFailure, matchesResourceDescriptionExcludeHelper(rer, admissionInfo, resource, namespaceLabels, gvk, subresource, operation)...)
reasonsForFailure = append(reasonsForFailure, matchesResourceDescriptionExcludeHelper(rer, admissionInfo, resource, namespaceLabels, gvk, subresource, operations)...)
}
} else if len(rule.ExcludeResources.All) > 0 {
// exclude the object if ALL the criteria match
excludedByAll := true
for _, rer := range rule.ExcludeResources.All {
// we got no errors inplying a resource did NOT exclude it
// "matchesResourceDescriptionExcludeHelper" returns errors if resource is excluded by a filter
if len(matchesResourceDescriptionExcludeHelper(rer, admissionInfo, resource, namespaceLabels, gvk, subresource, operation)) == 0 {
if len(matchesResourceDescriptionExcludeHelper(rer, admissionInfo, resource, namespaceLabels, gvk, subresource, operations)) == 0 {
excludedByAll = false
break
}
Expand All @@ -231,7 +234,7 @@ func MatchesResourceDescription(
}
} else {
rer := kyvernov1.ResourceFilter{UserInfo: rule.ExcludeResources.UserInfo, ResourceDescription: rule.ExcludeResources.ResourceDescription}
reasonsForFailure = append(reasonsForFailure, matchesResourceDescriptionExcludeHelper(rer, admissionInfo, resource, namespaceLabels, gvk, subresource, operation)...)
reasonsForFailure = append(reasonsForFailure, matchesResourceDescriptionExcludeHelper(rer, admissionInfo, resource, namespaceLabels, gvk, subresource, operations)...)
}
}

Expand All @@ -257,7 +260,7 @@ func matchesResourceDescriptionMatchHelper(
namespaceLabels map[string]string,
gvk schema.GroupVersionKind,
subresource string,
operation kyvernov1.AdmissionOperation,
operations []kyvernov1.AdmissionOperation,
) []error {
var errs []error
if datautils.DeepEqual(admissionInfo, kyvernov1beta1.RequestInfo{}) {
Expand All @@ -267,7 +270,7 @@ func matchesResourceDescriptionMatchHelper(
// checking if resource matches the rule
if !datautils.DeepEqual(rmr.ResourceDescription, kyvernov1.ResourceDescription{}) ||
!datautils.DeepEqual(rmr.UserInfo, kyvernov1.UserInfo{}) {
matchErrs := doesResourceMatchConditionBlock(rmr.ResourceDescription, rmr.UserInfo, admissionInfo, resource, namespaceLabels, gvk, subresource, operation)
matchErrs := doesResourceMatchConditionBlock(rmr.ResourceDescription, rmr.UserInfo, admissionInfo, resource, namespaceLabels, gvk, subresource, operations)
errs = append(errs, matchErrs...)
} else {
errs = append(errs, fmt.Errorf("match cannot be empty"))
Expand All @@ -282,13 +285,13 @@ func matchesResourceDescriptionExcludeHelper(
namespaceLabels map[string]string,
gvk schema.GroupVersionKind,
subresource string,
operation kyvernov1.AdmissionOperation,
operations []kyvernov1.AdmissionOperation,
) []error {
var errs []error
// checking if resource matches the rule
if !datautils.DeepEqual(rer.ResourceDescription, kyvernov1.ResourceDescription{}) ||
!datautils.DeepEqual(rer.UserInfo, kyvernov1.UserInfo{}) {
excludeErrs := doesResourceMatchConditionBlock(rer.ResourceDescription, rer.UserInfo, admissionInfo, resource, namespaceLabels, gvk, subresource, operation)
excludeErrs := doesResourceMatchConditionBlock(rer.ResourceDescription, rer.UserInfo, admissionInfo, resource, namespaceLabels, gvk, subresource, operations)
// it was a match so we want to exclude it
if len(excludeErrs) == 0 {
errs = append(errs, fmt.Errorf("resource excluded since one of the criteria excluded it"))
Expand Down
Loading

0 comments on commit d258fa8

Please sign in to comment.