Skip to content

Commit

Permalink
add unit test
Browse files Browse the repository at this point in the history
  • Loading branch information
bowenislandsong committed Nov 13, 2019
1 parent aec56a8 commit 4979bee
Show file tree
Hide file tree
Showing 3 changed files with 104 additions and 49 deletions.
45 changes: 0 additions & 45 deletions pkg/api/wrappers/deployment_install_client_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -409,51 +409,6 @@ func TestEnsureServiceAccount(t *testing.T) {
returnedError: testErr,
},
},
{
name: "Service Account updates dereference older secrets",
subname: "updated service account does not have secrets",
state: state{
namespace: "test-namespace",
existingServiceAccount: &corev1.ServiceAccount{
ObjectMeta: metav1.ObjectMeta{
Name: "test-service-account",
Labels: map[string]string{
"test": "no-secrets-on-updated-service-account",
},
OwnerReferences: []metav1.OwnerReference{
ownerReferenceFromCSV(&mockOwner),
},
},
},
getServiceAccountError: nil,
createServiceAccountError: nil,
},
input: input{
serviceAccountName: "test-service-account",
serviceAccount: &corev1.ServiceAccount{
ObjectMeta: metav1.ObjectMeta{
Name: "test-service-account",
},
Secrets: []corev1.ObjectReference{
{Name: "older secret"},
},
},
},
expect: expect{
returnedServiceAccount: &corev1.ServiceAccount{
ObjectMeta: metav1.ObjectMeta{
Name: "test-service-account",
Labels: map[string]string{
"test": "no-secrets-on-updated-service-account",
},
OwnerReferences: []metav1.OwnerReference{
ownerReferenceFromCSV(&mockOwner),
},
},
},
returnedError: nil,
},
},
}

for _, tt := range tests {
Expand Down
102 changes: 100 additions & 2 deletions pkg/controller/operators/catalog/operator_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
rbacv1 "k8s.io/api/rbac/v1"
"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1"
apiextensionsfake "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset/fake"
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"
Expand Down Expand Up @@ -341,6 +342,7 @@ func TestExecutePlan(t *testing.T) {
testName string
in *v1alpha1.InstallPlan
want []runtime.Object
lack []runtime.Object
err error
}{
{
Expand Down Expand Up @@ -382,6 +384,61 @@ func TestExecutePlan(t *testing.T) {
want: []runtime.Object{service("service", namespace), csv("csv", namespace, nil, nil)},
err: nil,
},
{
testName: "CreateServiceAccount",
in: withSteps(installPlan("p", namespace, v1alpha1.InstallPlanPhaseInstalling, "csv"),
[]*v1alpha1.Step{
{
Resource: v1alpha1.StepResource{
CatalogSource: "catalog",
CatalogSourceNamespace: namespace,
Group: "",
Version: "v1",
Kind: "ServiceAccount",
Name: "sa",
Manifest: toManifest(serviceAccount("sa", namespace, objectReference("old secret"))),
},
Status: v1alpha1.StepStatusUnknown,
},
},
),
want: []runtime.Object{serviceAccount("sa", namespace, objectReference("old secret")), secret("old secret", namespace)},
err: nil,
},
{
testName: "UpdateServiceAccount",
in: withSteps(installPlan("p", namespace, v1alpha1.InstallPlanPhaseInstalling, "csv"),
[]*v1alpha1.Step{
{
Resource: v1alpha1.StepResource{
CatalogSource: "catalog",
CatalogSourceNamespace: namespace,
Group: "",
Version: "v1",
Kind: "ServiceAccount",
Name: "sa",
Manifest: toManifest(serviceAccount("sa", namespace, objectReference("old secret"))),
},
Status: v1alpha1.StepStatusUnknown,
},
{
Resource: v1alpha1.StepResource{
CatalogSource: "catalog",
CatalogSourceNamespace: namespace,
Group: "",
Version: "v1",
Kind: "ServiceAccount",
Name: "sa",
Manifest: toManifest(serviceAccount("sa", namespace, nil)),
},
Status: v1alpha1.StepStatusUnknown,
},
},
),
want: []runtime.Object{serviceAccount("sa", namespace, nil)},
lack: []runtime.Object{secret("old secret", namespace)},
err: nil,
},
}

for _, tt := range tests {
Expand All @@ -392,6 +449,10 @@ func TestExecutePlan(t *testing.T) {
op, err := NewFakeOperator(ctx, namespace, []string{namespace}, withClientObjs(tt.in))
require.NoError(t, err)

// Creates an old secret. This is done by a Kubernetes controller; therefore, we manually do it here.
_, err = op.opClient.CreateSecret(secret("old secret", namespace))
require.NoError(t, err, "error creating old secret")

err = op.ExecutePlan(tt.in)
require.Equal(t, tt.err, err)

Expand All @@ -411,6 +472,8 @@ func TestExecutePlan(t *testing.T) {
fetched, err = op.opClient.GetRoleBinding(namespace, o.GetName())
case *corev1.ServiceAccount:
fetched, err = op.opClient.GetServiceAccount(namespace, o.GetName())
case *corev1.Secret:
fetched, err = op.opClient.GetSecret(namespace, o.GetName())
case *corev1.Service:
fetched, err = op.opClient.GetService(namespace, o.GetName())
case *v1alpha1.ClusterServiceVersion:
Expand All @@ -422,6 +485,18 @@ func TestExecutePlan(t *testing.T) {
require.NoError(t, err, "couldn't fetch %s %v", namespace, obj)
require.EqualValues(t, obj, fetched)
}

for _, obj := range tt.lack {
var err error
switch o := obj.(type) {
case *corev1.Secret:
_, err = op.opClient.GetSecret(namespace, o.GetName())
default:
require.Failf(t, "test is not supporting object", "%#v", obj)
}
require.True(t, k8serrors.IsNotFound(err), "found the object that is expected to be deleted %s %v",
namespace, obj)
}
})
}
}
Expand Down Expand Up @@ -512,7 +587,7 @@ func TestSyncCatalogSources(t *testing.T) {
Namespace: "cool-namespace",
UID: types.UID("configmap-uid"),
ResourceVersion: "resource-version",
LastUpdateTime: now,
LastUpdateTime: now,
},
RegistryServiceStatus: nil,
},
Expand Down Expand Up @@ -917,7 +992,7 @@ func NewFakeOperator(ctx context.Context, namespace string, watchedNamespaces []
reconciler: config.reconciler,
clientAttenuator: scoped.NewClientAttenuator(logger, &rest.Config{}, opClientFake, clientFake),
serviceAccountQuerier: scoped.NewUserDefinedServiceAccountQuerier(logger, clientFake),
catsrcQueueSet: queueinformer.NewEmptyResourceQueueSet(),
catsrcQueueSet: queueinformer.NewEmptyResourceQueueSet(),
}
op.sources = grpc.NewSourceStore(config.logger, 1*time.Second, 5*time.Second, op.syncSourceState)
if op.reconciler == nil {
Expand Down Expand Up @@ -1010,6 +1085,29 @@ func service(name, namespace string) *corev1.Service {
}
}

func serviceAccount(name, namespace string, secretRef *corev1.ObjectReference) *corev1.ServiceAccount {
if secretRef == nil {
return &corev1.ServiceAccount{
ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: namespace},
}
}
return &corev1.ServiceAccount{
ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: namespace},
Secrets: []corev1.ObjectReference{*secretRef},
}
}

func objectReference(name string) *corev1.ObjectReference {
if name == "" {
return &corev1.ObjectReference{}
}
return &corev1.ObjectReference{Name: name}
}

func secret(secretName, namespace string) *corev1.Secret {
return &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: secretName, Namespace: namespace}}
}

func toManifest(obj runtime.Object) string {
raw, _ := json.Marshal(obj)
return string(raw)
Expand Down
6 changes: 4 additions & 2 deletions pkg/controller/operators/catalog/step_ensurer.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -117,11 +117,13 @@ func (o *StepEnsurer) EnsureServiceAccount(namespace string, sa *corev1.ServiceA
}

// Before UpdateServiceAccount and dereference secrets of the older SA,
// we capture those secrets and removed them after update is successful. Without this,
// older secrets will lay around till the operator is uninstalled.
// we capture those secrets and removed them right after update is successful. Without this,
// the older secrets will lay around till the operator is uninstalled.
// We clean up instead of attaching those older secrets to ensure that new secrets for the SA are valid.
preSa, getErr := o.kubeClient.KubernetesInterface().CoreV1().ServiceAccounts(namespace).Get(sa.Name,
metav1.GetOptions{})
// Returns error if no secret is found for a service account. Because according to Kubernetes,
// The controller loop ensures a secret with an API token exists for each service account.
if getErr != nil {
err = errorwrap.Wrapf(getErr, "error getting older secrets for service account: %s", sa.GetName())
return
Expand Down

0 comments on commit 4979bee

Please sign in to comment.