Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: expose sinks pkg for direct rule evaluation; add ValidationResult creation & finalization helpers #366

Merged
merged 3 commits into from
Aug 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions cmd/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,10 @@ import (

validationv1alpha1 "github.com/validator-labs/validator/api/v1alpha1"
"github.com/validator-labs/validator/internal/controller"
"github.com/validator-labs/validator/internal/kube"
"github.com/validator-labs/validator/internal/sinks"
"github.com/validator-labs/validator/pkg/helm"
"github.com/validator-labs/validator/pkg/helm/release"
"github.com/validator-labs/validator/pkg/kube"
"github.com/validator-labs/validator/pkg/sinks"
//+kubebuilder:scaffold:imports
)

Expand Down
4 changes: 2 additions & 2 deletions internal/controller/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,10 @@ import (
"sigs.k8s.io/controller-runtime/pkg/log/zap"

"github.com/validator-labs/validator/api/v1alpha1"
"github.com/validator-labs/validator/internal/kube"
"github.com/validator-labs/validator/internal/sinks"
"github.com/validator-labs/validator/pkg/helm"
"github.com/validator-labs/validator/pkg/helm/release"
"github.com/validator-labs/validator/pkg/kube"
"github.com/validator-labs/validator/pkg/sinks"
"github.com/validator-labs/validator/pkg/util"
//+kubebuilder:scaffold:imports
)
Expand Down
2 changes: 1 addition & 1 deletion internal/controller/validationresult_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ import (
"sigs.k8s.io/controller-runtime/pkg/client"

v1alpha1 "github.com/validator-labs/validator/api/v1alpha1"
"github.com/validator-labs/validator/internal/sinks"
"github.com/validator-labs/validator/pkg/constants"
"github.com/validator-labs/validator/pkg/sinks"
"github.com/validator-labs/validator/pkg/types"
)

Expand Down
2 changes: 1 addition & 1 deletion internal/controller/validatorconfig_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ import (
kyaml "sigs.k8s.io/yaml"

"github.com/validator-labs/validator/api/v1alpha1"
"github.com/validator-labs/validator/internal/test"
"github.com/validator-labs/validator/pkg/helm"
"github.com/validator-labs/validator/pkg/test"
//+kubebuilder:scaffold:imports
)

Expand Down
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
80 changes: 62 additions & 18 deletions pkg/validationresult/validation_result.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

import (
"context"
"fmt"
"strings"
"time"

Expand All @@ -25,8 +26,44 @@
Patch(ctx context.Context, obj client.Object, opts ...patch.Option) error
}

// HandleExistingValidationResult processes a preexisting validation result for the active validator.
func HandleExistingValidationResult(vr *v1alpha1.ValidationResult, l logr.Logger) {
// ValidationRule is an interface for validation rules.
type ValidationRule interface {
client.Object
PluginCode() string
ResultCount() int
}

// Build creates a new ValidationResult for a specific ValidationRule.
func Build(r ValidationRule) *v1alpha1.ValidationResult {
return &v1alpha1.ValidationResult{
ObjectMeta: metav1.ObjectMeta{
Name: Name(r),
Namespace: r.GetNamespace(),
OwnerReferences: []metav1.OwnerReference{

Check warning on line 42 in pkg/validationresult/validation_result.go

View check run for this annotation

Codecov / codecov/patch

pkg/validationresult/validation_result.go#L37-L42

Added lines #L37 - L42 were not covered by tests
{
APIVersion: r.GetObjectKind().GroupVersionKind().Version,
Kind: r.GetObjectKind().GroupVersionKind().Kind,
Name: r.GetName(),
UID: r.GetUID(),
Controller: util.Ptr(true),
},
},
},
Spec: v1alpha1.ValidationResultSpec{
Plugin: r.PluginCode(),
ExpectedResults: r.ResultCount(),
},

Check warning on line 55 in pkg/validationresult/validation_result.go

View check run for this annotation

Codecov / codecov/patch

pkg/validationresult/validation_result.go#L44-L55

Added lines #L44 - L55 were not covered by tests
}
}

// Name returns the name of a ValidationResult for a specific ValidationRule.
func Name(r ValidationRule) string {
name := fmt.Sprintf("validator-plugin-%s-%s", r.PluginCode(), r.GetName())
return util.Sanitize(name)

Check warning on line 62 in pkg/validationresult/validation_result.go

View check run for this annotation

Codecov / codecov/patch

pkg/validationresult/validation_result.go#L60-L62

Added lines #L60 - L62 were not covered by tests
}

// HandleExisting processes a preexisting validation result for the active validator.
func HandleExisting(vr *v1alpha1.ValidationResult, l logr.Logger) {
l = l.WithValues("name", vr.Name, "namespace", vr.Namespace, "state", vr.Status.State)

switch vr.Status.State {
Expand All @@ -49,8 +86,8 @@
}
}

// HandleNewValidationResult creates a new validation result for the active validator.
func HandleNewValidationResult(ctx context.Context, c client.Client, p Patcher, vr *v1alpha1.ValidationResult, l logr.Logger) error {
// HandleNew creates a new validation result for the active validator.
func HandleNew(ctx context.Context, c client.Client, p Patcher, vr *v1alpha1.ValidationResult, l logr.Logger) error {
l = l.WithValues("name", vr.Name, "namespace", vr.Namespace)

// Create the ValidationResult
Expand All @@ -75,11 +112,26 @@
return nil
}

// SafeUpdateValidationResult updates a ValidationResult, ensuring
// that the overall validation status remains failed if a single rule fails.
func SafeUpdateValidationResult(ctx context.Context, p Patcher, vr *v1alpha1.ValidationResult, vrr types.ValidationResponse, l logr.Logger) error {
// SafeUpdate processes and patches a ValidationResult with retry logic.
func SafeUpdate(ctx context.Context, p Patcher, vr *v1alpha1.ValidationResult, vrr types.ValidationResponse, l logr.Logger) error {
l = l.WithValues("name", vr.Name, "namespace", vr.Namespace)

if err := Finalize(vr, vrr, l); err != nil {
l.Error(err, "failed to finalize ValidationResult")
return err

Check warning on line 121 in pkg/validationresult/validation_result.go

View check run for this annotation

Codecov / codecov/patch

pkg/validationresult/validation_result.go#L120-L121

Added lines #L120 - L121 were not covered by tests
}
if err := patchValidationResult(ctx, p, vr); err != nil {
l.Error(err, "failed to patch ValidationResult")
return err
}

l.V(0).Info("Successfully patched ValidationResult", "state", vr.Status.State)
return nil
}

// Finalize finalizes a ValidationResult, ensuring
// that the overall validation status remains failed if a single rule fails.
func Finalize(vr *v1alpha1.ValidationResult, vrr types.ValidationResponse, l logr.Logger) error {
for i, r := range vrr.ValidationRuleResults {
// Handle nil ValidationRuleResult
if r == nil {
Expand All @@ -90,21 +142,13 @@
}
}
// Update overall ValidationResult status
updateValidationResultStatus(vr, r, vrr.ValidationRuleErrors[i], l)
}

l.V(0).Info("Preparing to patch ValidationResult")
if err := patchValidationResult(ctx, p, vr); err != nil {
l.Error(err, "failed to patch ValidationResult")
return err
updateStatus(vr, r, vrr.ValidationRuleErrors[i], l)
}

l.V(0).Info("Successfully patched ValidationResult", "state", vr.Status.State)
return nil
}

// updateValidationResultStatus updates a ValidationResult's status with the result of a single validation rule.
func updateValidationResultStatus(vr *v1alpha1.ValidationResult, vrr *types.ValidationRuleResult, vrrErr error, l logr.Logger) {
// updateStatus updates a ValidationResult's status with the result of a single validation rule.
func updateStatus(vr *v1alpha1.ValidationResult, vrr *types.ValidationRuleResult, vrrErr error, l logr.Logger) {

// Finalize result State and Condition in the event of an unexpected error
if vrrErr != nil {
Expand Down
10 changes: 5 additions & 5 deletions pkg/validationresult/validation_result_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ import (
corev1 "k8s.io/api/core/v1"

"github.com/validator-labs/validator/api/v1alpha1"
"github.com/validator-labs/validator/internal/test"
"github.com/validator-labs/validator/pkg/constants"
"github.com/validator-labs/validator/pkg/test"
"github.com/validator-labs/validator/pkg/types"
"github.com/validator-labs/validator/pkg/util"
)
Expand Down Expand Up @@ -69,7 +69,7 @@ func TestHandleExistingValidationResult(t *testing.T) {
}
for _, c := range cs {
t.Log(c.name)
HandleExistingValidationResult(c.vr, logr.Logger{})
HandleExisting(c.vr, logr.Logger{})
}
}

Expand Down Expand Up @@ -133,7 +133,7 @@ func TestHandleNewValidationResult(t *testing.T) {
}
for _, c := range cs {
t.Log(c.name)
err = HandleNewValidationResult(context.Background(), c.client, c.patcher, c.vr, logr.Logger{})
err = HandleNew(context.Background(), c.client, c.patcher, c.vr, logr.Logger{})
if err != nil && !reflect.DeepEqual(c.expected.Error(), err.Error()) {
t.Errorf("expected (%v), got (%v)", c.expected, err)
}
Expand Down Expand Up @@ -221,7 +221,7 @@ func TestSafeUpdateValidationResult(t *testing.T) {
}
for _, c := range cs {
t.Log(c.name)
SafeUpdateValidationResult(context.Background(), c.patcher, c.vr, c.vrr, logr.Logger{})
SafeUpdate(context.Background(), c.patcher, c.vr, c.vrr, logr.Logger{})
}
}

Expand Down Expand Up @@ -299,7 +299,7 @@ func TestUpdateValidationResultStatus(t *testing.T) {
}
for _, c := range cs {
t.Log(c.name)
updateValidationResultStatus(c.vrCurr, c.vrr, c.vrrErr, logr.Logger{})
updateStatus(c.vrCurr, c.vrr, c.vrrErr, logr.Logger{})
if !reflect.DeepEqual(c.vrCurr.Hash(), c.vrExpected.Hash()) {
t.Errorf("expected (%+v), got (%+v)", c.vrExpected, c.vrCurr)
}
Expand Down