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
| 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. |
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