diff --git a/cmd/subctl/root.go b/cmd/subctl/root.go index 083742a83..8f1355203 100644 --- a/cmd/subctl/root.go +++ b/cmd/subctl/root.go @@ -43,7 +43,8 @@ import ( type suppressWarnings struct{} func (suppressWarnings) HandleWarningHeader(code int, agent, message string) { - if code == 299 && strings.Contains(message, "would violate PodSecurity") { + if code == 299 && (strings.Contains(message, "would violate PodSecurity") || + strings.Contains(message, "instead of auto-generated secret-based tokens")) { return } diff --git a/internal/rbac/rbac.go b/internal/rbac/rbac.go index f319f100c..53593f5ec 100644 --- a/internal/rbac/rbac.go +++ b/internal/rbac/rbac.go @@ -20,40 +20,34 @@ package rbac import ( "context" - "fmt" - "strings" "github.com/pkg/errors" + corev1 "k8s.io/api/core/v1" v1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/fields" + "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/client-go/kubernetes" ) -// maxGeneratedNameLength is the maximum generated length for a token, excluding the random suffix -// See k8s.io/apiserver/pkg/storage/names. -const maxGeneratedNameLength = 63 - 5 - func GetClientTokenSecret(ctx context.Context, kubeClient kubernetes.Interface, namespace, serviceAccountName string) (*v1.Secret, error) { - sa, err := kubeClient.CoreV1().ServiceAccounts(namespace).Get(ctx, serviceAccountName, metav1.GetOptions{}) + saSecrets, err := kubeClient.CoreV1().Secrets(namespace).List(ctx, metav1.ListOptions{ + FieldSelector: fields.OneTermEqualSelector("type", string(corev1.SecretTypeServiceAccountToken)).String(), + }) if err != nil { - return nil, errors.Wrapf(err, "ServiceAccount %s get failed", serviceAccountName) - } - - if len(sa.Secrets) < 1 { - return nil, fmt.Errorf("ServiceAccount %s does not have any secret", sa.Name) - } - - tokenPrefix := fmt.Sprintf("%s-token-", serviceAccountName) - if len(tokenPrefix) > maxGeneratedNameLength { - tokenPrefix = tokenPrefix[:maxGeneratedNameLength] + return nil, errors.Wrapf(err, "failed to list secrets of type %q in namespace %q", + corev1.SecretTypeServiceAccountToken, namespace) } - for _, secret := range sa.Secrets { - if strings.HasPrefix(secret.Name, tokenPrefix) { - //nolint:wrapcheck // No need to wrap here - return kubeClient.CoreV1().Secrets(namespace).Get(ctx, secret.Name, metav1.GetOptions{}) + for i := range saSecrets.Items { + if saSecrets.Items[i].Annotations[corev1.ServiceAccountNameKey] == serviceAccountName { + return &saSecrets.Items[i], nil } } - return nil, fmt.Errorf("ServiceAccount %s does not have a secret of type token", serviceAccountName) + return nil, apierrors.NewNotFound(schema.GroupResource{ + Group: corev1.SchemeGroupVersion.Group, + Resource: "secrets", + }, serviceAccountName) } diff --git a/pkg/broker/ensure.go b/pkg/broker/ensure.go index fc81884f8..6ccacbe58 100644 --- a/pkg/broker/ensure.go +++ b/pkg/broker/ensure.go @@ -20,11 +20,9 @@ package broker import ( "context" - "time" "github.com/pkg/errors" "github.com/submariner-io/subctl/internal/component" - "github.com/submariner-io/subctl/internal/constants" "github.com/submariner-io/subctl/internal/rbac" "github.com/submariner-io/subctl/pkg/gateway" "github.com/submariner-io/subctl/pkg/namespace" @@ -39,7 +37,6 @@ import ( rbacv1 "k8s.io/api/rbac/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/util/wait" "k8s.io/client-go/kubernetes" ) @@ -88,9 +85,7 @@ func Ensure(ctx context.Context, crdUpdater crd.Updater, kubeClient kubernetes.I return errors.Wrap(err, "error creating broker role") } - _, err = WaitForClientToken(ctx, kubeClient, constants.SubmarinerBrokerAdminSA, brokerNS) - - return err + return nil } // CreateSAForCluster creates a new SA for each new cluster joined and binds it to the submariner cluster role. @@ -107,8 +102,8 @@ func CreateSAForCluster(ctx context.Context, kubeClient kubernetes.Interface, cl return nil, errors.Wrap(err, "error binding sa to cluster role") } - clientToken, err := WaitForClientToken(ctx, kubeClient, saName, inNamespace) - if err != nil && !apierrors.IsAlreadyExists(err) { + clientToken, err := rbac.GetClientTokenSecret(ctx, kubeClient, inNamespace, saName) + if err != nil { return nil, errors.Wrap(err, "error getting cluster sa token") } @@ -137,37 +132,6 @@ func createBrokerAdministratorRoleAndSA(ctx context.Context, kubeClient kubernet return nil } -func WaitForClientToken(ctx context.Context, kubeClient kubernetes.Interface, submarinerBrokerSA, inNamespace string) (*v1.Secret, error) { - // wait for the client token to be ready, while implementing - // exponential backoff pattern, it will wait a total of: - // sum(n=0..9, 1.2^n * 5) seconds, = 130 seconds - backoff := wait.Backoff{ - Steps: 10, - Duration: 5 * time.Second, - Factor: 1.2, - Jitter: 1, - } - - var secret *v1.Secret - var lastErr error - - err := wait.ExponentialBackoff(backoff, func() (bool, error) { - secret, lastErr = rbac.GetClientTokenSecret(ctx, kubeClient, inNamespace, submarinerBrokerSA) - if lastErr != nil { - //nolint:nilerr // Intentional - the error is propagated via the outer-scoped var 'lastErr' - return false, nil - } - - return true, nil - }) - - if wait.Interrupted(err) { - return nil, lastErr //nolint:wrapcheck // No need to wrap here - } - - return secret, err //nolint:wrapcheck // No need to wrap here -} - //nolint:wrapcheck // No need to wrap here func CreateOrUpdateClusterBrokerRole(ctx context.Context, kubeClient kubernetes.Interface, inNamespace string) (bool, error) { return role.EnsureFromYAML(ctx, kubeClient, inNamespace, embeddedyamls.Config_broker_broker_client_role_yaml) @@ -196,7 +160,7 @@ func CreateOrUpdateBrokerAdminRoleBinding(ctx context.Context, kubeClient kubern //nolint:wrapcheck // No need to wrap here func CreateNewBrokerSA(ctx context.Context, kubeClient kubernetes.Interface, submarinerBrokerSA, inNamespace string) (err error) { sa := NewBrokerSA(submarinerBrokerSA) - _, err = serviceaccount.Ensure(ctx, kubeClient, inNamespace, sa, true) + _, err = serviceaccount.Ensure(ctx, kubeClient, inNamespace, sa) return err } diff --git a/pkg/serviceaccount/ensure.go b/pkg/serviceaccount/ensure.go index c5049ed92..8b681980e 100644 --- a/pkg/serviceaccount/ensure.go +++ b/pkg/serviceaccount/ensure.go @@ -21,18 +21,19 @@ package serviceaccount import ( "context" "fmt" - "strings" + "time" "github.com/pkg/errors" "github.com/submariner-io/admiral/pkg/resource" - resourceutil "github.com/submariner-io/subctl/pkg/resource" + "github.com/submariner-io/admiral/pkg/util" + "github.com/submariner-io/subctl/internal/rbac" "github.com/submariner-io/subctl/pkg/secret" "github.com/submariner-io/submariner-operator/pkg/embeddedyamls" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/fields" - "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/util/wait" "k8s.io/client-go/kubernetes" ) @@ -50,7 +51,7 @@ func ensureFromYAML(ctx context.Context, kubeClient kubernetes.Interface, namesp return nil, err //nolint:wrapcheck // No need to wrap errors here. } - err = ensure(ctx, kubeClient, namespace, sa, true) + err = ensure(ctx, kubeClient, namespace, sa) if err != nil { return nil, err } @@ -59,26 +60,20 @@ func ensureFromYAML(ctx context.Context, kubeClient kubernetes.Interface, namesp } //nolint:wrapcheck // No need to wrap errors here. -func ensure(ctx context.Context, kubeClient kubernetes.Interface, namespace string, sa *corev1.ServiceAccount, onlyCreate bool) error { - if onlyCreate { - _, err := kubeClient.CoreV1().ServiceAccounts(namespace).Get(ctx, sa.Name, metav1.GetOptions{}) - - if err == nil || !apierrors.IsNotFound(err) { - return err - } - } - - _, err := resourceutil.CreateOrUpdate(ctx, resource.ForServiceAccount(kubeClient, namespace), sa) +func ensure(ctx context.Context, kubeClient kubernetes.Interface, namespace string, sa *corev1.ServiceAccount) error { + _, err := util.CreateOrUpdate(ctx, resource.ForServiceAccount(kubeClient, namespace), sa, + func(existing runtime.Object) (runtime.Object, error) { + existing.(*corev1.ServiceAccount).Secrets = nil + return existing, nil + }) return err } //nolint:wrapcheck // No need to wrap errors here. -func Ensure(ctx context.Context, - kubeClient kubernetes.Interface, namespace string, sa *corev1.ServiceAccount, onlyCreate bool) ( - *corev1.ServiceAccount, error, -) { - err := ensure(ctx, kubeClient, namespace, sa, onlyCreate) +func Ensure(ctx context.Context, kubeClient kubernetes.Interface, namespace string, sa *corev1.ServiceAccount, +) (*corev1.ServiceAccount, error) { + err := ensure(ctx, kubeClient, namespace, sa) if err != nil { return nil, err } @@ -108,86 +103,56 @@ func EnsureFromYAML(ctx context.Context, kubeClient kubernetes.Interface, namesp } func EnsureSecretFromSA(ctx context.Context, client kubernetes.Interface, saName, namespace string) (*corev1.Secret, error) { - sa, err := client.CoreV1().ServiceAccounts(namespace).Get(ctx, saName, metav1.GetOptions{}) - if err != nil { - return nil, errors.Wrapf(err, "failed to get ServiceAccount %s/%s", namespace, saName) - } - - saSecret := getSecretFromSA(ctx, client, sa) - - if saSecret != nil { + saSecret, err := rbac.GetClientTokenSecret(ctx, client, namespace, saName) + if err == nil { return saSecret, nil } - // We couldn't find right secret from this SA, search all Secrets - saSecret, err = getSecretForSA(ctx, client, sa) - if err != nil && !apierrors.IsNotFound(err) { - return nil, err + if !apierrors.IsNotFound(err) { + return nil, err //nolint:wrapcheck // No need to wrap } - if err != nil { - newSecret := &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - GenerateName: fmt.Sprintf("%s-token-", sa.Name), - Namespace: namespace, - Annotations: map[string]string{ - corev1.ServiceAccountNameKey: saName, - createdByAnnotation: creatorName, - }, + newSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: fmt.Sprintf("%s-token-", saName), + Namespace: namespace, + Annotations: map[string]string{ + corev1.ServiceAccountNameKey: saName, + createdByAnnotation: creatorName, }, - Type: corev1.SecretTypeServiceAccountToken, - } - - saSecret, err = secret.Ensure(ctx, client, newSecret.Namespace, newSecret) - if err != nil { - return nil, errors.Wrapf(err, "failed to create secret for ServiceAccount %v", saName) - } - } - - secretRef := corev1.ObjectReference{ - Name: saSecret.Name, + }, + Type: corev1.SecretTypeServiceAccountToken, } - sa.Secrets = append(sa.Secrets, secretRef) - err = ensure(ctx, client, namespace, sa, false) - + saSecret, err = secret.Ensure(ctx, client, newSecret.Namespace, newSecret) if err != nil { - return nil, errors.Wrapf(err, "failed to update ServiceAccount %v with Secret reference %v", saName, secretRef.Name) + return nil, errors.Wrapf(err, "failed to create secret for service account %q", saName) } - return saSecret, nil -} + // Ensure the token has been generated for the secret. + backoff := wait.Backoff{ + Steps: 30, + Duration: 100 * time.Millisecond, + Factor: 1.3, + Jitter: 1, + } -func getSecretFromSA(ctx context.Context, client kubernetes.Interface, sa *corev1.ServiceAccount) *corev1.Secret { - secretNamePrefix := fmt.Sprintf("%s-token-", sa.Name) - for _, saSecretRef := range sa.Secrets { - if strings.HasPrefix(saSecretRef.Name, secretNamePrefix) { - saSecret, _ := client.CoreV1().Secrets(sa.Namespace).Get(ctx, saSecretRef.Name, metav1.GetOptions{}) - if saSecret.Annotations[corev1.ServiceAccountNameKey] == sa.Name && saSecret.Type == corev1.SecretTypeServiceAccountToken { - return saSecret - } + err = wait.ExponentialBackoff(backoff, func() (bool, error) { + saSecret, err = client.CoreV1().Secrets(namespace).Get(ctx, saSecret.Name, metav1.GetOptions{}) + if err != nil { + return false, errors.Wrapf(err, "error getting secret %q", saSecret.Name) } - } - return nil -} + if len(saSecret.Data["token"]) == 0 { + return false, nil + } -func getSecretForSA(ctx context.Context, client kubernetes.Interface, sa *corev1.ServiceAccount) (*corev1.Secret, error) { - saSecrets, err := client.CoreV1().Secrets(sa.Namespace).List(ctx, metav1.ListOptions{ - FieldSelector: fields.OneTermEqualSelector("type", "kubernetes.io/service-account-token").String(), + return true, nil }) - if err != nil { - return nil, errors.Wrapf(err, "failed to get secrets of type service-account-token in %v", sa.Namespace) - } - for i := 0; i < len(saSecrets.Items); i++ { - if saSecrets.Items[i].Annotations[corev1.ServiceAccountNameKey] == sa.Name { - return &saSecrets.Items[i], nil - } + if wait.Interrupted(err) { + return nil, fmt.Errorf("the token was not generated for secret %q", saSecret.Name) } - return nil, apierrors.NewNotFound(schema.GroupResource{ - Group: corev1.SchemeGroupVersion.Group, - Resource: "secrets", - }, sa.Name) + return saSecret, err //nolint:wrapcheck // No need to wrap here } diff --git a/pkg/serviceaccount/ensure_test.go b/pkg/serviceaccount/ensure_test.go index 34a8e03fd..5888139cc 100644 --- a/pkg/serviceaccount/ensure_test.go +++ b/pkg/serviceaccount/ensure_test.go @@ -71,7 +71,7 @@ var _ = Describe("EnsureFromYAML", func() { ObjectMeta: metav1.ObjectMeta{ Name: "test-sa", }, - }, false) + }) Expect(err).To(Succeed()) assertServiceAccount()