Repository navigation
[pull] main from nephio-project:main - #40
Merged
Merged
Conversation
Bumps [github/codeql-action](https://github.com/github/codeql-action) from 4.37.9 to 4.38.0. - [Release notes](https://github.com/github/codeql-action/releases) - [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md) - [Commits](github/codeql-action@v4.37.9...v4.38.0) --- updated-dependencies: - dependency-name: github/codeql-action dependency-version: 4.38.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com>
The GitHub client is built once from the personal access token in the secret,
and GetMyUserInfo and the four Repositories calls all go through it. Minting an
installation token replaced it:
r.githubClient = github.NewClient(nil).WithAuthToken(installToken)
CreateAccessToken is the only caller and the Token reconciler calls it on every
reconcile, so from the first token onwards every user and repository call ran as
the installation. GitHub's authenticated-user endpoint does not accept an
installation token, so those calls start coming back 401. Recording what a test
server receives, against the code before this:
1. /user Bearer personal-access-token
2. /app/installations/2/access_tokens Bearer <app JWT>
3. (never arrives) GET https://api.github.com/user: 401 Bad credentials
The third request does not reach the test server at all, because
github.NewClient(nil) also discards whatever base URL the client had. To be
exact about what that buys: nothing in Start configures a non-default base URL
today and the mint is still addressed at api.github.com, so this is not GitHub
Enterprise support. It means a base URL, once configured, survives a mint.
Both reconcilers share one client through the singleton, so this is not confined
to whichever of them ran first.
Removing the write does not make the client race-free. GetClient publishes
singleInstance and then Start writes appID, installID, privateKey and
githubClient from another goroutine while IsInitialized reads one of them, and
the first nil check sits outside the lock. That is initialization rather than
refresh, it arrived with #1176, and it wants its own change: #1192.
The token endpoint and the client it goes through are fields now. The endpoint
keeps its old default; the client does not, because http.DefaultClient has no
timeout and a mint that is never answered would hold the reconcile open for as
long as the connection stays up. Otherwise the only way to reach the mint from a test is to
swap http.DefaultClient.Transport, which the whole process shares, and to
rewrite the request inside RoundTrip, which the http.RoundTripper contract asks
callers not to do. The test server is TLS, so a client that does not carry its
certificate cannot reach it, and the injection is something the test checks
rather than decoration.
The test drives the real path rather than asserting on a field: GetMyUserInfo,
CreateAccessToken, GetMyUserInfo, GetRepo, checking which credential each
request carried and that the base URL survived. The mint's credential is
verified as a JWT this app signed, with the algorithm and issuer it claims;
asserting only that the PAT is absent would also accept no credential at all,
or any other bearer string. A repository call after a mint that failed is
checked too, since that is the call this change is named for.
One thing in that function needed fixing on the way. A 201 that carries no
token, an empty one or a null one was returned as a credential and written into
the Secret as the password; it is refused now.
TestClient takes its context from context.Background rather than
ctrl.SetupSignalHandler, which installs a process-wide handler and panics the
second time it is called, so the package could only ever be run once:
$ go test -count=2 ./git/github/
panic: close of closed channel
A unit test wants a context, not signal handling, and -count is how a new test
earns being trusted.
Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
#1176 narrowed the Gitea adapter's DeleteAccessToken to an int64 token id. The token reconciler holds the name, so since that change every deletion has failed on the type check and the Token's finalizer has stayed on. The SDK has taken a name or an id since Gitea 1.13, so the name goes back through. It cannot go through unexamined, though. Gitea reads the path segment of DELETE /users/{user}/tokens/{token} with ParseInt(token, 0, 64) and looks it up as a name only when that yields zero, so a name that reads as an id deletes whichever token holds that id. Each row below starts from an empty token list on gitea/gitea:1.19.3, the image nephio-project/catalog pins at distros/sandbox/gitea/statefulset-gitea.yaml, with one token holding the id and a second carrying the name: id 28 some-other-cluster, "28" DELETE .../28 204 deleted id 28 id 50 some-other-cluster, "062" DELETE .../062 204 deleted id 50 id 72 some-other-cluster, "0x48" DELETE .../0x48 204 deleted id 72 id 94 some-other-cluster, "0b1011110" DELETE .../0b1011110 204 deleted id 94 Gitea discards that parse error and branches on the value alone, so the guard here has to as well. ParseInt answers a range error with a saturated, non-zero integer, so a twenty-digit name is an id to Gitea however wrong it looks: id 1 "99999999999999999999" DELETE .../99999999999999999999 404, nothing deleted and a 404 is what this change treats as the token being gone. Base 0 also lets the leading characters pick the base, so this reaches past the decimal names. Each of those answers 204 having destroyed a token belonging to a different Token, and left the one that was asked for alive on the server. Token.GetTokenName() is the object's own name in the default namespace and a Kubernetes name may be all digits, so a Token named "28" is all it takes. Names that read as an id are now resolved over the token list and deleted by the id that carries them; ordinary names still go out as one request. The guard runs the same parse the server runs, so the two agree on which names are which. A test for decimal digits would not: it misses 0x48 and 0b1011110, which Gitea reads as 72 and 94. Saying that no token carries the name takes the whole list, and the whole list read before anything is deleted. Gitea's own name path answers 422 when more than one token carries a name rather than picking one, so this collects the matches and refuses to guess; deleting the first would report success while the others stayed. A resolved id has to be positive: zero goes back out as a name lookup, and a negative one is not an id at all. The lookup also checks it got the whole list. Gitea reports the row count in X-Total-Count, and a server that paged anyway, listed a null token, or did not say how many rows there are has not established absence; each of those is reported rather than read as a token that is already gone. Not knowing the size of the list is not the same as knowing the name is not in it, so it is an error rather than a success. Sending a resolved id of zero would land back on the name path this change exists to avoid. Three smaller things the delete path needed. A 404 means the token is not on the server, which is the state deletion is asking for, so the finalizer comes off rather than the Token being held open on a token somebody already removed. The SDK leaves its response nil on a transport error, which the adapter used to dereference. And a lookup that fails hands back no response at all, so a listing that answers 404 cannot be read as the token being gone. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
[git/gitea] Restore access-token deletion by name
…he-pat [git/github] Keep repository calls on the personal access token
…s/github/codeql-action-4.38.0 Bump github/codeql-action from 4.37.9 to 4.38.0
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )