Rethink Cobra code structure to remove use of global variables

[phillebaba]

Describe what should be investigated or refactored

The getting started guide in the Cobra documentations demos the library by using the init() function to register the command and configure flags. Which has resulted in a lot of projects following this pattern for their own code. The subsequent examples in the documentation does not use this pattern.

Error propagation becomes a large challenge when using functions init() as errors cannot be returned and they are magically called when the module is loaded. Additionally testing can become a hassle. This has led to problems like #2756 because there is no easy path to deal with these problems. As the functions are called automatically there is no easy way to pass function parameters, which instead led to using global variables to pass critical information. This includes things like command flags, embedded file systems, and build flags. Using such a pattern creates loose coupling between where the data is set and consumed.

My suggestion is that we refactor the CLI code base a bit to remove any use of the init() function. This would mean following a pattern similar to what has been suggested in this issue.

https://github.com/spf13/cobra/issues/1041#issuecomment-1050938423

To make things clean we could have a single function for each subcommand/package which would then be responsible for calling and configuring commands downstream. This way we could pass any dependencies down from main to the subcommand which needs it.

Links to any relevant code

https://github.com/zarf-dev/zarf/blob/81f1c7085ea954fd0d8e1610bc516d2b342965e8/main.go#L43-L44

https://github.com/zarf-dev/zarf/blob/81f1c7085ea954fd0d8e1610bc516d2b342965e8/src/cmd/initialize.go#L179-L228

Additional context

N/A

[mkcp]

I wrote up a similar issue #3183 that ended up being a duplicate. Copying the context over to here, cause there's code links and an example in zarf say

Describe what should be investigated or refactored

Right now generate most of Zarf's CLI commands and flags inside of cmd pkg inits. While these inits() do have a reliable order, the order is not at all obvious when reading the code. We can make the code more maintainable by being explicit about when and how we generate commands and flags. Let's move this functionality into root and call the generate functions linearly, rather than in a bunch of init()s.

One pain point in this migration is that the logical commands in Zarf, e.g. zarf package ..., are each grouped by file names but are all in the cmd pkg. The many init functions help group and split these up, so we take that pain back on by moving them together. Taking additional time to move commands into their own subpackages, e.g. cmd/dev will help avoid overpopulating cmd with implementation details of specific commands.

Links to any relevant code

Connect: https://github.com/zarf-dev/zarf/blob/main/src/cmd/connect.go#L106-L116 Destroy: https://github.com/zarf-dev/zarf/blob/main/src/cmd/destroy.go#L106-L113 Dev: https://github.com/zarf-dev/zarf/blob/main/src/cmd/dev.go#L332-L376 Zarf init: https://github.com/zarf-dev/zarf/blob/main/src/cmd/initialize.go#L181-L230 Internal: https://github.com/zarf-dev/zarf/blob/main/src/cmd/internal.go#L370-L385 Package: https://github.com/zarf-dev/zarf/blob/main/src/cmd/package.go#L464-L485 Root: https://github.com/zarf-dev/zarf/blob/main/src/cmd/root.go#L151-L182 Version: https://github.com/zarf-dev/zarf/blob/main/src/cmd/version.go#L85-L88

Additional context

Zarf Say https://github.com/zarf-dev/zarf/blob/main/src/cmd/say/say.go is an example of how we'd contain the complexity of a specific command to one package, then declare that command explicitly in root.