diff --git a/controllers/alphacontrollers/rulegroup_controller.go b/controllers/alphacontrollers/rulegroup_controller.go index 56b12ef..a15ca03 100644 --- a/controllers/alphacontrollers/rulegroup_controller.go +++ b/controllers/alphacontrollers/rulegroup_controller.go @@ -78,6 +78,7 @@ func (r *RuleGroupReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( return ctrl.Result{RequeueAfter: defaultErrRequeuePeriod}, err } + //TODO: rename finalizer // name of our custom finalizer myFinalizerName := "batch.tutorial.kubebuilder.io/finalizer" @@ -129,20 +130,29 @@ func (r *RuleGroupReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( return ctrl.Result{}, nil } - var notFount bool - var err error - var actualState *coralogixv1alpha1.RuleGroupStatus + var ( + notFound bool + err error + actualState *coralogixv1alpha1.RuleGroupStatus + ) + if id := ruleGroupCRD.Status.ID; id == nil { log.V(1).Info("ruleGroup wasn't created") - notFount = true + notFound = true } else if getRuleGroupResp, err := rulesGroupsClient.GetRuleGroup(ctx, &rulesgroups.GetRuleGroupRequest{GroupId: *id}); status.Code(err) == codes.NotFound { + // TODO: this needs more error handling, what if it returns internal error? + log.V(1).Info("ruleGroup doesn't exist in Coralogix backend") - notFount = true + notFound = true } else if err == nil { - actualState = flattenRuleGroup(getRuleGroupResp.GetRuleGroup()) + actualState, err = flattenRuleGroup(getRuleGroupResp.GetRuleGroup()) + if err != nil { + log.Error(err, "Error mapping coralogix API response", "Name", ruleGroupCRD.Name, "Namespace", ruleGroupCRD.Namespace) + return ctrl.Result{RequeueAfter: defaultErrRequeuePeriod}, err + } } - if notFount { + if notFound { createRuleGroupReq := ruleGroupCRD.Spec.ExtractCreateRuleGroupRequest() jstr, _ := jsm.MarshalToString(createRuleGroupReq) log.V(1).Info("Creating Rule-Group", "ruleGroup", jstr) @@ -158,7 +168,12 @@ func (r *RuleGroupReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( return ctrl.Result{RequeueAfter: defaultErrRequeuePeriod}, err } - ruleGroupCRD.Status = *flattenRuleGroup(createRuleGroupResp.GetRuleGroup()) + status, err := flattenRuleGroup(createRuleGroupResp.GetRuleGroup()) + if err != nil { + log.Error(err, "Error mapping coralogix API response", "Name", ruleGroupCRD.Name, "Namespace", ruleGroupCRD.Namespace) + return ctrl.Result{RequeueAfter: defaultErrRequeuePeriod}, err + } + ruleGroupCRD.Status = *status if err := r.Status().Update(ctx, ruleGroupCRD); err != nil { log.V(1).Error(err, "updating crd") } @@ -187,7 +202,7 @@ func (r *RuleGroupReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( return ctrl.Result{RequeueAfter: defaultRequeuePeriod}, nil } -func flattenRuleGroup(ruleGroup *rulesgroups.RuleGroup) *coralogixv1alpha1.RuleGroupStatus { +func flattenRuleGroup(ruleGroup *rulesgroups.RuleGroup) (*coralogixv1alpha1.RuleGroupStatus, error) { var status coralogixv1alpha1.RuleGroupStatus status.ID = new(string) @@ -197,7 +212,11 @@ func flattenRuleGroup(ruleGroup *rulesgroups.RuleGroup) *coralogixv1alpha1.RuleG status.Active = ruleGroup.GetEnabled().GetValue() - status.Applications, status.Subsystems, status.Severities = flattenRuleMatcher(ruleGroup.GetRuleMatchers()) + var err error + status.Applications, status.Subsystems, status.Severities, err = flattenRuleMatcher(ruleGroup.GetRuleMatchers()) + if err != nil { + return nil, fmt.Errorf("flattenRuleGroup name: %s: %w", ruleGroup.GetName().GetValue(), err) + } status.Description = ruleGroup.Description.GetValue() @@ -208,21 +227,27 @@ func flattenRuleGroup(ruleGroup *rulesgroups.RuleGroup) *coralogixv1alpha1.RuleG status.Hidden = ruleGroup.GetHidden().GetValue() - status.RuleSubgroups = flattenRuleSubGroups(ruleGroup.GetRuleSubgroups()) - - return &status + subGroups, err := flattenRuleSubGroups(ruleGroup.GetRuleSubgroups()) + if err != nil { + return nil, fmt.Errorf("flattenRuleGroup name: %s: %w", ruleGroup.GetName().GetValue(), err) + } + status.RuleSubgroups = subGroups + return &status, nil } -func flattenRuleSubGroups(subgroups []*rulesgroups.RuleSubgroup) []coralogixv1alpha1.RuleSubGroup { +func flattenRuleSubGroups(subgroups []*rulesgroups.RuleSubgroup) ([]coralogixv1alpha1.RuleSubGroup, error) { result := make([]coralogixv1alpha1.RuleSubGroup, 0, len(subgroups)) for _, sg := range subgroups { - subgroup := flattenRuleSubGroup(sg) + subgroup, err := flattenRuleSubGroup(sg) + if err != nil { + return nil, fmt.Errorf("flattenRuleSubGroups subGroupId: %s: %w", sg.GetId().GetValue(), err) + } result = append(result, subgroup) } - return result + return result, nil } -func flattenRuleSubGroup(subGroup *rulesgroups.RuleSubgroup) coralogixv1alpha1.RuleSubGroup { +func flattenRuleSubGroup(subGroup *rulesgroups.RuleSubgroup) (coralogixv1alpha1.RuleSubGroup, error) { var result coralogixv1alpha1.RuleSubGroup result.ID = new(string) @@ -233,21 +258,27 @@ func flattenRuleSubGroup(subGroup *rulesgroups.RuleSubgroup) coralogixv1alpha1.R result.Order = new(int32) *result.Order = int32(subGroup.GetOrder().GetValue()) - result.Rules = flattenRules(subGroup.Rules) - - return result + rules, err := flattenRules(subGroup.Rules) + if err != nil { + return coralogixv1alpha1.RuleSubGroup{}, fmt.Errorf("flattenRuleSubGroup: %w", err) + } + result.Rules = rules + return result, nil } -func flattenRules(rules []*rulesgroups.Rule) []coralogixv1alpha1.Rule { +func flattenRules(rules []*rulesgroups.Rule) ([]coralogixv1alpha1.Rule, error) { result := make([]coralogixv1alpha1.Rule, 0, len(rules)) for _, r := range rules { - rule := flattenRule(r) + rule, err := flattenRule(r) + if err != nil { + return nil, fmt.Errorf("flattenRules: %w", err) + } result = append(result, rule) } - return result + return result, nil } -func flattenRule(rule *rulesgroups.Rule) coralogixv1alpha1.Rule { +func flattenRule(rule *rulesgroups.Rule) (coralogixv1alpha1.Rule, error) { var result coralogixv1alpha1.Rule result.Name = rule.GetName().GetValue() result.Active = rule.GetEnabled().GetValue() @@ -325,12 +356,12 @@ func flattenRule(rule *rulesgroups.Rule) coralogixv1alpha1.Rule { KeepDestinationField: !(jsonParseParameters.GetOverrideDest().GetValue()), } default: - panic(fmt.Sprintf("unexpected type %T for r parameters", ruleParams)) + return coralogixv1alpha1.Rule{}, fmt.Errorf("unexpected type %T for rule parameters, name: %s", ruleParams, rule.GetName().GetValue()) } - return result + return result, nil } -func flattenRuleMatcher(ruleMatchers []*rulesgroups.RuleMatcher) (applications, subsystems []string, severities []coralogixv1alpha1.RuleSeverity) { +func flattenRuleMatcher(ruleMatchers []*rulesgroups.RuleMatcher) (applications, subsystems []string, severities []coralogixv1alpha1.RuleSeverity, err error) { for _, ruleMatcher := range ruleMatchers { switch matcher := ruleMatcher.Constraint.(type) { case *rulesgroups.RuleMatcher_ApplicationName: @@ -341,7 +372,7 @@ func flattenRuleMatcher(ruleMatchers []*rulesgroups.RuleMatcher) (applications, severity := matcher.Severity.GetValue() severities = append(severities, coralogixv1alpha1.RulesProtoSeverityToSchemaSeverity[severity]) default: - panic(fmt.Sprintf("unexpected type %T for rule matcher", ruleMatcher)) + return nil, nil, nil, fmt.Errorf("unexpected type %T for rule matcher", ruleMatcher) } } diff --git a/controllers/alphacontrollers/rulegroup_controller_test.go b/controllers/alphacontrollers/rulegroup_controller_test.go new file mode 100644 index 0000000..e7be2a5 --- /dev/null +++ b/controllers/alphacontrollers/rulegroup_controller_test.go @@ -0,0 +1,123 @@ +package alphacontrollers + +import ( + "testing" + + coralogixv1alpha1 "github.com/coralogix/coralogix-operator/apis/coralogix/v1alpha1" + rulesgroups "github.com/coralogix/coralogix-operator/controllers/clientset/grpc/rules-groups/v1" + "github.com/stretchr/testify/assert" + "google.golang.org/protobuf/types/known/wrapperspb" +) + +func TestFlattenRuleGroupsErrorsOnBadResponse(t *testing.T) { + ruleGroup := &rulesgroups.RuleGroup{ + Id: wrapperspb.String("id"), + Name: wrapperspb.String("name"), + Description: wrapperspb.String("description"), + Creator: wrapperspb.String("creator"), + Enabled: wrapperspb.Bool(true), + Hidden: wrapperspb.Bool(false), + RuleMatchers: []*rulesgroups.RuleMatcher{}, + RuleSubgroups: []*rulesgroups.RuleSubgroup{ + { + Rules: []*rulesgroups.Rule{ + { + Id: wrapperspb.String("rule_id"), + Name: wrapperspb.String("rule_name"), + Description: wrapperspb.String("rule_description"), + SourceField: wrapperspb.String("text"), + Parameters: &rulesgroups.RuleParameters{ + RuleParameters: nil, + }, + Enabled: wrapperspb.Bool(true), + Order: wrapperspb.UInt32(1), + }, + }, + }, + }, + Order: wrapperspb.UInt32(1), + } + + status, err := flattenRuleGroup(ruleGroup) + assert.Error(t, err) + assert.Nil(t, status) +} + +func TestFlattenRuleGroups(t *testing.T) { + ruleGroup := &rulesgroups.RuleGroup{ + Id: wrapperspb.String("id"), + Name: wrapperspb.String("name"), + Description: wrapperspb.String("description"), + Creator: wrapperspb.String("creator"), + Enabled: wrapperspb.Bool(true), + Hidden: wrapperspb.Bool(false), + RuleMatchers: []*rulesgroups.RuleMatcher{}, + RuleSubgroups: []*rulesgroups.RuleSubgroup{ + { + Id: wrapperspb.String("subgroup_id"), + Order: wrapperspb.UInt32(2), + Rules: []*rulesgroups.Rule{ + { + Id: wrapperspb.String("rule_id"), + Name: wrapperspb.String("rule_name"), + Description: wrapperspb.String("rule_description"), + SourceField: wrapperspb.String("text"), + Parameters: &rulesgroups.RuleParameters{ + RuleParameters: &rulesgroups.RuleParameters_JsonExtractParameters{ + JsonExtractParameters: &rulesgroups.JsonExtractParameters{ + DestinationField: rulesgroups.JsonExtractParameters_DESTINATION_FIELD_SEVERITY, + Rule: wrapperspb.String(`{"severity": "info"}`), + }, + }, + }, + Enabled: wrapperspb.Bool(true), + Order: wrapperspb.UInt32(3), + }, + }, + }, + }, + Order: wrapperspb.UInt32(1), + } + + status, err := flattenRuleGroup(ruleGroup) + assert.NoError(t, err) + + id := "id" + subgroupId := "subgroup_id" + expected := &coralogixv1alpha1.RuleGroupStatus{ + ID: &id, + Name: "name", + Description: "description", + Active: true, + Applications: nil, + Subsystems: nil, + Severities: nil, + Hidden: false, + Creator: "creator", + Order: int32Ptr(1), + RuleSubgroups: []coralogixv1alpha1.RuleSubGroup{ + { + ID: &subgroupId, + Active: false, + Order: int32Ptr(2), + Rules: []coralogixv1alpha1.Rule{ + { + Name: "rule_name", + Description: "rule_description", + Active: true, + Parse: nil, + Block: nil, + JsonExtract: &coralogixv1alpha1.JsonExtract{ + DestinationField: coralogixv1alpha1.DestinationFieldRuleSeverity, + JsonKey: "{\"severity\": \"info\"}", + }, + }, + }, + }, + }, + } + + assert.Equal(t, expected, status) +} + +func int32Ptr(i int32) *int32 { return &i } diff --git a/go.mod b/go.mod index e4c336a..40d3153 100644 --- a/go.mod +++ b/go.mod @@ -9,6 +9,7 @@ require ( github.com/onsi/ginkgo/v2 v2.9.5 github.com/onsi/gomega v1.27.7 github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring v0.64.1 + github.com/stretchr/testify v1.8.2 google.golang.org/grpc v1.53.0 google.golang.org/protobuf v1.30.0 k8s.io/apimachinery v0.27.2 @@ -47,12 +48,12 @@ require ( github.com/modern-go/reflect2 v1.0.2 // indirect github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect github.com/pkg/errors v0.9.1 // indirect + github.com/pmezard/go-difflib v1.0.0 // indirect github.com/prometheus/client_golang v1.15.1 // indirect github.com/prometheus/client_model v0.4.0 // indirect github.com/prometheus/common v0.42.0 // indirect github.com/prometheus/procfs v0.9.0 // indirect github.com/spf13/pflag v1.0.5 // indirect - github.com/stretchr/testify v1.8.2 // indirect go.uber.org/atomic v1.10.0 // indirect go.uber.org/multierr v1.6.0 // indirect go.uber.org/zap v1.24.0 // indirect