GetPlatform enhancement to the CLI

[fgiorgetti]

Moving GetPlatform function to internal pkg and including os.Args to be evaluated.
This helps the CLI properly getting the platform when --platform flag is provided.

[nluaces]

I think this is not necessary to do here, I can fix it in the commands by getting the value of the flag.

[nluaces]

when configuring a command, the default is the value of GetPlatform, unless the platform flag was specified:

skupper/internal/cmd/skupper/common/command.go

Line 37 in 781927c

if cmd.Flag("platform") != nil && cmd.Flag("platform").Value.String() != "" {

[fgiorgetti]

when configuring a command, the default is the value of GetPlatform, unless the platform flag was specified:

skupper/internal/cmd/skupper/common/command.go

Line 37 in 781927c

if cmd.Flag("platform") != nil && cmd.Flag("platform").Value.String() != "" {

For flag validation, I think this is working.

But for the generic commands being added, like in the code below:

skupper/internal/cmd/skupper/site/site.go

Lines 27 to 30 in 781927c

	cmd.AddCommand(CmdSiteCreateFactory(config.GetPlatform()))
	cmd.AddCommand(CmdSiteUpdateFactory(config.GetPlatform()))
	cmd.AddCommand(CmdSiteStatusFactory(config.GetPlatform()))
	cmd.AddCommand(CmdSiteDeleteFactory(config.GetPlatform()))

The platform is coming straight from config.GetPlatform which is causing incorrect flags to show up as part
of the --help message when using non-kube platforms.

[nluaces]

CmdSiteCreateFactory is calling ConfigureCobraCommand that checks the value of the flag

https://github.com/skupperproject/skupper/blob/781927cf1dd432e5c839a5d0b5ed2e9efd597a74/internal/cmd/skupper/site/site.go#L47C15-L47C38

All the factories are calling ConfigureCobracommand that checks the platform flag.

[fgiorgetti]

CmdSiteCreateFactory is calling ConfigureCobraCommand that checks the value of the flag

https://github.com/skupperproject/skupper/blob/781927cf1dd432e5c839a5d0b5ed2e9efd597a74/internal/cmd/skupper/site/site.go#L47C15-L47C38

All the factories are calling ConfigureCobracommand that checks the platform flag.

Correct.

But when CmdSiteCreateFactory() is being called the configuredPlatform variable is set using config.GetPlatform().
So it is not yet including the value of --platform flag.

As an example, looking at the #1778 PR and building the skupper cli, then showing site create --help, I do not
see the non-kube flags (bind-host and subject-alternative-names):

$ make build-cmd 
GOOS=linux GOARCH=amd64 go build -ldflags="-s -w  -X github.com/skupperproject/skupper/pkg/version.Version=2.0.0-preview-1-46-g0ef2360d-modified"  -o skupper ./cmd/skupper
$ ./skupper --platform podman help site create
A site is a place where components of your application are running.
Sites are linked to form application networks.
There can be only one site definition per namespace.

Usage:
  skupper site create <name> [flags]

Flags:
      --enable-link-access        allow access for incoming links from remote sites (default: false)
  -h, --help                      help for create
      --link-access-type string   configure external access for links from remote sites.
                                  Choices: [route|loadbalancer]. Default: On OpenShift, route is the default; 
                                  for other Kubernetes flavors, loadbalancer is the default.
  -o, --output string             print resources to the console instead of submitting them to the Skupper controller. Choices: json, yaml
      --service-account string    the Kubernetes service account under which to run the Skupper controller
      --timeout duration          raise an error if the operation does not complete in the given period of time (expressed in seconds). (default 1m0s)

Global Flags:
  -c, --context string      Set the kubeconfig context
      --kubeconfig string   Path to the kubeconfig file to use
  -n, --namespace string    Set the namespace
  -p, --platform string     Set the platform type to use [kubernetes, podman, docker, systemd]

[fgiorgetti]

Maybe instead of passing config.GetPlatform() when adding commands, we should call a method that does both
things: parsing the --platform flag, then calling config.GetPlatform() next, leaving the latter as is. Thoughts?

[nluaces]

Now I understand your scenario, using --help flag in conjunction with --p flag. The cobra help function was not overridden, so I'm hiding commands not in execution but when defining and configuring the command, letting the --help flag not use the platform passed with the -p flag.

Let me check how to override the help function first.

[nluaces]

Using the value of the -p flag while executing --help would imply override all the help from all the commands. Which is not a good solution in comparison with this one. My bad!

(One of the main points of ConfigureCobraCommand was precisely to avoid parsing the -p flag as a string argument and instead use the cobra methods.)

[nluaces]

The shortcut -p is missing

[fgiorgetti]

Done.

[c-kruse]

👋 Once this is merged, I propose we delete pkg/config.

In cleaning up stray prometheus related stuff, I found that platform.go was the only used portion.

Also suspect that ConfigFileHandlerCommon and ConfigFileHandler may be unused.