Skip to content

Commit

Permalink
minor lint/readability (#2976)
Browse files Browse the repository at this point in the history 
* simplify a couple loops

* if/else -> switch

* drop redundant else

* comment + drop var declaration + explicit zero return

* lint (whitespace/fmt.Errorf)
  • Loading branch information
@mmetc
mmetc authored May 2, 2024
1 parent 91fbc63 commit 529d3b2
Show file tree
Hide file tree
Showing 5 changed files with 212 additions and 128 deletions.
36 changes: 23 additions & 13 deletions pkg/alertcontext/alertcontext.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,7 @@ const (
maxContextValueLen = 4000
)

var (
alertContext = Context{}
)
var alertContext = Context{}

type Context struct {
ContextToSend map[string][]string
Expand All @@ -37,19 +35,21 @@ func ValidateContextExpr(key string, expressions []string) error {
return fmt.Errorf("compilation of '%s' failed: %v", expression, err)
}
}

return nil
}

func NewAlertContext(contextToSend map[string][]string, valueLength int) error {
var clog = log.New()
clog := log.New()
if err := types.ConfigureLogger(clog); err != nil {
return fmt.Errorf("couldn't create logger for alert context: %s", err)
return fmt.Errorf("couldn't create logger for alert context: %w", err)
}

if valueLength == 0 {
clog.Debugf("No console context value length provided, using default: %d", maxContextValueLen)
valueLength = maxContextValueLen
}

if valueLength > maxContextValueLen {
clog.Debugf("Provided console context value length (%d) is higher than the maximum, using default: %d", valueLength, maxContextValueLen)
valueLength = maxContextValueLen
Expand All @@ -76,6 +76,7 @@ func NewAlertContext(contextToSend map[string][]string, valueLength int) error {
if err != nil {
return fmt.Errorf("compilation of '%s' context value failed: %v", value, err)
}

alertContext.ContextToSendCompiled[key] = append(alertContext.ContextToSendCompiled[key], valueCompiled)
alertContext.ContextToSend[key] = append(alertContext.ContextToSend[key], value)
}
Expand All @@ -85,16 +86,13 @@ func NewAlertContext(contextToSend map[string][]string, valueLength int) error {
}

func truncate(values []string, contextValueLen int) (string, error) {
var ret string
valueByte, err := json.Marshal(values)
if err != nil {
return "", fmt.Errorf("unable to dump metas: %s", err)
return "", fmt.Errorf("unable to dump metas: %w", err)
}
ret = string(valueByte)
for {
if len(ret) <= contextValueLen {
break
}

ret := string(valueByte)
for len(ret) > contextValueLen {
// if there is only 1 value left and that the size is too big, truncate it
if len(values) == 1 {
valueToTruncate := values[0]
Expand All @@ -106,12 +104,15 @@ func truncate(values []string, contextValueLen int) (string, error) {
// if there is multiple value inside, just remove the last one
values = values[:len(values)-1]
}

valueByte, err = json.Marshal(values)
if err != nil {
return "", fmt.Errorf("unable to dump metas: %s", err)
return "", fmt.Errorf("unable to dump metas: %w", err)
}

ret = string(valueByte)
}

return ret, nil
}

Expand All @@ -120,18 +121,22 @@ func EventToContext(events []types.Event) (models.Meta, []error) {

metas := make([]*models.MetaItems0, 0)
tmpContext := make(map[string][]string)

for _, evt := range events {
for key, values := range alertContext.ContextToSendCompiled {
if _, ok := tmpContext[key]; !ok {
tmpContext[key] = make([]string, 0)
}

for _, value := range values {
var val string

output, err := expr.Run(value, map[string]interface{}{"evt": evt})
if err != nil {
errors = append(errors, fmt.Errorf("failed to get value for %s : %v", key, err))
continue
}

switch out := output.(type) {
case string:
val = out
Expand All @@ -141,20 +146,24 @@ func EventToContext(events []types.Event) (models.Meta, []error) {
errors = append(errors, fmt.Errorf("unexpected return type for %s : %T", key, output))
continue
}

if val != "" && !slices.Contains(tmpContext[key], val) {
tmpContext[key] = append(tmpContext[key], val)
}
}
}
}

for key, values := range tmpContext {
if len(values) == 0 {
continue
}

valueStr, err := truncate(values, alertContext.ContextValueLen)
if err != nil {
log.Warningf(err.Error())
}

meta := models.MetaItems0{
Key: key,
Value: valueStr,
Expand All @@ -163,5 +172,6 @@ func EventToContext(events []types.Event) (models.Meta, []error) {
}

ret := models.Meta(metas)

return ret, errors
}
58 changes: 33 additions & 25 deletions pkg/apiserver/apic.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,12 +81,12 @@ func randomDuration(d time.Duration, delta time.Duration) time.Duration {

func (a *apic) FetchScenariosListFromDB() ([]string, error) {
scenarios := make([]string, 0)
machines, err := a.dbClient.ListMachines()

machines, err := a.dbClient.ListMachines()
if err != nil {
return nil, fmt.Errorf("while listing machines: %w", err)
}
//merge all scenarios together
// merge all scenarios together
for _, v := range machines {
machineScenarios := strings.Split(v.Scenarios, ",")
log.Debugf("%d scenarios for machine %d", len(machineScenarios), v.ID)
Expand All @@ -113,7 +113,7 @@ func decisionsToApiDecisions(decisions []*models.Decision) models.AddSignalsRequ
Origin: ptr.Of(*decision.Origin),
Scenario: ptr.Of(*decision.Scenario),
Scope: ptr.Of(*decision.Scope),
//Simulated: *decision.Simulated,
// Simulated: *decision.Simulated,
Type: ptr.Of(*decision.Type),
Until: decision.Until,
Value: ptr.Of(*decision.Value),
Expand Down Expand Up @@ -196,8 +196,8 @@ func NewAPIC(config *csconfig.OnlineApiClientCfg, dbClient *database.Client, con
}

password := strfmt.Password(config.Credentials.Password)
apiURL, err := url.Parse(config.Credentials.URL)

apiURL, err := url.Parse(config.Credentials.URL)
if err != nil {
return nil, fmt.Errorf("while parsing '%s': %w", config.Credentials.URL, err)
}
Expand Down Expand Up @@ -376,7 +376,6 @@ func (a *apic) Send(cacheOrig *models.AddSignalsRequest) {
defer cancel()

_, _, err := a.apiClient.Signal.Add(ctx, &send)

if err != nil {
log.Errorf("sending signal to central API: %s", err)
return
Expand All @@ -391,9 +390,8 @@ func (a *apic) Send(cacheOrig *models.AddSignalsRequest) {
defer cancel()

_, _, err := a.apiClient.Signal.Add(ctx, &send)

if err != nil {
//we log it here as well, because the return value of func might be discarded
// we log it here as well, because the return value of func might be discarded
log.Errorf("sending signal to central API: %s", err)
}

Expand All @@ -407,8 +405,8 @@ func (a *apic) CAPIPullIsOld() (bool, error) {
alerts := a.dbClient.Ent.Alert.Query()
alerts = alerts.Where(alert.HasDecisionsWith(decision.OriginEQ(database.CapiMachineID)))
alerts = alerts.Where(alert.CreatedAtGTE(time.Now().UTC().Add(-time.Duration(1*time.Hour + 30*time.Minute)))) //nolint:unconvert
count, err := alerts.Count(a.dbClient.CTX)

count, err := alerts.Count(a.dbClient.CTX)
if err != nil {
return false, fmt.Errorf("while looking for CAPI alert: %w", err)
}
Expand Down Expand Up @@ -506,6 +504,7 @@ func createAlertsForDecisions(decisions []*models.Decision) []*models.Alert {
if sub.Scenario == nil {
log.Warningf("nil scenario in %+v", sub)
}

if *sub.Scenario == *decision.Scenario {
found = true
break
Expand Down Expand Up @@ -567,7 +566,7 @@ func createAlertForDecision(decision *models.Decision) *models.Alert {
// This function takes in list of parent alerts and decisions and then pairs them up.
func fillAlertsWithDecisions(alerts []*models.Alert, decisions []*models.Decision, addCounters map[string]map[string]int) []*models.Alert {
for _, decision := range decisions {
//count and create separate alerts for each list
// count and create separate alerts for each list
updateCounterForDecision(addCounters, decision.Origin, decision.Scenario, 1)

/*CAPI might send lower case scopes, unify it.*/
Expand All @@ -579,7 +578,7 @@ func fillAlertsWithDecisions(alerts []*models.Alert, decisions []*models.Decisio
}

found := false
//add the individual decisions to the right list
// add the individual decisions to the right list
for idx, alert := range alerts {
if *decision.Origin == types.CAPIOrigin {
if *alert.Source.Scope == types.CAPIOrigin {
Expand All @@ -592,6 +591,7 @@ func fillAlertsWithDecisions(alerts []*models.Alert, decisions []*models.Decisio
if *alert.Source.Scope == types.ListOrigin && *alert.Scenario == *decision.Scenario {
alerts[idx].Decisions = append(alerts[idx].Decisions, decision)
found = true

break
}
} else {
Expand All @@ -613,8 +613,8 @@ func fillAlertsWithDecisions(alerts []*models.Alert, decisions []*models.Decisio
func (a *apic) PullTop(forcePull bool) error {
var err error

//A mutex with TryLock would be a bit simpler
//But go does not guarantee that TryLock will be able to acquire the lock even if it is available
// A mutex with TryLock would be a bit simpler
// But go does not guarantee that TryLock will be able to acquire the lock even if it is available
select {
case a.isPulling <- true:
defer func() {
Expand All @@ -633,6 +633,7 @@ func (a *apic) PullTop(forcePull bool) error {
}

log.Debug("Acquiring lock for pullCAPI")

err = a.dbClient.AcquirePullCAPILock()
if a.dbClient.IsLocked(err) {
log.Info("PullCAPI is already running, skipping")
Expand All @@ -642,6 +643,7 @@ func (a *apic) PullTop(forcePull bool) error {
/*defer lock release*/
defer func() {
log.Debug("Releasing lock for pullCAPI")

if err := a.dbClient.ReleasePullCAPILock(); err != nil {
log.Errorf("while releasing lock: %v", err)
}
Expand Down Expand Up @@ -681,7 +683,7 @@ func (a *apic) PullTop(forcePull bool) error {

// create one alert for community blocklist using the first decision
decisions := a.apiClient.Decisions.GetDecisionsFromGroups(data.New)
//apply APIC specific whitelists
// apply APIC specific whitelists
decisions = a.ApplyApicWhitelists(decisions)

alert := createAlertForDecision(decisions[0])
Expand Down Expand Up @@ -740,7 +742,7 @@ func (a *apic) ApplyApicWhitelists(decisions []*models.Decision) []*models.Decis
if a.whitelists == nil || len(a.whitelists.Cidrs) == 0 && len(a.whitelists.Ips) == 0 {
return decisions
}
//deal with CAPI whitelists for fire. We want to avoid having a second list, so we shrink in place
// deal with CAPI whitelists for fire. We want to avoid having a second list, so we shrink in place
outIdx := 0

for _, decision := range decisions {
Expand All @@ -753,7 +755,7 @@ func (a *apic) ApplyApicWhitelists(decisions []*models.Decision) []*models.Decis
decisions[outIdx] = decision
outIdx++
}
//shrink the list, those are deleted items
// shrink the list, those are deleted items
return decisions[:outIdx]
}

Expand Down Expand Up @@ -782,8 +784,8 @@ func (a *apic) ShouldForcePullBlocklist(blocklist *modelscapi.BlocklistLink) (bo
alertQuery := a.dbClient.Ent.Alert.Query()
alertQuery.Where(alert.SourceScopeEQ(fmt.Sprintf("%s:%s", types.ListOrigin, *blocklist.Name)))
alertQuery.Order(ent.Desc(alert.FieldCreatedAt))
alertInstance, err := alertQuery.First(context.Background())

alertInstance, err := alertQuery.First(context.Background())
if err != nil {
if ent.IsNotFound(err) {
log.Debugf("no alert found for %s, force refresh", *blocklist.Name)
Expand All @@ -795,8 +797,8 @@ func (a *apic) ShouldForcePullBlocklist(blocklist *modelscapi.BlocklistLink) (bo

decisionQuery := a.dbClient.Ent.Decision.Query()
decisionQuery.Where(decision.HasOwnerWith(alert.IDEQ(alertInstance.ID)))
firstDecision, err := decisionQuery.First(context.Background())

firstDecision, err := decisionQuery.First(context.Background())
if err != nil {
if ent.IsNotFound(err) {
log.Debugf("no decision found for %s, force refresh", *blocklist.Name)
Expand Down Expand Up @@ -872,7 +874,7 @@ func (a *apic) updateBlocklist(client *apiclient.ApiClient, blocklist *modelscap
log.Infof("blocklist %s has no decisions", *blocklist.Name)
return nil
}
//apply APIC specific whitelists
// apply APIC specific whitelists
decisions = a.ApplyApicWhitelists(decisions)
alert := createAlertForDecision(decisions[0])
alertsFromCapi := []*models.Alert{alert}
Expand Down Expand Up @@ -911,12 +913,17 @@ func (a *apic) UpdateBlocklists(links *modelscapi.GetDecisionsStreamResponseLink
}

func setAlertScenario(alert *models.Alert, addCounters map[string]map[string]int, deleteCounters map[string]map[string]int) {
if *alert.Source.Scope == types.CAPIOrigin {
switch *alert.Source.Scope {
case types.CAPIOrigin:
*alert.Source.Scope = types.CommunityBlocklistPullSourceScope
alert.Scenario = ptr.Of(fmt.Sprintf("update : +%d/-%d IPs", addCounters[types.CAPIOrigin]["all"], deleteCounters[types.CAPIOrigin]["all"]))
} else if *alert.Source.Scope == types.ListOrigin {
alert.Scenario = ptr.Of(fmt.Sprintf("update : +%d/-%d IPs",
addCounters[types.CAPIOrigin]["all"],
deleteCounters[types.CAPIOrigin]["all"]))
case types.ListOrigin:
*alert.Source.Scope = fmt.Sprintf("%s:%s", types.ListOrigin, *alert.Scenario)
alert.Scenario = ptr.Of(fmt.Sprintf("update : +%d/-%d IPs", addCounters[types.ListOrigin][*alert.Scenario], deleteCounters[types.ListOrigin][*alert.Scenario]))
alert.Scenario = ptr.Of(fmt.Sprintf("update : +%d/-%d IPs",
addCounters[types.ListOrigin][*alert.Scenario],
deleteCounters[types.ListOrigin][*alert.Scenario]))
}
}

Expand Down Expand Up @@ -988,11 +995,12 @@ func makeAddAndDeleteCounters() (map[string]map[string]int, map[string]map[strin
}

func updateCounterForDecision(counter map[string]map[string]int, origin *string, scenario *string, totalDecisions int) {
if *origin == types.CAPIOrigin {
switch *origin {
case types.CAPIOrigin:
counter[*origin]["all"] += totalDecisions
} else if *origin == types.ListOrigin {
case types.ListOrigin:
counter[*origin][*scenario] += totalDecisions
} else {
default:
log.Warningf("Unknown origin %s", *origin)
}
}
Loading

0 comments on commit 529d3b2

Please sign in to comment.