Merge pull request #2256 from robpblake/ocm-9619-test-cli-structure

OCM-9619 | fix: Test expected ROSA CLI command structure and command Args to prevent regressions

.github/merge_request_template/Default.md

@@ -12,10 +12,8 @@
 
 ## PR Author Check List
 
-* [ ]  All commit messages adhere to the project [standard](https://github.com/openshift/rosa/blob/master/CONTRIBUTING.md) 
-
+* [ ]  All commit messages adhere to the project [standard](https://github.com/openshift/rosa/blob/master/CONTRIBUTING.md)
+* [ ]  All code aligns with the [Style Guide](../../CONTRIBUTING.md#style-guide)
 * [ ]  All commits are squashed into a single commit `git rebase -i HEAD~<number of commits>`
-
 * [ ]  All code changes have unit test coverage
-
 * [ ]  All changes have been tested locally and the steps documented in the `How to Test?` section

.gitignore

@@ -1,6 +1,6 @@
 .idea/
 .vscode/
-docs/
+/docs
 /rosa
 /rosa-darwin-amd64
 /rosa-darwin-amd64.sha256

CONTRIBUTING.md

@@ -83,15 +83,36 @@ configured in https://github.com/openshift/release repo.
 `.golangciversion` file is read by the `lint` job commands there:
 https://github.com/openshift/release/blob/master/ci-operator/config/openshift/rosa/openshift-rosa-master.yaml
 
-## Contributing and Error Handling in Cobra
+# Style Guide
 
-1. If you are contributing code, please ensure that you are handling errors
-   properly. Please use `Run: run` instead of `RunE: runE` when writing commands,
-   in order to stop the **usage info** being printed when an error is returned.
+## Adding a New Command
+
+### Add your Command to expected CLI Structure
+
+We automatically test the structure of the ROSA CLI to ensure commands and command flags are not accidentally added or removed.
+When you first create a new command, the test suite will fail because of this.
 
-## GitHub Workflows
+You need to add your command to the following file [command_structure](cmd/rosa/structure_test/command_structure.yml) in the correct
+location within the command tree in order for this test to pass.
 
-This repository also uses GitHub actions which are configured at `./github/workflows`
+You additionally need to create a directory under the [command_args](cmd/rosa/structure_test/command_args) sub-directory 
+and create a file called `command_args.yml`. This file should contain a simple yaml list of the `flags` supported by your command.
+For example, a command with flag `foo`, `bar`, `bob` would have the following `command_args.yml`:
+
+```yaml
+- name: foo
+- name: bar
+- name: bob
+```
+
+## Error Handling in Commands
+
+If you are contributing code, please ensure that you are handling errors properly. You should
+not call `os.Exit()` in your Command (there is a significant amount of this in our code which we
+are working to remove)
+
+Please use `Run: run` instead of `RunE: runE` when writing commands,
+   in order to stop the **usage info** being printed when an error is returned.
 
 ## Version-gating a feature
 

cmd/rosa/main_test.go

@@ -1,39 +1,101 @@
 package main
 
 import (
-	"fmt"
+	"os"
+	"path/filepath"
+	"strings"
 	"testing"
 
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	"github.com/spf13/cobra"
+
+	. "github.com/openshift/rosa/pkg/test"
 )
 
 func TestCommandStructure(t *testing.T) {
 	RegisterFailHandler(Fail)
 	RunSpecs(t, "rosa command structure")
 }
 
+const (
+	structureTestDirectory = "./structure_test"
+)
+
 var _ = Describe("ROSA Commands", func() {
 	It("Have all basic fields defined correctly", func() {
 		assertCommand(root)
 	})
+
+	It("Are correctly registered in the command structure", func() {
+		/*
+			Validates the command structure of the CLI at build time. New commands should be
+			added to command_structure.yml in order for this test to pass.
+		*/
+		structureVerifier := NewStructureVerifier(structureTestDirectory, root)
+		structureVerifier.AssertCommandStructure()
+	})
+
+	It("Have all command flags correctly registered", func() {
+		/*
+			Validates all command flags are in command_args.yml for each command. New flags should
+			be added to the correct command_args.yml file.
+		*/
+		assertCommandArgs(root)
+	})
+
+	XIt("Re-generates the command_arg directory structure and files", func() {
+		/*
+			This test can be used to regenerate the structure_test/command_args directory and files.
+			It should remain skipped for CI.
+		*/
+		generateCommandArgsFiles(root)
+	})
 })
 
+func assertCommandArgs(command *cobra.Command) {
+	if len(command.Commands()) == 0 {
+		verifier := NewArgVerifier(structureTestDirectory, command)
+		verifier.AssertCommandArgs()
+	} else {
+		for _, c := range command.Commands() {
+			assertCommandArgs(c)
+		}
+	}
+}
+
+func generateCommandArgsFiles(command *cobra.Command) {
+	cmdPath := filepath.Join(strings.Split(command.CommandPath(), " ")...)
+	dirPath := filepath.Join(structureTestDirectory, CommandArgDirectoryName, cmdPath)
+	_, err := os.Stat(dirPath)
+	if os.IsNotExist(err) {
+		Expect(os.MkdirAll(dirPath, 0600)).To(Succeed())
+	}
+
+	if len(command.Commands()) != 0 {
+		for _, c := range command.Commands() {
+			generateCommandArgsFiles(c)
+		}
+	} else {
+		generator := NewArgGenerator(filepath.Join(dirPath, CommandArgFileName), command)
+		generator.GenerateArgsFile()
+	}
+}
+
 func assertCommand(command *cobra.Command) {
-	Expect(command.Use).NotTo(BeNil(), fmt.Sprintf("Use cannot be nil on command '%s'", command.CommandPath()))
+	Expect(command.Use).NotTo(BeNil(), "Use cannot be nil on command '%s'", command.CommandPath())
 	Expect(command.Short).NotTo(
-		BeNil(), fmt.Sprintf("Short description is not set on command '%s'", command.CommandPath()))
+		BeNil(), "Short description is not set on command '%s'", command.CommandPath())
 	Expect(command.Long).NotTo(
-		BeNil(), fmt.Sprintf("Long description is not set on command '%s'", command.CommandPath()))
+		BeNil(), "Long description is not set on command '%s'", command.CommandPath())
 	Expect(command.Example).NotTo(
-		BeNil(), fmt.Sprintf("Example is not set on command '%s'", command.CommandPath()))
+		BeNil(), "Example is not set on command '%s'", command.CommandPath())
 	Expect(command.Args).NotTo(
-		BeNil(), fmt.Sprintf("command.Args function is not set on command '%s'", command.CommandPath()))
+		BeNil(), "command.Args function is not set on command '%s'", command.CommandPath())
 
 	if len(command.Commands()) == 0 {
 		Expect(command.Run).NotTo(
-			BeNil(), fmt.Sprintf("The run function is not defined on command '%s'", command.CommandPath()))
+			BeNil(), "The run function is not defined on command '%s'", command.CommandPath())
 	} else {
 		for _, c := range command.Commands() {
 			assertCommand(c)

cmd/rosa/structure_test/command_args/rosa/attach/policy/command_args.yml

@@ -0,0 +1,3 @@
+- name: mode
+- name: policy-arns
+- name: role-name

cmd/rosa/structure_test/command_args/rosa/completion/command_args.yml

@@ -0,0 +1 @@
+[]

cmd/rosa/structure_test/command_args/rosa/config/get/command_args.yml

@@ -0,0 +1 @@
+[]

cmd/rosa/structure_test/command_args/rosa/config/set/command_args.yml

@@ -0,0 +1 @@
+[]

cmd/rosa/structure_test/command_args/rosa/create/account-roles/command_args.yml

@@ -0,0 +1,15 @@
+- name: channel-group
+- name: classic
+- name: force-policy-creation
+- name: hosted-cp
+- name: interactive
+- name: managed-policies
+- name: mode
+- name: mp
+- name: path
+- name: permissions-boundary
+- name: prefix
+- name: profile
+- name: region
+- name: version
+- name: "yes"

cmd/rosa/structure_test/command_args/rosa/create/admin/command_args.yml

@@ -0,0 +1,6 @@
+- name: cluster
+- name: output
+- name: password
+- name: profile
+- name: region
+- name: "yes"

cmd/rosa/structure_test/command_args/rosa/create/autoscaler/command_args.yml

@@ -0,0 +1,25 @@
+- name: cluster
+- name: interactive
+- name: balance-similar-node-groups
+- name: skip-nodes-with-local-storage
+- name: log-verbosity
+- name: max-pod-grace-period
+- name: pod-priority-threshold
+- name: ignore-daemonsets-utilization
+- name: max-node-provision-time
+- name: balancing-ignored-labels
+- name: max-nodes-total
+- name: min-cores
+- name: max-cores
+- name: min-memory
+- name: max-memory
+- name: gpu-limit
+- name: scale-down-enabled
+- name: scale-down-unneeded-time
+- name: scale-down-utilization-threshold
+- name: scale-down-delay-after-add
+- name: scale-down-delay-after-delete
+- name: scale-down-delay-after-failure
+- name: profile
+- name: region
+- name: "yes"

cmd/rosa/structure_test/command_args/rosa/create/break-glass-credential/command_args.yml

@@ -0,0 +1,7 @@
+- name: cluster
+- name: expiration
+- name: interactive
+- name: profile
+- name: region
+- name: username
+- name: "yes"

cmd/rosa/structure_test/command_args/rosa/create/cluster/command_args.yml

@@ -0,0 +1,102 @@
+- name: name
+- name: cluster-name
+- name: domain-prefix
+- name: sts
+- name: non-sts
+- name: mint-mode
+- name: role-arn
+- name: external-id
+- name: support-role-arn
+- name: controlplane-iam-role-arn
+- name: master-iam-role
+- name: worker-iam-role-arn
+- name: operator-iam-roles
+- name: operator-roles-prefix
+- name: oidc-config-id
+- name: classic-oidc-config
+- name: external-auth-providers-enabled
+- name: tags
+- name: multi-az
+- name: region
+- name: version
+- name: channel-group
+- name: flavour
+- name: etcd-encryption
+- name: fips
+- name: http-proxy
+- name: https-proxy
+- name: no-proxy
+- name: additional-trust-bundle-file
+- name: additional-allowed-principals
+- name: enable-customer-managed-key
+- name: kms-key-arn
+- name: etcd-encryption-kms-arn
+- name: expiration-time
+- name: expiration
+- name: private-link
+- name: ec2-metadata-http-tokens
+- name: subnet-ids
+- name: availability-zones
+- name: compute-machine-type
+- name: compute-nodes
+- name: replicas
+- name: enable-autoscaling
+- name: autoscaler-balance-similar-node-groups
+- name: autoscaler-skip-nodes-with-local-storage
+- name: autoscaler-log-verbosity
+- name: autoscaler-max-pod-grace-period
+- name: autoscaler-pod-priority-threshold
+- name: autoscaler-ignore-daemonsets-utilization
+- name: autoscaler-max-node-provision-time
+- name: autoscaler-balancing-ignored-labels
+- name: autoscaler-max-nodes-total
+- name: autoscaler-min-cores
+- name: autoscaler-max-cores
+- name: autoscaler-min-memory
+- name: autoscaler-max-memory
+- name: autoscaler-gpu-limit
+- name: autoscaler-scale-down-enabled
+- name: autoscaler-scale-down-unneeded-time
+- name: autoscaler-scale-down-utilization-threshold
+- name: autoscaler-scale-down-delay-after-add
+- name: autoscaler-scale-down-delay-after-delete
+- name: autoscaler-scale-down-delay-after-failure
+- name: min-replicas
+- name: max-replicas
+- name: worker-mp-labels
+- name: network-type
+- name: machine-cidr
+- name: service-cidr
+- name: pod-cidr
+- name: host-prefix
+- name: private
+- name: disable-scp-checks
+- name: disable-workload-monitoring
+- name: watch
+- name: dry-run
+- name: fake-cluster
+- name: properties
+- name: use-local-credentials
+- name: permissions-boundary
+- name: hosted-cp
+- name: worker-disk-size
+- name: billing-account
+- name: create-admin-user
+- name: no-cni
+- name: cluster-admin-user
+- name: cluster-admin-password
+- name: audit-log-arn
+- name: default-ingress-route-selector
+- name: default-ingress-excluded-namespaces
+- name: default-ingress-wildcard-policy
+- name: default-ingress-namespace-ownership-policy
+- name: private-hosted-zone-id
+- name: shared-vpc-role-arn
+- name: base-domain
+- name: additional-compute-security-group-ids
+- name: additional-infra-security-group-ids
+- name: additional-control-plane-security-group-ids
+- name: mode
+- name: interactive
+- name: output
+- name: "yes"

cmd/rosa/structure_test/command_args/rosa/create/dns-domain/command_args.yml

@@ -0,0 +1,3 @@
+- name: profile
+- name: region
+- name: "yes"

cmd/rosa/structure_test/command_args/rosa/create/external-auth-provider/command_args.yml

@@ -0,0 +1,14 @@
+- name: claim-mapping-groups-claim
+- name: claim-mapping-username-claim
+- name: claim-validation-rule
+- name: cluster
+- name: console-client-id
+- name: console-client-secret
+- name: interactive
+- name: issuer-audiences
+- name: issuer-ca-file
+- name: issuer-url
+- name: name
+- name: profile
+- name: region
+- name: "yes"

cmd/rosa/structure_test/command_args/rosa/create/idp/command_args.yml

@@ -0,0 +1,34 @@
+- name: cluster
+- name: type
+- name: name
+- name: mapping-method
+- name: client-id
+- name: client-secret
+- name: ca
+- name: hostname
+- name: organizations
+- name: teams
+- name: host-url
+- name: hosted-domain
+- name: url
+- name: insecure
+- name: bind-dn
+- name: bind-password
+- name: id-attributes
+- name: username-attributes
+- name: name-attributes
+- name: email-attributes
+- name: issuer-url
+- name: email-claims
+- name: name-claims
+- name: username-claims
+- name: groups-claims
+- name: extra-scopes
+- name: username
+- name: password
+- name: users
+- name: from-file
+- name: interactive
+- name: profile
+- name: region
+- name: "yes"