From 2f8657a02559f14dcb0efc0cb4a524c35fd3c35f Mon Sep 17 00:00:00 2001
From: streamer45 <cstcld91@gmail.com>
Date: Thu, 12 Oct 2023 07:41:33 -0600
Subject: [PATCH] Fix env override parsing for FailedJobsRetentionTime

---
 cmd/offloader/main.go  |  5 +++--
 service/config.go      | 21 ++++++++++++++++++---
 service/config_test.go | 31 +++++++++++++++++++++++++++++++
 3 files changed, 52 insertions(+), 5 deletions(-)

diff --git a/cmd/offloader/main.go b/cmd/offloader/main.go
index 2afd23a..59991dd 100644
--- a/cmd/offloader/main.go
+++ b/cmd/offloader/main.go
@@ -15,7 +15,6 @@ import (
 	"github.com/mattermost/calls-offloader/service"
 
 	"github.com/BurntSushi/toml"
-	"github.com/kelseyhightower/envconfig"
 )
 
 // loadConfig reads the config file and returns a new Config,
@@ -29,9 +28,11 @@ func loadConfig(path string) (service.Config, error) {
 	} else if _, err := toml.DecodeFile(path, &cfg); err != nil {
 		return cfg, fmt.Errorf("failed to decode config file: %w", err)
 	}
-	if err := envconfig.Process("", &cfg); err != nil {
+
+	if err := cfg.ParseFromEnv(); err != nil {
 		return cfg, err
 	}
+
 	return cfg, nil
 }
 
diff --git a/service/config.go b/service/config.go
index 053e606..5d8f491 100644
--- a/service/config.go
+++ b/service/config.go
@@ -5,6 +5,7 @@ package service
 
 import (
 	"fmt"
+	"os"
 	"regexp"
 	"strconv"
 	"time"
@@ -12,6 +13,8 @@ import (
 	"github.com/mattermost/calls-offloader/logger"
 	"github.com/mattermost/calls-offloader/service/api"
 	"github.com/mattermost/calls-offloader/service/auth"
+
+	"github.com/kelseyhightower/envconfig"
 )
 
 var (
@@ -71,14 +74,14 @@ func (c StoreConfig) IsValid() error {
 type JobAPIType string
 
 const (
-	JobAPITypeDocker     = "docker"
-	JobAPITypeKubernetes = "kubernetes"
+	JobAPITypeDocker     JobAPIType = "docker"
+	JobAPITypeKubernetes            = "kubernetes"
 )
 
 type JobsConfig struct {
 	APIType                 JobAPIType    `toml:"api_type"`
 	MaxConcurrentJobs       int           `toml:"max_concurrent_jobs"`
-	FailedJobsRetentionTime time.Duration `toml:"failed_jobs_retention_time"`
+	FailedJobsRetentionTime time.Duration `toml:"failed_jobs_retention_time" ignored:"true"`
 }
 
 // We need some custom parsing since duration doesn't support days.
@@ -170,6 +173,18 @@ type Config struct {
 	Logger logger.Config
 }
 
+func (c *Config) ParseFromEnv() error {
+	if val := os.Getenv("JOBS_FAILEDJOBSRETENTIONTIME"); val != "" {
+		d, err := parseRetentionTime(val)
+		if err != nil {
+			return fmt.Errorf("failed to parse FailedJobsRetentionTime: %w", err)
+		}
+		c.Jobs.FailedJobsRetentionTime = d
+	}
+
+	return envconfig.Process("", c)
+}
+
 func (c Config) IsValid() error {
 	if err := c.API.IsValid(); err != nil {
 		return err
diff --git a/service/config_test.go b/service/config_test.go
index defedc3..b507971 100644
--- a/service/config_test.go
+++ b/service/config_test.go
@@ -4,6 +4,7 @@
 package service
 
 import (
+	"os"
 	"testing"
 	"time"
 
@@ -67,3 +68,33 @@ func TestParseRetentionTime(t *testing.T) {
 		})
 	}
 }
+
+func TestParseFromEnv(t *testing.T) {
+	t.Run("no env", func(t *testing.T) {
+		var cfg Config
+		err := cfg.ParseFromEnv()
+		require.NoError(t, err)
+		require.Empty(t, cfg)
+	})
+
+	t.Run("FailedJobsRetentionTime", func(t *testing.T) {
+		os.Setenv("JOBS_FAILEDJOBSRETENTIONTIME", "1d")
+		defer os.Unsetenv("JOBS_FAILEDJOBSRETENTIONTIME")
+
+		var cfg Config
+		err := cfg.ParseFromEnv()
+		require.NoError(t, err)
+		require.Equal(t, time.Hour*24, cfg.Jobs.FailedJobsRetentionTime)
+	})
+
+	t.Run("override", func(t *testing.T) {
+		var cfg Config
+		cfg.Jobs.APIType = JobAPITypeKubernetes
+
+		os.Setenv("JOBS_APITYPE", "docker")
+		defer os.Unsetenv("JOBS_APITYPE")
+		err := cfg.ParseFromEnv()
+		require.NoError(t, err)
+		require.Equal(t, JobAPITypeDocker, cfg.Jobs.APIType)
+	})
+}