Re-use public credential provider API #9502
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: saschagrunert The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
ad8dc24 to
486c307
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #9502 +/- ##
==========================================
- Coverage 67.02% 66.96% -0.07%
==========================================
Files 202 202
Lines 28155 28168 +13
==========================================
- Hits 18872 18863 -9
- Misses 7706 7722 +16
- Partials 1577 1583 +6 🚀 New features to boost your workflow:
|
|
/retest |
| ext := filepath.Ext(authFilePath) | ||
| newAuthFileName := fmt.Sprintf("%s-%s%s", strings.TrimSuffix(filepath.Base(authFilePath), ext), uuid.New(), ext) | ||
|
|
||
| tempAuthFilePath := filepath.Join(inUseAuthDirPath, newAuthFileName) |
There was a problem hiding this comment.
This description may be outdated? The final auth file name will be <NAMESPACE>-<IMAGE_NAME_SHA256>-<UUID>.json.
There was a problem hiding this comment.
Good catch, updated the docs.
| if err := os.Rename(authFilePath, tempAuthFilePath); err != nil { | ||
| return cleanup, fmt.Errorf("unable to move auth file path to temporary location: %w", err) | ||
| } |
There was a problem hiding this comment.
#9475 (comment)
We should mention potential race scenarios (same name with different tag images pulled at once) and the reason why it's acceptable for posterity.
There was a problem hiding this comment.
Updated the doc within the function to explain the possible race.
|
|
||
| inUseAuthDirPath := filepath.Join(s.config.NamespacedAuthDir, "in-use") | ||
| if err := os.MkdirAll(inUseAuthDirPath, 0o700); err != nil { | ||
| return cleanup, fmt.Errorf("unable to ensure in-use auth dir: %w", err) |
There was a problem hiding this comment.
I'm wondering if we want to cleanup authFilePath when it fails in the middle. Does it make this more race-prone?
There was a problem hiding this comment.
Makes sense, I added another cleanup function if that method errors.
af2338a to
82f69f4
Compare
|
/retest |
82f69f4 to
27dcdcc
Compare
Re-using the public auth and config API from the credential provider to keep both projects in sync. Signed-off-by: Sascha Grunert <sgrunert@redhat.com>
27dcdcc to
93121f4
Compare
|
/lgtm |
What type of PR is this?
/kind cleanup
What this PR does / why we need it:
Re-using the public auth and config API from the credential provider to keep both projects in sync.
Which issue(s) this PR fixes:
None
Special notes for your reviewer:
None
Does this PR introduce a user-facing change?