add repository to the credentials prompt #2596
Conversation
|
Skipping CI for Draft Pull Request. |
knative-prow
bot
added
the
do-not-merge/work-in-progress
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: gauron99 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 |
knative-prow
bot
added
approved
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2596 +/- ##
==========================================
+ Coverage 64.14% 65.41% +1.26%
==========================================
Files 130 130
Lines 15513 15515 +2
==========================================
+ Hits 9951 10149 +198
+ Misses 4623 4389 -234
- Partials 939 977 +38
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
pkg/docker/creds/credentials.go
Outdated
| ) | ||
|
|
||
| type CredentialsCallback func(registry string) (docker.Credentials, error) | ||
| type CredentialsCallback func(f fn.Function, registry string) (docker.Credentials, error) |
There was a problem hiding this comment.
@gauron99 I do not see any reason for adding fn.Function here. The registry parameter is completely sufficient.
There was a problem hiding this comment.
There was a problem hiding this comment.
fmt.Fprintf(out, "Incorrect credentials for registry '%s'. Please make sure the registry is correct and try again.\n", registry)There was a problem hiding this comment.
it is so that I can provide the image name and print it out, making it more obvious what are the credentials "used for". the registry is just the docker.io part
There was a problem hiding this comment.
Hmm I thought that registry also included project/namespace something like docker.io/johndoe.
There was a problem hiding this comment.
I will clean it up a bit, wait. I will ping you when its ready
knative-prow-robot
added
the
needs-rebase
gauron99
force-pushed
the
registry-error-message
branch
from
845eaa4
to
0c162c3
Compare
knative-prow-robot
removed
the
needs-rebase
Signed-off-by: David Fridrich <[email protected]>
gauron99
force-pushed
the
registry-error-message
branch
from
0c162c3
to
1aac8ed
Compare
Signed-off-by: David Fridrich <[email protected]>
Signed-off-by: David Fridrich <[email protected]>
|
@matejvasek what about this. I have changed the sole argument that needed it. Added a repository to it so now it is displayed when the prompt asks you for your credentials when trying to 'push' WITH the actual 'registry/repository' |
gauron99
changed the title
This seems reasonable. Just call it IMO that how our flag should have been called: instead of |
|
PR needs rebase. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
knative-prow-robot
added
the
needs-rebase
Added
repositoryvalue (now it isregistry/repository) to prompt for credentials where its used for printing out tto terminal "during the prompt" to make it easier for the user to see what he is trying to provide credentials for.Additionally I have created a new issue that is a followup to this and its' purpose is to further optimize this "credentials" process
fix #2562