feat: Handle Secret Lookup Rate Limit errors

Author: adityathebe

api/context/context.go

@@ -5,14 +5,15 @@ import (
 	"fmt"
 	"strings"
 
-	"github.com/flanksource/canary-checker/api/external"
-	v1 "github.com/flanksource/canary-checker/api/v1"
-	"github.com/flanksource/canary-checker/pkg"
 	"github.com/flanksource/duty/connection"
 	dutyCtx "github.com/flanksource/duty/context"
 	"github.com/flanksource/duty/models"
 	"github.com/flanksource/duty/types"
 	"github.com/flanksource/gomplate/v3"
+
+	"github.com/flanksource/canary-checker/api/external"
+	v1 "github.com/flanksource/canary-checker/api/v1"
+	"github.com/flanksource/canary-checker/pkg"
 )
 
 var DefaultContext dutyCtx.Context

checks/kubernetes_resource.go

@@ -200,7 +200,7 @@ func (c *KubernetesResourceChecker) Check(ctx context.Context, check v1.Kubernet
 
 			ctx = _ctx.(context.Context)
 			checkCtx := context.New(ctx.Context, virtualCanary)
-			res, err := Exec(checkCtx)
+			res, err := RunChecksNoPersistence(checkCtx)
 			if err != nil {
 				return fmt.Errorf("error executing check: %w", err)
 			} else {

checks/runchecks.go

@@ -9,20 +9,37 @@ import (
 	"time"
 
 	"github.com/flanksource/artifacts"
+	dutyCtx "github.com/flanksource/duty/context"
+	"github.com/flanksource/duty/models"
+	"github.com/google/uuid"
+	gocache "github.com/patrickmn/go-cache"
+	"golang.org/x/time/rate"
+
 	"github.com/flanksource/canary-checker/api/context"
 	"github.com/flanksource/canary-checker/api/external"
 	v1 "github.com/flanksource/canary-checker/api/v1"
 	"github.com/flanksource/canary-checker/pkg"
 	"github.com/flanksource/canary-checker/pkg/utils"
-	"github.com/flanksource/duty/models"
-	"github.com/google/uuid"
-	gocache "github.com/patrickmn/go-cache"
+)
+
+const (
+	defaultSecretLookupFailureThreshold = 50
+	defaultSecretLookupFailureWindow    = 30 * time.Minute
 )
 
 var checksCache = gocache.New(5*time.Minute, 5*time.Minute)
 
+var secretLookupRateLimiter = rate.NewLimiter(
+	rate.Limit(float64(defaultSecretLookupFailureThreshold)/defaultSecretLookupFailureWindow.Seconds()),
+	defaultSecretLookupFailureThreshold,
+)
+
 var DisabledChecks []string
 
+type RunChecksMeta struct {
+	SecretLookupRateLimitSkipped int
+}
+
 func getDisabledChecks(ctx *context.Context) (map[string]struct{}, error) {
 	if val, ok := checksCache.Get("disabledChecks"); ok {
 		return val.(map[string]struct{}), nil
@@ -150,8 +167,13 @@ func sortChecksByDependency(checks []external.Check) ([]external.Check, error) {
 	return append(unnamedChecks, sorted...), nil
 }
 
-// Exec runs the actions specified and returns the results, without saving artifacts
-func Exec(ctx *context.Context) ([]*pkg.CheckResult, error) {
+// RunChecksNoPersistence executes checks and returns transformed results without persistence side effects.
+//
+// Unlike RunChecks, it does not perform canary deletion checks, dependency ordering,
+// artifact saving, or final result post-processing (e.g. resultMode handling).
+// It exports only check spec metrics (not per-result emitted metrics) and is intended
+// for embedded execution paths (e.g. topology lookups and kubernetesResource sub-checks).
+func RunChecksNoPersistence(ctx *context.Context) ([]*pkg.CheckResult, error) {
 	var results []*pkg.CheckResult
 	disabledChecks, err := getDisabledChecks(ctx)
 	if err != nil {
@@ -174,9 +196,11 @@ func Exec(ctx *context.Context) ([]*pkg.CheckResult, error) {
 
 		result := c.Run(ctx)
 		transformedResults := TransformResults(ctx, result)
-		results = append(results, transformedResults...)
-		ExportCheckMetrics(ctx, transformedResults, false)
+		_, filteredResults := filterSecretLookupRateLimitedResults(ctx, transformedResults)
+		results = append(results, filteredResults...)
+		ExportCheckMetrics(ctx, filteredResults, false)
 	}
+
 	return results, nil
 }
 
@@ -189,23 +213,24 @@ func hasDependencies(checks []external.Check) bool {
 	return false
 }
 
-func RunChecks(ctx *context.Context) ([]*pkg.CheckResult, error) {
+func RunChecks(ctx *context.Context) ([]*pkg.CheckResult, RunChecksMeta, error) {
 	var results []*pkg.CheckResult
+	meta := RunChecksMeta{}
 	disabledChecks, err := getDisabledChecks(ctx)
 	if err != nil {
-		return nil, fmt.Errorf("error getting disabled checks: %v", err)
+		return nil, meta, fmt.Errorf("error getting disabled checks: %v", err)
 	}
 
 	// Check if canary is not marked deleted in DB
 	if ctx.DB() != nil && ctx.Canary.GetPersistedID() != "" {
 		var deletedAt sql.NullTime
 		err := ctx.DB().Table("canaries").Select("deleted_at").Where("id = ? and deleted_at < now()", ctx.Canary.GetPersistedID()).Scan(&deletedAt).Error
 		if err != nil {
-			return nil, fmt.Errorf("error getting canary: %v", err)
+			return nil, meta, fmt.Errorf("error getting canary: %v", err)
 		}
 
 		if deletedAt.Valid {
-			return nil, nil
+			return nil, meta, nil
 		}
 	}
 
@@ -215,7 +240,7 @@ func RunChecks(ctx *context.Context) ([]*pkg.CheckResult, error) {
 	if hasDependencies(checks) {
 		sortedChecks, err := sortChecksByDependency(checks)
 		if err != nil {
-			return nil, fmt.Errorf("failed to sort checks: %v", err)
+			return nil, meta, fmt.Errorf("failed to sort checks: %v", err)
 		}
 
 		for _, check := range sortedChecks {
@@ -241,11 +266,13 @@ func RunChecks(ctx *context.Context) ([]*pkg.CheckResult, error) {
 
 			result := singleRunner.Check(ctx, check)
 			transformedResults := TransformResults(ctx, result)
-			results = append(results, transformedResults...)
-			ExportCheckMetrics(ctx, transformedResults, true)
+			skippedCount, filteredResults := filterSecretLookupRateLimitedResults(ctx, transformedResults)
+			meta.SecretLookupRateLimitSkipped += skippedCount
+			results = append(results, filteredResults...)
+			ExportCheckMetrics(ctx, filteredResults, true)
 
-			if check.GetName() != "" && len(transformedResults) > 0 && transformedResults[0].Pass {
-				ctx.SetOutput(check.GetName(), transformedResults[0])
+			if check.GetName() != "" && len(filteredResults) > 0 && filteredResults[0].Pass {
+				ctx.SetOutput(check.GetName(), filteredResults[0])
 			}
 		}
 	} else {
@@ -259,16 +286,18 @@ func RunChecks(ctx *context.Context) ([]*pkg.CheckResult, error) {
 
 			result := c.Run(ctx)
 			transformedResults := TransformResults(ctx, result)
-			results = append(results, transformedResults...)
-			ExportCheckMetrics(ctx, transformedResults, true)
+			skippedCount, filteredResults := filterSecretLookupRateLimitedResults(ctx, transformedResults)
+			meta.SecretLookupRateLimitSkipped += skippedCount
+			results = append(results, filteredResults...)
+			ExportCheckMetrics(ctx, filteredResults, true)
 		}
 	}
 
 	if err := saveArtifacts(ctx, results); err != nil {
 		ctx.Errorf("error saving artifacts: %v", err)
 	}
 
-	return ProcessResults(ctx, results), nil
+	return ProcessResults(ctx, results), meta, nil
 }
 
 func saveArtifacts(ctx *context.Context, results pkg.Results) error {
@@ -356,6 +385,48 @@ func TransformResults(ctx *context.Context, in []*pkg.CheckResult) (out []*pkg.C
 	return out
 }
 
+func isSecretLookupRateLimitResult(result *pkg.CheckResult) bool {
+	if result == nil {
+		return false
+	}
+	if dutyCtx.IsSecretLookupRateLimited(result.ErrorObject) {
+		return true
+	}
+	return strings.Contains(strings.ToLower(result.Error), strings.ToLower(dutyCtx.ErrSecretLookupRateLimited.Error()))
+}
+
+func filterSecretLookupRateLimitedResults(ctx *context.Context, in []*pkg.CheckResult) (int, []*pkg.CheckResult) {
+	if len(in) == 0 {
+		return 0, in
+	}
+
+	skipped := 0
+	out := make([]*pkg.CheckResult, 0, len(in))
+
+	for _, result := range in {
+		if !isSecretLookupRateLimitResult(result) {
+			out = append(out, result)
+			continue
+		}
+
+		if secretLookupRateLimiter.Allow() {
+			skipped++
+			ctx.Warnf("skipping check result due to secret lookup rate limiting for check=%s (threshold=%d window=%s)", result.GetName(), defaultSecretLookupFailureThreshold, defaultSecretLookupFailureWindow.Round(time.Second))
+			continue
+		}
+
+		ctx.Warnf("secret lookup rate limit threshold exceeded for check=%s (threshold=%d window=%s), recording as failure", result.GetName(), defaultSecretLookupFailureThreshold, defaultSecretLookupFailureWindow.Round(time.Second))
+		result.Invalid = false
+		result.Pass = false
+		if result.Error == "" {
+			result.Error = "secret lookup repeatedly rate limited"
+		}
+		out = append(out, result)
+	}
+
+	return skipped, out
+}
+
 func ProcessResults(ctx *context.Context, results []*pkg.CheckResult) []*pkg.CheckResult {
 	if ctx.Canary.Spec.ResultMode == "" {
 		return results

cmd/run.go

@@ -67,7 +67,7 @@ var Run = &cobra.Command{
 				go func() {
 					defer wg.Done()
 
-					res, err := checks.RunChecks(apicontext.New(apicontext.DefaultContext.WithName(_config.Name), _config))
+					res, _, err := checks.RunChecks(apicontext.New(apicontext.DefaultContext.WithName(_config.Name), _config))
 					if err != nil {
 						log.Errorf("error running checks: %v", err)
 						return

go.mod

@@ -19,7 +19,7 @@ require (
 	github.com/elastic/go-elasticsearch/v8 v8.19.0
 	github.com/flanksource/artifacts v1.0.18
 	github.com/flanksource/commons v1.44.1
-	github.com/flanksource/duty v1.0.1175
+	github.com/flanksource/duty v1.0.1176
 	github.com/flanksource/gomplate/v3 v3.24.66
 	github.com/flanksource/is-healthy v1.0.82
 	github.com/flanksource/kommons v0.31.7

go.sum

@@ -1030,6 +1030,8 @@ github.com/flanksource/deps v1.0.23 h1:yamxNjZe/gbxxS60QWqhq+0X1CzJjDjxAm9WZjQ3p
 github.com/flanksource/deps v1.0.23/go.mod h1:s0HIPy85aWYkLxAsTywMenz6N29VdEKrytzE/bP8o48=
 github.com/flanksource/duty v1.0.1175 h1:FrQxOcEn8LlWc1dJNkk14/MbpaZlT+i3VVxXxFs6mZg=
 github.com/flanksource/duty v1.0.1175/go.mod h1:rirTJLPHps66SNz9erbriF3aHiSMb05x72361t7Gfsk=
+github.com/flanksource/duty v1.0.1176 h1:pGgaUX8d1U6oe3upgND6LggqvULJul+a2jMuboD4G54=
+github.com/flanksource/duty v1.0.1176/go.mod h1:rirTJLPHps66SNz9erbriF3aHiSMb05x72361t7Gfsk=
 github.com/flanksource/gomplate/v3 v3.20.4/go.mod h1:27BNWhzzSjDed1z8YShO6W+z6G9oZXuxfNFGd/iGSdc=
 github.com/flanksource/gomplate/v3 v3.24.66 h1:fTaN0s9t+YZCau+KlgcLn9pMcLTsSiMjBnZUbhGY/oY=
 github.com/flanksource/gomplate/v3 v3.24.66/go.mod h1:PiYJOAk971BpG/suhFP9YAZSjfz4KiRaqwYlQZZJp0Q=

pkg/api/run_now.go

@@ -49,7 +49,7 @@ func RunCanaryHandler(c echo.Context) error {
 	}
 
 	ctx := context.New(duty, *canary)
-	results, err := checks.RunChecks(ctx)
+	results, _, err := checks.RunChecks(ctx)
 	if err != nil {
 		return errorResponse(c, err, http.StatusInternalServerError)
 	}

pkg/jobs/canary/canary_jobs.go

@@ -3,6 +3,7 @@ package canary
 import (
 	"errors"
 	"fmt"
+	"math/rand"
 	"sync"
 	"time"
 
@@ -101,11 +102,15 @@ func (j CanaryJob) Run(ctx dutyjob.JobRuntime) error {
 		attribute.String("canary.namespace", j.Canary.Namespace),
 	)
 
-	results, err := checks.RunChecks(canaryCtx)
+	results, runMeta, err := checks.RunChecks(canaryCtx)
 	if err != nil {
 		return err
 	}
 
+	if runMeta.SecretLookupRateLimitSkipped > 0 {
+		maybeRescheduleAfterSecretLookupRateLimit(ctx.Context, j.DBCanary, j.Canary, runMeta.SecretLookupRateLimitSkipped)
+	}
+
 	// Get transformed checks before and after, and then delete the olds ones that are not in new set.
 	// NOTE: Webhook checks, although they are transformed, are handled entirely by the webhook caller
 	// and should not be deleted manually in here.
@@ -164,6 +169,28 @@ func (j CanaryJob) Run(ctx dutyjob.JobRuntime) error {
 	return nil
 }
 
+func maybeRescheduleAfterSecretLookupRateLimit(ctx context.Context, dbCanary pkg.Canary, canary v1.Canary, skipped int) {
+	nextRuntime, err := canary.NextRuntime(time.Now())
+	if err != nil || nextRuntime == nil {
+		ctx.Warnf("failed to compute next runtime for canary[%s/%s]: %v", canary.Namespace, canary.Name, err)
+		return
+	}
+
+	if time.Until(*nextRuntime) <= 5*time.Minute {
+		return
+	}
+
+	rng := rand.New(rand.NewSource(time.Now().UnixNano()))
+	delay := time.Minute + time.Duration(rng.Intn(60))*time.Second
+	runAt := time.Now().Add(delay)
+	if err := TriggerAt(ctx, dbCanary, runAt); err != nil {
+		ctx.Warnf("failed to schedule earlier retry for canary[%s/%s] after %d skipped results: %v", canary.Namespace, canary.Name, skipped, err)
+		return
+	}
+
+	ctx.Infof("scheduled earlier retry for canary[%s/%s] in %s after %d secret lookup rate-limited skips", canary.Namespace, canary.Name, delay.Round(time.Second), skipped)
+}
+
 func SaveResults(ctx context.Context, results []*pkg.CheckResult) ([]string, map[string]string, error) {
 	var transformedChecksCreated []string
 	// Transformed checks have a delete strategy

pkg/jobs/canary/status.go

@@ -22,6 +22,10 @@ func UpdateCanaryStatusAndEvent(ctx context.Context, canary v1.Canary, results [
 		return
 	}
 
+	if len(results) == 0 {
+		return
+	}
+
 	// Skip function if canary is not sourced from Kubernetes CRD
 	if !strings.HasPrefix(canary.Annotations["source"], "kubernetes") {
 		return

pkg/topology/run.go

@@ -220,7 +220,7 @@ func lookup(ctx *ComponentContext, name string, spec v1.CanarySpec) ([]interface
 	// canaryCtx.Environment = ctx.
 	// canaryCtx.Logger = ctx.Logger
 
-	checkResults, err := checks.Exec(canaryCtx)
+	checkResults, err := checks.RunChecksNoPersistence(canaryCtx)
 	if err != nil {
 		return nil, err
 	}