kro cli login command #656
Conversation
|
Unknown CLA label state. Rechecking for CLA labels. Send feedback to sig-contributor-experience at kubernetes/community. /check-cla |
k8s-ci-robot
added
the
cncf-cla: yes
a-hilaly
left a comment
a-hilaly
left a comment
There was a problem hiding this comment.
Thank you @DhairyaMajmudar ! left a few comments in line
| loginCmd.PersistentFlags().StringVarP(&loginConfig.password, | ||
| "password", "p", "", | ||
| "Password for the registry (not recommended, use interactive mode)", | ||
| ) |
There was a problem hiding this comment.
in general it is recommended to not pass passowrds as flags, let's maybe opt for --password-stdin where users could pipe the password instead?
| Auths map[string]RegistryAuth `json:"auths"` | ||
| } | ||
|
|
||
| var loginConfig = &LoginConfig{} |
There was a problem hiding this comment.
nit: let's not declare this a high level variable
| type LoginConfig struct { | ||
| registry string | ||
| username string | ||
| password string | ||
| } | ||
|
|
||
| type RegistryAuth struct { | ||
| Username string `json:"username"` | ||
| Password string `json:"password"` | ||
| Auth string `json:"auth"` | ||
| } | ||
|
|
||
| type ConfigFile struct { | ||
| Auths map[string]RegistryAuth `json:"auths"` | ||
| } |
There was a problem hiding this comment.
these structures aren't imported anywhere let's just keep them private. Also can you please add GoDocs for the structure and their fields?
| switch registry { | ||
| case "docker.io", "index.docker.io": | ||
| return "https://index.docker.io/v1/", nil | ||
| case "ghcr.io": | ||
| return "ghcr.io", nil | ||
| } |
| if !strings.Contains(registry, "://") { | ||
| registry = "https://" + registry | ||
| } |
There was a problem hiding this comment.
Can we just rely on url.parse here?
| generate "github.com/kro-run/kro/cmd/kro/commands/generate" | ||
| login "github.com/kro-run/kro/cmd/kro/commands/login" | ||
| validate "github.com/kro-run/kro/cmd/kro/commands/validate" |
There was a problem hiding this comment.
nit: import aliases not needed
There was a problem hiding this comment.
can we avoid updating top level go module? there is one for kro cmd now
| if err != nil { | ||
| return filepath.Join(".", ".kro", "registry", "config.json") | ||
| } | ||
| return filepath.Join(homeDir, ".config", "kro", "registry", "config.json") |
There was a problem hiding this comment.
@jakobmoellerdev thoughts on this? it's very similar to what helm does
| hashedPassword, err := bcrypt.GenerateFromPassword([]byte(password), bcrypt.DefaultCost) | ||
| if err != nil { | ||
| return fmt.Errorf("failed to hash password: %w", err) | ||
| } |
There was a problem hiding this comment.
how are you going to use a password if it's hashed? is it something standard?
k8s-ci-robot
added
the
needs-rebase
|
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. |
|
@DhairyaMajmudar: The following tests failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. 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. I understand the commands that are listed here. |
4 participants
Description
The kro cli login command is used to store user credentials (passwords, username) for different registers into the config file.
storage location:
$HOME/.config/kro/registry/config.jsonUsage
kro login -f [FILE] -r [REGISTRY_URL] -u [USER_NAME] -p [PASSWORD]Flags:
Note: If username or password is not provided via flags the cli will prompt the user to provide them in further steps.
Demo
Screencast.from.2025-08-31.20-16-24.webm