Registry config command #2220
Registry config command #2220
Conversation
kyma-bot
added
cla: yes
| } | ||
|
|
||
| func runConfig(cfg *cfgConfig) clierror.Error { | ||
| registryConfig, err := registry.GetConfig(cfg.Ctx, cfg.KubeClient) |
There was a problem hiding this comment.
We can't use the registry.GetConfig func here because it returns secrets containing data used for local (in-cluster) connection, but users may expect config they can use for external connection.
| } | ||
|
|
||
| if cfg.externalurl { | ||
| fmt.Print(registryConfig.SecretData.PushRegAddr) |
There was a problem hiding this comment.
I would suggest using fmt.Println, or adding \n at the end of the format because when I've specified both flags the output looks weird:
./kyma-cli alpha registry config --dockerconfig --externalurl
Docker config saved to config.jsondockerregistry.kyma-system.svc.cluster.local:5000| } | ||
|
|
||
| cfg.KubeClientConfig.AddFlag(cmd) | ||
| cmd.Flags().BoolVar(&cfg.dockerconfig, "dockerconfig", false, "Generate a docker config.json file for the Kyma registry") |
There was a problem hiding this comment.
I would suggest removing this flag and dealing with its behavior as a default for the registry config command. Right now the registry config default is useless without passing additional flags which may be really confusing
|
|
||
| cfg.KubeClientConfig.AddFlag(cmd) | ||
| cmd.Flags().BoolVar(&cfg.dockerconfig, "dockerconfig", false, "Generate a docker config.json file for the Kyma registry") | ||
| cmd.Flags().BoolVar(&cfg.externalurl, "externalurl", false, "External URL for the Kyma registry") |
There was a problem hiding this comment.
I would suggest replacing the --externalurl flag with the --external-url one.
Also, the description is confusing because it does not describe what is causing this flag. Maybe this would be better:
Print docker registry external URL
There was a problem hiding this comment.
In general, I don't get the goal of this flag, because it's not cooperating with the --output flag and exactly the same information will be located in the config.json after running the registry config --dockerconfig command. Maybe we should remove this flag? or move it to a command command like registry external-up? or I don't know...
But this is only a suggestion. For sure this flag should be re-designed
Description
Changes proposed in this pull request:
Related issue(s)
#2217