chore: Address comments in PR #854

pkg/argocd/argocd.go

@@ -9,6 +9,7 @@ import (
  "time"
 
  "github.com/argoproj-labs/argocd-image-updater/pkg/common"
+ "github.com/argoproj-labs/argocd-image-updater/pkg/env"
  "github.com/argoproj-labs/argocd-image-updater/pkg/image"
  "github.com/argoproj-labs/argocd-image-updater/pkg/kube"
  "github.com/argoproj-labs/argocd-image-updater/pkg/log"
@@ -31,7 +32,7 @@ func (client *k8sClient) GetApplication(ctx context.Context, appName string) (*v
  log.Debugf("Getting application %s across all namespaces", appName)
 
  // List all applications across all namespaces (using empty labelSelector)
- appList, err := client.ListApplications("")
+ appList, err := client.ListApplications(v1.NamespaceAll)
  if err != nil {
  return nil, fmt.Errorf("error listing applications: %w", err)
  }
@@ -44,7 +45,7 @@ func (client *k8sClient) GetApplication(ctx context.Context, appName string) (*v
  }
 
  // Retrieve the application in the specified namespace
- return client.kubeClient.ApplicationsClientset.ArgoprojV1alpha1().Applications(app.Namespace).Get(ctx, app.Name, v1.GetOptions{})
+ return app, nil
 }
 
 // ListApplications lists all applications across all namespaces.
@@ -85,13 +86,7 @@ func (client *k8sClient) UpdateSpec(ctx context.Context, spec *application.Appli
  const baseDelay = 100 * time.Millisecond // Initial delay before retrying
 
  // Allow overriding max retries for testing purposes
- maxRetries := defaultMaxRetries
- if overrideRetries, ok := os.LookupEnv("OVERRIDE_MAX_RETRIES"); ok {
- var retries int
- if _, err := fmt.Sscanf(overrideRetries, "%d", &retries); err == nil {
- maxRetries = retries
- }
- }
+ maxRetries := env.ParseNumFromEnv("OVERRIDE_MAX_RETRIES", defaultMaxRetries, 0, 100)
 
  for attempts := 0; attempts < maxRetries; attempts++ {
  app, err := client.GetApplication(ctx, spec.GetName())

pkg/env/env.go

@@ -1,8 +1,12 @@
 package env
 
 import (
+ "math"
  "os"
+ "strconv"
  "strings"
+
+ "github.com/argoproj-labs/argocd-image-updater/pkg/log"
 )
 
 // Package env provides some utility functions to interact with the environment
@@ -30,3 +34,33 @@ func GetStringVal(envVar string, defaultValue string) string {
  return defaultValue
  }
 }
+
+// Helper function to parse a number from an environment variable. Returns a
+// default if env is not set, is not parseable to a number, exceeds max (if
+// max is greater than 0) or is less than min.
+//
+// nolint:unparam
+func ParseNumFromEnv(env string, defaultValue, min, max int) int {
+ str := os.Getenv(env)
+ if str == "" {
+ return defaultValue
+ }
+ num, err := strconv.ParseInt(str, 10, 0)
+ if err != nil {
+ log.Warnf("Could not parse '%s' as a number from environment %s", str, env)
+ return defaultValue
+ }
+ if num > math.MaxInt || num < math.MinInt {
+ log.Warnf("Value in %s is %d is outside of the min and max %d allowed values. Using default %d", env, num, min, defaultValue)
+ return defaultValue
+ }
+ if int(num) < min {
+ log.Warnf("Value in %s is %d, which is less than minimum %d allowed", env, num, min)
+ return defaultValue
+ }
+ if int(num) > max {
+ log.Warnf("Value in %s is %d, which is greater than maximum %d allowed", env, num, max)
+ return defaultValue
+ }
+ return int(num)
+}

pkg/env/env_test.go

@@ -36,3 +36,30 @@ func Test_GetStringVal(t *testing.T) {
  assert.Equal(t, "invalid", GetStringVal("TEST_STRING_VAL", "invalid"))
  })
 }
+
+func Test_ParseNumFromEnv(t *testing.T) {
+ t.Run("Get number from existing env var within range", func(t *testing.T) {
+ _ = os.Setenv("TEST_NUM_VAL", "5")
+ defer os.Setenv("TEST_NUM_VAL", "")
+ assert.Equal(t, 5, ParseNumFromEnv("TEST_NUM_VAL", 0, 1, 10))
+ })
+ t.Run("Get default value from non-existing env var", func(t *testing.T) {
+ _ = os.Setenv("TEST_NUM_VAL", "")
+ assert.Equal(t, 10, ParseNumFromEnv("TEST_NUM_VAL", 10, 1, 20))
+ })
+ t.Run("Get default value from env var with non-numeric value", func(t *testing.T) {
+ _ = os.Setenv("TEST_NUM_VAL", "abc")
+ defer os.Setenv("TEST_NUM_VAL", "")
+ assert.Equal(t, 10, ParseNumFromEnv("TEST_NUM_VAL", 10, 1, 20))
+ })
+ t.Run("Get default value from env var with value less than min", func(t *testing.T) {
+ _ = os.Setenv("TEST_NUM_VAL", "0")
+ defer os.Setenv("TEST_NUM_VAL", "")
+ assert.Equal(t, 10, ParseNumFromEnv("TEST_NUM_VAL", 10, 1, 20))
+ })
+ t.Run("Get default value from env var with value greater than max", func(t *testing.T) {
+ _ = os.Setenv("TEST_NUM_VAL", "30")
+ defer os.Setenv("TEST_NUM_VAL", "")
+ assert.Equal(t, 10, ParseNumFromEnv("TEST_NUM_VAL", 10, 1, 20))
+ })
+}