Enable fast unit test feedback, add Token controller tests (#188)

controllers/binding_controller_test.go

@@ -29,6 +29,10 @@ func mustLoadObject(t *testing.T, file string, obj runtime.Object, meta *metav1.
 }
 
 func TestBinding(t *testing.T) {
+	if testing.Short() {
+		t.SkipNow()
+	}
+
 	const (
 		servicefile = "testdata/translator-2.yaml"
 		bindingfile = "testdata/translator-binding.yaml"

controllers/service_controller_test.go

@@ -20,6 +20,10 @@ import (
 )
 
 func TestService(t *testing.T) {
+	if testing.Short() {
+		t.SkipNow()
+	}
+
 	ready := t.Run("should be ready", func(t *testing.T) {
 		for _, specfile := range []string{
 			"translator.yaml",
@@ -169,6 +173,10 @@ func getServiceInstanceFromObj(logt logr.Logger, service *ibmcloudv1beta1.Servic
 }
 
 func TestServiceV1Alpha1Compat(t *testing.T) {
+	if testing.Short() {
+		t.SkipNow()
+	}
+
 	service := new(ibmcloudv1beta1.Service)
 	mustLoadObject(t, filepath.Join("testdata", "translator-v1alpha1.yaml"), service, &service.ObjectMeta)
 	ctx := context.TODO()

controllers/suite_config_test.go

@@ -2,6 +2,7 @@ package controllers
 
 import (
 	"context"
+	"testing"
 	"time"
 
 	"github.com/IBM-Cloud/bluemix-go"
@@ -11,14 +12,22 @@ import (
 	"github.com/IBM-Cloud/bluemix-go/models"
 	"github.com/IBM-Cloud/bluemix-go/rest"
 	"github.com/IBM-Cloud/bluemix-go/session"
+	"github.com/go-logr/logr"
+	"github.com/go-logr/zapr"
+	ibmcloudv1beta1 "github.com/ibm/cloud-operators/api/v1beta1"
 	"github.com/ibm/cloud-operators/internal/config"
 	"github.com/pkg/errors"
+	"github.com/stretchr/testify/require"
+	"go.uber.org/zap"
+	"go.uber.org/zap/zaptest"
+	corev1 "k8s.io/api/core/v1"
 	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
 )
 
 var (
-	testCfg = config.GetIBMCloud()
+	testCfg config.IBMCloud
 )
 
 const (
@@ -27,6 +36,7 @@ const (
 )
 
 func setup() error {
+	testCfg = config.MustGetIBMCloud()
 	if err := setupAuth(); err != nil {
 		return err
 	}
@@ -146,3 +156,18 @@ func getAuthTokens(sess *session.Session) (uaaAccessToken, uaaRefreshToken strin
 	}
 	return config.UAAAccessToken, config.UAARefreshToken, nil
 }
+
+func schemas(t *testing.T) *runtime.Scheme {
+	scheme, err := ibmcloudv1beta1.SchemeBuilder.Build()
+	require.NoError(t, err)
+	require.NoError(t, corev1.SchemeBuilder.AddToScheme(scheme))
+	return scheme
+}
+
+func testLogger(t *testing.T) logr.Logger {
+	opts := []zap.Option{
+		zap.AddCaller(),
+		zap.AddCallerSkip(1),
+	}
+	return zapr.NewLogger(zaptest.NewLogger(t, zaptest.WrapOptions(opts...)))
+}

controllers/suite_test.go

@@ -18,6 +18,7 @@ package controllers
 
 import (
 	"context"
+	"flag"
 	"fmt"
 	"net/http"
 	"os"
@@ -66,6 +67,11 @@ func TestMain(m *testing.M) {
 }
 
 func run(m *testing.M) int {
+	flag.Parse() // required to check for '-short' flag setting
+	if testing.Short() {
+		return m.Run()
+	}
+
 	ctx, cancel := context.WithCancel(context.Background())
 	defer func() {
 		cancel()

controllers/token_controller.go

@@ -101,6 +101,7 @@ func (r *TokenReconciler) Reconcile(request ctrl.Request) (ctrl.Result, error) {
 		Data: creds.MarshalSecret(),
 	}
 
+	// TODO(johnstarich) switch to CreateOrUpdate for atomic replace behavior
 	err = r.Delete(ctx, tokens)
 	if err != nil && !errors.IsNotFound(err) {
 		return ctrl.Result{}, err

controllers/token_controller_test.go

@@ -2,14 +2,21 @@ package controllers
 
 import (
 	"context"
+	"fmt"
 	"testing"
+	"time"
 
 	"github.com/ibm/cloud-operators/internal/ibmcloud/auth"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	corev1 "k8s.io/api/core/v1"
+	k8sErrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/types"
+	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/client/fake"
 )
 
 var (
@@ -18,6 +25,10 @@ var (
 )
 
 func TestToken(t *testing.T) {
+	if testing.Short() {
+		t.SkipNow()
+	}
+
 	// Create the secret object and expect the Reconcile
 	const (
 		secretName   = "dummyapikey"
@@ -66,3 +77,239 @@ func TestToken(t *testing.T) {
 	assert.Contains(t, secret.Data, "uaa_token")
 	assert.Contains(t, secret.Data, "uaa_refresh_token")
 }
+
+func TestTokenFailedAuth(t *testing.T) {
+	scheme := schemas(t)
+	objects := []runtime.Object{
+		&corev1.Secret{
+			ObjectMeta: metav1.ObjectMeta{Name: "secret"},
+			Data: map[string][]byte{
+				"api-key": []byte(`bogus key`),
+			},
+		},
+	}
+	r := &TokenReconciler{
+		Client: fake.NewFakeClientWithScheme(scheme, objects...),
+		Log:    testLogger(t),
+		Scheme: scheme,
+		Authenticate: func(apiKey, region string) (auth.Credentials, error) {
+			return auth.Credentials{}, fmt.Errorf("failure")
+		},
+	}
+
+	result, err := r.Reconcile(ctrl.Request{
+		NamespacedName: types.NamespacedName{Name: "secret"},
+	})
+	assert.EqualError(t, err, "failure")
+	assert.Equal(t, ctrl.Result{}, result)
+}
+
+func TestTokenFailedSecretLookup(t *testing.T) {
+	scheme := schemas(t)
+	r := &TokenReconciler{
+		Client:       fake.NewFakeClientWithScheme(scheme),
+		Log:          testLogger(t),
+		Scheme:       scheme,
+		Authenticate: nil, // should not be called
+	}
+
+	t.Run("not found", func(t *testing.T) {
+		result, err := r.Reconcile(ctrl.Request{
+			NamespacedName: types.NamespacedName{Name: "secret"},
+		})
+		assert.NoError(t, err, "Don't retry (return err) if secret no longer exists")
+		assert.Equal(t, ctrl.Result{}, result)
+	})
+
+	r.Client = fake.NewFakeClientWithScheme(runtime.NewScheme()) // fail to read the type Secret
+	t.Run("failed to read secret", func(t *testing.T) {
+		result, err := r.Reconcile(ctrl.Request{
+			NamespacedName: types.NamespacedName{Name: "secret"},
+		})
+		assert.Error(t, err)
+		assert.False(t, k8sErrors.IsNotFound(err))
+		assert.Equal(t, ctrl.Result{}, result)
+	})
+}
+
+func TestTokenSecretIsDeleting(t *testing.T) {
+	scheme := schemas(t)
+	now := metav1.Now()
+	objects := []runtime.Object{
+		&corev1.Secret{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:              "secret",
+				DeletionTimestamp: &now,
+			},
+		},
+	}
+	r := &TokenReconciler{
+		Client:       fake.NewFakeClientWithScheme(scheme, objects...),
+		Log:          testLogger(t),
+		Scheme:       scheme,
+		Authenticate: nil, // should not be called
+	}
+
+	result, err := r.Reconcile(ctrl.Request{
+		NamespacedName: types.NamespacedName{Name: "secret"},
+	})
+	assert.NoError(t, err, "Don't retry (return err) if secret is deleting")
+	assert.Equal(t, ctrl.Result{}, result)
+}
+
+func TestTokenAPIKeyIsMissing(t *testing.T) {
+	scheme := schemas(t)
+	objects := []runtime.Object{
+		&corev1.Secret{
+			ObjectMeta: metav1.ObjectMeta{Name: "secret"},
+			Data:       nil, // no API key
+		},
+	}
+	r := &TokenReconciler{
+		Client:       fake.NewFakeClientWithScheme(scheme, objects...),
+		Log:          testLogger(t),
+		Scheme:       scheme,
+		Authenticate: nil, // should not be called
+	}
+
+	result, err := r.Reconcile(ctrl.Request{
+		NamespacedName: types.NamespacedName{Name: "secret"},
+	})
+	assert.NoError(t, err, "Don't retry (return err) if secret does not contain an api-key entry")
+	assert.Equal(t, ctrl.Result{}, result)
+}
+
+func TestTokenAuthInvalidConfig(t *testing.T) {
+	scheme := schemas(t)
+	const (
+		apiKey = "some API key"
+		region = "some invalid region"
+	)
+	objects := []runtime.Object{
+		&corev1.Secret{
+			ObjectMeta: metav1.ObjectMeta{Name: "secret"},
+			Data: map[string][]byte{
+				"api-key": []byte(apiKey),
+				"region":  []byte(region),
+			},
+		},
+	}
+	r := &TokenReconciler{
+		Client: fake.NewFakeClientWithScheme(scheme, objects...),
+		Log:    testLogger(t),
+		Scheme: scheme,
+		Authenticate: func(actualAPIKey, actualRegion string) (auth.Credentials, error) {
+			assert.Equal(t, apiKey, actualAPIKey)
+			assert.Equal(t, region, actualRegion)
+			return auth.Credentials{}, auth.InvalidConfigError{Err: fmt.Errorf("Invalid region")}
+		},
+	}
+
+	result, err := r.Reconcile(ctrl.Request{
+		NamespacedName: types.NamespacedName{Name: "secret"},
+	})
+	assert.NoError(t, err, "Don't retry (return err) if secret region is invalid")
+	assert.Equal(t, ctrl.Result{}, result)
+}
+
+func TestTokenDeleteFailed(t *testing.T) {
+	scheme := schemas(t)
+	const (
+		apiKey      = "some API key"
+		region      = "some invalid region"
+		accessToken = "some access token"
+	)
+	objects := []runtime.Object{
+		&corev1.Secret{
+			ObjectMeta: metav1.ObjectMeta{Name: "secret"},
+			Data: map[string][]byte{
+				"api-key": []byte(apiKey),
+				"region":  []byte(region),
+			},
+		},
+	}
+	var r *TokenReconciler
+	r = &TokenReconciler{
+		Client: fake.NewFakeClientWithScheme(scheme, objects...),
+		Log:    testLogger(t),
+		Scheme: scheme,
+		Authenticate: func(actualAPIKey, actualRegion string) (auth.Credentials, error) {
+			assert.Equal(t, apiKey, actualAPIKey)
+			assert.Equal(t, region, actualRegion)
+			r.Client = fake.NewFakeClientWithScheme(runtime.NewScheme()) // trigger later failure of r.Client.Delete
+			return auth.Credentials{
+				IAMAccessToken: accessToken,
+			}, nil
+		},
+	}
+
+	result, err := r.Reconcile(ctrl.Request{
+		NamespacedName: types.NamespacedName{Name: "secret"},
+	})
+	assert.Error(t, err)
+	assert.False(t, k8sErrors.IsNotFound(err))
+	assert.Equal(t, ctrl.Result{}, result)
+}
+
+func TestTokenRaceCreateFailed(t *testing.T) {
+	scheme := schemas(t)
+	const (
+		apiKey      = "some API key"
+		region      = "some invalid region"
+		accessToken = "some access token"
+	)
+	tokensSecret := &corev1.Secret{
+		ObjectMeta: metav1.ObjectMeta{Name: "secret-tokens"},
+		Data: map[string][]byte{
+			"access_token": []byte("old " + accessToken),
+		},
+	}
+	objects := []runtime.Object{
+		&corev1.Secret{
+			ObjectMeta: metav1.ObjectMeta{Name: "secret"},
+			Data: map[string][]byte{
+				"api-key": []byte(apiKey),
+				"region":  []byte(region),
+			},
+		},
+		tokensSecret,
+	}
+	r := &TokenReconciler{
+		Client: fake.NewFakeClientWithScheme(scheme, objects...),
+		Log:    testLogger(t),
+		Scheme: scheme,
+		Authenticate: func(actualAPIKey, actualRegion string) (auth.Credentials, error) {
+			assert.Equal(t, apiKey, actualAPIKey)
+			assert.Equal(t, region, actualRegion)
+			return auth.Credentials{
+				IAMAccessToken: accessToken,
+			}, nil
+		},
+	}
+
+	ctx, cancel := context.WithCancel(context.Background())
+	go func() {
+		for {
+			select {
+			case <-ctx.Done():
+				return
+			default:
+				// re-create the secret constantly during the test to trigger race condition
+				_ = r.Client.Create(context.Background(), tokensSecret)
+			}
+		}
+	}()
+	defer cancel()
+
+	var result ctrl.Result
+	var err error
+	require.Eventually(t, func() bool {
+		result, err = r.Reconcile(ctrl.Request{
+			NamespacedName: types.NamespacedName{Name: "secret"},
+		})
+		return err != nil
+	}, 5*time.Second, 10*time.Millisecond)
+	assert.Error(t, err)
+	assert.True(t, k8sErrors.IsAlreadyExists(err))
+	assert.Equal(t, ctrl.Result{}, result)
+}