Skip to content

Commit 3a9dc79

Browse files
authored
feat: replace non-expiring Dex SA token with short-lived TokenRequest-based token (argoproj-labs#2163)
* feat: replace non-expiring Dex SA token with short-lived TokenRequest-based token Signed-off-by: Rizwana777 <rizwananaaz177@gmail.com> * Address review comments Signed-off-by: Rizwana777 <rizwananaaz177@gmail.com> --------- Signed-off-by: Rizwana777 <rizwananaaz177@gmail.com>
1 parent 319bcd2 commit 3a9dc79

12 files changed

Lines changed: 700 additions & 125 deletions

File tree

common/defaults.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,14 @@ const (
118118
// ArgoCDDefaultDexServiceAccountName is the default Service Account name for the Dex server.
119119
ArgoCDDefaultDexServiceAccountName = "argocd-dex-server"
120120

121+
// ArgoCDDexServerTokenExpirySecs is the Dex SA token lifetime in seconds (1 hour).
122+
ArgoCDDexServerTokenExpirySecs = int64(3600)
123+
124+
// ArgoCDDexServerTokenRenewalThresholdPercent (1-99): renew the Dex token when less than this percent
125+
// of ArgoCDDexServerTokenExpirySecs remains (default 33 is approximately equivalent to the last third
126+
// of a 1h nominal lifetime).
127+
ArgoCDDexServerTokenRenewalThresholdPercent int64 = 33
128+
121129
// ArgoCDDefaultDexVersion is the Dex container image tag to use when not specified.
122130
ArgoCDDefaultDexVersion = "sha256:b08a58c9731c693b8db02154d7afda798e1888dc76db30d34c4a0d0b8a26d913" // v2.43.0
123131

controllers/argocd/argocd_controller.go

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,11 @@ type ReconcileArgoCD struct {
9797
LocalUsers *LocalUsersInfo
9898
// FipsConfigChecker checks if the deployment needs FIPS specific environment variables set.
9999
FipsConfigChecker argoutil.FipsConfigChecker
100+
101+
// dexTokenRequeueAfter stores the duration after which the reconciler should
102+
// re-run to renew the Dex OAuth client token before it expires.
103+
// Key: ArgoCD namespace, Value: time.Duration
104+
dexTokenRequeueAfter sync.Map
100105
}
101106

102107
var log = logr.Log.WithName("controller_argocd")
@@ -365,7 +370,20 @@ func (r *ReconcileArgoCD) internalReconcile(ctx context.Context, request ctrl.Re
365370
return reconcile.Result{}, argocd, argoCDStatus, err
366371
}
367372

368-
// Return and don't requeue
373+
// If Dex is in use, requeue before the token reaches its renewal threshold so
374+
// the operator proactively renews it without waiting for an external event.
375+
if UseDex(argocd) {
376+
if v, ok := r.dexTokenRequeueAfter.Load(argocd.Namespace); ok {
377+
if d, ok := v.(time.Duration); ok {
378+
return reconcile.Result{RequeueAfter: d}, argocd, argoCDStatus, nil
379+
}
380+
}
381+
} else {
382+
// Dex is disabled; remove any stale requeue entry so it does not fire
383+
// if Dex is later re-enabled and a fresh duration has not been stored yet.
384+
r.dexTokenRequeueAfter.Delete(argocd.Namespace)
385+
}
386+
369387
return reconcile.Result{}, argocd, argoCDStatus, nil
370388
}
371389

controllers/argocd/argocd_controller_test.go

Lines changed: 7 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ func TestReconcileArgoCD_DexWorkloads(t *testing.T) {
101101
runtimeObjs := []runtime.Object{}
102102
sch := makeTestReconcilerScheme(argoproj.AddToScheme, configv1.Install, routev1.Install)
103103
cl := makeTestReconcilerClient(sch, resObjs, subresObjs, runtimeObjs)
104-
r := makeTestReconciler(cl, sch, testclient.NewSimpleClientset())
104+
r := makeTestReconciler(cl, sch, makeTestK8sClientWithTokenReactor("mock-dex-token"))
105105

106106
assert.NoError(t, createNamespace(r, a.Namespace, ""))
107107

@@ -139,10 +139,6 @@ func TestReconcileArgoCD_DexWorkloads(t *testing.T) {
139139
assert.True(t, len(objectToVerify.GetOwnerReferences()) > 0)
140140
}
141141

142-
var secretList corev1.SecretList
143-
err = r.List(context.TODO(), &secretList, client.InNamespace(a.Namespace))
144-
assert.NoError(t, err)
145-
146142
configMap := &corev1.ConfigMap{
147143
ObjectMeta: metav1.ObjectMeta{
148144
Name: "argocd-cm",
@@ -154,16 +150,12 @@ func TestReconcileArgoCD_DexWorkloads(t *testing.T) {
154150

155151
assert.Equal(t, configMap.Data["dex.config"], a.Spec.SSO.Dex.Config)
156152

157-
var dexSecret *corev1.Secret
158-
159-
for idx := range secretList.Items {
160-
secret := secretList.Items[idx]
161-
if strings.HasPrefix(secret.Name, "argocd-dex-server-token-") {
162-
dexSecret = &secret
163-
break
164-
}
165-
}
166-
assert.NotNil(t, dexSecret)
153+
// Verify the expiring token Secret (Opaque type) was created with the expected fixed name.
154+
dexTokenSecret := &corev1.Secret{}
155+
expectedTokenSecretName := getDexServerTokenSecretName(a)
156+
err = r.Get(context.TODO(), types.NamespacedName{Name: expectedTokenSecretName, Namespace: a.Namespace}, dexTokenSecret)
157+
assert.NoError(t, err, "expected dex token secret %q to exist", expectedTokenSecretName)
158+
assert.Equal(t, corev1.SecretTypeOpaque, dexTokenSecret.Type)
167159

168160
err = r.Get(context.TODO(), client.ObjectKeyFromObject(a), a)
169161
assert.NoError(t, err)

controllers/argocd/dex.go

Lines changed: 159 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -2,20 +2,20 @@ package argocd
22

33
import (
44
"context"
5-
e "errors"
5+
"errors"
66
"fmt"
77
"reflect"
88
"strings"
9-
109
"time"
1110

1211
configv1 "github.com/openshift/api/config/v1"
1312

1413
"gopkg.in/yaml.v2"
14+
authv1 "k8s.io/api/authentication/v1"
15+
corev1 "k8s.io/api/core/v1"
16+
apierrors "k8s.io/apimachinery/pkg/api/errors"
1517
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1618
"k8s.io/apimachinery/pkg/types"
17-
18-
corev1 "k8s.io/api/core/v1"
1919
"k8s.io/apimachinery/pkg/util/intstr"
2020
"sigs.k8s.io/controller-runtime/pkg/client"
2121
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
@@ -42,73 +42,179 @@ func UseDex(cr *argoproj.ArgoCD) bool {
4242
return false
4343
}
4444

45-
// getDexOAuthClientSecret will return the OAuth client secret for the given ArgoCD.
45+
// getDexServerTokenSecretName returns the name of the Secret that stores the Dex OAuth client token.
46+
func getDexServerTokenSecretName(cr *argoproj.ArgoCD) string {
47+
return argoutil.GetSecretNameWithSuffix(cr, common.ArgoCDDefaultDexServiceAccountName+"-token")
48+
}
49+
50+
// dexServerTokenRenewalThreshold is how much nominal lifetime may remain before we treat the Dex token
51+
// as due for renewal (ExpirySecs * ArgoCDDexServerTokenRenewalThresholdPercent / 100).
52+
func dexServerTokenRenewalThreshold() time.Duration {
53+
return time.Duration(common.ArgoCDDexServerTokenExpirySecs*common.ArgoCDDexServerTokenRenewalThresholdPercent/100) * time.Second
54+
}
55+
56+
// needsDexTokenRenewal returns true when the token is missing, unparseable, or within the renewal window.
57+
func needsDexTokenRenewal(secret *corev1.Secret) bool {
58+
if secret == nil || secret.Data == nil {
59+
return true
60+
}
61+
tokenBytes, ok := secret.Data["token"]
62+
if !ok || len(tokenBytes) == 0 {
63+
return true
64+
}
65+
expiryBytes, ok := secret.Data["expiry"]
66+
if !ok {
67+
return true
68+
}
69+
expiry, err := time.Parse(time.RFC3339, string(expiryBytes))
70+
if err != nil {
71+
log.Error(err, "dex token secret has unparseable expiry, renewal needed",
72+
"secret", fmt.Sprintf("%s/%s", secret.Namespace, secret.Name),
73+
"expiry", string(expiryBytes))
74+
return true
75+
}
76+
return time.Until(expiry) < dexServerTokenRenewalThreshold()
77+
}
78+
79+
// getDexOAuthClientSecret returns a time-limited Dex OAuth client token via the TokenRequest API.
4680
func (r *ReconcileArgoCD) getDexOAuthClientSecret(cr *argoproj.ArgoCD) (*string, error) {
4781
sa := newServiceAccountWithName(common.ArgoCDDefaultDexServiceAccountName, cr)
4882
if err := argoutil.FetchObject(r.Client, cr.Namespace, sa.Name, sa); err != nil {
4983
return nil, err
5084
}
5185

52-
// Find the token secret
53-
var tokenSecret *corev1.ObjectReference
54-
for _, saSecret := range sa.Secrets {
55-
if strings.Contains(saSecret.Name, "token") {
56-
tokenSecret = &saSecret
57-
break
58-
}
59-
}
60-
61-
if tokenSecret == nil {
62-
// This change of creating secret for dex service account,is due to
63-
// change of reduction of secret-based service account tokens in k8s
64-
// v1.24 so from k8s v1.24 no default secret for service account is
65-
// created, but for dex to work we need to provide token of secret used
66-
// by dex service account as a oauth token, this change helps to achieve
67-
// it, in long run we should see do dex really requires a secret or it
68-
// manages to create one using TokenRequest API or may be change how dex
69-
// is used or configured by operator
70-
secret := &corev1.Secret{
71-
ObjectMeta: metav1.ObjectMeta{
72-
GenerateName: "argocd-dex-server-token-",
73-
Namespace: cr.Namespace,
74-
Annotations: map[string]string{
75-
corev1.ServiceAccountNameKey: sa.Name,
76-
},
77-
},
78-
Type: corev1.SecretTypeServiceAccountToken,
86+
tokenSecretName := getDexServerTokenSecretName(cr)
87+
tokenSecret := &corev1.Secret{}
88+
fetchErr := argoutil.FetchObject(r.Client, cr.Namespace, tokenSecretName, tokenSecret)
89+
if fetchErr != nil && !apierrors.IsNotFound(fetchErr) {
90+
return nil, fetchErr
91+
}
92+
secretExists := fetchErr == nil
93+
94+
// Return the cached token if it is still valid.
95+
if secretExists && !needsDexTokenRenewal(tokenSecret) {
96+
token := string(tokenSecret.Data["token"])
97+
// Schedule the next reconcile to run just before the renewal threshold so
98+
// the token is proactively renewed without waiting for an external event.
99+
expiry, parseErr := time.Parse(time.RFC3339, string(tokenSecret.Data["expiry"]))
100+
if parseErr != nil {
101+
log.Error(parseErr, "dex token secret expiry unparseable when scheduling requeue, returning cached token",
102+
"secret", fmt.Sprintf("%s/%s", tokenSecret.Namespace, tokenSecret.Name))
103+
} else {
104+
if d := time.Until(expiry) - dexServerTokenRenewalThreshold(); d > 0 {
105+
r.dexTokenRequeueAfter.Store(cr.Namespace, d)
106+
}
79107
}
80-
argoutil.AddTrackedByOperatorLabel(&secret.ObjectMeta)
81-
argoutil.LogResourceCreation(log, secret)
82-
err := r.Create(context.TODO(), secret)
83-
if err != nil {
84-
return nil, e.New("unable to locate and create ServiceAccount token for OAuth client secret")
85-
}
86-
err = controllerutil.SetControllerReference(cr, secret, r.Scheme)
87-
if err != nil {
108+
return &token, nil
109+
}
110+
111+
// Request a new time-limited token via the TokenRequest API.
112+
expirationSeconds := common.ArgoCDDexServerTokenExpirySecs
113+
tokenRequest, err := r.K8sClient.CoreV1().ServiceAccounts(cr.Namespace).CreateToken(
114+
context.TODO(),
115+
sa.Name,
116+
&authv1.TokenRequest{
117+
Spec: authv1.TokenRequestSpec{
118+
ExpirationSeconds: &expirationSeconds,
119+
},
120+
},
121+
metav1.CreateOptions{},
122+
)
123+
if err != nil {
124+
return nil, fmt.Errorf("failed to create token for dex service account %s: %w", sa.Name, err)
125+
}
126+
127+
expiryStr := tokenRequest.Status.ExpirationTimestamp.UTC().Format(time.RFC3339)
128+
tokenData := map[string][]byte{
129+
"token": []byte(tokenRequest.Status.Token),
130+
"expiry": []byte(expiryStr),
131+
}
132+
133+
if !secretExists {
134+
newSecret := argoutil.NewSecretWithSuffix(cr, common.ArgoCDDefaultDexServiceAccountName+"-token")
135+
newSecret.Type = corev1.SecretTypeOpaque
136+
newSecret.Data = tokenData
137+
argoutil.AddTrackedByOperatorLabel(&newSecret.ObjectMeta)
138+
if err := controllerutil.SetControllerReference(cr, newSecret, r.Scheme); err != nil {
88139
return nil, err
89140
}
90-
tokenSecret = &corev1.ObjectReference{
91-
Name: secret.Name,
92-
Namespace: cr.Namespace,
141+
argoutil.LogResourceCreation(log, newSecret)
142+
if err := r.Create(context.TODO(), newSecret); err != nil {
143+
return nil, err
93144
}
94-
sa.Secrets = append(sa.Secrets, *tokenSecret)
95-
argoutil.LogResourceUpdate(log, sa, "adding ServiceAccount token for OAuth client secret")
96-
err = r.Update(context.TODO(), sa)
97-
if err != nil {
98-
return nil, e.New("failed to add ServiceAccount token for OAuth client secret")
145+
} else {
146+
tokenSecret.Data = tokenData
147+
argoutil.LogResourceUpdate(log, tokenSecret, "renewing dex OAuth client token")
148+
if err := r.Update(context.TODO(), tokenSecret); err != nil {
149+
return nil, err
99150
}
100151
}
101152

102-
// Fetch the secret to obtain the token
103-
secret := argoutil.NewSecretWithName(cr, tokenSecret.Name)
104-
if err := argoutil.FetchObject(r.Client, cr.Namespace, secret.Name, secret); err != nil {
105-
return nil, err
153+
// Schedule the next reconcile just before the renewal threshold.
154+
if d := time.Until(tokenRequest.Status.ExpirationTimestamp.Time) - dexServerTokenRenewalThreshold(); d > 0 {
155+
r.dexTokenRequeueAfter.Store(cr.Namespace, d)
106156
}
107157

108-
token := string(secret.Data["token"])
158+
token := tokenRequest.Status.Token
109159
return &token, nil
110160
}
111161

162+
// reconcileDexLegacySATokenSecrets deletes non-expiring kubernetes.io/service-account-token
163+
// Secrets for the Dex SA and removes their stale references from the SA.
164+
func (r *ReconcileArgoCD) reconcileDexLegacySATokenSecrets(cr *argoproj.ArgoCD) error {
165+
dexSAName := newServiceAccountWithName(common.ArgoCDDefaultDexServiceAccountName, cr).Name
166+
secretList := &corev1.SecretList{}
167+
if err := r.List(context.TODO(), secretList,
168+
client.InNamespace(cr.Namespace),
169+
client.MatchingLabels(map[string]string{
170+
common.ArgoCDTrackedByOperatorLabel: common.ArgoCDAppName,
171+
}),
172+
); err != nil {
173+
return err
174+
}
175+
var deleteErrs []error
176+
for i := range secretList.Items {
177+
s := &secretList.Items[i]
178+
if s.Type != corev1.SecretTypeServiceAccountToken {
179+
continue
180+
}
181+
if s.Annotations[corev1.ServiceAccountNameKey] != dexSAName {
182+
continue
183+
}
184+
185+
if !strings.HasPrefix(s.Name, dexSAName+"-token-") {
186+
continue
187+
}
188+
argoutil.LogResourceDeletion(log, s, "removing legacy Dex service account token secret")
189+
if err := r.Delete(context.TODO(), s); err != nil && !apierrors.IsNotFound(err) {
190+
deleteErrs = append(deleteErrs, fmt.Errorf("delete legacy dex token secret %s/%s: %w", s.Namespace, s.Name, err))
191+
}
192+
}
193+
sa := newServiceAccountWithName(common.ArgoCDDefaultDexServiceAccountName, cr)
194+
if err := argoutil.FetchObject(r.Client, cr.Namespace, sa.Name, sa); err != nil {
195+
if apierrors.IsNotFound(err) {
196+
return errors.Join(deleteErrs...)
197+
}
198+
return errors.Join(append([]error{err}, deleteErrs...)...)
199+
}
200+
var filtered []corev1.ObjectReference
201+
for _, ref := range sa.Secrets {
202+
// Legacy auto token refs only (matches delete loop).
203+
if strings.HasPrefix(ref.Name, dexSAName+"-token-") {
204+
continue
205+
}
206+
filtered = append(filtered, ref)
207+
}
208+
if len(filtered) != len(sa.Secrets) {
209+
sa.Secrets = filtered
210+
argoutil.LogResourceUpdate(log, sa, "removing legacy token secret references from Dex service account")
211+
if err := r.Update(context.TODO(), sa); err != nil {
212+
return errors.Join(append([]error{err}, deleteErrs...)...)
213+
}
214+
}
215+
return errors.Join(deleteErrs...)
216+
}
217+
112218
// reconcileDexConfiguration will ensure that Dex is configured properly.
113219
func (r *ReconcileArgoCD) reconcileDexConfiguration(cm *corev1.ConfigMap, cr *argoproj.ArgoCD) error {
114220
actual := cm.Data[common.ArgoCDKeyDexConfig]

0 commit comments

Comments
 (0)