CCXDEV-15589: set limit for gathering interval

pkg/config/config.go

@@ -34,15 +34,15 @@ func (i *InsightsConfigurationSerialized) ToConfig() *InsightsConfiguration {
 		},
 	}
 	if i.DataReporting.Interval != "" {
-		ic.DataReporting.Interval = parseInterval(i.DataReporting.Interval, defaultGatherFrequency)
+		ic.DataReporting.Interval = parseInterval(i.DataReporting.Interval, defaultGatherFrequency, minimumGatherFrequency)
 	}
 
 	if i.SCA.Interval != "" {
-		ic.SCA.Interval = parseInterval(i.SCA.Interval, defaultSCAFfrequency)
+		ic.SCA.Interval = parseInterval(i.SCA.Interval, defaultSCAFfrequency, 0)
 	}
 
 	if i.ClusterTransfer.Interval != "" {
-		ic.ClusterTransfer.Interval = parseInterval(i.ClusterTransfer.Interval, defaultClusterTransferFrequency)
+		ic.ClusterTransfer.Interval = parseInterval(i.ClusterTransfer.Interval, defaultClusterTransferFrequency, 0)
 	}
 
 	if i.Alerting.Disabled != "" {
@@ -56,17 +56,27 @@ func (i *InsightsConfigurationSerialized) ToConfig() *InsightsConfiguration {
 	return ic
 }
 
-// parseInterval tries to parse the "interval" string as time duration and if there is an error
-// or negative time value then the provided default time duration is used
-func parseInterval(interval string, defaultValue time.Duration) time.Duration {
+// parseInterval parses the interval string as a time.Duration and validates it.
+// If parsing fails or the value is <= 0, it returns defaultValue.
+// If minIntervalValue > 0 and the parsed interval is less than the minimum, it returns minIntervalValue.
+// Pass 0 for minIntervalValue to skip minimum validation.
+func parseInterval(interval string, defaultValue, minIntervalValue time.Duration) time.Duration {
 	durationInt, err := time.ParseDuration(interval)
 	if err != nil {
 		klog.Errorf("Cannot parse interval time duration: %v. Using default value %s", err, defaultValue)
 		return defaultValue
 	}
+
 	if durationInt <= 0 {
-		durationInt = defaultValue
+		klog.Warningf("Interval %s is below or equal to zero. Using default value %s.", durationInt, defaultValue)
+		return defaultValue
 	}
+
+	if minIntervalValue > 0 && durationInt < minIntervalValue {
+		klog.Warningf("Interval %s is below minimum %s. Using minimum.", durationInt, minIntervalValue)
+		return minIntervalValue
+	}
+
 	return durationInt
 }
 

pkg/config/config_test.go

@@ -17,7 +17,7 @@ func TestToConfig(t *testing.T) {
 			name: "basic test",
 			serializedConfig: InsightsConfigurationSerialized{
 				DataReporting: DataReportingSerialized{
-					Interval:       "5m",
+					Interval:       "15m",
 					UploadEndpoint: "test.upload.endpoint/v1",
 					StoragePath:    "/tmp/test/path",
 					Obfuscation: Obfuscation{
@@ -39,7 +39,7 @@ func TestToConfig(t *testing.T) {
 			},
 			config: &InsightsConfiguration{
 				DataReporting: DataReporting{
-					Interval:       5 * time.Minute,
+					Interval:       15 * time.Minute,
 					UploadEndpoint: "test.upload.endpoint/v1",
 					StoragePath:    "/tmp/test/path",
 					Obfuscation: Obfuscation{
@@ -72,31 +72,42 @@ func TestParseInterval(t *testing.T) {
 		name             string
 		intervalString   string
 		defaultValue     time.Duration
+		minValue         time.Duration
 		expectedInterval time.Duration
 	}{
 		{
-			name:             "basic test with meaningful interval value",
+			name:             "basic test with meaningful interval value and minimum",
 			intervalString:   "1h",
 			defaultValue:     30 * time.Minute,
+			minValue:         10 * time.Minute,
 			expectedInterval: 1 * time.Hour,
 		},
 		{
 			name:             "interval cannot be parsed",
 			intervalString:   "not a duration",
 			defaultValue:     30 * time.Minute,
+			minValue:         0 * time.Minute,
 			expectedInterval: 30 * time.Minute,
 		},
 		{
 			name:             "interval is negative duration",
 			intervalString:   "-10m",
 			defaultValue:     30 * time.Minute,
+			minValue:         0 * time.Minute,
 			expectedInterval: 30 * time.Minute,
 		},
+		{
+			name:             "interval is less than minimum duration",
+			intervalString:   "20m",
+			defaultValue:     30 * time.Minute,
+			minValue:         60 * time.Minute,
+			expectedInterval: 60 * time.Minute,
+		},
 	}
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			interval := parseInterval(tt.intervalString, tt.defaultValue)
+			interval := parseInterval(tt.intervalString, tt.defaultValue, tt.minValue)
 			assert.Equal(t, tt.expectedInterval, interval)
 		})
 	}

pkg/config/configobserver/config.go

@@ -16,8 +16,8 @@ type Config struct {
 	config.Controller
 }
 
-// MinDuration defines the minimal report interval
-const MinDuration = 10 * time.Second
+// minDuration defines the minimal report interval
+const minDuration = 10 * time.Minute
 
 // LoadConfigFromSecret loads the controller config with given secret data
 func LoadConfigFromSecret(secret *v1.Secret) (config.Controller, error) {
@@ -35,17 +35,18 @@ func LoadConfigFromSecret(secret *v1.Secret) (config.Controller, error) {
 	if intervalString, ok := secret.Data["interval"]; ok {
 		var duration time.Duration
 		duration, err = time.ParseDuration(string(intervalString))
-		if err == nil && duration < MinDuration {
-			err = fmt.Errorf("too short")
-		}
-		if err == nil {
-			cfg.Interval = duration
-		} else {
-			err = fmt.Errorf(
-				"insights secret interval must be a duration (1h, 10m) greater than or equal to ten seconds: %v",
-				err)
+		if err != nil {
 			cfg.Report = false
+			return cfg.Controller, fmt.Errorf(
+				"insights secret interval must be a duration (1h, 10m) greater than or equal to ten minutes: %v",
+				err)
 		}
+
+		if duration < minDuration {
+			return cfg.Controller, fmt.Errorf("interval value too short, minimal value is 10 minutes")
+		}
+
+		cfg.Interval = duration
 	}
 
 	return cfg.Controller, err

pkg/config/configobserver/config_test.go

@@ -5,6 +5,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/stretchr/testify/assert"
 	v1 "k8s.io/api/core/v1"
 
 	"github.com/openshift/insights-operator/pkg/config"
@@ -155,10 +156,10 @@ func TestConfig_loadReport(t *testing.T) {
 
 func TestLoadConfigFromSecret(t *testing.T) {
 	tests := []struct {
-		name    string
-		secret  *v1.Secret
-		want    config.Controller
-		wantErr bool
+		name           string
+		secret         *v1.Secret
+		expectedConfig config.Controller
+		expectedErr    bool
 	}{
 		{
 			name: "Can load from secret",
@@ -170,7 +171,7 @@ func TestLoadConfigFromSecret(t *testing.T) {
 					"scaPullDisabled": []byte("false"),
 				},
 			},
-			want: config.Controller{
+			expectedConfig: config.Controller{
 				Report:             true,
 				Endpoint:           "http://endpoint",
 				ReportEndpoint:     "http://report",
@@ -182,19 +183,33 @@ func TestLoadConfigFromSecret(t *testing.T) {
 					SCADisabled: false,
 				},
 			},
-			wantErr: false,
+			expectedErr: false,
+		},
+		{
+			name: "Interval needs to be longer than 10m",
+			secret: &v1.Secret{
+				Data: map[string][]byte{
+					"endpoint":        []byte("http://endpoint"),
+					"noProxy":         []byte("no-proxy"),
+					"reportEndpoint":  []byte("http://report"),
+					"scaPullDisabled": []byte("false"),
+					"interval":        []byte("5m"),
+				},
+			},
+			expectedConfig: config.Controller{},
+			expectedErr:    true,
 		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			got, err := LoadConfigFromSecret(tt.secret)
-			if (err != nil) != tt.wantErr {
-				t.Errorf("LoadConfigFromSecret() error = %v, wantErr %v", err, tt.wantErr)
+			if tt.expectedErr {
+				assert.Error(t, err)
 				return
 			}
-			if !reflect.DeepEqual(got, tt.want) {
-				t.Errorf("LoadConfigFromSecret() got = %v, want %v", got, tt.want)
-			}
+
+			assert.Nil(t, err)
+			assert.Equal(t, tt.expectedConfig, got)
 		})
 	}
 }

pkg/config/configobserver/secretconfigobserver_test.go

@@ -3,10 +3,10 @@ package configobserver
 import (
 	"context"
 	"fmt"
-	"reflect"
 	"testing"
 	"time"
 
+	"github.com/stretchr/testify/assert"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/client-go/kubernetes"
@@ -31,7 +31,7 @@ const (
 
 // nolint: lll, funlen
 func Test_ConfigObserver_ChangeSupportConfig(t *testing.T) {
-	var cases = []struct {
+	cases := []struct {
 		name      string
 		config    map[string]*corev1.Secret
 		expConfig *config.Controller
@@ -47,7 +47,7 @@ func Test_ConfigObserver_ChangeSupportConfig(t *testing.T) {
 					"interval": []byte("1s"),
 				}},
 			},
-			expErr: fmt.Errorf("insights secret interval must be a duration (1h, 10m) greater than or equal to ten seconds: too short"),
+			expErr: fmt.Errorf("interval value too short, minimal value is 10 minutes"),
 		},
 		{
 			name: "interval incorrect format",
@@ -56,7 +56,7 @@ func Test_ConfigObserver_ChangeSupportConfig(t *testing.T) {
 					"interval": []byte("every second"),
 				}},
 			},
-			expErr: fmt.Errorf("insights secret interval must be a duration (1h, 10m) greater than or equal to ten seconds: time: invalid duration \"every second\""),
+			expErr: fmt.Errorf("insights secret interval must be a duration (1h, 10m) greater than or equal to ten minutes: time: invalid duration \"every second\""),
 		},
 		{
 			name: "reportPullingDelay incorrect format",
@@ -89,11 +89,11 @@ func Test_ConfigObserver_ChangeSupportConfig(t *testing.T) {
 			name: "correct interval",
 			config: map[string]*corev1.Secret{
 				supportKey: {Data: map[string][]byte{
-					"interval": []byte("1m"),
+					"interval": []byte("15m"),
 				}},
 			},
 			expConfig: &config.Controller{
-				Interval: 1 * time.Minute,
+				Interval: 15 * time.Minute,
 			},
 			expErr: nil,
 		},
@@ -154,24 +154,19 @@ func Test_ConfigObserver_ChangeSupportConfig(t *testing.T) {
 				defaultConfig: ctrl,
 			}
 			c.mergeConfig()
+
 			err := c.updateToken(context.Background())
 			if err == nil {
 				err = c.updateConfig(context.Background())
 			}
-			expErrS := ""
+
 			if tt.expErr != nil {
-				expErrS = tt.expErr.Error()
-			}
-			errS := ""
-			if err != nil {
-				errS = err.Error()
-			}
-			if expErrS != errS {
-				t.Fatalf("The test expected error doesn't match actual error.\nExpected: %s Actual: %s", tt.expErr, err)
-			}
-			if tt.expConfig != nil && !reflect.DeepEqual(tt.expConfig, c.config) {
-				t.Fatalf("The test expected config doesn't match actual config.\nExpected: %v Actual: %v", tt.expConfig, c.config)
+				assert.Error(t, err)
+				assert.Equal(t, tt.expErr, err)
+				return
 			}
+
+			assert.Equal(t, tt.expConfig, c.config)
 		})
 	}
 }

pkg/config/types.go

@@ -7,6 +7,8 @@ import (
 const (
 	// defines default frequency of the data gathering
 	defaultGatherFrequency = 2 * time.Hour
+	// defines minimum frequency of the data gathering
+	minimumGatherFrequency = 10 * time.Minute
 	// defines default frequency of the SCA download
 	defaultSCAFfrequency = 8 * time.Hour
 	// defines default frequency of the Cluster Transfer download