Merge pull request #84 from martinkunc/invalid-interval-not-reported2

Fix reporting duration error and add tests

Author: openshift-merge-robot

config/local.yaml

@@ -4,5 +4,5 @@ leaderElection:
   disable: true
 interval: "30s"
 storagePath: /tmp/insights-operator
-endpoint: http://localhost:8081
+endpoint: http://[::1]:8081
 #impersonate: system:serviceaccount:openshift-insights:gather

go.mod

@@ -17,7 +17,7 @@ require (
 	github.com/spf13/cobra v0.0.5
 	github.com/spf13/pflag v1.0.5
 	github.com/tmc/grpc-websocket-proxy v0.0.0-20190109142713-0ad062ec5ee5 // indirect
-	golang.org/x/net v0.0.0-20200114155413-6afb5195e5aa // indirect
+	golang.org/x/net v0.0.0-20200301022130-244492dfa37a // indirect
 	golang.org/x/time v0.0.0-20190308202827-9d24e82272b4
 	google.golang.org/appengine v1.6.1 // indirect
 	k8s.io/api v0.17.1

go.sum

@@ -468,6 +468,8 @@ golang.org/x/net v0.0.0-20191209160850-c0dbc17a3553 h1:efeOvDhwQ29Dj3SdAV/MJf8ou
 golang.org/x/net v0.0.0-20191209160850-c0dbc17a3553/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=
 golang.org/x/net v0.0.0-20200114155413-6afb5195e5aa h1:F+8P+gmewFQYRk6JoLQLwjBCTu3mcIURZfNkVweuRKA=
 golang.org/x/net v0.0.0-20200114155413-6afb5195e5aa/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=
+golang.org/x/net v0.0.0-20200301022130-244492dfa37a h1:GuSPYbZzB5/dcLNCwLQLsg3obCJtX9IJhpXkvY7kzk0=
+golang.org/x/net v0.0.0-20200301022130-244492dfa37a/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=
 golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be h1:vEDujvNQGv4jgYKudGeI/+DAX4Jffq6hpD55MmoEvKs=
 golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U=
 golang.org/x/oauth2 v0.0.0-20190226205417-e64efc72b421/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw=

pkg/config/configobserver/configobserver.go

@@ -130,7 +130,8 @@ func (c *Controller) retrieveConfig() error {
 		nextConfig.Report = len(nextConfig.Endpoint) > 0
 
 		if intervalString, ok := secret.Data["interval"]; ok {
-			duration, err := time.ParseDuration(string(intervalString))
+			var duration time.Duration
+			duration, err = time.ParseDuration(string(intervalString))
 			if err == nil && duration < time.Minute {
 				err = fmt.Errorf("too short")
 			}

pkg/config/configobserver/configobserver_test.go

@@ -0,0 +1,145 @@
+package configobserver
+
+import (
+	"encoding/json"
+	"fmt"
+	"io/ioutil"
+	"os"
+	"reflect"
+	"testing"
+	"time"
+
+	"github.com/openshift/insights-operator/pkg/config"
+	"github.com/openshift/insights-operator/pkg/utils"
+	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/client-go/kubernetes"
+	clsetfake "k8s.io/client-go/kubernetes/fake"
+	corefake "k8s.io/client-go/kubernetes/typed/core/v1/fake"
+	"k8s.io/klog"
+
+	clienttesting "k8s.io/client-go/testing"
+)
+
+func TestChangeSupportConfig(t *testing.T) {
+	var cases = []struct {
+		name      string
+		config    map[string]*corev1.Secret
+		expConfig *config.Controller
+		expErr    error
+	}{
+		{name: "interval too short",
+			config: map[string]*corev1.Secret{
+				pullSecretKey: &corev1.Secret{Data: map[string][]byte{
+					".dockerconfigjson": nil,
+				}},
+				supportKey: &corev1.Secret{Data: map[string][]byte{
+					"username": []byte("someone"),
+					"password": []byte("secret"),
+					"endpoint": []byte("http://po.rt"),
+					"interval": []byte("1s"),
+				}},
+			},
+			expErr: fmt.Errorf("insights secret interval must be a duration (1h, 10m) greater than or equal to one minute: too short"),
+		},
+		{name: "correct interval",
+			config: map[string]*corev1.Secret{
+				pullSecretKey: &corev1.Secret{Data: map[string][]byte{
+					".dockerconfigjson": nil,
+				}},
+				supportKey: &corev1.Secret{Data: map[string][]byte{
+					"interval": []byte("1m"),
+				}},
+			},
+			expConfig: &config.Controller{
+				Interval: 1 * time.Minute,
+			},
+			expErr: nil,
+		},
+	}
+
+	for _, tt := range cases {
+		t.Run(tt.name, func(t *testing.T) {
+
+			// log klog to test
+			klog.SetOutput(utils.NewTestLog(t).Writer())
+
+			ctrl := config.Controller{}
+			kube := kubeClientResponder{}
+			secs := tt.config
+			// setup mock responses for secretes by secret name
+			kube.CoreV1().(*corefake.FakeCoreV1).Fake.AddReactor("get", "secrets",
+				func(action clienttesting.Action) (handled bool, ret runtime.Object, err error) {
+					actionName := ""
+					if getAction, ok := action.(clienttesting.GetAction); ok {
+						actionName = getAction.GetName()
+					}
+
+					key := fmt.Sprintf("(%s) %s.%s", action.GetResource(), action.GetNamespace(), actionName)
+					sv, ok := secs[key]
+					if !ok {
+						return false, nil, nil
+					}
+					return true, sv, nil
+				})
+			// imitates New function
+			c := &Controller{
+				kubeClient:    &kube,
+				defaultConfig: ctrl,
+			}
+			c.mergeConfigLocked()
+			err := c.retrieveToken()
+			if err == nil {
+				err = c.retrieveConfig()
+			}
+			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)
+			}
+		})
+	}
+
+}
+
+const (
+	pullSecretKey = "(/v1, Resource=secrets) openshift-config.pull-secret"
+	supportKey    = "(/v1, Resource=secrets) openshift-config.support"
+	intervalKey   = "interval"
+)
+
+func mustMarshal(v interface{}) []byte {
+	bt, err := json.Marshal(v)
+	if err != nil {
+		panic(err)
+	}
+	return bt
+}
+
+func mustLoad(filename string) []byte {
+	f, err := os.Open(filename)
+	if err != nil {
+		panic(fmt.Errorf("test failed to load data: %v", err))
+	}
+	defer f.Close()
+	bts, err := ioutil.ReadAll(f)
+	if err != nil {
+		panic(fmt.Errorf("test failed to read data: %v", err))
+	}
+	return bts
+}
+
+type kubeClientResponder struct {
+	clsetfake.Clientset
+}
+
+var _ kubernetes.Interface = (*kubeClientResponder)(nil)

pkg/utils/testlog.go

@@ -0,0 +1,21 @@
+package utils
+
+import (
+	"log"
+	"testing"
+)
+
+func NewTestLog(t testing.TB) *log.Logger {
+	t.Helper()
+	return log.New(testWriter{TB: t}, t.Name()+" ", log.LstdFlags|log.Lshortfile|log.LUTC)
+}
+
+type testWriter struct {
+	testing.TB
+}
+
+func (tw testWriter) Write(p []byte) (int, error) {
+	tw.Helper()
+	tw.Logf("%s", p)
+	return len(p), nil
+}