Skip to content

Introduce token cache and use it for GitHub App tokens #1745

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 12, 2025
Merged

Introduce token cache and use it for GitHub App tokens #1745

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 8 additions & 7 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ require (
cloud.google.com/go/storage v1.48.0
github.com/AdaLogics/go-fuzz-headers v0.0.0-20240806141605-e8a1dd7889d6
github.com/Azure/azure-sdk-for-go/sdk/azcore v1.17.0
github.com/Azure/azure-sdk-for-go/sdk/azidentity v1.8.1
github.com/Azure/azure-sdk-for-go/sdk/azidentity v1.8.2
github.com/Azure/azure-sdk-for-go/sdk/storage/azblob v1.5.0
github.com/Masterminds/semver/v3 v3.3.1
github.com/cyphar/filepath-securejoin v0.4.1
Expand All @@ -24,7 +24,8 @@ require (
github.com/fluxcd/cli-utils v0.36.0-flux.12
github.com/fluxcd/pkg/apis/event v0.16.0
github.com/fluxcd/pkg/apis/meta v1.10.0
github.com/fluxcd/pkg/auth v0.3.0
github.com/fluxcd/pkg/auth v0.8.0
github.com/fluxcd/pkg/cache v0.7.0
github.com/fluxcd/pkg/git v0.24.0
github.com/fluxcd/pkg/git/gogit v0.24.0
github.com/fluxcd/pkg/gittestserver v0.16.0
Expand Down Expand Up @@ -61,7 +62,7 @@ require (
github.com/sigstore/sigstore v1.8.15
github.com/sirupsen/logrus v1.9.3
github.com/spf13/pflag v1.0.6
golang.org/x/crypto v0.33.0
golang.org/x/crypto v0.34.0
golang.org/x/oauth2 v0.26.0
golang.org/x/sync v0.11.0
google.golang.org/api v0.211.0
Expand Down Expand Up @@ -98,7 +99,7 @@ require (
github.com/Azure/go-autorest/logger v0.2.1 // indirect
github.com/Azure/go-autorest/tracing v0.6.0 // indirect
github.com/Azure/go-ntlmssp v0.0.0-20221128193559-754e69321358 // indirect
github.com/AzureAD/microsoft-authentication-library-for-go v1.3.3 // indirect
github.com/AzureAD/microsoft-authentication-library-for-go v1.4.0 // indirect
github.com/BurntSushi/toml v1.4.0 // indirect
github.com/GoogleCloudPlatform/opentelemetry-operations-go/detectors/gcp v1.24.1 // indirect
github.com/GoogleCloudPlatform/opentelemetry-operations-go/exporter/metric v0.48.1 // indirect
Expand Down Expand Up @@ -142,7 +143,7 @@ require (
github.com/beorn7/perks v1.0.1 // indirect
github.com/blang/semver v3.5.1+incompatible // indirect
github.com/blang/semver/v4 v4.0.0 // indirect
github.com/bradleyfalzon/ghinstallation/v2 v2.13.0 // indirect
github.com/bradleyfalzon/ghinstallation/v2 v2.14.0 // indirect
github.com/bshuster-repo/logrus-logstash-hook v1.0.0 // indirect
github.com/buildkite/agent/v3 v3.81.0 // indirect
github.com/buildkite/go-pipeline v0.13.1 // indirect
Expand Down Expand Up @@ -227,10 +228,10 @@ require (
github.com/google/btree v1.1.3 // indirect
github.com/google/certificate-transparency-go v1.2.1 // indirect
github.com/google/gnostic-models v0.6.9 // indirect
github.com/google/go-cmp v0.6.0 // indirect
github.com/google/go-cmp v0.7.0 // indirect
github.com/google/go-containerregistry/pkg/authn/kubernetes v0.0.0-20230516205744-dbecb1de8cfa // indirect
github.com/google/go-github/v55 v55.0.0 // indirect
github.com/google/go-github/v68 v68.0.0 // indirect
github.com/google/go-github/v69 v69.2.0 // indirect
github.com/google/go-querystring v1.1.0 // indirect
github.com/google/gofuzz v1.2.0 // indirect
github.com/google/s2a-go v0.1.8 // indirect
Expand Down
33 changes: 18 additions & 15 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,10 @@ github.com/Azure/azure-sdk-for-go v68.0.0+incompatible h1:fcYLmCpyNYRnvJbPerq7U0
github.com/Azure/azure-sdk-for-go v68.0.0+incompatible/go.mod h1:9XXNKU+eRnpl9moKnB4QOLf1HestfXbmab5FXxiDBjc=
github.com/Azure/azure-sdk-for-go/sdk/azcore v1.17.0 h1:g0EZJwz7xkXQiZAI5xi9f3WWFYBlX1CPTrR+NDToRkQ=
github.com/Azure/azure-sdk-for-go/sdk/azcore v1.17.0/go.mod h1:XCW7KnZet0Opnr7HccfUw1PLc4CjHqpcaxW8DHklNkQ=
github.com/Azure/azure-sdk-for-go/sdk/azidentity v1.8.1 h1:1mvYtZfWQAnwNah/C+Z+Jb9rQH95LPE2vlmMuWAHJk8=
github.com/Azure/azure-sdk-for-go/sdk/azidentity v1.8.1/go.mod h1:75I/mXtme1JyWFtz8GocPHVFyH421IBoZErnO16dd0k=
github.com/Azure/azure-sdk-for-go/sdk/azidentity/cache v0.3.1 h1:Bk5uOhSAenHyR5P61D/NzeQCv+4fEVV8mOkJ82NqpWw=
github.com/Azure/azure-sdk-for-go/sdk/azidentity/cache v0.3.1/go.mod h1:QZ4pw3or1WPmRBxf0cHd1tknzrT54WPBOQoGutCPvSU=
github.com/Azure/azure-sdk-for-go/sdk/azidentity v1.8.2 h1:F0gBpfdPLGsw+nsgk6aqqkZS1jiixa5WwFe3fk/T3Ys=
github.com/Azure/azure-sdk-for-go/sdk/azidentity v1.8.2/go.mod h1:SqINnQ9lVVdRlyC8cd1lCI0SdX4n2paeABd2K8ggfnE=
github.com/Azure/azure-sdk-for-go/sdk/azidentity/cache v0.3.2 h1:yz1bePFlP5Vws5+8ez6T3HWXPmwOK7Yvq8QxDBD3SKY=
github.com/Azure/azure-sdk-for-go/sdk/azidentity/cache v0.3.2/go.mod h1:Pa9ZNPuoNu/GztvBSKk9J1cDJW6vk/n0zLtV4mgd8N8=
github.com/Azure/azure-sdk-for-go/sdk/internal v1.10.0 h1:ywEEhmNahHBihViHepv3xPBn1663uRv2t2q/ESv9seY=
github.com/Azure/azure-sdk-for-go/sdk/internal v1.10.0/go.mod h1:iZDifYGJTIgIIkYRNWPENUnqx6bJ2xnSDFI2tjwZNuY=
github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/storage/armstorage v1.6.0 h1:PiSrjRPpkQNjrM8H0WwKMnZUdu1RGMtd/LdGKUrOo+c=
Expand Down Expand Up @@ -84,8 +84,8 @@ github.com/Azure/go-ntlmssp v0.0.0-20221128193559-754e69321358 h1:mFRzDkZVAjdal+
github.com/Azure/go-ntlmssp v0.0.0-20221128193559-754e69321358/go.mod h1:chxPXzSsl7ZWRAuOIE23GDNzjWuZquvFlgA8xmpunjU=
github.com/AzureAD/microsoft-authentication-extensions-for-go/cache v0.1.1 h1:WJTmL004Abzc5wDB5VtZG2PJk5ndYDgVacGqfirKxjM=
github.com/AzureAD/microsoft-authentication-extensions-for-go/cache v0.1.1/go.mod h1:tCcJZ0uHAmvjsVYzEFivsRTN00oz5BEsRgQHu5JZ9WE=
github.com/AzureAD/microsoft-authentication-library-for-go v1.3.3 h1:H5xDQaE3XowWfhZRUpnfC+rGZMEVoSiji+b+/HFAPU4=
github.com/AzureAD/microsoft-authentication-library-for-go v1.3.3/go.mod h1:wP83P5OoQ5p6ip3ScPr0BAq0BvuPAvacpEuSzyouqAI=
github.com/AzureAD/microsoft-authentication-library-for-go v1.4.0 h1:MUkXAnvvDHgvPItl0nBj0hgk0f7hnnQbGm0h0+YxbN4=
github.com/AzureAD/microsoft-authentication-library-for-go v1.4.0/go.mod h1:wP83P5OoQ5p6ip3ScPr0BAq0BvuPAvacpEuSzyouqAI=
github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU=
github.com/BurntSushi/toml v1.4.0 h1:kuoIxZQy2WRRk1pttg9asf+WVv6tWQuBNVmK8+nqPr0=
github.com/BurntSushi/toml v1.4.0/go.mod h1:ukJfTF/6rtPPRCnwkur4qwRxa8vTRFBF0uk2lLoLwho=
Expand Down Expand Up @@ -222,8 +222,8 @@ github.com/blang/semver v3.5.1+incompatible h1:cQNTCjp13qL8KC3Nbxr/y2Bqb63oX6wdn
github.com/blang/semver v3.5.1+incompatible/go.mod h1:kRBLl5iJ+tD4TcOOxsy/0fnwebNt5EWlYSAyrTnjyyk=
github.com/blang/semver/v4 v4.0.0 h1:1PFHFE6yCCTv8C1TeyNNarDzntLi7wMI5i/pzqYIsAM=
github.com/blang/semver/v4 v4.0.0/go.mod h1:IbckMUScFkM3pff0VJDNKRiT6TG/YpiHIM2yvyW5YoQ=
github.com/bradleyfalzon/ghinstallation/v2 v2.13.0 h1:5FhjW93/YLQJDmPdeyMPw7IjAPzqsr+0jHPfrPz0sZI=
github.com/bradleyfalzon/ghinstallation/v2 v2.13.0/go.mod h1:EJ6fgedVEHa2kUyBTTvslJCXJafS/mhJNNKEOCspZXQ=
github.com/bradleyfalzon/ghinstallation/v2 v2.14.0 h1:0D4vKCHOvYrDU8u61TnE2JfNT4VRrBLphmxtqazTO+M=
github.com/bradleyfalzon/ghinstallation/v2 v2.14.0/go.mod h1:LOVmdZYVZ8jqdr4n9wWm1ocDiMz9IfMGfRkaYC1a52A=
github.com/bshuster-repo/logrus-logstash-hook v1.0.0 h1:e+C0SB5R1pu//O4MQ3f9cFuPGoOVeF2fE4Og9otCc70=
github.com/bshuster-repo/logrus-logstash-hook v1.0.0/go.mod h1:zsTqEiSzDgAa/8GZR7E1qaXrhYNDKBYy5/dWPTIflbk=
github.com/bsm/ginkgo/v2 v2.7.0/go.mod h1:AiKlXPm7ItEHNc/2+OkrNG4E0ITzojb9/xWzvQ9XZ9w=
Expand Down Expand Up @@ -374,8 +374,10 @@ github.com/fluxcd/pkg/apis/event v0.16.0 h1:ffKc/3erowPnh72lFszz7sPQhLZ7bhqNrq+p
github.com/fluxcd/pkg/apis/event v0.16.0/go.mod h1:D/QQi5lHT9/Ur3OMFLJO71D4KDQHbJ5s8dQV3h1ZAT0=
github.com/fluxcd/pkg/apis/meta v1.10.0 h1:rqbAuyl5ug7A5jjRf/rNwBXmNl6tJ9wG2iIsriwnQUk=
github.com/fluxcd/pkg/apis/meta v1.10.0/go.mod h1:n7NstXHDaleAUMajcXTVkhz0MYkvEXy1C/eLI/t1xoI=
github.com/fluxcd/pkg/auth v0.3.0 h1:I1A3e81O+bpAgEcJ3e+rXqObKPjzBu6FLYXQTSxXLOs=
github.com/fluxcd/pkg/auth v0.3.0/go.mod h1:g9KJ4iNcCd6Sb7al4yN1+olgOfgwmU4lgCWbwvMsFRE=
github.com/fluxcd/pkg/auth v0.8.0 h1:E5iDhsomYBZKOA2XkVW1/DZFGeMLJ4TFDV0C5xCBJi0=
github.com/fluxcd/pkg/auth v0.8.0/go.mod h1:St5N+xY4KXDTFvuP8UMlgWkaZv6cpU+CQ9SL7Z0bsJg=
github.com/fluxcd/pkg/cache v0.7.0 h1:6TTWbxCyAxErIAT338KrLTy96ds+vSDw4sEyypSISfs=
github.com/fluxcd/pkg/cache v0.7.0/go.mod h1:EHpyMSXf/ECKIoKEQmNCOesH2wfAdpmXR/ZXD6VwWRg=
github.com/fluxcd/pkg/git v0.24.0 h1:aMAL8MUNPZXyRia+LVVudTpmLHIpzmz9F5tedvhhLzs=
github.com/fluxcd/pkg/git v0.24.0/go.mod h1:vxUhjBwnkvbAByN7UC5Go33/mgrLSIIg1rH+dyOZVRo=
github.com/fluxcd/pkg/git/gogit v0.24.0 h1:i59dkijZZ+IfsI++tDFzTmGvi8VqeenvSijRy8pd8ts=
Expand Down Expand Up @@ -541,8 +543,9 @@ github.com/google/go-cmp v0.5.2/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/
github.com/google/go-cmp v0.5.3/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
github.com/google/go-cmp v0.5.5/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
github.com/google/go-cmp v0.5.9/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI=
github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
github.com/google/go-cmp v0.7.0 h1:wk8382ETsv4JYUZwIsn6YpYiWiBsYLSJiTsyBybVuN8=
github.com/google/go-cmp v0.7.0/go.mod h1:pXiqmnSA92OHEEa9HXL2W4E7lf9JzCmGVUdgjX3N/iU=
github.com/google/go-containerregistry v0.20.3 h1:oNx7IdTI936V8CQRveCjaxOiegWwvM7kqkbXTpyiovI=
github.com/google/go-containerregistry v0.20.3/go.mod h1:w00pIgBRDVUDFM6bq+Qx8lwNWK+cxgCuX1vd3PIBDNI=
github.com/google/go-containerregistry/pkg/authn/k8schain v0.0.0-20241111191718-6bce25ecf029 h1:0G7T22yXy+FqumvxcEg48EU4llskcDeQ2eM3vaTr64c=
Expand All @@ -551,8 +554,8 @@ github.com/google/go-containerregistry/pkg/authn/kubernetes v0.0.0-2023051620574
github.com/google/go-containerregistry/pkg/authn/kubernetes v0.0.0-20230516205744-dbecb1de8cfa/go.mod h1:KdL98/Va8Dy1irB6lTxIRIQ7bQj4lbrlvqUzKEQ+ZBU=
github.com/google/go-github/v55 v55.0.0 h1:4pp/1tNMB9X/LuAhs5i0KQAE40NmiR/y6prLNb9x9cg=
github.com/google/go-github/v55 v55.0.0/go.mod h1:JLahOTA1DnXzhxEymmFF5PP2tSS9JVNj68mSZNDwskA=
github.com/google/go-github/v68 v68.0.0 h1:ZW57zeNZiXTdQ16qrDiZ0k6XucrxZ2CGmoTvcCyQG6s=
github.com/google/go-github/v68 v68.0.0/go.mod h1:K9HAUBovM2sLwM408A18h+wd9vqdLOEqTUCbnRIcx68=
github.com/google/go-github/v69 v69.2.0 h1:wR+Wi/fN2zdUx9YxSmYE0ktiX9IAR/BeePzeaUUbEHE=
github.com/google/go-github/v69 v69.2.0/go.mod h1:xne4jymxLR6Uj9b7J7PyTpkMYstEMMwGZa0Aehh1azM=
github.com/google/go-querystring v1.1.0 h1:AnCroh3fv4ZBgVIf1Iwtovgjaw/GiKJo8M8yD/fhyJ8=
github.com/google/go-querystring v1.1.0/go.mod h1:Kcdr2DB4koayq7X8pmAG4sNG59So17icRSOU623lUBU=
github.com/google/gofuzz v1.0.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg=
Expand Down Expand Up @@ -1123,8 +1126,8 @@ golang.org/x/crypto v0.15.0/go.mod h1:4ChreQoLWfG3xLDer1WdlH5NdlQ3+mwnQq1YTKY+72
golang.org/x/crypto v0.19.0/go.mod h1:Iy9bg/ha4yyC70EfRS8jz+B6ybOBKMaSxLj6P6oBDfU=
golang.org/x/crypto v0.23.0/go.mod h1:CKFgDieR+mRhux2Lsu27y0fO304Db0wZe70UKqHu0v8=
golang.org/x/crypto v0.31.0/go.mod h1:kDsLvtWBEx7MV9tJOj9bnXsPbxwJQ6csT/x4KIN4Ssk=
golang.org/x/crypto v0.33.0 h1:IOBPskki6Lysi0lo9qQvbxiQ+FvsCC/YWOecCHAixus=
golang.org/x/crypto v0.33.0/go.mod h1:bVdXmD7IV/4GdElGPozy6U7lWdRXA4qyRVGJV57uQ5M=
golang.org/x/crypto v0.34.0 h1:+/C6tk6rf/+t5DhUketUbD1aNGqiSX3j15Z6xuIDlBA=
golang.org/x/crypto v0.34.0/go.mod h1:dy7dXNW32cAb/6/PRuTNsix8T+vJAqvuIy5Bli/x0YQ=
golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA=
golang.org/x/exp v0.0.0-20240719175910-8a7402abbf56 h1:2dVuKD2vS7b0QIHQbpyTISPd0LeHDbnYEryqj5Q1ug8=
golang.org/x/exp v0.0.0-20240719175910-8a7402abbf56/go.mod h1:M4RDyNAINzryxdtnbRXRL/OHtkFuWGRjvuhBJpk2IlY=
Expand Down
10 changes: 10 additions & 0 deletions internal/controller/gitrepository_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ import (

eventv1 "github.com/fluxcd/pkg/apis/event/v1beta1"
"github.com/fluxcd/pkg/apis/meta"
"github.com/fluxcd/pkg/cache"
"github.com/fluxcd/pkg/git"
"github.com/fluxcd/pkg/git/gogit"
"github.com/fluxcd/pkg/git/repository"
Expand Down Expand Up @@ -136,11 +137,14 @@ type GitRepositoryReconciler struct {
features map[string]bool

patchOptions []patch.Option

tokenCache *cache.TokenCache
}

type GitRepositoryReconcilerOptions struct {
DependencyRequeueInterval time.Duration
RateLimiter workqueue.TypedRateLimiter[reconcile.Request]
TokenCache *cache.TokenCache
}

// gitRepositoryReconcileFunc is the function type for all the
Expand All @@ -160,6 +164,8 @@ func (r *GitRepositoryReconciler) SetupWithManagerAndOptions(mgr ctrl.Manager, o
r.features = features.FeatureGates()
}

r.tokenCache = opts.TokenCache

return ctrl.NewControllerManagedBy(mgr).
For(&sourcev1.GitRepository{}, builder.WithPredicates(
predicate.Or(predicate.GenerationChangedPredicate{}, predicates.ReconcileRequestedPredicate{}),
Expand Down Expand Up @@ -677,6 +683,7 @@ func (r *GitRepositoryReconciler) getAuthOpts(ctx context.Context, obj *sourcev1
Name: sourcev1.GitProviderGitHub,
GitHubOpts: []github.OptFunc{
github.WithAppData(authData),
github.WithCache(r.tokenCache, sourcev1.GitRepositoryKind, obj.GetName(), obj.GetNamespace()),
Copy link
Member

@dipti-pai dipti-pai Mar 7, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This pkg PR introduces cache in the auth/github pkg and caches the token once it is fetched. In scenarios where the token is invalid (for example: due to incorrectly permissions configured by the user), the git operations would fail with an error (similar to Unauthorized). The token needs to be invalidated from the cache in this case.

When we had attempted to add the cache as part of enabling Azure OIDC for gitrepository, we had discussed in the PR comment and the dev meetings to add the cache in the respective clients instead where the token is actually used. For example, for git operations, the cache would be added to git client. During clone operations, cached token would be fetched from the cache first. In case of a cache HIT, it would be used to do the clone. If the clone operation fails and the token is no longer valid it would be invalidated from the cache. In case of a cache miss or invalid token, a new token would be fetched using auth/github GetToken API. This token would be cached after successful clone operation.

Copy link
Member Author

@matheuscscp matheuscscp Mar 7, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Scenario 1: Permission errors

I just did a simple test here with our implementation for flux-operator, we have this code running in a cluster for a few days.

This was my experiment:

  1. I revoked the GitHub App permission to read pull requests.
  2. The object reconciliation started showing this error: failed to call provider failed to list requests: could not list pull requests: GET https://api.github.com/repos/matheuscscp-test-org/test-repo/pulls?per_page=100&state=open: 403 Resource not accessible by integration []
  3. I re-added the permission to read pull requests to the GitHub App.
  4. The issue was resolved, the reconciliation resumed.

To make sure that the cached token was not purged and was reused after I re-added the permission, I ran this command:

k logs flux-operator-79d48cc994-68cds | grep refresh
{"level":"debug","ts":"2025-03-07T20:12:05.691Z","msg":"item refreshed",
"controller":"resourcesetinputprovider","controllerGroup":"fluxcd.controlplane.io",
"controllerKind":"ResourceSetInputProvider","ResourceSetInputProvider":{
"name":"rsip-gh-app-token-cache-test","namespace":"flux-system"},"namespace":"flux-system",
"name":"rsip-gh-app-token-cache-test","reconcileID":"01dd2fc7-ef25-49ab-bbb9-bf4e64e095aa",
"key":"githubAppID=<redacted>,githubAppInstallationID=<redacted>,githubAppBaseURL=,githubAppPrivateKeyDigest=<redacted>",
"token":{"duration":"59m59.308435414s"}}

Only one log was printed, the same one I saw before the experiment by running the same command and looking at the timestamp.

Conclusion: Purging the token from the cache doesn't help when the token is invalid due to permissions. That's because the token represents an identity. The set of permissions associated with that identity are not embedded on the token. So the token is not really invalid, it just happens that the identity it represents lacks permissions. The token remains valid and that is proven by the fact that the reconciliation resumed after I re-added the permissions.

Scenario 2: GitHub App is deleted/replaced, or reinstalled in the org/repo, or the private key is rotated, or the base URL is changed

If any of these events happen, the token key will change, see how the cache key is built:

https://github.com/fluxcd/pkg/blob/8b1f852228b117123efcc7b5708ce112801416b6/auth/github/client.go#L232-L241

func (p *Client) buildCacheKey() string {
	privateKeyDigest := sha256.Sum256(p.privateKey)
	keyParts := []string{
		fmt.Sprintf("%s=%s", AppIDKey, p.appID),
		fmt.Sprintf("%s=%s", AppInstallationIDKey, p.installationID),
		fmt.Sprintf("%s=%s", AppBaseUrlKey, p.apiURL),
		fmt.Sprintf("%sDigest=%x", AppPrivateKey, privateKeyDigest),
	}
	return strings.Join(keyParts, ",")
}

Conclusion: The cached token is effectively a function of (appID, installationID, baseURL, privateKey). If these remain the same, the cached token remains valid if it's not expired.

Overall Conclusion

There are no real scenarios where purging the token from the cache really helps, we don't need to worry about that.

P.S.: We expire tokens with 80% of their lifetime, so there's no risk of getting errors due to expired token.

Copy link
Member Author

@matheuscscp matheuscscp Mar 7, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add the cache in the respective clients instead where the token is actually used

I don't think this approach is good, because a GitHub App could be used for multiple different things, not just cloning a git repo. We're gonna use GitHub App tokens for other things in Flux e.g. for the notification providers. We shouldn't cache a token twice, once for the github provider and once for the githubdispatch provider, if the underlying app/installation/baseURL/privateKey is the same for both Provider objects.

So, again, the token is a function of (appID, installationID, baseURL, privateKey), not (appID, installationID, baseURL, privateKey, use case).

The same idea will apply when we implement caching for OIDC tokens. They are a function of the identity they represent, not (identity, use case). We will share a token for a Bucket and an OCIRepository object if the identity they want to impersonate is the same and we already have a token for it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense, thanks for the detailed test scenarios and explanation.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment was very important 👍 Let's leave this thread unresolved as it contains a lot of information that is important for whoever reads this PR in the future, even after merged

},
}
default:
Expand Down Expand Up @@ -1089,6 +1096,9 @@ func (r *GitRepositoryReconciler) reconcileDelete(ctx context.Context, obj *sour
// Remove our finalizer from the list
controllerutil.RemoveFinalizer(obj, sourcev1.SourceFinalizer)

// Cleanup caches.
r.tokenCache.DeleteEventsForObject(sourcev1.GitRepositoryKind, obj.GetName(), obj.GetNamespace())

// Stop reconciliation as the object is being deleted
return sreconcile.ResultEmpty, nil
}
Expand Down
24 changes: 23 additions & 1 deletion main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,10 @@ import (
ctrlcache "sigs.k8s.io/controller-runtime/pkg/cache"
ctrlclient "sigs.k8s.io/controller-runtime/pkg/client"
ctrlcfg "sigs.k8s.io/controller-runtime/pkg/config"
ctrlmetrics "sigs.k8s.io/controller-runtime/pkg/metrics"
metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server"

pkgcache "github.com/fluxcd/pkg/cache"
"github.com/fluxcd/pkg/git"
"github.com/fluxcd/pkg/runtime/client"
helper "github.com/fluxcd/pkg/runtime/controller"
Expand All @@ -50,7 +52,7 @@ import (
"github.com/fluxcd/pkg/runtime/pprof"
"github.com/fluxcd/pkg/runtime/probes"

"github.com/fluxcd/source-controller/api/v1"
v1 "github.com/fluxcd/source-controller/api/v1"
"github.com/fluxcd/source-controller/api/v1beta2"

// +kubebuilder:scaffold:imports
Expand Down Expand Up @@ -89,6 +91,10 @@ func init() {
}

func main() {
const (
tokenCacheDefaultMaxSize = 0
)

var (
metricsAddr string
eventsAddr string
Expand All @@ -114,6 +120,7 @@ func main() {
artifactRetentionTTL time.Duration
artifactRetentionRecords int
artifactDigestAlgo string
tokenCacheOptions pkgcache.TokenFlags
)

flag.StringVar(&metricsAddr, "metrics-addr", envOrDefault("METRICS_ADDR", ":8080"),
Expand Down Expand Up @@ -160,6 +167,7 @@ func main() {
featureGates.BindFlags(flag.CommandLine)
watchOptions.BindFlags(flag.CommandLine)
intervalJitterOptions.BindFlags(flag.CommandLine)
tokenCacheOptions.BindFlags(flag.CommandLine, tokenCacheDefaultMaxSize)

flag.Parse()

Expand Down Expand Up @@ -187,6 +195,19 @@ func main() {
mustSetupHelmLimits(helmIndexLimit, helmChartLimit, helmChartFileLimit)
helmIndexCache, helmIndexCacheItemTTL := mustInitHelmCache(helmCacheMaxSize, helmCacheTTL, helmCachePurgeInterval)

var tokenCache *pkgcache.TokenCache
if tokenCacheOptions.MaxSize > 0 {
var err error
tokenCache, err = pkgcache.NewTokenCache(tokenCacheOptions.MaxSize,
pkgcache.WithMaxDuration(tokenCacheOptions.MaxDuration),
pkgcache.WithMetricsRegisterer(ctrlmetrics.Registry),
pkgcache.WithMetricsPrefix("gotk_token_"))
if err != nil {
setupLog.Error(err, "unable to create token cache")
os.Exit(1)
}
}

ctx := ctrl.SetupSignalHandler()

if err := (&controller.GitRepositoryReconciler{
Expand All @@ -198,6 +219,7 @@ func main() {
}).SetupWithManagerAndOptions(mgr, controller.GitRepositoryReconcilerOptions{
DependencyRequeueInterval: requeueDependency,
RateLimiter: helper.GetRateLimiter(rateLimiterOptions),
TokenCache: tokenCache,
}); err != nil {
setupLog.Error(err, "unable to create controller", "controller", v1.GitRepositoryKind)
os.Exit(1)
Expand Down
Loading