diff --git a/Makefile b/Makefile index d0a42de..4c71412 100644 --- a/Makefile +++ b/Makefile @@ -66,7 +66,7 @@ build: generate .PHONY: test test: generate - $(GOTEST) -v -cover ./... + $(GOTEST) -cover ./... .PHONY: run run: generate diff --git a/pkg/service/api_service_test.go b/pkg/service/api_service_test.go index bdae16f..d3a6faf 100644 --- a/pkg/service/api_service_test.go +++ b/pkg/service/api_service_test.go @@ -122,28 +122,76 @@ var ( }, }, } + // A secret that seems to have the correct annotation and data, but is not of type service account token + wrongSecret = &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "bootstrap-token", + Namespace: clusterA.Namespace, + Annotations: map[string]string{ + corev1.ServiceAccountNameKey: clusterA.Name, + }, + }, + Type: corev1.SecretTypeBootstrapToken, + Data: map[string][]byte{"token": []byte("notAtoken")}, + } + clusterBSecret = &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "anotherName", // We do not have guarantees that the secret name matches any fixed naming scheme + Namespace: clusterB.Namespace, + CreationTimestamp: metav1.NewTime(time.Now().Add(-1 * time.Hour)), + Annotations: map[string]string{ + corev1.ServiceAccountNameKey: clusterB.Name, + }, + }, + Type: corev1.SecretTypeServiceAccountToken, + Data: map[string][]byte{"token": []byte("someothertoken")}, + } + clusterASecret = &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: clusterA.Name, + Namespace: clusterA.Namespace, + CreationTimestamp: metav1.NewTime(time.Now().Add(-1 * time.Hour)), + Annotations: map[string]string{ + corev1.ServiceAccountNameKey: clusterA.Name, + }, + }, + Type: corev1.SecretTypeServiceAccountToken, + Data: map[string][]byte{"token": []byte("sometoken")}, + } + newClusterASecret = &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "new-secret", + Namespace: clusterA.Namespace, + CreationTimestamp: metav1.NewTime(time.Now()), + Annotations: map[string]string{ + corev1.ServiceAccountNameKey: clusterA.Name, + }, + }, + Type: corev1.SecretTypeServiceAccountToken, + Data: map[string][]byte{"token": []byte("newtoken")}, + } + clusterASA = &corev1.ServiceAccount{ ObjectMeta: metav1.ObjectMeta{ Name: clusterA.Name, Namespace: clusterA.Namespace, }, - Data: map[string][]byte{"token": []byte("sometoken")}, } testObjects = []client.Object{ tenantA, tenantB, clusterA, + wrongSecret, + clusterBSecret, + newClusterASecret, clusterASecret, clusterB, + clusterASA, &corev1.ServiceAccount{ ObjectMeta: metav1.ObjectMeta{ - Name: clusterA.Name, - Namespace: clusterA.Namespace, + Name: clusterB.Name, + Namespace: clusterB.Namespace, }, - Secrets: []corev1.ObjectReference{{ - Name: clusterASecret.Name, - Namespace: clusterASecret.Namespace, - }}, }, } ) @@ -167,8 +215,12 @@ func TestNewServer(t *testing.T) { } func setupTest(t *testing.T, _ ...[]runtime.Object) (*echo.Echo, client.Client) { + return rawSetupTest(t, testObjects...) +} + +func rawSetupTest(t *testing.T, obj ...client.Object) (*echo.Echo, client.Client) { - f := fake.NewClientBuilder().WithScheme(scheme).WithObjects(testObjects...).Build() + f := fake.NewClientBuilder().WithScheme(scheme).WithObjects(obj...).Build() testMiddleWare := KubernetesAuth{ CreateClientFunc: func(token string) (client.Client, error) { return f, nil diff --git a/pkg/service/steward.go b/pkg/service/steward.go index 6b5008b..8e28254 100644 --- a/pkg/service/steward.go +++ b/pkg/service/steward.go @@ -14,7 +14,6 @@ import ( "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" "github.com/projectsyn/lieutenant-api/pkg/api" @@ -96,25 +95,39 @@ func (s *APIImpl) InstallSteward(c echo.Context, params api.InstallStewardParams } func (s *APIImpl) getServiceAccountToken(ctx *APIContext, saName string) (string, error) { - serviceAccount := &corev1.ServiceAccount{} - if err := ctx.client.Get(ctx.Request().Context(), types.NamespacedName{Name: saName, Namespace: s.namespace}, serviceAccount); err != nil { - return "", err - } - if len(serviceAccount.Secrets) < 1 { - return "", echo.NewHTTPError(http.StatusInternalServerError, "No secret found for ServiceAccount: '%s'", saName) - } - secretName := serviceAccount.Secrets[0] - secret := &corev1.Secret{} - if err := ctx.client.Get(ctx.Request().Context(), types.NamespacedName{Name: secretName.Name, Namespace: serviceAccount.Namespace}, secret); err != nil { + secrets := &corev1.SecretList{} + if err := ctx.client.List( + ctx.Request().Context(), + secrets, + client.InNamespace(s.namespace), + client.MatchingFields{"type": string(corev1.SecretTypeServiceAccountToken)}, + ); err != nil { return "", err } - if len(secret.Data["token"]) < 1 { - return "", echo.NewHTTPError(http.StatusInternalServerError, "Secret doesn't contain a token: '%s'", secretName.Name) + token := findOldestSAToken(secrets.Items, saName) + if token == "" { + return "", echo.NewHTTPError(http.StatusServiceUnavailable, "Unable to find token for Cluster. This error might be transient, please try again.") } + return token, nil +} - return string(secret.Data["token"]), nil +func findOldestSAToken(secrets []corev1.Secret, saName string) string { + token := "" + var created *metav1.Time + + for i, secret := range secrets { + if secret.Type == corev1.SecretTypeServiceAccountToken && // Not strictly necessary but our testing framework can't handle field selectors + secret.Annotations[corev1.ServiceAccountNameKey] == saName && + len(secret.Data[corev1.ServiceAccountTokenKey]) > 0 && + !created.Before(&secret.CreationTimestamp) { + + token = string(secret.Data[corev1.ServiceAccountTokenKey]) + created = &secrets[i].CreationTimestamp + } + } + return token } func createRBAC() []runtime.RawExtension { diff --git a/pkg/service/steward_test.go b/pkg/service/steward_test.go index 16933a3..54d2410 100644 --- a/pkg/service/steward_test.go +++ b/pkg/service/steward_test.go @@ -3,51 +3,148 @@ package service import ( "net/http" "testing" + "time" "github.com/deepmap/oapi-codegen/pkg/testutil" "github.com/stretchr/testify/assert" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/serializer/json" + "sigs.k8s.io/controller-runtime/pkg/client" "github.com/projectsyn/lieutenant-api/pkg/api" ) func TestInstallSteward(t *testing.T) { - e, _ := setupTest(t) - result := testutil.NewRequest(). - WithHeader("X-Forwarded-Proto", "https"). - Get("/install/steward.json?token="+clusterA.Status.BootstrapToken.Token). - Go(t, e) - assert.Equal(t, http.StatusOK, result.Code()) - manifests := &corev1.List{} - err := result.UnmarshalJsonToObject(&manifests) - assert.NoError(t, err) - assert.Len(t, manifests.Items, 6) - decoder := json.NewSerializer(json.DefaultMetaFactory, scheme, scheme, true) - foundSecret := false - foundDeployment := false - for i, item := range manifests.Items { - obj, err := runtime.Decode(decoder, item.Raw) - assert.NoError(t, err) - if i == 0 { - _, ok := obj.(*corev1.Namespace) - assert.True(t, ok, "First object needs to be a namespace") - } - if secret, ok := obj.(*corev1.Secret); ok { - foundSecret = true - assert.Equal(t, secret.StringData["token"], string(clusterASecret.Data["token"])) - } - if deployment, ok := obj.(*appsv1.Deployment); ok { - foundDeployment = true - assert.Equal(t, "https://example.com", deployment.Spec.Template.Spec.Containers[0].Env[0].Value) - assert.Equal(t, clusterA.Name, deployment.Spec.Template.Spec.Containers[0].Env[1].Value) - } + tcs := map[string]struct { + bootstrapToken string + objs []client.Object + saToken string + clusterName string + }{ + "default": { + bootstrapToken: clusterA.Status.BootstrapToken.Token, + objs: testObjects, + saToken: "sometoken", + clusterName: clusterA.Name, + }, + "reordered": { + bootstrapToken: clusterA.Status.BootstrapToken.Token, + objs: []client.Object{ + newClusterASecret, + clusterA, + tenantA, + wrongSecret, + clusterASA, + clusterASecret, + }, + saToken: "sometoken", + clusterName: clusterA.Name, + }, + "older secret": { + bootstrapToken: clusterA.Status.BootstrapToken.Token, + objs: []client.Object{ + newClusterASecret, + tenantA, + clusterASecret, + &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "old-secret", + Namespace: clusterA.Namespace, + CreationTimestamp: metav1.NewTime(time.Now().Add(-24 * time.Hour)), + Annotations: map[string]string{ + corev1.ServiceAccountNameKey: clusterA.Name, + }, + }, + Type: corev1.SecretTypeServiceAccountToken, + Data: map[string][]byte{"token": []byte("someoldertoken")}, + }, + clusterA, + wrongSecret, + clusterASA, + }, + saToken: "someoldertoken", + clusterName: clusterA.Name, + }, + "even older secret": { + bootstrapToken: clusterA.Status.BootstrapToken.Token, + objs: []client.Object{ + tenantA, + &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "old-secret", + Namespace: clusterA.Namespace, + CreationTimestamp: metav1.NewTime(time.Now().Add(-24 * time.Hour)), + Annotations: map[string]string{ + corev1.ServiceAccountNameKey: clusterA.Name, + }, + }, + Type: corev1.SecretTypeServiceAccountToken, + Data: map[string][]byte{"token": []byte("someoldertoken")}, + }, + clusterA, + wrongSecret, + clusterASA, + &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "arcane-secret", + Namespace: clusterA.Namespace, + CreationTimestamp: metav1.NewTime(time.Unix(0, 0)), + Annotations: map[string]string{ + corev1.ServiceAccountNameKey: clusterA.Name, + }, + }, + Type: corev1.SecretTypeServiceAccountToken, + Data: map[string][]byte{"token": []byte("mysterytoken")}, + }, + newClusterASecret, + clusterASecret, + }, + saToken: "mysterytoken", + clusterName: clusterA.Name, + }, + } + + for n, tc := range tcs { + t.Run(n, func(t *testing.T) { + e, _ := rawSetupTest(t, tc.objs...) + + result := testutil.NewRequest(). + WithHeader("X-Forwarded-Proto", "https"). + Get("/install/steward.json?token="+tc.bootstrapToken). + Go(t, e) + assert.Equal(t, http.StatusOK, result.Code()) + manifests := &corev1.List{} + err := result.UnmarshalJsonToObject(&manifests) + assert.NoError(t, err) + assert.Len(t, manifests.Items, 6) + decoder := json.NewSerializer(json.DefaultMetaFactory, scheme, scheme, true) + foundSecret := false + foundDeployment := false + for i, item := range manifests.Items { + obj, err := runtime.Decode(decoder, item.Raw) + assert.NoError(t, err) + if i == 0 { + _, ok := obj.(*corev1.Namespace) + assert.True(t, ok, "First object needs to be a namespace") + } + if secret, ok := obj.(*corev1.Secret); ok { + foundSecret = true + assert.Equal(t, tc.saToken, secret.StringData["token"]) + } + if deployment, ok := obj.(*appsv1.Deployment); ok { + foundDeployment = true + assert.Equal(t, "https://example.com", deployment.Spec.Template.Spec.Containers[0].Env[0].Value) + assert.Equal(t, tc.clusterName, deployment.Spec.Template.Spec.Containers[0].Env[1].Value) + } + } + assert.True(t, foundSecret, "Could not find secret with steward token") + assert.True(t, foundDeployment, "Could not find deployment for steward") + }) } - assert.True(t, foundSecret, "Could not find secret with steward token") - assert.True(t, foundDeployment, "Could not find deployment for steward") } func TestInstallStewardNoToken(t *testing.T) {