Add package updating/creation tips to dev-docs

Signed-off-by: Daniel Weiße <dw@edgeless.systems>

cli/README.md

@@ -0,0 +1,121 @@
+# Constellation CLI design guide
+
+This document aims to outline the design principals behind the CLI code and packages.
+It should be consulted by developers when making changes to the code of packages in [`cli/internal/`](./internal/).
+
+## Basic layout
+
+[`cli/`](./) consists of 2 packages, [`cli/cmd/`](./cmd/), and [`cli/internal/`](./internal/), and a [`main.go`](./main.go), which serves as the entrypoint for the CLI.
+No new packages or files should be added to this base directory.
+
+[`cli/cmd/`](./cmd/) defines the root command of the CLI. Any changes concerning the root command, or additions like persistent flags (flags that can be accessed by all subcommands) should be added here.
+
+TODO: Discuss: Root command could be added to `cli/internal/cmd/`
+
+[`cli/internal/`](./internal/) holds the actual logic of the CLI, and will be discussed in detail in the following sections.
+The code is kept as internal packages to avoid accidentally exposing non public API logic of the CLI to other packages of Constellation.
+Any shared code should instead go into the global [`internal/` package](../internal/).
+
+## The internal package
+
+[`cli/internal/`](./internal/) implements logic of the CLI and is split into multiple distinct packages.
+Each package should aim to implement a specific functionality. Multi purposes packages should be avoided.
+
+The following packages are listed and explained in alphabetical order. New packages should be added here when they are created.
+
+### [`cloudcmd`](./internal/cloudcmd/)
+
+This package focuses on the interaction with the cloud provider.
+It separates the cloud provider specific code from the rest of the CLI, and
+provides a common interface for all cloud providers.
+Exported functions defined here should be cloud provider agnostic, meaning there should be no functions for specific CSPs.
+Rather the `cloudcmd` package should declare functions that take a `cloudprovier.Provider` as argument,
+perform CSP specific logic, and return a universally usable result.
+
+It is used by the [`cmd` package](#cmd) to handle creation of cloud resources and other CSP specific interactions.
+
+### [`clusterid`](./internal/clusterid)
+
+Defines the structure of the Constellation cluster ID file.
+Logic in this package should be kept minimal.
+
+### [`cmd`](./internal/cmd)
+
+Defines the commands of the CLI.
+Logic should be kept to input/output parsing whenever possible.
+Any more complex code should usually be implemented in one of the other CLI packages.
+
+The code here should be kept as cloud provider agnostic as possible.
+Any CSP specific tasks should be handled by [`cloudcmd`](#cloudcmd).
+
+All filepaths handled by the CLI code should originate from here.
+Common filepaths are defined as constants in [`internal/constants`](../internal/constants/).
+To generate workspace correct filepaths for printing, use the functions from [`workspace.go`](./internal/cmd/workspace.go) // TODO: Discuss if we should move this to its own package
+
+### [`featureset`](./internal/featureset)
+
+`featureset` defines feature gate constants for the CLI and should not implement any logic.
+
+### [`helm`](./internal/helm)
+
+Helm is used to manage Kubernetes deployments of a Constellation cluster.
+The package implements functionality to install, update, and manage Helm charts.
+
+The charts themselves are embedded in a built CLI binary, and values are dynamically updated depending on configuration.
+The charts can be found in [`cli/internal/helm/charts/`](./internal/helm/charts/).
+
+Helm logic should not be implemented outside this package.
+All values loading, parsing, installing, uninstalling, and updating of charts should be implemented here.
+As such, the number of exported functions should be kept minimal.
+
+### [`iamid`](./internal/iamid)
+
+Defines the structure of the IAM service account file.
+
+TODO: Discuss if this should be renamed.
+The cluster ID file was named as such because it contains the cluster ID.
+A similar thing is not the case for this file.
+
+TODO: Discuss if this should be moved.
+Having this as a standalone package seems slightly weird, since it contains just CSP specific information generated by Terraform.
+Additionally the content of this struct are never written as a file, the output simply printed to the user, or written to their config.
+Maybe restructuring this as a return value for the IAM cloud creator would be a better fit.
+
+### [`kubernetes`](./internal/kubernetes)
+
+For fetching status information updating resources in Kubernetes not directly managed by Helm,
+the CLI needs to interact with the Kubernetes API directly.
+This functionality is implemented in the `kubernetes` package.
+
+The package should be used for:
+
+* Fetching status information about the cluster
+* Creating, deleting, or migrating resources not managed by Helm
+
+The package should not be used for anything that doesn't just require the Kubernetes API.
+For example, Terraform and Helm actions should not be accessed by this package.
+
+### [`libvirt`](./internal/libvirt)
+
+Constellation uses libvirt to run clusters on local hardware.
+To enable easy deployment across different Linux distributions, the CLI can deploy libvirt inside a Docker container.
+The required files and Go code to start the container can be found in [`cli/internal/libvirt/`](./internal/libvirt/).
+
+The code in this package should be kept minimal, and likely won't need to be changed unless we do a major refactoring of our QEMU/libvirt installation.
+
+### [`terraform`](./internal/terraform)
+
+Terraform is used to create cloud resources and IAM resources required for Constellation clusters.
+The package implements functionality to create, update, delete, and view said resources.
+
+The Terraform files are embedded in a built CLI binary, and variables are dynamically update depending on configuration.
+The files can be found in [`cli/internal/terraform/terraform/`](./internal/terraform/terraform/).
+
+Functions of this package should be kept mostly cloud agnostic (There should be no "CreateAzureCluster" function),
+as loading the correct values and calling the correct functions for a given CSP is handled by the [`cloudcmd` package](#cloudcmd).
+
+### [`upgrade`](./internal/upgrade)
+
+This package implements some logic to upgrade resources for new Constellation releases.
+
+TODO: Remove this package, since it s functionality should be implemented by other packages.

dev-docs/conventions.md

@@ -98,6 +98,50 @@ pkgsite
 You can now view the documentation on <http://localhost:8080/github.com/edgelesssys/constellation/v2>
 </details>
 
+## Adding to a package
+
+Adding new functionality to a package is often required whenever new features for Constellation are implemented.
+
+Before adding to a package, ask yourself:
+
+* Does this feature implement functionality outside the scope of this package?
+  * Keep in mind the design goals of the package you are proposing to edit
+  * If yes, consider adding it to a different package, or [add a new one](#adding-new-packages)
+* Does another package already provide this functionality?
+  * If yes, use that package instead
+* Do other parts of Constellation (binaries, tools, etc.) also require this feature?
+  * If yes, consider adding it an existing, or create a new package, in the global [`internal`](../internal/) package instead.
+* Do other parts of Constellation already implement this feature?
+  * If yes, evaluate if you can reasonably move the functionality from that part of Constellation to the global [`internal`](../internal/) package to avoid code duplication.
+
+If the answer to all of the questions was "No", extend the package with your chosen functionality.
+
+Remember to:
+
+* Update the package description if the package received any major changes
+* Add unit tests
+
+## Adding new packages
+
+If you are implementing a feature you might find yourself adding code that does not share anything with the existing packages of the binary/tool you are working on.
+In that case you might need to add a new package.
+
+Before adding a new package, ask yourself:
+
+* Does your new package provide functionality that could reasonably be added to one of the existing packages?
+  * Keep in mind the design goals of the existing package: Don't add out of scope functionality to an existing package to avoid creating a new one.
+* Do other parts of Constellation (binaries, tools, etc.) also require this feature?
+  * If yes, consider adding the new package to the global [`internal`](../internal/) package.
+* Do other parts of Constellation already implement this feature?
+  * If yes, evaluate if you can reasonably move the functionality from that part of Constellation to the global [`internal`](../internal/) package to avoid code duplication.
+
+If the answer to all of the questions was "No', add the new package to the binary/tool you are working on.
+
+Remember to:
+
+* Add a description to your new package
+* Add unit tests
+
 ## CLI reference
 
 The command reference within the CLI should follow the following conventions: