🌱 deploy.sh to go code to fix gosec related issues

[maxrantil]

What this PR does / why we need it:
This PR transitions the functionality of the deploy.sh script into Go, aiming to fix gosec related issues. There is still one exception related to gosec that remains within the pipeCommands function:

OLD

A env var, KUBECTL_ARGS, introduces a potential security risk. The validateCmd function is currently the best approach to ador the option to choose kubeconfigdressing this issue

[metal3-io-bot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign zaneb for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[tuminoid]

A env var, KUBECTL_ARGS, introduces a potential security risk. The validateCmd function is currently the best approach to addressing this

What kind of legit arguments is passed there? Something like --foo --bar=3 ? If so, then you should be validating versus legit patterns like --\w and --\w=\w. It is very hard to try look for unsafe characters, there are so many ways around it, via UTF etc.

[maxrantil]

This is an example from metal3-dev-env/tests/roles/run_tests/vars/main.yml:
KUBECTL_ARGS: "--kubeconfig=/tmp/kubeconfig-{{ CLUSTER_NAME }}.yaml"

Also in baremetal-operator/docs/deploying.md is says:
- KUBECTL_ARGS : Additional arguments to kubectl apply

From my testing (setting up with dev-env) there has been nothing set to KUBECTL_ARGS

[tuminoid]

This is an example from metal3-dev-env/tests/roles/run_tests/vars/main.yml: KUBECTL_ARGS: "--kubeconfig=/tmp/kubeconfig-{{ CLUSTER_NAME }}.yaml"

Also in baremetal-operator/docs/deploying.md is says: - KUBECTL_ARGS : Additional arguments to kubectl apply

From my testing (setting up with dev-env) there has been nothing set to KUBECTL_ARGS

Then definitely it is worth doing the regex validation the way I suggested. There aren't many legit ways to format legit params.

[dtantsur]

Do you want to remove the old deploy.sh to avoid confusion?

[kashifest]

Do you want to remove the old deploy.sh to avoid confusion?

Thats the ultimate goal, for the time being they can coexist until we test the go code properly.

[maxrantil]

I'm currently updating the code to incorporate the k8s go-client. However, I'm unsure about the necessary options for KUBECTL_ARGS. Is specifying KUBECTL_ARGS="--kubeconfig=/path/to/custom-kubeconfig.yaml" sufficient, or should we support additional options? For example, should we include options like KUBECTL_ARGS="--namespace=my-namespace" and more? It is not that straight forward to add every possible option with k8s go-client.

[kashifest]

I'm currently updating the code to incorporate the k8s go-client. However, I'm unsure about the necessary options for KUBECTL_ARGS. Is specifying KUBECTL_ARGS="--kubeconfig=/path/to/custom-kubeconfig.yaml" sufficient, or should we support additional options? For example, should we include options like KUBECTL_ARGS="--namespace=my-namespace" and more? It is not that straight forward to add every possible option with k8s go-client.

Can you elaborate a bit more what do you mean my specifying options? Would you push the code so that we can check what is it that we may need?

[maxrantil]

So the solution I came up with for now is to use $KUBECLT_ARGS exclusively for selecting the kubeconfig. This is the priority and function I came up with:

// getKubeconfigPath determines the kubeconfig path from KUBECTL_ARGS, KUBECONFIG_PATH, or defaults to ~/.kube/config.
func getKubeconfigPath() string {
	kubectlArgs := os.Getenv("KUBECTL_ARGS")
	kubeconfigPath := ""

	kubeconfigPrefix := "--kubeconfig="
	if strings.Contains(kubectlArgs, kubeconfigPrefix) {
		if strings.Contains(kubectlArgs, " ") {
			log.Fatalf("Error: $KUBECTL_ARGS accepts only one argument.")
		}
		kubeconfigPath = strings.TrimPrefix(kubectlArgs, kubeconfigPrefix)
	} else if kubectlArgs != "" {
		log.Fatalf("Error: Invalid format in $KUBECTL_ARGS. Expected format: '--kubeconfig=/path/to/kubeconfig'.")
	}

	if kubeconfigPath == "" {
		kubeconfigPath = os.Getenv("KUBECONFIG_PATH")
		if kubeconfigPath == "" {
			homeDir, err := os.UserHomeDir()
			if err != nil {
				log.Fatalf("Failed to get user home directory: %v", err)
			}
			kubeconfigPath = filepath.Join(homeDir, ".kube", "config")
		}
	}

	return kubeconfigPath
}

[tuminoid]

Please check if you can use built-ins or de facto libraries instead of DIY. Also, logging a fatal and continuing is weird. If you can fall thru, then its not a fatal but a warning, or you error out if its really fatal.

[maxrantil]

Thanks @tuminoid for the review! Will look into these comments today

[maxrantil]

/test metal3-bmo-e2e-test-pull
/ test-centos-e2e-integration-main

[metal3-io-bot]

PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[metal3-io-bot]

@maxrantil: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
generate	e34ef0a	link	true	/test generate
unit	e34ef0a	link	true	/test unit
gomod	e34ef0a	link	true	/test gomod
manifestlint	e34ef0a	link	true	/test manifestlint

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

[lentzi90]

Closing this in favor of #1669
/close

[metal3-io-bot]

@lentzi90: Closed this PR.

In response to this:

Closing this in favor of #1669
/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.