Skip to content
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.

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

Clean up obtaining bearer tokens for registries #2480

Merged
merged 6 commits into from
Jul 11, 2024

Conversation

mtrmac
Copy link
Collaborator

@mtrmac mtrmac commented Jul 9, 2024

This is a few refactors currently living in #1968, split for separate review and merging — without the substance of that feature.

The net effect of this PR is to decrease memory requirements a tiny bit, and to add make tests a tiny bit more relevant and decoupled.

See individual commit messages for details.

Remove an unnecessary cast. Should not change (test) behavior.

Signed-off-by: Miloslav Trmač <[email protected]>
We will want to add locks and more to the in-memory type;
sharing that with JSON gets awkward, and an explicit separation
between the externally-imposed structure and internal records
is cleaner anyway.

For now, just introduces a separate type with the same structure,
should not change behavior.

Signed-off-by: Miloslav Trmač <[email protected]>
No need to make it a public field now.

Should not change behavior.

Signed-off-by: Miloslav Trmač <[email protected]>
That's the value that really matters, not the inputs;
and we will remove the inputs from bearerToken.

Signed-off-by: Miloslav Trmač <[email protected]>
These fields need to exist when parsing JSON; but we can just
record the outcome of processing them.

Should not change behavior.

Signed-off-by: Miloslav Trmač <[email protected]>
Only moves unchanged code, should not change behavior.

Signed-off-by: Miloslav Trmač <[email protected]>
@rhatdan
Copy link
Member

rhatdan commented Jul 11, 2024

LGTM
@giuseppe PTAL

Copy link
Member

@giuseppe giuseppe left a comment

Choose a reason for hiding this comment

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

/lgtm

@rhatdan rhatdan merged commit 2416f89 into containers:main Jul 11, 2024
10 checks passed
@mtrmac mtrmac deleted the token-cleanup branch July 11, 2024 14:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants